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

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

[Jérémy Jean]

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.

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.")
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));
-
 	len = skb->len - sizeof(struct ipv6hdr);
 	if (len > IPV6_MAXPLEN)
 		len = 0;
-- 
2.47.3

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

[Sabrina Dubroca]

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

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

[Jérémy Jean]

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@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)?

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@oss.cyber.gouv.fr/

Regards,
Jérémy

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

[Sabrina Dubroca]

2026-09-28, 21:33:41 +0200, Jérémy Jean wrote:
> On 2026-09-28 18:19, Sabrina Dubroca wrote:
> > 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.

Ok, I see now. One call to validate_xmit_xfrm() that calls
skb_gso_segment() and feeds those N non-GSO skbs to ->xmit one by one.

In that case, the seqno used in the IV wouldn't match the one that the
peer will reconstruct using the bottom 32b of seqno present in the
header, and it would never manage to decrypt anything we sent.

But luckily, it seems commenting out the memcpy(iv, seqno) line has no
effect (I think that's because all algorithms rely on either seqiv or
echainiv).

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

I don't think that happens? The other packet will go through ->xmit
too and use the wrong seqno too.

-- 
Sabrina

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

[Jérémy Jean]

Hello Sabrina,

On 2026-09-29 01:48, Sabrina Dubroca wrote:
> 2026-09-28, 21:33:41 +0200, Jérémy Jean wrote:
>> On 2026-09-28 18:19, Sabrina Dubroca wrote:
>> > 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.
> 
> Ok, I see now. One call to validate_xmit_xfrm() that calls
> skb_gso_segment() and feeds those N non-GSO skbs to ->xmit one by one.

Yes, precisely.

> In that case, the seqno used in the IV wouldn't match the one that the
> peer will reconstruct using the bottom 32b of seqno present in the
> header, and it would never manage to decrypt anything we sent.

I don't think the peer uses its internal counter to reconstruct the IV?
The IV is part of the GCM ciphertext and the peer uses that value it
received to decrypt the payload. AFAICT, the decryption is ultimately 
done
in seqiv_aead_decrypt(), where one can see the IV copy (121), and the
actual decryption call (123).

crypto/seqiv.c:
     99	static int seqiv_aead_decrypt(struct aead_request *req)
    100	{
                 // ...
    116		aead_request_set_callback(subreq, req->base.flags, compl, data);
    117		aead_request_set_crypt(subreq, req->src, req->dst,
    118				       req->cryptlen - ivsize, req->iv);
    119		aead_request_set_ad(subreq, req->assoclen + ivsize);
    120
    121		scatterwalk_map_and_copy(req->iv, req->src, req->assoclen, 
ivsize, 0);
    122
    123		return crypto_aead_decrypt(subreq);
    124	}

> But luckily, it seems commenting out the memcpy(iv, seqno) line has no
> effect (I think that's because all algorithms rely on either seqiv or
> echainiv).

I may misunderstand, but which memcpy() do you refer to ?
If you do not memcpy, then IV is always null, and the nonce reuse is 
even worse, no?

>> 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.
> 
> I don't think that happens? The other packet will go through ->xmit
> too and use the wrong seqno too.

Which "wrong seqno"? The other packet will indeed, go through ->xmit,
but its lack of XFRM_GSO_SEGMENT will make it skip the increment and
reuse the previous counter value.

Regards,
Jérémy

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

[Sabrina Dubroca]

2026-09-29, 11:47:47 +0200, Jérémy Jean wrote:
> Hello Sabrina,
> 
> On 2026-09-29 01:48, Sabrina Dubroca wrote:
> > 2026-09-28, 21:33:41 +0200, Jérémy Jean wrote:
> > > On 2026-09-28 18:19, Sabrina Dubroca wrote:
> > > > 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.
> > 
> > Ok, I see now. One call to validate_xmit_xfrm() that calls
> > skb_gso_segment() and feeds those N non-GSO skbs to ->xmit one by one.
> 
> Yes, precisely.
> 
> > In that case, the seqno used in the IV wouldn't match the one that the
> > peer will reconstruct using the bottom 32b of seqno present in the
> > header, and it would never manage to decrypt anything we sent.
> 
> I don't think the peer uses its internal counter to reconstruct the IV?

For some reason when looking at this last night I thought it was
rebuilding it from the seqno in the esp header.

> The IV is part of the GCM ciphertext and the peer uses that value it
> received to decrypt the payload. AFAICT, the decryption is ultimately done
> in seqiv_aead_decrypt(), where one can see the IV copy (121), and the
> actual decryption call (123).
> 
> crypto/seqiv.c:
>     99	static int seqiv_aead_decrypt(struct aead_request *req)
>    100	{
>                 // ...
>    116		aead_request_set_callback(subreq, req->base.flags, compl, data);
>    117		aead_request_set_crypt(subreq, req->src, req->dst,
>    118				       req->cryptlen - ivsize, req->iv);
>    119		aead_request_set_ad(subreq, req->assoclen + ivsize);
>    120
>    121		scatterwalk_map_and_copy(req->iv, req->src, req->assoclen, ivsize,
> 0);
>    122
>    123		return crypto_aead_decrypt(subreq);
>    124	}
> 
> > But luckily, it seems commenting out the memcpy(iv, seqno) line has no
> > effect (I think that's because all algorithms rely on either seqiv or
> > echainiv).
> 
> I may misunderstand, but which memcpy() do you refer to ?
> If you do not memcpy, then IV is always null, and the nonce reuse is even
> worse, no?

Yeah right. I thought there was something dodgy there.

> > > 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.
> > 
> > I don't think that happens? The other packet will go through ->xmit
> > too and use the wrong seqno too.
> 
> Which "wrong seqno"? The other packet will indeed, go through ->xmit,
> but its lack of XFRM_GSO_SEGMENT will make it skip the increment and
> reuse the previous counter value.

Eh, ok. I thought you were saying one goes through ->xmit (with the
wrong seqno because it has been incremented) and the other through
->output.

Then I guess this makes sense.


Could you:

 1. fix both bugs you found so that the code looks similar, probably
    bundled as a small series (this stuff is not ipv*-specific, so
    there's no reason for it to be implemented differently)

 2. rewrite the commit messages based on this thread to be much more
    precise and also less verbose

I'd also reduce the amount of crypto detail about GCM, I don't think
it's super relevant. Or move it to a cover letter.


Thanks.

-- 
Sabrina

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

[Jérémy Jean]

On 2026-09-29 15:00, Sabrina Dubroca wrote:
> 2026-09-29, 11:47:47 +0200, Jérémy Jean wrote:
>> Hello Sabrina,
>> 
>> On 2026-09-29 01:48, Sabrina Dubroca wrote:
>> > 2026-09-28, 21:33:41 +0200, Jérémy Jean wrote:
>> > > On 2026-09-28 18:19, Sabrina Dubroca wrote:
>> > > > 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.
>> >
>> > Ok, I see now. One call to validate_xmit_xfrm() that calls
>> > skb_gso_segment() and feeds those N non-GSO skbs to ->xmit one by one.
>> 
>> Yes, precisely.
>> 
>> > In that case, the seqno used in the IV wouldn't match the one that the
>> > peer will reconstruct using the bottom 32b of seqno present in the
>> > header, and it would never manage to decrypt anything we sent.
>> 
>> I don't think the peer uses its internal counter to reconstruct the 
>> IV?
> 
> For some reason when looking at this last night I thought it was
> rebuilding it from the seqno in the esp header.
> 
>> The IV is part of the GCM ciphertext and the peer uses that value it
>> received to decrypt the payload. AFAICT, the decryption is ultimately 
>> done
>> in seqiv_aead_decrypt(), where one can see the IV copy (121), and the
>> actual decryption call (123).
>> 
>> crypto/seqiv.c:
>>     99	static int seqiv_aead_decrypt(struct aead_request *req)
>>    100	{
>>                 // ...
>>    116		aead_request_set_callback(subreq, req->base.flags, compl, 
>> data);
>>    117		aead_request_set_crypt(subreq, req->src, req->dst,
>>    118				       req->cryptlen - ivsize, req->iv);
>>    119		aead_request_set_ad(subreq, req->assoclen + ivsize);
>>    120
>>    121		scatterwalk_map_and_copy(req->iv, req->src, req->assoclen, 
>> ivsize,
>> 0);
>>    122
>>    123		return crypto_aead_decrypt(subreq);
>>    124	}
>> 
>> > But luckily, it seems commenting out the memcpy(iv, seqno) line has no
>> > effect (I think that's because all algorithms rely on either seqiv or
>> > echainiv).
>> 
>> I may misunderstand, but which memcpy() do you refer to ?
>> If you do not memcpy, then IV is always null, and the nonce reuse is 
>> even
>> worse, no?
> 
> Yeah right. I thought there was something dodgy there.
> 
>> > > 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.
>> >
>> > I don't think that happens? The other packet will go through ->xmit
>> > too and use the wrong seqno too.
>> 
>> Which "wrong seqno"? The other packet will indeed, go through ->xmit,
>> but its lack of XFRM_GSO_SEGMENT will make it skip the increment and
>> reuse the previous counter value.
> 
> Eh, ok. I thought you were saying one goes through ->xmit (with the
> wrong seqno because it has been incremented) and the other through
> ->output.
> 
> Then I guess this makes sense.

Good that we agree then, thanks :)

> Could you:
> 
>  1. fix both bugs you found so that the code looks similar, probably
>     bundled as a small series (this stuff is not ipv*-specific, so
>     there's no reason for it to be implemented differently)
> 
>  2. rewrite the commit messages based on this thread to be much more
>     precise and also less verbose

Yes, I will try do this shortly.

> I'd also reduce the amount of crypto detail about GCM, I don't think
> it's super relevant. Or move it to a cover letter.

I purposedly emphasized the crypto part about GCM to make it clear to
non-crypto people that the nonce reuse impact is catastrophic (all zero 
or
even a single reuse). I will move it to the cover letter then.

Regards,
Jérémy