IPV6_PKTINFO support for v4-mapped IPv6 sockets

[bz]

Summary

When using v4-mapped IPv6 sockets with IPV6_PKTINFO we do not
respect the given v4-mapped src address on the IPv4 socket.
Implement the needed functionality. This allows single-socket
UDP applications (such as OpenVPN) to work better on FreeBSD.

Requested by: Gert Doering (gert greenie.net)

Test Plan

Leave that to Gert, I don't like v4-mapped IPv6 sockets.

[bz]

Actually check that PRUS_IPV6 is set before accepting the IPv6
control message in IPv4 lands.

[melifaro]

Thank you for adding this! As this functionality is not the commonly-used one, would it be possible to add some verification tests as well?

It looks like some python can easily do the trick w.r.t send path:

import socket, struct

s = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM)
s.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_V6ONLY, struct.pack('I', 0))
# Send with default src
s.sendto('test'.encode('utf-8'), ('::ffff:192.168.53.1', 4242))
# Send with custom src
sopt = struct.pack('16pI', socket.inet_pton(socket.AF_INET6, '::ffff:192.168.53.199'), 0)
s.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_PKTINFO, sopt)
s.sendto('test'.encode('utf-8'), ('::ffff:192.168.53.1', 4242))

I guess it could be more or less easily embodied as an additional test in the files added by https://reviews.freebsd.org/D24138

sys/netinet/udp_usrreq.c

1156 ↗

(On Diff #69714)

Would it be possible to make this code an (inlined) function like udp_set_saddr_mapped(struct cmsghdr *cm, struct sockaddr_in *sin)?
This could also remove the necessity of err: label.

1184 ↗

(On Diff #69714)

Would it be possible to add RFC references like RFC 3542, clause 6.5 to ease the desired functionality understanding by the readers?

[gert_greenie.muc.de]

Hi,

I gave this a test today, and it did not work for me. Namely, when using sendto() on a 12.1-RELEASE-p3 kernel with this patch applied (it applied with little fuzz, so I assume this was okay to do) I get EINVAL for every IPv4-UDP packet sent over my dual-stack socket. Unmodified 12.1-RELEASE-p3 with the same code will just ignore the option and use the primary IPv4 address of the interface.

Looking at the patch, EINVAL can only be triggered by "not sizeof(in6_pktinfo)" or "not a v4-mapped address".

So I added some printf() to udp_usrreq.c, and got back "the size of the structure is good", but it does not like the address I pass in. printf()'ing the address confirms "it is what I pass in, and it's *so* an v4 mapped address, and IN6_IS_ADDR_V4MAPPED() actually confirms... - so: the logic is wrong, it needs to be

if (!IN6_IS_ADDR_V4MAPPED(&pktinfo->ipi6_addr) && 
    !IN6_IS_ADDR_UNSPECIFIED(&pktinfo->ipi6_addr

(or "if( ! (v4mapped || unspecified) )", which might be easier to read).

With that change, I can confirm that OpenVPN will now handle multiple different v4 addresses on a dual-stacked v6 socket just fine. Thanks, bz, for diving into this.

[gert_greenie.muc.de]

sys/netinet/udp_usrreq.c

1166 ↗

(On Diff #69714)

this needs to be "&&"

[bz]

Correct logic bug identified by gert.
Factor out the code into its own function as requested my @melifaro.

[bz]

sys/netinet/udp_usrreq.c

1184 ↗

(On Diff #69714)

I added a reference to the main implementation.

[gert_greenie.muc.de]

This new patch, with the logic error regarding PRUS_IPV6 check fixed, still leads to EINVAL for me.

Tracing this with liberally applied printf()'s, the EINVAL is coming from in_pcbbind_setup(), namely this check

} else {
        sin = (struct sockaddr_in *)nam;
        if (nam->sa_len != sizeof (*sin))

with the values "nam->sa_len=8, sizeof(*sin)=16"...

AAAH! tracked this to a missing "*" in "sizeof(*src)" (oversight when converting from struct to pointer). Commented inline as well.

With these two changes applied, OpenVPN is "back to working".

sys/netinet/udp_usrreq.c

1095 ↗

(On Diff #69765)

this needs to be "=="

Patch as is leads to "no UDP packet is generated at all", which is not what I'd expect, tbh ("wrong source address" or "EINVAL"). Not sure why.

*With* that change, I get EINVAL (again).

1133 ↗

(On Diff #69765)

This needs to be "sizeof(*src)".

[bz]

Fix more errors of in-between work.
I should really write that test case which would be simple.

[melifaro]

LGTM. Though, adding a test for this scenario would really help.
Just in case: I've committed D24138, so writing a test case based on common/sender.py and output.sh:output_udp_setup_success should be pretty easy, if you are okay with reusing it.