linux-fscrypt.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] Add KDF implementations to crypto API
@ 2021-01-24 14:01 Stephan Müller
  2021-01-24 14:01 ` [PATCH v2 1/7] crypto: Add key derivation self-test support code Stephan Müller
                   ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: Stephan Müller @ 2021-01-24 14:01 UTC (permalink / raw)
  To: herbert
  Cc: ebiggers, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

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 KDF implementations into the
crypto API.

The KDF implementations are provided as service functions. Yet, the
interface to the two provided KDFs are identical with the goal to allow
them to be transformed into a crypto API template eventually.

The KDFs execute 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.

The HKDF addition is used to replace the implementation in the filesystem
crypto extension. This code was tested by using an EXT4 encrypted file
system that was created and contains files written to by the current
implementation. Using the new implementation a successful read of the
existing files was possible and new files / directories were created
and read successfully. These newly added file system objects could be
successfully read using the current code. Yet if there is a test suite
to validate whether the invokcation of the HKDF calculates the same
result as the existing implementation, I would be happy to validate
the implementation accordingly.

Changes v2:

* change HKDF function names
* change HKDF/SP800-108 KDF extract / seed function prototype
* ensure clearing of memory of destination buffer in KDF implementation
  if KDF operation fails
* security DH: split the removal of dead code into separate patch

Stephan Mueller (7):
  crypto: Add key derivation self-test support code
  crypto: add SP800-108 counter key derivation function
  crypto: add RFC5869 HKDF
  security: DH - remove dead code for zero padding
  security: DH - use KDF implementation from crypto API
  fs: use HKDF implementation from kernel crypto API
  fs: HKDF - remove duplicate memory clearing

 crypto/Kconfig                         |  14 ++
 crypto/Makefile                        |   6 +
 crypto/hkdf.c                          | 199 +++++++++++++++++++++++++
 crypto/kdf_sp800108.c                  | 149 ++++++++++++++++++
 fs/crypto/Kconfig                      |   2 +-
 fs/crypto/hkdf.c                       | 103 +++----------
 include/crypto/hkdf.h                  |  48 ++++++
 include/crypto/internal/kdf_selftest.h |  71 +++++++++
 include/crypto/kdf_sp800108.h          |  61 ++++++++
 security/keys/Kconfig                  |   2 +-
 security/keys/dh.c                     | 118 ++-------------
 11 files changed, 586 insertions(+), 187 deletions(-)
 create mode 100644 crypto/hkdf.c
 create mode 100644 crypto/kdf_sp800108.c
 create mode 100644 include/crypto/hkdf.h
 create mode 100644 include/crypto/internal/kdf_selftest.h
 create mode 100644 include/crypto/kdf_sp800108.h

-- 
2.26.2





^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/7] crypto: Add key derivation self-test support code
  2021-01-24 14:01 [PATCH v2 0/7] Add KDF implementations to crypto API Stephan Müller
@ 2021-01-24 14:01 ` Stephan Müller
  2021-01-24 14:02 ` [PATCH v2 2/7] crypto: add SP800-108 counter key derivation function Stephan Müller
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Stephan Müller @ 2021-01-24 14:01 UTC (permalink / raw)
  To: herbert
  Cc: ebiggers, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

As a preparation to add the key derivation implementations, the
self-test data structure definition and the common test code is made
available.

The test framework follows the testing applied by the NIST CAVP test
approach.

The structure of the test code follows the implementations found in
crypto/testmgr.c|h. In case the KDF implementations will be made
available via a kernel crypto API templates, the test code is intended
to be merged into testmgr.c|h.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 include/crypto/internal/kdf_selftest.h | 71 ++++++++++++++++++++++++++
 1 file changed, 71 insertions(+)
 create mode 100644 include/crypto/internal/kdf_selftest.h

diff --git a/include/crypto/internal/kdf_selftest.h b/include/crypto/internal/kdf_selftest.h
new file mode 100644
index 000000000000..373fcb11f2fa
--- /dev/null
+++ b/include/crypto/internal/kdf_selftest.h
@@ -0,0 +1,71 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (C) 2020, Stephan Mueller <smueller@chronox.de>
+ */
+
+#ifndef _CRYPTO_KDF_SELFTEST_H
+#define _CRYPTO_KDF_SELFTEST_H
+
+#include <crypto/hash.h>
+#include <linux/uio.h>
+
+struct kdf_testvec {
+	unsigned char *key;
+	size_t keylen;
+	unsigned char *ikm;
+	size_t ikmlen;
+	struct kvec info;
+	unsigned char *expected;
+	size_t expectedlen;
+};
+
+static inline int
+kdf_test(const struct kdf_testvec *test, const char *name,
+	 int (*crypto_kdf_setkey)(struct crypto_shash *kmd,
+				  const u8 *key, size_t keylen,
+				  const u8 *ikm, size_t ikmlen),
+	 int (*crypto_kdf_generate)(struct crypto_shash *kmd,
+				    const struct kvec *info,
+				    unsigned int info_nvec,
+				    u8 *dst, unsigned int dlen))
+{
+	struct crypto_shash *kmd;
+	int ret;
+	u8 *buf = kzalloc(test->expectedlen, GFP_KERNEL);
+
+	if (!buf)
+		return -ENOMEM;
+
+	kmd = crypto_alloc_shash(name, 0, 0);
+	if (IS_ERR(kmd)) {
+		pr_err("alg: kdf: could not allocate hash handle for %s\n",
+		       name);
+		kfree(buf);
+		return -ENOMEM;
+	}
+
+	ret = crypto_kdf_setkey(kmd, test->key, test->keylen,
+				test->ikm, test->ikmlen);
+	if (ret) {
+		pr_err("alg: kdf: could not set key derivation key\n");
+		goto err;
+	}
+
+	ret = crypto_kdf_generate(kmd, &test->info, 1, buf, test->expectedlen);
+	if (ret) {
+		pr_err("alg: kdf: could not obtain key data\n");
+		goto err;
+	}
+
+	ret = memcmp(test->expected, buf, test->expectedlen);
+	if (ret)
+		ret = -EINVAL;
+
+err:
+	crypto_free_shash(kmd);
+	kfree(buf);
+	return ret;
+}
+
+#endif /* _CRYPTO_KDF_SELFTEST_H */
-- 
2.26.2





^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/7] crypto: add SP800-108 counter key derivation function
  2021-01-24 14:01 [PATCH v2 0/7] Add KDF implementations to crypto API Stephan Müller
  2021-01-24 14:01 ` [PATCH v2 1/7] crypto: Add key derivation self-test support code Stephan Müller
@ 2021-01-24 14:02 ` Stephan Müller
  2021-01-24 14:03 ` [PATCH v2 3/7] crypto: add RFC5869 HKDF Stephan Müller
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Stephan Müller @ 2021-01-24 14:02 UTC (permalink / raw)
  To: herbert
  Cc: ebiggers, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

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@chronox.de>
---
 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 a367fcfeb5d4..9f375c2350f5 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_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 b279483fba50..46845a70850d 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -197,3 +197,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..84b45e0cadf5
--- /dev/null
+++ b/crypto/kdf_sp800108.c
@@ -0,0 +1,149 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * SP800-108 Key-derivation function
+ *
+ * Copyright (C) 2020, Stephan Mueller <smueller@chronox.de>
+ */
+
+#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 <smueller@chronox.de>");
+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..fdc360bd9cd6
--- /dev/null
+++ b/include/crypto/kdf_sp800108.h
@@ -0,0 +1,61 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+/*
+ * Copyright (C) 2020, Stephan Mueller <smueller@chronox.de>
+ */
+
+#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.26.2





^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/7] crypto: add RFC5869 HKDF
  2021-01-24 14:01 [PATCH v2 0/7] Add KDF implementations to crypto API Stephan Müller
  2021-01-24 14:01 ` [PATCH v2 1/7] crypto: Add key derivation self-test support code Stephan Müller
  2021-01-24 14:02 ` [PATCH v2 2/7] crypto: add SP800-108 counter key derivation function Stephan Müller
@ 2021-01-24 14:03 ` Stephan Müller
  2021-01-28 20:08   ` Eric Biggers
  2021-01-24 14:03 ` [PATCH v2 4/7] security: DH - remove dead code for zero padding Stephan Müller
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 16+ messages in thread
From: Stephan Müller @ 2021-01-24 14:03 UTC (permalink / raw)
  To: herbert
  Cc: ebiggers, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

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

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/Kconfig        |   7 ++
 crypto/Makefile       |   1 +
 crypto/hkdf.c         | 199 ++++++++++++++++++++++++++++++++++++++++++
 include/crypto/hkdf.h |  48 ++++++++++
 4 files changed, 255 insertions(+)
 create mode 100644 crypto/hkdf.c
 create mode 100644 include/crypto/hkdf.h

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.
+
 config CRYPTO_KDF800108_CTR
 	tristate "Counter KDF (SP800-108)"
 	select CRYPTO_HASH
diff --git a/crypto/Makefile b/crypto/Makefile
index 46845a70850d..55a4d8c31a45 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -201,4 +201,5 @@ obj-$(CONFIG_CRYPTO_SIMD) += crypto_simd.o
 #
 # Key derivation function
 #
+obj-$(CONFIG_CRYPTO_HKDF) += hkdf.o
 obj-$(CONFIG_CRYPTO_KDF800108_CTR) += kdf_sp800108.o
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 <smueller@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);
+
+/*
+ * 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) {
+		/* Set the salt as HMAC key */
+		err = crypto_shash_setkey(kmd, salt, saltlen);
+	} else {
+		/* Set the default HMAC key as none was provided */
+		err = crypto_shash_setkey(kmd, null_salt, h);
+	}
+
+	if (err)
+		return err;
+
+	/* Extract the PRK */
+	err = crypto_shash_init(desc);
+	if (err)
+		goto err;
+
+	err = crypto_shash_finup(desc, ikm, ikmlen, prk);
+	if (err)
+		goto err;
+
+	/* Set the PRK for the expand phase */
+	err = crypto_shash_setkey(kmd, prk, h);
+
+err:
+	shash_desc_zero(desc);
+	memzero_explicit(prk, h);
+	return err;
+}
+EXPORT_SYMBOL(crypto_hkdf_extract);
+
+/* 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
+	}
+};
+
+static int __init crypto_hkdf_init(void)
+{
+	unsigned int i;
+	int ret;
+
+	for (i = 0; i < ARRAY_SIZE(hkdf_hmac_sha256_tv_template); i++) {
+		ret = kdf_test(&hkdf_hmac_sha256_tv_template[i], "hmac(sha256)",
+			       crypto_hkdf_extract, crypto_hkdf_expand);
+
+		if (ret) {
+			pr_warn("alg: self-tests for HKDF (hmac(sha256)) failed (rc=%d)\n",
+				ret);
+			return ret;
+		}
+	}
+
+	pr_info("alg: self-tests for HKDF (hmac(sha256)) passed\n");
+
+	return 0;
+}
+
+static void __exit crypto_hkdf_exit(void) { }
+
+module_init(crypto_hkdf_init);
+module_exit(crypto_hkdf_exit);
+
+MODULE_LICENSE("GPL v2");
+MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
+MODULE_DESCRIPTION("HKDF HMAC-based Extract-and-Expand Key Derivation Function  (conformant to RFC5869)");
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 <smueller@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)
+ *
+ * @return 0 on success, < 0 on error
+ */
+int crypto_hkdf_extract(struct crypto_shash *kmd,
+			const u8 *salt, size_t saltlen,
+			const u8 *ikm, size_t ikmlen);
+
+#endif /* _CRYPTO_HKDF_H */
-- 
2.26.2





^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/7] security: DH - remove dead code for zero padding
  2021-01-24 14:01 [PATCH v2 0/7] Add KDF implementations to crypto API Stephan Müller
                   ` (2 preceding siblings ...)
  2021-01-24 14:03 ` [PATCH v2 3/7] crypto: add RFC5869 HKDF Stephan Müller
@ 2021-01-24 14:03 ` Stephan Müller
  2021-01-24 14:04 ` [PATCH v2 5/7] security: DH - use KDF implementation from crypto API Stephan Müller
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Stephan Müller @ 2021-01-24 14:03 UTC (permalink / raw)
  To: herbert
  Cc: ebiggers, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

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 <smueller@chronox.de>
---
 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.26.2





^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/7] security: DH - use KDF implementation from crypto API
  2021-01-24 14:01 [PATCH v2 0/7] Add KDF implementations to crypto API Stephan Müller
                   ` (3 preceding siblings ...)
  2021-01-24 14:03 ` [PATCH v2 4/7] security: DH - remove dead code for zero padding Stephan Müller
@ 2021-01-24 14:04 ` Stephan Müller
  2021-01-24 14:04 ` [PATCH v2 6/7] fs: use HKDF implementation from kernel " Stephan Müller
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: Stephan Müller @ 2021-01-24 14:04 UTC (permalink / raw)
  To: herbert
  Cc: ebiggers, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

The kernel crypto API provides the SP800-108 counter KDF implementation.
Thus, the separate implementation provided as part of the keys subsystem
can be replaced with calls to the KDF offered by the kernel crypto API.

The keys subsystem uses the counter KDF with a hash primitive. Thus,
it only uses the call to crypto_kdf108_ctr_generate.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 security/keys/Kconfig |  2 +-
 security/keys/dh.c    | 97 +++++++------------------------------------
 2 files changed, 15 insertions(+), 84 deletions(-)

diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index 83bc23409164..e6604499f0a8 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -106,7 +106,7 @@ config KEY_DH_OPERATIONS
        bool "Diffie-Hellman operations on retained keys"
        depends on KEYS
        select CRYPTO
-       select CRYPTO_HASH
+       select CRYPTO_KDF800108_CTR
        select CRYPTO_DH
        help
 	 This option provides support for calculating Diffie-Hellman
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 56e12dae4534..46fa442b81ec 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -11,6 +11,7 @@
 #include <crypto/hash.h>
 #include <crypto/kpp.h>
 #include <crypto/dh.h>
+#include <crypto/kdf_sp800108.h>
 #include <keys/user-type.h>
 #include "internal.h"
 
@@ -79,16 +80,9 @@ static void dh_crypto_done(struct crypto_async_request *req, int err)
 	complete(&compl->completion);
 }
 
-struct kdf_sdesc {
-	struct shash_desc shash;
-	char ctx[];
-};
-
-static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname)
+static int kdf_alloc(struct crypto_shash **hash, char *hashname)
 {
 	struct crypto_shash *tfm;
-	struct kdf_sdesc *sdesc;
-	int size;
 	int err;
 
 	/* allocate synchronous hash */
@@ -102,14 +96,7 @@ static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname)
 	if (crypto_shash_digestsize(tfm) == 0)
 		goto out_free_tfm;
 
-	err = -ENOMEM;
-	size = sizeof(struct shash_desc) + crypto_shash_descsize(tfm);
-	sdesc = kmalloc(size, GFP_KERNEL);
-	if (!sdesc)
-		goto out_free_tfm;
-	sdesc->shash.tfm = tfm;
-
-	*sdesc_ret = sdesc;
+	*hash = tfm;
 
 	return 0;
 
@@ -118,76 +105,20 @@ static int kdf_alloc(struct kdf_sdesc **sdesc_ret, char *hashname)
 	return err;
 }
 
-static void kdf_dealloc(struct kdf_sdesc *sdesc)
-{
-	if (!sdesc)
-		return;
-
-	if (sdesc->shash.tfm)
-		crypto_free_shash(sdesc->shash.tfm);
-
-	kfree_sensitive(sdesc);
-}
-
-/*
- * Implementation of the KDF in counter mode according to SP800-108 section 5.1
- * as well as SP800-56A section 5.8.1 (Single-step KDF).
- *
- * SP800-56A:
- * The src pointer is defined as Z || other info where Z is the shared secret
- * from DH and other info is an arbitrary string (see SP800-56A section
- * 5.8.1.2).
- *
- * '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)
+static void kdf_dealloc(struct crypto_shash *hash)
 {
-	struct shash_desc *desc = &sdesc->shash;
-	unsigned int h = crypto_shash_digestsize(desc->tfm);
-	int err = 0;
-	u8 *dst_orig = dst;
-	__be32 counter = cpu_to_be32(1);
-
-	while (dlen) {
-		err = crypto_shash_init(desc);
-		if (err)
-			goto err;
-
-		err = crypto_shash_update(desc, (u8 *)&counter, sizeof(__be32));
-		if (err)
-			goto err;
-
-		if (src && slen) {
-			err = crypto_shash_update(desc, src, slen);
-			if (err)
-				goto err;
-		}
-
-		err = crypto_shash_final(desc, dst);
-		if (err)
-			goto err;
-
-		dlen -= h;
-		dst += h;
-		counter = cpu_to_be32(be32_to_cpu(counter) + 1);
-	}
-
-	return 0;
-
-err:
-	memzero_explicit(dst_orig, dlen);
-	return err;
+	if (hash)
+		crypto_free_shash(hash);
 }
 
-static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
+static int keyctl_dh_compute_kdf(struct crypto_shash *hash,
 				 char __user *buffer, size_t buflen,
 				 uint8_t *kbuf, size_t kbuflen)
 {
+	struct kvec kbuf_iov = { .iov_base = kbuf, .iov_len = kbuflen };
 	uint8_t *outbuf = NULL;
 	int ret;
-	size_t outbuf_len = roundup(buflen,
-				    crypto_shash_digestsize(sdesc->shash.tfm));
+	size_t outbuf_len = roundup(buflen, crypto_shash_digestsize(hash));
 
 	outbuf = kmalloc(outbuf_len, GFP_KERNEL);
 	if (!outbuf) {
@@ -195,7 +126,7 @@ static int keyctl_dh_compute_kdf(struct kdf_sdesc *sdesc,
 		goto err;
 	}
 
-	ret = kdf_ctr(sdesc, kbuf, kbuflen, outbuf, outbuf_len);
+	ret = crypto_kdf108_ctr_generate(hash, &kbuf_iov, 1, outbuf, outbuf_len);
 	if (ret)
 		goto err;
 
@@ -224,7 +155,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
 	struct kpp_request *req;
 	uint8_t *secret;
 	uint8_t *outbuf;
-	struct kdf_sdesc *sdesc = NULL;
+	struct crypto_shash *hash = NULL;
 
 	if (!params || (!buffer && buflen)) {
 		ret = -EINVAL;
@@ -257,7 +188,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
 		}
 
 		/* allocate KDF from the kernel crypto API */
-		ret = kdf_alloc(&sdesc, hashname);
+		ret = kdf_alloc(&hash, hashname);
 		kfree(hashname);
 		if (ret)
 			goto out1;
@@ -367,7 +298,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
 			goto out6;
 		}
 
-		ret = keyctl_dh_compute_kdf(sdesc, buffer, buflen, outbuf,
+		ret = keyctl_dh_compute_kdf(hash, buffer, buflen, outbuf,
 					    req->dst_len + kdfcopy->otherinfolen);
 	} else if (copy_to_user(buffer, outbuf, req->dst_len) == 0) {
 		ret = req->dst_len;
@@ -386,7 +317,7 @@ long __keyctl_dh_compute(struct keyctl_dh_params __user *params,
 out2:
 	dh_free_data(&dh_inputs);
 out1:
-	kdf_dealloc(sdesc);
+	kdf_dealloc(hash);
 	return ret;
 }
 
-- 
2.26.2





^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/7] fs: use HKDF implementation from kernel crypto API
  2021-01-24 14:01 [PATCH v2 0/7] Add KDF implementations to crypto API Stephan Müller
                   ` (4 preceding siblings ...)
  2021-01-24 14:04 ` [PATCH v2 5/7] security: DH - use KDF implementation from crypto API Stephan Müller
@ 2021-01-24 14:04 ` Stephan Müller
  2021-01-28 20:16   ` Eric Biggers
  2021-01-28 20:18   ` Eric Biggers
  2021-01-24 14:04 ` [PATCH v2 7/7] fs: HKDF - remove duplicate memory clearing Stephan Müller
  2021-01-24 14:23 ` [PATCH v2 0/7] Add KDF implementations to crypto API Ard Biesheuvel
  7 siblings, 2 replies; 16+ messages in thread
From: Stephan Müller @ 2021-01-24 14:04 UTC (permalink / raw)
  To: herbert
  Cc: ebiggers, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

As the kernel crypto API implements HKDF, replace the
file-system-specific HKDF implementation with the generic HKDF
implementation.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 fs/crypto/Kconfig |  2 +-
 fs/crypto/hkdf.c  | 98 +++++++++--------------------------------------
 2 files changed, 20 insertions(+), 80 deletions(-)

diff --git a/fs/crypto/Kconfig b/fs/crypto/Kconfig
index a5f5c30368a2..9450e958f1d1 100644
--- a/fs/crypto/Kconfig
+++ b/fs/crypto/Kconfig
@@ -2,7 +2,7 @@
 config FS_ENCRYPTION
 	bool "FS Encryption (Per-file encryption)"
 	select CRYPTO
-	select CRYPTO_HASH
+	select CRYPTO_HKDF
 	select CRYPTO_SKCIPHER
 	select CRYPTO_LIB_SHA256
 	select KEYS
diff --git a/fs/crypto/hkdf.c b/fs/crypto/hkdf.c
index e0ec21055505..ae236b42b1f0 100644
--- a/fs/crypto/hkdf.c
+++ b/fs/crypto/hkdf.c
@@ -9,7 +9,7 @@
  * Copyright 2019 Google LLC
  */
 
-#include <crypto/hash.h>
+#include <crypto/hkdf.h>
 #include <crypto/sha2.h>
 
 #include "fscrypt_private.h"
@@ -37,23 +37,7 @@
  * unnecessarily long master keys.  Thus fscrypt still does HKDF-Extract.  No
  * salt is used, since fscrypt master keys should already be pseudorandom and
  * there's no way to persist a random salt per master key from kernel mode.
- */
-
-/* HKDF-Extract (RFC 5869 section 2.2), unsalted */
-static int hkdf_extract(struct crypto_shash *hmac_tfm, const u8 *ikm,
-			unsigned int ikmlen, u8 prk[HKDF_HASHLEN])
-{
-	static const u8 default_salt[HKDF_HASHLEN];
-	int err;
-
-	err = crypto_shash_setkey(hmac_tfm, default_salt, HKDF_HASHLEN);
-	if (err)
-		return err;
-
-	return crypto_shash_tfm_digest(hmac_tfm, ikm, ikmlen, prk);
-}
-
-/*
+ *
  * Compute HKDF-Extract using the given master key as the input keying material,
  * and prepare an HMAC transform object keyed by the resulting pseudorandom key.
  *
@@ -64,7 +48,6 @@ int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
 		      unsigned int master_key_size)
 {
 	struct crypto_shash *hmac_tfm;
-	u8 prk[HKDF_HASHLEN];
 	int err;
 
 	hmac_tfm = crypto_alloc_shash(HKDF_HMAC_ALG, 0, 0);
@@ -74,16 +57,14 @@ int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
 		return PTR_ERR(hmac_tfm);
 	}
 
-	if (WARN_ON(crypto_shash_digestsize(hmac_tfm) != sizeof(prk))) {
+	if (WARN_ON(crypto_shash_digestsize(hmac_tfm) != HKDF_HASHLEN)) {
 		err = -EINVAL;
 		goto err_free_tfm;
 	}
 
-	err = hkdf_extract(hmac_tfm, master_key, master_key_size, prk);
-	if (err)
-		goto err_free_tfm;
-
-	err = crypto_shash_setkey(hmac_tfm, prk, sizeof(prk));
+	/* HKDF-Extract (RFC 5869 section 2.2), unsalted */
+	err = crypto_hkdf_extract(hmac_tfm, NULL, 0,
+				  master_key, master_key_size);
 	if (err)
 		goto err_free_tfm;
 
@@ -93,7 +74,6 @@ int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
 err_free_tfm:
 	crypto_free_shash(hmac_tfm);
 out:
-	memzero_explicit(prk, sizeof(prk));
 	return err;
 }
 
@@ -112,62 +92,22 @@ int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context,
 			const u8 *info, unsigned int infolen,
 			u8 *okm, unsigned int okmlen)
 {
-	SHASH_DESC_ON_STACK(desc, hkdf->hmac_tfm);
-	u8 prefix[9];
-	unsigned int i;
-	int err;
-	const u8 *prev = NULL;
-	u8 counter = 1;
-	u8 tmp[HKDF_HASHLEN];
-
-	if (WARN_ON(okmlen > 255 * HKDF_HASHLEN))
-		return -EINVAL;
-
-	desc->tfm = hkdf->hmac_tfm;
-
-	memcpy(prefix, "fscrypt\0", 8);
-	prefix[8] = context;
-
-	for (i = 0; i < okmlen; i += HKDF_HASHLEN) {
+	const struct kvec info_iov[] = { {
+		.iov_base = "fscrypt\0",
+		.iov_len = 8,
+	}, {
+		.iov_base = &context,
+		.iov_len = 1,
+	}, {
+		.iov_base = (u8 *)info,
+		.iov_len = infolen,
+	} };
+	int err = crypto_hkdf_expand(hkdf->hmac_tfm,
+				     info_iov, ARRAY_SIZE(info_iov),
+				     okm, okmlen);
 
-		err = crypto_shash_init(desc);
-		if (err)
-			goto out;
-
-		if (prev) {
-			err = crypto_shash_update(desc, prev, HKDF_HASHLEN);
-			if (err)
-				goto out;
-		}
-
-		err = crypto_shash_update(desc, prefix, sizeof(prefix));
-		if (err)
-			goto out;
-
-		err = crypto_shash_update(desc, info, infolen);
-		if (err)
-			goto out;
-
-		BUILD_BUG_ON(sizeof(counter) != 1);
-		if (okmlen - i < HKDF_HASHLEN) {
-			err = crypto_shash_finup(desc, &counter, 1, tmp);
-			if (err)
-				goto out;
-			memcpy(&okm[i], tmp, okmlen - i);
-			memzero_explicit(tmp, sizeof(tmp));
-		} else {
-			err = crypto_shash_finup(desc, &counter, 1, &okm[i]);
-			if (err)
-				goto out;
-		}
-		counter++;
-		prev = &okm[i];
-	}
-	err = 0;
-out:
 	if (unlikely(err))
 		memzero_explicit(okm, okmlen); /* so caller doesn't need to */
-	shash_desc_zero(desc);
 	return err;
 }
 
-- 
2.26.2





^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 7/7] fs: HKDF - remove duplicate memory clearing
  2021-01-24 14:01 [PATCH v2 0/7] Add KDF implementations to crypto API Stephan Müller
                   ` (5 preceding siblings ...)
  2021-01-24 14:04 ` [PATCH v2 6/7] fs: use HKDF implementation from kernel " Stephan Müller
@ 2021-01-24 14:04 ` Stephan Müller
  2021-01-28 20:21   ` Eric Biggers
  2021-01-24 14:23 ` [PATCH v2 0/7] Add KDF implementations to crypto API Ard Biesheuvel
  7 siblings, 1 reply; 16+ messages in thread
From: Stephan Müller @ 2021-01-24 14:04 UTC (permalink / raw)
  To: herbert
  Cc: ebiggers, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

The clearing of the OKM memory buffer in case of an error is already
performed by the HKDF implementation crypto_hkdf_expand. Thus, the
code clearing is not needed any more in the file system code base.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 fs/crypto/hkdf.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/fs/crypto/hkdf.c b/fs/crypto/hkdf.c
index ae236b42b1f0..c48dd8ca3a46 100644
--- a/fs/crypto/hkdf.c
+++ b/fs/crypto/hkdf.c
@@ -102,13 +102,10 @@ int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context,
 		.iov_base = (u8 *)info,
 		.iov_len = infolen,
 	} };
-	int err = crypto_hkdf_expand(hkdf->hmac_tfm,
-				     info_iov, ARRAY_SIZE(info_iov),
-				     okm, okmlen);
 
-	if (unlikely(err))
-		memzero_explicit(okm, okmlen); /* so caller doesn't need to */
-	return err;
+	return crypto_hkdf_expand(hkdf->hmac_tfm,
+				  info_iov, ARRAY_SIZE(info_iov),
+				  okm, okmlen);
 }
 
 void fscrypt_destroy_hkdf(struct fscrypt_hkdf *hkdf)
-- 
2.26.2





^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/7] Add KDF implementations to crypto API
  2021-01-24 14:01 [PATCH v2 0/7] Add KDF implementations to crypto API Stephan Müller
                   ` (6 preceding siblings ...)
  2021-01-24 14:04 ` [PATCH v2 7/7] fs: HKDF - remove duplicate memory clearing Stephan Müller
@ 2021-01-24 14:23 ` Ard Biesheuvel
  2021-01-24 14:32   ` Ard Biesheuvel
  2021-01-24 14:34   ` Stephan Müller
  7 siblings, 2 replies; 16+ messages in thread
From: Ard Biesheuvel @ 2021-01-24 14:23 UTC (permalink / raw)
  To: Stephan Müller
  Cc: Herbert Xu, Eric Biggers, Jarkko Sakkinen, Mat Martineau,
	David Howells, Linux Crypto Mailing List, linux-fscrypt,
	Linux Kernel Mailing List, keyrings, simo

On Sun, 24 Jan 2021 at 15:10, Stephan Müller <smueller@chronox.de> wrote:
>
> 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 KDF implementations into the
> crypto API.
>
> The KDF implementations are provided as service functions. Yet, the
> interface to the two provided KDFs are identical with the goal to allow
> them to be transformed into a crypto API template eventually.
>

Why? There are typically two reasons to use the crypto API abstractions:
- the algorithm is not known at compile time, so we need the runtime
dispatch that the crypto API implements,
- the algorithm may be implemented by a h/w accelerator which is
discovered at runtime via the driver stack

In other cases, a library API is much more suitable, even in the case
where we may provide arch-specific accelerated implementations of such
an algorithm.



> The KDFs execute 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.
>
> The HKDF addition is used to replace the implementation in the filesystem
> crypto extension. This code was tested by using an EXT4 encrypted file
> system that was created and contains files written to by the current
> implementation. Using the new implementation a successful read of the
> existing files was possible and new files / directories were created
> and read successfully. These newly added file system objects could be
> successfully read using the current code. Yet if there is a test suite
> to validate whether the invokcation of the HKDF calculates the same
> result as the existing implementation, I would be happy to validate
> the implementation accordingly.
>
> Changes v2:
>
> * change HKDF function names
> * change HKDF/SP800-108 KDF extract / seed function prototype
> * ensure clearing of memory of destination buffer in KDF implementation
>   if KDF operation fails
> * security DH: split the removal of dead code into separate patch
>
> Stephan Mueller (7):
>   crypto: Add key derivation self-test support code
>   crypto: add SP800-108 counter key derivation function
>   crypto: add RFC5869 HKDF
>   security: DH - remove dead code for zero padding
>   security: DH - use KDF implementation from crypto API
>   fs: use HKDF implementation from kernel crypto API
>   fs: HKDF - remove duplicate memory clearing
>
>  crypto/Kconfig                         |  14 ++
>  crypto/Makefile                        |   6 +
>  crypto/hkdf.c                          | 199 +++++++++++++++++++++++++
>  crypto/kdf_sp800108.c                  | 149 ++++++++++++++++++
>  fs/crypto/Kconfig                      |   2 +-
>  fs/crypto/hkdf.c                       | 103 +++----------
>  include/crypto/hkdf.h                  |  48 ++++++
>  include/crypto/internal/kdf_selftest.h |  71 +++++++++
>  include/crypto/kdf_sp800108.h          |  61 ++++++++
>  security/keys/Kconfig                  |   2 +-
>  security/keys/dh.c                     | 118 ++-------------
>  11 files changed, 586 insertions(+), 187 deletions(-)
>  create mode 100644 crypto/hkdf.c
>  create mode 100644 crypto/kdf_sp800108.c
>  create mode 100644 include/crypto/hkdf.h
>  create mode 100644 include/crypto/internal/kdf_selftest.h
>  create mode 100644 include/crypto/kdf_sp800108.h
>
> --
> 2.26.2
>
>
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/7] Add KDF implementations to crypto API
  2021-01-24 14:23 ` [PATCH v2 0/7] Add KDF implementations to crypto API Ard Biesheuvel
@ 2021-01-24 14:32   ` Ard Biesheuvel
  2021-01-24 14:36     ` Stephan Müller
  2021-01-24 14:34   ` Stephan Müller
  1 sibling, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2021-01-24 14:32 UTC (permalink / raw)
  To: Stephan Müller
  Cc: Herbert Xu, Eric Biggers, Jarkko Sakkinen, Mat Martineau,
	David Howells, Linux Crypto Mailing List, linux-fscrypt,
	Linux Kernel Mailing List, keyrings, simo

On Sun, 24 Jan 2021 at 15:23, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Sun, 24 Jan 2021 at 15:10, Stephan Müller <smueller@chronox.de> wrote:
> >
> > 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 KDF implementations into the
> > crypto API.
> >
> > The KDF implementations are provided as service functions. Yet, the
> > interface to the two provided KDFs are identical with the goal to allow
> > them to be transformed into a crypto API template eventually.
> >
>
> Why? There are typically two reasons to use the crypto API abstractions:
> - the algorithm is not known at compile time, so we need the runtime
> dispatch that the crypto API implements,
> - the algorithm may be implemented by a h/w accelerator which is
> discovered at runtime via the driver stack
>
> In other cases, a library API is much more suitable, even in the case
> where we may provide arch-specific accelerated implementations of such
> an algorithm.
>

Hmm, apologies if I got the wrong end of the stick here - this prose
and the naming of some of the crypto_hkdf_xxx routines and function
pointers in the test code made me think that this is more than it
actually is.

What we are talking about are basically library wrappers around shash
instances to perform HKDF, right?

>
>
> > The KDFs execute 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.
> >
> > The HKDF addition is used to replace the implementation in the filesystem
> > crypto extension. This code was tested by using an EXT4 encrypted file
> > system that was created and contains files written to by the current
> > implementation. Using the new implementation a successful read of the
> > existing files was possible and new files / directories were created
> > and read successfully. These newly added file system objects could be
> > successfully read using the current code. Yet if there is a test suite
> > to validate whether the invokcation of the HKDF calculates the same
> > result as the existing implementation, I would be happy to validate
> > the implementation accordingly.
> >
> > Changes v2:
> >
> > * change HKDF function names
> > * change HKDF/SP800-108 KDF extract / seed function prototype
> > * ensure clearing of memory of destination buffer in KDF implementation
> >   if KDF operation fails
> > * security DH: split the removal of dead code into separate patch
> >
> > Stephan Mueller (7):
> >   crypto: Add key derivation self-test support code
> >   crypto: add SP800-108 counter key derivation function
> >   crypto: add RFC5869 HKDF
> >   security: DH - remove dead code for zero padding
> >   security: DH - use KDF implementation from crypto API
> >   fs: use HKDF implementation from kernel crypto API
> >   fs: HKDF - remove duplicate memory clearing
> >
> >  crypto/Kconfig                         |  14 ++
> >  crypto/Makefile                        |   6 +
> >  crypto/hkdf.c                          | 199 +++++++++++++++++++++++++
> >  crypto/kdf_sp800108.c                  | 149 ++++++++++++++++++
> >  fs/crypto/Kconfig                      |   2 +-
> >  fs/crypto/hkdf.c                       | 103 +++----------
> >  include/crypto/hkdf.h                  |  48 ++++++
> >  include/crypto/internal/kdf_selftest.h |  71 +++++++++
> >  include/crypto/kdf_sp800108.h          |  61 ++++++++
> >  security/keys/Kconfig                  |   2 +-
> >  security/keys/dh.c                     | 118 ++-------------
> >  11 files changed, 586 insertions(+), 187 deletions(-)
> >  create mode 100644 crypto/hkdf.c
> >  create mode 100644 crypto/kdf_sp800108.c
> >  create mode 100644 include/crypto/hkdf.h
> >  create mode 100644 include/crypto/internal/kdf_selftest.h
> >  create mode 100644 include/crypto/kdf_sp800108.h
> >
> > --
> > 2.26.2
> >
> >
> >
> >

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/7] Add KDF implementations to crypto API
  2021-01-24 14:23 ` [PATCH v2 0/7] Add KDF implementations to crypto API Ard Biesheuvel
  2021-01-24 14:32   ` Ard Biesheuvel
@ 2021-01-24 14:34   ` Stephan Müller
  1 sibling, 0 replies; 16+ messages in thread
From: Stephan Müller @ 2021-01-24 14:34 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Herbert Xu, Eric Biggers, Jarkko Sakkinen, Mat Martineau,
	David Howells, Linux Crypto Mailing List, linux-fscrypt,
	Linux Kernel Mailing List, keyrings, simo

Am Sonntag, 24. Januar 2021, 15:23:29 CET schrieb Ard Biesheuvel:

Hi Ard,

> On Sun, 24 Jan 2021 at 15:10, Stephan Müller <smueller@chronox.de> wrote:
> > 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 KDF implementations into the
> > crypto API.
> > 
> > The KDF implementations are provided as service functions. Yet, the
> > interface to the two provided KDFs are identical with the goal to allow
> > them to be transformed into a crypto API template eventually.
> 
> Why? There are typically two reasons to use the crypto API abstractions:
> - the algorithm is not known at compile time, so we need the runtime
> dispatch that the crypto API implements,
> - the algorithm may be implemented by a h/w accelerator which is
> discovered at runtime via the driver stack
> 
> In other cases, a library API is much more suitable, even in the case
> where we may provide arch-specific accelerated implementations of such
> an algorithm.

In case your "why" refers to why I stated that the KDF implementations are 
similar to eventually consolidate them into a template eventually:

A KDF is conceptually a logic on top of a (hash) algorithm like a block 
chaining mode on top of a block cipher or a deterministic RNG on top of an 
underlying cipher.

So, conceptually with the kernel crypto API, we would have a KDF template that 
can be used like hkdf(sha256) or hkdf(sha256-avx2).

The behavior of a KDF is identical to a deterministic RNG. Thus, a long time 
ago, I had a patch developed that adds a very small addition to the existing 
RNG API to allow the KDFs to be used. See [1]. Yet, that was not desired at 
the time due to different reasons.

Yet, the crypto API as it stands today knows of templates and basic 
algorithms. Having a separate library API providing a crypto algorithm is new 
to the crypto API. You see that with the test manager which works well with 
the templates / algorithms but does not provide any helpers for some "library 
APIs".



In case your "why" refers to whether I am not using a template to begin with:

Some time back I provided the patch using a template (see [1] for example). At 
that time, Herbert wanted to have a service API instead.

[1] http://www.chronox.de/kdf.html

Ciao
Stephan



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 0/7] Add KDF implementations to crypto API
  2021-01-24 14:32   ` Ard Biesheuvel
@ 2021-01-24 14:36     ` Stephan Müller
  0 siblings, 0 replies; 16+ messages in thread
From: Stephan Müller @ 2021-01-24 14:36 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: Herbert Xu, Eric Biggers, Jarkko Sakkinen, Mat Martineau,
	David Howells, Linux Crypto Mailing List, linux-fscrypt,
	Linux Kernel Mailing List, keyrings, simo

Am Sonntag, 24. Januar 2021, 15:32:59 CET schrieb Ard Biesheuvel:

Hi Ard,

> On Sun, 24 Jan 2021 at 15:23, Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Sun, 24 Jan 2021 at 15:10, Stephan Müller <smueller@chronox.de> wrote:
> > > 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 KDF implementations into the
> > > crypto API.
> > > 
> > > The KDF implementations are provided as service functions. Yet, the
> > > interface to the two provided KDFs are identical with the goal to allow
> > > them to be transformed into a crypto API template eventually.
> > 
> > Why? There are typically two reasons to use the crypto API abstractions:
> > - the algorithm is not known at compile time, so we need the runtime
> > dispatch that the crypto API implements,
> > - the algorithm may be implemented by a h/w accelerator which is
> > discovered at runtime via the driver stack
> > 
> > In other cases, a library API is much more suitable, even in the case
> > where we may provide arch-specific accelerated implementations of such
> > an algorithm.
> 
> Hmm, apologies if I got the wrong end of the stick here - this prose
> and the naming of some of the crypto_hkdf_xxx routines and function
> pointers in the test code made me think that this is more than it
> actually is.
> 
> What we are talking about are basically library wrappers around shash
> instances to perform HKDF, right?

Sorry, our emails just crossed each other.

Yes, you are absolutely correct. The KDF implementations are wrappers around 
the SHASH API. Conceptually the provided API is what templates actually should 
do.

As mentioned in the other email, however, adding a template was and is not 
considered appropriate at the time. Yet, I would like to keep the path open to 
transform the KDF implementations into a template.

Ciao
Stephan



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/7] crypto: add RFC5869 HKDF
  2021-01-24 14:03 ` [PATCH v2 3/7] crypto: add RFC5869 HKDF Stephan Müller
@ 2021-01-28 20:08   ` Eric Biggers
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Biggers @ 2021-01-28 20:08 UTC (permalink / raw)
  To: Stephan Müller
  Cc: herbert, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

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 <smueller@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 <smueller@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

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 6/7] fs: use HKDF implementation from kernel crypto API
  2021-01-24 14:04 ` [PATCH v2 6/7] fs: use HKDF implementation from kernel " Stephan Müller
@ 2021-01-28 20:16   ` Eric Biggers
  2021-01-28 20:18   ` Eric Biggers
  1 sibling, 0 replies; 16+ messages in thread
From: Eric Biggers @ 2021-01-28 20:16 UTC (permalink / raw)
  To: Stephan Müller
  Cc: herbert, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

Please prefix the commit subject with "fscrypt: " rather than "fs: ".

On Sun, Jan 24, 2021 at 03:04:31PM +0100, Stephan Müller wrote:
> diff --git a/fs/crypto/hkdf.c b/fs/crypto/hkdf.c
> index e0ec21055505..ae236b42b1f0 100644
> --- a/fs/crypto/hkdf.c
> +++ b/fs/crypto/hkdf.c
> @@ -9,7 +9,7 @@
>   * Copyright 2019 Google LLC
>   */
>  
> -#include <crypto/hash.h>
> +#include <crypto/hkdf.h>
>  #include <crypto/sha2.h>
>  
>  #include "fscrypt_private.h"
> @@ -37,23 +37,7 @@
>   * unnecessarily long master keys.  Thus fscrypt still does HKDF-Extract.  No
>   * salt is used, since fscrypt master keys should already be pseudorandom and
>   * there's no way to persist a random salt per master key from kernel mode.
> - */
> -
> -/* HKDF-Extract (RFC 5869 section 2.2), unsalted */
> -static int hkdf_extract(struct crypto_shash *hmac_tfm, const u8 *ikm,
> -			unsigned int ikmlen, u8 prk[HKDF_HASHLEN])
> -{
> -	static const u8 default_salt[HKDF_HASHLEN];
> -	int err;
> -
> -	err = crypto_shash_setkey(hmac_tfm, default_salt, HKDF_HASHLEN);
> -	if (err)
> -		return err;
> -
> -	return crypto_shash_tfm_digest(hmac_tfm, ikm, ikmlen, prk);
> -}
> -
> -/*
> + *
>   * Compute HKDF-Extract using the given master key as the input keying material,
>   * and prepare an HMAC transform object keyed by the resulting pseudorandom key.
>   *

I don't think this comment should be joined with the one above it.  The earlier
comment describes the general approach taken with fscrypt and HKDF (including
all steps), while the one beginning with "Compute HKDF-Extract" describes
fscrypt_init_hkdf() specifically.

- Eric

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 6/7] fs: use HKDF implementation from kernel crypto API
  2021-01-24 14:04 ` [PATCH v2 6/7] fs: use HKDF implementation from kernel " Stephan Müller
  2021-01-28 20:16   ` Eric Biggers
@ 2021-01-28 20:18   ` Eric Biggers
  1 sibling, 0 replies; 16+ messages in thread
From: Eric Biggers @ 2021-01-28 20:18 UTC (permalink / raw)
  To: Stephan Müller
  Cc: herbert, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

On Sun, Jan 24, 2021 at 03:04:31PM +0100, Stephan Müller wrote:
> @@ -74,16 +57,14 @@ int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
>  		return PTR_ERR(hmac_tfm);
>  	}
>  
> -	if (WARN_ON(crypto_shash_digestsize(hmac_tfm) != sizeof(prk))) {
> +	if (WARN_ON(crypto_shash_digestsize(hmac_tfm) != HKDF_HASHLEN)) {
>  		err = -EINVAL;
>  		goto err_free_tfm;
>  	}
>  
> -	err = hkdf_extract(hmac_tfm, master_key, master_key_size, prk);
> -	if (err)
> -		goto err_free_tfm;
> -
> -	err = crypto_shash_setkey(hmac_tfm, prk, sizeof(prk));
> +	/* HKDF-Extract (RFC 5869 section 2.2), unsalted */
> +	err = crypto_hkdf_extract(hmac_tfm, NULL, 0,
> +				  master_key, master_key_size);
>  	if (err)
>  		goto err_free_tfm;
>  
> @@ -93,7 +74,6 @@ int fscrypt_init_hkdf(struct fscrypt_hkdf *hkdf, const u8 *master_key,
>  err_free_tfm:
>  	crypto_free_shash(hmac_tfm);
>  out:
> -	memzero_explicit(prk, sizeof(prk));
>  	return err;
>  }

The 'out' label isn't needed anymore.  'goto out' should be replaced with
'return 0'.

- Eric

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 7/7] fs: HKDF - remove duplicate memory clearing
  2021-01-24 14:04 ` [PATCH v2 7/7] fs: HKDF - remove duplicate memory clearing Stephan Müller
@ 2021-01-28 20:21   ` Eric Biggers
  0 siblings, 0 replies; 16+ messages in thread
From: Eric Biggers @ 2021-01-28 20:21 UTC (permalink / raw)
  To: Stephan Müller
  Cc: herbert, Jarkko Sakkinen, mathew.j.martineau, dhowells,
	linux-crypto, linux-fscrypt, linux-kernel, keyrings, simo

On Sun, Jan 24, 2021 at 03:04:50PM +0100, Stephan Müller wrote:
> The clearing of the OKM memory buffer in case of an error is already
> performed by the HKDF implementation crypto_hkdf_expand. Thus, the
> code clearing is not needed any more in the file system code base.
> 
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
>  fs/crypto/hkdf.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/crypto/hkdf.c b/fs/crypto/hkdf.c
> index ae236b42b1f0..c48dd8ca3a46 100644
> --- a/fs/crypto/hkdf.c
> +++ b/fs/crypto/hkdf.c
> @@ -102,13 +102,10 @@ int fscrypt_hkdf_expand(const struct fscrypt_hkdf *hkdf, u8 context,
>  		.iov_base = (u8 *)info,
>  		.iov_len = infolen,
>  	} };
> -	int err = crypto_hkdf_expand(hkdf->hmac_tfm,
> -				     info_iov, ARRAY_SIZE(info_iov),
> -				     okm, okmlen);
>  
> -	if (unlikely(err))
> -		memzero_explicit(okm, okmlen); /* so caller doesn't need to */
> -	return err;
> +	return crypto_hkdf_expand(hkdf->hmac_tfm,
> +				  info_iov, ARRAY_SIZE(info_iov),
> +				  okm, okmlen);
>  }
>  

Shoudn't this just be folded into the previous patch, which converted
fscrypt_hkdf_expand() to use crypto_hkdf_expand() in the first place?

- Eric

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-01-28 20:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-24 14:01 [PATCH v2 0/7] Add KDF implementations to crypto API Stephan Müller
2021-01-24 14:01 ` [PATCH v2 1/7] crypto: Add key derivation self-test support code Stephan Müller
2021-01-24 14:02 ` [PATCH v2 2/7] crypto: add SP800-108 counter key derivation function Stephan Müller
2021-01-24 14:03 ` [PATCH v2 3/7] crypto: add RFC5869 HKDF Stephan Müller
2021-01-28 20:08   ` Eric Biggers
2021-01-24 14:03 ` [PATCH v2 4/7] security: DH - remove dead code for zero padding Stephan Müller
2021-01-24 14:04 ` [PATCH v2 5/7] security: DH - use KDF implementation from crypto API Stephan Müller
2021-01-24 14:04 ` [PATCH v2 6/7] fs: use HKDF implementation from kernel " Stephan Müller
2021-01-28 20:16   ` Eric Biggers
2021-01-28 20:18   ` Eric Biggers
2021-01-24 14:04 ` [PATCH v2 7/7] fs: HKDF - remove duplicate memory clearing Stephan Müller
2021-01-28 20:21   ` Eric Biggers
2021-01-24 14:23 ` [PATCH v2 0/7] Add KDF implementations to crypto API Ard Biesheuvel
2021-01-24 14:32   ` Ard Biesheuvel
2021-01-24 14:36     ` Stephan Müller
2021-01-24 14:34   ` Stephan Müller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).