go-libp2p-pubsub
go-libp2p-pubsub copied to clipboard
libp2p
Have a configuration option for Subscriptions receiving duplicate messages
Assuming #215 is implemented there may be reasons why subscribers will want to see all the peers that sent them messages, not just the first peer to send them a given message.
The simplest way to do this seems to be just having a SubOpt that allows us to send messages even if we've seen them
https://github.com/libp2p/go-libp2p-pubsub/blob/2247a54a8cfc7f84d54d28429a60fa4ce89d5c43/pubsub.go#L676
Unfortunately, this the logic for ignoring duplicates exists in the pubsub logic instead of the subscription logic so this means some code will have to get moved around.
Messages are not content-addressed, so to make this secure we'll need to call the validators every time we intend to deliver a duplicate (otherwise, an attacker could send a different message with the same ID, and have it delivered as a duplicate). Alternatively, we could require signatures to be enabled to use this feature -- that would guarantee message integrity. We still need to cache validation results, though.
With signatures we can easily cache validation results (ie have a second time cache that tracks valid messages) and deliver duplicates without re-entering the validation pipeline.
Maybe I'm just missing something, but how are signatures going to help here?
Is it because of the message hash? If so, it's possible that protobuf's lack of canonical serialization will hurt us here.
Is it because of the signature over the sequence number? If so, then there's some potential attack avenues here.
@aschmahmann
The message signature is over the entire message & not just the SeqNo as written here.
I think what @raulk means is that the key in the validated messages cache should also incorporate the validated message signature(unlike using the hash, this also solves the 'canonical serialization' problem) in addition to the source peer & sequence number so that a user can not send another message with the same sequence number but different content/signature & have us treat it as a duplicate thus by-passing validation.
Wdyt ?
@vyzo So, for the validated messages cache:
- The key will be "msg.From + msg.SeqNo + Validated msg.Signature"
- The duration will be the same as the seenMessages timeCache i.e. 2 minutes
- For a signed message, we validate the signature and a) If the key is in the cache, skip validation b) If the key isn't in the cache, add the message to the cache upon successful validation.
Wdyt ?
The key should be just the message Id, no need for anything else. And yes, it would be a timecache with some appropriate duration. But don't rush to implement this please, it is trickier than it looks.
@vyzo Not sure what you mean. If the key is just the messageID and a peer sends us a message with the same seqNo as before but with different content & hence a different signature, how do we decide whether to validate that message or not ?
We would still have to validate the signature, but in general we use messageID's for tracking things internally. A peer sending us a message with the same seqNo and different content is buggy, and will result to this message being ignored by the seen cache. But if we were to do the validation cache for duplicate message delivery, this complicates things. You see? Trickier than it looks.
@vyzo I understand what you mean.
But, a malicious peer could send us a message with the same ID and different content to skip validation as mentioned by @raulk here. If duplicate message delivery is turned on, such a message would skip validation & erroneously reach the subscribers.
Having the signature as part of the "validated messages cache" key protects us from this attack when duplicate message delivery is turned on. But, maybe there is another way to do this.
@aschmahmann came up with a good question during our discussion.
Assuming a threat model where peers can send "false duplicates"(same seqNo, different data), what do we do when we receive a false duplicate ? Should we simply drop the message, blacklist the peer or send an event to the user about this etc etc ?
Drop the message and Warn is probably fine; but we might want to consider blacklisting in this case as well. I am just a little weary of the pubsub library blacklisting internally without user interaction.