Skip to content
This repository has been archived by the owner on Jun 14, 2024. It is now read-only.

Waku v2: Decide what to do about signature field #150

Closed
oskarth opened this issue Jul 13, 2020 · 6 comments
Closed

Waku v2: Decide what to do about signature field #150

oskarth opened this issue Jul 13, 2020 · 6 comments
Assignees

Comments

@oskarth
Copy link
Contributor

oskarth commented Jul 13, 2020

Problem

Currently the alpha spec says (https://specs.vac.dev/specs/waku/waku-v2.html#protobuf):

The signature field MAY contain the signature of the message, thus providing authentication of the message. See PubSub spec for more details.

There are some trade-offs involved in using this or not.

Acceptance criteria

  1. Clarity on whether we want to rely on signature field or not
  2. Documented with rationale/caveats/recommendations (e.g. sig above layer?)

Details

Currently the signature field has some questionable choices:

  • they are relying on protobuf serialization being consistent across peers
  • they also can't add I think any field in the protobuf without breaking signature scheme, depending on the version used

However:

  • doubtful protobuf will change the behavior, many people already incorrectly rely on it

See libp2p/specs#276 for upstream issue

Alternatively, to add fields without breaking signature scheme:

  • pass the encoded payload and sign that one { bytes signature; bytes payload}
  • then unmarshall that one and verify against already marshalled payload
  • never marshal to verify the signature (indeterministic)
  • and you don't care about the fields inside the signed payload (future proof)
  • (the author of protobuf suggests this method as well)

h/t @cammellos for ideas

Possible Solutions

  1. Use signature field as is
  2. Use signature field with caveats (e.g. recommend not relying on it too much, etc)
  3. Discourage usage of signature

Notes

@oskarth
Copy link
Contributor Author

oskarth commented Jul 13, 2020

@oskarth
Copy link
Contributor Author

oskarth commented Sep 22, 2020

There's a new NoSign policy being introduced: libp2p/specs#294

Being worked on upstream here vacp2p/nim-libp2p#365

Once that's merged, we can revisit this and use that I believe.

@kdeme
Copy link
Contributor

kdeme commented Sep 22, 2020

I think we should go with 3.
As I also wrote here: #159 (comment)

It links the creation of the PubSub message with libp2p peerId and thus indirectly with the IP address of the user publishing the message. This is typically something we do not want.
So it should be put in the specs as either:

  • MAY sign or SHOULD NOT and if you still do it these are the consequences...
  • Or perhaps simply MUST NOT sign.

@oskarth
Copy link
Contributor Author

oskarth commented Sep 22, 2020

Agree, I think on a spec level this will be StrictNoSign policy as per 1.1 once that is upstream. Or do you think there is something compared with that?

@kdeme
Copy link
Contributor

kdeme commented Sep 23, 2020

Agree, I think on a spec level this will be StrictNoSign policy as per 1.1 once that is upstream. Or do you think there is something compared with that?

Yeah, sounds like we should use that when it gets in.

@oskarth oskarth self-assigned this Oct 28, 2020
@oskarth
Copy link
Contributor Author

oskarth commented Oct 28, 2020

Decided already, see #228

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

No branches or pull requests

2 participants