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

Have a configuration option for Subscriptions receiving duplicate messages #216

Open
aschmahmann opened this issue Oct 18, 2019 · 11 comments

Comments

@aschmahmann
Copy link
Contributor

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

type SubOpt func(sub *Subscription) error

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.

@raulk
Copy link
Member

raulk commented Oct 18, 2019

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.

@vyzo
Copy link
Collaborator

vyzo commented Oct 18, 2019

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.

@aschmahmann
Copy link
Contributor Author

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.

@aarshkshah1992
Copy link
Contributor

aarshkshah1992 commented Nov 17, 2019

@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 ?

@aarshkshah1992
Copy link
Contributor

aarshkshah1992 commented Nov 17, 2019

@vyzo So, for the validated messages cache:

  1. The key will be "msg.From + msg.SeqNo + Validated msg.Signature"
  2. The duration will be the same as the seenMessages timeCache i.e. 2 minutes
  3. 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 ?

@vyzo
Copy link
Collaborator

vyzo commented Nov 17, 2019

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.

@aarshkshah1992
Copy link
Contributor

aarshkshah1992 commented Nov 17, 2019

@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 ?

@vyzo
Copy link
Collaborator

vyzo commented Nov 17, 2019

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.

@aarshkshah1992
Copy link
Contributor

aarshkshah1992 commented Nov 17, 2019

@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.

@aarshkshah1992
Copy link
Contributor

@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 ?

@vyzo
Copy link
Collaborator

vyzo commented Nov 21, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants