Re: [PATCH] nfsd: Use MD5 library instead of crypto_shash

[Simo Sorce <simo@redhat.com>]

On Sun, 2025-10-12 at 10:00 -0700, Eric Biggers wrote:
> On Sun, Oct 12, 2025 at 07:12:26AM -0400, Jeff Layton wrote:
> > On Sat, 2025-10-11 at 11:52 -0700, Eric Biggers wrote:
> > > Update NFSD's support for "legacy client tracking" (which uses MD5) to
> > > use the MD5 library instead of crypto_shash.  This has several benefits:
> > > 
> > > - Simpler code.  Notably, much of the error-handling code is no longer
> > >   needed, since the library functions can't fail.
> > > 
> > > - Improved performance due to reduced overhead.  A microbenchmark of
> > >   nfs4_make_rec_clidname() shows a speedup from 1455 cycles to 425.
> > > 
> > > - The MD5 code can now safely be built as a loadable module when nfsd is
> > >   built as a loadable module.  (Previously, nfsd forced the MD5 code to
> > >   built-in, presumably to work around the unreliablity of the name-based
> > >   loading.)  Thus, select MD5 from the tristate option NFSD if
> > >   NFSD_LEGACY_CLIENT_TRACKING, instead of from the bool option NFSD_V4.
> > > 
> > > To preserve the existing behavior of legacy client tracking support
> > > being disabled when the kernel is booted with "fips=1", make
> > > nfsd4_legacy_tracking_init() return an error if fips_enabled.  I don't
> > > know if this is truly needed, but it preserves the existing behavior.
> > > 
> > 
> > FIPS is pretty draconian about algorithms, AIUI. We're not using MD5 in
> > a cryptographically significant way here, but the FIPS gods won't bless
> > a kernel that uses MD5 at all, so I think it is needed.
> 
> If it's not being used for a security purpose, then I think you can just
> drop the fips_enabled check.  People are used to the old API where MD5
> was always forbidden when fips_enabled, but it doesn't actually need to
> be that strict.  For this patch I wasn't certain about the use case
> though, so I just opted to preserve the existing behavior for now.  A
> follow-on patch to remove the check could make sense.

It would be nice to move MD5 (and reasonably soon after SHA-1 too) out
of lib/crypto and in some generic hashing utility place because they
are not cryptographic algorithms anymore and nobody should use them as
such.

That said MD5 appears to be used for cryptographic purposes (key/IV
derivation) in ecryptfs (which is pretty bad) and therefore ecryptfs
should be disabled in fips mode regardless (at least until they change
this aspect of the fs).

Specifically for this patch though I do not think you should keep
disabling nfsd4_legacy_tracking_init() in fips mode, as md5 here is not
used in a cryptographic capacity, it is just an identifier that is
easier to index.

Simo.

-- 
Simo Sorce
Distinguished Engineer
RHEL Crypto Team
Red Hat, Inc