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