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

Re-broadcast interval resets when receiving the self-broadcasted #15

Open
hsanjuan opened this issue Aug 14, 2019 · 4 comments
Open

Re-broadcast interval resets when receiving the self-broadcasted #15

hsanjuan opened this issue Aug 14, 2019 · 4 comments
Labels
good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws)

Comments

@hsanjuan
Copy link
Collaborator

A peer doing a re-broadcast will receive it's own block and reset the timer so the interval doubles. Thus an interval of 10s actually becomes 20s.

@hsanjuan hsanjuan added kind/bug A bug in existing code (including security flaws) help wanted Seeking public contribution on this issue good first issue Good issue for new contributors labels Aug 14, 2019
@ncabatoff
Copy link
Contributor

It's not clear how this should be resolved. As far as I can see, there's no way to distinguish between a broadcast of a head by us or a peer. If broadcasts identified the publisher of the head, then the solution would be easy.

I found this comment in the code:

		// TODO: We should store trusted-peer signatures associated to
		// each head in a timecache. When we broadcast, attach the
		// signatures (along with our own) to the broadcast.
		// Other peers can use the signatures to verify that the
		// received CIDs have been issued by a trusted peer.

Would it make sense to kill two birds with one stone here? It seems like this would allow us to identify the broadcaster of a head to solve this issue, and at the same time protect against bad behaviour by other peers.

@hsanjuan
Copy link
Collaborator Author

Yeah, that might be the best way.

@ncabatoff
Copy link
Contributor

Here's what I had in mind to implement, what do you think?

  • New now takes an optional private key
  • Head PB gets new optional CreatorSignature and BroadcasterSignature fields
  • When receiving a head broadcast, store the CreatorSignature along with the corresponding head CID
  • When broadcasting a head we've received, populate CreatorSignature with the same value we received
  • When we have a private key and are performing a head broadcast:
    • for heads we've created, populate CreatorSignature by signing the CID
    • for heads we've received, populate BroadcasterSignature by signing the CID+CreatorSignature

I'm not sure I see the need for a timecache. It seems that we need to retain the CreatorSignature of received heads for as long as they remain heads, rather than on a time basis.

As for verification of signatures, maybe make New take a signature verifier function, and ignore heads for which the verifier returns false on both signatures? The thinking behind trusting either the broadcaster or the creator (and not requiring that both signatures can be verified) is that we might not want to have to know every peer's key, but provided there's a chain of trust that should be sufficient. Alternatively, we could always populate BroadcasterSignature when we have a private key, even if we're the creator, and only verify that signature on reception.

@hsanjuan
Copy link
Collaborator Author

I'm going to need more time to think about this.

The issue is that in a crdt swarm where only a subset of peers is "trusted", and I am simply a follower. We'd want that if the trusted peers go away for some reason, the rest of the peers, without trusting themselves, can still sync the crdt-chain if the heads were actually issued by a trusted peer.

Currently, a non trusted peer will re-broadcast a head if it does not see anyone else doing so, but:

  • This will be filtered by other peers because this peer is not "trusted".
  • Because other peers are dropping pubsub messages from non-trusted peers, they will in turn re-broadcast their heads too, thinking no one else is doing it.

However:

  • go-ds-crdt has no concept of trust at all. This is handled externally using pubsub to sign and verify messages before they are passed to the go-ds-crdt application. I am not 100% sure that go-ds-crdt should handle trust at all although it does handle the heads-broadcasting format and in order to trust a head you need to know the issuer.

  • Trust should still be handled as a pubsub validation callback that can stop the pubsub distribution of messages that are not valid before they are returned in the subscription channel.

All in all, it seems this may need to be a full wrapper around the broadcasting part (putting an envelope around the current broadcast protobuf) transparently to this library. But this also has problems (i.e. a broadcast with several heads first issued by several different peers).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good issue for new contributors help wanted Seeking public contribution on this issue kind/bug A bug in existing code (including security flaws)
Projects
None yet
Development

No branches or pull requests

2 participants