| 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
| ||
|
Message-ID: <20251007011142.GA77681@sol>
Date: Mon, 6 Oct 2025 18:11:42 -0700
From: Eric Biggers <ebiggers@...nel.org>
To: Ard Biesheuvel <ardb@...nel.org>
Cc: linux-crypto@...r.kernel.org, linux-kernel@...r.kernel.org,
"Jason A . Donenfeld" <Jason@...c4.com>,
Vegard Nossum <vegard.nossum@...cle.com>,
Joachim Vandersmissen <git@...sn.com>,
David Howells <dhowells@...hat.com>,
Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH] lib/crypto: Add FIPS pre-operational self-test for SHA
algorithms
On Tue, Oct 07, 2025 at 01:53:25AM +0200, Ard Biesheuvel wrote:
> On Mon, 6 Oct 2025 at 19:28, Eric Biggers <ebiggers@...nel.org> wrote:
> >
> > Add FIPS pre-operational self-tests for all SHA-1 and SHA-2 algorithms.
> > Following the "Implementation Guidance for FIPS 140-3" document, to
> > achieve this it's sufficient to just test a single test vector for each
> > of HMAC-SHA1, HMAC-SHA256, and HMAC-SHA512.
> >
> > Link: https://lore.kernel.org/linux-crypto/20250917184856.GA2560@quark/
> > Signed-off-by: Eric Biggers <ebiggers@...nel.org>
> > ---
> >
> > Since there seemed to be more interest in complaining that these are
> > missing than actually writing a patch, I decided to just do it.
> >
> > lib/crypto/fips.h | 38 +++++++++++++++++++++++++++++
> > lib/crypto/sha1.c | 19 ++++++++++++++-
> > lib/crypto/sha256.c | 19 ++++++++++++++-
> > lib/crypto/sha512.c | 19 ++++++++++++++-
> > scripts/crypto/gen-fips-testvecs.py | 33 +++++++++++++++++++++++++
> > 5 files changed, 125 insertions(+), 3 deletions(-)
> > create mode 100644 lib/crypto/fips.h
> > create mode 100755 scripts/crypto/gen-fips-testvecs.py
> >
> > diff --git a/lib/crypto/fips.h b/lib/crypto/fips.h
> > new file mode 100644
> > index 0000000000000..78a1bdd33a151
> > --- /dev/null
> > +++ b/lib/crypto/fips.h
> > @@ -0,0 +1,38 @@
> > +/* SPDX-License-Identifier: GPL-2.0-or-later */
> > +/* This file was generated by: gen-fips-testvecs.py */
> > +
> > +#include <linux/fips.h>
> > +
> > +static const u8 fips_test_data[] __initconst __maybe_unused = {
> > + 0x66, 0x69, 0x70, 0x73, 0x20, 0x74, 0x65, 0x73,
> > + 0x74, 0x20, 0x64, 0x61, 0x74, 0x61, 0x00, 0x00,
> > +};
> > +
> > +static const u8 fips_test_key[] __initconst __maybe_unused = {
> > + 0x66, 0x69, 0x70, 0x73, 0x20, 0x74, 0x65, 0x73,
> > + 0x74, 0x20, 0x6b, 0x65, 0x79, 0x00, 0x00, 0x00,
> > +};
> > +
> > +static const u8 fips_test_hmac_sha1_value[] __initconst __maybe_unused = {
> > + 0x29, 0xa9, 0x88, 0xb8, 0x5c, 0xb4, 0xaf, 0x4b,
> > + 0x97, 0x2a, 0xee, 0x87, 0x5b, 0x0a, 0x02, 0x55,
> > + 0x99, 0xbf, 0x86, 0x78,
> > +};
> > +
> > +static const u8 fips_test_hmac_sha256_value[] __initconst __maybe_unused = {
> > + 0x59, 0x25, 0x85, 0xcc, 0x40, 0xe9, 0x64, 0x2f,
> > + 0xe9, 0xbf, 0x82, 0xb7, 0xd3, 0x15, 0x3d, 0x43,
> > + 0x22, 0x0b, 0x4c, 0x00, 0x90, 0x14, 0x25, 0xcf,
> > + 0x9e, 0x13, 0x2b, 0xc2, 0x30, 0xe6, 0xe8, 0x93,
> > +};
> > +
> > +static const u8 fips_test_hmac_sha512_value[] __initconst __maybe_unused = {
> > + 0x6b, 0xea, 0x5d, 0x27, 0x49, 0x5b, 0x3f, 0xea,
> > + 0xde, 0x2d, 0xfa, 0x32, 0x75, 0xdb, 0x77, 0xc8,
> > + 0x26, 0xe9, 0x4e, 0x95, 0x4d, 0xad, 0x88, 0x02,
> > + 0x87, 0xf9, 0x52, 0x0a, 0xd1, 0x92, 0x80, 0x1d,
> > + 0x92, 0x7e, 0x3c, 0xbd, 0xb1, 0x3c, 0x49, 0x98,
> > + 0x44, 0x9c, 0x8f, 0xee, 0x3f, 0x02, 0x71, 0x51,
> > + 0x57, 0x0b, 0x15, 0x38, 0x95, 0xd8, 0xa3, 0x81,
> > + 0xba, 0xb3, 0x15, 0x37, 0x5c, 0x6d, 0x57, 0x2b,
> > +};
> > diff --git a/lib/crypto/sha1.c b/lib/crypto/sha1.c
> > index 5904e4ae85d24..001059cb0fce4 100644
> > --- a/lib/crypto/sha1.c
> > +++ b/lib/crypto/sha1.c
> > @@ -10,10 +10,11 @@
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/string.h>
> > #include <linux/unaligned.h>
> > #include <linux/wordpart.h>
> > +#include "fips.h"
> >
> > static const struct sha1_block_state sha1_iv = {
> > .h = { SHA1_H0, SHA1_H1, SHA1_H2, SHA1_H3, SHA1_H4 },
> > };
> >
> > @@ -328,14 +329,30 @@ void hmac_sha1_usingrawkey(const u8 *raw_key, size_t raw_key_len,
> > hmac_sha1_update(&ctx, data, data_len);
> > hmac_sha1_final(&ctx, out);
> > }
> > EXPORT_SYMBOL_GPL(hmac_sha1_usingrawkey);
> >
> > -#ifdef sha1_mod_init_arch
> > +#if defined(sha1_mod_init_arch) || defined(CONFIG_CRYPTO_FIPS)
> > static int __init sha1_mod_init(void)
> > {
> > +#ifdef sha1_mod_init_arch
> > sha1_mod_init_arch();
> > +#endif
> > + if (fips_enabled) {
> > + /*
> > + * FIPS pre-operational self-test. As per the FIPS
> > + * Implementation Guidance, testing HMAC-SHA1 satisfies the test
> > + * requirement for SHA-1 too.
> > + */
> > + u8 mac[SHA1_DIGEST_SIZE];
> > +
> > + hmac_sha1_usingrawkey(fips_test_key, sizeof(fips_test_key),
> > + fips_test_data, sizeof(fips_test_data),
> > + mac);
> > + if (memcmp(fips_test_hmac_sha1_value, mac, sizeof(mac)) != 0)
> > + panic("sha1: FIPS pre-operational self-test failed\n");
> > + }
> > return 0;
> > }
> > subsys_initcall(sha1_mod_init);
> >
>
> In the builtin case, couldn't this execute only after the first calls
> into the library? That would mean it does not quite fit the
> requirements of the pre-operational selftest.
Only if other builtin code in the kernel actually calls it before
subsys_initcall, i.e. during very early boot long before userspace
starts. Such calls can occur only from within the FIPS module (i.e. the
kernel) itself, so arbitrary external users need not be considered here.
x86's SHA-256 microcode checksumming is the only such call I've noticed.
But even then I doubt it actually matters. There are several possible
reasons why even that early SHA-256 could be fine. E.g. it could be
interpreted as not being used for a security purpose, or as not
producing any output from the FIPS module (i.e. the kernel) yet
considering that lib/crypto/ is an internal interface.
And of course, the x86 microcode checksumming predated my SHA-256
cleanups in 6.16 and 6.17. So it's not like anyone cared before...
> So perhaps, we should wrap the fips test in a separate function, and
> in the builtin case, add a call to it to every exported library
> routine, conditional on some static key that gets set on success? With
> the right macro foo, it doesn't have to be that ugly, and it can just
> disappear entirely if FIPS support is disabled.
>
> (For all I care, we don't bother with this at all, but if we add this
> it should be solid)
I think we should try the simpler solution from this patch first, which
is already used in crypto/kdf_sp800108.c, crypto/hkdf.c,
crypto/asymmetric_keys/selftest.c, and lib/crypto/aesgcm.c. I suspect
the FIPS labs will be fine with it; subsys_initcall is very early. If
they were actually using an overly-strict and pedantic interpretation of
FIPS, then FIPS-certified Linux kernels would not already exist.
- Eric
Powered by blists - more mailing lists