wireguard: allow to set IPv6 endpoint
ClosedPublic
Actions

Authored by naito.yuichiro_gmail.com on Feb 26 2021, 1:11 AM.

Details

Reviewers

grehan
donner

Group Reviewers

network

Summary

Most of codes work for IPv6 endpoint, but handling of IPv6 address length is wrong.
This patch fixes to support both of the size of sockaddr_in and sockaddr_in6.
I can confirm this patch works for IPv6 endpoint.

Test Plan

Wireguard tunnel can be set up via IPv6.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

naito.yuichiro_gmail.com requested review of this revision.Feb 26 2021, 1:11 AM

naito.yuichiro_gmail.com created this revision.

kbowling added a reviewer: grehan.Feb 26 2021, 1:39 AM

zarychtam_plan-b.pwste.edu.pl added a subscriber: zarychtam_plan-b.pwste.edu.pl.Feb 26 2021, 5:06 AM

ae added a reviewer: network.Feb 26 2021, 6:44 AM

donner added a subscriber: donner.Feb 26 2021, 7:27 AM

donner added inline comments.

sys/dev/if_wg/module/if_wg_session.c
1885–1894	So we have uninitialized space, if sa_len < sizeof(...)? So we can overwrite memory, if sa_len > sizeof(...)?

There's a more complete fix for this in the pfsense repo, that also stores the source address so CARP destination addresses will work:

https://github.com/pfsense/FreeBSD-src/commit/1940e7d3

... also the minor fix for ipv6 tcpdump

https://github.com/pfsense/FreeBSD-src/commit/825ed9ee

In D28933#647903, @grehan wrote:
There's a more complete fix for this in the pfsense repo, that also stores the source address so CARP destination addresses will work:
https://github.com/pfsense/FreeBSD-src/commit/1940e7d3

I think it's better than my code.
Would you please commit pfsense patch after my patch?
It will record the authors of the codes clearly.

... also the minor fix for ipv6 tcpdump
https://github.com/pfsense/FreeBSD-src/commit/825ed9ee

I think it's also necessary for us.

sys/dev/if_wg/module/if_wg_session.c
1885–1894	I'll fix to see sa_family to determine the length of sockaddr_in{6}.

Fix to see sa_family to determine the length of struct sockaddr_in{6}.

donner accepted this revision as: donner.Feb 26 2021, 2:12 PM

This revision is now accepted and ready to land.Feb 26 2021, 2:12 PM

The IPv6 endpoint is accepted fine, but the tunnel is still not able to carry the data, at least legacy IP. Is it the complete solution or am I missing anything? At a glance, some outgoing and returning traffic can be dumped on the wg(4) interface, but it doesn't look fully functional since it behaves like a sinkhole with no local exit.

In D28933#648167, @zarychtam_plan-b.pwste.edu.pl wrote:

The IPv6 endpoint is accepted fine, but the tunnel is still not able to carry the data, at least legacy IP. Is it the complete solution or am I missing anything? At a glance, some outgoing and returning traffic can be dumped on the wg(4) interface, but it doesn't look fully functional since it behaves like a sinkhole with no local exit.

Yes, this patch works for me.
I'm going to show you a simple example between hostA and hostB.

After setting up wireguard tunnel:

hostA:
ifconfig wg0 inet 10.0.0.1
route add -host 10.0.0.2 -iface wg0

hostB:
ifocnfig wg0 inet 10.0.0.2
route add -host 10.0.0.1 -iface wg0
ping 10.0.0.1

Could you try this one?
wg(4) is a interface so that outgoing packet must be routed via the interface.

FYI the cumulative pfsense wireguard changes are in D28962

In D28933#648397, @grehan wrote:

FYI the cumulative pfsense wireguard changes are in D28962

D28962 looks good to me.
If you commit D28962, I'll close this review.

In D28933#648321, @naito.yuichiro_gmail.com wrote:

In D28933#648167, @zarychtam_plan-b.pwste.edu.pl wrote:

The IPv6 endpoint is accepted fine, but the tunnel is still not able to carry the data, at least legacy IP. Is it the complete solution or am I missing anything? At a glance, some outgoing and returning traffic can be dumped on the wg(4) interface, but it doesn't look fully functional since it behaves like a sinkhole with no local exit.

Yes, this patch works for me.
I'm going to show you a simple example between hostA and hostB.

After setting up wireguard tunnel:

hostA:
ifconfig wg0 inet 10.0.0.1
route add -host 10.0.0.2 -iface wg0

hostB:
ifocnfig wg0 inet 10.0.0.2
route add -host 10.0.0.1 -iface wg0
ping 10.0.0.1

Could you try this one?
wg(4) is a interface so that outgoing packet must be routed via the interface.