-
Notifications
You must be signed in to change notification settings - Fork 281
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
gossipsub v1.1: introduce message signing policy #294
Changes from all commits
df4e932
e4eb5ee
0ccb3b0
e80c67c
b585401
dc1fe8b
58df0dd
a7862dd
e8b85d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -37,6 +37,8 @@ See the [lifecycle document][lifecycle-spec] for context about maturity level an | |
- [Explicit Peering Agreements](#explicit-peering-agreements) | ||
- [PRUNE Backoff and Peer Exchange](#prune-backoff-and-peer-exchange) | ||
- [Protobuf](#protobuf) | ||
- [Signature Policy](#signature-policy) | ||
- [Signature Policy Options](#signature-policy-options) | ||
- [Flood Publishing](#flood-publishing) | ||
- [Adaptive Gossip Dissemination](#adaptive-gossip-dissemination) | ||
- [Outbound Mesh Quotas](#outbound-mesh-quotas) | ||
|
@@ -134,6 +136,50 @@ message PeerInfo { | |
} | ||
``` | ||
|
||
### Signature Policy | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not sure that this should be in the gossipsub spec -- it is a generic pubsub policy that applies to all routing algorithms. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, will move it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was looking for a place to version it, but since it's not breaking anything necessarily, and applies to pubsub, moving it to the pubsub doc sounds good. |
||
|
||
The usage of the `signature`, `key`, `from`, and `seqno` fields in `Message` is now configurable per topic, in the manners specified in this section. | ||
> [[ Implementation note ]]: At the time of writing this section, go-libp2p-pubsub (reference implementation of this spec) allows for configuring the signature policy at a global pubsub instance level. This needs to be pushed down to topic-level configuration. Other implementations are encouraged to support topic-level configuration, as the spec mandates. | ||
|
||
In the default origin-stamped messaging, the fields need to be strictly enforced: | ||
the `seqno` and `from` fields form the `message_id`, and should be verified to avoid `message_id` collisions. | ||
|
||
In content-stamped messaging, the fields may negatively affect privacy: | ||
revealing the relationship between `data` and `from`/`seqno`. | ||
Comment on lines
+144
to
+148
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure what the goal of this is. This might be better moved to after "Signature Policy Options", and put under a "Usage indications/notes" section. I would reword to something like this:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The idea was to give some context/intuition why the fields are handled like they are. It could be worded directly as |
||
|
||
#### Signature Policy Options | ||
|
||
In gossipsub v1.1, these fields are strictly present and verified, or completely omitted altogether: | ||
- `StrictSign`: | ||
- On the producing side: | ||
- Build messages with the `signature`, `key` (`from` may be enough for certain inlineable public key types), `from` and `seqno` fields. | ||
- On the consuming side: | ||
- Enforce the fields to be present, reject otherwise. | ||
- Propagate only if the fields are valid and signature can be verified, reject otherwise. | ||
- `StrictNoSign`: | ||
- On the producing side: | ||
- Build messages without the `signature`, `key`, `from` and `seqno` fields. | ||
- The corresponding protobuf key-value pairs are absent from the marshalled message, not just empty. | ||
- On the consuming side: | ||
- Enforce the fields to be absent, reject otherwise. | ||
- Propagate only if the fields are absent, reject otherwise. | ||
- A `message_id` function will not be able to use the above fields, and should instead rely on the `data` field. A commonplace strategy is to calculate a hash. | ||
|
||
In gossipsub v1.0, a legacy "lax" signing policy could be configured, to only verify signatures when present. For security reasons, this is strategy is discarded in subsequent versions, but MAY still be supported for backwards-compatibility. If so, its use should be discouraged through prominent deprecation warnings. These strategies will be entirely dropped in the future. | ||
- `LaxSign`: *this was never an original gossipsub 1.0 option, but it's defined here for completeness, and considered insecure*. Always sign, and verify incoming signatures, and but accept unsigned messages. | ||
- On the producing side: | ||
- Build messages with the `signature`, `key` (`from` may be enough), `from` and `seqno` fields. | ||
- On the consuming side: | ||
- `signature` may be absent, and not verified. | ||
- Verify `signature`, iff the `signature` is present, then reject if `signature` is invalid. | ||
- `LaxNoSign`: *Previous default for no-verification*. Do not sign nor origin-stamp, but verify incoming signatures, and accept unsigned messages. | ||
- On the producing side: | ||
- Build messages without the `signature`, `key`, `from` and `seqno` fields. | ||
- On the consuming side: | ||
- Accept and propagate messages with above fields. | ||
- Verify `signature`, iff the `signature` is present, then reject if `signature` is invalid. | ||
|
||
|
||
### Flood Publishing | ||
|
||
In gossipsub v1.0, peers publish new messages to the members of their mesh if they are subscribed to | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is problematic because protobuf is non-deterministic in its marshalling - the way to write this specification would be to remove mentions of protobuf and instead explicitly outline the marshalling in terms of key-value pairs and field types - it would have to be done carefully such that the result is readable by a compliant protobuf parser - it might also happen by chance that some protobuf encoders produce a valid byte stream, but with unrecognised fields in particular, the likelihood is low.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's hoist the protobuf deterministic serialisation discussion to a new issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's been recurrent enough that it needs to be a first-class discussion with a solid outcome.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should specify canonical encoding here to remove all ambiguity.