[PATCH ipsec v2 0/7] xfrm: fix ESP IV generation and ESN authentication

[PATCH ipsec v2 0/7] xfrm: fix ESP IV generation and ESN authentication

[Jérémy Jean]

Hello,

This series fixes nonce reuse in AES-GCM and ESN authentication errors
in the IPv4 and IPv6 ESP offload paths:
* 1/7 and 2/7: Save the current sequence number before incrementing it,
               so that GCM IV construction prevents nonce reuse,
* 3/7: Use the current sequence for software AAD and hardware offload,
* 4/7: Give each early GSO segment a private secpath,
* 5/7: Use the packet sequence directly for mlx5 IV generation,
* 6/7: Segment untrusted GSO packets before allocating sequence numbers,
* 7/7: Keep the sequence counter unchanged after full ESN overflow.

Note that triggering the bug fixed by 7/7 is impractical: it requires
processing about 2^64 packets under a single key. Yet, I still believe
it's cleaner to fix it.

On the crypto side, as a reminder, reusing a nonce under the same GCM
key is catastrophic for security: it exposes the XOR of the plaintexts
and can reveal GCM authentication key, which opens the possibilty for
an adversary to forge ciphertexts without recovering the AES key.

This series combines and extends these two individual reports:
* https://lore.kernel.org/all/20260925095105.446269-2-Jeremy.Jean@oss.cyber.gouv.fr/
* https://lore.kernel.org/all/20260925095128.446450-2-Jeremy.Jean@oss.cyber.gouv.fr/

Recent discussion with Sabrina Dubroca may also be of interest:
https://lore.kernel.org/all/c443bad568f4e03d05b848b1b245707d@oss.cyber.gouv.fr/T/#u

Regards,
Jérémy

---

Changes in v2:
- Rebase onto ipsec/master.
- Clarify Patch 4's description and comment.
- Remove the duplicate mlx5 IV helper and callback in Patch 5.

Jérémy Jean (7):
  xfrm: esp6: use the current sequence number for the IV
  xfrm: esp4: use the current sequence number for the IV
  xfrm: esp: use the current sequence number for AAD and offload
  xfrm: prevent AES-GCM nonce reuse after early GSO
  net/mlx5e: Use the packet sequence number for the IPsec IV
  xfrm: segment untrusted GSO packets before sequence allocation
  xfrm: leave the sequence counter unchanged on ESN overflow

 .../mellanox/mlx5/core/en_accel/ipsec.c       | 18 -----------
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  2 --
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c  | 31 +++----------------
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  |  4 ---
 net/ipv4/esp4.c                               | 13 +++-----
 net/ipv4/esp4_offload.c                       |  7 +++--
 net/ipv6/esp6.c                               | 13 +++-----
 net/ipv6/esp6_offload.c                       |  7 +++--
 net/xfrm/xfrm_output.c                        | 16 ++++++++--
 net/xfrm/xfrm_replay.c                        |  1 -
 10 files changed, 36 insertions(+), 76 deletions(-)

-- 
2.47.3

[PATCH ipsec v2 1/7] xfrm: esp6: use the current sequence number for the IV

[Jérémy Jean]

When a GSO packet is split in software for IPv6 ESP,
validate_xmit_xfrm() calls esp6_xmit() for each segment. These
segments have XFRM_GSO_SEGMENT set, but after the split, skb_is_gso()
returns false for each skb, so each call increments xo->seq.low by one
after writing the ESP sequence number.

When encryption is done in software, the AES-GCM IV is generated from
the incremented value: if the last segment has sequence number 100, it
uses 101 to generate its IV. A bug arises when the next ordinary
packet is processed: it also goes through esp6_xmit() with sequence
number 101, but since it does not have XFRM_GSO_SEGMENT, no increment
happens and the same IV is reused.

Save the full sequence number before incrementing xo->seq, and use
that value to generate the IV.

Fixes: 3dca3f38cfb8 ("xfrm: Separate ESP handling from segmentation for GRO packets.")
Cc: stable@vger.kernel.org
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 22895521a57d0..05d13cce22e47 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

[PATCH ipsec v2 2/7] xfrm: esp4: use the current sequence number for the IV

[Jérémy Jean]

When a GSO packet is split in software for IPv4 ESP,
validate_xmit_xfrm() calls esp_xmit() for each segment. These
segments have XFRM_GSO_SEGMENT set, but after the split, skb_is_gso()
returns false for each skb, so each call increments xo->seq.low by one
after writing the ESP sequence number.

The low bits are saved in seq before the increment, but esp.seqno is
set afterwards, using the saved low bits and the high bits from
xo->seq.hi.

When encryption is done in software with ESN enabled, the segment at
(H, 0xffffffff) uses (H+1, 0xffffffff) to generate the AES-GCM IV
because xo->seq.hi has already been incremented when the low bits
wrapped. One full 32-bit cycle later, if the packet at
(H+1, 0xffffffff) on the same SA is non-GSO, it generates the same IV.

Move the assignment to esp.seqno before xo->seq is advanced. This
saves H before the wrap increments xo->seq.hi to H+1, so the segment
at (H, 0xffffffff) generates its IV from (H, 0xffffffff).

Fixes: 4b549ccce941 ("xfrm: replay: Fix ESN wrap around for GSO")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/ipv4/esp4_offload.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index abd77162f5e75..3a0aafe991f47 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -316,6 +316,7 @@ static int esp_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));
 
 	esph = esp.esph;
 	esph->spi = x->id.spi;
@@ -334,8 +335,6 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
 	if (xo->seq.low < seq)
 		xo->seq.hi++;
 
-	esp.seqno = cpu_to_be64(seq + ((u64)xo->seq.hi << 32));
-
 	if (hw_offload && encap_type == UDP_ENCAP_ESPINUDP) {
 		/* In the XFRM stack, the encapsulation protocol is set to iphdr->protocol by
 		 * setting *skb_mac_header(skb) (see esp_output_udp_encap()) where skb->mac_header
-- 
2.47.3

[PATCH ipsec v2 3/7] xfrm: esp: use the current sequence number for AAD and offload

[Jérémy Jean]

When a GSO packet is split in software, esp_xmit() and esp6_xmit()
advance xo->seq before software encryption or the handoff to the NIC.
If the 64-bit sequence number of the segment is (H, 0xffffffff), the
counter becomes (H+1, 0). Both the software ESN helpers and drivers
such as Chelsio then use H+1 instead of H as part of the associated
data that is authenticated.

However, the ESN 32 high bits are not present in the ESP header: the
receiver reconstructs the 64-bit value from the received 32 low bits
and its own replay state, and uses it to check cryptographic
integrity. It uses H for this packet at the boundary, so the GCM tag
computed by the sender with H+1 fails verification, as it is different
from the one obtained by the receiver using H.

Use the high bits saved in esp->seqno for the software AAD. For
hardware encryption, restore the current sequence after skb_ext_add()
makes the secpath private, leaving the shared counter advanced for
the remaining segments. Restore both halves because drivers can also
use xo->seq to generate the IV.

Fixes: 3dca3f38cfb8 ("xfrm: Separate ESP handling from segmentation for GRO packets.")
Fixes: 4b549ccce941 ("xfrm: replay: Fix ESN wrap around for GSO")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/ipv4/esp4.c         | 13 ++++---------
 net/ipv4/esp4_offload.c |  4 ++++
 net/ipv6/esp6.c         | 13 ++++---------
 net/ipv6/esp6_offload.c |  4 ++++
 4 files changed, 16 insertions(+), 18 deletions(-)

diff --git a/net/ipv4/esp4.c b/net/ipv4/esp4.c
index 111bea0bd2601..df4857beca02c 100644
--- a/net/ipv4/esp4.c
+++ b/net/ipv4/esp4.c
@@ -271,20 +271,15 @@ static void esp_output_restore_header(struct sk_buff *skb)
 static struct ip_esp_hdr *esp_output_set_extra(struct sk_buff *skb,
 					       struct xfrm_state *x,
 					       struct ip_esp_hdr *esph,
-					       struct esp_output_extra *extra)
+					       struct esp_output_extra *extra,
+					       __be64 seqno)
 {
 	/* For ESN we move the header forward by 4 bytes to
 	 * accommodate the high bits.  We will move it back after
 	 * encryption.
 	 */
 	if ((x->props.flags & XFRM_STATE_ESN)) {
-		__u32 seqhi;
-		struct xfrm_offload *xo = xfrm_offload(skb);
-
-		if (xo)
-			seqhi = xo->seq.hi;
-		else
-			seqhi = XFRM_SKB_CB(skb)->seq.output.hi;
+		__u32 seqhi = upper_32_bits(be64_to_cpu(seqno));
 
 		extra->esphoff = (unsigned char *)esph -
 				 skb_transport_header(skb);
@@ -543,7 +538,7 @@ int esp_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info *
 	else
 		dsg = &sg[esp->nfrags];
 
-	esph = esp_output_set_extra(skb, x, esp->esph, extra);
+	esph = esp_output_set_extra(skb, x, esp->esph, extra, esp->seqno);
 	esp->esph = esph;
 
 	sg_init_table(sg, esp->nfrags);
diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index 3a0aafe991f47..6f6ce9ea5c491 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -358,6 +358,10 @@ static int esp_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features_
 		if (!xo)
 			return -EINVAL;
 
+		/* Keep the current sequence number in the private copy. */
+		xo->seq.low = seq;
+		xo->seq.hi = upper_32_bits(be64_to_cpu(esp.seqno));
+
 		xo->flags |= XFRM_XMIT;
 		return 0;
 	}
diff --git a/net/ipv6/esp6.c b/net/ipv6/esp6.c
index b287c807b0186..7509fff7f6682 100644
--- a/net/ipv6/esp6.c
+++ b/net/ipv6/esp6.c
@@ -308,20 +308,15 @@ static void esp_output_restore_header(struct sk_buff *skb)
 static struct ip_esp_hdr *esp_output_set_esn(struct sk_buff *skb,
 					     struct xfrm_state *x,
 					     struct ip_esp_hdr *esph,
-					     struct esp_output_extra *extra)
+					     struct esp_output_extra *extra,
+					     __be64 seqno)
 {
 	/* For ESN we move the header forward by 4 bytes to
 	 * accommodate the high bits.  We will move it back after
 	 * encryption.
 	 */
 	if ((x->props.flags & XFRM_STATE_ESN)) {
-		__u32 seqhi;
-		struct xfrm_offload *xo = xfrm_offload(skb);
-
-		if (xo)
-			seqhi = xo->seq.hi;
-		else
-			seqhi = XFRM_SKB_CB(skb)->seq.output.hi;
+		__u32 seqhi = upper_32_bits(be64_to_cpu(seqno));
 
 		extra->esphoff = (unsigned char *)esph -
 				 skb_transport_header(skb);
@@ -575,7 +570,7 @@ int esp6_output_tail(struct xfrm_state *x, struct sk_buff *skb, struct esp_info
 	else
 		dsg = &sg[esp->nfrags];
 
-	esph = esp_output_set_esn(skb, x, esp->esph, extra);
+	esph = esp_output_set_esn(skb, x, esp->esph, extra, esp->seqno);
 	esp->esph = esph;
 
 	sg_init_table(sg, esp->nfrags);
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 05d13cce22e47..682063bfe685f 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -379,6 +379,10 @@ static int esp6_xmit(struct xfrm_state *x, struct sk_buff *skb,  netdev_features
 		if (!xo)
 			return -EINVAL;
 
+		/* Keep the current sequence number in the private copy. */
+		xo->seq.low = seq;
+		xo->seq.hi = upper_32_bits(be64_to_cpu(esp.seqno));
+
 		xo->flags |= XFRM_XMIT;
 		return 0;
 	}
-- 
2.47.3

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

[Jérémy Jean]

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.

Fixes: a204aef9fd77 ("xfrm: call xfrm_output_gso when inner_protocol is set in xfrm_output")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/xfrm/xfrm_output.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index f1f612cd7b438..99d1b33db52a6 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -672,6 +672,15 @@ static int xfrm_output_gso(struct net *net, struct sock *sk, struct sk_buff *skb
 	skb_list_walk_safe(segs, segs, nskb) {
 		int err;
 
+		/* Each segment reserves its own sequence number and stores
+		 * it in the secpath.
+		 */
+		if (xfrm_offload(segs) && !secpath_set(segs)) {
+			XFRM_INC_STATS(net, LINUX_MIB_XFRMOUTERROR);
+			kfree_skb_list(segs);
+			return -ENOMEM;
+		}
+
 		skb_mark_not_on_list(segs);
 		err = xfrm_output2(net, sk, segs);
 
-- 
2.47.3

[PATCH ipsec v2 5/7] net/mlx5e: Use the packet sequence number for the IPsec IV

[Jérémy Jean]

mlx5e_ipsec_set_iv_esn() may decrement xo->seq.hi for a GSO skb based
on the SA's current output counter. The metadata already contains the
high word for the first packet, so this can produce the wrong IV.

For a three-segment GSO skb starting at (H, 0), oseq is 2 and
oseq - gso_segs wraps to 0xffffffff. The helper then uses (H-1, 0)
for the IV instead of (H, 0). This can create a situation where GCM
reuses a nonce.

Remove the high-word adjustment. mlx5e_ipsec_set_iv_esn() then becomes
identical to mlx5e_ipsec_set_iv(), so delete it. Since there is no
longer an alternate IV implementation to select, remove the per-SA
callback and call the common helper directly for outbound crypto
offload.

Fixes: cb01008390bb ("net/mlx5: IPSec, Add support for ESN")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
Changes in v2:
- Simplifications in the code (removal of mlx5e_ipsec_set_esn_ops and
  set_iv_op function pointer), thanks to Tariq Toukan.

 .../mellanox/mlx5/core/en_accel/ipsec.c       | 18 -----------
 .../mellanox/mlx5/core/en_accel/ipsec.h       |  2 --
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.c  | 31 +++----------------
 .../mellanox/mlx5/core/en_accel/ipsec_rxtx.h  |  4 ---
 4 files changed, 4 insertions(+), 51 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
index db260e3..de2e740 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.c
@@ -651,22 +651,6 @@ static void mlx5e_ipsec_modify_state(struct work_struct *_work)
 	mlx5_accel_esp_modify_xfrm(sa_entry, attrs);
 }
 
-static void mlx5e_ipsec_set_esn_ops(struct mlx5e_ipsec_sa_entry *sa_entry)
-{
-	struct xfrm_state *x = sa_entry->x;
-
-	if (x->xso.type != XFRM_DEV_OFFLOAD_CRYPTO ||
-	    x->xso.dir != XFRM_DEV_OFFLOAD_OUT)
-		return;
-
-	if (x->props.flags & XFRM_STATE_ESN) {
-		sa_entry->set_iv_op = mlx5e_ipsec_set_iv_esn;
-		return;
-	}
-
-	sa_entry->set_iv_op = mlx5e_ipsec_set_iv;
-}
-
 static void mlx5e_ipsec_handle_netdev_event(struct work_struct *_work)
 {
 	struct mlx5e_ipsec_work *work =
@@ -859,8 +843,6 @@ static int mlx5e_xfrm_add_state(struct net_device *dev,
 	if (err)
 		goto err_add_rule;
 
-	mlx5e_ipsec_set_esn_ops(sa_entry);
-
 	if (sa_entry->dwork)
 		queue_delayed_work(ipsec->wq, &sa_entry->dwork->dwork,
 				   MLX5_IPSEC_RESCHED);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
index abcbd38..172fce9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec.h
@@ -278,8 +278,6 @@ struct mlx5e_ipsec_sa_entry {
 	struct net_device *dev;
 	struct mlx5e_ipsec *ipsec;
 	struct mlx5_accel_esp_xfrm_attrs attrs;
-	void (*set_iv_op)(struct sk_buff *skb, struct xfrm_state *x,
-			  struct xfrm_offload *xo);
 	u32 ipsec_obj_id;
 	u32 enc_key_id;
 	struct mlx5e_ipsec_rule ipsec_rule;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
index 6056106..4a9c334 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.c
@@ -150,30 +150,7 @@ static void mlx5e_ipsec_set_swp(struct sk_buff *skb,
 
 }
 
-void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
-			    struct xfrm_offload *xo)
-{
-	struct xfrm_replay_state_esn *replay_esn = x->replay_esn;
-	__u32 oseq = replay_esn->oseq;
-	int iv_offset;
-	__be64 seqno;
-	u32 seq_hi;
-
-	if (unlikely(skb_is_gso(skb) && oseq < MLX5E_IPSEC_ESN_SCOPE_MID &&
-		     MLX5E_IPSEC_ESN_SCOPE_MID < (oseq - skb_shinfo(skb)->gso_segs))) {
-		seq_hi = xo->seq.hi - 1;
-	} else {
-		seq_hi = xo->seq.hi;
-	}
-
-	/* Place the SN in the IV field */
-	seqno = cpu_to_be64(xo->seq.low + ((u64)seq_hi << 32));
-	iv_offset = skb_transport_offset(skb) + sizeof(struct ip_esp_hdr);
-	skb_store_bits(skb, iv_offset, &seqno, 8);
-}
-
-void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_state *x,
-			struct xfrm_offload *xo)
+static void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_offload *xo)
 {
 	int iv_offset;
 	__be64 seqno;
@@ -264,7 +241,6 @@ bool mlx5e_ipsec_handle_tx_skb(struct net_device *netdev,
 {
 	struct mlx5e_priv *priv = netdev_priv(netdev);
 	struct xfrm_offload *xo = xfrm_offload(skb);
-	struct mlx5e_ipsec_sa_entry *sa_entry;
 	struct xfrm_state *x;
 	struct sec_path *sp;
 
@@ -293,8 +269,9 @@ bool mlx5e_ipsec_handle_tx_skb(struct net_device *netdev,
 			goto drop;
 		}
 
-	sa_entry = (struct mlx5e_ipsec_sa_entry *)x->xso.offload_handle;
-	sa_entry->set_iv_op(skb, x, xo);
+	if (x->xso.type == XFRM_DEV_OFFLOAD_CRYPTO &&
+	    x->xso.dir == XFRM_DEV_OFFLOAD_OUT)
+		mlx5e_ipsec_set_iv(skb, xo);
 	mlx5e_ipsec_set_state(priv, skb, x, xo, ipsec_st);
 
 	return true;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
index 45b0d19..9a7e032 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ipsec_rxtx.h
@@ -53,10 +53,6 @@ struct mlx5e_accel_tx_ipsec_state {
 
 #ifdef CONFIG_MLX5_EN_IPSEC
 
-void mlx5e_ipsec_set_iv_esn(struct sk_buff *skb, struct xfrm_state *x,
-			    struct xfrm_offload *xo);
-void mlx5e_ipsec_set_iv(struct sk_buff *skb, struct xfrm_state *x,
-			struct xfrm_offload *xo);
 bool mlx5e_ipsec_handle_tx_skb(struct net_device *netdev,
 			       struct sk_buff *skb,
 			       struct mlx5e_accel_tx_ipsec_state *ipsec_st);
-- 
2.47.3

[PATCH ipsec v2 6/7] xfrm: segment untrusted GSO packets before sequence allocation

[Jérémy Jean]

Packets received through TUN or virtio-net can have gso_segs == 0,
with SKB_GSO_DODGY set until the segmentation metadata is checked.
When ESP defers segmentation, gso_segs is still zero, so the SA
sequence counter is not advanced. The segments receive sequence
numbers starting at oseq + 1, which subsequent packets can reuse,
causing AES-GCM nonce reuse.

This affects IPv4 and IPv6, with or without ESN, when the ESP offload
type is registered. Hardware encryption is not required.

Segment SKB_GSO_DODGY packets in xfrm_output() before allocating
sequence numbers, so each segment gets its own number. Set the ESP
encapsulation flag in xfrm_output_one(), after early segmentation, so
UFO fragment offsets and flags are filled in correctly.

Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/xfrm/xfrm_output.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 99d1b33db52a6..a1fb60971c6bc 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -505,6 +505,9 @@ static int xfrm_output_one(struct sk_buff *skb, int err)
 	if (err <= 0 || x->xso.type == XFRM_DEV_OFFLOAD_PACKET)
 		goto resume;
 
+	if (xfrm_offload(skb))
+		skb->encapsulation = 1;
+
 	do {
 		err = xfrm_skb_check_space(skb);
 		if (err) {
@@ -814,10 +817,10 @@ int xfrm_output(struct sock *sk, struct sk_buff *skb)
 		xfrm_state_hold(x);
 
 		xfrm_get_inner_ipproto(skb, x);
-		skb->encapsulation = 1;
 
 		if (skb_is_gso(skb)) {
-			if (skb->inner_protocol && x->props.mode == XFRM_MODE_TUNNEL)
+			if ((skb_shinfo(skb)->gso_type & SKB_GSO_DODGY) ||
+			    (skb->inner_protocol && x->props.mode == XFRM_MODE_TUNNEL))
 				return xfrm_output_gso(net, sk, skb);
 
 			skb_shinfo(skb)->gso_type |= SKB_GSO_ESP;
-- 
2.47.3

[PATCH ipsec v2 7/7] xfrm: leave the sequence counter unchanged on ESN overflow

[Jérémy Jean]

When the 64-bit ESN counter overflows, xfrm_replay_overflow_offload_esn()
rejects the packet and rolls back the stored counter. It decrements
replay_esn->oseq even though only the local oseq has advanced, which
can later induce a reuse of the last sequence number and the
corresponding AES-GCM nonce. Both IPv4 and IPv6 are affected, yet,
processing about 2^64 packets under a single key is required to
trigger this bug, which is highly unlikely in regular use cases.

Leave replay_esn->oseq unchanged on overflow. Keep the decrement of
replay_esn->oseq_hi to undo its earlier increment.

Fixes: d7dbefc45cf5 ("xfrm: Add xfrm_replay_overflow functions for offloading")
Cc: stable@vger.kernel.org
Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
---
 net/xfrm/xfrm_replay.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/xfrm/xfrm_replay.c b/net/xfrm/xfrm_replay.c
index 9394953247ab9..01504e5b1ad41 100644
--- a/net/xfrm/xfrm_replay.c
+++ b/net/xfrm/xfrm_replay.c
@@ -721,7 +721,6 @@ static int xfrm_replay_overflow_offload_esn(struct xfrm_state *x, struct sk_buff
 				xo->seq.hi = oseq_hi;
 			}
 			if (replay_esn->oseq_hi == 0) {
-				replay_esn->oseq--;
 				replay_esn->oseq_hi--;
 				xfrm_audit_state_replay_overflow(x, skb);
 				err = -EOVERFLOW;
-- 
2.47.3

Re: [PATCH ipsec v2 0/7] xfrm: fix ESP IV generation and ESN authentication

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.