| 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:29 -0600 |
| 4 | Subject: [PATCH] wireguard: queueing: preserve flow hash across packet |
| 5 | scrubbing |
| 6 | MIME-Version: 1.0 |
| 7 | Content-Type: text/plain; charset=UTF-8 |
| 8 | Content-Transfer-Encoding: 8bit |
| 9 | |
| 10 | commit c78a0b4a78839d572d8a80f6a62221c0d7843135 upstream. |
| 11 | |
| 12 | It's important that we clear most header fields during encapsulation and |
| 13 | decapsulation, because the packet is substantially changed, and we don't |
| 14 | want any info leak or logic bug due to an accidental correlation. But, |
| 15 | for encapsulation, it's wrong to clear skb->hash, since it's used by |
| 16 | fq_codel and flow dissection in general. Without it, classification does |
| 17 | not proceed as usual. This change might make it easier to estimate the |
| 18 | number of innerflows by examining clustering of out of order packets, |
| 19 | but this shouldn't open up anything that can't already be inferred |
| 20 | otherwise (e.g. syn packet size inference), and fq_codel can be disabled |
| 21 | anyway. |
| 22 | |
| 23 | Furthermore, it might be the case that the hash isn't used or queried at |
| 24 | all until after wireguard transmits the encrypted UDP packet, which |
| 25 | means skb->hash might still be zero at this point, and thus no hash |
| 26 | taken over the inner packet data. In order to address this situation, we |
| 27 | force a calculation of skb->hash before encrypting packet data. |
| 28 | |
| 29 | Of course this means that fq_codel might transmit packets slightly more |
| 30 | out of order than usual. Toke did some testing on beefy machines with |
| 31 | high quantities of parallel flows and found that increasing the |
| 32 | reply-attack counter to 8192 takes care of the most pathological cases |
| 33 | pretty well. |
| 34 | |
| 35 | Reported-by: Dave Taht <dave.taht@gmail.com> |
| 36 | Reviewed-and-tested-by: Toke Høiland-Jørgensen <toke@toke.dk> |
| 37 | Fixes: e7096c131e51 ("net: WireGuard secure network tunnel") |
| 38 | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> |
| 39 | Signed-off-by: David S. Miller <davem@davemloft.net> |
| 40 | Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> |
| 41 | --- |
| 42 | drivers/net/wireguard/messages.h | 2 +- |
| 43 | drivers/net/wireguard/queueing.h | 10 +++++++++- |
| 44 | drivers/net/wireguard/receive.c | 2 +- |
| 45 | drivers/net/wireguard/send.c | 7 ++++++- |
| 46 | 4 files changed, 17 insertions(+), 4 deletions(-) |
| 47 | |
| 48 | --- a/drivers/net/wireguard/messages.h |
| 49 | +++ b/drivers/net/wireguard/messages.h |
| 50 | @@ -32,7 +32,7 @@ enum cookie_values { |
| 51 | }; |
| 52 | |
| 53 | enum counter_values { |
| 54 | - COUNTER_BITS_TOTAL = 2048, |
| 55 | + COUNTER_BITS_TOTAL = 8192, |
| 56 | COUNTER_REDUNDANT_BITS = BITS_PER_LONG, |
| 57 | COUNTER_WINDOW_SIZE = COUNTER_BITS_TOTAL - COUNTER_REDUNDANT_BITS |
| 58 | }; |
| 59 | --- a/drivers/net/wireguard/queueing.h |
| 60 | +++ b/drivers/net/wireguard/queueing.h |
| 61 | @@ -87,12 +87,20 @@ static inline bool wg_check_packet_proto |
| 62 | return real_protocol && skb->protocol == real_protocol; |
| 63 | } |
| 64 | |
| 65 | -static inline void wg_reset_packet(struct sk_buff *skb) |
| 66 | +static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating) |
| 67 | { |
| 68 | + u8 l4_hash = skb->l4_hash; |
| 69 | + u8 sw_hash = skb->sw_hash; |
| 70 | + u32 hash = skb->hash; |
| 71 | skb_scrub_packet(skb, true); |
| 72 | memset(&skb->headers_start, 0, |
| 73 | offsetof(struct sk_buff, headers_end) - |
| 74 | offsetof(struct sk_buff, headers_start)); |
| 75 | + if (encapsulating) { |
| 76 | + skb->l4_hash = l4_hash; |
| 77 | + skb->sw_hash = sw_hash; |
| 78 | + skb->hash = hash; |
| 79 | + } |
| 80 | skb->queue_mapping = 0; |
| 81 | skb->nohdr = 0; |
| 82 | skb->peeked = 0; |
| 83 | --- a/drivers/net/wireguard/receive.c |
| 84 | +++ b/drivers/net/wireguard/receive.c |
| 85 | @@ -484,7 +484,7 @@ int wg_packet_rx_poll(struct napi_struct |
| 86 | if (unlikely(wg_socket_endpoint_from_skb(&endpoint, skb))) |
| 87 | goto next; |
| 88 | |
| 89 | - wg_reset_packet(skb); |
| 90 | + wg_reset_packet(skb, false); |
| 91 | wg_packet_consume_data_done(peer, skb, &endpoint); |
| 92 | free = false; |
| 93 | |
| 94 | --- a/drivers/net/wireguard/send.c |
| 95 | +++ b/drivers/net/wireguard/send.c |
| 96 | @@ -167,6 +167,11 @@ static bool encrypt_packet(struct sk_buf |
| 97 | struct sk_buff *trailer; |
| 98 | int num_frags; |
| 99 | |
| 100 | + /* Force hash calculation before encryption so that flow analysis is |
| 101 | + * consistent over the inner packet. |
| 102 | + */ |
| 103 | + skb_get_hash(skb); |
| 104 | + |
| 105 | /* Calculate lengths. */ |
| 106 | padding_len = calculate_skb_padding(skb); |
| 107 | trailer_len = padding_len + noise_encrypted_len(0); |
| 108 | @@ -295,7 +300,7 @@ void wg_packet_encrypt_worker(struct wor |
| 109 | skb_list_walk_safe(first, skb, next) { |
| 110 | if (likely(encrypt_packet(skb, |
| 111 | PACKET_CB(first)->keypair))) { |
| 112 | - wg_reset_packet(skb); |
| 113 | + wg_reset_packet(skb, true); |
| 114 | } else { |
| 115 | state = PACKET_STATE_DEAD; |
| 116 | break; |