Am Mittwoch, 17. November 2021, 20:11:03 CET schrieb Eric Biggers:
Hi Eric,
thanks for your comments.
> On Mon, Nov 15, 2021 at 09:43:13AM +0100, Stephan Müller wrote:
> > SP800-108 defines three KDFs - this patch provides the counter KDF
> > implementation.
> >
> > The KDF is implemented as a service function where the caller has to
> > maintain the hash / HMAC state. Apart from this hash/HMAC state, no
> > additional state is required to be maintained by either the caller or
> > the KDF implementation.
> >
> > The key for the KDF is set with the crypto_kdf108_setkey function which
> > is intended to be invoked before the caller requests a key derivation
> > operation via crypto_kdf108_ctr_generate.
> >
> > SP800-108 allows the use of either a HMAC or a hash as crypto primitive
> > for the KDF. When a HMAC primtive is intended to be used,
> > crypto_kdf108_setkey must be used to set the HMAC key. Otherwise, for a
> > hash crypto primitve crypto_kdf108_ctr_generate can be used immediately
> > after allocating the hash handle.
> >
> > Signed-off-by: Stephan Mueller <smueller@xxxxxxxxxx>
> > ---
> >
> > crypto/Kconfig | 7 ++
> > crypto/Makefile | 5 ++
> > crypto/kdf_sp800108.c | 149 ++++++++++++++++++++++++++++++++++
> > include/crypto/kdf_sp800108.h | 61 ++++++++++++++
> > 4 files changed, 222 insertions(+)
> > create mode 100644 crypto/kdf_sp800108.c
> > create mode 100644 include/crypto/kdf_sp800108.h
> >
> > diff --git a/crypto/Kconfig b/crypto/Kconfig
> > index 285f82647d2b..09c393a57b58 100644
> > --- a/crypto/Kconfig
> > +++ b/crypto/Kconfig
> > @@ -1845,6 +1845,13 @@ config CRYPTO_JITTERENTROPY
> >
> > random numbers. This Jitterentropy RNG registers with
> > the kernel crypto API and can be used by any caller.
> >
> > +config CRYPTO_KDF800108_CTR
> > + tristate "Counter KDF (SP800-108)"
> > + select CRYPTO_HASH
> > + help
> > + Enable the key derivation function in counter mode compliant to
> > + SP800-108.
>
> These are just some library functions, so they shouldn't be user-selectable.
Ok, I will remove the user-visible entry in the kernel configuration.
> > +/*
> > + * The seeding of the KDF
> > + */
> > +int crypto_kdf108_setkey(struct crypto_shash *kmd,
> > + const u8 *key, size_t keylen,
> > + const u8 *ikm, size_t ikmlen)
> > +{
> > + unsigned int ds = crypto_shash_digestsize(kmd);
> > +
> > + /* SP800-108 does not support IKM */
> > + if (ikm || ikmlen)
> > + return -EINVAL;
>
> Why have the ikm parameter if it's not supported?
The original idea is that we have a common function declaration for SP800-108
and HKDF. I am still thinking that in the long run, a KDF template support may
make sense. In this case, a common function declaration would be needed for
all KDF implementations.
Furthermore, the test code can be shared between the different KDFs when we
allow the ikm/ikmlen parameter for this function.
>
> > + /*
> > + * We require that we operate on a MAC -- if we do not operate on a
> > + * MAC, this function returns an error.
> > + */
> > + return crypto_shash_setkey(kmd, key, keylen);
> > +}
> > +EXPORT_SYMBOL(crypto_kdf108_setkey);
>
> Well, crypto_shash_setkey() will succeed if the hash algorithm takes a
> "key". That doesn't necessarily mean that it's a MAC. It could be crc32 or
> xxhash64, for example; those interpret the "key" as the initial value.
Agreed. But I am not sure a check in this regard would be needed considering
that this KDF is only an internal service function.
I have updated the comment accordingly.
>
> > +static int __init crypto_kdf108_init(void)
> > +{
> > + int ret = kdf_test(&kdf_ctr_hmac_sha256_tv_template[0],
"hmac(sha256)",
> > + crypto_kdf108_setkey, crypto_kdf108_ctr_generate);
> > +
> > + if (ret)
> > + pr_warn("alg: self-tests for CTR-KDF (hmac(sha256)) failed
(rc=%d)\n",
> > + ret);
>
> This should be a WARN() since it indicates a kernel bug.
Changed. Considering that the test result behavior should be identical to
testmgr.c, I have added also the panic() call in case of fips_enabled.
Thanks a lot for your review.
>
> - Eric
Ciao
Stephan
