Re: [PATCH] xfrm: esp6: fix off-by-one IV counter causing AES-GCM nonce reuse

[Sabrina Dubroca <sd@queasysnail.net>]

The subject prefix should be "PATCH ipsec" for IPsec bugfixes.

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


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.

> Assisted-by: LLM
> Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
> ---
>  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)?

-- 
Sabrina