Re: potentially disallowing IP fragmentation on wg packets, and handling routing loops better

2021-06-06 Thread Roman Mamedov
On Sun, 6 Jun 2021 11:13:36 +0200
"Jason A. Donenfeld"  wrote:

> Specifically the change would be to not allow IP fragmentation of the
> encrypted UDP packets. This way, in the case of a loop, eventually the
> packet size exceeds MTU, and it gets dropped: dumb and effective.
> Depending on how this discussion goes, a compromise would be to not
> allow fragmentation, but only for forwarded and kernel-generated
> packets, not not for locally generated userspace packets. That's more
> complex and I don't like it as much as just disallowing IP
> fragmentation all together.
> 
> Pros:
> - It solves the routing loop problem very simply.

Doesn't TTL already solve this?

> - Maybe people are running
> wireguard-over-gre-over-vxlan-over-l2tp-over-pppoe-over-god-knows-what-else,
> and this reduces the MTU to below 1280, yet they still want to put
> IPv6 through wireguard, and are willing to accept the performance
> implications.

Not only that. Sometimes transparent bridging of 1500 MTU LANs is required.

VXLAN does not allow tunnel endpoints to produce fragmented VXLAN packets.

With WG we can fragment them one level lower, *and* gain a higher efficiency
compared to hypothetical VXLAN's fragmentation, due to less header overhead on
2nd and further packets in a chain.

It would be unfortunate if this will become no longer possible.

It appears to me that people who might need to transparently join multiple
Ethernet LANs due to legacy network topologies they have to work with, weird
requirements, various legacy software etc, would outnumber those who even run
WG over WG at all, let alone getting themselves into a routing loop that way.

-- 
With respect,
Roman


Re: Certain private keys being mangled by wg on FreeBSD

2021-06-06 Thread Jason A. Donenfeld
On 6/6/21, Christian McDonald  wrote:
> Would it not be better for wg to just fail outright instead of
> transforming a poorly generated key entered by a user, regardless of
> where the key came from? Especially if that problematic key passes the
> regex validation that was provided in another thread in this email
> list?

No, it would not be better. There is nothing wrong with using those
keys. They're not "poorly generated" or "problematic" or dangerous in
the least. This is only a concern with your UI.

The kernel is doing the correct thing -- clamping keys -- and
displaying an unambiguous identifier to the user: the key that it will
actually be using.

I suspect the best thing to do for your UI would be to hide private
(and preshared) keys, and only show public keys, unless explicitly
exported into a config file. This not only reduces potential confusion
with this issue, but mitigates another potential footgun down the
line. It's also what wg(8)'s show command does by default (while
showconf will export all).


Re: Certain private keys being mangled by wg on FreeBSD

2021-06-06 Thread Christian McDonald
Would it not be better for wg to just fail outright instead of
transforming a poorly generated key entered by a user, regardless of
where the key came from? Especially if that problematic key passes the
regex validation that was provided in another thread in this email
list? If not, what would be an appropriate solution to catch
situations like this and in turn alert users? This seems like it could
be a larger discussion on interoperability, especially when dealing
with keys that are being generated by VPN providers.

Granted, this certainly isn’t my area of expertise. Though, the
behavior is just unexpected (and confusing) from an end user
perspective.

On Sun, Jun 6, 2021 at 11:09 AM Jason A. Donenfeld  wrote:
>
> It looks like whatever is generating those private keys is not
> clamping them. Specifically, all private keys should undergo this
> transformation:
>
> key[0] &= 248;
> key[31] = (key[31] & 127) | 64;
>
> In your case, your `Lm` prefix (first byte: 0x2c) is being anded with
> 248, and thus turns into KG (first byte: 0x28).
>
> The kernel properly clamps the keys on input, though, in case
> generators forget to clamp them. So what you're seeing is correct
> behavior.



-- 
R. Christian McDonald
M: (616) 856-9291
E: rcmcdonal...@gmail.com


Re: Certain private keys being mangled by wg on FreeBSD

2021-06-06 Thread Jason A. Donenfeld
It looks like whatever is generating those private keys is not
clamping them. Specifically, all private keys should undergo this
transformation:

key[0] &= 248;
key[31] = (key[31] & 127) | 64;

In your case, your `Lm` prefix (first byte: 0x2c) is being anded with
248, and thus turns into KG (first byte: 0x28).

The kernel properly clamps the keys on input, though, in case
generators forget to clamp them. So what you're seeing is correct
behavior.


Certain private keys being mangled by wg on FreeBSD

2021-06-06 Thread Christian McDonald
Hi Jason,

I've got an interesting case here. I've got what appears to be a few
private keys provided by Mullvad that are being mangled by wg(8) on
FreeBSD12.2 (pfSense 2.5.1+).

Private key ```Lm0BA4UyPilHH5qnXCfr6Lw8ynecOPor88tljLy3ALk=``` ...
becomes ```KG0BA4UyPilHH5qnXCfr6Lw8ynecOPor88tljLy3AHk=``` on wg
showconf

another example is ```zQwldrChCYP3P2fZeTtHmj3c/88hxM+TaLIq1mJ28ZY=``` turns into
```yAwldrChCYP3P2fZeTtHmj3c/88hxM+TaLIq1mJ28VY=```


I must have just gotten lucky with the keys Mullvad provided to me, as
those worked and continue to work fine for me.

ref : https://github.com/theonemcdonald/pfSense-pkg-WireGuard/issues/105

Best,

--
R. Christian McDonald
E: rcmcdonal...@gmail.com


[ANNOUNCE] wireguard-freebsd snapshot v0.0.20210606 is available

2021-06-06 Thread Jason A. Donenfeld
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256

Hi,

An experimental snapshot, v0.0.20210606, of WireGuard for FreeBSD has been
been tagged in the git repository.

At this time this code is new, unvetted, possibly buggy, and should be
considered "experimental". It might contain security issues. We gladly
welcome your testing and bug reports, but do keep in mind that this code
is new, so some caution should be exercised at the moment for using it
in mission critical environments.

== Changes ==

  * netns: use `exit 0` for early exit
  * ci: add a Cirrus-CI config file to build + smoke test
  * if_wg: pass back result of selftests and enable in CI
  * netns: trim test to working parts and rework jail logic
  * netns: account for FreeBSD 12 quirks
  * netns: use massive datagrams
  * if_wg: do not crash if deiniting before vnet is up
  * ci: test on 12.1 and 12.2
  
  We now have the vnet CI running on every commit on 12.1, 12.2, and 13.0, and
  so a decent amount of work went into enabling this and tidying up the tests we
  run.
  
  * global: destroy rwlocks and mtxs
  * global: replace rwlock with mtx if never rlocked
  
  This uses some fancy SmPL to simplify our lock types when possible.
  
  * compat: account for lack of CSUM_SND_TAG on ≤12.2
  * if_wg: add braces for 12.1 compiler warning
  
  Usual compat work.

This snapshot contains commits from: Jason A. Donenfeld and Ed Maste.

The source repository is available at the usual location:
  git clone https://git.zx2c4.com/wireguard-freebsd

This snapshot is available in compressed tarball form:
  
https://git.zx2c4.com/wireguard-freebsd/snapshot/wireguard-freebsd-0.0.20210606.tar.xz
  SHA2-256: d7501666030619e4ee86e68d2b9fd6a718be6541be83bf726414b821c60383d0

Thank you,
Jason Donenfeld


-BEGIN PGP SIGNATURE-

iQJEBAEBCAAuFiEEq5lC5tSkz8NBJiCnSfxwEqXeA64FAmC8z7UQHGphc29uQHp4
MmM0LmNvbQAKCRBJ/HASpd4DrpTQD/9VenrT33AbdVf688t11dyKD6S/aZ0XLsF8
Qvjpv8tJuLOOHqSm1d7c6UKdsDrc9okbAvQvHoiX/VhU9CxBKzQoK74iTPiO+bL4
Y7Z+fe+25vzjyzJmW8KhPymgQzfhw2JW7KwjtwXqh4NnZ3zKFoaY/9hWBnZAIF2o
pTrjBE6jL5W6smFDyHsa6L0f9hfOJ7PUvNRj9Tym9VQiGxY0Q3z7N845n2gvgwJD
J2UKTugz12zdVt/sKSpqLaLrbGaonqY+10SdLVUNO19+6iIx7hRBPw+BKTakUVGl
aqji3kM7nsszWIt9yEKzdmG62oIMfZg0524yU3ZW9f38TyN2x/5bNoaRcHxNvcA/
SJ+25mVsSKNiNvVho89e6NxSK9tqywkXCbVd62p6KyOKdJSOvsvuT5LhPzlA6RMz
Oli8bpZHQucxNHFzNdnuojo44/G9RlKriCKqMtPY2BH+LJOswzFG071HcWn785hR
96wDkxPvL4QqD/7tHssyPna33Ees/IRSlnYTXaZ5L9wwBTilI0XnKO/X+whpm4jj
21rXfXEXlCbe9tA90H6tQS/sHcn9Ezj2+5MEgCxhZ6yRjeibDZvKR8kCuIcXJxAi
/Y1iF5wyVvWSndNKruvUV7t4+w39WzzxaLNokZ3UZYysmLBvwbf+Ddl5yVUxtEkr
HmA9Tocw9g==
=eYnb
-END PGP SIGNATURE-


[PATCH wireguard-linux] Basic support for binding the transport socket to a device

2021-06-06 Thread Marvin Gaube
This patch depends on da5095d052860baa7fe2932fb1209628dd3e3813 from udp_tunnel 
module,
and allows to bind the transport socket to a specific interface.

With this patch, it is possible to use wireguard with VRFs:
The transport uses a separate "WAN" VRF, cleanly isolating Local/VPN and WAN 
Routing.

The userspace API is designed to transmit the device index of the device to 
listen on.
Listening on a device does only work if the socketdev_index is set/changed 
before the socket is brought up.

Signed-off-by: Marvin Gaube 
---
 drivers/net/wireguard/device.h  |  1 +
 drivers/net/wireguard/netlink.c |  8 +++-
 drivers/net/wireguard/socket.c  | 10 ++
 include/uapi/linux/wireguard.h  |  3 +++
 4 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireguard/device.h b/drivers/net/wireguard/device.h
index 854bc3d97150..a08f0893f687 100644
--- a/drivers/net/wireguard/device.h
+++ b/drivers/net/wireguard/device.h
@@ -56,6 +56,7 @@ struct wg_device {
struct list_head device_list, peer_list;
unsigned int num_peers, device_update_gen;
u32 fwmark;
+   u32 socketdev_index;
u16 incoming_port;
 };
 
diff --git a/drivers/net/wireguard/netlink.c b/drivers/net/wireguard/netlink.c
index d0f3b6d7f408..c594a7063038 100644
--- a/drivers/net/wireguard/netlink.c
+++ b/drivers/net/wireguard/netlink.c
@@ -27,7 +27,8 @@ static const struct nla_policy device_policy[WGDEVICE_A_MAX + 
1] = {
[WGDEVICE_A_FLAGS]  = { .type = NLA_U32 },
[WGDEVICE_A_LISTEN_PORT]= { .type = NLA_U16 },
[WGDEVICE_A_FWMARK] = { .type = NLA_U32 },
-   [WGDEVICE_A_PEERS]  = { .type = NLA_NESTED }
+   [WGDEVICE_A_PEERS]  = { .type = NLA_NESTED },
+   [WGDEVICE_A_SOCKETDEV_INDEX]= { .type = NLA_U32 }
 };
 
 static const struct nla_policy peer_policy[WGPEER_A_MAX + 1] = {
@@ -232,6 +233,7 @@ static int wg_get_device_dump(struct sk_buff *skb, struct 
netlink_callback *cb)
if (nla_put_u16(skb, WGDEVICE_A_LISTEN_PORT,
wg->incoming_port) ||
nla_put_u32(skb, WGDEVICE_A_FWMARK, wg->fwmark) ||
+   nla_put_u32(skb, WGDEVICE_A_SOCKETDEV_INDEX, 
wg->socketdev_index) ||
nla_put_u32(skb, WGDEVICE_A_IFINDEX, wg->dev->ifindex) ||
nla_put_string(skb, WGDEVICE_A_IFNAME, wg->dev->name))
goto out;
@@ -538,6 +540,10 @@ static int wg_set_device(struct sk_buff *skb, struct 
genl_info *info)
goto out;
}
 
+   if (info->attrs[WGDEVICE_A_SOCKETDEV_INDEX]) {
+   wg->socketdev_index = 
nla_get_u32(info->attrs[WGDEVICE_A_SOCKETDEV_INDEX]);
+   }
+
if (flags & WGDEVICE_F_REPLACE_PEERS)
wg_peer_remove_all(wg);
 
diff --git a/drivers/net/wireguard/socket.c b/drivers/net/wireguard/socket.c
index d9ad850daa79..885d8a890079 100644
--- a/drivers/net/wireguard/socket.c
+++ b/drivers/net/wireguard/socket.c
@@ -370,8 +370,18 @@ int wg_socket_init(struct wg_device *wg, u16 port)
.use_udp6_rx_checksums = true,
.ipv6_v6only = true
};
+   if (wg->socketdev_index > 0) {
+   port6.bind_ifindex = wg->socketdev_index;
+   } else {
+   port6.bind_ifindex = 0;
+   }
 #endif
 
+   if (wg->socketdev_index > 0) {
+   port4.bind_ifindex = wg->socketdev_index;
+   } else {
+   port4.bind_ifindex = 0;
+   }
rcu_read_lock();
net = rcu_dereference(wg->creating_net);
net = net ? maybe_get_net(net) : NULL;
diff --git a/include/uapi/linux/wireguard.h b/include/uapi/linux/wireguard.h
index ae88be14c947..52b1bd422f50 100644
--- a/include/uapi/linux/wireguard.h
+++ b/include/uapi/linux/wireguard.h
@@ -29,6 +29,7 @@
  *WGDEVICE_A_PUBLIC_KEY: NLA_EXACT_LEN, len WG_KEY_LEN
  *WGDEVICE_A_LISTEN_PORT: NLA_U16
  *WGDEVICE_A_FWMARK: NLA_U32
+ *WGDEVICE_A_SOCKETDEV_INDEX: NLA_U32
  *WGDEVICE_A_PEERS: NLA_NESTED
  *0: NLA_NESTED
  *WGPEER_A_PUBLIC_KEY: NLA_EXACT_LEN, len WG_KEY_LEN
@@ -83,6 +84,7 @@
  *WGDEVICE_A_PRIVATE_KEY: len WG_KEY_LEN, all zeros to remove
  *WGDEVICE_A_LISTEN_PORT: NLA_U16, 0 to choose randomly
  *WGDEVICE_A_FWMARK: NLA_U32, 0 to disable
+ *WGDEVICE_A_SOCKETDEV_INDEX: NLA_U32, 0 to disable
  *WGDEVICE_A_PEERS: NLA_NESTED
  *0: NLA_NESTED
  *WGPEER_A_PUBLIC_KEY: len WG_KEY_LEN
@@ -157,6 +159,7 @@ enum wgdevice_attribute {
WGDEVICE_A_LISTEN_PORT,
WGDEVICE_A_FWMARK,
WGDEVICE_A_PEERS,
+   WGDEVICE_A_SOCKETDEV_INDEX,
__WGDEVICE_A_LAST
 };
 #define WGDEVICE_A_MAX (__WGDEVICE_A_LAST - 1)
-- 
2.25.1



Re: potentially disallowing IP fragmentation on wg packets, and handling routing loops better

2021-06-06 Thread Peter Linder
This would break things for me. We're doing a lot of L2 over L3 site to 
site stuff and we are using wireguard as the outer layer. Inner layer is 
vxlan or l2tpv3.


In particular, people connect lots of stuff with no regard for MTU. For 
some things it's also very hard to change so we just assume people 
don't. Since the L3 network typically has the same MTU as the inner L2 
network, we need fragmentation. There is no practical way to be able to 
tell hosts on the L2 network about the limited mtu, for all we know they 
don't even run IP


It really does work without a hassle, it is not very very slow at all. 
Performance is down perhaps by a factor of 3 compared to setting a 
smaller MTU/MSS, but we can still push 350mbit/s with an atom 2ghz cpu, 
and around 800mbit/s with a xeon cpu, with fragmentation for most 
packets. This is one case where wireguard really works well!


IMHO, having wireguard generating fragmentable packets adds a lot to its 
usefulness. With that said, it's not the end of the world for me as I 
can just compile my own but I'd rather not :-)



On 2021-06-06 11:13, Jason A. Donenfeld wrote:

Hi,

WireGuard is an encrypted point-to-multipoint tunnel, where onion
layering of packets via a single interface or multiple is a useful
feature. This makes handling routing loops very hard to manage and
detect. I'm considering changing and simplifying loop mitigation to a
different strategy, but not without some discussion of its
implications.

Specifically the change would be to not allow IP fragmentation of the
encrypted UDP packets. This way, in the case of a loop, eventually the
packet size exceeds MTU, and it gets dropped: dumb and effective.
Depending on how this discussion goes, a compromise would be to not
allow fragmentation, but only for forwarded and kernel-generated
packets, not not for locally generated userspace packets. That's more
complex and I don't like it as much as just disallowing IP
fragmentation all together.

Pros:
- It solves the routing loop problem very simply.
- Usually when people are fragmenting packets like that, things become
very, very slow anyway, and it'd be better to just stop working
entirely, so that people adjust their MTU.
- Is anybody actually relying on this?

Cons:
- Maybe people are running
wireguard-over-gre-over-vxlan-over-l2tp-over-pppoe-over-god-knows-what-else,
and this reduces the MTU to below 1280, yet they still want to put
IPv6 through wireguard, and are willing to accept the performance
implications.
- Some people don't know how to fix their MTUs, and breaking rather
than just becoming really slow isn't the best outcome there, maybe.
- Maybe people are relying on this?

Before anybody asks: we're not going to add a knob for this, simply by
virtue of this being a decision with pros and cons. Please don't bring
that up.

I'd be very interested in opinions about this. Are there additional
pros and cons? I know the matter has come up a few times on the list,
mostly with people _wanting_ fragmentation (I've CCd a few people from
those threads - Roman, I expect you to vigorously argue the
pro-fragmentation stance ;-). but I'm not convinced the outcome of
those threads was correct, other than, "yea, that's easy enough to
enable." But on the other hand, maybe the cons are real enough we
should rethink this.

Please let me know thoughts and ideas.

Thanks,
Jason


Re: potentially disallowing IP fragmentation on wg packets, and handling routing loops better

2021-06-06 Thread Nico Schottelius


Hello,

so given that fragmentation is disallowed the PMTU discovery always
needs to work and the wireguard MTU needs to be correctly adjusted.

Speaking of a DC situation, I think this might be tricky. Imagine the
following situation:

- endhost A has an MTU of 9k. PMTU 9k. wg 8920.
- the path changes, the PMTU reduces to 1.5k (this is something we see
 happening from time to time)
- How is the wg MTU adjusted in this situation?

And to clarify: with disallowing IP frag, you are obviously only
referring to the outter transport. Within the tunnels, IPv6 and IPv6
packets can still be fragmented, so application operation is not really
affected.

Interesting approach, I am not really sure if realisticly feasible,
especially when thinking about long range/low bandwidth media where
you'd basically say "wg cannot do IPv6 on these mediums". Satelite
systems should probably work fine, I am more concerned about mesh
networks, in which wg is quite popular already.

Cheers,

Nico

--
Sustainable and modern Infrastructures by ungleich.ch


Re: potentially disallowing IP fragmentation on wg packets, and handling routing loops better

2021-06-06 Thread Vasili Pupkin

Hi,

I've dig into the subject two years ago and only vague remember details. 
As far as I can recall there was a time when WireGuard set DF flag by 
default and there were two issues:


1) for security reasons WireGuard doesn't issue ICMP fragmentation 
required response in the unencrypted channel if an encrypted packed 
didn't fit and was dropped
2) there is no way client can tell the server of MTU limitation it has 
on its side


Combining the two we have a situation in a chained wireguard VPN setup 
when MTU size is misconfigured on the server and the remote host 
wouldn't get any icmp to help with its PMTUD algorithm. The client can 
still set MSS in its TCP connection though.


Again sorry if I missed or messed something, it was long ago and I don't 
remember details.



On 06.06.2021 12:13, Jason A. Donenfeld wrote:

Hi,

WireGuard is an encrypted point-to-multipoint tunnel, where onion
layering of packets via a single interface or multiple is a useful
feature. This makes handling routing loops very hard to manage and
detect. I'm considering changing and simplifying loop mitigation to a
different strategy, but not without some discussion of its
implications.

Specifically the change would be to not allow IP fragmentation of the
encrypted UDP packets. This way, in the case of a loop, eventually the
packet size exceeds MTU, and it gets dropped: dumb and effective.
Depending on how this discussion goes, a compromise would be to not
allow fragmentation, but only for forwarded and kernel-generated
packets, not not for locally generated userspace packets. That's more
complex and I don't like it as much as just disallowing IP
fragmentation all together.

Pros:
- It solves the routing loop problem very simply.
- Usually when people are fragmenting packets like that, things become
very, very slow anyway, and it'd be better to just stop working
entirely, so that people adjust their MTU.
- Is anybody actually relying on this?

Cons:
- Maybe people are running
wireguard-over-gre-over-vxlan-over-l2tp-over-pppoe-over-god-knows-what-else,
and this reduces the MTU to below 1280, yet they still want to put
IPv6 through wireguard, and are willing to accept the performance
implications.
- Some people don't know how to fix their MTUs, and breaking rather
than just becoming really slow isn't the best outcome there, maybe.
- Maybe people are relying on this?

Before anybody asks: we're not going to add a knob for this, simply by
virtue of this being a decision with pros and cons. Please don't bring
that up.

I'd be very interested in opinions about this. Are there additional
pros and cons? I know the matter has come up a few times on the list,
mostly with people _wanting_ fragmentation (I've CCd a few people from
those threads - Roman, I expect you to vigorously argue the
pro-fragmentation stance ;-). but I'm not convinced the outcome of
those threads was correct, other than, "yea, that's easy enough to
enable." But on the other hand, maybe the cons are real enough we
should rethink this.

Please let me know thoughts and ideas.

Thanks,
Jason





potentially disallowing IP fragmentation on wg packets, and handling routing loops better

2021-06-06 Thread Jason A. Donenfeld
Hi,

WireGuard is an encrypted point-to-multipoint tunnel, where onion
layering of packets via a single interface or multiple is a useful
feature. This makes handling routing loops very hard to manage and
detect. I'm considering changing and simplifying loop mitigation to a
different strategy, but not without some discussion of its
implications.

Specifically the change would be to not allow IP fragmentation of the
encrypted UDP packets. This way, in the case of a loop, eventually the
packet size exceeds MTU, and it gets dropped: dumb and effective.
Depending on how this discussion goes, a compromise would be to not
allow fragmentation, but only for forwarded and kernel-generated
packets, not not for locally generated userspace packets. That's more
complex and I don't like it as much as just disallowing IP
fragmentation all together.

Pros:
- It solves the routing loop problem very simply.
- Usually when people are fragmenting packets like that, things become
very, very slow anyway, and it'd be better to just stop working
entirely, so that people adjust their MTU.
- Is anybody actually relying on this?

Cons:
- Maybe people are running
wireguard-over-gre-over-vxlan-over-l2tp-over-pppoe-over-god-knows-what-else,
and this reduces the MTU to below 1280, yet they still want to put
IPv6 through wireguard, and are willing to accept the performance
implications.
- Some people don't know how to fix their MTUs, and breaking rather
than just becoming really slow isn't the best outcome there, maybe.
- Maybe people are relying on this?

Before anybody asks: we're not going to add a knob for this, simply by
virtue of this being a decision with pros and cons. Please don't bring
that up.

I'd be very interested in opinions about this. Are there additional
pros and cons? I know the matter has come up a few times on the list,
mostly with people _wanting_ fragmentation (I've CCd a few people from
those threads - Roman, I expect you to vigorously argue the
pro-fragmentation stance ;-). but I'm not convinced the outcome of
those threads was correct, other than, "yea, that's easy enough to
enable." But on the other hand, maybe the cons are real enough we
should rethink this.

Please let me know thoughts and ideas.

Thanks,
Jason