Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
noise: introduce an extension registry #453
noise: introduce an extension registry #453
Changes from all commits
685ef18
7a517af
215419c
0991510
b0818fa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
As a security precaution is it helpful to specify that the "early data" should not be processed before the handshake is fully finished? thus gives us more confidence on the data, what are your thoughts on this?
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.
In WebTransport, we process it immediately, and fail the handshake if the certificate hash doesn't match.
Maybe we should point out that consumers of early data need to be careful, and that delaying the processing / acting upon the early data can be one strategy to deal with this.
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.
Great, document that is a good point.
Processing early data before authentication runs the risk of opening an attack surface for flooding / DOS attack.
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.
For the record, rust-libp2p does not make use of
data
. @marten-seemann do any of the other implementations?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.
go-libp2p doesn't, and js-libp2p doesn't either (as far as I know).
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.
What about nim-libp2p cpp-libp2p jvm-libp2p @marten-seemann?
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’m not familiar with their code bases. This PR was open for more than a week, that should’ve been plenty of time to chime in. Also note that this is backwards-compatible change since we didn’t reuse field 3 in the protobuf.
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.
As far as I understand we tried to use multicodec for all magic numbers in the libp2p ecosystem thus far. If I understand this proposal correctly, it runs somewhat against this convention.
I am not opposed to that, I just think it is worth pointing out in the specification. E.g. a short paragraph of why we are not using multicodec for this.
What do you think @marten-seemann?
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.
Good point. I'm not super familiar with the design philosophy behind multicoded. @Stebalien and @lidel, could you help us out here?