| b.liu | e958203 | 2025-04-17 19:18:16 +0800 | [diff] [blame] | 1 | From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 |
| 2 | From: "Jason A. Donenfeld" <Jason@zx2c4.com> |
| 3 | Date: Tue, 19 May 2020 22:49:28 -0600 |
| 4 | Subject: [PATCH] wireguard: noise: read preshared key while taking lock |
| 5 | |
| 6 | commit bc67d371256f5c47d824e2eec51e46c8d62d022e upstream. |
| 7 | |
| 8 | Prior we read the preshared key after dropping the handshake lock, which |
| 9 | isn't an actual crypto issue if it races, but it's still not quite |
| 10 | correct. So copy that part of the state into a temporary like we do with |
| 11 | the rest of the handshake state variables. Then we can release the lock, |
| 12 | operate on the temporary, and zero it out at the end of the function. In |
| 13 | performance tests, the impact of this was entirely unnoticable, probably |
| 14 | because those bytes are coming from the same cacheline as other things |
| 15 | that are being copied out in the same manner. |
| 16 | |
| 17 | Reported-by: Matt Dunwoodie <ncon@noconroy.net> |
| 18 | Fixes: e7096c131e51 ("net: WireGuard secure network tunnel") |
| 19 | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> |
| 20 | Signed-off-by: David S. Miller <davem@davemloft.net> |
| 21 | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> |
| 22 | --- |
| 23 | drivers/net/wireguard/noise.c | 6 +++++- |
| 24 | 1 file changed, 5 insertions(+), 1 deletion(-) |
| 25 | |
| 26 | --- a/drivers/net/wireguard/noise.c |
| 27 | +++ b/drivers/net/wireguard/noise.c |
| 28 | @@ -715,6 +715,7 @@ wg_noise_handshake_consume_response(stru |
| 29 | u8 e[NOISE_PUBLIC_KEY_LEN]; |
| 30 | u8 ephemeral_private[NOISE_PUBLIC_KEY_LEN]; |
| 31 | u8 static_private[NOISE_PUBLIC_KEY_LEN]; |
| 32 | + u8 preshared_key[NOISE_SYMMETRIC_KEY_LEN]; |
| 33 | |
| 34 | down_read(&wg->static_identity.lock); |
| 35 | |
| 36 | @@ -733,6 +734,8 @@ wg_noise_handshake_consume_response(stru |
| 37 | memcpy(chaining_key, handshake->chaining_key, NOISE_HASH_LEN); |
| 38 | memcpy(ephemeral_private, handshake->ephemeral_private, |
| 39 | NOISE_PUBLIC_KEY_LEN); |
| 40 | + memcpy(preshared_key, handshake->preshared_key, |
| 41 | + NOISE_SYMMETRIC_KEY_LEN); |
| 42 | up_read(&handshake->lock); |
| 43 | |
| 44 | if (state != HANDSHAKE_CREATED_INITIATION) |
| 45 | @@ -750,7 +753,7 @@ wg_noise_handshake_consume_response(stru |
| 46 | goto fail; |
| 47 | |
| 48 | /* psk */ |
| 49 | - mix_psk(chaining_key, hash, key, handshake->preshared_key); |
| 50 | + mix_psk(chaining_key, hash, key, preshared_key); |
| 51 | |
| 52 | /* {} */ |
| 53 | if (!message_decrypt(NULL, src->encrypted_nothing, |
| 54 | @@ -783,6 +786,7 @@ out: |
| 55 | memzero_explicit(chaining_key, NOISE_HASH_LEN); |
| 56 | memzero_explicit(ephemeral_private, NOISE_PUBLIC_KEY_LEN); |
| 57 | memzero_explicit(static_private, NOISE_PUBLIC_KEY_LEN); |
| 58 | + memzero_explicit(preshared_key, NOISE_SYMMETRIC_KEY_LEN); |
| 59 | up_read(&wg->static_identity.lock); |
| 60 | return ret_peer; |
| 61 | } |