Re: [PATCH] xfrm: esp6: fix off-by-one IV counter causing AES-GCM nonce reuse
From: Jérémy Jean
Date: Mon Sep 28 2026 - 15:48:36 EST
Hello Sabrina,
On 2026-09-28 18:19, Sabrina Dubroca wrote:
The subject prefix should be "PATCH ipsec" for IPsec bugfixes.
I will try to think about that next time.
2026-09-25, 09:51:06 +0000, Jérémy Jean wrote:
An off-by-one error in esp6_xmit() advances the IV counter before
encrypting each software-GSO segment. For N segments with sequence
numbers X through X+N-1, the IV counters are therefore X+1 through X+N.
The following non-GSO packet uses X+N for both its sequence number and
IV counter, repeating the last segment's AES-GCM nonce under the same
key.
I find this description very unclear. All I'm managing to understand
from this is "there's some situation where a packet isn't getting the
seqno it should". I don't know where the "+1" comes from since for GSO
the function does +N (xo->seq.low += skb_shinfo(skb)->gso_segs).
I tried to be as explicit as possible, but I apologize if it was not
good enough. My understanding on the full GSO processing isn't as
deep as yours, so here is another try at explaining.
The bug happens after software segmentation in GSO. When a large
amount of data needs to span across several packets, software
segmentation splits it into N smaller skb. After this split, each
smaller skb holding the individual packets has skb_is_gso(skb)
returning false, yet each skb keeps the flag XFRM_GSO_SEGMENT stating
that this skb resulted from a segmentation. Consequently, the skb
goes through the increment below in esp6_xmit():
net/ipv6/esp6_offload.c:
355 if (xo->flags & XFRM_GSO_SEGMENT) {
356 esp.esph->seq_no = htonl(seq);
357
358 if (!skb_is_gso(skb))
359 xo->seq.low++; // <<< increment here
360 else
361 xo->seq.low += skb_shinfo(skb)->gso_segs;
362 }
There are N calls to esp6_xmit() for all the smaller packets, and for
each of them, the current sequence number is first written into the
header, and then the shared counter for the next packet is
incremented. However, the value esp.seqno used to construct the IV is
derived from the counter value _after_ the increment. For example, if
the last packet produced by segmentation has sequence number 100, the
IV is constructed using counter value 101. Then, a subsequent
ordinary packet not going through segmentation is allocated sequence
number 101, yet since it does not have the flag XFRM_GSO_SEGMENT,
there is no increment, and its value is constructed from value 101 as
well. Hence the nonce repetition.
The fix proposes to move the computation of esp.seqno _before_ the
increment.
Anyway, one process nit and one question on the code:
The repeated nonce allows first a passive attacker who knows partial
plaintext from one packet to recover corresponding bytes from another
one, and second, an active attacker to recover GCM authentication key
to forge authentication tags without recovering the AES key.
Fix this by saving the complete current sequence number in esp.seqno
before advancing the shared GSO sequence state.
Fixes: 3dca3f38cfb8 ("xfrm: Separate ESP handling from segmentation for GRO packets.")
And if there's a crypto leak, this should probably have a "Cc: stable"
tag.
Noted, thanks.
Assisted-by: LLM
Signed-off-by: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
---
net/ipv6/esp6_offload.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 2289552..05d13cc 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -346,6 +346,7 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff *skb, netdev_features
}
seq = xo->seq.low;
+ esp.seqno = cpu_to_be64(seq + ((u64)xo->seq.hi << 32));
esp.esph = ip_esp_hdr(skb);
esp.esph->spi = x->id.spi;
@@ -364,8 +365,6 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff *skb, netdev_features
if (xo->seq.low < seq)
xo->seq.hi++;
- esp.seqno = cpu_to_be64(xo->seq.low + ((u64)xo->seq.hi << 32));
But then esp.seqno can have an inconsistent view of xo->seq.hi
compared to what esp6_output_tail/esp_output_set_esn will see
(xo->seq.hi++ just above this)?
This looks like another bug, similar to the boundary case I described here for ipv4?
https://lore.kernel.org/all/20260925095128.446450-2-Jeremy.Jean@xxxxxxxxxxxxxxxxx/
Regards,
Jérémy