Re: [PATCH ipsec 4/7] xfrm: prevent AES-GCM nonce reuse after early GSO

[Sabrina Dubroca <sd@queasysnail.net>]

Hi Jérémy, sorry for the delay.

2026-10-01, 23:07:55 +0200, Jérémy Jean wrote:
> On 2026-10-01 15:31, Sabrina Dubroca wrote:
> > 2026-09-30, 14:45:21 +0000, Jérémy Jean wrote:
> > > In the software ESP offload path, xfrm_output_gso() leaves its
> > > segments
> > > sharing a secpath extension. Each segment gets its own ESP sequence
> > > number, but stores it in the same xo->seq field for IV generation.
> > > 
> > > If encryption is delayed, later segments overwrite the value needed by
> > > earlier ones: esp*_xmit() can then encrypt several packets with the
> > > same
> > > AES-GCM nonce, despite their distinct ESP sequence numbers.
> > 
> > Here again, your commit message could describe much more precisely
> > what actually happens.
> > 
> > I'm guessing you mean something that starts like
> > 
> > xfrm_output_gso
> >   segs = skb0,skb1... all with the same xo
> >   ...
> >   xfrm_output_one(skb0)
> >     xfrm_replay_overflow(skb0) xo->seq = N
> >   xfrm_output_one(skb1)
> >     xfrm_replay_overflow(skb1) xo->seq = N+1
> >   ...
> > 
> > [and then some more stuff happens that gives them the right
> > esph->seq_no but wrong 64b seqno used for the IV]
> > 
> > But please help reviewers trace the codepath you've already gone down,
> > without having to guess what you mean. You don't need to give the full
> > call graph with the state of each variable, but there needs to be more
> > than just the very beginning and the very end.
> 
> Here is a more verbose changelog I wrote for that patch:
> 
> ---
> With software ESP offload, xfrm_output() attaches a secpath before
> calling xfrm_output_gso() for an encapsulated GSO packet, but
> skb_gso_segment() leaves the splitted skbs sharing the same secpath,
> including xo->seq.
> 
> For the i-th splitted segment, xfrm_output_gso() reaches
> xfrm_replay_overflow() through xfrm_output_one(), which allocates
> sequence number N+i to that segment, and then incorrectly overwrites
> the shared xo->seq with N+i. If encryption is delayed until all t
> segments have been processed, xo->seq ends up with value N+t-1 for all
> segments. When validate_xmit_xfrm() calls esp*_xmit() for each
> segment, the IV is constructed from the same value N+t-1, causing
> nonce reuse in AES-GCM. Yet, each ESP header correctly keeps using
> value N+i from its private XFRM_SKB_CB.
> 
> Call secpath_set() for each offloaded segment before xfrm_output2(),
> so sequence allocation writes to private metadata that cannot be
> overwritten by later segments.
> ---

Thanks, this is much more helpful.

The comment in the diff could be dropped (git blame will guide the
reader to the commit message), or extended with "and stores it in the
secpath".

-- 
Sabrina