On Sun, Jan 24, 2021 at 03:03:28PM +0100, Stephan Müller wrote:
> RFC5869 specifies an extract and expand two-step key derivation
> function. The HKDF implementation is provided as a service function that
> operates on a caller-provided HMAC handle. The caller has to allocate
> the HMAC shash handle and then can invoke the HKDF service functions.
> The HKDF implementation ensures that the entire state is kept with the
> HMAC shash handle which implies that no additional state is required to
> be maintained by the HKDF implementation.
> 
> The extract function is invoked via the crypto_hkdf_extract call. RFC5869
> allows two optional parameters to be provided to the extract operation:
> the salt and input key material (IKM). Both are to be provided with the
> seed parameter where the salt is the first entry of the seed parameter
> and all subsequent entries are handled as IKM. If the caller intends to
> invoke the HKDF without salt, it has to provide a NULL/0 entry as first
> entry in seed.

The part about the "seed parameter" is outdated.

> The expand function is invoked via crypto_hkdf_expand and can be
> invoked multiple times. This function allows the caller to provide a
> context for the key derivation operation. As specified in RFC5869, it is
> optional. In case such context is not provided, the caller must provide
> NULL / 0 for the info / info_nvec parameters.

This commit message doesn't actually mention of *why* this patch is useful.  Is
it because there are going to be more uses of HKDF in the kernel besides the one
in fs/crypto/?  If so, what are those planned uses?

> diff --git a/crypto/Kconfig b/crypto/Kconfig
> index 9f375c2350f5..661287d7283b 100644
> --- a/crypto/Kconfig
> +++ b/crypto/Kconfig
> @@ -1862,6 +1862,13 @@ config CRYPTO_JITTERENTROPY
>         random numbers. This Jitterentropy RNG registers with
>         the kernel crypto API and can be used by any caller.
>  
> +config CRYPTO_HKDF
> +     tristate "Extract and Expand HKDF (RFC 5869)"
> +     select CRYPTO_HASH
> +     help
> +       Enable the extract and expand key derivation function compliant
> +       to RFC 5869.

This is just a library function, so it shouldn't be user-selectable.
It should just be selected by the kconfig options that need it.

> diff --git a/crypto/hkdf.c b/crypto/hkdf.c
> new file mode 100644
> index 000000000000..8e80eca202e7
> --- /dev/null
> +++ b/crypto/hkdf.c
> @@ -0,0 +1,199 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * HMAC-based Extract-and-Expand Key Derivation Function (conformant to 
> RFC5869)
> + *
> + * Copyright (C) 2020, Stephan Mueller <smuel...@chronox.de>
> + */
> +
> +#include <linux/module.h>
> +#include <crypto/hkdf.h>
> +#include <crypto/internal/kdf_selftest.h>
> +
> +/*
> + * HKDF expand phase
> + */
> +int crypto_hkdf_expand(struct crypto_shash *kmd,
> +                    const struct kvec *info, unsigned int info_nvec,
> +                    u8 *dst, unsigned int dlen)
> +{
> +     SHASH_DESC_ON_STACK(desc, kmd);
> +     const unsigned int h = crypto_shash_digestsize(kmd), dlen_orig = dlen;
> +     unsigned int i;
> +     int err = 0;
> +     u8 *dst_orig = dst;
> +     const u8 *prev = NULL;
> +     u8 ctr = 0x01;
> +
> +     if (dlen > h * 255)
> +             return -EINVAL;
> +
> +     desc->tfm = kmd;
> +
> +     /* T(1) and following */
> +     while (dlen) {
> +             err = crypto_shash_init(desc);
> +             if (err)
> +                     goto out;
> +
> +             if (prev) {
> +                     err = crypto_shash_update(desc, prev, h);
> +                     if (err)
> +                             goto out;
> +             }
> +
> +             for (i = 0; i < info_nvec; i++) {
> +                     err = crypto_shash_update(desc, info[i].iov_base,
> +                                               info[i].iov_len);
> +                     if (err)
> +                             goto out;
> +             }
> +
> +             if (dlen < h) {
> +                     u8 tmpbuffer[HASH_MAX_DIGESTSIZE];
> +
> +                     err = crypto_shash_finup(desc, &ctr, 1, tmpbuffer);
> +                     if (err)
> +                             goto out;
> +                     memcpy(dst, tmpbuffer, dlen);
> +                     memzero_explicit(tmpbuffer, h);
> +                     goto out;
> +             }
> +
> +             err = crypto_shash_finup(desc, &ctr, 1, dst);
> +             if (err)
> +                     goto out;
> +
> +             prev = dst;
> +             dst += h;
> +             dlen -= h;
> +             ctr++;
> +     }
> +
> +out:
> +     if (err)
> +             memzero_explicit(dst_orig, dlen_orig);
> +     shash_desc_zero(desc);
> +     return err;
> +}
> +EXPORT_SYMBOL(crypto_hkdf_expand);

EXPORT_SYMBOL_GPL?

> +
> +/*
> + * HKDF extract phase.
> + */
> +int crypto_hkdf_extract(struct crypto_shash *kmd,
> +                     const u8 *salt, size_t saltlen,
> +                     const u8 *ikm, size_t ikmlen)
> +{
> +     SHASH_DESC_ON_STACK(desc, kmd);
> +     unsigned int h = crypto_shash_digestsize(kmd);
> +     int err;
> +     static const u8 null_salt[HASH_MAX_DIGESTSIZE] = { 0 };
> +     u8 prk[HASH_MAX_DIGESTSIZE];
> +
> +     desc->tfm = kmd;
> +
> +     if (salt && saltlen) {

Checking 'salt && saltlen' like this is poor practice, as then if people
accidentally use salt == NULL && saltlen != 0 when they intended to use a salt,
the bug won't be detected.  Just doing 'if (saltlen)' would be better.

> +
> +     /* Extract the PRK */
> +     err = crypto_shash_init(desc);
> +     if (err)
> +             goto err;
> +
> +     err = crypto_shash_finup(desc, ikm, ikmlen, prk);
> +     if (err)
> +             goto err;

This should use crypto_shash_digest() instead of crypto_shash_init() +
crypto_shash_finup().

> +/* Test vectors from RFC 5869 appendix A */
> +static const struct kdf_testvec hkdf_hmac_sha256_tv_template[] = {
> +     {
> +             /* salt */
> +             .key = "\x00\x01\x02\x03\x04\x05\x06\x07"
> +                    "\x08\x09\x0a\x0b\x0c",
> +             .keylen  = 13,
> +             .ikm = "\x0b\x0b\x0b\x0b\x0b\x0b\x0b\x0b"
> +                    "\x0b\x0b\x0b\x0b\x0b\x0b\x0b\x0b"
> +                    "\x0b\x0b\x0b\x0b\x0b\x0b",
> +             .ikmlen = 22,
> +             .info = {
> +                     .iov_base = "\xf0\xf1\xf2\xf3\xf4\xf5\xf6\xf7"
> +                                 "\xf8\xf9",
> +                     .iov_len  = 10
> +             },
> +             .expected         = "\x3c\xb2\x5f\x25\xfa\xac\xd5\x7a"
> +                                 "\x90\x43\x4f\x64\xd0\x36\x2f\x2a"
> +                                 "\x2d\x2d\x0a\x90\xcf\x1a\x5a\x4c"
> +                                 "\x5d\xb0\x2d\x56\xec\xc4\xc5\xbf"
> +                                 "\x34\x00\x72\x08\xd5\xb8\x87\x18"
> +                                 "\x58\x65",
> +             .expectedlen      = 42
> +     }, {
> +             /* salt */
> +             .key = NULL,
> +             .keylen  = 0,
> +             .ikm = "\x0b\x0b\x0b\x0b\x0b\x0b\x0b\x0b"
> +                    "\x0b\x0b\x0b\x0b\x0b\x0b\x0b\x0b"
> +                    "\x0b\x0b\x0b\x0b\x0b\x0b",
> +             .ikmlen  = 22,
> +             .info = {
> +                     .iov_base = NULL,
> +                     .iov_len  = 0
> +             },
> +             .expected         = "\x8d\xa4\xe7\x75\xa5\x63\xc1\x8f"
> +                                 "\x71\x5f\x80\x2a\x06\x3c\x5a\x31"
> +                                 "\xb8\xa1\x1f\x5c\x5e\xe1\x87\x9e"
> +                                 "\xc3\x45\x4e\x5f\x3c\x73\x8d\x2d"
> +                                 "\x9d\x20\x13\x95\xfa\xa4\xb6\x1a"
> +                                 "\x96\xc8",
> +             .expectedlen      = 42
> +     }
> +};

The case of multiple entries in the 'info' kvec isn't being tested.

Also, it is confusing having both 'key' and 'ikm'.  'key' really should be
'salt'.

> diff --git a/include/crypto/hkdf.h b/include/crypto/hkdf.h
> new file mode 100644
> index 000000000000..c6989f786860
> --- /dev/null
> +++ b/include/crypto/hkdf.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Copyright (C) 2020, Stephan Mueller <smuel...@chronox.de>
> + */
> +
> +#ifndef _CRYPTO_HKDF_H
> +#define _CRYPTO_HKDF_H
> +
> +#include <crypto/hash.h>
> +#include <linux/uio.h>
> +
> +/**
> + * RFC 5869 HKDF expand operation
> + *
> + * @kmd Keyed message digest whose key was set with crypto_hkdf_extract
> + * @info optional context and application specific information - this may be
> + *    NULL
> + * @info_vec number of optional context/application specific information 
> entries
> + * @dst destination buffer that the caller already allocated
> + * @dlen length of the destination buffer - the KDF derives that amount of
> + *    bytes.
> + *
> + * @return 0 on success, < 0 on error
> + */
> +int crypto_hkdf_expand(struct crypto_shash *kmd,
> +                    const struct kvec *info, unsigned int info_nvec,
> +                    u8 *dst, unsigned int dlen);
> +
> +/**
> + * RFC 5869 HKDF extract operation
> + *
> + * @kmd Keyed message digest allocated by the caller. The key should not have
> + *   been set.
> + * @salt The salt used for the KDF. It is permissible to provide NULL as salt
> + *    which implies that the default salt is used.
> + * @saltlen Length of the salt buffer.
> + * @ikm The input key material (IKM). It is permissible to provide NULL as 
> IKM.
> + * @ikmlen Length of the IKM buffer
> + * @seed_nvec number of seed entries (must be at least 1)

seed_nvec no longer exists.

- Eric

Reply via email to