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

Graduate pubsub from being experimental #5383

Closed
whyrusleeping opened this issue Aug 15, 2018 · 16 comments
Closed

Graduate pubsub from being experimental #5383

whyrusleeping opened this issue Aug 15, 2018 · 16 comments

Comments

@whyrusleeping
Copy link
Member

It's probably about time to graduate pubsub out of the realm of just being an experiment.

We should still keep ipns pubsub as an experimental flag, but i think the default floodsub stuff should be enabled.

To do this, let's first have a separate pubsub doc in the docs folder that contains an expanded guide on using pubsub (beyond whats in the experimental features doc). And then just set the flags default value to true.

If the user isnt using any pubsub stuff, there shouldnt be any extra activity going on as a result.

@Stebalien
Copy link
Member

So, I have two major issues with the current system:

  1. Unauthenticated "from" field in messages.
  2. Unauthenticated sequence numbers.

Basically, it's embarrassingly insecure in ways that can't be fixed at the application layer (because users can be DoSed at the protocol layer). The DoS here is that an attacker can easily send messages "from" other peers to hide their messages.

I can see two ways forward.

Content addressed

We can replace sequence numbers/from with a hash of the message (plus maybe timeout?). Users would have to put something new into a message to make it a "new" message.

We could also just take the from, data, and seqno and hash them together to get the "ID". However, the fact that the from isn't authenticated still bugs me. This would be a non-breaking way to fix floodsub but, unfortunately, it'd still break gossipsub.

Sign them

We could authenticate these messages out of the box. Honestly, that's probably what most users would prefer. However, key distribution becomes a bit painful. When receiving a message, the user may have to go back to the sender and ask for the key.

Downsides:

  • This would be a completely breaking change.
  • Asking for the key could get complicated/slow (for bootstrapping). We could also include the key (or do some kind of optimistic send) but that's also tricky.
  • Some systems may not care about authorship. Content addressing + validators allows this kind of thing to be pushed to a higher layer.

Upside:

  • Many systems will care about authorship and applications will likely find authenticated messages useful. However, we can always provide a pubsub + auth layer to do this...

Personally, I'd prefer to just make pubsub2 (fork the code and protocol) and say "it's experimental".

We could then:

  1. Use IPLD for messages.
  2. Use CIDs for "message IDs".
  3. Do validation at a higher layer.

@whyrusleeping
Copy link
Member Author

We should just implement signing as planned. Also remember EC keys sidesteps the key distribution problem. For RSA keys we could just force them to be included in the message. This would motivate users to upgrade to ed25519 asap

@Stebalien
Copy link
Member

Signing as planned works for me (but it will break existing clients).

@Stebalien
Copy link
Member

@vyzo want to take a crack at adding signatures?

@vyzo
Copy link
Contributor

vyzo commented Aug 16, 2018

Sure! We really want those EC keys though, 2-4kbit overhead per message is quite a bit for RSA keys.

@vyzo
Copy link
Contributor

vyzo commented Aug 16, 2018

We also have a deployment/compatibility problem with enforcing signatures.
We can't blanket enforce signatures at one step, it will break existing nodes and be incompatible with them.
Instead, we will have to introduce signatures gradually -- initially sign messages but have optional strict validation, and after signing has been deployed only then make strict validation the default.

@Stebalien
Copy link
Member

Sure, but given that this is experimental, I'd say that one release is enough.

@whyrusleeping
Copy link
Member Author

The original idea was the have the authentication scheme be specified in the topic descriptor. So if you listen on a topic that has signatures, then you better sign your messages or they will get discarded.

@Stebalien
Copy link
Member

The original idea was the have the authentication scheme be specified in the topic descriptor. So if you listen on a topic that has signatures, then you better sign your messages or they will get discarded.

Given that we're using from/seqno for deduplication, I think we really have to sign all messages (although we don't necessarily need to encrypt them).

I agree that the topic descriptor should specify the authentication scheme. However, I think that relates more to who can publish, not whether or not messages should be signed.

@vyzo
Copy link
Contributor

vyzo commented Aug 27, 2018

Message signing implementation: libp2p/go-libp2p-pubsub#97

@rklaehn
Copy link

rklaehn commented Jan 28, 2019

So any update on when this feature (I guess gossipsub now) will be made non-experimental? Note that non-experimental does not mean that it is bug-free, but just that it should usually work and will not change in a fundamental way.

For people like us ( actyx.io ) that use this in production it is really scary to depend on several experimental features ( private swarms + pubsub )....

@whyrusleeping
Copy link
Member Author

whyrusleeping commented Jan 28, 2019 via email

@daviddias
Copy link
Member

daviddias commented Jan 28, 2019

@whyrusleeping here is a proposal. Let's get the PubSub API written as a spec and officially change the stamp from draft to reliable -- https://github.com/ipfs/specs#work-in-progress

@Stebalien
Copy link
Member

I have no more protocol level concerns. My only remaining concern is implementation related: we still have some races concerning disconnects/reconnects (libp2p/go-libp2p-pubsub#133). However, there's no reason to block on that as we can fix it without any protocol breaking changes.

@vyzo?

@vyzo
Copy link
Contributor

vyzo commented Jan 28, 2019

no objections here...

@lidel
Copy link
Member

lidel commented Jan 20, 2021

IIUC discussion moved to #6621

@lidel lidel closed this as completed Jan 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

6 participants