Development discussion of WireGuard
 help / color / mirror / Atom feed
* [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
@ 2021-03-19 16:57 Frank Behrens
  2021-03-19 17:35 ` Jason A. Donenfeld
  2021-03-19 17:38 ` Jason A. Donenfeld
  0 siblings, 2 replies; 17+ messages in thread
From: Frank Behrens @ 2021-03-19 16:57 UTC (permalink / raw)
  To: WireGuard mailing list

Hello Jason, hello community,

although I regret the recent removal of FreeBSD kernel driver I can live 
with the present development model.

With the current sources in the external repository I could create a 
working VPN for the first time with my Android phone. It works well, 
thanks to all contributors. The setup was done only with ifconfig(8), I 
have the now removed parts still in place. :-)

For my setup I needed a different route for the encapsulated packets and 
so I implemented the missing parts. With this message I give that part 
to the community (matching the existing BSD-2-Clause-FreeBSD license).
For the sake of simplicity I created an own branch on github, the commit 
is 
https://github.com/frbehrens/wireguard-freebsd/commit/f0445be7b5b30a98da11bf2e209739a2155a59bb 
or use for direct patch download:
https://github.com/frbehrens/wireguard-freebsd/commit/f0445be7b5b30a98da11bf2e209739a2155a59bb.patch

Kind regards,
     Frank

-- 
Frank Behrens
Osterwieck, Germany


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-19 16:57 [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets Frank Behrens
@ 2021-03-19 17:35 ` Jason A. Donenfeld
  2021-03-20 17:05   ` Frank Behrens
  2021-03-19 17:38 ` Jason A. Donenfeld
  1 sibling, 1 reply; 17+ messages in thread
From: Jason A. Donenfeld @ 2021-03-19 17:35 UTC (permalink / raw)
  To: Frank Behrens; +Cc: WireGuard mailing list

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-19 16:57 [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets Frank Behrens
  2021-03-19 17:35 ` Jason A. Donenfeld
@ 2021-03-19 17:38 ` Jason A. Donenfeld
  1 sibling, 0 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2021-03-19 17:38 UTC (permalink / raw)
  To: Frank Behrens; +Cc: WireGuard mailing list

Hi again,

As well, you can now do:

git push git@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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-19 17:35 ` Jason A. Donenfeld
@ 2021-03-20 17:05   ` Frank Behrens
  2021-03-20 18:59     ` Franco Fichtner
  2021-03-22 17:43     ` Jason A. Donenfeld
  0 siblings, 2 replies; 17+ messages in thread
From: Frank Behrens @ 2021-03-20 17:05 UTC (permalink / raw)
  Cc: WireGuard mailing list

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-20 17:05   ` Frank Behrens
@ 2021-03-20 18:59     ` Franco Fichtner
  2021-03-22 17:43     ` Jason A. Donenfeld
  1 sibling, 0 replies; 17+ messages in thread
From: Franco Fichtner @ 2021-03-20 18:59 UTC (permalink / raw)
  To: Frank Behrens; +Cc: WireGuard mailing list

Hi Frank,

> On 20. Mar 2021, at 6:05 PM, Frank Behrens <frank@harz.behrens.de> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-20 17:05   ` Frank Behrens
  2021-03-20 18:59     ` Franco Fichtner
@ 2021-03-22 17:43     ` Jason A. Donenfeld
  2021-03-22 18:14       ` Jason A. Donenfeld
  1 sibling, 1 reply; 17+ messages in thread
From: Jason A. Donenfeld @ 2021-03-22 17:43 UTC (permalink / raw)
  To: Frank Behrens; +Cc: WireGuard mailing list

Hi Frank,

On Sat, Mar 20, 2021 at 11:15 AM Frank Behrens <frank@harz.behrens.de> 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->sc_socket;
+ struct socket *so4, *so6;
+ struct sockopt sopt;
+ int rc;
+
+ so4 = atomic_load_ptr(&so->so_so4);
+ so6 = atomic_load_ptr(&so->so_so6);

These are epoch-protected members. So the line above that should have
NET_EPOCH_ASSERT().

+ if (so4) {
+ rc = sosetopt(so4, &sopt);
+ MPASS(rc == 0);
+ }
+ if (so6) {
+ rc = sosetopt(so6, &sopt);
+ 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-22 17:43     ` Jason A. Donenfeld
@ 2021-03-22 18:14       ` Jason A. Donenfeld
  2021-03-23  5:51         ` Frank Behrens
  0 siblings, 1 reply; 17+ messages in thread
From: Jason A. Donenfeld @ 2021-03-22 18:14 UTC (permalink / raw)
  To: Frank Behrens; +Cc: WireGuard mailing list

Applied to git with some small modifications:

https://git.zx2c4.com/wireguard-freebsd/commit/?id=0a5c6abdfaa1f4f09269a222c1720e2ff3b8aa02

This will be in the next snapshot. And feel free to push other stuff
up to the repo as you feel is useful.

Jason

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-22 18:14       ` Jason A. Donenfeld
@ 2021-03-23  5:51         ` Frank Behrens
  2021-03-31 19:05           ` Frank Behrens
  0 siblings, 1 reply; 17+ messages in thread
From: Frank Behrens @ 2021-03-23  5:51 UTC (permalink / raw)
  To: WireGuard mailing list

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-23  5:51         ` Frank Behrens
@ 2021-03-31 19:05           ` Frank Behrens
  2021-03-31 19:11             ` Jason A. Donenfeld
  0 siblings, 1 reply; 17+ messages in thread
From: Frank Behrens @ 2021-03-31 19:05 UTC (permalink / raw)
  To: wireguard

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-31 19:05           ` Frank Behrens
@ 2021-03-31 19:11             ` Jason A. Donenfeld
  2021-03-31 19:16               ` Frank Behrens
  2021-04-01 16:27               ` Frank Behrens
  0 siblings, 2 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2021-03-31 19:11 UTC (permalink / raw)
  To: Frank Behrens; +Cc: WireGuard mailing list

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-31 19:11             ` Jason A. Donenfeld
@ 2021-03-31 19:16               ` Frank Behrens
  2021-04-01 16:27               ` Frank Behrens
  1 sibling, 0 replies; 17+ messages in thread
From: Frank Behrens @ 2021-03-31 19:16 UTC (permalink / raw)
  To: WireGuard mailing list

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


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-03-31 19:11             ` Jason A. Donenfeld
  2021-03-31 19:16               ` Frank Behrens
@ 2021-04-01 16:27               ` Frank Behrens
  2021-04-13  2:57                 ` Jason A. Donenfeld
  1 sibling, 1 reply; 17+ messages in thread
From: Frank Behrens @ 2021-04-01 16:27 UTC (permalink / raw)
  To: WireGuard mailing list

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-04-01 16:27               ` Frank Behrens
@ 2021-04-13  2:57                 ` Jason A. Donenfeld
  2021-04-17 13:08                   ` Frank Behrens
  0 siblings, 1 reply; 17+ messages in thread
From: Jason A. Donenfeld @ 2021-04-13  2:57 UTC (permalink / raw)
  To: Frank Behrens; +Cc: WireGuard mailing list

Hi Frank,

Can you let me know if this fixes the issue?

https://git.zx2c4.com/wireguard-freebsd/commit/?id=cdb18ebf44a5babb57cddccd6b33e9f19cfdf365

Jason

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-04-13  2:57                 ` Jason A. Donenfeld
@ 2021-04-17 13:08                   ` Frank Behrens
  2021-04-17 15:00                     ` Jason A. Donenfeld
  0 siblings, 1 reply; 17+ messages in thread
From: Frank Behrens @ 2021-04-17 13:08 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-04-17 13:08                   ` Frank Behrens
@ 2021-04-17 15:00                     ` Jason A. Donenfeld
  2021-04-17 15:23                       ` Frank Behrens
  0 siblings, 1 reply; 17+ messages in thread
From: Jason A. Donenfeld @ 2021-04-17 15:00 UTC (permalink / raw)
  To: Frank Behrens; +Cc: WireGuard mailing list

Hi Frank,

On 4/17/21, Frank Behrens <frank@harz.behrens.de> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-04-17 15:00                     ` Jason A. Donenfeld
@ 2021-04-17 15:23                       ` Frank Behrens
  2021-04-17 16:49                         ` Jason A. Donenfeld
  0 siblings, 1 reply; 17+ messages in thread
From: Frank Behrens @ 2021-04-17 15:23 UTC (permalink / raw)
  To: Jason A. Donenfeld; +Cc: WireGuard mailing list


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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets
  2021-04-17 15:23                       ` Frank Behrens
@ 2021-04-17 16:49                         ` Jason A. Donenfeld
  0 siblings, 0 replies; 17+ messages in thread
From: Jason A. Donenfeld @ 2021-04-17 16:49 UTC (permalink / raw)
  To: Frank Behrens; +Cc: WireGuard mailing list

Hi Frank,

On Sat, Apr 17, 2021 at 9:23 AM Frank Behrens <frank@harz.behrens.de> 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

^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2021-04-17 16:49 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-19 16:57 [PATCH] freebsd: Implement selection of FIB (routing table) for tunneled packets Frank Behrens
2021-03-19 17:35 ` Jason A. Donenfeld
2021-03-20 17:05   ` Frank Behrens
2021-03-20 18:59     ` Franco Fichtner
2021-03-22 17:43     ` Jason A. Donenfeld
2021-03-22 18:14       ` Jason A. Donenfeld
2021-03-23  5:51         ` Frank Behrens
2021-03-31 19:05           ` Frank Behrens
2021-03-31 19:11             ` Jason A. Donenfeld
2021-03-31 19:16               ` Frank Behrens
2021-04-01 16:27               ` Frank Behrens
2021-04-13  2:57                 ` Jason A. Donenfeld
2021-04-17 13:08                   ` Frank Behrens
2021-04-17 15:00                     ` Jason A. Donenfeld
2021-04-17 15:23                       ` Frank Behrens
2021-04-17 16:49                         ` Jason A. Donenfeld
2021-03-19 17:38 ` Jason A. Donenfeld

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).