Re: [PATCH net-next v2 1/5] net/tcp-ao: Drop support for most non-RFC-specified algorithms

[Eric Biggers]

On Wed, Apr 29, 2026 at 07:58:29PM +0100, Simon Horman wrote:
> Is this a uAPI regression for existing users of TCP_AO_ADD_KEY?
> 
> Before this change, tcp_ao_add::alg_name accepted any crypto_ahash
> algorithm name.  The commit message itself notes "arbitrary HMAC algorithms
> probably do work", and the selftest prior to this patch exercised
> hmac(sha512), hmac(sha384), hmac(sha224), hmac(sha3-512), hmac(rmd160) and
> hmac(md5), which demonstrates those configurations were functional in
> practice.
> 
> A setsockopt call that previously succeeded with e.g. alg_name =
> "hmac(sha512)" now returns -ENOENT after a kernel upgrade, with no
> deprecation window, no Kconfig opt-in, and no runtime toggle.  HMAC-SHA512
> in particular seems like a plausible choice that a deployment may already
> rely on.
> 
> Would it make sense to keep hmac(sha512) (and possibly hmac(sha384)) in the
> accepted list, or to gate the removal behind a Kconfig/sysctl while warning
> for a few releases?

This is intentional and has already been explained and discussed
extensively.

> Also, -ENOENT is the same error returned when the crypto module for an
> otherwise-accepted algorithm is simply not built in.  Should a distinct
> errno (for example -EOPNOTSUPP) be used here so userspace can tell
> "algorithm rejected by policy" apart from "algorithm module missing"?

This review seems a bit inconsistent, as now it's advocating *for* a
UAPI change.  I suggest we just stay with ENOENT, as this patch does.

> > diff --git a/tools/testing/selftests/net/tcp_ao/config b/tools/testing/selftests/net/tcp_ao/config
> > index f22148512365..47228a7d0b90 100644
> > --- a/tools/testing/selftests/net/tcp_ao/config
> > +++ b/tools/testing/selftests/net/tcp_ao/config
> > @@ -1,6 +1,5 @@
> >  CONFIG_CRYPTO_CMAC=y
> >  CONFIG_CRYPTO_HMAC=y
> > -CONFIG_CRYPTO_RMD160=y
> >  CONFIG_CRYPTO_SHA1=y
> >  CONFIG_IPV6=y
> >  CONFIG_IPV6_MULTIPLE_TABLES=y
> 
> Should CONFIG_CRYPTO_SHA256=y (and possibly CONFIG_CRYPTO_AES=y) be added
> here?

This is an existing bug, which patch 2 fixes by making CONFIG_TCP_AO
select the algorithms it uses.

> > diff --git a/tools/testing/selftests/net/tcp_ao/key-management.c b/tools/testing/selftests/net/tcp_ao/key-management.c
> > index 69d9a7a05d5c..d86bb380b79f 100644
> > --- a/tools/testing/selftests/net/tcp_ao/key-management.c
> > +++ b/tools/testing/selftests/net/tcp_ao/key-management.c
> 
> [ ... ]
> 
> > -const char *test_algos[] = {
> > -	"cmac(aes128)",
> > -	"hmac(sha1)", "hmac(sha512)", "hmac(sha384)", "hmac(sha256)",
> > -	"hmac(sha224)", "hmac(sha3-512)",
> > -	/* only if !CONFIG_FIPS */
> > -#define TEST_NON_FIPS_ALGOS	2
> > -	"hmac(rmd160)", "hmac(md5)"
> > -};
> > +const char *test_algos[] = { "cmac(aes128)", "hmac(sha1)", "hmac(sha256)" };
> 
> With the FIPS/algorithm-probing logic removed and roughly one third of
> generated keys now using hmac(sha256), and with the kernel side now
> returning -ENOENT for non-whitelisted algorithms, tcp_sigpool_alloc_ahash()
> will fail for every hmac(sha256) key on a kernel built from this config
> unless CRYPTO_SHA256 is selected.
> 
> CRYPTO_SHA256 is not implied by CRYPTO_HMAC or TCP_AO, so on a kernel built
> strictly from tools/testing/selftests/net/tcp_ao/config the tests would
> appear to fail rather than be skipped.  Can the config fragment be updated
> to match the new required algorithm set?

Again, this is an existing bug, which patch 2 fixes by making
CONFIG_TCP_AO select the algorithms it uses.

> One more question, on the commit message and documentation rather than the
> diff: Documentation/networking/tcp_ao.rst still describes TCP-AO as "May
> support any hashing algorithm"

That "May support any hashing algorithm" statement has always been
incorrect, so I wouldn't pay much attention to it.  It also appears in a
table describing TCP-AO as a protocol, not the kernel's implementation.

> and does not mention the newly enforced
> whitelist or the -ENOENT failure mode.  Should tcp_ao.rst be updated in
> this patch to list the accepted algorithm strings and the rationale (e.g.
> the 20-byte TCP option MAC cap), so userspace has a documented contract?

As stated in the commit message, the list of MAC algorithms supported by
the kernel's implementation of TCP-AO has always been undocumented.  It
should be documented, but I would suggest documentation improvements
belong in a separate patch.

- Eric