The IPv6 SR HMAC implementation supports sha1 and sha256, but would
silently accept any other value configured for the ->alg_id -- it just
would fail to create such an HMAC then.

That's certainly fine, as users attempting to configure random ->alg_ids
don't deserve any better.

However, a subsequent patch will enable a scenario where the instantiation
of a supported HMAC algorithm may fail, namely with SHA1 when booted in
FIPS mode.

As such an instantiation failure would depend on the system configuration,
i.e. whether FIPS mode is enabled or not, it would be better to report a
proper error back at configuration time rather than to e.g. silently drop
packets during operation.

Make __hmac_get_algo() to filter algos with ->tfms == NULL, indicating
an instantiation failure. Note that this cannot happen yet at this very
moment, as the IPv6 SR HMAC __init code would have failed then. As said,
it's a scenario enabled only with a subsequent patch.

Make seg6_hmac_info_add() to return -ENOENT to the user in case
__hmac_get_algo() fails to find a matching algo.

Signed-off-by: Nicolai Stange <nstange@suse.de>
---
 net/ipv6/seg6_hmac.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
index bbf5b84a70fc..77bdb41d3b82 100644
--- a/net/ipv6/seg6_hmac.c
+++ b/net/ipv6/seg6_hmac.c
@@ -108,7 +108,7 @@ static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
 	alg_count = ARRAY_SIZE(hmac_algos);
 	for (i = 0; i < alg_count; i++) {
 		algo = &hmac_algos[i];
-		if (algo->alg_id == alg_id)
+		if (algo->alg_id == alg_id && algo->tfms)
 			return algo;
 	}
 
@@ -293,8 +293,13 @@ EXPORT_SYMBOL(seg6_hmac_info_lookup);
 int seg6_hmac_info_add(struct net *net, u32 key, struct seg6_hmac_info *hinfo)
 {
 	struct seg6_pernet_data *sdata = seg6_pernet(net);
+	struct seg6_hmac_algo *algo;
 	int err;
 
+	algo = __hmac_get_algo(hinfo->alg_id);
+	if (!algo || !algo->tfms)
+		return -ENOENT;
+
 	err = rhashtable_lookup_insert_fast(&sdata->hmac_infos, &hinfo->node,
 					    rht_params);
 
-- 
2.47.1

On 3/10/25 5:58 PM, Nicolai Stange wrote:
> The IPv6 SR HMAC implementation supports sha1 and sha256, but would
> silently accept any other value configured for the ->alg_id -- it just
> would fail to create such an HMAC then.
> 
> That's certainly fine, as users attempting to configure random ->alg_ids
> don't deserve any better.
> 
> However, a subsequent patch will enable a scenario where the instantiation
> of a supported HMAC algorithm may fail, namely with SHA1 when booted in
> FIPS mode.
> 
> As such an instantiation failure would depend on the system configuration,
> i.e. whether FIPS mode is enabled or not, it would be better to report a
> proper error back at configuration time rather than to e.g. silently drop
> packets during operation.
> 
> Make __hmac_get_algo() to filter algos with ->tfms == NULL, indicating
> an instantiation failure. Note that this cannot happen yet at this very
> moment, as the IPv6 SR HMAC __init code would have failed then. As said,
> it's a scenario enabled only with a subsequent patch.
> 
> Make seg6_hmac_info_add() to return -ENOENT to the user in case
> __hmac_get_algo() fails to find a matching algo.
> 
> Signed-off-by: Nicolai Stange <nstange@suse.de>

Please specify the target tree inside the subj prefix, in this case
'net-next'.

> ---
>  net/ipv6/seg6_hmac.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c
> index bbf5b84a70fc..77bdb41d3b82 100644
> --- a/net/ipv6/seg6_hmac.c
> +++ b/net/ipv6/seg6_hmac.c
> @@ -108,7 +108,7 @@ static struct seg6_hmac_algo *__hmac_get_algo(u8 alg_id)
>  	alg_count = ARRAY_SIZE(hmac_algos);
>  	for (i = 0; i < alg_count; i++) {
>  		algo = &hmac_algos[i];
> -		if (algo->alg_id == alg_id)
> +		if (algo->alg_id == alg_id && algo->tfms)
>  			return algo;
>  	}
>  
> @@ -293,8 +293,13 @@ EXPORT_SYMBOL(seg6_hmac_info_lookup);
>  int seg6_hmac_info_add(struct net *net, u32 key, struct seg6_hmac_info *hinfo)
>  {
>  	struct seg6_pernet_data *sdata = seg6_pernet(net);
> +	struct seg6_hmac_algo *algo;
>  	int err;
>  
> +	algo = __hmac_get_algo(hinfo->alg_id);
> +	if (!algo || !algo->tfms)

The above check '!algo->tfms' looks redundant with the previous one.

Thanks,

Paolo