-
Notifications
You must be signed in to change notification settings - Fork 234
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
IPv6 support for wireguard #5059
base: develop
Are you sure you want to change the base?
Conversation
9601175
to
36a7454
Compare
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
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.
Reviewable status: 0 of 26 files reviewed, 1 unresolved discussion (waiting on @neacsu and @octol)
service-providers/authenticator/src/config/mod.rs
line 201 at r2 (raw file):
/// Private IP address of the wireguard gateway. /// default: `2001:db8:a160:1::1`
Based on docs 2001:db8::/32 fall under documentation addresses which are unroutable. Are we sure we want that as a default?
https://www.ripe.net/media/documents/ipv6_reference_card.pdf
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.
Reviewed 1 of 26 files at r1.
Reviewable status: 1 of 26 files reviewed, 1 unresolved discussion (waiting on @neacsu and @octol)
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.
Reviewed 2 of 26 files at r1.
Reviewable status: 3 of 26 files reviewed, 2 unresolved discussions (waiting on @neacsu and @octol)
service-providers/authenticator/src/mixnet_listener.rs
line 262 at r2 (raw file):
peer.allowed_ips.push(IpAddrMask::new( final_message.gateway_client.private_ips.ipv6.into(), 32,
Shouldn't this be a 128
mask for ipv6?
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.
Reviewed 1 of 26 files at r1.
Reviewable status: 4 of 26 files reviewed, 2 unresolved discussions (waiting on @neacsu and @octol)
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.
Reviewed 2 of 26 files at r1.
Reviewable status: 6 of 26 files reviewed, 2 unresolved discussions (waiting on @neacsu and @octol)
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.
Reviewed 1 of 26 files at r1.
Reviewable status: 7 of 26 files reviewed, 2 unresolved discussions (waiting on @neacsu and @octol)
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.
Reviewed 1 of 26 files at r1.
Reviewable status: 8 of 26 files reviewed, 2 unresolved discussions (waiting on @neacsu and @octol)
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.
Reviewed 1 of 26 files at r1.
Reviewable status: 9 of 26 files reviewed, 2 unresolved discussions (waiting on @neacsu and @octol)
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.
Reviewed 1 of 26 files at r1.
Reviewable status: 10 of 26 files reviewed, 2 unresolved discussions (waiting on @neacsu and @octol)
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.
Reviewable status: 10 of 26 files reviewed, 2 unresolved discussions (waiting on @octol and @pronebird)
service-providers/authenticator/src/mixnet_listener.rs
line 262 at r2 (raw file):
Previously, pronebird (Andrej Mihajlov) wrote…
Shouldn't this be a
128
mask for ipv6?
It should, good catch!
service-providers/authenticator/src/config/mod.rs
line 201 at r2 (raw file):
Previously, pronebird (Andrej Mihajlov) wrote…
Based on docs 2001:db8::/32 fall under documentation addresses which are unroutable. Are we sure we want that as a default?
https://www.ripe.net/media/documents/ipv6_reference_card.pdf
Right, it's probably better to use a different one then. It's what we're using for mixnet mode IPv6, maybe we should change it there too. From the document you linked, the equivalent we could use is fc00::/16 (equivalent to 10.0.0.0/8)
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.
Reviewable status: 5 of 32 files reviewed, 2 unresolved discussions (waiting on @mfahampshire, @neacsu, @octol, and @serinko)
service-providers/authenticator/src/config/mod.rs
line 201 at r2 (raw file):
Previously, neacsu (Bogdan-Ștefan Neacşu) wrote…
Right, it's probably better to use a different one then. It's what we're using for mixnet mode IPv6, maybe we should change it there too. From the document you linked, the equivalent we could use is fc00::/16 (equivalent to 10.0.0.0/8)
That's my conclusion as well.
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.
Reviewed 1 of 12 files at r3.
Reviewable status: 6 of 32 files reviewed, 1 unresolved discussion (waiting on @mfahampshire, @octol, and @serinko)
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.
Reviewed 1 of 26 files at r1.
Reviewable status: 7 of 32 files reviewed, 1 unresolved discussion (waiting on @mfahampshire, @octol, and @serinko)
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.
Reviewed 2 of 26 files at r1, 1 of 2 files at r2, 2 of 2 files at r4.
Reviewable status: 11 of 32 files reviewed, 1 unresolved discussion (waiting on @mfahampshire, @octol, and @serinko)
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.
Reviewed 1 of 12 files at r3.
Reviewable status: 12 of 32 files reviewed, 1 unresolved discussion (waiting on @mfahampshire, @octol, and @serinko)
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.
Reviewed 1 of 2 files at r2.
Reviewable status: 13 of 32 files reviewed, 1 unresolved discussion (waiting on @mfahampshire, @octol, and @serinko)
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.
Dismissed @octol from a discussion.
Reviewable status: 13 of 32 files reviewed, all discussions resolved (waiting on @mfahampshire, @octol, @pronebird, and @serinko)
service-providers/authenticator/src/config/mod.rs
line 201 at r2 (raw file):
Previously, pronebird (Andrej Mihajlov) wrote…
That's my conclusion as well.
Changed to fc00::1 for mixnet and fc001::1 for wireguard
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.
Reviewable status: 13 of 32 files reviewed, all discussions resolved (waiting on @mfahampshire, @octol, and @serinko)
service-providers/authenticator/src/config/mod.rs
line 201 at r2 (raw file):
Previously, neacsu (Bogdan-Ștefan Neacşu) wrote…
Changed to fc00::1 for mixnet and fc001::1 for wireguard
Bravo!
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.
Reviewed 1 of 12 files at r3.
Reviewable status: 14 of 32 files reviewed, all discussions resolved (waiting on @mfahampshire, @octol, and @serinko)
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.
Reviewed 1 of 12 files at r3.
Reviewable status: 15 of 32 files reviewed, all discussions resolved (waiting on @mfahampshire, @octol, and @serinko)
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.
Reviewed 2 of 26 files at r1, 2 of 12 files at r3.
Reviewable status: 19 of 32 files reviewed, all discussions resolved (waiting on @mfahampshire, @octol, and @serinko)
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.
Reviewed 3 of 26 files at r1.
Reviewable status: 22 of 32 files reviewed, all discussions resolved (waiting on @mfahampshire, @octol, and @serinko)
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.
Reviewed 4 of 26 files at r1, 6 of 12 files at r3.
Reviewable status: 31 of 32 files reviewed, all discussions resolved (waiting on @mfahampshire, @octol, and @serinko)
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.
Reviewed 1 of 1 files at r5.
Reviewable status: all files reviewed (commit messages unreviewed), all discussions resolved (waiting on @mfahampshire, @octol, and @serinko)
This change is