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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aKIKGI72EklLRfwO@shredder>
Date: Sun, 17 Aug 2025 19:58:00 +0300
From: Ido Schimmel <idosch@...sch.org>
To: Eric Biggers <ebiggers@...nel.org>
Cc: Kuniyuki Iwashima <kuniyu@...gle.com>, andrea.mayer@...roma2.it,
	dlebrun@...gle.com, netdev@...r.kernel.org,
	Minhong He <heminhong@...inos.cn>
Subject: Re: [PATCH net-next 2/3] ipv6: sr: Use HMAC-SHA1 and HMAC-SHA256
 library functions

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@...nel.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().