Re: [PATCH v3 2/4] crypto: add SP800-108 counter key derivation function

From: Eric Biggers
Date: Wed Nov 17 2021 - 14:11:08 EST


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.

> +/*
> + * 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?

> + /*
> + * 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.

> +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.

- Eric