Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hi Frank, On Sat, Apr 17, 2021 at 9:23 AM Frank Behrens wrote: > > > Am 17.04.2021 um 17:00 schrieb Jason A. Donenfeld: > > Does this actually fix or change anything? Don't new sockets have > > fib==0 right out of the gate already? > > New sockets inherit the fib from the current process. If you create > the wg interface from a process with different fib, that fib will also > be used for this socket. Probably the difference in code is not very > important for the case of a system default boot. But that may vary > for jails/vnets with different default fibs. > > In my test case the sequence > > setfib 1 ifconfig wg0 create > > ifconfig wg0 tunnelfib 0 > failed. Ahh, interesting. I applied your patch. Thanks for the persistence in getting this feature working well. Jason
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Am 17.04.2021 um 17:00 schrieb Jason A. Donenfeld: Does this actually fix or change anything? Don't new sockets have fib==0 right out of the gate already? New sockets inherit the fib from the current process. If you create the wg interface from a process with different fib, that fib will also be used for this socket. Probably the difference in code is not very important for the case of a system default boot. But that may vary for jails/vnets with different default fibs. In my test case the sequence > setfib 1 ifconfig wg0 create > ifconfig wg0 tunnelfib 0 failed. Frank -- Frank Behrens Osterwieck, Germany
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hi Frank, On 4/17/21, Frank Behrens wrote: > Hi Jason! > > Am 13.04.2021 um 04:57 schrieb Jason A. Donenfeld: >> Can you let me know if this fixes the issue? >> >> https://git.zx2c4.com/wireguard-freebsd/commit/?id=cdb18ebf44a5babb57cddccd6b33e9f19cfdf365 > > It looks better, but a little too much optimized. ;-) > > The fix is in https://git.zx2c4.com/wireguard-freebsd/log/?h=fb/fib3 Does this actually fix or change anything? Don't new sockets have fib==0 right out of the gate already? Jason
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hi Jason! Am 13.04.2021 um 04:57 schrieb Jason A. Donenfeld: Can you let me know if this fixes the issue? https://git.zx2c4.com/wireguard-freebsd/commit/?id=cdb18ebf44a5babb57cddccd6b33e9f19cfdf365 It looks better, but a little too much optimized. ;-) The fix is in https://git.zx2c4.com/wireguard-freebsd/log/?h=fb/fib3 Best regards, Frank -- Frank Behrens Osterwieck, Germany
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hi Frank, Can you let me know if this fixes the issue? https://git.zx2c4.com/wireguard-freebsd/commit/?id=cdb18ebf44a5babb57cddccd6b33e9f19cfdf365 Jason
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hello Jason! Am 31.03.2021 um 21:11 schrieb Jason A. Donenfeld: Thanks for the patch. Does the line `so4->so_fibnum = so6->so_fibnum = sc->sc_socket.so_fibnum;` also need to be changed too in initiation, or is that one fine? Thanks for the pointer. That part has to be changed as well. I updated my branch. Kind regards, Frank -- Frank Behrens Osterwieck, Germany
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hello Jason! Am 31.03.2021 um 21:11 schrieb Jason A. Donenfeld: Hi Frank, Thanks for the patch. Does the line `so4->so_fibnum = so6->so_fibnum = sc->sc_socket.so_fibnum;` also need to be changed too in initiation, or is that one fine? Good catch! I'll check this in the next few days (and update the branch if necessary). Frank -- Frank Behrens Osterwieck, Germany
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hi Frank, Thanks for the patch. Does the line `so4->so_fibnum = so6->so_fibnum = sc->sc_socket.so_fibnum;` also need to be changed too in initiation, or is that one fine? Jason
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hello! Am 23.03.2021 um 06:51 schrieb Frank Behrens: Am 22.03.2021 um 19:14 schrieb Jason A. Donenfeld: Applied to git with some small modifications: https://git.zx2c4.com/wireguard-freebsd/commit/?id=0a5c6abdfaa1f4f09269a222c1720e2ff3b8aa02 Thanky you! That looks very good. I'm sorry, that I didn't test this in detail. Unfortunately it does not work, I made a patch in branch fb/fib2. Kind regards, Frank -- Frank Behrens Osterwieck, Germany
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hi Jason! Am 22.03.2021 um 19:14 schrieb Jason A. Donenfeld: Applied to git with some small modifications: https://git.zx2c4.com/wireguard-freebsd/commit/?id=0a5c6abdfaa1f4f09269a222c1720e2ff3b8aa02 Thanky you! That looks very good. -- Frank Behrens Osterwieck, Germany
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hi Frank, On Sat, Mar 20, 2021 at 11:15 AM Frank Behrens wrote: > > Hi Jason, > > thanks for your response. > > Am 19.03.2021 schrieb Jason A. Donenfeld: > > In other words, you have push access to all branches beginning with fb/ . > That works, thanks. Meanwhile I pushed my branch to fb/fib. > > > Right now we have the `wg set wg0 fwmark ...` mapped to > > SO_USER_COOKIE, as I'm sure you saw there. But maybe FIB would be a > > better thing to use for that? We could adjust wireguard-go to do the > > same with the tuntap ioctl. > I believe we have different, orthogonal things: > > 1. The selection of routing table (fib) for received, decrypted packets. > -> Already implemented in wg_deliver_in() #2098 and controlled > by "ifconfig wg0 fib 1" > > 2. The selection of routing table for outgoing, encrypted packets. > -> That is addressed by my patch and controlled by > "ifconfig wg0 tunnelfib 1". Maybe wg(8) should receive also > an option for that purpose, if other OS use equivalent functions. > > 3. The setting of special marks, useable in packet filter/firewall > processing. I guess, that is the meaning for "wg.. fwmark". I'm not > sure, how best to implement that for FreeBSD. For ipfw(4) there is some > functionality using socket cookies, as already implemented. For pf(4) > packet filter the documentation mentions mbuf_tags(9). Apparently > we need some input from a FreeBSD packet filter developer. It looks like user_cookie is independently useful for use in ipfw, so I'll keep fwmark mapped to that. But your tunnelfib patch is _also_ useful, so I'll apply this patch. Some bugs/questions, though: +static void +wg_socket_setfib(struct wg_softc *sc) +{ + struct wg_socket *so = >sc_socket; + struct socket *so4, *so6; + struct sockopt sopt; + int rc; + + so4 = atomic_load_ptr(>so_so4); + so6 = atomic_load_ptr(>so_so6); These are epoch-protected members. So the line above that should have NET_EPOCH_ASSERT(). + if (so4) { + rc = sosetopt(so4, ); + MPASS(rc == 0); + } + if (so6) { + rc = sosetopt(so6, ); + MPASS(rc == 0); + } Rather than MPASS, this error should be passed back to the caller and reported back to userspace, in the same way that we account for errors with socket binding. + case SIOCSTUNFIB: + if ((ret = priv_check(curthread, PRIV_NET_SETIFFIB)) != 0) + break; This priv check should be relative to our stored creds that control the socket right? There are some jail considerations to think about here. + if (ifr->ifr_fib >= rt_numfibs) { + ret = EINVAL; + } else { + sc->sc_fibnum = ifr->ifr_fib; + wg_socket_setfib(sc); + } There are two ways to implement this. Either we check for 0 <= ifr_fib < rt_numfibs, and then directly assign it to so->so_fibnum, like we do for user_cookie, and avoid sosockopt. This sounds simplest and I wouldn't mind doing that. Or we use sosockopt, but the bounds testing remains in sosockopt and not duplicated here, and we rely on wg_socket_setfib bubbling the error back up. Let's pick one of these approaches, rather than combining them. struct wg_softc { LIST_ENTRY(wg_softc) sc_entry; struct ifnet *sc_ifp; int sc_flags; + u_int sc_fibnum; Shouldn't this be added to `struct wg_socket`, alongside the so_user_cookie member there, and managed in the same way? Jason
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hi Frank, > On 20. Mar 2021, at 6:05 PM, Frank Behrens wrote: > > 3. The setting of special marks, useable in packet filter/firewall > processing. I guess, that is the meaning for "wg.. fwmark". I'm not > sure, how best to implement that for FreeBSD. For ipfw(4) there is some > functionality using socket cookies, as already implemented. For pf(4) > packet filter the documentation mentions mbuf_tags(9). Apparently > we need some input from a FreeBSD packet filter developer. In pf(4) the tags are stored using mtag and that's reachable through the kernel only for direct tagging (normally it matches through ruleset and applies tags to packets in fly-by), although it is difficult to look up the tag name to tag integer from static functions inside pf_ioctl.c and keeping the index in sync with the tags that could change when the ruleset changes, see pf_tag_packet() in pf.c for low level tagging using the tag integer translated from the tag name during the last ruleset apply. Cheers, Franco
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hi Jason, thanks for your response. Am 19.03.2021 schrieb Jason A. Donenfeld: In other words, you have push access to all branches beginning with fb/ . That works, thanks. Meanwhile I pushed my branch to fb/fib. Right now we have the `wg set wg0 fwmark ...` mapped to SO_USER_COOKIE, as I'm sure you saw there. But maybe FIB would be a better thing to use for that? We could adjust wireguard-go to do the same with the tuntap ioctl. I believe we have different, orthogonal things: 1. The selection of routing table (fib) for received, decrypted packets. -> Already implemented in wg_deliver_in() #2098 and controlled by "ifconfig wg0 fib 1" 2. The selection of routing table for outgoing, encrypted packets. -> That is addressed by my patch and controlled by "ifconfig wg0 tunnelfib 1". Maybe wg(8) should receive also an option for that purpose, if other OS use equivalent functions. 3. The setting of special marks, useable in packet filter/firewall processing. I guess, that is the meaning for "wg.. fwmark". I'm not sure, how best to implement that for FreeBSD. For ipfw(4) there is some functionality using socket cookies, as already implemented. For pf(4) packet filter the documentation mentions mbuf_tags(9). Apparently we need some input from a FreeBSD packet filter developer. Kind regards, Frank -- Frank Behrens Osterwieck, Germany
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hey Frank, Thanks for the patch. That looks terrific. One question, though: Right now we have the `wg set wg0 fwmark ...` mapped to SO_USER_COOKIE, as I'm sure you saw there. But maybe FIB would be a better thing to use for that? We could adjust wireguard-go to do the same with the tuntap ioctl. Jason
Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
Hi again, As well, you can now do: git push g...@git.zx2c4.com:wireguard-freebsd master:fb/whatever-you-want In other words, you have push access to all branches beginning with fb/ . Poke me on IRC (I'm zx2c4) there if you want to chat about dev. Jason