Hi,
The key derviation functions are considered to be a cryptographic
operation. As cryptographic operations are provided via the kernel
crypto API, this patch set consolidates the SP800-108 KDF
implementation into the crypto API.
If this patch is accepted, another patch set will be published attempting
to move the HKDF implementation from the crypto file system code base
to the kernel crypto API.
The KDF implementation is provided as service functions. Yet, the
interface to the the provided KDF is modeled such, that additional
KDF implementation can use the same API style. The goal is to allow
the transformation from a service function into a crypto API template
eventually.
The KDF executes a power-on self test with test vectors from commonly
known sources.
Tbe SP800-108 KDF implementation is used to replace the implementation
in the keys subsystem. The implementation was verified using the
keyutils command line test code provided in
tests/keyctl/dh_compute/valid. All tests show that the expected values
are calculated with the new code.
Changes v3:
* port to kernel 5.16-rc1
* remove the HKDF patch to only leave the SP800-108 patch
Stephan Mueller (4):
crypto: Add key derivation self-test support code
crypto: add SP800-108 counter key derivation function
security: DH - remove dead code for zero padding
security: DH - use KDF implementation from crypto API
crypto/Kconfig | 7 ++
crypto/Makefile | 5 +
crypto/kdf_sp800108.c | 149 +++++++++++++++++++++++++
include/crypto/internal/kdf_selftest.h | 71 ++++++++++++
include/crypto/kdf_sp800108.h | 61 ++++++++++
security/keys/Kconfig | 2 +-
security/keys/dh.c | 118 +++-----------------
7 files changed, 310 insertions(+), 103 deletions(-)
create mode 100644 crypto/kdf_sp800108.c
create mode 100644 include/crypto/internal/kdf_selftest.h
create mode 100644 include/crypto/kdf_sp800108.h
--
2.33.1
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 <[email protected]>
---
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.
+
config CRYPTO_USER_API
tristate
diff --git a/crypto/Makefile b/crypto/Makefile
index 429c4d57458c..d76bff8d0ffd 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -200,3 +200,8 @@ obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += asymmetric_keys/
obj-$(CONFIG_CRYPTO_HASH_INFO) += hash_info.o
crypto_simd-y := simd.o
obj-$(CONFIG_CRYPTO_SIMD) += crypto_simd.o
+
+#
+# Key derivation function
+#
+obj-$(CONFIG_CRYPTO_KDF800108_CTR) += kdf_sp800108.o
diff --git a/crypto/kdf_sp800108.c b/crypto/kdf_sp800108.c
new file mode 100644
index 000000000000..1b7516615c34
--- /dev/null
+++ b/crypto/kdf_sp800108.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * SP800-108 Key-derivation function
+ *
+ * Copyright (C) 2021, Stephan Mueller <[email protected]>
+ */
+
+#include <linux/module.h>
+#include <crypto/kdf_sp800108.h>
+#include <crypto/internal/kdf_selftest.h>
+
+/*
+ * SP800-108 CTR KDF implementation
+ */
+int crypto_kdf108_ctr_generate(struct crypto_shash *kmd,
+ const struct kvec *info, unsigned int info_nvec,
+ u8 *dst, unsigned int dlen)
+{
+ SHASH_DESC_ON_STACK(desc, kmd);
+ __be32 counter = cpu_to_be32(1);
+ const unsigned int h = crypto_shash_digestsize(kmd), dlen_orig = dlen;
+ unsigned int i;
+ int err = 0;
+ u8 *dst_orig = dst;
+
+ desc->tfm = kmd;
+
+ while (dlen) {
+ err = crypto_shash_init(desc);
+ if (err)
+ goto out;
+
+ err = crypto_shash_update(desc, (u8 *)&counter, sizeof(__be32));
+ 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_final(desc, tmpbuffer);
+ if (err)
+ goto out;
+ memcpy(dst, tmpbuffer, dlen);
+ memzero_explicit(tmpbuffer, h);
+ goto out;
+ }
+
+ err = crypto_shash_final(desc, dst);
+ if (err)
+ goto out;
+
+ dlen -= h;
+ dst += h;
+ counter = cpu_to_be32(be32_to_cpu(counter) + 1);
+ }
+
+out:
+ if (err)
+ memzero_explicit(dst_orig, dlen_orig);
+ shash_desc_zero(desc);
+ return err;
+}
+EXPORT_SYMBOL(crypto_kdf108_ctr_generate);
+
+/*
+ * 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;
+
+ /* Check according to SP800-108 section 7.2 */
+ if (ds > keylen)
+ return -EINVAL;
+
+ /*
+ * 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);
+
+/*
+ * Test vector obtained from
+ * http://csrc.nist.gov/groups/STM/cavp/documents/KBKDF800-108/CounterMode.zip
+ */
+static const struct kdf_testvec kdf_ctr_hmac_sha256_tv_template[] = {
+ {
+ .key = "\xdd\x1d\x91\xb7\xd9\x0b\x2b\xd3"
+ "\x13\x85\x33\xce\x92\xb2\x72\xfb"
+ "\xf8\xa3\x69\x31\x6a\xef\xe2\x42"
+ "\xe6\x59\xcc\x0a\xe2\x38\xaf\xe0",
+ .keylen = 32,
+ .ikm = NULL,
+ .ikmlen = 0,
+ .info = {
+ .iov_base = "\x01\x32\x2b\x96\xb3\x0a\xcd\x19"
+ "\x79\x79\x44\x4e\x46\x8e\x1c\x5c"
+ "\x68\x59\xbf\x1b\x1c\xf9\x51\xb7"
+ "\xe7\x25\x30\x3e\x23\x7e\x46\xb8"
+ "\x64\xa1\x45\xfa\xb2\x5e\x51\x7b"
+ "\x08\xf8\x68\x3d\x03\x15\xbb\x29"
+ "\x11\xd8\x0a\x0e\x8a\xba\x17\xf3"
+ "\xb4\x13\xfa\xac",
+ .iov_len = 60
+ },
+ .expected = "\x10\x62\x13\x42\xbf\xb0\xfd\x40"
+ "\x04\x6c\x0e\x29\xf2\xcf\xdb\xf0",
+ .expectedlen = 16
+ }
+};
+
+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);
+ else
+ pr_info("alg: self-tests for CTR-KDF (hmac(sha256)) passed\n");
+
+ return ret;
+}
+
+static void __exit crypto_kdf108_exit(void) { }
+
+module_init(crypto_kdf108_init);
+module_exit(crypto_kdf108_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Stephan Mueller <[email protected]>");
+MODULE_DESCRIPTION("Key Derivation Function conformant to SP800-108");
diff --git a/include/crypto/kdf_sp800108.h b/include/crypto/kdf_sp800108.h
new file mode 100644
index 000000000000..b7b20a778fb7
--- /dev/null
+++ b/include/crypto/kdf_sp800108.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (C) 2021, Stephan Mueller <[email protected]>
+ */
+
+#ifndef _CRYPTO_KDF108_H
+#define _CRYPTO_KDF108_H
+
+#include <crypto/hash.h>
+#include <linux/uio.h>
+
+/**
+ * Counter KDF generate operation according to SP800-108 section 5.1
+ * as well as SP800-56A section 5.8.1 (Single-step KDF).
+ *
+ * @kmd Keyed message digest whose key was set with crypto_kdf108_setkey or
+ * unkeyed message digest
+ * @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.
+ *
+ * To comply with SP800-108, the caller must provide Label || 0x00 || Context
+ * in the info parameter.
+ *
+ * @return 0 on success, < 0 on error
+ */
+int crypto_kdf108_ctr_generate(struct crypto_shash *kmd,
+ const struct kvec *info, unsigned int info_nvec,
+ u8 *dst, unsigned int dlen);
+
+/**
+ * Counter KDF setkey operation
+ *
+ * @kmd Keyed message digest allocated by the caller. The key should not have
+ * been set.
+ * @key Seed key to be used to initialize the keyed message digest context.
+ * @keylen This length of the key buffer.
+ * @ikm The SP800-108 KDF does not support IKM - this parameter must be NULL
+ * @ikmlen This parameter must be 0.
+ *
+ * According to SP800-108 section 7.2, the seed key must be at least as large as
+ * the message digest size of the used keyed message digest. This limitation
+ * is enforced by the implementation.
+ *
+ * SP800-108 allows the use of either a HMAC or a hash primitive. When
+ * the caller intends to use a hash primitive, the call to
+ * crypto_kdf108_setkey is not required and the key derivation operation can
+ * immediately performed using crypto_kdf108_ctr_generate after allocating
+ * a handle.
+ *
+ * @return 0 on success, < 0 on error
+ */
+int crypto_kdf108_setkey(struct crypto_shash *kmd,
+ const u8 *key, size_t keylen,
+ const u8 *ikm, size_t ikmlen);
+
+#endif /* _CRYPTO_KDF108_H */
--
2.33.1
Remove the specific code that adds a zero padding that was intended
to be invoked when the DH operation result was smaller than the
modulus. However, this cannot occur any more these days because the
function mpi_write_to_sgl is used in the code path that calculates the
shared secret in dh_compute_value. This MPI service function guarantees
that leading zeros are introduced as needed to ensure the resulting data
is exactly as long as the modulus. This implies that the specific code
to add zero padding is dead code which can be safely removed.
Signed-off-by: Stephan Mueller <[email protected]>
---
security/keys/dh.c | 25 ++++---------------------
1 file changed, 4 insertions(+), 21 deletions(-)
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 1abfa70ed6e1..56e12dae4534 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -141,7 +141,7 @@ static void kdf_dealloc(struct kdf_sdesc *sdesc)
* 'dlen' must be a multiple of the digest size.
*/
static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
- u8 *dst, unsigned int dlen, unsigned int zlen)
+ u8 *dst, unsigned int dlen)
{
struct shash_desc *desc = &sdesc->shash;
unsigned int h = crypto_shash_digestsize(desc->tfm);
@@ -158,22 +158,6 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
if (err)
goto err;
- if (zlen && h) {
- u8 tmpbuffer[32];
- size_t chunk = min_t(size_t, zlen, sizeof(tmpbuffer));
- memset(tmpbuffer, 0, chunk);
-
- do {
- err = crypto_shash_update(desc, tmpbuffer,
- chunk);
- if (err)
- goto err;
-
- zlen -= chunk;
- chunk = min_t(size_t, zlen, sizeof(tmpbuffer));
- } while (zlen);
- }
-
if (src && slen) {
err = crypto_shash_update(desc, src, slen);
if (err)
@@ -198,7 +182,7 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
char __user *buffer, size_t buflen,
- uint8_t *kbuf, size_t kbuflen, size_t lzero)
+ uint8_t *kbuf, size_t kbuflen)
{
uint8_t *outbuf = NULL;
int ret;
@@ -211,7 +195,7 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
goto err;
}
- ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len, lzero);
+ ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len);
if (ret)
goto err;
@@ -384,8 +368,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
}
ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, outbuf,
- req->dst_len + kdfcopy->otherinfolen,
- outlen - req->dst_len);
+ req->dst_len + kdfcopy->otherinfolen);
} else if (copy_to_user(buffer, outbuf, req->dst_len) == 0) {
ret = req->dst_len;
} else {
--
2.33.1
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 <[email protected]>
> ---
> 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
On Mon, 15 Nov 2021, Stephan M?ller wrote:
> Remove the specific code that adds a zero padding that was intended
> to be invoked when the DH operation result was smaller than the
> modulus. However, this cannot occur any more these days because the
> function mpi_write_to_sgl is used in the code path that calculates the
> shared secret in dh_compute_value. This MPI service function guarantees
> that leading zeros are introduced as needed to ensure the resulting data
> is exactly as long as the modulus. This implies that the specific code
> to add zero padding is dead code which can be safely removed.
>
> Signed-off-by: Stephan Mueller <[email protected]>
> ---
> security/keys/dh.c | 25 ++++---------------------
> 1 file changed, 4 insertions(+), 21 deletions(-)
Hi Stephan -
Thanks for the cleanup!
Acked-by: Mat Martineau <[email protected]>
>
> diff --git a/security/keys/dh.c b/security/keys/dh.c
> index 1abfa70ed6e1..56e12dae4534 100644
> --- a/security/keys/dh.c
> +++ b/security/keys/dh.c
> @@ -141,7 +141,7 @@ static void kdf_dealloc(struct kdf_sdesc *sdesc)
> * 'dlen' must be a multiple of the digest size.
> */
> static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
> - u8 *dst, unsigned int dlen, unsigned int zlen)
> + u8 *dst, unsigned int dlen)
> {
> struct shash_desc *desc = &sdesc->shash;
> unsigned int h = crypto_shash_digestsize(desc->tfm);
> @@ -158,22 +158,6 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
> if (err)
> goto err;
>
> - if (zlen && h) {
> - u8 tmpbuffer[32];
> - size_t chunk = min_t(size_t, zlen, sizeof(tmpbuffer));
> - memset(tmpbuffer, 0, chunk);
> -
> - do {
> - err = crypto_shash_update(desc, tmpbuffer,
> - chunk);
> - if (err)
> - goto err;
> -
> - zlen -= chunk;
> - chunk = min_t(size_t, zlen, sizeof(tmpbuffer));
> - } while (zlen);
> - }
> -
> if (src && slen) {
> err = crypto_shash_update(desc, src, slen);
> if (err)
> @@ -198,7 +182,7 @@ static int kdf_ctr(struct kdf_sdesc *sdesc, const u8 *src, unsigned int slen,
>
> static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
> char __user *buffer, size_t buflen,
> - uint8_t *kbuf, size_t kbuflen, size_t lzero)
> + uint8_t *kbuf, size_t kbuflen)
> {
> uint8_t *outbuf = NULL;
> int ret;
> @@ -211,7 +195,7 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
> goto err;
> }
>
> - ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len, lzero);
> + ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len);
> if (ret)
> goto err;
>
> @@ -384,8 +368,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
> }
>
> ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, outbuf,
> - req->dst_len + kdfcopy->otherinfolen,
> - outlen - req->dst_len);
> + req->dst_len + kdfcopy->otherinfolen);
> } else if (copy_to_user(buffer, outbuf, req->dst_len) == 0) {
> ret = req->dst_len;
> } else {
> --
> 2.33.1
>
>
>
>
>
--
Mat Martineau
Intel
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 <[email protected]>
> > ---
> >
> > 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
Am Mittwoch, 17. November 2021, 22:28:46 CET schrieb Mat Martineau:
Hi Mat,
> On Mon, 15 Nov 2021, Stephan M?ller wrote:
> > Remove the specific code that adds a zero padding that was intended
> > to be invoked when the DH operation result was smaller than the
> > modulus. However, this cannot occur any more these days because the
> > function mpi_write_to_sgl is used in the code path that calculates the
> > shared secret in dh_compute_value. This MPI service function guarantees
> > that leading zeros are introduced as needed to ensure the resulting data
> > is exactly as long as the modulus. This implies that the specific code
> > to add zero padding is dead code which can be safely removed.
> >
> > Signed-off-by: Stephan Mueller <[email protected]>
> > ---
> > security/keys/dh.c | 25 ++++---------------------
> > 1 file changed, 4 insertions(+), 21 deletions(-)
>
> Hi Stephan -
>
> Thanks for the cleanup!
Thank you for the review.
>
> Acked-by: Mat Martineau <[email protected]>
I have added your signature to the patch.
Ciao
Stephan