mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRH
@ 2026-09-23  4:51 Yuya Kusakabe
  2026-09-26  4:17 ` [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRHA Andrea Mayer
  2026-09-27  5:10 ` [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRH netdev-bot+sashiko
  0 siblings, 2 replies; 3+ messages in thread
From: Yuya Kusakabe @ 2026-09-23  4:51 UTC (permalink / raw)
  To: Andrea Mayer, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, David Ahern, Ido Schimmel,
	David Lebrun, Ahmed Abdelsalam
  Cc: netdev, linux-kernel, Yuya Kusakabe

seg6_hmac_validate_skb() derived the SRH from skb_transport_header().
That only holds while the two coincide, which is not true on the
seg6_local input path.

ip6_rcv_core() leaves the transport header just past the IPv6 header.
A Hop-by-Hop options header is consumed before the route lookup and
advances it, but a Destination Options header is not: the seg6_local
lwtunnel is entered through an input redirect from the route lookup,
which bypasses the extension header handlers. The transport header
then still points at the Destination Options header while
seg6_get_srh() has located the real SRH further down the chain.

The HMAC is therefore computed over the Destination Options header,
and a packet carrying a valid HMAC TLV is dropped when
seg6_require_hmac is set. Such a packet is legitimate: RFC 8200 allows
Destination Options before a routing header, and get_srh() has walked
the header chain since commit 5829d70b0b6c ("ipv6: sr: fix get_srh() to
comply with IPv6 standard "RFC 8200""). The misread header is covered
by the pskb_may_pull() in seg6_get_srh(), so the result is a wrong
verdict, not an out-of-bounds access.

Reproduce by giving a node a seg6local End SID with
net.ipv6.conf.<dev>.seg6_require_hmac=1 and a key installed with
"ip sr hmac set <keyid> sha1", then sending

  IPv6 -> Destination Options -> SRH (carrying a valid HMAC TLV) -> payload

to that SID: it is dropped, while the same packet without the
Destination Options header passes.

Fixes: 5829d70b0b6c ("ipv6: sr: fix get_srh() to comply with IPv6 standard "RFC 8200"")
Assisted-by: LLM
Signed-off-by: Yuya Kusakabe <yuya.kusakabe@gmail.com>
---
 include/net/seg6_hmac.h | 3 ++-
 net/ipv6/exthdrs.c      | 2 +-
 net/ipv6/seg6_hmac.c    | 5 +----
 net/ipv6/seg6_local.c   | 6 +++---
 4 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/include/net/seg6_hmac.h b/include/net/seg6_hmac.h
index e9f41725933e..3161a8104b89 100644
--- a/include/net/seg6_hmac.h
+++ b/include/net/seg6_hmac.h
@@ -48,7 +48,8 @@ extern int seg6_hmac_info_add(struct net *net, u32 key,
 extern int seg6_hmac_info_del(struct net *net, u32 key);
 extern int seg6_push_hmac(struct net *net, struct in6_addr *saddr,
 			  struct ipv6_sr_hdr *srh);
-extern bool seg6_hmac_validate_skb(struct sk_buff *skb);
+extern bool seg6_hmac_validate_skb(struct sk_buff *skb,
+				   struct ipv6_sr_hdr *srh);
 #ifdef CONFIG_IPV6_SEG6_HMAC
 extern int seg6_hmac_net_init(struct net *net);
 extern void seg6_hmac_net_exit(struct net *net);
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 09a4552f7f08..3ef3c2635581 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -387,7 +387,7 @@ static int ipv6_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
 	}
 
 #ifdef CONFIG_IPV6_SEG6_HMAC
-	if (!seg6_hmac_validate_skb(skb)) {
+	if (!seg6_hmac_validate_skb(skb, hdr)) {
 		kfree_skb(skb);
 		return -1;
 	}
diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index e6964c6b0d38..bd2704d4c7a0 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -173,13 +173,12 @@ EXPORT_SYMBOL(seg6_hmac_compute);
  *
  * called with rcu_read_lock()
  */
-bool seg6_hmac_validate_skb(struct sk_buff *skb)
+bool seg6_hmac_validate_skb(struct sk_buff *skb, struct ipv6_sr_hdr *srh)
 {
 	u8 hmac_output[SEG6_HMAC_FIELD_LEN];
 	struct net *net = dev_net(skb->dev);
 	struct seg6_hmac_info *hinfo;
 	struct sr6_tlv_hmac *tlv;
-	struct ipv6_sr_hdr *srh;
 	struct inet6_dev *idev;
 	int require_hmac;
 
@@ -187,8 +186,6 @@ bool seg6_hmac_validate_skb(struct sk_buff *skb)
 	if (!idev)
 		return false;
 
-	srh = (struct ipv6_sr_hdr *)skb_transport_header(skb);
-
 	tlv = seg6_get_tlv_hmac(srh);
 
 	require_hmac = READ_ONCE(idev->cnf.seg6_require_hmac);
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index d1070aec7b72..67eeb27ee43a 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -222,7 +222,7 @@ static struct ipv6_sr_hdr *get_and_validate_srh(struct sk_buff *skb)
 		return NULL;
 
 #ifdef CONFIG_IPV6_SEG6_HMAC
-	if (!seg6_hmac_validate_skb(skb))
+	if (!seg6_hmac_validate_skb(skb, srh))
 		return NULL;
 #endif
 
@@ -239,7 +239,7 @@ static bool decap_and_validate(struct sk_buff *skb, int proto)
 		return false;
 
 #ifdef CONFIG_IPV6_SEG6_HMAC
-	if (srh && !seg6_hmac_validate_skb(skb))
+	if (srh && !seg6_hmac_validate_skb(skb, srh))
 		return false;
 #endif
 
@@ -771,7 +771,7 @@ static int end_flv8986_core(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 	srhoff = srh ? ((unsigned char *)srh - skb->data) : 0;
 	pinfo = seg6_get_srh_pktinfo(srh);
 #ifdef CONFIG_IPV6_SEG6_HMAC
-	if (srh && !seg6_hmac_validate_skb(skb))
+	if (srh && !seg6_hmac_validate_skb(skb, srh))
 		goto drop;
 #endif
 	flvmask = finfo->flv_ops;

---
base-commit: 17741334d00bf5ebd37f8c1c36bc9c146a351deb
change-id: 20260922-b4-seg6-hmac-transport-header-2158048b4bc6

Best regards,
--  
Yuya Kusakabe <yuya.kusakabe@gmail.com>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRHA
  2026-09-23  4:51 [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRH Yuya Kusakabe
@ 2026-09-26  4:17 ` Andrea Mayer
  2026-09-27  5:10 ` [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRH netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: Andrea Mayer @ 2026-09-26  4:17 UTC (permalink / raw)
  To: Yuya Kusakabe
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, David Ahern, Ido Schimmel, David Lebrun,
	Ahmed Abdelsalam, netdev, linux-kernel, stefano.salsano,
	Andrea Mayer

On Wed, 23 Sep 2026 13:51:37 +0900
Yuya Kusakabe <yuya.kusakabe@gmail.com> wrote:

> seg6_hmac_validate_skb() derived the SRH from skb_transport_header().
> That only holds while the two coincide, which is not true on the
> seg6_local input path.
> 
> ip6_rcv_core() leaves the transport header just past the IPv6 header.
> A Hop-by-Hop options header is consumed before the route lookup and
> advances it, but a Destination Options header is not: the seg6_local
> lwtunnel is entered through an input redirect from the route lookup,
> which bypasses the extension header handlers. The transport header
> then still points at the Destination Options header while
> seg6_get_srh() has located the real SRH further down the chain.
> 
> The HMAC is therefore computed over the Destination Options header,
> and a packet carrying a valid HMAC TLV is dropped when
> seg6_require_hmac is set. Such a packet is legitimate: RFC 8200 allows
> Destination Options before a routing header, and get_srh() has walked
> the header chain since commit 5829d70b0b6c ("ipv6: sr: fix get_srh() to
> comply with IPv6 standard "RFC 8200""). The misread header is covered
> by the pskb_may_pull() in seg6_get_srh(), so the result is a wrong
> verdict, not an out-of-bounds access.

Thanks for the patch.

This patch also closes an out-of-bounds read that the commit message
does not mention. The commit says the misread gives a wrong verdict,
not an out-of-bounds access. That holds for Destination Options, which
has the same length encoding as the SRH. A Fragment or an Authentication
header is encoded differently, and a forged packet can put one before
the SRH. The old code read that header at skb_transport_header() as an
SRH, and seg6_get_tlv_hmac() computed the TLV pointer from its second
byte. The pull in seg6_get_srh() covers only the bytes up to the end of
the real SRH, and the TLV pointer can land past them. This runs before
seg6_require_hmac is read, and the default configuration reaches it.

Before this patch, a forged Fragment before the SRH toward an End SID
faults in that read (KASAN, DEBUG_PAGEALLOC):

  BUG: unable to handle page fault for address: ffff888106304078
  RIP: seg6_hmac_validate_skb (net/ipv6/seg6_hmac.c:91 net/ipv6/seg6_hmac.c:192)
   input_action_end_core.isra.0 (net/ipv6/seg6_local.c:225 net/ipv6/seg6_local.c:413)
   seg6_local_input_core (net/ipv6/seg6_local.c:1640)
   seg6_local_input (net/ipv6/seg6_local.c:1662)
  Kernel panic - not syncing: Fatal exception in interrupt

Could the commit message also mention the out-of-bounds read?

Ciao,
Andrea

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRH
  2026-09-23  4:51 [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRH Yuya Kusakabe
  2026-09-26  4:17 ` [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRHA Andrea Mayer
@ 2026-09-27  5:10 ` netdev-bot+sashiko
  1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-09-27  5:10 UTC (permalink / raw)
  To: yuya.kusakabe
  Cc: andrea.mayer, davem, edumazet, kuba, pabeni, horms, dsahern,
	idosch, david.lebrun, amsalam20, netdev, linux-kernel

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The commit message says the misread header "is covered by the
  pskb_may_pull() in seg6_get_srh(), so the result is a wrong verdict,…
- [Medium] The code change is correct, but the commit message describes
  the bug only as a false drop (a 'wrong verdict' that rejects a…

--

seg6: fix HMAC validation when an extension header precedes the SRH

This patch passes the SRH found by the caller into
seg6_hmac_validate_skb(). Before, the function derived the SRH from
skb_transport_header(). On the seg6_local input path the transport header
can still point at an extension header that sits in front of the SRH, so
the HMAC was checked against the wrong header.

> The HMAC is therefore computed over the Destination Options header,
> and a packet carrying a valid HMAC TLV is dropped when
> seg6_require_hmac is set.

[Severity: Medium]
Does this cover the whole impact of the bug? Before this patch, the
mismatch could also let through packets that should have been rejected,
even with seg6_require_hmac=1.

seg6_hmac_compute() only hashes saddr, first_segment, flags, hmackeyid
and the segment list:

net/ipv6/seg6_hmac.c:seg6_hmac_compute() {
    ...
	/* saddr(16) + first_seg(1) + flags(1) + keyid(4) + seglist(16n) */
	plen = 16 + 1 + 1 + 4 + (hdr->first_segment + 1) * 16;
    ...
}

It does not hash nexthdr, hdrlen, type or segments_left. A sender who has
captured one SRH with a valid HMAC could:

1. copy the captured bytes into a Destination Options header and set its
   nexthdr byte to IPPROTO_ROUTING. ipv6_find_hdr() sizes this header
   with ipv6_optlen(), the same way it sizes an SRH, and does not check
   the option TLVs.
2. append an SRH of its own choosing, with segments_left > 0.
3. send the packet to the SID from the captured source address.

The old seg6_hmac_validate_skb() took srh from skb_transport_header(),
which points at the Destination Options copy, so the captured HMAC would
match.

get_and_validate_srh(), decap_and_validate() and end_flv8986_core() would
then act on the SRH returned by seg6_get_srh(). That is the
unauthenticated SRH.

A similar splice looks possible with a leading routing header that has
segments_left=0, because get_and_validate_srh() passes IP6_FH_F_SKIP_RH
to seg6_get_srh().

This patch closes the hole, since the HMAC is now checked over the same
pointer that is processed afterwards. Should the changelog describe this
HMAC bypass as well as the false drop?

> The misread header is covered
> by the pskb_may_pull() in seg6_get_srh(), so the result is a wrong
> verdict, not an out-of-bounds access.

[Severity: Medium]
Does this statement hold for every header that can sit in front of the
SRH?

The code change looks correct. However, the same root cause can be hit
when a Fragment header with a zero fragment offset, or an AH header, sits
between the IPv6 (or Hop-by-Hop) header and the SRH. ipv6_find_hdr() steps
over both of them:

net/ipv6/exthdrs_core.c:ipv6_find_hdr() {
    ...
			_frag_off = ntohs(*fp) & ~0x7;
			if (_frag_off) {
				...
			}
			hdrlen = 8;
		} else if (nexthdr == NEXTHDR_AUTH) {
			...
			hdrlen = ipv6_authlen(hp);
    ...
}

So seg6_get_srh() pulls the real SRH further down the chain. Meanwhile
the transport header set in ip6_rcv_core() still points at the Fragment
or AH header.

In a Fragment header, the byte the old code read as srh->hdrlen is the
reserved byte, and first_segment and flags are Identification bytes. In
AH they are the payload length byte and SPI bytes. The sender controls
all of these bytes.

Take reserved = 255, id[0] = 0 and id[1] = 0x08. With those values
seg6_get_tlv_hmac() passes both its hdrlen check and sr_has_hmac(), and
then does:

net/ipv6/seg6_hmac.c:seg6_get_tlv_hmac() {
    ...
	tlv = (struct sr6_tlv_hmac *)
	      ((char *)srh + ((srh->hdrlen + 1) << 3) - 40);

	if (tlv->tlvhdr.type != SR6_TLV_HMAC || tlv->tlvhdr.len != 38)
    ...
}

That reads 2008 bytes past the Fragment header. With a minimal 24-byte
real SRH, the pskb_may_pull() in seg6_get_srh() only guarantees about 72
bytes of linear data. Can this read go past skb->len, and even past
skb->end?

For AH, the misread length is larger than the real AH length by four
times the AH length byte.

The pre-patch seg6_hmac_validate_skb() called seg6_get_tlv_hmac() before
it looked at seg6_require_hmac. Wouldn't this read then happen with any
sysctl value, including the default of 0? If the bytes there happen to
match, tlv->hmackeyid and tlv->hmac would also be read out of bounds.

This patch does fix these cases, because it now passes the SRH that
seg6_get_srh() validated. Could the changelog describe the out-of-bounds
read reachable through get_and_validate_srh(), decap_and_validate() and
end_flv8986_core(), instead of saying the result is only a wrong verdict?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260923-b4-seg6-hmac-transport-header-v1-1-3ae85dc0fdb9%40gmail.com

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-09-27  5:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-23  4:51 [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRH Yuya Kusakabe
2026-09-26  4:17 ` [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRHA Andrea Mayer
2026-09-27  5:10 ` [PATCH net] seg6: fix HMAC validation when an extension header precedes the SRH netdev-bot+sashiko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®