[PATCH net-next 0/3] ipv6: sr: HMAC fix and cleanups

[PATCH net-next 0/3] ipv6: sr: HMAC fix and cleanups

[Eric Biggers]

This series cleans up the IPv6 Segment Routing HMAC calculations.

Eric Biggers (3):
  ipv6: sr: Fix MAC comparison to be constant-time
  ipv6: sr: Use HMAC-SHA1 and HMAC-SHA256 library functions
  ipv6: sr: Prepare HMAC key ahead of time

 include/net/seg6_hmac.h |  20 ++--
 net/ipv6/Kconfig        |   7 +-
 net/ipv6/seg6.c         |  20 ++--
 net/ipv6/seg6_hmac.c    | 202 +++++-----------------------------------
 4 files changed, 46 insertions(+), 203 deletions(-)


base-commit: 8f5ae30d69d7543eee0d70083daf4de8fe15d585
-- 
2.50.1

[PATCH net-next 1/3] ipv6: sr: Fix MAC comparison to be constant-time

[Eric Biggers]

To prevent timing attacks, MACs need to be compared in constant time.
Use the appropriate helper function for this.

Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 net/ipv6/seg6_hmac.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index f78ecb6ad8383..5dae892bbc73b 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -33,10 +33,11 @@
 #include <net/ip6_route.h>
 #include <net/addrconf.h>
 #include <net/xfrm.h>
 
 #include <crypto/hash.h>
+#include <crypto/utils.h>
 #include <net/seg6.h>
 #include <net/genetlink.h>
 #include <net/seg6_hmac.h>
 #include <linux/random.h>
 
@@ -278,11 +279,11 @@ bool seg6_hmac_validate_skb(struct sk_buff *skb)
 		return false;
 
 	if (seg6_hmac_compute(hinfo, srh, &ipv6_hdr(skb)->saddr, hmac_output))
 		return false;
 
-	if (memcmp(hmac_output, tlv->hmac, SEG6_HMAC_FIELD_LEN) != 0)
+	if (crypto_memneq(hmac_output, tlv->hmac, SEG6_HMAC_FIELD_LEN))
 		return false;
 
 	return true;
 }
 EXPORT_SYMBOL(seg6_hmac_validate_skb);
-- 
2.50.1

[PATCH net-next 2/3] ipv6: sr: Use HMAC-SHA1 and HMAC-SHA256 library functions

[Eric Biggers]

Use the HMAC-SHA1 and HMAC-SHA256 library functions instead of
crypto_shash.  This is simpler and faster.  Pre-allocating per-CPU hash
transformation objects and descriptors is no longer needed, and a
microbenchmark on x86_64 shows seg6_hmac_compute() (with HMAC-SHA256)
dropping from ~2494 cycles to ~1978 cycles, a 20% improvement.

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 include/net/seg6_hmac.h |  12 ---
 net/ipv6/Kconfig        |   7 +-
 net/ipv6/seg6.c         |   7 --
 net/ipv6/seg6_hmac.c    | 200 +++++-----------------------------------
 4 files changed, 24 insertions(+), 202 deletions(-)

diff --git a/include/net/seg6_hmac.h b/include/net/seg6_hmac.h
index 24f733b3e3fe9..3fe4123dbbf0a 100644
--- a/include/net/seg6_hmac.h
+++ b/include/net/seg6_hmac.h
@@ -17,11 +17,10 @@
 #include <linux/route.h>
 #include <net/seg6.h>
 #include <linux/seg6_hmac.h>
 #include <linux/rhashtable-types.h>
 
-#define SEG6_HMAC_MAX_DIGESTSIZE	160
 #define SEG6_HMAC_RING_SIZE		256
 
 struct seg6_hmac_info {
 	struct rhash_head node;
 	struct rcu_head rcu;
@@ -30,17 +29,10 @@ struct seg6_hmac_info {
 	char secret[SEG6_HMAC_SECRET_LEN];
 	u8 slen;
 	u8 alg_id;
 };
 
-struct seg6_hmac_algo {
-	u8 alg_id;
-	char name[64];
-	struct crypto_shash * __percpu *tfms;
-	struct shash_desc * __percpu *shashs;
-};
-
 extern int seg6_hmac_compute(struct seg6_hmac_info *hinfo,
 			     struct ipv6_sr_hdr *hdr, struct in6_addr *saddr,
 			     u8 *output);
 extern struct seg6_hmac_info *seg6_hmac_info_lookup(struct net *net, u32 key);
 extern int seg6_hmac_info_add(struct net *net, u32 key,
@@ -48,17 +40,13 @@ 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);
 #ifdef CONFIG_IPV6_SEG6_HMAC
-extern int seg6_hmac_init(void);
-extern void seg6_hmac_exit(void);
 extern int seg6_hmac_net_init(struct net *net);
 extern void seg6_hmac_net_exit(struct net *net);
 #else
-static inline int seg6_hmac_init(void) { return 0; }
-static inline void seg6_hmac_exit(void) {}
 static inline int seg6_hmac_net_init(struct net *net) { return 0; }
 static inline void seg6_hmac_net_exit(struct net *net) {}
 #endif
 
 #endif
diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
index 1c9c686d9522f..b8f9a8c0302ee 100644
--- a/net/ipv6/Kconfig
+++ b/net/ipv6/Kconfig
@@ -302,14 +302,13 @@ config IPV6_SEG6_LWTUNNEL
 	  If unsure, say N.
 
 config IPV6_SEG6_HMAC
 	bool "IPv6: Segment Routing HMAC support"
 	depends on IPV6
-	select CRYPTO
-	select CRYPTO_HMAC
-	select CRYPTO_SHA1
-	select CRYPTO_SHA256
+	select CRYPTO_LIB_SHA1
+	select CRYPTO_LIB_SHA256
+	select CRYPTO_LIB_UTILS
 	help
 	  Support for HMAC signature generation and verification
 	  of SR-enabled packets.
 
 	  If unsure, say N.
diff --git a/net/ipv6/seg6.c b/net/ipv6/seg6.c
index 180da19c148c1..a5c4c629b788c 100644
--- a/net/ipv6/seg6.c
+++ b/net/ipv6/seg6.c
@@ -520,20 +520,14 @@ int __init seg6_init(void)
 
 	err = seg6_local_init();
 	if (err)
 		goto out_unregister_iptun;
 
-	err = seg6_hmac_init();
-	if (err)
-		goto out_unregister_seg6;
-
 	pr_info("Segment Routing with IPv6\n");
 
 out:
 	return err;
-out_unregister_seg6:
-	seg6_local_exit();
 out_unregister_iptun:
 	seg6_iptunnel_exit();
 out_unregister_genl:
 	genl_unregister_family(&seg6_genl_family);
 out_unregister_pernet:
@@ -541,11 +535,10 @@ int __init seg6_init(void)
 	goto out;
 }
 
 void seg6_exit(void)
 {
-	seg6_hmac_exit();
 	seg6_local_exit();
 	seg6_iptunnel_exit();
 	genl_unregister_family(&seg6_genl_family);
 	unregister_pernet_subsys(&ip6_segments_ops);
 }
diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index 5dae892bbc73b..816aef69e0cea 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -14,11 +14,10 @@
 #include <linux/net.h>
 #include <linux/netdevice.h>
 #include <linux/in6.h>
 #include <linux/icmpv6.h>
 #include <linux/mroute6.h>
-#include <linux/slab.h>
 #include <linux/rhashtable.h>
 
 #include <linux/netfilter.h>
 #include <linux/netfilter_ipv6.h>
 
@@ -32,11 +31,12 @@
 #include <net/ndisc.h>
 #include <net/ip6_route.h>
 #include <net/addrconf.h>
 #include <net/xfrm.h>
 
-#include <crypto/hash.h>
+#include <crypto/sha1.h>
+#include <crypto/sha2.h>
 #include <crypto/utils.h>
 #include <net/seg6.h>
 #include <net/genetlink.h>
 #include <net/seg6_hmac.h>
 #include <linux/random.h>
@@ -76,21 +76,10 @@ static const struct rhashtable_params rht_params = {
 	.key_len		= sizeof(u32),
 	.automatic_shrinking	= true,
 	.obj_cmpfn		= seg6_hmac_cmpfn,
 };
 
-static struct seg6_hmac_algo hmac_algos[] = {
-	{
-		.alg_id = SEG6_HMAC_ALGO_SHA1,
-		.name = "hmac(sha1)",
-	},
-	{
-		.alg_id = SEG6_HMAC_ALGO_SHA256,
-		.name = "hmac(sha256)",
-	},
-};
-
 static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
 {
 	struct sr6_tlv_hmac *tlv;
 
 	if (srh->hdrlen < (srh->first_segment + 1) * 2 + 5)
@@ -106,79 +95,17 @@ static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
 		return NULL;
 
 	return tlv;
 }
 
-static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
-{
-	struct seg6_hmac_algo *algo;
-	int i, alg_count;
-
-	alg_count = ARRAY_SIZE(hmac_algos);
-	for (i = 0; i < alg_count; i++) {
-		algo = &hmac_algos[i];
-		if (algo->alg_id == alg_id)
-			return algo;
-	}
-
-	return NULL;
-}
-
-static int __do_hmac(struct seg6_hmac_info *hinfo, const char *text, u8 psize,
-		     u8 *output, int outlen)
-{
-	struct seg6_hmac_algo *algo;
-	struct crypto_shash *tfm;
-	struct shash_desc *shash;
-	int ret, dgsize;
-
-	algo = __hmac_get_algo(hinfo->alg_id);
-	if (!algo)
-		return -ENOENT;
-
-	tfm = *this_cpu_ptr(algo->tfms);
-
-	dgsize = crypto_shash_digestsize(tfm);
-	if (dgsize > outlen) {
-		pr_debug("sr-ipv6: __do_hmac: digest size too big (%d / %d)\n",
-			 dgsize, outlen);
-		return -ENOMEM;
-	}
-
-	ret = crypto_shash_setkey(tfm, hinfo->secret, hinfo->slen);
-	if (ret < 0) {
-		pr_debug("sr-ipv6: crypto_shash_setkey failed: err %d\n", ret);
-		goto failed;
-	}
-
-	shash = *this_cpu_ptr(algo->shashs);
-	shash->tfm = tfm;
-
-	ret = crypto_shash_digest(shash, text, psize, output);
-	if (ret < 0) {
-		pr_debug("sr-ipv6: crypto_shash_digest failed: err %d\n", ret);
-		goto failed;
-	}
-
-	return dgsize;
-
-failed:
-	return ret;
-}
-
 int seg6_hmac_compute(struct seg6_hmac_info *hinfo, struct ipv6_sr_hdr *hdr,
 		      struct in6_addr *saddr, u8 *output)
 {
 	__be32 hmackeyid = cpu_to_be32(hinfo->hmackeyid);
-	u8 tmp_out[SEG6_HMAC_MAX_DIGESTSIZE];
-	int plen, i, dgsize, wrsize;
+	int plen, i, ret = 0;
 	char *ring, *off;
 
-	/* a 160-byte buffer for digest output allows to store highest known
-	 * hash function (RadioGatun) with up to 1216 bits
-	 */
-
 	/* saddr(16) + first_seg(1) + flags(1) + keyid(4) + seglist(16n) */
 	plen = 16 + 1 + 1 + 4 + (hdr->first_segment + 1) * 16;
 
 	/* this limit allows for 14 segments */
 	if (plen >= SEG6_HMAC_RING_SIZE)
@@ -217,26 +144,30 @@ int seg6_hmac_compute(struct seg6_hmac_info *hinfo, struct ipv6_sr_hdr *hdr,
 	for (i = 0; i < hdr->first_segment + 1; i++) {
 		memcpy(off, hdr->segments + i, 16);
 		off += 16;
 	}
 
-	dgsize = __do_hmac(hinfo, ring, plen, tmp_out,
-			   SEG6_HMAC_MAX_DIGESTSIZE);
+	switch (hinfo->alg_id) {
+	case SEG6_HMAC_ALGO_SHA1:
+		hmac_sha1_usingrawkey(hinfo->secret, hinfo->slen, ring, plen,
+				      output);
+		static_assert(SEG6_HMAC_FIELD_LEN > SHA1_DIGEST_SIZE);
+		memset(&output[SHA1_DIGEST_SIZE], 0,
+		       SEG6_HMAC_FIELD_LEN - SHA1_DIGEST_SIZE);
+		break;
+	case SEG6_HMAC_ALGO_SHA256:
+		hmac_sha256_usingrawkey(hinfo->secret, hinfo->slen, ring, plen,
+					output);
+		static_assert(SEG6_HMAC_FIELD_LEN == SHA256_DIGEST_SIZE);
+		break;
+	default:
+		ret = -ENOENT;
+		break;
+	}
 	local_unlock_nested_bh(&hmac_storage.bh_lock);
 	local_bh_enable();
-
-	if (dgsize < 0)
-		return dgsize;
-
-	wrsize = SEG6_HMAC_FIELD_LEN;
-	if (wrsize > dgsize)
-		wrsize = dgsize;
-
-	memset(output, 0, SEG6_HMAC_FIELD_LEN);
-	memcpy(output, tmp_out, wrsize);
-
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(seg6_hmac_compute);
 
 /* checks if an incoming SR-enabled packet's HMAC status matches
  * the incoming policy.
@@ -358,106 +289,17 @@ int seg6_push_hmac(struct net *net, struct in6_addr *saddr,
 	rcu_read_unlock();
 	return err;
 }
 EXPORT_SYMBOL(seg6_push_hmac);
 
-static int seg6_hmac_init_algo(void)
-{
-	struct seg6_hmac_algo *algo;
-	struct crypto_shash *tfm;
-	struct shash_desc *shash;
-	int i, alg_count, cpu;
-	int ret = -ENOMEM;
-
-	alg_count = ARRAY_SIZE(hmac_algos);
-
-	for (i = 0; i < alg_count; i++) {
-		struct crypto_shash **p_tfm;
-		int shsize;
-
-		algo = &hmac_algos[i];
-		algo->tfms = alloc_percpu(struct crypto_shash *);
-		if (!algo->tfms)
-			goto error_out;
-
-		for_each_possible_cpu(cpu) {
-			tfm = crypto_alloc_shash(algo->name, 0, 0);
-			if (IS_ERR(tfm)) {
-				ret = PTR_ERR(tfm);
-				goto error_out;
-			}
-			p_tfm = per_cpu_ptr(algo->tfms, cpu);
-			*p_tfm = tfm;
-		}
-
-		p_tfm = raw_cpu_ptr(algo->tfms);
-		tfm = *p_tfm;
-
-		shsize = sizeof(*shash) + crypto_shash_descsize(tfm);
-
-		algo->shashs = alloc_percpu(struct shash_desc *);
-		if (!algo->shashs)
-			goto error_out;
-
-		for_each_possible_cpu(cpu) {
-			shash = kzalloc_node(shsize, GFP_KERNEL,
-					     cpu_to_node(cpu));
-			if (!shash)
-				goto error_out;
-			*per_cpu_ptr(algo->shashs, cpu) = shash;
-		}
-	}
-
-	return 0;
-
-error_out:
-	seg6_hmac_exit();
-	return ret;
-}
-
-int __init seg6_hmac_init(void)
-{
-	return seg6_hmac_init_algo();
-}
-
 int __net_init seg6_hmac_net_init(struct net *net)
 {
 	struct seg6_pernet_data *sdata = seg6_pernet(net);
 
 	return rhashtable_init(&sdata->hmac_infos, &rht_params);
 }
 
-void seg6_hmac_exit(void)
-{
-	struct seg6_hmac_algo *algo = NULL;
-	struct crypto_shash *tfm;
-	struct shash_desc *shash;
-	int i, alg_count, cpu;
-
-	alg_count = ARRAY_SIZE(hmac_algos);
-	for (i = 0; i < alg_count; i++) {
-		algo = &hmac_algos[i];
-
-		if (algo->shashs) {
-			for_each_possible_cpu(cpu) {
-				shash = *per_cpu_ptr(algo->shashs, cpu);
-				kfree(shash);
-			}
-			free_percpu(algo->shashs);
-		}
-
-		if (algo->tfms) {
-			for_each_possible_cpu(cpu) {
-				tfm = *per_cpu_ptr(algo->tfms, cpu);
-				crypto_free_shash(tfm);
-			}
-			free_percpu(algo->tfms);
-		}
-	}
-}
-EXPORT_SYMBOL(seg6_hmac_exit);
-
 void __net_exit seg6_hmac_net_exit(struct net *net)
 {
 	struct seg6_pernet_data *sdata = seg6_pernet(net);
 
 	rhashtable_free_and_destroy(&sdata->hmac_infos, seg6_free_hi, NULL);
-- 
2.50.1

[PATCH net-next 3/3] ipv6: sr: Prepare HMAC key ahead of time

[Eric Biggers]

Prepare the HMAC key when it is added to the kernel, instead of
preparing it implicitly for every packet.  This significantly improves
the performance of seg6_hmac_compute().  A microbenchmark on x86_64
shows seg6_hmac_compute() (with HMAC-SHA256) dropping from ~1978 cycles
to ~1419 cycles, a 28% improvement.

The size of 'struct seg6_hmac_info' increases by 128 bytes, but that
should be fine, since there should not be a massive number of keys.

As a side effect, invalid values for SEG6_ATTR_ALGID (i.e., values other
than SEG6_HMAC_ALGO_SHA1 and SEG6_HMAC_ALGO_SHA256) now cause an error
immediately when the key is added, rather than later when computing a
HMAC value is attempted.  This seems like the expected behavior anyway.

Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
 include/net/seg6_hmac.h |  8 ++++++++
 net/ipv6/seg6.c         | 13 +++++++++++++
 net/ipv6/seg6_hmac.c    |  9 ++++-----
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/include/net/seg6_hmac.h b/include/net/seg6_hmac.h
index 3fe4123dbbf0a..e9f41725933e4 100644
--- a/include/net/seg6_hmac.h
+++ b/include/net/seg6_hmac.h
@@ -7,10 +7,12 @@
  */
 
 #ifndef _NET_SEG6_HMAC_H
 #define _NET_SEG6_HMAC_H
 
+#include <crypto/sha1.h>
+#include <crypto/sha2.h>
 #include <net/flow.h>
 #include <net/ip6_fib.h>
 #include <net/sock.h>
 #include <linux/ip.h>
 #include <linux/ipv6.h>
@@ -24,13 +26,19 @@
 struct seg6_hmac_info {
 	struct rhash_head node;
 	struct rcu_head rcu;
 
 	u32 hmackeyid;
+	/* The raw key, kept only so it can be returned back to userspace */
 	char secret[SEG6_HMAC_SECRET_LEN];
 	u8 slen;
 	u8 alg_id;
+	/* The prepared key, which the calculations actually use */
+	union {
+		struct hmac_sha1_key sha1;
+		struct hmac_sha256_key sha256;
+	} key;
 };
 
 extern int seg6_hmac_compute(struct seg6_hmac_info *hinfo,
 			     struct ipv6_sr_hdr *hdr, struct in6_addr *saddr,
 			     u8 *output);
diff --git a/net/ipv6/seg6.c b/net/ipv6/seg6.c
index a5c4c629b788c..313acdf1b2158 100644
--- a/net/ipv6/seg6.c
+++ b/net/ipv6/seg6.c
@@ -211,10 +211,23 @@ static int seg6_genl_sethmac(struct sk_buff *skb, struct genl_info *info)
 	memcpy(hinfo->secret, secret, slen);
 	hinfo->slen = slen;
 	hinfo->alg_id = algid;
 	hinfo->hmackeyid = hmackeyid;
 
+	switch (algid) {
+	case SEG6_HMAC_ALGO_SHA1:
+		hmac_sha1_preparekey(&hinfo->key.sha1, secret, slen);
+		break;
+	case SEG6_HMAC_ALGO_SHA256:
+		hmac_sha256_preparekey(&hinfo->key.sha256, secret, slen);
+		break;
+	default:
+		kfree(hinfo);
+		err = -EINVAL;
+		goto out_unlock;
+	}
+
 	err = seg6_hmac_info_add(net, hmackeyid, hinfo);
 	if (err)
 		kfree(hinfo);
 
 out_unlock:
diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index 816aef69e0cea..5cdcb3011cb3e 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -146,23 +146,22 @@ int seg6_hmac_compute(struct seg6_hmac_info *hinfo, struct ipv6_sr_hdr *hdr,
 		off += 16;
 	}
 
 	switch (hinfo->alg_id) {
 	case SEG6_HMAC_ALGO_SHA1:
-		hmac_sha1_usingrawkey(hinfo->secret, hinfo->slen, ring, plen,
-				      output);
+		hmac_sha1(&hinfo->key.sha1, ring, plen, output);
 		static_assert(SEG6_HMAC_FIELD_LEN > SHA1_DIGEST_SIZE);
 		memset(&output[SHA1_DIGEST_SIZE], 0,
 		       SEG6_HMAC_FIELD_LEN - SHA1_DIGEST_SIZE);
 		break;
 	case SEG6_HMAC_ALGO_SHA256:
-		hmac_sha256_usingrawkey(hinfo->secret, hinfo->slen, ring, plen,
-					output);
+		hmac_sha256(&hinfo->key.sha256, ring, plen, output);
 		static_assert(SEG6_HMAC_FIELD_LEN == SHA256_DIGEST_SIZE);
 		break;
 	default:
-		ret = -ENOENT;
+		WARN_ON_ONCE(1);
+		ret = -EINVAL;
 		break;
 	}
 	local_unlock_nested_bh(&hmac_storage.bh_lock);
 	local_bh_enable();
 	return ret;
-- 
2.50.1

Re: [PATCH net-next 2/3] ipv6: sr: Use HMAC-SHA1 and HMAC-SHA256 library functions

[Kuniyuki Iwashima]

From: Eric Biggers <ebiggers@kernel.org>
Date: Fri, 15 Aug 2025 20:11:35 -0700
> @@ -106,79 +95,17 @@ static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
>  		return NULL;
>  
>  	return tlv;
>  }
>  
> -static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
> -{
> -	struct seg6_hmac_algo *algo;
> -	int i, alg_count;
> -
> -	alg_count = ARRAY_SIZE(hmac_algos);
> -	for (i = 0; i < alg_count; i++) {
> -		algo = &hmac_algos[i];
> -		if (algo->alg_id == alg_id)
> -			return algo;
> -	}
> -
> -	return NULL;
> -}

This chunk will cause build failure when net.git is merged
to net-next due to the patch below.  You may want to respin
the series after this lands to net-next.

https://lore.kernel.org/netdev/20250815063845.85426-1-heminhong@kylinos.cn/

Re: [PATCH net-next 2/3] ipv6: sr: Use HMAC-SHA1 and HMAC-SHA256 library functions

[Eric Biggers]

On Sat, Aug 16, 2025 at 07:01:28AM +0000, Kuniyuki Iwashima wrote:
> From: Eric Biggers <ebiggers@kernel.org>
> Date: Fri, 15 Aug 2025 20:11:35 -0700
> > @@ -106,79 +95,17 @@ static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
> >  		return NULL;
> >  
> >  	return tlv;
> >  }
> >  
> > -static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
> > -{
> > -	struct seg6_hmac_algo *algo;
> > -	int i, alg_count;
> > -
> > -	alg_count = ARRAY_SIZE(hmac_algos);
> > -	for (i = 0; i < alg_count; i++) {
> > -		algo = &hmac_algos[i];
> > -		if (algo->alg_id == alg_id)
> > -			return algo;
> > -	}
> > -
> > -	return NULL;
> > -}
> 
> This chunk will cause build failure when net.git is merged
> to net-next due to the patch below.  You may want to respin
> the series after this lands to net-next.
> 
> https://lore.kernel.org/netdev/20250815063845.85426-1-heminhong@kylinos.cn/

Thanks for pointing that out.  I hadn't seen that patch.  Patch 3 in my
series actually fixes the exact same problem, though in my patch it's
more of a side effect of preparing the HMAC key rather than the main
point of the patch.  If that patch lands first, I'll rebase my series.

We do need to decide whether the algorithm ID validation and key
preparation should be done in seg6_hmac_info_add() as in that patch, or
in seg6_genl_sethmac() as in my patch.  seg6_hmac_info_add() is fine I
guess, but let me know if you have a preference.

- Eric

Re: [PATCH net-next 2/3] ipv6: sr: Use HMAC-SHA1 and HMAC-SHA256 library functions

[Ido Schimmel]

On Sat, Aug 16, 2025 at 12:26:39AM -0700, Eric Biggers wrote:
> On Sat, Aug 16, 2025 at 07:01:28AM +0000, Kuniyuki Iwashima wrote:
> > From: Eric Biggers <ebiggers@kernel.org>
> > Date: Fri, 15 Aug 2025 20:11:35 -0700
> > > @@ -106,79 +95,17 @@ static struct sr6_tlv_hmac *seg6_get_tlv_hmac(struct ipv6_sr_hdr *srh)
> > >  		return NULL;
> > >  
> > >  	return tlv;
> > >  }
> > >  
> > > -static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
> > > -{
> > > -	struct seg6_hmac_algo *algo;
> > > -	int i, alg_count;
> > > -
> > > -	alg_count = ARRAY_SIZE(hmac_algos);
> > > -	for (i = 0; i < alg_count; i++) {
> > > -		algo = &hmac_algos[i];
> > > -		if (algo->alg_id == alg_id)
> > > -			return algo;
> > > -	}
> > > -
> > > -	return NULL;
> > > -}
> > 
> > This chunk will cause build failure when net.git is merged
> > to net-next due to the patch below.  You may want to respin
> > the series after this lands to net-next.
> > 
> > https://lore.kernel.org/netdev/20250815063845.85426-1-heminhong@kylinos.cn/
> 
> Thanks for pointing that out.  I hadn't seen that patch.  Patch 3 in my
> series actually fixes the exact same problem, though in my patch it's
> more of a side effect of preparing the HMAC key rather than the main
> point of the patch.  If that patch lands first, I'll rebase my series.
> 
> We do need to decide whether the algorithm ID validation and key
> preparation should be done in seg6_hmac_info_add() as in that patch, or
> in seg6_genl_sethmac() as in my patch.  seg6_hmac_info_add() is fine I
> guess, but let me know if you have a preference.

FWIW, I think that as you have it now is fine given the other parameters
are also validated in seg6_genl_sethmac(). Exposing __hmac_get_algo()
seemed wrong to me so I suggested either moving the check to
seg6_hmac_info_add() or exposing something like 'bool
seg6_hmac_algo_is_valid(u8 alg_id)' which internally calls
__hmac_get_algo().

Re: [PATCH net-next 1/3] ipv6: sr: Fix MAC comparison to be constant-time

[Andrea Mayer]

On Fri, 15 Aug 2025 20:11:34 -0700
Eric Biggers <ebiggers@kernel.org> wrote:

> To prevent timing attacks, MACs need to be compared in constant time.
> Use the appropriate helper function for this.
> 
> Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> ---
>  net/ipv6/seg6_hmac.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 

Hi Eric,

Thanks for the fix!

I believe it would be best to submit this fix separately from the current patch
set. Since this addresses a bug rather than an enhancement or cleanup, sending
it individually with the 'net' tag will help facilitate applying this patch to
the net tree.

Ciao,
Andrea

> diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
> index f78ecb6ad8383..5dae892bbc73b 100644
> --- a/net/ipv6/seg6_hmac.c
> +++ b/net/ipv6/seg6_hmac.c
> @@ -33,10 +33,11 @@
>  #include <net/ip6_route.h>
>  #include <net/addrconf.h>
>  #include <net/xfrm.h>
>  
>  #include <crypto/hash.h>
> +#include <crypto/utils.h>
>  #include <net/seg6.h>
>  #include <net/genetlink.h>
>  #include <net/seg6_hmac.h>
>  #include <linux/random.h>
>  
> @@ -278,11 +279,11 @@ bool seg6_hmac_validate_skb(struct sk_buff *skb)
>  		return false;
>  
>  	if (seg6_hmac_compute(hinfo, srh, &ipv6_hdr(skb)->saddr, hmac_output))
>  		return false;
>  
> -	if (memcmp(hmac_output, tlv->hmac, SEG6_HMAC_FIELD_LEN) != 0)
> +	if (crypto_memneq(hmac_output, tlv->hmac, SEG6_HMAC_FIELD_LEN))
>  		return false;
>  
>  	return true;
>  }
>  EXPORT_SYMBOL(seg6_hmac_validate_skb);
> -- 
> 2.50.1
> 

Re: [PATCH net-next 1/3] ipv6: sr: Fix MAC comparison to be constant-time

[Eric Biggers]

On Mon, Aug 18, 2025 at 09:16:07PM +0200, Andrea Mayer wrote:
> On Fri, 15 Aug 2025 20:11:34 -0700
> Eric Biggers <ebiggers@kernel.org> wrote:
> 
> > To prevent timing attacks, MACs need to be compared in constant time.
> > Use the appropriate helper function for this.
> > 
> > Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > ---
> >  net/ipv6/seg6_hmac.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> 
> Hi Eric,
> 
> Thanks for the fix!
> 
> I believe it would be best to submit this fix separately from the current patch
> set. Since this addresses a bug rather than an enhancement or cleanup, sending
> it individually with the 'net' tag will help facilitate applying this patch to
> the net tree.
> 
> Ciao,
> Andrea

Then there would be a merge conflict between the two patchsets.

I can do that if you want, but then I'd probably have to wait for this
patch to reach net-next before the rest can proceed.

- Eric

Re: [PATCH net-next 1/3] ipv6: sr: Fix MAC comparison to be constant-time

[Eric Biggers]

On Mon, Aug 18, 2025 at 07:38:52PM +0000, Eric Biggers wrote:
> On Mon, Aug 18, 2025 at 09:16:07PM +0200, Andrea Mayer wrote:
> > On Fri, 15 Aug 2025 20:11:34 -0700
> > Eric Biggers <ebiggers@kernel.org> wrote:
> > 
> > > To prevent timing attacks, MACs need to be compared in constant time.
> > > Use the appropriate helper function for this.
> > > 
> > > Fixes: bf355b8d2c30 ("ipv6: sr: add core files for SR HMAC support")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Eric Biggers <ebiggers@kernel.org>
> > > ---
> > >  net/ipv6/seg6_hmac.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > 
> > 
> > Hi Eric,
> > 
> > Thanks for the fix!
> > 
> > I believe it would be best to submit this fix separately from the current patch
> > set. Since this addresses a bug rather than an enhancement or cleanup, sending
> > it individually with the 'net' tag will help facilitate applying this patch to
> > the net tree.
> > 
> > Ciao,
> > Andrea
> 
> Then there would be a merge conflict between the two patchsets.
> 
> I can do that if you want, but then I'd probably have to wait for this
> patch to reach net-next before the rest can proceed.

I guess patches 2 and 3 have to wait for "ipv6: sr: validate HMAC
algorithm ID in seg6_hmac_info_add" anyway, as they conflict with that
too.  Okay, I resent patch 1 as a standalone patch targeting "net":
https://lore.kernel.org/netdev/20250818202724.15713-1-ebiggers@kernel.org/

- Eric