Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPIP-270: Bitswap 1.3.0 - Tokens (and auth) support #270

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aschmahmann
Copy link
Contributor

This is a spec proposal continuing from #260 and other issues related to support for authentication, payments, etc. in Bitswap. This builds off of #269 which helps clean up the Bitswap spec a bit and bring it up to date.

The main thrust of the proposal is support for adding a set of tokens to requests and responses to allow metadata. These tokens can convey various application specific requirements or interactions

A bonus component added to the proposal is adding a BlockTooBig type of BlockPresence so that proper error messages can be communicated for that use case rather than either causing a stream reset or something more confusing like returning a Have, DontHave, or not responding at all.

This kind of change has been needed for a while, and while this may not resolve everything it seems like it will help us to have a concrete proposal to critique and improve.

cc some folks who have been asking about this for a while: @Stebalien @ianopolous @csuwildcat @obo20 @carsonfarmer @bmann


I'll give some background and pre-empt some questions here. If any of this is interesting enough to put in the spec we can do so there.

Unfortunately, GH is pretty bad at threaded discussions not tied to a particular piece of code so if you have a comment about something not explicitly in the code just grab some empty line in the PR and start a thread there.

Background

In Bitswap versions 1.2.0 and earlier it is not feasible for a server to decide on whether a peer is authorized to receive a particular block of data other than by discriminating on the peer itself. Consider if Alice wants to request a block B from Bob he will serve it to her if she is on his allowlist, and otherwise will not. If Alice wants to gain authorization to download B she can use some external protocol /authproto to negotiate her authorization for B with Bob.

This precludes a number of use cases including:

  • Multiple auth mechanisms per peer
    • Multitenancy: If Alice was a multitenancy client that was trying to download data from Bob on behalf of Charlie and Dan she doesn't have a good way to know which of Charlie or Dan are supposed to have access to B which means she could be handing over B to Dan even though he shouldn't have been given it in the first place. This might be very useful for semi-trusted intermediaries that are able to download data via Bitswap on behalf of users in more restrictive environments.
    • Per Retrieval Payments: Similar issues arise in the case of Alice and Bob trying to negotiate payments for the retrieval of chunks of two DAGs D1 and D2 both of which contain the block B. When Bob sends Alice B if payments occur after receipt then Alice doesn't know if Bob is expecting to be paid the D1 or the D2 payment rate for B. While probably Bob should just charge whichever is lesser, if the payment scheme is more complicated this might be non-obvious or generally determinable
  • Using auth as a fallback: i.e. trying to fetch data without auth before falling back to using auth
    • If Bob has B and Alice asks for it but is not yet authorized there is no way for Bob to communicate the difference between "I don't have the data" and "I have the data but you need to authorization" if he so chooses. This is particularly problematic in the case where the authorization is open (e.g. anyone who wants to pay for the data, anyone who solves a CAPTCHA, etc.) since then the options are to not believe any DONT_HAVEs and waste resources negotiating authorization or for Alice not be able to download data from any peer she didn't know in advance she needed to authenticate with

Alternatives/FAQs

  • Why not remove the AuthRequired token and instead put auth requirements in the content routing system?
    • Requires going to the content routing system frequently (potentially for every block) to figure out if authorization is required which defeats the important Bitswap optimization of skipping content routing by asking connected peers if they can send us blocks
    • Requires content routing systems to support the auth mechanisms which is a bigger ask than if it could be done at the exchange layer
  • Why support arbitrary token types?
    • Proposals like Bitswap auth #260 that do not differentiate different token types make it difficult for the application layer to know what to do with the tokens it received. It may have to do some testing on the tokens to figure out what they are for. It may also lead to ambiguity when a token could be used for two separate authentication schemes
    • IMO a generally useful piece of multiformats is that for the low cost of a few bytes you can figure out what your data is, that seems like something to take advantage of here
  • Why not dictate exactly what tokens can/should be used for?
    • Mostly because application developers will do whatever they want here anyway unless it explicitly breaks the protocol. We should give guidance, but any MUSTs here are likely pipe-dreams even if we wanted them
  • Why is there a level of indirection around tokens and token indices?
    • Basically to save bytes since we're not using something like gzip on the messages. It is expected that certain scenarios will have many blocks reuse the same tokens and we can save a lot of bytes there, while not being too costly in the one-token-per-block case

@ianopolous
Copy link
Member

ianopolous commented Mar 7, 2022

Hi @aschmahmann Thanks for making a start on this and doing the prior spec update!

I would argue that a byte[] is the most general you can get. In fact, for our usage in Peergos these byte[] are cbor. So we get the benefit of types, upgradability etc. but at the application layer and without the centralisation point of multicodecs. And this also doesn't force everyone to pay the overhead of the multicodec. This means applications can innovate faster and without requiring anyone's permission.

The other change I would suggest is to allow per cid auth tokens inline, as we do. Otherwise you're forcing someone like us to pay the extra overhead of the indirection (No auth tokens are ever shared between blocks for us, and arguably that would be a vulnerability (e.g. S3 doesn't allow this)). Probably there is a use case for both, especially if you include non capability-based schemes. It should be easy to support both inline tokens and indirect references.

Do you have a concrete use case in mind for allowing multiple auth tokens for a single cid in the same request? Our usage in Peergos is maximally fine grained and doesn't need this. For example, Amazon S3 doesn't need this and they clearly support almost all use cases in the world right now.

BITSWAP.md Outdated Show resolved Hide resolved
Copy link
Contributor Author

@aschmahmann aschmahmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thoughts so far, I gave some feedback but definitely room for improvement. I'm trying to use threads to make the conversation a bit more manageable, we'll see how it goes 😄.


### Tokens

The major change in this protocol version is the introduction of the ability to pass tokens along with requests and responses. A token is defined as `<multicode><data>` where the multicode is an identifier in the multicodec table, and the data is token-specific data associated with that code.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From @ianopolous

I would argue that a byte[] is the most general you can get. In fact, for our usage in Peergos these byte[] are cbor. So we get the benefit of types, upgradability etc. but at the application layer and without the centralisation point of multicodecs. And this also doesn't force everyone to pay the overhead of the multicodec. This means applications can innovate faster and without requiring anyone's permission.

I'll break this into pieces:

I would argue that a byte[] is the most general you can get.

Yes, although it comes with the risks associated with lack of explicit signaling. For example, if my application wants to support Peergos auth for some blocks and Foo for other blocks and both look like cbor then I have to waste time computing both. Even worse, Peergos and Foo might have similar enough formats as to not be distinguishable and therefore the application will have to try both auth mechanisms before responding.

without the centralisation point of multicodecs .... This means applications can innovate faster and without requiring anyone's permission.

Getting a code in the code table doesn't have to be hard and people can test with the reserved range. However, if we want this to be permissionless we could also resolve this by just adding some standard code that people can use. A strawman might be <the-code><varint><my-custom-value>, then the reservation of a code from the table is just a way for an application to save itself a few bytes.

For the cost of an extra couple bytes per token if you really didn't want to reserve a code, or use a string I guess you could reserve a code <yolo-code><bytes> and do acceptance testing on the tokens that'd also work. Not my recommendation, but doable.

And this also doesn't force everyone to pay the overhead of the multicodec.

I sort of feel for this, but also it's the cost of compatibility. IMO paying a couple of bytes per token is probably worth not having to waste the machine resources of guessing the token type. Does the few bytes really seem that expensive?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cbor is exactly as capable as multicodecs at signalling types. Just that in cbor it is optional and you can keep the logic outside of your serialization layer. This is exactly what we've done for years with all our custom IPLD types (things like our take on public encryption keys, public signing keys, champ nodes etc). We haven't bothered to register any multicodecs, because we don't need to, but they are all still typed, and upgradable. The signalling is still there, it's just on the application layer, rather than the bitswap layer. This keeps bitswap (/ipld) as simple and isolated from other concerns or dependencies as possible.

What do you mean by "waste time computing both"? An application can support many auth schemes, even if they all use cbor, so long as the type signature of each is distinguishable. The only "computation" here is trying to parse the token as cbor, which doesn't seem problematic. Compare this with S3 which does more work to check tokens by canonicalising the request headers and parameters before computing an hmac. If we decide we really need this type signalling on the protobuf layer, then we could make it optional in an extra field so not everyone has to pay it.

I guess I'm just saying we should leave the tokens opaque and let the application handle them (as in the earlier proposal ipfs/kubo#7871) which aligns with how we use it in Peergos. This keeps bitswap closer to being a narrow-waist of a protocol.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cbor is exactly as capable as multicodecs at signalling types. Just that in cbor it is optional and you can keep the logic outside of your serialization layer.

Not quite. CBOR is exactly as capable as multicodecs as signaling types if everyone uses the same semantics for signaling types and don't accidentally collide on namespaces. For example we could make the spec declare that tokens look roughly like: { type: <type identifier>, token : <token bytes> } and that would work fine as long as people didn't collide on type identifiers (in libp2p people avoid this with namespacing like /ipfs/bitswap/1.2.0 or /fil/data-transfer/1.0.0.

If we do it the raw bytes way then implementers have to write code like:

for validator in tokenValidators:
    if validator.matches(token) {
         return validator.IsValid(token, CID, peerID)
    }
return "no validator found"

If we do it the explicitly signaled way (whether using a table code, a namespaced string, etc.) we get something like:

if tokenValidators.Contains(token.signal) {
    tokenValidators[token.signal].IsValid(token, CID, peerID)
}

The former requires O(Validators) checks and the latter is O(1). Additionally, the latter can prevent accidental collisions e.g. the same bytes could belong to either TokenType1 or TokenType2 which could mean we have to check validator.IsValid twice, or we're in a weird situation where we've received a token that is accidentally valid which would be a security problem (admittedly this seems unlikely to be a serious problem, but it's easy enough to avoid)

If we decide we really need this type signalling on the protobuf layer, then we could make it optional in an extra field so not everyone has to pay it.

I think this misses the point. The reason to have the signaling is so that systems can have uncoordinated convergence (i.e. someone can choose to use Peergos' auth system if they want to without worrying about how it interacts with any other tokens they support and without requiring an IPFS spec or Peergos spec change).

Without some form of signaling every time I want to add support for a new token type I have to make sure everything still plays together nicely.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically, if you don't use a multicodec prefix, you either need to:

  1. Get everyone to agree on the same encoding scheme (cbor) and the same layout (e.g., some field to signal the "type").
  2. Do a bunch of duck-typing. I.e., try to decode as cbor, try to fit into the expected type, then pass to the next token handler, etc.

The first case just won't happen. That's significantly harder than getting everyone to prefix their tokens with a unique (per-protocol), fixed, prefix-free code.

In terms of the second case, yes, you can do this. But that leads to ambiguity that can be very difficult to reason about. In that case, I can't just register multiple token handlers, I need to think about how these different handlers might interact.

This is exactly multicodecs exist. It makes it really easy to remove ambiguity without having to get everyone to agree on a complex format, structure, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dignifiedquire #270 (comment)

so I am strongly in favor against requiring cbor.

IIUC no one wants to require cbor here. The discussion is around whether we should prefix with a multicode or use a duck-typing approach.

I was countering the argument in #270 (comment)

cbor is exactly as capable as multicodecs at signalling types

by saying that it's only as capable if everyone agrees to it (as Steven mentioned above) and in that case a varint prefix seems preferable to requiring cbor inside of the protobufs 😄.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you're both saying, but this ties a lot of high level application stuff, and a bunch of totally unrelated codecs that will never be used here, into the low level wire format. Thus making the bitswap protocol less of a thin waist protocol and increasingly a large waist with a bunch of constants from random protocols in it. I won't argue it anymore though, it's a stylistic choice basically. Both can function exactly the same performance and code wise (either agreeing on codec, or agreeing on a format and type signalling).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's still a pretty significant misunderstanding here.

I see what you're both saying, but this ties a lot of high level application stuff, and a bunch of totally unrelated codecs that will never be used here, into the low level wire format.

Bitswap already uses CIDs and multihashes, which are built on multicodecs.

Thus making the bitswap protocol less of a thin waist protocol and increasingly a large waist with a bunch of constants from random protocols in it.

Bitswap doesn't care about specific multicodecs, just that the token format is <multicodec> ++ data. It just cares that you prefix your token with a varint-encoded "thing".

it's a stylistic choice basically. Both can function exactly the same performance and code wise (either agreeing on codec, or agreeing on a format and type signalling).

It's a pretty important design decision.

  1. Just bytes. This is definitely the simplest, but it doesn't compose. I want to be able to have a single IPFS node integrate fetch blocks from multiple networks with multiple token types.
  2. Typed
    1. CBOR + type signaling
      1. Mandates CBOR (complex, not everyone will want to use it).
      2. Needs a new "table of token types".
    2. Multicodec
      1. Just enough format to unambiguously distinguish between different types. Applications can use whatever they want for the actual token format (JWT, etc.).
      2. Re-uses the table of types we already have.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. Minor point - it would almost certainly be broken security to send a JWT as the token here for the reasons I've mentioned elsewhere here.

BITSWAP.md Outdated Show resolved Hide resolved
- For non-deployed testing purposes only - use a code in the application reserved range of the code table
- Note: if codes in the application reserved range will not be reservable in the code table which means that the code may conflict with another one used in the ecosystem which could cause application problems on collision. It is high recommended to not use these codes outside of testing or early development.

To save space within the message the list of tokens used within the message are declared within the top level message and all other references to tokens are instead to the indices within the token list in the top level message.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From @ianopolous:

The other change I would suggest is to allow per cid auth tokens inline, as we do. Otherwise you're forcing someone like us to pay the extra overhead of the indirection (No auth tokens are ever shared between blocks for us, and arguably that would be a vulnerability (e.g. S3 doesn't allow this)). Probably there is a use case for both, especially if you include non capability-based schemes. It should be easy to support both inline tokens and indirect references.

We could add another field at each entry repeated tokens bytes in the Entry, Block and BlockPresence messages. It adds more complexity to save some bytes, I wonder if it's really that helpful. Another byte saving approach which would might save more bytes would be to store another list of the token-codes used in the token list. e.g. if your token code is > 3 bytes and you put it into the token list and then put your per-block tokens into the token list that would probably save you more bytes than this. Also, it seems like if we wanted to support strings/arbitrary bytes as custom "codes" then the above might save a whole bunch of data if we defined the codes as <code-number>[optional varint + data that goes with the code]<data> and stored the first two pieces in the second token list.

Both of these are about trying to shave a few bytes in exchange for some complexity. The initial proposal tried to cut down on the complexity unless the duplicated bytes were really excessive, finding which (if any) other complexity we want to add here though seems like a reasonable thing to discuss.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Logically the simplest proposal is to have an auth string per cid, but you rightly point out that that has a large overhead when an auth string is repeated. If we're bumping the bitswap version number then maybe it's worth considering just gzipping the message?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're bumping the bitswap version number then maybe it's worth considering just gzipping the message?

I think gzipping would be nice, although it'd be nicer if it could be done at the libp2p layer (e.g. think of how HTTP is allowed to negotiate compression schemes https://en.wikipedia.org/wiki/HTTP_compression). I'm not sure it's worth the complexity to require it in the protocol especially since Bitswap versions tend to hang around for a while which means if we want to change things people will still be supporting this for a long time.

If people feel strongly that we need gzipping though I could get on board with it

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It can be done at the libp2p layer.


Bitswap 1.3.0 extends the Bitswap 1.2.0 protocol with the following changes:
1. Having a list of tokens that may be sent with the message and referenced within the message
2. Allowing each entry in the wantlist to contain a set of tokens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From @ianopolous:

Do you have a concrete use case in mind for allowing multiple auth tokens for a single cid in the same request? Our usage in Peergos is maximally fine grained and doesn't need this. For example, Amazon S3 doesn't need this and they clearly support almost all use cases in the world right now.

A couple things come to mind:

  • Requiring two tokens from different systems: e.g. a Peergos token for authentication and some token related to paying for retrieval of that data
  • "Suggested" ACLs: The server wants to tell the client which token caused me to give them the block (e.g. for multitenancy auth or payment purposes), but I also want to tell the client some general ACL I'd like them to respect

Maybe it's overkill, but we're making it easier for people to do auth (e.g. they could've just used peerID auth and some offline channel to coordinate authentication and payments) and we should probably try to make systems composable in-band. Thoughts?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separating the auth token from a payment token is an interesting idea. Has anyone done anything like this (is your angle composability with filecoin?)? I would normally imagine you would just pay for access to the content. I.e. pay to get an auth token/key. But I'm just hypothesising here. I guess what you're saying is different because you're not paying for access, you're paying for delivery, which should be independent.

Note that in our multi-tenant case, the client already knows which token was authed to get a block without needing this.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use cases I have talked to folks about:

  • ACLs without payments
  • ACLs with payments
  • request limiter, e.g. this token allows you to retrieve 100 times

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, one absolutely critical property of any usage of this is that auth tokens MUST be tied to the sender's nodeId and this must be verified before releasing a block. Otherwise you are trivially vulnerable to replay attacks because you broadcast the tokens to the DHT. I think it would be worthwhile to flesh out a few of these use cases to make sure it is possible to do securely. E.g. a naive approach of using this to do an ACL by having tokens that the recipient looks up in a db to check that a token is allowed for a cid is insecure. To make it secure the db would also need to store the nodeId's allowed to use a given token, but then it wouldn't need this extension at all or any auth tokens, just use the nodeid of the requestor in normal bitswap.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The more I think about trying to use this to do ACLs (as opposed to capabilities) the more I think it is impossible to do securely because of the broadcast nature of bitswap, except in the trivial case where the ACL only depends on the nodeId. In that case you don't need this PR at all and you should just use a custom allow function as suggested here and as we've implemented here.

Fundamentally, it comes down to ACLs depending on identity and the only identity between ipfs nodes is the node id. So the ACL will only be able to depend on the node id, in which case why not just use the node id which is already authenticated and unfakeable without this PR.

There is one way around this though, and that is single-use (not N>1) tokens. If a token is enforced to be only used once, then clearly a replay attack can't occur. That sounds like a massive footgun to me and also a scalabilty and centralisation problem as well.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scratch my last statement about single use tokens, because whilst true, there is also a race condition between the originator and anyone else in the DHT they send the token to. So yes that is a footgun.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, one absolutely critical property of any usage of this is that auth tokens MUST be tied to the sender's nodeId and this must be verified before releasing a block. Otherwise you are trivially vulnerable to replay attacks because you broadcast the tokens to the DHT.

Bitswap is a data transfer (and signaling) protocol that's independent of not just the IPFS Public DHT but any other content routing mechanism. If an application chooses to leverage secret tokens there's nothing to be done other than to make sure the tokens are only sent to the correct peer(s) since as you pointed out broadcasting a secret is bad news no matter how you do it.

For example, go-ipfs having a command like ipfs get bafyfoobar --token=<some-secret-token> would be a footgun that IMO shouldn't be implemented. On the other hand, there appear to be a variety of folks who have asked in ipfs/kubo#7871 and related issues to be able to do the equivalent of sending HTTP request headers along with a request. To take advantage of that protocol feature they'd have to write their own clients + servers though.

The nice thing is that despite them having to write their own clients and servers for their custom token authenticated requests: 1) they can do non-token authenticated requests with everyone else 2) Bitswap client/server implementers can choose to support whichever schemes start to take off without requiring a spec change and convincing people the next version of Bitswap should support their scheme

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitswap doesn't have a mechanism to only send wants to "the correct" peers. From a security perspective Wants are broadcast to the world. (If I want to receive an auth token for a given cid then I just need to pretend to the DHT that I have that cid and then the requests will come in with valid tokens). This is super important, and will trivially break security for most things people will try and do with it.

It is possible to do securely with the broadcast assumption, which is what we've figured out in Peergos. A simplified analogy would be that if a secret S allows access to cid C. Then if node A is requesting C then the token sent with bitswap could be some form of signature using S of (A, C). Then the recipient can verify that the signature is valid, and the request came from A. Then there is no possibility for replay attacks because the signature is tied to the sender's node id.

So I highly recommend speccing out some of these proposed usages and checking they are not totally broken security-wise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bitswap doesn't have a mechanism to only send wants to "the correct" peers

I don't understand this. Bitswap is a message sending protocol between two peers.

From a security perspective Wants are broadcast to the world.

This is only the case in go-bitswap, the decision as to which peers to send messages to is up to the implementation. Is there somewhere in the spec that suggests that wants are globally broadcast?

Security aside implementations may not want to broadcast to everyone simply as a way of cutting down on spamming and bandwidth usage. For example, even in the non-auth case I could see an implementation making the argument to only ask peers it's connected to over a LAN or where they were recommended by the content routing system.

So I highly recommend speccing out some of these proposed usages and checking they are not totally broken security-wise.

A couple that are more suitable for open networks have been described (e.g. bats and payments), but doing simple bearer token style auth seems very easy as well. It just means that writing some CLI command like fetch-cid <cid> <bearer-token> is meaningless and instead you'd need to specify fetch-cid <cid> <multiaddr> [bearer-token] or fetch-cid <cid> [tuple of mulitaddr and bearer token].

I'll try and reach out to some of the folks that have requested this functionality in the past for comments since AFAIK none of the people who have commented on this PR are planning on implementing support for something closer to a bearer-token style which wouldn't be appropriate for broadcast.

However, I'm confused as to what this has to do with the spec proposal. Even bats + payments require a spec change that looks like this though, right?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is only the case in go-bitswap

To my knowledge, go-bitswap is the main implementation of bitswap, which is used by go-ipfs, the main implementation of ipfs. Are you suggesting that existing ipfs apis wouldn't use this at all? If only new apis like you suggest will use this then that's much easier to avoid footguns.

@BigLep BigLep added this to the go-ipfs 0.13 milestone Mar 7, 2022
BITSWAP.md Outdated
message BlockPresence {
bytes cid = 1;
BlockPresenceType type = 2;
repeated tokens int32 = 3; // the indices of the tokens in the token list
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any implications on the extensibility of the protobuf with a repeated field? My guess is the field numbers make this still extensible?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK everything is fine with extensibility and we have other repeated fields in the Bitswap protobuf.

However, perhaps there's an argument to define it as packed since it's an int32 I don't have a strong opinion about this though.

@ianopolous
Copy link
Member

ianopolous commented Mar 8, 2022

One other nice to have is to maintain backwards compatibility with our prior art coming up with a secure capability based auth scheme for Peergos and the associated bitswap extension. We have just extended the protobuf for bitswap 1.2, and this PR is making a new protocol id 1.3. However at the moment all versions of bitswap share the same protobuf. This is a nice simplification to not have to maintain code around N different versions of the protobuf. I think the simplest thing is just to avoid the field indices we've used in #260 ? Or even better, extend our protobuf which only adds optional auth strings. This means users can choose between the indirect list of auth tokens, or an inline one.


Bitswap 1.3.0 extends the Bitswap 1.2.0 protocol with the following changes:
1. Having a list of tokens that may be sent with the message and referenced within the message
2. Allowing each entry in the wantlist to contain a set of tokens
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To be clear, one absolutely critical property of any usage of this is that auth tokens MUST be tied to the sender's nodeId and this must be verified before releasing a block. Otherwise you are trivially vulnerable to replay attacks because you broadcast the tokens to the DHT.

Bitswap is a data transfer (and signaling) protocol that's independent of not just the IPFS Public DHT but any other content routing mechanism. If an application chooses to leverage secret tokens there's nothing to be done other than to make sure the tokens are only sent to the correct peer(s) since as you pointed out broadcasting a secret is bad news no matter how you do it.

For example, go-ipfs having a command like ipfs get bafyfoobar --token=<some-secret-token> would be a footgun that IMO shouldn't be implemented. On the other hand, there appear to be a variety of folks who have asked in ipfs/kubo#7871 and related issues to be able to do the equivalent of sending HTTP request headers along with a request. To take advantage of that protocol feature they'd have to write their own clients + servers though.

The nice thing is that despite them having to write their own clients and servers for their custom token authenticated requests: 1) they can do non-token authenticated requests with everyone else 2) Bitswap client/server implementers can choose to support whichever schemes start to take off without requiring a spec change and convincing people the next version of Bitswap should support their scheme


### Tokens

The major change in this protocol version is the introduction of the ability to pass tokens along with requests and responses. A token is defined as `<multicode><data>` where the multicode is an identifier in the multicodec table, and the data is token-specific data associated with that code.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cbor is exactly as capable as multicodecs at signalling types. Just that in cbor it is optional and you can keep the logic outside of your serialization layer.

Not quite. CBOR is exactly as capable as multicodecs as signaling types if everyone uses the same semantics for signaling types and don't accidentally collide on namespaces. For example we could make the spec declare that tokens look roughly like: { type: <type identifier>, token : <token bytes> } and that would work fine as long as people didn't collide on type identifiers (in libp2p people avoid this with namespacing like /ipfs/bitswap/1.2.0 or /fil/data-transfer/1.0.0.

If we do it the raw bytes way then implementers have to write code like:

for validator in tokenValidators:
    if validator.matches(token) {
         return validator.IsValid(token, CID, peerID)
    }
return "no validator found"

If we do it the explicitly signaled way (whether using a table code, a namespaced string, etc.) we get something like:

if tokenValidators.Contains(token.signal) {
    tokenValidators[token.signal].IsValid(token, CID, peerID)
}

The former requires O(Validators) checks and the latter is O(1). Additionally, the latter can prevent accidental collisions e.g. the same bytes could belong to either TokenType1 or TokenType2 which could mean we have to check validator.IsValid twice, or we're in a weird situation where we've received a token that is accidentally valid which would be a security problem (admittedly this seems unlikely to be a serious problem, but it's easy enough to avoid)

If we decide we really need this type signalling on the protobuf layer, then we could make it optional in an extra field so not everyone has to pay it.

I think this misses the point. The reason to have the signaling is so that systems can have uncoordinated convergence (i.e. someone can choose to use Peergos' auth system if they want to without worrying about how it interacts with any other tokens they support and without requiring an IPFS spec or Peergos spec change).

Without some form of signaling every time I want to add support for a new token type I have to make sure everything still plays together nicely.

- For non-deployed testing purposes only - use a code in the application reserved range of the code table
- Note: if codes in the application reserved range will not be reservable in the code table which means that the code may conflict with another one used in the ecosystem which could cause application problems on collision. It is high recommended to not use these codes outside of testing or early development.

To save space within the message the list of tokens used within the message are declared within the top level message and all other references to tokens are instead to the indices within the token list in the top level message.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we're bumping the bitswap version number then maybe it's worth considering just gzipping the message?

I think gzipping would be nice, although it'd be nicer if it could be done at the libp2p layer (e.g. think of how HTTP is allowed to negotiate compression schemes https://en.wikipedia.org/wiki/HTTP_compression). I'm not sure it's worth the complexity to require it in the protocol especially since Bitswap versions tend to hang around for a while which means if we want to change things people will still be supporting this for a long time.

If people feel strongly that we need gzipping though I could get on board with it

BITSWAP.md Outdated
message BlockPresence {
bytes cid = 1;
BlockPresenceType type = 2;
repeated tokens int32 = 3; // the indices of the tokens in the token list
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AFAIK everything is fine with extensibility and we have other repeated fields in the Bitswap protobuf.

However, perhaps there's an argument to define it as packed since it's an int32 I don't have a strong opinion about this though.

BITSWAP.md Outdated Show resolved Hide resolved
@dignifiedquire
Copy link
Member

I think gzipping would be nice, although it'd be nicer if it could be done at the libp2p layer

strong agree, we should push for adding compression in libp2p, not try to add it manually to protocols

@dignifiedquire
Copy link
Member

Without some form of signaling every time I want to add support for a new token type I have to make sure everything still plays together nicely.

I would argue you have to do that anyway, if you want the systems to actually attribute real meaning, just having cbor in there will not help that much in practice, as the semantic meaning is much more critical than the encoding format.

In general, I expect not to be using cbor at all, especially in protocols that are based on protobufs, so adding a requirement in here for cbor, would mean requiring another encoding format ontop of the one this is wrapped in, so I am strongly in favor against requiring cbor.

@matheus23
Copy link

Hi 👋 I'm part of the fission team. This is an interesting proposal for us, since it'd technically make using UCANs possible with bitswap directly.

Thoughts:

  • Multiple tokens is a 👍 from me. Technically you could combine tokens into your own custom list format inside the byte array, but in combination with the multicodec proposal I think this way has some advantages, e.g. if tokens get forwarded into the DHT, different nodes may want to pick up different subsets of tokens, and we'd want them to agree on the format for "finding the subset".
  • Multicodecs for tokens is also a 👍. Multicodecs are the most minimal signalling protocol I know and they're used throughout the IPFS stack. I think the biggest value of this is forcing applications to use some signalling in the most minimal way. The worst possible situation would be allowing any bytes and someone ending up using a token where all byte sequences have a valid meaning.
  • I'm undecided on whether to remove the "custom compression" and just go gzip on another level. I'm generally in favor of simplifying protocols, however in this case I think the complexity is really low and it's going to be a very common case of re-using tokens for multiple CIDs, if not the very default case. The repeated bytes will have to be represented at some point in memory, even if we're streaming them into gzip, the stream will need to repeat the token. Just wanted to note this, it may still be premature optimization, as I said, I'm not quite decided on that yet.

In any case, I can see this making UCAN use-cases possible with bitswap which is exciting for fission.

@ianopolous
Copy link
Member

@matheus23 Just a warning that using a UCAN in the token field here would also be totally insecure (in normal ipfs bitswap usage) as per the discussion above unless the audience of the ucan is the node id of the requesting ipfs node AND this is verified on the receiving end against the actual requestor, in addition to the usual ucan checks.

@matheus23
Copy link

Just a warning that using a UCAN in the token field here would also be totally insecure (in normal ipfs bitswap usage) as per the discussion above unless the audience of the ucan is the node id of the requesting ipfs node AND this is verified on the receiving end against the actual requestor, in addition to the usual ucan checks.

Thanks for making that clear. We're aware of that. 👍
Sorry, I'm trying to read between the lines here... Would you expect my stance to change given that information?

@aschmahmann
Copy link
Contributor Author

Status Update:

Long overdue status update from the implementers sync (https://pl-strflt.notion.site/IPFS-Implementers-Sync-2022-05-12-b338b20567904923b3772c58f1c9186c) here is that this spec PR needs to be updated but should otherwise be reasonably solid. IIUC the remaining points were 1) smaller fixes to text in the spec 2) changing the protobuf numbers to skip the ones used by Peergos.


Ideally I'd like us to have two users of this work to help prove out the spec change's utility. Peergos is one use case that's also not blocked since they currently have an alternative mechanism riding on top of Bitswap 1.2. I was previously leaning on some of the folks working on payments to provide that second user story, but the folks working on Boost have delayed that work.

@matheus23 is this something Fission is interested in working on/using? From the Bitswap implementations I've looked at the server side changes to support tokens are fairly minimal, but the client side changes (as Ian has mentioned) require more work to fit the use case and consider how clients figure out which peers to send a given token to.

@expede
Copy link

expede commented Jun 20, 2022

Otherwise you are trivially vulnerable to replay attacks because you broadcast the tokens to the DHT

We've done some work with UCAN to avoid replays, too 👍 The most general solution is to enforce token uniqueness (via a nonce), so that replays are impossible (under normal assumptions: the protocol is adhered to, etc).

unless the audience of the ucan is the node id of the requesting ipfs node AND this is verified on the receiving end against the actual requestor,

@ianopolous I think some of the roles are swapped, but yes that's how UCANs must be used in all circumstances. The issuers and audience need to form a chain all the way back to the original node, including the node that is fulfilling the request.

For what it's worth, putting a UCAN right in a bitswap header probably doesn't make sense due to its size. As alluded to earlier, in point-to-point sessions, it's possible to do a handshake with a "blank"/self-targetted UCAN, and then send the triple aud, ucan_cid, nonce to shrink the size of the payload. We've also started playing around with the idea of a hash OTP, which seems to work well.

@expede
Copy link

expede commented Jun 20, 2022

cc some folks who have been asking about this for a while

Maybe to clarify a bit: the Fission team definitely welcomes this as a privacy measure, and I can see this potentially enabling more generic protocols for interacting with service providers, creating ad hoc private networks, etc. Given the often long lead times for changes in libp2p to get into projects like IPFS, working on this now seems like a good idea 💯

That said, Fission doesn't have a burning need for these features today. I like the idea conceptually, but we probably won't go out of our way to use it until it's more baked in.

I also wonder if it would be possible to move this kind of a spec up a level from Bitswap? We're starting to explore alternatives to Bitswap, and having a generic authenticated session in libp2p is attractive.

@ianopolous
Copy link
Member

Otherwise you are trivially vulnerable to replay attacks because you broadcast the tokens to the DHT

We've done some work with UCAN to avoid replays, too +1 The most general solution is to enforce token uniqueness (via a nonce), so that replays are impossible (under normal assumptions: the protocol is adhered to, etc).

@expede Unfortunately, this is still broken by what I mentioned above - it reduces the replay to a (trivially exploitable) race condition. Nonces don't make it secure.

unless the audience of the ucan is the node id of the requesting ipfs node AND this is verified on the receiving end against the actual requestor,

@ianopolous I think some of the roles are swapped, but yes that's how UCANs must be used in all circumstances. The issuers and audience need to form a chain all the way back to the original node, including the node that is fulfilling the request.

Based on my reading of the ucan spec the terms are correct. What I'm saying is in addition to a being a normal well formed ucan.

@aschmahmann
Copy link
Contributor Author

aschmahmann commented Jun 21, 2022

That said, Fission doesn't have a burning need for these features today. I like the idea conceptually, but we probably won't go out of our way to use it until it's more baked in.

Could you clarify what you mean by "more baked in"? I want to double check that you're not waiting on go-ipfs (now kubo) to implement something like ipfs get bafyfoobar --token=<some-token> since that's not something that's planned to be implemented for a combination of the UX and security reasons that have been mentioned above. However, in the case of other implementations and applications, supporting tokens like this makes things like downloading data authenticated with tokens much easier (#270 (comment)).

I also wonder if it would be possible to move this kind of a spec up a level from Bitswap? We're starting to explore alternatives to Bitswap, and having a generic authenticated session in libp2p is attractive.

Having a generic authenticated session in libp2p seems like an interesting idea, although I don't think that solves our problems here since now if there are multiple possible tokens to use to download a given block of data (or multiple tokens that must be used together) life becomes much more complicated.

If you have a proposal in mind that seems like a great IPIP (#289), or a libp2p spec proposal. As you may have seen, the suggestion for a compression libp2p protocol that other protocols could layer on top of has come up which seems like it would need similar mechanisms, and is an area I think would be pretty cool for people to explore. However, it both seems like more work than this proposal and so far seems like it'd be less efficient as well so I think this proposal as-is currently makes more sense.

@expede
Copy link

expede commented Jun 21, 2022

it reduces the replay to a (trivially exploitable) race condition

To be clear, one absolutely critical property of any usage of this is that auth tokens MUST be tied to the sender's nodeId and this must be verified before releasing a block.

@ianopolous you're going to have to walk me through that race condition. They can't connect as a node in the DID chain. This is an extremely limited number of peers, all of who are granted access by transitive closure. It ties them to the nodeId in the chain.

@expede
Copy link

expede commented Jun 21, 2022

I want to double check that you're not waiting on go-ipfs (now ipfs/kubo#8959 (comment)) to implement something like ipfs get bafyfoobar --token= since that's not something that's planned to be implemented for a combination of the UX and security reasons that have been mentioned above. However, in the case of other implementations and applications, supporting tokens like this makes things like downloading data authenticated with tokens much easier (#270 (comment)).

@aschmahmann yup, that's kind of the idea. I was just downplaying my own comments as possibly not being relevant since if it doesn't land in Kubo or Iroh, we likely won't use this feature in the near-to-medium term, but I like the general concept. Boris was pinged on the issue, so we wanted to make sure that Fission weighed in!

We've had some inbound interest in using UCAN for these kinds of use cases, so there's some experimentation happening with that at other layers. They're probably too big to go with every request, without some extra protocol on top to reduce that size in a session.

@ianopolous
Copy link
Member

@ianopolous you're going to have to walk me through that race condition. They can't connect as a node in the DID chain. This is an extremely limited number of peers, all of who are granted access by transitive closure. It ties them to the nodeId in the chain.

@expede We can go through it in person at the IPFS workshop. I suspect you have an additional assumption or two on top of the ucan spec that I'm not assuming.

4. If C asked for a block that S has but it is bigger than the maximum block size it should return `BlockTooBig`
3. When C no longer needs a block it previously asked for it should send a Cancel message for that request to any peers that have not already responded about that particular block. It should particularly send Cancel messages for Block requests (as opposed to Have requests) that have not yet been answered.

### Tokens
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIUC, "tokens" are not auth-specific, and are really just arbitrary key-value metadata pairs which are useful for passing auth tokens, among other things. If that's the case, would it make sense to call this something other than "token"? Generally "token" is used in auth contexts, and so its use here implies that it's intended only for auth, which isn't really the case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that while it can be more general, token isn't a particularly bad name for this.

@willscott
Copy link
Contributor

It would be great to get a summary of what needs to happen to move this from draft to ready for review, since this has been sitting for a few months now and we know there are several implementation interested in making use of this mechanism.

@aschmahmann aschmahmann force-pushed the feat/bitswap-spec-improvement branch 2 times, most recently from 231112f to 906da55 Compare August 25, 2022 19:03
Base automatically changed from feat/bitswap-spec-improvement to main August 26, 2022 14:25
@aschmahmann
Copy link
Contributor Author

It would be great to get a summary of what needs to happen to move this from draft to ready for review

See #270 (comment).

I've fixed up the small changes in the spec, but we're still lacking multiple groups planning to implement and utilize this behavior so that we can make sure that the abstraction here is sufficiently usable for folks. Maybe it's missing something or will be a pain to work with on the implementation, especially on the client side.

As has been noted multiple times in this PR depending on what types of tokens you're planning on passing along things implementations might need to deviate from how they work today, particularly on the client side. IMO merging a spec change to a core protocol while it's not being used is not responsible.

and we know there are several implementation interested in making use of this mechanism.

While there have been multiple groups asking for this functionality in the past, AFAIK Peergos is the only group that has expressed an interest in actually building out the pieces they need to use this and IIUC they're currently fine using their fork of Bitswap 1.2.0 which does enough of the job for them but doesn't really support any other use cases.

Some of the other groups that initially wanted this (e.g. for wanting to support paid retrieval of data served over Bitswap, limiting serving data using some permissioning schemes similar to bearer tokens or UCANs, etc.) have since decided that their need is not yet high enough for them to want to prioritized implementing the changes. If this has changed and we have an additional group or two that want to implement and utilize these changes then it'd be great to see how the implementations go so we can either merge the spec as is, or make any changes that end up being needed for implementers.

@BigLep BigLep removed this from the go-ipfs 0.13 milestone Sep 1, 2022
@lidel lidel changed the title Bitswap 1.3.0 - Tokens (and auth) support IPIP-270: Bitswap 1.3.0 - Tokens (and auth) support Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs Triage
Development

Successfully merging this pull request may close these issues.

Bitswap auth
9 participants