Re: [PATCH net] inet: frags: prevent acceptance of partially authenticated reassembly

[Eric Dumazet]

Le ven. 23 oct. 2026 à 09:45, Jérémy Jean
<Jeremy.Jean@xxxxxxxxxxxxxxxxx> a écrit :
>
> A reassembled skb retains the secpath from its first fragment, which
> makes it possible for the final policy check to accept bytes that were
> not covered by the transform recorded on the skb. This can affect IPsec
> tunnel traffic, including AH and ESP, and allow a packet to be accepted
> even though only part of its payload was authenticated.
>
> Reject a fragment when its secpath differs from the fragments already
> in the queue, and keep overlap statistics limited to actual overlaps.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
> ---
>  include/net/inet_frag.h  |  1 +
>  include/net/xfrm.h       | 17 +++++++++++++++++
>  net/ipv4/inet_fragment.c |  5 +++++
>  net/ipv4/ip_fragment.c   |  3 ++-
>  net/ipv6/reassembly.c    |  3 ++-
>  5 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h
> index 365925c9d2628..b02d2472b003e 100644
> --- a/include/net/inet_frag.h
> +++ b/include/net/inet_frag.h
> @@ -171,6 +171,7 @@ extern const u8 ip_frag_ecn_table[16];
>  #define IPFRAG_OK      0
>  #define IPFRAG_DUP     1
>  #define IPFRAG_OVERLAP 2
> +#define IPFRAG_MIXED_SECPATH   3
>  int inet_frag_queue_insert(struct inet_frag_queue *q, struct sk_buff *skb,
>                            int offset, int end);
>  void *inet_frag_reasm_prepare(struct inet_frag_queue *q, struct sk_buff *skb,
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index a6d69aaa6cd2d..493faac0faa45 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -1201,6 +1201,23 @@ secpath_reset(struct sk_buff *skb)
>  #endif
>  }
>
> +static inline bool
> +secpath_equal(const struct sk_buff *skb1, const struct sk_buff *skb2)
> +{
> +       const struct sec_path *sp1 = skb_sec_path(skb1);
> +       const struct sec_path *sp2 = skb_sec_path(skb2);
> +       int i;
> +
> +       if (!sp1 || !sp2)
> +               return sp1 == sp2;
> +       if (sp1->len != sp2->len || sp1->verified_cnt != sp2->verified_cnt)
> +               return false;

verified_cnt is policy check bookkeeping (written by
__xfrm_policy_check()), it says nothing about which states
authenticated the payload. I would only compare len and xvec[].

With the verified_cnt part removed:

Reviewed-by: Eric Dumazet <edumazet@xxxxxxxxxx>



> +       for (i = 0; i < sp1->len; i++)
> +               if (sp1->xvec[i] != sp2->xvec[i])
> +                       return false;
> +       return true;
> +}
> +
>  static inline int
>  xfrm_addr_any(const xfrm_address_t *addr, unsigned short family)
>  {
> diff --git a/net/ipv4/inet_fragment.c b/net/ipv4/inet_fragment.c
> index b286ee429da8f..c18da91c1f031 100644
> --- a/net/ipv4/inet_fragment.c
> +++ b/net/ipv4/inet_fragment.c
> @@ -23,6 +23,7 @@
>  #include <net/inet_ecn.h>
>  #include <net/ip.h>
>  #include <net/ipv6.h>
> +#include <net/xfrm.h>
>
>  /* Use skb->cb to track consecutive/adjacent fragments coming at
>   * the end of the queue. Nodes in the rb-tree queue will
> @@ -441,6 +442,10 @@ int inet_frag_queue_insert(struct inet_frag_queue *q, struct sk_buff *skb,
>  {
>         struct sk_buff *last = q->fragments_tail;
>
> +       /* The reassembled skb keeps the first fragment's secpath. */
> +       if (last && !secpath_equal(last, skb))
> +               return IPFRAG_MIXED_SECPATH;
> +
>         /* An IP fragment is never a GSO packet, but an untrusted source
>          * (virtio_net_hdr) may have attached GSO metadata to it. Do not let
>          * that reach the reassembled skb, whose head keeps the first
> diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
> index c790d2f494870..36105dd192823 100644
> --- a/net/ipv4/ip_fragment.c
> +++ b/net/ipv4/ip_fragment.c
> @@ -378,8 +378,9 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb, int *refs)
>                 err = -EINVAL;
>                 goto err;
>         }
> +       if (err == IPFRAG_OVERLAP)
> +               __IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
>         err = -EINVAL;
> -       __IP_INC_STATS(net, IPSTATS_MIB_REASM_OVERLAPS);
>  discard_qp:
>         inet_frag_kill(&qp->q, refs);
>         __IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
> diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
> index 05c51f669754a..53a685f5c6b7f 100644
> --- a/net/ipv6/reassembly.c
> +++ b/net/ipv6/reassembly.c
> @@ -240,8 +240,9 @@ static int ip6_frag_queue(struct net *net,
>                 err = -EINVAL;
>                 goto err;
>         }
> +       if (err == IPFRAG_OVERLAP)
> +               __IP6_INC_STATS(net, idev, IPSTATS_MIB_REASM_OVERLAPS);
>         err = -EINVAL;
> -       __IP6_INC_STATS(net, idev, IPSTATS_MIB_REASM_OVERLAPS);
>  discard_fq:
>         inet_frag_kill(&fq->q, refs);
>         __IP6_INC_STATS(net, idev, IPSTATS_MIB_REASMFAILS);
> --
> 2.47.3
>