netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/5] wireguard fixes for 5.6-rc7
@ 2020-03-19  0:30 Jason A. Donenfeld
  2020-03-19  0:30 ` [PATCH net 1/5] wireguard: selftests: remove duplicated include <sys/types.h> Jason A. Donenfeld
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Jason A. Donenfeld @ 2020-03-19  0:30 UTC (permalink / raw)
  To: davem, netdev; +Cc: Jason A. Donenfeld

Hi Dave,

I originally intended to spend this cycle working on fun optimizations
and architecture for WireGuard for 5.7, but I've been a bit neurotic
about having 5.6 ship without any show stopper bugs. WireGuard has been
stable for a long time now, but that doesn't make me any less nervous
about the real deal in 5.6. To that end, I've been doing code reviews
and having discussions, and we also had a security firm audit the code.
That audit didn't turn up any vulnerabilities, but they did make a good
defense-in-depth suggestion. This series contains:

1) Removal of a duplicated header, from YueHaibing.
2) Testing with 64-bit time in our test suite.
3) Account for skb->protocol==0 due to AF_PACKET sockets, suggested
   by Florian Fainelli.
4) Clean up some code in an unreachable switch/case branch, suggested
   by Florian Fainelli.
5) Better handling of low-order points, discussed with Mathias
   Hall-Andersen.

Thanks,
Jason

Jason A. Donenfeld (4):
  wireguard: selftests: test using new 64-bit time_t
  wireguard: queueing: account for skb->protocol==0
  wireguard: receive: remove dead code from default packet type case
  wireguard: noise: error out precomputed DH during handshake rather
    than config

YueHaibing (1):
  wireguard: selftests: remove duplicated include <sys/types.h>

 drivers/net/wireguard/device.c                |  2 +-
 drivers/net/wireguard/netlink.c               |  8 +--
 drivers/net/wireguard/noise.c                 | 55 ++++++++++---------
 drivers/net/wireguard/noise.h                 | 12 ++--
 drivers/net/wireguard/peer.c                  |  7 +--
 drivers/net/wireguard/queueing.h              |  8 ++-
 drivers/net/wireguard/receive.c               |  7 +--
 tools/testing/selftests/wireguard/netns.sh    |  6 --
 .../testing/selftests/wireguard/qemu/Makefile |  2 +-
 tools/testing/selftests/wireguard/qemu/init.c |  1 -
 .../selftests/wireguard/qemu/kernel.config    |  1 -
 11 files changed, 51 insertions(+), 58 deletions(-)

-- 
2.25.1


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

* [PATCH net 1/5] wireguard: selftests: remove duplicated include <sys/types.h>
  2020-03-19  0:30 [PATCH net 0/5] wireguard fixes for 5.6-rc7 Jason A. Donenfeld
@ 2020-03-19  0:30 ` Jason A. Donenfeld
  2020-03-19  0:30 ` [PATCH net 2/5] wireguard: selftests: test using new 64-bit time_t Jason A. Donenfeld
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Jason A. Donenfeld @ 2020-03-19  0:30 UTC (permalink / raw)
  To: davem, netdev; +Cc: YueHaibing, Jason A . Donenfeld

From: YueHaibing <yuehaibing@huawei.com>

This commit removes a duplicated include.

Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/testing/selftests/wireguard/qemu/init.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tools/testing/selftests/wireguard/qemu/init.c b/tools/testing/selftests/wireguard/qemu/init.c
index 90bc9813cadc..c9698120ac9d 100644
--- a/tools/testing/selftests/wireguard/qemu/init.c
+++ b/tools/testing/selftests/wireguard/qemu/init.c
@@ -13,7 +13,6 @@
 #include <fcntl.h>
 #include <sys/wait.h>
 #include <sys/mount.h>
-#include <sys/types.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/io.h>
-- 
2.25.1


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

* [PATCH net 2/5] wireguard: selftests: test using new 64-bit time_t
  2020-03-19  0:30 [PATCH net 0/5] wireguard fixes for 5.6-rc7 Jason A. Donenfeld
  2020-03-19  0:30 ` [PATCH net 1/5] wireguard: selftests: remove duplicated include <sys/types.h> Jason A. Donenfeld
@ 2020-03-19  0:30 ` Jason A. Donenfeld
  2020-03-19  0:30 ` [PATCH net 3/5] wireguard: queueing: account for skb->protocol==0 Jason A. Donenfeld
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Jason A. Donenfeld @ 2020-03-19  0:30 UTC (permalink / raw)
  To: davem, netdev; +Cc: Jason A. Donenfeld

In case this helps expose bugs with the newer 64-bit time_t types, we do
our testing with the newer musl that supports this as well as
CONFIG_COMPAT_32BIT_TIME=n. This matters to us, since wireguard does in
fact deal with timestamps.

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/testing/selftests/wireguard/qemu/Makefile      | 2 +-
 tools/testing/selftests/wireguard/qemu/kernel.config | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/testing/selftests/wireguard/qemu/Makefile b/tools/testing/selftests/wireguard/qemu/Makefile
index 28d477683e8a..90598a425c18 100644
--- a/tools/testing/selftests/wireguard/qemu/Makefile
+++ b/tools/testing/selftests/wireguard/qemu/Makefile
@@ -41,7 +41,7 @@ $(DISTFILES_PATH)/$(1):
 	flock -x $$@.lock -c '[ -f $$@ ] && exit 0; wget -O $$@.tmp $(MIRROR)$(1) || wget -O $$@.tmp $(2)$(1) || rm -f $$@.tmp; [ -f $$@.tmp ] || exit 1; if echo "$(3)  $$@.tmp" | sha256sum -c -; then mv $$@.tmp $$@; else rm -f $$@.tmp; exit 71; fi'
 endef
 
-$(eval $(call tar_download,MUSL,musl,1.1.24,.tar.gz,https://www.musl-libc.org/releases/,1370c9a812b2cf2a7d92802510cca0058cc37e66a7bedd70051f0a34015022a3))
+$(eval $(call tar_download,MUSL,musl,1.2.0,.tar.gz,https://musl.libc.org/releases/,c6de7b191139142d3f9a7b5b702c9cae1b5ee6e7f57e582da9328629408fd4e8))
 $(eval $(call tar_download,IPERF,iperf,3.7,.tar.gz,https://downloads.es.net/pub/iperf/,d846040224317caf2f75c843d309a950a7db23f9b44b94688ccbe557d6d1710c))
 $(eval $(call tar_download,BASH,bash,5.0,.tar.gz,https://ftp.gnu.org/gnu/bash/,b4a80f2ac66170b2913efbfb9f2594f1f76c7b1afd11f799e22035d63077fb4d))
 $(eval $(call tar_download,IPROUTE2,iproute2,5.4.0,.tar.xz,https://www.kernel.org/pub/linux/utils/net/iproute2/,fe97aa60a0d4c5ac830be18937e18dc3400ca713a33a89ad896ff1e3d46086ae))
diff --git a/tools/testing/selftests/wireguard/qemu/kernel.config b/tools/testing/selftests/wireguard/qemu/kernel.config
index af9323a0b6e0..d531de13c95b 100644
--- a/tools/testing/selftests/wireguard/qemu/kernel.config
+++ b/tools/testing/selftests/wireguard/qemu/kernel.config
@@ -56,7 +56,6 @@ CONFIG_NO_HZ_IDLE=y
 CONFIG_NO_HZ_FULL=n
 CONFIG_HZ_PERIODIC=n
 CONFIG_HIGH_RES_TIMERS=y
-CONFIG_COMPAT_32BIT_TIME=y
 CONFIG_ARCH_RANDOM=y
 CONFIG_FILE_LOCKING=y
 CONFIG_POSIX_TIMERS=y
-- 
2.25.1


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

* [PATCH net 3/5] wireguard: queueing: account for skb->protocol==0
  2020-03-19  0:30 [PATCH net 0/5] wireguard fixes for 5.6-rc7 Jason A. Donenfeld
  2020-03-19  0:30 ` [PATCH net 1/5] wireguard: selftests: remove duplicated include <sys/types.h> Jason A. Donenfeld
  2020-03-19  0:30 ` [PATCH net 2/5] wireguard: selftests: test using new 64-bit time_t Jason A. Donenfeld
@ 2020-03-19  0:30 ` Jason A. Donenfeld
  2020-03-19  0:30 ` [PATCH net 4/5] wireguard: receive: remove dead code from default packet type case Jason A. Donenfeld
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Jason A. Donenfeld @ 2020-03-19  0:30 UTC (permalink / raw)
  To: davem, netdev; +Cc: Jason A. Donenfeld

We carry out checks to the effect of:

  if (skb->protocol != wg_examine_packet_protocol(skb))
    goto err;

By having wg_skb_examine_untrusted_ip_hdr return 0 on failure, this
means that the check above still passes in the case where skb->protocol
is zero, which is possible to hit with AF_PACKET:

  struct sockaddr_pkt saddr = { .spkt_device = "wg0" };
  unsigned char buffer[5] = { 0 };
  sendto(socket(AF_PACKET, SOCK_PACKET, /* skb->protocol = */ 0),
         buffer, sizeof(buffer), 0, (const struct sockaddr *)&saddr, sizeof(saddr));

Additional checks mean that this isn't actually a problem in the code
base, but I could imagine it becoming a problem later if the function is
used more liberally.

I would prefer to fix this by having wg_examine_packet_protocol return a
32-bit ~0 value on failure, which will never match any value of
skb->protocol, which would simply change the generated code from a mov
to a movzx. However, sparse complains, and adding __force casts doesn't
seem like a good idea, so instead we just add a simple helper function
to check for the zero return value. Since wg_examine_packet_protocol
itself gets inlined, this winds up not adding an additional branch to
the generated code, since the 0 return value already happens in a
mergable branch.

Reported-by: Fabian Freyer <fabianfreyer@radicallyopensecurity.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/net/wireguard/device.c   | 2 +-
 drivers/net/wireguard/queueing.h | 8 +++++++-
 drivers/net/wireguard/receive.c  | 4 ++--
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireguard/device.c b/drivers/net/wireguard/device.c
index cdc96968b0f4..3ac3f8570ca1 100644
--- a/drivers/net/wireguard/device.c
+++ b/drivers/net/wireguard/device.c
@@ -122,7 +122,7 @@ static netdev_tx_t wg_xmit(struct sk_buff *skb, struct net_device *dev)
 	u32 mtu;
 	int ret;
 
-	if (unlikely(wg_skb_examine_untrusted_ip_hdr(skb) != skb->protocol)) {
+	if (unlikely(!wg_check_packet_protocol(skb))) {
 		ret = -EPROTONOSUPPORT;
 		net_dbg_ratelimited("%s: Invalid IP packet\n", dev->name);
 		goto err;
diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h
index fecb559cbdb6..cf1e0e2376d8 100644
--- a/drivers/net/wireguard/queueing.h
+++ b/drivers/net/wireguard/queueing.h
@@ -66,7 +66,7 @@ struct packet_cb {
 #define PACKET_PEER(skb) (PACKET_CB(skb)->keypair->entry.peer)
 
 /* Returns either the correct skb->protocol value, or 0 if invalid. */
-static inline __be16 wg_skb_examine_untrusted_ip_hdr(struct sk_buff *skb)
+static inline __be16 wg_examine_packet_protocol(struct sk_buff *skb)
 {
 	if (skb_network_header(skb) >= skb->head &&
 	    (skb_network_header(skb) + sizeof(struct iphdr)) <=
@@ -81,6 +81,12 @@ static inline __be16 wg_skb_examine_untrusted_ip_hdr(struct sk_buff *skb)
 	return 0;
 }
 
+static inline bool wg_check_packet_protocol(struct sk_buff *skb)
+{
+	__be16 real_protocol = wg_examine_packet_protocol(skb);
+	return real_protocol && skb->protocol == real_protocol;
+}
+
 static inline void wg_reset_packet(struct sk_buff *skb)
 {
 	skb_scrub_packet(skb, true);
diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
index 4a153894cee2..243ed7172dd2 100644
--- a/drivers/net/wireguard/receive.c
+++ b/drivers/net/wireguard/receive.c
@@ -56,7 +56,7 @@ static int prepare_skb_header(struct sk_buff *skb, struct wg_device *wg)
 	size_t data_offset, data_len, header_len;
 	struct udphdr *udp;
 
-	if (unlikely(wg_skb_examine_untrusted_ip_hdr(skb) != skb->protocol ||
+	if (unlikely(!wg_check_packet_protocol(skb) ||
 		     skb_transport_header(skb) < skb->head ||
 		     (skb_transport_header(skb) + sizeof(struct udphdr)) >
 			     skb_tail_pointer(skb)))
@@ -388,7 +388,7 @@ static void wg_packet_consume_data_done(struct wg_peer *peer,
 	 */
 	skb->ip_summed = CHECKSUM_UNNECESSARY;
 	skb->csum_level = ~0; /* All levels */
-	skb->protocol = wg_skb_examine_untrusted_ip_hdr(skb);
+	skb->protocol = wg_examine_packet_protocol(skb);
 	if (skb->protocol == htons(ETH_P_IP)) {
 		len = ntohs(ip_hdr(skb)->tot_len);
 		if (unlikely(len < sizeof(struct iphdr)))
-- 
2.25.1


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

* [PATCH net 4/5] wireguard: receive: remove dead code from default packet type case
  2020-03-19  0:30 [PATCH net 0/5] wireguard fixes for 5.6-rc7 Jason A. Donenfeld
                   ` (2 preceding siblings ...)
  2020-03-19  0:30 ` [PATCH net 3/5] wireguard: queueing: account for skb->protocol==0 Jason A. Donenfeld
@ 2020-03-19  0:30 ` Jason A. Donenfeld
  2020-03-19  0:30 ` [PATCH net 5/5] wireguard: noise: error out precomputed DH during handshake rather than config Jason A. Donenfeld
  2020-03-19  1:54 ` [PATCH net 0/5] wireguard fixes for 5.6-rc7 David Miller
  5 siblings, 0 replies; 9+ messages in thread
From: Jason A. Donenfeld @ 2020-03-19  0:30 UTC (permalink / raw)
  To: davem, netdev; +Cc: Jason A. Donenfeld

The situation in which we wind up hitting the default case here
indicates a major bug in earlier parsing code. It is not a usual thing
that should ever happen, which means a "friendly" message for it doesn't
make sense. Rather, replace this with a WARN_ON, just like we do earlier
in the file for a similar situation, so that somebody sends us a bug
report and we can fix it.

Reported-by: Fabian Freyer <fabianfreyer@radicallyopensecurity.com>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/net/wireguard/receive.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/wireguard/receive.c b/drivers/net/wireguard/receive.c
index 243ed7172dd2..da3b782ab7d3 100644
--- a/drivers/net/wireguard/receive.c
+++ b/drivers/net/wireguard/receive.c
@@ -587,8 +587,7 @@ void wg_packet_receive(struct wg_device *wg, struct sk_buff *skb)
 		wg_packet_consume_data(wg, skb);
 		break;
 	default:
-		net_dbg_skb_ratelimited("%s: Invalid packet from %pISpfsc\n",
-					wg->dev->name, skb);
+		WARN(1, "Non-exhaustive parsing of packet header lead to unknown packet type!\n");
 		goto err;
 	}
 	return;
-- 
2.25.1


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

* [PATCH net 5/5] wireguard: noise: error out precomputed DH during handshake rather than config
  2020-03-19  0:30 [PATCH net 0/5] wireguard fixes for 5.6-rc7 Jason A. Donenfeld
                   ` (3 preceding siblings ...)
  2020-03-19  0:30 ` [PATCH net 4/5] wireguard: receive: remove dead code from default packet type case Jason A. Donenfeld
@ 2020-03-19  0:30 ` Jason A. Donenfeld
  2020-03-19  1:54 ` [PATCH net 0/5] wireguard fixes for 5.6-rc7 David Miller
  5 siblings, 0 replies; 9+ messages in thread
From: Jason A. Donenfeld @ 2020-03-19  0:30 UTC (permalink / raw)
  To: davem, netdev; +Cc: Jason A. Donenfeld

We precompute the static-static ECDH during configuration time, in order
to save an expensive computation later when receiving network packets.
However, not all ECDH computations yield a contributory result. Prior,
we were just not letting those peers be added to the interface. However,
this creates a strange inconsistency, since it was still possible to add
other weird points, like a valid public key plus a low-order point, and,
like points that result in zeros, a handshake would not complete. In
order to make the behavior more uniform and less surprising, simply
allow all peers to be added. Then, we'll error out later when doing the
crypto if there's an issue. This also adds more separation between the
crypto layer and the configuration layer.

Discussed-with: Mathias Hall-Andersen <mathias@hall-andersen.dk>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 drivers/net/wireguard/netlink.c            |  8 +---
 drivers/net/wireguard/noise.c              | 55 ++++++++++++----------
 drivers/net/wireguard/noise.h              | 12 ++---
 drivers/net/wireguard/peer.c               |  7 +--
 tools/testing/selftests/wireguard/netns.sh | 15 ++++--
 5 files changed, 49 insertions(+), 48 deletions(-)

diff --git a/drivers/net/wireguard/netlink.c b/drivers/net/wireguard/netlink.c
index bda26405497c..802099c8828a 100644
--- a/drivers/net/wireguard/netlink.c
+++ b/drivers/net/wireguard/netlink.c
@@ -411,11 +411,7 @@ static int set_peer(struct wg_device *wg, struct nlattr **attrs)
 
 		peer = wg_peer_create(wg, public_key, preshared_key);
 		if (IS_ERR(peer)) {
-			/* Similar to the above, if the key is invalid, we skip
-			 * it without fanfare, so that services don't need to
-			 * worry about doing key validation themselves.
-			 */
-			ret = PTR_ERR(peer) == -EKEYREJECTED ? 0 : PTR_ERR(peer);
+			ret = PTR_ERR(peer);
 			peer = NULL;
 			goto out;
 		}
@@ -569,7 +565,7 @@ static int wg_set_device(struct sk_buff *skb, struct genl_info *info)
 							 private_key);
 		list_for_each_entry_safe(peer, temp, &wg->peer_list,
 					 peer_list) {
-			BUG_ON(!wg_noise_precompute_static_static(peer));
+			wg_noise_precompute_static_static(peer);
 			wg_noise_expire_current_peer_keypairs(peer);
 		}
 		wg_cookie_checker_precompute_device_keys(&wg->cookie_checker);
diff --git a/drivers/net/wireguard/noise.c b/drivers/net/wireguard/noise.c
index 919d9d866446..708dc61c974f 100644
--- a/drivers/net/wireguard/noise.c
+++ b/drivers/net/wireguard/noise.c
@@ -44,32 +44,23 @@ void __init wg_noise_init(void)
 }
 
 /* Must hold peer->handshake.static_identity->lock */
-bool wg_noise_precompute_static_static(struct wg_peer *peer)
+void wg_noise_precompute_static_static(struct wg_peer *peer)
 {
-	bool ret;
-
 	down_write(&peer->handshake.lock);
-	if (peer->handshake.static_identity->has_identity) {
-		ret = curve25519(
-			peer->handshake.precomputed_static_static,
+	if (!peer->handshake.static_identity->has_identity ||
+	    !curve25519(peer->handshake.precomputed_static_static,
 			peer->handshake.static_identity->static_private,
-			peer->handshake.remote_static);
-	} else {
-		u8 empty[NOISE_PUBLIC_KEY_LEN] = { 0 };
-
-		ret = curve25519(empty, empty, peer->handshake.remote_static);
+			peer->handshake.remote_static))
 		memset(peer->handshake.precomputed_static_static, 0,
 		       NOISE_PUBLIC_KEY_LEN);
-	}
 	up_write(&peer->handshake.lock);
-	return ret;
 }
 
-bool wg_noise_handshake_init(struct noise_handshake *handshake,
-			   struct noise_static_identity *static_identity,
-			   const u8 peer_public_key[NOISE_PUBLIC_KEY_LEN],
-			   const u8 peer_preshared_key[NOISE_SYMMETRIC_KEY_LEN],
-			   struct wg_peer *peer)
+void wg_noise_handshake_init(struct noise_handshake *handshake,
+			     struct noise_static_identity *static_identity,
+			     const u8 peer_public_key[NOISE_PUBLIC_KEY_LEN],
+			     const u8 peer_preshared_key[NOISE_SYMMETRIC_KEY_LEN],
+			     struct wg_peer *peer)
 {
 	memset(handshake, 0, sizeof(*handshake));
 	init_rwsem(&handshake->lock);
@@ -81,7 +72,7 @@ bool wg_noise_handshake_init(struct noise_handshake *handshake,
 		       NOISE_SYMMETRIC_KEY_LEN);
 	handshake->static_identity = static_identity;
 	handshake->state = HANDSHAKE_ZEROED;
-	return wg_noise_precompute_static_static(peer);
+	wg_noise_precompute_static_static(peer);
 }
 
 static void handshake_zero(struct noise_handshake *handshake)
@@ -403,6 +394,19 @@ static bool __must_check mix_dh(u8 chaining_key[NOISE_HASH_LEN],
 	return true;
 }
 
+static bool __must_check mix_precomputed_dh(u8 chaining_key[NOISE_HASH_LEN],
+					    u8 key[NOISE_SYMMETRIC_KEY_LEN],
+					    const u8 precomputed[NOISE_PUBLIC_KEY_LEN])
+{
+	static u8 zero_point[NOISE_PUBLIC_KEY_LEN];
+	if (unlikely(!crypto_memneq(precomputed, zero_point, NOISE_PUBLIC_KEY_LEN)))
+		return false;
+	kdf(chaining_key, key, NULL, precomputed, NOISE_HASH_LEN,
+	    NOISE_SYMMETRIC_KEY_LEN, 0, NOISE_PUBLIC_KEY_LEN,
+	    chaining_key);
+	return true;
+}
+
 static void mix_hash(u8 hash[NOISE_HASH_LEN], const u8 *src, size_t src_len)
 {
 	struct blake2s_state blake;
@@ -531,10 +535,9 @@ wg_noise_handshake_create_initiation(struct message_handshake_initiation *dst,
 			NOISE_PUBLIC_KEY_LEN, key, handshake->hash);
 
 	/* ss */
-	kdf(handshake->chaining_key, key, NULL,
-	    handshake->precomputed_static_static, NOISE_HASH_LEN,
-	    NOISE_SYMMETRIC_KEY_LEN, 0, NOISE_PUBLIC_KEY_LEN,
-	    handshake->chaining_key);
+	if (!mix_precomputed_dh(handshake->chaining_key, key,
+				handshake->precomputed_static_static))
+		goto out;
 
 	/* {t} */
 	tai64n_now(timestamp);
@@ -595,9 +598,9 @@ wg_noise_handshake_consume_initiation(struct message_handshake_initiation *src,
 	handshake = &peer->handshake;
 
 	/* ss */
-	kdf(chaining_key, key, NULL, handshake->precomputed_static_static,
-	    NOISE_HASH_LEN, NOISE_SYMMETRIC_KEY_LEN, 0, NOISE_PUBLIC_KEY_LEN,
-	    chaining_key);
+	if (!mix_precomputed_dh(chaining_key, key,
+				handshake->precomputed_static_static))
+	    goto out;
 
 	/* {t} */
 	if (!message_decrypt(t, src->encrypted_timestamp,
diff --git a/drivers/net/wireguard/noise.h b/drivers/net/wireguard/noise.h
index 138a07bb817c..f532d59d3f19 100644
--- a/drivers/net/wireguard/noise.h
+++ b/drivers/net/wireguard/noise.h
@@ -94,11 +94,11 @@ struct noise_handshake {
 struct wg_device;
 
 void wg_noise_init(void);
-bool wg_noise_handshake_init(struct noise_handshake *handshake,
-			   struct noise_static_identity *static_identity,
-			   const u8 peer_public_key[NOISE_PUBLIC_KEY_LEN],
-			   const u8 peer_preshared_key[NOISE_SYMMETRIC_KEY_LEN],
-			   struct wg_peer *peer);
+void wg_noise_handshake_init(struct noise_handshake *handshake,
+			     struct noise_static_identity *static_identity,
+			     const u8 peer_public_key[NOISE_PUBLIC_KEY_LEN],
+			     const u8 peer_preshared_key[NOISE_SYMMETRIC_KEY_LEN],
+			     struct wg_peer *peer);
 void wg_noise_handshake_clear(struct noise_handshake *handshake);
 static inline void wg_noise_reset_last_sent_handshake(atomic64_t *handshake_ns)
 {
@@ -116,7 +116,7 @@ void wg_noise_expire_current_peer_keypairs(struct wg_peer *peer);
 void wg_noise_set_static_identity_private_key(
 	struct noise_static_identity *static_identity,
 	const u8 private_key[NOISE_PUBLIC_KEY_LEN]);
-bool wg_noise_precompute_static_static(struct wg_peer *peer);
+void wg_noise_precompute_static_static(struct wg_peer *peer);
 
 bool
 wg_noise_handshake_create_initiation(struct message_handshake_initiation *dst,
diff --git a/drivers/net/wireguard/peer.c b/drivers/net/wireguard/peer.c
index 071eedf33f5a..1d634bd3038f 100644
--- a/drivers/net/wireguard/peer.c
+++ b/drivers/net/wireguard/peer.c
@@ -34,11 +34,8 @@ struct wg_peer *wg_peer_create(struct wg_device *wg,
 		return ERR_PTR(ret);
 	peer->device = wg;
 
-	if (!wg_noise_handshake_init(&peer->handshake, &wg->static_identity,
-				     public_key, preshared_key, peer)) {
-		ret = -EKEYREJECTED;
-		goto err_1;
-	}
+	wg_noise_handshake_init(&peer->handshake, &wg->static_identity,
+				public_key, preshared_key, peer);
 	if (dst_cache_init(&peer->endpoint_cache, GFP_KERNEL))
 		goto err_1;
 	if (wg_packet_queue_init(&peer->tx_queue, wg_packet_tx_worker, false,
diff --git a/tools/testing/selftests/wireguard/netns.sh b/tools/testing/selftests/wireguard/netns.sh
index 138d46b3f330..936e1ca9410e 100755
--- a/tools/testing/selftests/wireguard/netns.sh
+++ b/tools/testing/selftests/wireguard/netns.sh
@@ -527,11 +527,16 @@ n0 wg set wg0 peer "$pub2" allowed-ips 0.0.0.0/0
 n0 wg set wg0 peer "$pub2" allowed-ips ::/0,1700::/111,5000::/4,e000::/37,9000::/75
 n0 wg set wg0 peer "$pub2" allowed-ips ::/0
 n0 wg set wg0 peer "$pub2" remove
-low_order_points=( AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= AQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= 4Ot6fDtBuK4WVuP68Z/EatoJjeucMrH9hmIFFl9JuAA= X5yVvKNQjCSx0LFVnIPvWwREXMRYHI6G2CJO3dCfEVc= 7P///////////////////////////////////////38= 7f///////////////////////////////////////38= 7v///////////////////////////////////////38= )
-n0 wg set wg0 private-key /dev/null ${low_order_points[@]/#/peer }
-[[ -z $(n0 wg show wg0 peers) ]]
-n0 wg set wg0 private-key <(echo "$key1") ${low_order_points[@]/#/peer }
-[[ -z $(n0 wg show wg0 peers) ]]
+for low_order_point in AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= AQAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= 4Ot6fDtBuK4WVuP68Z/EatoJjeucMrH9hmIFFl9JuAA= X5yVvKNQjCSx0LFVnIPvWwREXMRYHI6G2CJO3dCfEVc= 7P///////////////////////////////////////38= 7f///////////////////////////////////////38= 7v///////////////////////////////////////38=; do
+	n0 wg set wg0 peer "$low_order_point" persistent-keepalive 1 endpoint 127.0.0.1:1111
+done
+[[ -n $(n0 wg show wg0 peers) ]]
+exec 4< <(n0 ncat -l -u -p 1111)
+ncat_pid=$!
+waitncatudp $netns0 $ncat_pid
+ip0 link set wg0 up
+! read -r -n 1 -t 2 <&4 || false
+kill $ncat_pid
 ip0 link del wg0
 
 declare -A objects
-- 
2.25.1


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

* Re: [PATCH net 0/5] wireguard fixes for 5.6-rc7
  2020-03-19  0:30 [PATCH net 0/5] wireguard fixes for 5.6-rc7 Jason A. Donenfeld
                   ` (4 preceding siblings ...)
  2020-03-19  0:30 ` [PATCH net 5/5] wireguard: noise: error out precomputed DH during handshake rather than config Jason A. Donenfeld
@ 2020-03-19  1:54 ` David Miller
  2020-03-19  2:30   ` Jason A. Donenfeld
  5 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2020-03-19  1:54 UTC (permalink / raw)
  To: Jason; +Cc: netdev

From: "Jason A. Donenfeld" <Jason@zx2c4.com>
Date: Wed, 18 Mar 2020 18:30:42 -0600

> I originally intended to spend this cycle working on fun optimizations
> and architecture for WireGuard for 5.7, but I've been a bit neurotic
> about having 5.6 ship without any show stopper bugs. WireGuard has been
> stable for a long time now, but that doesn't make me any less nervous
> about the real deal in 5.6. To that end, I've been doing code reviews
> and having discussions, and we also had a security firm audit the code.
> That audit didn't turn up any vulnerabilities, but they did make a good
> defense-in-depth suggestion. This series contains:
> 
> 1) Removal of a duplicated header, from YueHaibing.
> 2) Testing with 64-bit time in our test suite.
> 3) Account for skb->protocol==0 due to AF_PACKET sockets, suggested
>    by Florian Fainelli.
> 4) Clean up some code in an unreachable switch/case branch, suggested
>    by Florian Fainelli.
> 5) Better handling of low-order points, discussed with Mathias
>    Hall-Andersen.

Series applied, please start providing appropriate Fixes: tags in the
future.

Thank you.

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

* Re: [PATCH net 0/5] wireguard fixes for 5.6-rc7
  2020-03-19  1:54 ` [PATCH net 0/5] wireguard fixes for 5.6-rc7 David Miller
@ 2020-03-19  2:30   ` Jason A. Donenfeld
  0 siblings, 0 replies; 9+ messages in thread
From: Jason A. Donenfeld @ 2020-03-19  2:30 UTC (permalink / raw)
  To: David Miller; +Cc: Netdev

On Wed, Mar 18, 2020 at 7:55 PM David Miller <davem@davemloft.net> wrote:
> Series applied, please start providing appropriate Fixes: tags in the
> future.

No problem, will do.

Jason

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

* Re: [PATCH net 0/5] wireguard fixes for 5.6-rc7
@ 2023-01-28 20:18 Mostafa Shahdadi
  0 siblings, 0 replies; 9+ messages in thread
From: Mostafa Shahdadi @ 2023-01-28 20:18 UTC (permalink / raw)
  To: jason; +Cc: davem, netdev



Sent from my iPad 15.7


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

end of thread, other threads:[~2023-01-28 20:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-19  0:30 [PATCH net 0/5] wireguard fixes for 5.6-rc7 Jason A. Donenfeld
2020-03-19  0:30 ` [PATCH net 1/5] wireguard: selftests: remove duplicated include <sys/types.h> Jason A. Donenfeld
2020-03-19  0:30 ` [PATCH net 2/5] wireguard: selftests: test using new 64-bit time_t Jason A. Donenfeld
2020-03-19  0:30 ` [PATCH net 3/5] wireguard: queueing: account for skb->protocol==0 Jason A. Donenfeld
2020-03-19  0:30 ` [PATCH net 4/5] wireguard: receive: remove dead code from default packet type case Jason A. Donenfeld
2020-03-19  0:30 ` [PATCH net 5/5] wireguard: noise: error out precomputed DH during handshake rather than config Jason A. Donenfeld
2020-03-19  1:54 ` [PATCH net 0/5] wireguard fixes for 5.6-rc7 David Miller
2020-03-19  2:30   ` Jason A. Donenfeld
2023-01-28 20:18 Mostafa Shahdadi

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).