-
Notifications
You must be signed in to change notification settings - Fork 36
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
add config options for security protocols #158
Conversation
p2pd/main.go
Outdated
securityOpts = append(securityOpts, libp2p.Security(tls.ID, tls.New)) | ||
} | ||
if c.Security.Noise { | ||
securityOpts = append(securityOpts, libp2p.Security(noise.ID, noise.Maker())) |
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 needs to be updated with the latest Noise.
securityOpts = append(securityOpts, libp2p.Security(noise.ID, noise.Maker())) | |
securityOpts = append(securityOpts, libp2p.Security(noise.ID, noise.New)) |
@yusefnapora IMO this branch is sufficient enough to get merged in. Noise isn't on by default so it's not an issue and would more easily enable ongoing interop testing. Can we get the dependencies updated in here so we can merge? I've been using this branch locally for interop testing and we have other contributors who this would be helpful for.
Have updated the deps on this PR & it now uses the latest Noise code. |
Except for testing, not yet. We want a release or two where it's available before we switch the default. |
0c0b515
to
bf39395
Compare
@Stebalien @jacobheun Please can I review ? This looks good to merge. |
Feel free to approve and merge. |
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.
LGTM, fyi js is also still using secio as the default. 🚢
This adds a new
Security
config section withSECIO
,TLS
andNoise
boolean flags, plus command line switches to enable / disable each protocol. Secio is still enabled by default, but now you can disable it with-secio=false
and enable others, e.g.-tls -noise
. If you disable secio without enabling any others, the daemon will fail with a fatal error.I'm opening this as a draft PR since I don't think Noise is quite ready to be a dependency of the daemon, but I want to have it available on a branch so we can setup interop tests with the javascript Noise implementation.
cc @vyzo, @vasco-santos