[RFC PATCH] KEYS: add SP800-56A KDF support for DH

[RFC PATCH] KEYS: add SP800-56A KDF support for DH

[Stephan Mueller]

Hi Mat, David,

This patch is based on the cryptodev-2.6 tree with the patch
4693fc734d675c5518ea9bd4c9623db45bc37402 ("KEYS: Add placeholder
for KDF usage with DH") from Linus' tree added on top.

I am aware of the fact that the compat code is not present. This
patch is intended for review to verify that the user space
interface follows the general approach for the keys subsystem.

The patch to add KDF to the kernel crypto API will be appended to
this patch.

The patch for the keyutils user space extension will also be added.

Ciao
Stephan

---8<---

SP800-56A define the use of DH with key derivation function based on a
counter. The input to the KDF is defined as (DH shared secret || other
information). The value for the "other information" is to be provided by
the caller.

The KDF is provided by the kernel crypto API. The SP800-56A KDF is equal
to the SP800-108 counter KDF. However, the caller is allowed to specify
the KDF type that he wants to use to derive the key material allowing
the use of the other KDFs provided with the kernel crypto API.

As the KDF implements the proper truncation of the DH shared secret to
the requested size, this patch fills the caller buffer up to its size.

The patch is tested with a new test added to the keyutils user space
code which uses a CAVS test vector testing the compliance with
SP800-56A.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 include/uapi/linux/keyctl.h | 10 +++++
 security/keys/Kconfig       |  1 +
 security/keys/dh.c          | 98 ++++++++++++++++++++++++++++++++++++++++-----
 security/keys/internal.h    |  5 ++-
 security/keys/keyctl.c      |  2 +-
 5 files changed, 103 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
index 86eddd6..cc4ce7c 100644
--- a/include/uapi/linux/keyctl.h
+++ b/include/uapi/linux/keyctl.h
@@ -68,4 +68,14 @@ struct keyctl_dh_params {
 	__s32 base;
 };
 
+struct keyctl_kdf_params {
+#define KEYCTL_KDF_MAX_OUTPUTLEN	1024	/* max length of KDF output */
+#define KEYCTL_KDF_MAX_STRING_LEN	64	/* maximum length of strings */
+	char *kdfname;
+	__u32 kdfnamelen;
+	char *otherinfo;
+	__u32 otherinfolen;
+	__u32 flags;
+};
+
 #endif /*  _LINUX_KEYCTL_H */
diff --git a/security/keys/Kconfig b/security/keys/Kconfig
index f826e87..56491fe 100644
--- a/security/keys/Kconfig
+++ b/security/keys/Kconfig
@@ -90,6 +90,7 @@ config KEY_DH_OPERATIONS
        bool "Diffie-Hellman operations on retained keys"
        depends on KEYS
        select MPILIB
+       select CRYPTO_KDF
        help
 	 This option provides support for calculating Diffie-Hellman
 	 public keys and shared secrets using values stored as keys
diff --git a/security/keys/dh.c b/security/keys/dh.c
index 531ed2e..4c93969 100644
--- a/security/keys/dh.c
+++ b/security/keys/dh.c
@@ -77,14 +77,74 @@ error:
 	return ret;
 }
 
+static int keyctl_dh_compute_kdf(struct keyctl_kdf_params *kdfcopy,
+				 char __user *buffer, size_t buflen,
+				 uint8_t *kbuf, size_t resultlen)
+{
+	char kdfname[CRYPTO_MAX_ALG_NAME] = { 0 };
+	struct crypto_rng *tfm;
+	uint8_t *outbuf = NULL;
+	int ret;
+
+	BUILD_BUG_ON(CRYPTO_MAX_ALG_NAME != KEYCTL_KDF_MAX_STRING_LEN);
+	if (!kdfcopy->kdfnamelen)
+		return -EFAULT;
+	if (copy_from_user(&kdfname, kdfcopy->kdfname,
+			   kdfcopy->kdfnamelen) != 0)
+		return -EFAULT;
+
+	/*
+	 * Concatenate otherinfo past DH shared secret -- the
+	 * input to the KDF is (DH shared secret || otherinfo)
+	 */
+	if (kdfcopy->otherinfo &&
+	    copy_from_user(kbuf + resultlen, kdfcopy->otherinfo,
+			   kdfcopy->otherinfolen)
+	    != 0)
+		return -EFAULT;
+
+	tfm = crypto_alloc_rng(kdfname, 0, 0);
+	if (IS_ERR(tfm))
+		return PTR_ERR(tfm);
+
+#if 0
+	/* we do not support HMAC currently */
+	ret = crypto_rng_reset(tfm, xx, xxlen);
+	if (ret) {
+		crypto_free_rng(tfm);
+		goto error5;
+	}
+#endif
+
+	outbuf = kmalloc(buflen, GFP_KERNEL);
+	if (!outbuf) {
+		ret = -ENOMEM;
+		goto err;
+	}
+
+	ret = crypto_rng_generate(tfm, kbuf, resultlen + kdfcopy->otherinfolen,
+				  outbuf, buflen);
+	if (ret)
+		goto err;
+
+	ret = buflen;
+	if (copy_to_user(buffer, outbuf, buflen) != 0)
+		ret = -EFAULT;
+
+err:
+	kzfree(outbuf);
+	crypto_free_rng(tfm);
+	return ret;
+}
 long keyctl_dh_compute(struct keyctl_dh_params __user *params,
 		       char __user *buffer, size_t buflen,
-		       void __user *reserved)
+		       struct keyctl_kdf_params __user *kdf)
 {
 	long ret;
 	MPI base, private, prime, result;
 	unsigned nbytes;
 	struct keyctl_dh_params pcopy;
+	struct keyctl_kdf_params kdfcopy;
 	uint8_t *kbuf;
 	ssize_t keylen;
 	size_t resultlen;
@@ -98,12 +158,24 @@ long keyctl_dh_compute(struct keyctl_dh_params __user *params,
 		goto out;
 	}
 
-	if (reserved) {
-		ret = -EINVAL;
-		goto out;
+	if (kdf) {
+		if (copy_from_user(&kdfcopy, kdf, sizeof(kdfcopy)) != 0) {
+			ret = -EFAULT;
+			goto out;
+		}
+		if (buflen > KEYCTL_KDF_MAX_OUTPUTLEN ||
+		    kdfcopy.otherinfolen > KEYCTL_KDF_MAX_STRING_LEN ||
+		    kdfcopy.kdfnamelen > KEYCTL_KDF_MAX_STRING_LEN) {
+			ret = -EMSGSIZE;
+			goto out;
+		}
 	}
 
-	keylen = mpi_from_key(pcopy.prime, buflen, &prime);
+	/*
+	 * If the caller requests postprocessing with a KDF, allow an
+	 * arbitrary output buffer size since the KDF ensures proper truncation.
+	 */
+	keylen = mpi_from_key(pcopy.prime, kdf ? SIZE_MAX : buflen, &prime);
 	if (keylen < 0 || !prime) {
 		/* buflen == 0 may be used to query the required buffer size,
 		 * which is the prime key length.
@@ -133,7 +205,8 @@ long keyctl_dh_compute(struct keyctl_dh_params __user *params,
 		goto error3;
 	}
 
-	kbuf = kmalloc(resultlen, GFP_KERNEL);
+	kbuf = kmalloc(kdf ? (resultlen + kdfcopy.otherinfolen) : resultlen,
+		       GFP_KERNEL);
 	if (!kbuf) {
 		ret = -ENOMEM;
 		goto error4;
@@ -147,12 +220,17 @@ long keyctl_dh_compute(struct keyctl_dh_params __user *params,
 	if (ret != 0)
 		goto error5;
 
-	ret = nbytes;
-	if (copy_to_user(buffer, kbuf, nbytes) != 0)
-		ret = -EFAULT;
+	if (kdf) {
+		ret = keyctl_dh_compute_kdf(&kdfcopy, buffer, buflen,
+					    kbuf, resultlen);
+	} else {
+		ret = nbytes;
+		if (copy_to_user(buffer, kbuf, nbytes) != 0)
+			ret = -EFAULT;
+	}
 
 error5:
-	kfree(kbuf);
+	kzfree(kbuf);
 error4:
 	mpi_free(result);
 error3:
diff --git a/security/keys/internal.h b/security/keys/internal.h
index a705a7d..35a8d11 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -259,12 +259,13 @@ static inline long keyctl_get_persistent(uid_t uid, key_serial_t destring)
 #endif
 
 #ifdef CONFIG_KEY_DH_OPERATIONS
+#include <crypto/rng.h>
 extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *,
-			      size_t, void __user *);
+			      size_t, struct keyctl_kdf_params __user *);
 #else
 static inline long keyctl_dh_compute(struct keyctl_dh_params __user *params,
 				     char __user *buffer, size_t buflen,
-				     void __user *reserved)
+				     struct keyctl_kdf_params __user *kdf)
 {
 	return -EOPNOTSUPP;
 }
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index d580ad0..b106898 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1689,7 +1689,7 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
 	case KEYCTL_DH_COMPUTE:
 		return keyctl_dh_compute((struct keyctl_dh_params __user *) arg2,
 					 (char __user *) arg3, (size_t) arg4,
-					 (void __user *) arg5);
+					 (struct keyctl_kdf_params __user *) arg5);
 
 	default:
 		return -EOPNOTSUPP;
-- 
2.7.2

[PATCH v3 0/4] crypto: Key Derivation Function (SP800-108)

[Stephan Mueller]

Hi,

this patch set implements all three key derivation functions defined in
SP800-108.

The implementation is provided as a template for random number generators,
since a KDF can be considered a form of deterministic RNG where the key
material is used as a seed.

With the KDF implemented as a template, all types of keyed hashes can be
utilized, including HMAC and CMAC. The testmgr tests are derived from
publicly available test vectors from NIST.

The KDF are all tested with a complete round of CAVS testing on 32 and 64 bit.

The patch set introduces an extension to the kernel crypto API in the first
patch by adding a template handling for random number generators based on the
same logic as for keyed hashes.

Changes v3:
* port testmgr patch to current cryptodev-2.6 tree
* add non-keyed KDF references to testmgr.c

Changes v2:
* port to 4.7-rc1

Stephan Mueller (4):
  crypto: add template handling for RNGs
  crypto: kdf - add known answer tests
  crypto: kdf - SP800-108 Key Derivation Function
  crypto: kdf - enable compilation

 crypto/Kconfig       |   7 +
 crypto/Makefile      |   1 +
 crypto/kdf.c         | 514 +++++++++++++++++++++++++++++++++++++++++++++++++++
 crypto/rng.c         |  31 ++++
 crypto/testmgr.c     | 226 ++++++++++++++++++++++
 crypto/testmgr.h     | 110 +++++++++++
 include/crypto/rng.h |  39 ++++
 7 files changed, 928 insertions(+)
 create mode 100644 crypto/kdf.c

-- 
2.7.4

[PATCH v3 1/4] crypto: add template handling for RNGs

[Stephan Mueller]

This patch adds the ability to register templates for RNGs. RNGs are
"meta" mechanisms using raw cipher primitives. Thus, RNGs can now be
implemented as templates to allow the complete flexibility the kernel
crypto API provides.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/rng.c         | 31 +++++++++++++++++++++++++++++++
 include/crypto/rng.h | 39 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/crypto/rng.c b/crypto/rng.c
index b81cffb..92cc02a 100644
--- a/crypto/rng.c
+++ b/crypto/rng.c
@@ -232,5 +232,36 @@ void crypto_unregister_rngs(struct rng_alg *algs, int count)
 }
 EXPORT_SYMBOL_GPL(crypto_unregister_rngs);
 
+void rng_free_instance(struct crypto_instance *inst)
+{
+	crypto_drop_spawn(crypto_instance_ctx(inst));
+	kfree(rng_instance(inst));
+}
+EXPORT_SYMBOL_GPL(rng_free_instance);
+
+static int rng_prepare_alg(struct rng_alg *alg)
+{
+	struct crypto_alg *base = &alg->base;
+
+	base->cra_type = &crypto_rng_type;
+	base->cra_flags &= ~CRYPTO_ALG_TYPE_MASK;
+	base->cra_flags |= CRYPTO_ALG_TYPE_RNG;
+
+	return 0;
+}
+
+int rng_register_instance(struct crypto_template *tmpl,
+			  struct rng_instance *inst)
+{
+	int err;
+
+	err = rng_prepare_alg(&inst->alg);
+	if (err)
+		return err;
+
+	return crypto_register_instance(tmpl, rng_crypto_instance(inst));
+}
+EXPORT_SYMBOL_GPL(rng_register_instance);
+
 MODULE_LICENSE("GPL");
 MODULE_DESCRIPTION("Random Number Generator");
diff --git a/include/crypto/rng.h b/include/crypto/rng.h
index b95ede3..b8a6ea3 100644
--- a/include/crypto/rng.h
+++ b/include/crypto/rng.h
@@ -15,6 +15,7 @@
 #define _CRYPTO_RNG_H
 
 #include <linux/crypto.h>
+#include <crypto/algapi.h>
 
 struct crypto_rng;
 
@@ -197,4 +198,42 @@ static inline int crypto_rng_seedsize(struct crypto_rng *tfm)
 	return crypto_rng_alg(tfm)->seedsize;
 }
 
+struct rng_instance {
+	struct rng_alg alg;
+};
+
+static inline struct rng_instance *rng_alloc_instance(
+	const char *name, struct crypto_alg *alg)
+{
+	return crypto_alloc_instance2(name, alg,
+				      sizeof(struct rng_alg) - sizeof(*alg));
+}
+
+static inline struct crypto_instance *rng_crypto_instance(
+	struct rng_instance *inst)
+{
+	return container_of(&inst->alg.base, struct crypto_instance, alg);
+}
+
+static inline void *rng_instance_ctx(struct rng_instance *inst)
+{
+	return crypto_instance_ctx(rng_crypto_instance(inst));
+}
+
+static inline struct rng_alg *__crypto_rng_alg(struct crypto_alg *alg)
+{
+	return container_of(alg, struct rng_alg, base);
+}
+
+static inline struct rng_instance *rng_instance(
+	struct crypto_instance *inst)
+{
+	return container_of(__crypto_rng_alg(&inst->alg),
+			    struct rng_instance, alg);
+}
+
+int rng_register_instance(struct crypto_template *tmpl,
+			  struct rng_instance *inst);
+void rng_free_instance(struct crypto_instance *inst);
+
 #endif
-- 
2.7.4

[PATCH v3 2/4] crypto: kdf - add known answer tests

[Stephan Mueller]

Add known answer tests to the testmgr for the KDF (SP800-108) cipher.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/testmgr.c | 226 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 crypto/testmgr.h | 110 +++++++++++++++++++++++++++
 2 files changed, 336 insertions(+)

diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index 8ea0d3f..a513d71 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -116,6 +116,11 @@ struct drbg_test_suite {
 	unsigned int count;
 };
 
+struct kdf_test_suite {
+	struct kdf_testvec *vecs;
+	unsigned int count;
+};
+
 struct akcipher_test_suite {
 	struct akcipher_testvec *vecs;
 	unsigned int count;
@@ -139,6 +144,7 @@ struct alg_test_desc {
 		struct hash_test_suite hash;
 		struct cprng_test_suite cprng;
 		struct drbg_test_suite drbg;
+		struct kdf_test_suite kdf;
 		struct akcipher_test_suite akcipher;
 		struct kpp_test_suite kpp;
 	} suite;
@@ -1758,6 +1764,64 @@ outbuf:
 	return ret;
 }
 
+static int kdf_cavs_test(struct kdf_testvec *test,
+			 const char *driver, u32 type, u32 mask)
+{
+	int ret = -EAGAIN;
+	struct crypto_rng *drng;
+	unsigned char *buf = kzalloc(test->expectedlen, GFP_KERNEL);
+
+	if (!buf)
+		return -ENOMEM;
+
+	drng = crypto_alloc_rng(driver, type | CRYPTO_ALG_INTERNAL, mask);
+	if (IS_ERR(drng)) {
+		printk(KERN_ERR "alg: kdf: could not allocate cipher handle "
+		       "for %s\n", driver);
+		kzfree(buf);
+		return -ENOMEM;
+	}
+
+	ret = crypto_rng_reset(drng, test->K1, test->K1len);
+	if (ret) {
+		printk(KERN_ERR "alg: kdf: could not set key derivation key\n");
+		goto err;
+	}
+
+	ret = crypto_rng_generate(drng, test->context, test->contextlen,
+				  buf, test->expectedlen);
+	if (ret) {
+		printk(KERN_ERR "alg: kdf: could not obtain key data\n");
+		goto err;
+	}
+
+	ret = memcmp(test->expected, buf, test->expectedlen);
+
+err:
+	crypto_free_rng(drng);
+	kzfree(buf);
+	return ret;
+}
+
+static int alg_test_kdf(const struct alg_test_desc *desc, const char *driver,
+			u32 type, u32 mask)
+{
+	int err = 0;
+	unsigned int i = 0;
+	struct kdf_testvec *template = desc->suite.kdf.vecs;
+	unsigned int tcount = desc->suite.kdf.count;
+
+	for (i = 0; i < tcount; i++) {
+		err = kdf_cavs_test(&template[i], driver, type, mask);
+		if (err) {
+			printk(KERN_ERR "alg: kdf: Test %d failed for %s\n",
+			       i, driver);
+			err = -EINVAL;
+			break;
+		}
+	}
+	return err;
+}
 
 static int alg_test_drbg(const struct alg_test_desc *desc, const char *driver,
 			 u32 type, u32 mask)
@@ -3464,6 +3528,168 @@ static const struct alg_test_desc alg_test_descs[] = {
 		.fips_allowed = 1,
 		.test = alg_test_null,
 	}, {
+		.alg = "kdf_ctr(cmac(aes))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_ctr(cmac(des3_ede))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_ctr(hmac(sha1))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_ctr(hmac(sha224))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_ctr(hmac(sha256))",
+		.test = alg_test_kdf,
+		.fips_allowed = 1,
+		.suite = {
+			.kdf = {
+				.vecs = kdf_ctr_hmac_sha256_tv_template,
+				.count = ARRAY_SIZE(kdf_ctr_hmac_sha256_tv_template)
+			}
+		}
+	}, {
+		.alg = "kdf_ctr(hmac(sha384))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_ctr(hmac(sha512))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_ctr(sha1)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_ctr(sha224)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_ctr(sha256)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_ctr(sha384)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_ctr(sha512)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(cmac(aes))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(cmac(des3_ede))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(hmac(sha1))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(hmac(sha224))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(hmac(sha256))",
+		.test = alg_test_kdf,
+		.fips_allowed = 1,
+		.suite = {
+			.kdf = {
+				.vecs = kdf_dpi_hmac_sha256_tv_template,
+				.count = ARRAY_SIZE(kdf_dpi_hmac_sha256_tv_template)
+			}
+		}
+	}, {
+		.alg = "kdf_dpi(hmac(sha384))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(hmac(sha512))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(sha1)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(sha224)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(sha256)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(sha384)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_dpi(sha512)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(cmac(aes))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(cmac(des3_ede))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(hmac(sha1))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(hmac(sha224))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(hmac(sha256))",
+		.test = alg_test_kdf,
+		.fips_allowed = 1,
+		.suite = {
+			.kdf = {
+				.vecs = kdf_fb_hmac_sha256_tv_template,
+				.count = ARRAY_SIZE(kdf_fb_hmac_sha256_tv_template)
+			}
+		}
+	}, {
+		.alg = "kdf_fb(hmac(sha384))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(hmac(sha512))",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(sha1)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(sha224)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(sha256)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(sha384)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
+		.alg = "kdf_fb(sha512)",
+		.test = alg_test_null,
+		.fips_allowed = 1,
+	}, {
 		.alg = "kw(aes)",
 		.test = alg_test_skcipher,
 		.fips_allowed = 1,
diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index 4ce2d86..bfb5da3 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -123,6 +123,15 @@ struct drbg_testvec {
 	size_t expectedlen;
 };
 
+struct kdf_testvec {
+	unsigned char *K1;
+	size_t K1len;
+	unsigned char *context;
+	size_t contextlen;
+	unsigned char *expected;
+	size_t expectedlen;
+};
+
 struct akcipher_testvec {
 	unsigned char *key;
 	unsigned char *m;
@@ -25629,6 +25638,107 @@ static struct drbg_testvec drbg_nopr_ctr_aes128_tv_template[] = {
 	},
 };
 
+/*
+ * Test vector obtained from
+ * http://csrc.nist.gov/groups/STM/cavp/documents/KBKDF800-108/CounterMode.zip
+ */
+static struct kdf_testvec kdf_ctr_hmac_sha256_tv_template[] = {
+	{
+		.K1 = (unsigned char *)
+			"\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",
+		.K1len = 32,
+		.context = (unsigned char *)
+			"\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",
+		.contextlen = 60,
+		.expected = (unsigned char *)
+			"\x10\x62\x13\x42\xbf\xb0\xfd\x40"
+			"\x04\x6c\x0e\x29\xf2\xcf\xdb\xf0",
+		.expectedlen = 16
+	}
+};
+
+/*
+ * Test vector obtained from
+ * http://csrc.nist.gov/groups/STM/cavp/documents/KBKDF800-108/FeedbackModeNOzeroiv.zip
+ */
+static struct kdf_testvec kdf_fb_hmac_sha256_tv_template[] = {
+	{
+		.K1 = (unsigned char *)
+			"\x93\xf6\x98\xe8\x42\xee\xd7\x53"
+			"\x94\xd6\x29\xd9\x57\xe2\xe8\x9c"
+			"\x6e\x74\x1f\x81\x0b\x62\x3c\x8b"
+			"\x90\x1e\x38\x37\x6d\x06\x8e\x7b",
+		.K1len = 32,
+		.context = (unsigned char *)
+			"\x9f\x57\x5d\x90\x59\xd3\xe0\xc0"
+			"\x80\x3f\x08\x11\x2f\x8a\x80\x6d"
+			"\xe3\xc3\x47\x19\x12\xcd\xf4\x2b"
+			"\x09\x53\x88\xb1\x4b\x33\x50\x8e"
+			"\x53\xb8\x9c\x18\x69\x0e\x20\x57"
+			"\xa1\xd1\x67\x82\x2e\x63\x6d\xe5"
+			"\x0b\xe0\x01\x85\x32\xc4\x31\xf7"
+			"\xf5\xe3\x7f\x77\x13\x92\x20\xd5"
+			"\xe0\x42\x59\x9e\xbe\x26\x6a\xf5"
+			"\x76\x7e\xe1\x8c\xd2\xc5\xc1\x9a"
+			"\x1f\x0f\x80",
+		.contextlen = 83,
+		.expected = (unsigned char *)
+			"\xbd\x14\x76\xf4\x3a\x4e\x31\x57"
+			"\x47\xcf\x59\x18\xe0\xea\x5b\xc0"
+			"\xd9\x87\x69\x45\x74\x77\xc3\xab"
+			"\x18\xb7\x42\xde\xf0\xe0\x79\xa9"
+			"\x33\xb7\x56\x36\x5a\xfb\x55\x41"
+			"\xf2\x53\xfe\xe4\x3c\x6f\xd7\x88"
+			"\xa4\x40\x41\x03\x85\x09\xe9\xee"
+			"\xb6\x8f\x7d\x65\xff\xbb\x5f\x95",
+		.expectedlen = 64
+	}
+};
+
+/*
+ * Test vector obtained from
+ * http://csrc.nist.gov/groups/STM/cavp/documents/KBKDF800-108/PipelineModewithCounter.zip
+ */
+static struct kdf_testvec kdf_dpi_hmac_sha256_tv_template[] = {
+	{
+		.K1 = (unsigned char *)
+			"\x02\xd3\x6f\xa0\x21\xc2\x0d\xdb"
+			"\xde\xe4\x69\xf0\x57\x94\x68\xba"
+			"\xe5\xcb\x13\xb5\x48\xb6\xc6\x1c"
+			"\xdf\x9d\x3e\xc4\x19\x11\x1d\xe2",
+		.K1len = 32,
+		.context = (unsigned char *)
+			"\x85\xab\xe3\x8b\xf2\x65\xfb\xdc"
+			"\x64\x45\xae\x5c\x71\x15\x9f\x15"
+			"\x48\xc7\x3b\x7d\x52\x6a\x62\x31"
+			"\x04\x90\x4a\x0f\x87\x92\x07\x0b"
+			"\x3d\xf9\x90\x2b\x96\x69\x49\x04"
+			"\x25\xa3\x85\xea\xdb\x0f\x9c\x76"
+			"\xe4\x6f\x0f",
+		.contextlen = 51,
+		.expected = (unsigned char *)
+			"\xd6\x9f\x74\xf5\x18\xc9\xf6\x4f"
+			"\x90\xa0\xbe\xeb\xab\x69\xf6\x89"
+			"\xb7\x3b\x5c\x13\xeb\x0f\x86\x0a"
+			"\x95\xca\xd7\xd9\x81\x4f\x8c\x50"
+			"\x6e\xb7\xb1\x79\xa5\xc5\xb4\x46"
+			"\x6a\x9e\xc1\x54\xc3\xbf\x1c\x13"
+			"\xef\xd6\xec\x0d\x82\xb0\x2c\x29"
+			"\xaf\x2c\x69\x02\x99\xed\xc4\x53",
+		.expectedlen = 64
+	}
+};
+
 /* Cast5 test vectors from RFC 2144 */
 #define CAST5_ENC_TEST_VECTORS		4
 #define CAST5_DEC_TEST_VECTORS		4
-- 
2.7.4

[PATCH v3 3/4] crypto: kdf - SP800-108 Key Derivation Function

[Stephan Mueller]

The SP800-108 compliant Key Derivation Function is implemented as a
random number generator considering that it behaves like a deterministic
RNG.

All three KDF types specified in SP800-108 are implemented.

The code comments provide details about how to invoke the different KDF
types.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/kdf.c | 514 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 514 insertions(+)
 create mode 100644 crypto/kdf.c

diff --git a/crypto/kdf.c b/crypto/kdf.c
new file mode 100644
index 0000000..b39bddf
--- /dev/null
+++ b/crypto/kdf.c
@@ -0,0 +1,514 @@
+/*
+ * Copyright (C) 2015, Stephan Mueller <smueller@chronox.de>
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ * 1. Redistributions of source code must retain the above copyright
+ *    notice, and the entire permission notice in its entirety,
+ *    including the disclaimer of warranties.
+ * 2. Redistributions in binary form must reproduce the above copyright
+ *    notice, this list of conditions and the following disclaimer in the
+ *    documentation and/or other materials provided with the distribution.
+ * 3. The name of the author may not be used to endorse or promote
+ *    products derived from this software without specific prior
+ *    written permission.
+ *
+ * ALTERNATIVELY, this product may be distributed under the terms of
+ * the GNU General Public License, in which case the provisions of the GPL2
+ * are required INSTEAD OF the above restrictions.  (This clause is
+ * necessary due to a potential bad interaction between the GPL and
+ * the restrictions contained in a BSD-style copyright.)
+ *
+ * THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
+ * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
+ * OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF
+ * WHICH ARE HEREBY DISCLAIMED.  IN NO EVENT SHALL THE AUTHOR BE
+ * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
+ * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
+ * OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
+ * BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
+ * LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
+ * USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH
+ * DAMAGE.
+ */
+
+/*
+ * For performing a KDF operation, the following input is required
+ * from the caller:
+ *
+ *	* Keying material to be used to derive the new keys from
+ *	  (denoted as Ko in SP800-108)
+ *	* Label -- a free form binary string
+ *	* Context -- a free form binary string
+ *
+ * The KDF is implemented as a random number generator.
+ *
+ * The Ko keying material is to be provided with the initialization of the KDF
+ * "random number generator", i.e. with the crypto_rng_reset function.
+ *
+ * The Label and Context concatenated string is provided when obtaining random
+ * numbers, i.e. with the crypto_rng_generate function. The caller must format
+ * the free-form Label || Context input as deemed necessary for the given
+ * purpose. Note, SP800-108 mandates that the Label and Context are separated
+ * by a 0x00 byte, i.e. the caller shall provide the input as
+ * Label || 0x00 || Context when trying to be compliant to SP800-108. For
+ * the feedback KDF, an IV is required as documented below.
+ *
+ * Example without proper error handling:
+ *	char *keying_material = "\x00\x11\x22\x33\x44\x55\x66\x77";
+ *	char *label_context = "\xde\xad\xbe\xef\x00\xde\xad\xbe\xef";
+ *	kdf = crypto_alloc_rng(name, 0, 0);
+ *	crypto_rng_reset(kdf, keying_material, 8);
+ *	crypto_rng_generate(kdf, label_context, 9, outbuf, outbuflen);
+ *
+ * NOTE: Technically you can use one buffer for holding the label_context and
+ *	 the outbuf in the example above. Howerver, multiple rounds of the
+ *	 KDF are to be expected with the input must always be the same.
+ *	 The first round would replace the input in case of one buffer, and the
+ *	 KDF would calculate a cryptographically strong result which, however,
+ *	 is not portable to other KDF implementations! Thus, always use
+ *	 different buffers for the label_context and the outbuf. A safe
+ *	 in-place operation can only be done when only one round of the KDF
+ *	 is executed (i.e. the size of the requested buffer is equal to the
+ *	 digestsize of the used MAC).
+ */
+
+#include <linux/module.h>
+#include <crypto/rng.h>
+#include <crypto/hash.h>
+#include <crypto/internal/hash.h>
+
+struct crypto_kdf_ctx {
+	struct shash_desc shash;
+	char ctx[];
+};
+
+/* convert 32 bit integer into its string representation */
+static inline void crypto_kw_cpu_to_be32(u32 val, u8 *buf)
+{
+	__be32 *a = (__be32 *)buf;
+
+	*a = cpu_to_be32(val);
+}
+
+/*
+ * Implementation of the KDF in double pipeline iteration mode according with
+ * counter to SP800-108 section 5.3.
+ *
+ * The caller must provide Label || 0x00 || Context in src. This src pointer
+ * may also be NULL if the caller wishes not to provide anything.
+ */
+static int crypto_kdf_dpi_random(struct crypto_rng *rng,
+				 const u8 *src, unsigned int slen,
+				 u8 *dst, unsigned int dlen)
+{
+	struct crypto_kdf_ctx *ctx = crypto_tfm_ctx(crypto_rng_tfm(rng));
+	struct shash_desc *desc = &ctx->shash;
+	unsigned int h = crypto_shash_digestsize(desc->tfm);
+	unsigned int alignmask = crypto_shash_alignmask(desc->tfm);
+	int err = 0;
+	u8 *dst_orig = dst;
+	u8 Aiblock[h + alignmask];
+	u8 *Ai = PTR_ALIGN((u8 *)Aiblock, alignmask + 1);
+	u32 i = 1;
+	u8 iteration[sizeof(u32)];
+
+	/* enforce the note from above */
+	if (dlen != h && src == dst)
+		return -EINVAL;
+
+	memset(Ai, 0, h);
+
+	while (dlen) {
+		/* Calculate A(i) */
+		if (dst == dst_orig && src && slen)
+			/* 5.3 step 4 and 5.a */
+			err = crypto_shash_digest(desc, src, slen, Ai);
+		else
+			/* 5.3 step 5.a */
+			err = crypto_shash_digest(desc, Ai, h, Ai);
+		if (err)
+			goto err;
+
+		/* Calculate K(i) -- step 5.b */
+		err = crypto_shash_init(desc);
+		if (err)
+			goto err;
+
+		err = crypto_shash_update(desc, Ai, h);
+		if (err)
+			goto err;
+
+		crypto_kw_cpu_to_be32(i, iteration);
+		err = crypto_shash_update(desc, iteration, sizeof(u32));
+		if (err)
+			goto err;
+		if (src && slen) {
+			err = crypto_shash_update(desc, src, slen);
+			if (err)
+				goto err;
+		}
+
+		if (dlen < h) {
+			u8 tmpbuffer[h];
+
+			err = crypto_shash_final(desc, tmpbuffer);
+			if (err)
+				goto err;
+			memcpy(dst, tmpbuffer, dlen);
+			memzero_explicit(tmpbuffer, h);
+			goto ret;
+		} else {
+			err = crypto_shash_final(desc, dst);
+			if (err)
+				goto err;
+			dlen -= h;
+			dst += h;
+			i++;
+		}
+	}
+
+err:
+	memzero_explicit(dst_orig, dlen);
+ret:
+	memzero_explicit(Ai, h);
+	return err;
+}
+
+/*
+ * Implementation of the KDF in feedback mode with a non-NULL IV and with
+ * counter according to SP800-108 section 5.2. The IV is supplied with src
+ * and must be equal to the digestsize of the used cipher.
+ *
+ * In addition, the caller must provide Label || 0x00 || Context in src. This
+ * src pointer must not be NULL as the IV is required. The ultimate format of
+ * the src pointer is IV || Label || 0x00 || Context where the length of the
+ * IV is equal to the output size of the PRF.
+ */
+static int crypto_kdf_fb_random(struct crypto_rng *rng,
+				const u8 *src, unsigned int slen,
+				u8 *dst, unsigned int dlen)
+{
+	struct crypto_kdf_ctx *ctx = crypto_tfm_ctx(crypto_rng_tfm(rng));
+	struct shash_desc *desc = &ctx->shash;
+	unsigned int h = crypto_shash_digestsize(desc->tfm);
+	int err = 0;
+	u8 *dst_orig = dst;
+	const u8 *label;
+	unsigned int labellen = 0;
+	u32 i = 1;
+	u8 iteration[sizeof(u32)];
+
+	/* enforce the note from above */
+	if (dlen != h && src == dst)
+		return -EINVAL;
+
+	/* require the presence of an IV */
+	if (!src || slen < h)
+		return -EINVAL;
+
+	/* calculate the offset of the label / context data */
+	label = src + h;
+	labellen = slen - h;
+
+	while (dlen) {
+		err = crypto_shash_init(desc);
+		if (err)
+			goto err;
+
+		/*
+		 * Feedback mode applies to all rounds except first which uses
+		 * the IV.
+		 */
+		if (dst_orig == dst)
+			err = crypto_shash_update(desc, src, h);
+		else
+			err = crypto_shash_update(desc, dst - h, h);
+		if (err)
+			goto err;
+
+		crypto_kw_cpu_to_be32(i, iteration);
+		err = crypto_shash_update(desc, iteration, sizeof(u32));
+		if (err)
+			goto err;
+		if (labellen) {
+			err = crypto_shash_update(desc, label, labellen);
+			if (err)
+				goto err;
+		}
+
+		if (dlen < h) {
+			u8 tmpbuffer[h];
+
+			err = crypto_shash_final(desc, tmpbuffer);
+			if (err)
+				goto err;
+			memcpy(dst, tmpbuffer, dlen);
+			memzero_explicit(tmpbuffer, h);
+			return 0;
+		} else {
+			err = crypto_shash_final(desc, dst);
+			if (err)
+				goto err;
+			dlen -= h;
+			dst += h;
+			i++;
+		}
+	}
+
+	return 0;
+
+err:
+	memzero_explicit(dst_orig, dlen);
+	return err;
+}
+
+/*
+ * Implementation of the KDF in counter mode according to SP800-108 section 5.1.
+ *
+ * The caller must provide Label || 0x00 || Context in src. This src pointer
+ * may also be NULL if the caller wishes not to provide anything.
+ */
+static int crypto_kdf_ctr_random(struct crypto_rng *rng,
+				 const u8 *src, unsigned int slen,
+				 u8 *dst, unsigned int dlen)
+{
+	struct crypto_kdf_ctx *ctx = crypto_tfm_ctx(crypto_rng_tfm(rng));
+	struct shash_desc *desc = &ctx->shash;
+	unsigned int h = crypto_shash_digestsize(desc->tfm);
+	int err = 0;
+	u8 *dst_orig = dst;
+	u32 i = 1;
+	u8 iteration[sizeof(u32)];
+
+	/* enforce the note from above */
+	if (dlen != h && src == dst)
+		return -EINVAL;
+
+	while (dlen) {
+		err = crypto_shash_init(desc);
+		if (err)
+			goto err;
+
+		crypto_kw_cpu_to_be32(i, iteration);
+		err = crypto_shash_update(desc, iteration, sizeof(u32));
+		if (err)
+			goto err;
+
+		if (src && slen) {
+			err = crypto_shash_update(desc, src, slen);
+			if (err)
+				goto err;
+		}
+
+		if (dlen < h) {
+			u8 tmpbuffer[h];
+
+			err = crypto_shash_final(desc, tmpbuffer);
+			if (err)
+				goto err;
+			memcpy(dst, tmpbuffer, dlen);
+			memzero_explicit(tmpbuffer, h);
+			return 0;
+		} else {
+			err = crypto_shash_final(desc, dst);
+			if (err)
+				goto err;
+
+			dlen -= h;
+			dst += h;
+			i++;
+		}
+	}
+
+	return 0;
+
+err:
+	memzero_explicit(dst_orig, dlen);
+	return err;
+}
+
+/*
+ * The seeding of the KDF allows to set a key which must be at least
+ * digestsize long.
+ */
+static int crypto_kdf_seed(struct crypto_rng *rng,
+			   const u8 *seed, unsigned int slen)
+{
+	struct crypto_kdf_ctx *ctx = crypto_tfm_ctx(crypto_rng_tfm(rng));
+	unsigned int ds = crypto_shash_digestsize(ctx->shash.tfm);
+
+	/* Check according to SP800-108 section 7.2 */
+	if (ds > slen)
+		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(ctx->shash.tfm, seed, slen);
+}
+
+static int crypto_kdf_init_tfm(struct crypto_tfm *tfm)
+{
+	struct crypto_instance *inst = crypto_tfm_alg_instance(tfm);
+	struct crypto_shash_spawn *spawn = crypto_instance_ctx(inst);
+	struct crypto_kdf_ctx *ctx = crypto_tfm_ctx(tfm);
+	struct crypto_shash *hash;
+
+	hash = crypto_spawn_shash(spawn);
+	if (IS_ERR(hash))
+		return PTR_ERR(hash);
+
+	ctx->shash.tfm = hash;
+
+	return 0;
+}
+
+static void crypto_kdf_exit_tfm(struct crypto_tfm *tfm)
+{
+	struct crypto_kdf_ctx *ctx = crypto_tfm_ctx(tfm);
+
+	crypto_free_shash(ctx->shash.tfm);
+}
+
+static int crypto_kdf_alloc_common(struct crypto_template *tmpl,
+				   struct rtattr **tb,
+				   const u8 *name,
+				   int (*generate)(struct crypto_rng *tfm,
+						   const u8 *src,
+						   unsigned int slen,
+						   u8 *dst, unsigned int dlen))
+{
+	struct rng_instance *inst;
+	struct crypto_alg *alg;
+	struct shash_alg *salg;
+	int err;
+	unsigned int ds, ss;
+
+	err = crypto_check_attr_type(tb, CRYPTO_ALG_TYPE_RNG);
+	if (err)
+		return err;
+
+	salg = shash_attr_alg(tb[1], 0, 0);
+	if (IS_ERR(salg))
+		return PTR_ERR(salg);
+
+	ds = salg->digestsize;
+	ss = salg->statesize;
+	alg = &salg->base;
+
+	inst = rng_alloc_instance(name, alg);
+	err = PTR_ERR(inst);
+	if (IS_ERR(inst))
+		goto out_put_alg;
+
+	err = crypto_init_shash_spawn(rng_instance_ctx(inst), salg,
+				      rng_crypto_instance(inst));
+	if (err)
+		goto out_free_inst;
+
+	inst->alg.base.cra_priority	= alg->cra_priority;
+	inst->alg.base.cra_blocksize	= alg->cra_blocksize;
+	inst->alg.base.cra_alignmask	= alg->cra_alignmask;
+
+	inst->alg.generate		= generate;
+	inst->alg.seed			= crypto_kdf_seed;
+	inst->alg.seedsize		= ds;
+
+	inst->alg.base.cra_init		= crypto_kdf_init_tfm;
+	inst->alg.base.cra_exit		= crypto_kdf_exit_tfm;
+	inst->alg.base.cra_ctxsize	= ALIGN(sizeof(struct crypto_kdf_ctx) +
+					  ss * 2, crypto_tfm_ctx_alignment());
+
+	err = rng_register_instance(tmpl, inst);
+
+	if (err) {
+out_free_inst:
+		rng_free_instance(rng_crypto_instance(inst));
+	}
+
+out_put_alg:
+	crypto_mod_put(alg);
+	return err;
+}
+
+static int crypto_kdf_ctr_create(struct crypto_template *tmpl,
+				 struct rtattr **tb)
+{
+	return crypto_kdf_alloc_common(tmpl, tb, "kdf_ctr",
+				       crypto_kdf_ctr_random);
+}
+
+static struct crypto_template crypto_kdf_ctr_tmpl = {
+	.name = "kdf_ctr",
+	.create = crypto_kdf_ctr_create,
+	.free = rng_free_instance,
+	.module = THIS_MODULE,
+};
+
+static int crypto_kdf_fb_create(struct crypto_template *tmpl,
+				struct rtattr **tb) {
+	return crypto_kdf_alloc_common(tmpl, tb, "kdf_fb",
+				       crypto_kdf_fb_random);
+}
+
+static struct crypto_template crypto_kdf_fb_tmpl = {
+	.name = "kdf_fb",
+	.create = crypto_kdf_fb_create,
+	.free = rng_free_instance,
+	.module = THIS_MODULE,
+};
+
+static int crypto_kdf_dpi_create(struct crypto_template *tmpl,
+				 struct rtattr **tb) {
+	return crypto_kdf_alloc_common(tmpl, tb, "kdf_dpi",
+				       crypto_kdf_dpi_random);
+}
+
+static struct crypto_template crypto_kdf_dpi_tmpl = {
+	.name = "kdf_dpi",
+	.create = crypto_kdf_dpi_create,
+	.free = rng_free_instance,
+	.module = THIS_MODULE,
+};
+
+static int __init crypto_kdf_init(void)
+{
+	int err = crypto_register_template(&crypto_kdf_ctr_tmpl);
+
+	if (err)
+		return err;
+
+	err = crypto_register_template(&crypto_kdf_fb_tmpl);
+	if (err) {
+		crypto_unregister_template(&crypto_kdf_ctr_tmpl);
+		return err;
+	}
+
+	err = crypto_register_template(&crypto_kdf_dpi_tmpl);
+	if (err) {
+		crypto_unregister_template(&crypto_kdf_ctr_tmpl);
+		crypto_unregister_template(&crypto_kdf_fb_tmpl);
+	}
+	return err;
+}
+
+static void __exit crypto_kdf_exit(void)
+{
+	crypto_unregister_template(&crypto_kdf_ctr_tmpl);
+	crypto_unregister_template(&crypto_kdf_fb_tmpl);
+	crypto_unregister_template(&crypto_kdf_dpi_tmpl);
+}
+
+module_init(crypto_kdf_init);
+module_exit(crypto_kdf_exit);
+
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_AUTHOR("Stephan Mueller <smueller@chronox.de>");
+MODULE_DESCRIPTION("Key Derivation Function according to SP800-108");
+MODULE_ALIAS_CRYPTO("kdf_ctr");
+MODULE_ALIAS_CRYPTO("kdf_fb");
+MODULE_ALIAS_CRYPTO("kdf_dpi");
-- 
2.7.4

[PATCH v3 4/4] crypto: kdf - enable compilation

[Stephan Mueller]

Include KDF into Kconfig and Makefile for compilation.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/Kconfig  | 7 +++++++
 crypto/Makefile | 1 +
 2 files changed, 8 insertions(+)

diff --git a/crypto/Kconfig b/crypto/Kconfig
index 62fcbb9..7779af8 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -372,6 +372,13 @@ config CRYPTO_KEYWRAP
 	  Support for key wrapping (NIST SP800-38F / RFC3394) without
 	  padding.
 
+config CRYPTO_KDF
+	tristate "Key Derivation Function (SP800-108)"
+	select CRYPTO_RNG
+	help
+	  Support for KDF compliant to SP800-108. All three types of
+	  KDF specified in SP800-108 are implemented.
+
 comment "Hash modes"
 
 config CRYPTO_CMAC
diff --git a/crypto/Makefile b/crypto/Makefile
index df1bcfb..d3733a4 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_CRYPTO_LRW) += lrw.o
 obj-$(CONFIG_CRYPTO_XTS) += xts.o
 obj-$(CONFIG_CRYPTO_CTR) += ctr.o
 obj-$(CONFIG_CRYPTO_KEYWRAP) += keywrap.o
+obj-$(CONFIG_CRYPTO_KDF) += kdf.o
 obj-$(CONFIG_CRYPTO_GCM) += gcm.o
 obj-$(CONFIG_CRYPTO_CCM) += ccm.o
 obj-$(CONFIG_CRYPTO_CHACHA20POLY1305) += chacha20poly1305.o
-- 
2.7.4

[PATCH] DH support: add KDF handling support

[Stephan Mueller]

Hi Mat, David,

During the development of this patch, I saw that the test
framework seems to be broken: when I change the expected
values by one bit, the test framework will still mark the
received result as PASS even though the returned data does
not match the expected data.

---8<----

Add the interface logic to support DH with KDF handling support.

The dh_compute code now allows the following options:

- no KDF support / output of raw DH shared secret:
  dh_compute <private> <prime> <base>

- KDF support without "other information" string:
  dh_compute <private> <prime> <base> <output length> <KDF type>

- KDF support with "other information string:
  dh_compute <private> <prime> <base> <output length> <KDF type> <OI
  string>

The test to verify the code is based on a test vector used for the CAVS
testing of SP800-56A.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 keyctl.c                                 | 14 +++++-
 keyutils.c                               | 48 ++++++++++++++++++
 keyutils.h                               | 13 +++++
 tests/keyctl/dh_compute/valid/runtest.sh | 83 ++++++++++++++++++++++++++++++++
 4 files changed, 156 insertions(+), 2 deletions(-)

diff --git a/keyctl.c b/keyctl.c
index edb03de..32478b3 100644
--- a/keyctl.c
+++ b/keyctl.c
@@ -1638,14 +1638,24 @@ static void act_keyctl_dh_compute(int argc, char *argv[])
 	char *p;
 	int ret, sep, col;
 
-	if (argc != 4)
+	if (argc != 4 && argc != 6 && argc != 7)
 		format();
 
 	private = get_key_id(argv[1]);
 	prime = get_key_id(argv[2]);
 	base = get_key_id(argv[3]);
 
-	ret = keyctl_dh_compute_alloc(private, prime, base, &buffer);
+	if (argc == 4)
+		ret = keyctl_dh_compute_alloc(private, prime, base, &buffer);
+	else if (argc == 6)
+		ret = keyctl_dh_compute_kdf(private, prime, base, argv[4],
+					    argv[5], NULL, &buffer);
+	else if (argc == 7)
+		ret = keyctl_dh_compute_kdf(private, prime, base, argv[4],
+					    argv[5], argv[6], &buffer);
+	else
+		error("dh_compute: unknown number of arguments");
+
 	if (ret < 0)
 		error("keyctl_dh_compute_alloc");
 
diff --git a/keyutils.c b/keyutils.c
index 2a69304..ffdd622 100644
--- a/keyutils.c
+++ b/keyutils.c
@@ -386,6 +386,54 @@ int keyctl_dh_compute_alloc(key_serial_t private, key_serial_t prime,
 }
 
 /*
+ * fetch DH computation results processed by a KDF into an
+ * allocated buffer
+ * - resulting buffer has an extra NUL added to the end
+ * - returns count (not including extraneous NUL)
+ */
+int keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
+			  key_serial_t base, char *len, char *kdfname,
+			  char *otherinfo, void **_buffer)
+{
+	char *buf;
+	unsigned long buflen;
+	int ret;
+	struct keyctl_dh_params params = { .private = private,
+					   .prime = prime,
+					   .base = base };
+	struct keyctl_kdf_params kdfparams;
+
+	buflen = strtoul(len, NULL, 10);
+	if (buflen > KEYCTL_KDF_MAX_OUTPUTLEN)
+		return -1;
+
+	buf = malloc(buflen + 1);
+	if (!buf)
+		return -1;
+
+	if (otherinfo) {
+		kdfparams.kdfname = kdfname;
+		kdfparams.kdfnamelen = strlen(kdfname);
+		kdfparams.otherinfo = otherinfo;
+		kdfparams.otherinfolen = strlen(otherinfo);
+	} else {
+		kdfparams.kdfname = kdfname;
+		kdfparams.kdfnamelen = strlen(kdfname);
+		kdfparams.otherinfo = NULL;
+		kdfparams.otherinfolen = 0;
+	}
+	ret = keyctl(KEYCTL_DH_COMPUTE, &params, buf, buflen, &kdfparams);
+	if (ret < 0) {
+		free(buf);
+		return -1;
+	}
+
+	buf[ret] = 0;
+	*_buffer = buf;
+	return ret;
+}
+
+/*
  * Depth-first recursively apply a function over a keyring tree
  */
 static int recursive_key_scan_aux(key_serial_t parent, key_serial_t key,
diff --git a/keyutils.h b/keyutils.h
index b321aa8..5026270 100644
--- a/keyutils.h
+++ b/keyutils.h
@@ -108,6 +108,16 @@ struct keyctl_dh_params {
 	key_serial_t base;
 };
 
+struct keyctl_kdf_params {
+#define KEYCTL_KDF_MAX_OUTPUTLEN        1024    /* max length of KDF output */
+#define KEYCTL_KDF_MAX_STRING_LEN       64      /* maximum length of strings */
+	char *kdfname;
+	uint32_t kdfnamelen;
+	char *otherinfo;
+	uint32_t otherinfolen;
+	uint32_t flags;
+};
+
 /*
  * syscall wrappers
  */
@@ -172,6 +182,9 @@ extern int keyctl_read_alloc(key_serial_t id, void **_buffer);
 extern int keyctl_get_security_alloc(key_serial_t id, char **_buffer);
 extern int keyctl_dh_compute_alloc(key_serial_t private, key_serial_t prime,
 				   key_serial_t base, void **_buffer);
+int keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
+			  key_serial_t base, char *len, char *kdfname,
+			  char *otherinfo, void **_buffer);
 
 typedef int (*recursive_key_scanner_t)(key_serial_t parent, key_serial_t key,
 				       char *desc, int desc_len, void *data);
diff --git a/tests/keyctl/dh_compute/valid/runtest.sh b/tests/keyctl/dh_compute/valid/runtest.sh
index 40ec387..1c77268 100644
--- a/tests/keyctl/dh_compute/valid/runtest.sh
+++ b/tests/keyctl/dh_compute/valid/runtest.sh
@@ -80,6 +80,89 @@ marker "COMPUTE DH PUBLIC KEY"
 dh_compute $privateid $primeid $generatorid
 expect_payload payload $public
 
+
+################################################################
+# Testing DH compute with KDF according to SP800-56A
+#
+# test vectors from http://csrc.nist.gov/groups/STM/cavp/documents/keymgmt/KASTestVectorsFFC2014.zip
+################################################################
+
+# SHA-256
+
+# XephemCAVS
+private="\x81\xb2\xc6\x5f\x5c\xba\xc0\x0b\x13\x53\xac\x38\xbd\x77\xa2\x5a"
+private+="\x86\x50\xed\x48\x5e\x41\x3e\xac\x1d\x6c\x48\x85"
+
+# P
+prime="\xa3\xcc\x62\x23\xe5\x0c\x6e\x3f\x7b\xb0\x58\x1d\xcb\x9e\x9f\xf0"
+prime+="\x2c\x58\x07\x68\x32\x8a\x15\x20\x7b\x1c\x32\x31\x7f\xb7\x84\x96"
+prime+="\x81\x5e\x3c\xf7\xf9\xd0\x9c\xcb\x9f\xa8\x40\xff\x47\x98\x51\x1a"
+prime+="\x17\xb5\x59\x28\x72\x1e\x5d\xfb\xcc\xc5\x41\x47\xe0\xf0\x5f\x85"
+prime+="\xb3\xac\x41\x0b\x6a\xe3\xf5\x9b\x79\x6f\x3f\xea\xc7\xfc\x52\x49"
+prime+="\x21\x7e\xb2\xa0\x45\x88\x29\x3a\x5a\xde\x22\x78\x79\xf4\x6c\xeb"
+prime+="\x56\x45\x7b\x5c\x43\x12\x93\xe5\xe1\x04\xd1\xb9\x64\xbd\x2c\xdf"
+prime+="\xde\xff\xa0\x40\x49\xa9\x1e\x67\xee\x8c\x86\xe9\x44\xf0\x4f\x94"
+prime+="\x4a\x30\xe3\x61\xf8\xd1\x5d\x17\xe5\x01\x0c\xab\xb4\xef\x40\xc0"
+prime+="\xeb\xa5\xf4\xa2\x52\xd4\xfd\x6c\xf9\xda\xe6\x0e\x86\xe4\xb3\x00"
+prime+="\x9b\x1d\xfc\x92\x66\x70\x35\x72\x61\x58\x7a\xd0\x5c\x00\xa6\xc6"
+prime+="\xf0\x10\x6c\xec\x8f\xc5\x91\x31\x51\x50\x84\xa8\x70\x59\x41\x65"
+prime+="\xb4\x93\x90\xdb\x2d\x00\xe7\x53\x8f\x23\x0d\x53\x2f\x4a\x4e\xca"
+prime+="\x83\x09\xd7\x07\xc0\xb3\x83\x5c\xee\x04\xf3\xca\x55\x8a\x22\xc6"
+prime+="\xb5\x20\xfe\x25\xde\x6f\xfa\x90\xef\xda\x49\x27\xd0\x18\x59\x4c"
+prime+="\x0c\x0b\x77\x06\x73\x93\xb7\xf1\xe0\xfc\x7c\xf2\x16\xaf\xf3\x9f"
+
+# YephemIUT
+xa="\x9a\x70\x82\x2d\x3f\x06\x12\x3d\x0e\x51\x8e\xe1\x16\x51\xe5\xf6"
+xa+="\xb1\x19\xdc\x3b\x97\xd5\xb1\xc0\xa2\xa6\xf6\xde\x94\x25\x64\xba"
+xa+="\x10\x06\x1e\xec\xde\xb7\x36\x9c\xa5\x37\x49\x9e\x04\xb0\x36\xe9"
+xa+="\x7f\x44\x5a\x95\x6f\x63\x69\xae\x6e\x63\xfd\x27\xea\xe3\xe3\x47"
+xa+="\x85\x54\x47\xd3\xba\xc1\xc6\x0c\x10\xe7\x35\x07\x72\xc6\xc0\xc6"
+xa+="\xfb\xf9\xca\x3e\x38\xf0\xe8\x65\x88\x25\xd3\xb2\x0f\x1f\x02\x8f"
+xa+="\x35\xe3\x4d\x12\x35\x10\x3d\xf2\x33\x9b\x5b\x09\x9d\x3f\xe3\xe5"
+xa+="\x34\x6a\x69\x16\x42\xba\xc5\xb0\xbb\x03\xcd\x5d\x04\xd7\x56\x26"
+xa+="\x21\x49\x3f\xf1\xc4\x27\x3b\x6a\x45\xc5\xec\xb0\xb5\xe9\x08\xa0"
+xa+="\xf9\xf5\x62\x28\x2e\x85\x3e\xfc\x9a\x7e\xa1\x12\xe9\x47\x4f\xf6"
+xa+="\x94\x18\xf7\xc4\x7a\xe9\x66\xd4\x52\x4c\xa1\x70\x1b\x60\xa4\xbe"
+xa+="\x15\xc7\x5e\x27\xb4\x05\x80\x64\x68\x15\x6e\x02\xcb\xc5\x8f\xf4"
+xa+="\x66\x3c\x96\xac\x0c\x87\x36\x81\x35\xfa\x9b\x0b\xb6\x33\x7a\xe2"
+xa+="\x58\x52\x1d\x7d\x60\xc2\xa9\x1b\x4e\xd7\x72\xad\x65\x03\x40\x49"
+xa+="\x97\xf6\x79\x9d\xf6\x63\xa8\x99\x9c\xfd\x74\x7f\xa0\x67\xb9\x05"
+xa+="\x8a\xb3\x3b\xc1\x45\x94\x36\x6f\x28\xf5\xa2\xd9\x00\xb6\x46\x7a"
+
+# Z
+shared="0fdbd9a2 ebf50cba 489b4e4d 7cd6924a 42ee6324 a26988b2 22bc38e6 9cc445f1\n"
+shared+="eb47c1a4 62eca39f 39bcd7b8 19dede51 30bc38da ec99c16f 40a4e5c1 9c97b796\n"
+shared+="8b41823d a0650e37 13c73e6f 5f2a9dff 2e67dbf5 40ee66f4 e694c28f ba1d604b\n"
+shared+="71b57b8a eeb67a35 ba425a38 490b6fb9 f713db22 6f893b7a 8962f426 ba3046fb\n"
+shared+="cff8538c 16f583e8 ae947672 0ba55ff9 75b440d0 c4565cc7 5837d23a fea61a39\n"
+shared+="e0b7f6c4 e24c2154 7eb19fce f8dbed10 b06a9cce 971c0f0f ba7c1d5c b5035eaa\n"
+shared+="4fddd3ba fe757339 e3321e3e 4ebfe9e7 9c6c0401 4df63cf9 28d0a2c0 5b2d5521\n"
+shared+="030c35f1 c84c97fe 64cad509 8012a003 d52d24c4 1a1f9348 b7575251 3facb02f\n"
+
+# OI
+otherinfo="\xa1\xb2\xc3\xd4\xe5\x43\x41\x56\x53\x69\x64\x0d\x64\xc1\xb2"
+otherinfo+="\x33\x61\xb2\x61\xde\x78\x68\x8e\xa8\x65\xfc\xff\x11\x3c\x84"
+
+# DKM
+derived="8284e313 02c8a26b 393ec52d 9f9e0882\n"
+
+pcreate_key "-e $prime" user dh:prime @s
+expect_keyid primeid
+
+pcreate_key "-e $xa" user dh:xa @s
+expect_keyid xaid
+
+pcreate_key "-e $private" user dh:private @s
+expect_keyid privateid
+
+marker "COMPUTE DH SHARED SECRET"
+dh_compute $privateid $primeid $xaid
+expect_payload payload $shared
+
+marker "COMPUTE DERIVED KEY FROM DH SHARED SECRET (SHA-256)"
+dh_compute $privateid $primeid $xaid 16 "kdf_ctr(sha256)" "$(echo -e -n $otherinfo)"
+expect_payload payload $derived
+
 echo "++++ FINISHED TEST: $result" >>$OUTPUTFILE
 
 # --- then report the results in the database ---
-- 
2.7.4

Re: [PATCH] DH support: add KDF handling support

[Mat Martineau]

Stephan,

On Tue, 12 Jul 2016, Stephan Mueller wrote:

> Hi Mat, David,
>
> During the development of this patch, I saw that the test
> framework seems to be broken: when I change the expected
> values by one bit, the test framework will still mark the
> received result as PASS even though the returned data does
> not match the expected data.

I tracked this down to the expect_payload function in the test framework, 
which does not properly handle multiline strings. I'll send a patch that 
adds a new expect_multiline function that should handle multiline output 
correctly.

>
> ---8<----
>
> Add the interface logic to support DH with KDF handling support.
>
> The dh_compute code now allows the following options:
>
> - no KDF support / output of raw DH shared secret:
>  dh_compute <private> <prime> <base>
>
> - KDF support without "other information" string:
>  dh_compute <private> <prime> <base> <output length> <KDF type>

Why is <output length> needed? Can it be determined from <KDF type>?

>
> - KDF support with "other information string:
>  dh_compute <private> <prime> <base> <output length> <KDF type> <OI
>  string>
>
> The test to verify the code is based on a test vector used for the CAVS
> testing of SP800-56A.
>
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
> keyctl.c                                 | 14 +++++-
> keyutils.c                               | 48 ++++++++++++++++++
> keyutils.h                               | 13 +++++
> tests/keyctl/dh_compute/valid/runtest.sh | 83 ++++++++++++++++++++++++++++++++
> 4 files changed, 156 insertions(+), 2 deletions(-)
>
> diff --git a/keyctl.c b/keyctl.c
> index edb03de..32478b3 100644
> --- a/keyctl.c
> +++ b/keyctl.c
> @@ -1638,14 +1638,24 @@ static void act_keyctl_dh_compute(int argc, char *argv[])
> 	char *p;
> 	int ret, sep, col;
>
> -	if (argc != 4)
> +	if (argc != 4 && argc != 6 && argc != 7)
> 		format();
>
> 	private = get_key_id(argv[1]);
> 	prime = get_key_id(argv[2]);
> 	base = get_key_id(argv[3]);
>
> -	ret = keyctl_dh_compute_alloc(private, prime, base, &buffer);
> +	if (argc == 4)
> +		ret = keyctl_dh_compute_alloc(private, prime, base, &buffer);
> +	else if (argc == 6)
> +		ret = keyctl_dh_compute_kdf(private, prime, base, argv[4],
> +					    argv[5], NULL, &buffer);
> +	else if (argc == 7)
> +		ret = keyctl_dh_compute_kdf(private, prime, base, argv[4],
> +					    argv[5], argv[6], &buffer);
> +	else
> +		error("dh_compute: unknown number of arguments");
> +
> 	if (ret < 0)
> 		error("keyctl_dh_compute_alloc");
>
> diff --git a/keyutils.c b/keyutils.c
> index 2a69304..ffdd622 100644
> --- a/keyutils.c
> +++ b/keyutils.c
> @@ -386,6 +386,54 @@ int keyctl_dh_compute_alloc(key_serial_t private, key_serial_t prime,
> }
>
> /*
> + * fetch DH computation results processed by a KDF into an
> + * allocated buffer
> + * - resulting buffer has an extra NUL added to the end
> + * - returns count (not including extraneous NUL)
> + */
> +int keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
> +			  key_serial_t base, char *len, char *kdfname,
> +			  char *otherinfo, void **_buffer)

All of the other keyctl_* wrappers (without the _alloc) just do the 
syscall, with some packing/unpacking of structs where needed. The 
allocation behavior below is only found in the *_alloc functions (in the 
"utilities" section of keyutils.h) - I think it's worthwhile to have both 
keyctl_dh_compute_kdf() (for pre-allocated buffers) and 
keyctl_dh_compute_kdf_alloc().

> +{
> +	char *buf;
> +	unsigned long buflen;
> +	int ret;
> +	struct keyctl_dh_params params = { .private = private,
> +					   .prime = prime,
> +					   .base = base };
> +	struct keyctl_kdf_params kdfparams;
> +
> +	buflen = strtoul(len, NULL, 10);

The string to integer conversion needs to be done in 
act_keyctl_dh_compute(). Length can be passed in as a size_t if it's 
needed.

> +	if (buflen > KEYCTL_KDF_MAX_OUTPUTLEN)
> +		return -1;
> +
> +	buf = malloc(buflen + 1);

The other _alloc functions exist because the buffer length isn't known to 
the caller in advance. If the buffer length is known in advance, the 
caller should be allocating the buffer.

keyctl_dh_compute makes two syscalls: one to determine the buffer length, 
and one to do the DH calculations.

> +	if (!buf)
> +		return -1;
> +
> +	if (otherinfo) {
> +		kdfparams.kdfname = kdfname;
> +		kdfparams.kdfnamelen = strlen(kdfname);

If kdfname is a null-terminated string, then kdfnamelen seems 
unneccessary. I haven't reviewed the kernel side yet, but may comment more 
there. There are other examples of null-terminated string usage in the 
keyctl API, I'll comment more on this in the kernel patches.

> +		kdfparams.otherinfo = otherinfo;
> +		kdfparams.otherinfolen = strlen(otherinfo);

Same for otherinfolen.

> +	} else {
> +		kdfparams.kdfname = kdfname;
> +		kdfparams.kdfnamelen = strlen(kdfname);
> +		kdfparams.otherinfo = NULL;
> +		kdfparams.otherinfolen = 0;
> +	}
> +	ret = keyctl(KEYCTL_DH_COMPUTE, &params, buf, buflen, &kdfparams);
> +	if (ret < 0) {
> +		free(buf);
> +		return -1;
> +	}
> +
> +	buf[ret] = 0;
> +	*_buffer = buf;
> +	return ret;
> +}
> +
> +/*
>  * Depth-first recursively apply a function over a keyring tree
>  */
> static int recursive_key_scan_aux(key_serial_t parent, key_serial_t key,
> diff --git a/keyutils.h b/keyutils.h
> index b321aa8..5026270 100644
> --- a/keyutils.h
> +++ b/keyutils.h
> @@ -108,6 +108,16 @@ struct keyctl_dh_params {
> 	key_serial_t base;
> };
>
> +struct keyctl_kdf_params {
> +#define KEYCTL_KDF_MAX_OUTPUTLEN        1024    /* max length of KDF output */
> +#define KEYCTL_KDF_MAX_STRING_LEN       64      /* maximum length of strings */

It's better to leave this information out of the userspace as it may 
change depending on the kernel version. Let the kernel return an error if 
the lengths exceed limits.

> +	char *kdfname;
> +	uint32_t kdfnamelen;
> +	char *otherinfo;
> +	uint32_t otherinfolen;
> +	uint32_t flags;
> +};
> +
> /*
>  * syscall wrappers
>  */
> @@ -172,6 +182,9 @@ extern int keyctl_read_alloc(key_serial_t id, void **_buffer);
> extern int keyctl_get_security_alloc(key_serial_t id, char **_buffer);
> extern int keyctl_dh_compute_alloc(key_serial_t private, key_serial_t prime,
> 				   key_serial_t base, void **_buffer);
> +int keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
> +			  key_serial_t base, char *len, char *kdfname,
> +			  char *otherinfo, void **_buffer);
>
> typedef int (*recursive_key_scanner_t)(key_serial_t parent, key_serial_t key,
> 				       char *desc, int desc_len, void *data);
> diff --git a/tests/keyctl/dh_compute/valid/runtest.sh b/tests/keyctl/dh_compute/valid/runtest.sh
> index 40ec387..1c77268 100644
> --- a/tests/keyctl/dh_compute/valid/runtest.sh
> +++ b/tests/keyctl/dh_compute/valid/runtest.sh
> @@ -80,6 +80,89 @@ marker "COMPUTE DH PUBLIC KEY"
> dh_compute $privateid $primeid $generatorid
> expect_payload payload $public
>
> +
> +################################################################
> +# Testing DH compute with KDF according to SP800-56A
> +#
> +# test vectors from http://csrc.nist.gov/groups/STM/cavp/documents/keymgmt/KASTestVectorsFFC2014.zip
> +################################################################
> +
> +# SHA-256
> +
> +# XephemCAVS
> +private="\x81\xb2\xc6\x5f\x5c\xba\xc0\x0b\x13\x53\xac\x38\xbd\x77\xa2\x5a"
> +private+="\x86\x50\xed\x48\x5e\x41\x3e\xac\x1d\x6c\x48\x85"
> +
> +# P
> +prime="\xa3\xcc\x62\x23\xe5\x0c\x6e\x3f\x7b\xb0\x58\x1d\xcb\x9e\x9f\xf0"
> +prime+="\x2c\x58\x07\x68\x32\x8a\x15\x20\x7b\x1c\x32\x31\x7f\xb7\x84\x96"
> +prime+="\x81\x5e\x3c\xf7\xf9\xd0\x9c\xcb\x9f\xa8\x40\xff\x47\x98\x51\x1a"
> +prime+="\x17\xb5\x59\x28\x72\x1e\x5d\xfb\xcc\xc5\x41\x47\xe0\xf0\x5f\x85"
> +prime+="\xb3\xac\x41\x0b\x6a\xe3\xf5\x9b\x79\x6f\x3f\xea\xc7\xfc\x52\x49"
> +prime+="\x21\x7e\xb2\xa0\x45\x88\x29\x3a\x5a\xde\x22\x78\x79\xf4\x6c\xeb"
> +prime+="\x56\x45\x7b\x5c\x43\x12\x93\xe5\xe1\x04\xd1\xb9\x64\xbd\x2c\xdf"
> +prime+="\xde\xff\xa0\x40\x49\xa9\x1e\x67\xee\x8c\x86\xe9\x44\xf0\x4f\x94"
> +prime+="\x4a\x30\xe3\x61\xf8\xd1\x5d\x17\xe5\x01\x0c\xab\xb4\xef\x40\xc0"
> +prime+="\xeb\xa5\xf4\xa2\x52\xd4\xfd\x6c\xf9\xda\xe6\x0e\x86\xe4\xb3\x00"
> +prime+="\x9b\x1d\xfc\x92\x66\x70\x35\x72\x61\x58\x7a\xd0\x5c\x00\xa6\xc6"
> +prime+="\xf0\x10\x6c\xec\x8f\xc5\x91\x31\x51\x50\x84\xa8\x70\x59\x41\x65"
> +prime+="\xb4\x93\x90\xdb\x2d\x00\xe7\x53\x8f\x23\x0d\x53\x2f\x4a\x4e\xca"
> +prime+="\x83\x09\xd7\x07\xc0\xb3\x83\x5c\xee\x04\xf3\xca\x55\x8a\x22\xc6"
> +prime+="\xb5\x20\xfe\x25\xde\x6f\xfa\x90\xef\xda\x49\x27\xd0\x18\x59\x4c"
> +prime+="\x0c\x0b\x77\x06\x73\x93\xb7\xf1\xe0\xfc\x7c\xf2\x16\xaf\xf3\x9f"
> +
> +# YephemIUT
> +xa="\x9a\x70\x82\x2d\x3f\x06\x12\x3d\x0e\x51\x8e\xe1\x16\x51\xe5\xf6"
> +xa+="\xb1\x19\xdc\x3b\x97\xd5\xb1\xc0\xa2\xa6\xf6\xde\x94\x25\x64\xba"
> +xa+="\x10\x06\x1e\xec\xde\xb7\x36\x9c\xa5\x37\x49\x9e\x04\xb0\x36\xe9"
> +xa+="\x7f\x44\x5a\x95\x6f\x63\x69\xae\x6e\x63\xfd\x27\xea\xe3\xe3\x47"
> +xa+="\x85\x54\x47\xd3\xba\xc1\xc6\x0c\x10\xe7\x35\x07\x72\xc6\xc0\xc6"
> +xa+="\xfb\xf9\xca\x3e\x38\xf0\xe8\x65\x88\x25\xd3\xb2\x0f\x1f\x02\x8f"
> +xa+="\x35\xe3\x4d\x12\x35\x10\x3d\xf2\x33\x9b\x5b\x09\x9d\x3f\xe3\xe5"
> +xa+="\x34\x6a\x69\x16\x42\xba\xc5\xb0\xbb\x03\xcd\x5d\x04\xd7\x56\x26"
> +xa+="\x21\x49\x3f\xf1\xc4\x27\x3b\x6a\x45\xc5\xec\xb0\xb5\xe9\x08\xa0"
> +xa+="\xf9\xf5\x62\x28\x2e\x85\x3e\xfc\x9a\x7e\xa1\x12\xe9\x47\x4f\xf6"
> +xa+="\x94\x18\xf7\xc4\x7a\xe9\x66\xd4\x52\x4c\xa1\x70\x1b\x60\xa4\xbe"
> +xa+="\x15\xc7\x5e\x27\xb4\x05\x80\x64\x68\x15\x6e\x02\xcb\xc5\x8f\xf4"
> +xa+="\x66\x3c\x96\xac\x0c\x87\x36\x81\x35\xfa\x9b\x0b\xb6\x33\x7a\xe2"
> +xa+="\x58\x52\x1d\x7d\x60\xc2\xa9\x1b\x4e\xd7\x72\xad\x65\x03\x40\x49"
> +xa+="\x97\xf6\x79\x9d\xf6\x63\xa8\x99\x9c\xfd\x74\x7f\xa0\x67\xb9\x05"
> +xa+="\x8a\xb3\x3b\xc1\x45\x94\x36\x6f\x28\xf5\xa2\xd9\x00\xb6\x46\x7a"
> +
> +# Z
> +shared="0fdbd9a2 ebf50cba 489b4e4d 7cd6924a 42ee6324 a26988b2 22bc38e6 9cc445f1\n"
> +shared+="eb47c1a4 62eca39f 39bcd7b8 19dede51 30bc38da ec99c16f 40a4e5c1 9c97b796\n"
> +shared+="8b41823d a0650e37 13c73e6f 5f2a9dff 2e67dbf5 40ee66f4 e694c28f ba1d604b\n"
> +shared+="71b57b8a eeb67a35 ba425a38 490b6fb9 f713db22 6f893b7a 8962f426 ba3046fb\n"
> +shared+="cff8538c 16f583e8 ae947672 0ba55ff9 75b440d0 c4565cc7 5837d23a fea61a39\n"
> +shared+="e0b7f6c4 e24c2154 7eb19fce f8dbed10 b06a9cce 971c0f0f ba7c1d5c b5035eaa\n"
> +shared+="4fddd3ba fe757339 e3321e3e 4ebfe9e7 9c6c0401 4df63cf9 28d0a2c0 5b2d5521\n"
> +shared+="030c35f1 c84c97fe 64cad509 8012a003 d52d24c4 1a1f9348 b7575251 3facb02f\n"
> +
> +# OI
> +otherinfo="\xa1\xb2\xc3\xd4\xe5\x43\x41\x56\x53\x69\x64\x0d\x64\xc1\xb2"
> +otherinfo+="\x33\x61\xb2\x61\xde\x78\x68\x8e\xa8\x65\xfc\xff\x11\x3c\x84"
> +
> +# DKM
> +derived="8284e313 02c8a26b 393ec52d 9f9e0882\n"
> +
> +pcreate_key "-e $prime" user dh:prime @s
> +expect_keyid primeid
> +
> +pcreate_key "-e $xa" user dh:xa @s
> +expect_keyid xaid
> +
> +pcreate_key "-e $private" user dh:private @s
> +expect_keyid privateid
> +
> +marker "COMPUTE DH SHARED SECRET"
> +dh_compute $privateid $primeid $xaid
> +expect_payload payload $shared

As I mentioned at the top, we'll need an 'expect_multiline' function that 
handles values like $shared.

> +
> +marker "COMPUTE DERIVED KEY FROM DH SHARED SECRET (SHA-256)"
> +dh_compute $privateid $primeid $xaid 16 "kdf_ctr(sha256)" "$(echo -e -n $otherinfo)"
> +expect_payload payload $derived

Have you checked that expect_payload works correctly in this case? Seems 
like it should, since the output fits on one line.

> +
> echo "++++ FINISHED TEST: $result" >>$OUTPUTFILE
>
> # --- then report the results in the database ---
> -- 
> 2.7.4
>
>
>

--
Mat Martineau
Intel OTC

Re: [PATCH] DH support: add KDF handling support

[Stephan Mueller]

Am Mittwoch, 13. Juli 2016, 16:17:12 schrieb Mat Martineau:

Hi Mat,
> 
> > ---8<----
> > 
> > Add the interface logic to support DH with KDF handling support.
> > 
> > The dh_compute code now allows the following options:
> > 
> > - no KDF support / output of raw DH shared secret:
> >  dh_compute <private> <prime> <base>
> > 
> > - KDF support without "other information" string:
> >  dh_compute <private> <prime> <base> <output length> <KDF type>
> 
> Why is <output length> needed? Can it be determined from <KDF type>?

The KDF can be considered as a random number generator that is seeded by the 
shared secret. I.e. it can produce arbitrary string lengths. There is no 
default string length for any given KDF.

Note, as shared secrets potentially post-processed by a KDF usually are again 
used as key or data encryption keys, they need to be truncated/expanded to a 
specific length anyway. A KDF inherently provides the truncation support to 
any arbitrary length. Thus, I would think that the caller needs to provide 
that length but does not need to truncate the output itself.
> 
> > - KDF support with "other information string:
> >  dh_compute <private> <prime> <base> <output length> <KDF type> <OI
> >  string>
> > 
> > The test to verify the code is based on a test vector used for the CAVS
> > testing of SP800-56A.
> > 
> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > ---
> > keyctl.c                                 | 14 +++++-
> > keyutils.c                               | 48 ++++++++++++++++++
> > keyutils.h                               | 13 +++++
> > tests/keyctl/dh_compute/valid/runtest.sh | 83
> > ++++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 2
> > deletions(-)
> > 
> > diff --git a/keyctl.c b/keyctl.c
> > index edb03de..32478b3 100644
> > --- a/keyctl.c
> > +++ b/keyctl.c
> > @@ -1638,14 +1638,24 @@ static void act_keyctl_dh_compute(int argc, char
> > *argv[])> 
> > 	char *p;
> > 	int ret, sep, col;
> > 
> > -	if (argc != 4)
> > +	if (argc != 4 && argc != 6 && argc != 7)
> > 
> > 		format();
> > 	
> > 	private = get_key_id(argv[1]);
> > 	prime = get_key_id(argv[2]);
> > 	base = get_key_id(argv[3]);
> > 
> > -	ret = keyctl_dh_compute_alloc(private, prime, base, &buffer);
> > +	if (argc == 4)
> > +		ret = keyctl_dh_compute_alloc(private, prime, base, &buffer);
> > +	else if (argc == 6)
> > +		ret = keyctl_dh_compute_kdf(private, prime, base, argv[4],
> > +					    argv[5], NULL, &buffer);
> > +	else if (argc == 7)
> > +		ret = keyctl_dh_compute_kdf(private, prime, base, argv[4],
> > +					    argv[5], argv[6], &buffer);
> > +	else
> > +		error("dh_compute: unknown number of arguments");
> > +
> > 
> > 	if (ret < 0)
> > 	
> > 		error("keyctl_dh_compute_alloc");
> > 
> > diff --git a/keyutils.c b/keyutils.c
> > index 2a69304..ffdd622 100644
> > --- a/keyutils.c
> > +++ b/keyutils.c
> > @@ -386,6 +386,54 @@ int keyctl_dh_compute_alloc(key_serial_t private,
> > key_serial_t prime, }
> > 
> > /*
> > + * fetch DH computation results processed by a KDF into an
> > + * allocated buffer
> > + * - resulting buffer has an extra NUL added to the end
> > + * - returns count (not including extraneous NUL)
> > + */
> > +int keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
> > +			  key_serial_t base, char *len, char *kdfname,
> > +			  char *otherinfo, void **_buffer)
> 
> All of the other keyctl_* wrappers (without the _alloc) just do the
> syscall, with some packing/unpacking of structs where needed. The
> allocation behavior below is only found in the *_alloc functions (in the
> "utilities" section of keyutils.h) - I think it's worthwhile to have both
> keyctl_dh_compute_kdf() (for pre-allocated buffers) and
> keyctl_dh_compute_kdf_alloc().

Thank you for the note. I will change the code accordingly.

Though, shall I stuff the wrapper code back into the existing dh_compute 
functions or can I leave them as separate functions?
> 
> > +{
> > +	char *buf;
> > +	unsigned long buflen;
> > +	int ret;
> > +	struct keyctl_dh_params params = { .private = private,
> > +					   .prime = prime,
> > +					   .base = base };
> > +	struct keyctl_kdf_params kdfparams;
> > +
> > +	buflen = strtoul(len, NULL, 10);
> 
> The string to integer conversion needs to be done in
> act_keyctl_dh_compute(). Length can be passed in as a size_t if it's
> needed.
> 

Ok, will do.

> > +	if (buflen > KEYCTL_KDF_MAX_OUTPUTLEN)
> > +		return -1;
> > +
> > +	buf = malloc(buflen + 1);
> 
> The other _alloc functions exist because the buffer length isn't known to
> the caller in advance. If the buffer length is known in advance, the
> caller should be allocating the buffer.

With the implementation of the _alloc and "non-alloc" function, I would assume 
that this comment should be covered.
> 
> keyctl_dh_compute makes two syscalls: one to determine the buffer length,
> and one to do the DH calculations.

I am aware of that, but as explained above, in case of a KDF, there is no 
specifically required buffer size. Any buffer size the caller provides is 
supported and will be filled with data. Thus, in the KDF case, we can skip the 
first call.
> 
> > +	if (!buf)
> > +		return -1;
> > +
> > +	if (otherinfo) {
> > +		kdfparams.kdfname = kdfname;
> > +		kdfparams.kdfnamelen = strlen(kdfname);
> 
> If kdfname is a null-terminated string, then kdfnamelen seems
> unneccessary. I haven't reviewed the kernel side yet, but may comment more
> there. There are other examples of null-terminated string usage in the
> keyctl API, I'll comment more on this in the kernel patches.

Please let me know on the kernel side, how you would like to handle it. Note, 
we only need that length information to ensure copy_from_user copies a maximum 
buffer length anyway.

The string is used to find the proper crypto implementation via the kernel 
crypto API. The kernel crypto API will limit the maximum number of parsed 
bytes to 64 already.
> 
> > +		kdfparams.otherinfo = otherinfo;
> > +		kdfparams.otherinfolen = strlen(otherinfo);
> 
> Same for otherinfolen.

Sure. But note, otherinfo must support binary data. Thus, I think that the 
kernel side must support a length parameter.

Here, for user space keyctl support, surely we have some limitation regarding 
the support for binary data (i.e. the binary data must not contain a \0 as 
strlen would return the wrong size).
> 
> > +	} else {
> > +		kdfparams.kdfname = kdfname;
> > +		kdfparams.kdfnamelen = strlen(kdfname);
> > +		kdfparams.otherinfo = NULL;
> > +		kdfparams.otherinfolen = 0;
> > +	}
> > +	ret = keyctl(KEYCTL_DH_COMPUTE, &params, buf, buflen, &kdfparams);
> > +	if (ret < 0) {
> > +		free(buf);
> > +		return -1;
> > +	}
> > +
> > +	buf[ret] = 0;
> > +	*_buffer = buf;
> > +	return ret;
> > +}
> > +
> > +/*
> > 
> >  * Depth-first recursively apply a function over a keyring tree
> >  */
> > 
> > static int recursive_key_scan_aux(key_serial_t parent, key_serial_t key,
> > diff --git a/keyutils.h b/keyutils.h
> > index b321aa8..5026270 100644
> > --- a/keyutils.h
> > +++ b/keyutils.h
> > @@ -108,6 +108,16 @@ struct keyctl_dh_params {
> > 
> > 	key_serial_t base;
> > 
> > };
> > 
> > +struct keyctl_kdf_params {
> > +#define KEYCTL_KDF_MAX_OUTPUTLEN        1024    /* max length of KDF
> > output */ +#define KEYCTL_KDF_MAX_STRING_LEN       64      /* maximum
> > length of strings */
> It's better to leave this information out of the userspace as it may
> change depending on the kernel version. Let the kernel return an error if
> the lengths exceed limits.

Will do.
> 
> > +	char *kdfname;
> > +	uint32_t kdfnamelen;
> > +	char *otherinfo;
> > +	uint32_t otherinfolen;
> > +	uint32_t flags;
> > +};
> > +
> > /*
> > 
> >  * syscall wrappers
> >  */
> > 
> > @@ -172,6 +182,9 @@ extern int keyctl_read_alloc(key_serial_t id, void
> > **_buffer); extern int keyctl_get_security_alloc(key_serial_t id, char
> > **_buffer); extern int keyctl_dh_compute_alloc(key_serial_t private,
> > key_serial_t prime,> 
> > 				   key_serial_t base, void **_buffer);
> > 
> > +int keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
> > +			  key_serial_t base, char *len, char *kdfname,
> > +			  char *otherinfo, void **_buffer);
> > 
> > typedef int (*recursive_key_scanner_t)(key_serial_t parent, key_serial_t
> > key,> 
> > 				       char *desc, int desc_len, void *data);
> > 
> > diff --git a/tests/keyctl/dh_compute/valid/runtest.sh
> > b/tests/keyctl/dh_compute/valid/runtest.sh index 40ec387..1c77268 100644
> > --- a/tests/keyctl/dh_compute/valid/runtest.sh
> > +++ b/tests/keyctl/dh_compute/valid/runtest.sh
> > @@ -80,6 +80,89 @@ marker "COMPUTE DH PUBLIC KEY"
> > dh_compute $privateid $primeid $generatorid
> > expect_payload payload $public
> > 
> > +
> > +################################################################
> > +# Testing DH compute with KDF according to SP800-56A
> > +#
> > +# test vectors from
> > http://csrc.nist.gov/groups/STM/cavp/documents/keymgmt/KASTestVectorsFFC2
> > 014.zip +################################################################
> > +
> > +# SHA-256
> > +
> > +# XephemCAVS
> > +private="\x81\xb2\xc6\x5f\x5c\xba\xc0\x0b\x13\x53\xac\x38\xbd\x77\xa2\x5a
> > "
> > +private+="\x86\x50\xed\x48\x5e\x41\x3e\xac\x1d\x6c\x48\x85"
> > +
> > +# P
> > +prime="\xa3\xcc\x62\x23\xe5\x0c\x6e\x3f\x7b\xb0\x58\x1d\xcb\x9e\x9f\xf0"
> > +prime+="\x2c\x58\x07\x68\x32\x8a\x15\x20\x7b\x1c\x32\x31\x7f\xb7\x84\x96"
> > +prime+="\x81\x5e\x3c\xf7\xf9\xd0\x9c\xcb\x9f\xa8\x40\xff\x47\x98\x51\x1a"
> > +prime+="\x17\xb5\x59\x28\x72\x1e\x5d\xfb\xcc\xc5\x41\x47\xe0\xf0\x5f\x85"
> > +prime+="\xb3\xac\x41\x0b\x6a\xe3\xf5\x9b\x79\x6f\x3f\xea\xc7\xfc\x52\x49"
> > +prime+="\x21\x7e\xb2\xa0\x45\x88\x29\x3a\x5a\xde\x22\x78\x79\xf4\x6c\xeb"
> > +prime+="\x56\x45\x7b\x5c\x43\x12\x93\xe5\xe1\x04\xd1\xb9\x64\xbd\x2c\xdf"
> > +prime+="\xde\xff\xa0\x40\x49\xa9\x1e\x67\xee\x8c\x86\xe9\x44\xf0\x4f\x94"
> > +prime+="\x4a\x30\xe3\x61\xf8\xd1\x5d\x17\xe5\x01\x0c\xab\xb4\xef\x40\xc0"
> > +prime+="\xeb\xa5\xf4\xa2\x52\xd4\xfd\x6c\xf9\xda\xe6\x0e\x86\xe4\xb3\x00"
> > +prime+="\x9b\x1d\xfc\x92\x66\x70\x35\x72\x61\x58\x7a\xd0\x5c\x00\xa6\xc6"
> > +prime+="\xf0\x10\x6c\xec\x8f\xc5\x91\x31\x51\x50\x84\xa8\x70\x59\x41\x65"
> > +prime+="\xb4\x93\x90\xdb\x2d\x00\xe7\x53\x8f\x23\x0d\x53\x2f\x4a\x4e\xca"
> > +prime+="\x83\x09\xd7\x07\xc0\xb3\x83\x5c\xee\x04\xf3\xca\x55\x8a\x22\xc6"
> > +prime+="\xb5\x20\xfe\x25\xde\x6f\xfa\x90\xef\xda\x49\x27\xd0\x18\x59\x4c"
> > +prime+="\x0c\x0b\x77\x06\x73\x93\xb7\xf1\xe0\xfc\x7c\xf2\x16\xaf\xf3\x9f"
> > +
> > +# YephemIUT
> > +xa="\x9a\x70\x82\x2d\x3f\x06\x12\x3d\x0e\x51\x8e\xe1\x16\x51\xe5\xf6"
> > +xa+="\xb1\x19\xdc\x3b\x97\xd5\xb1\xc0\xa2\xa6\xf6\xde\x94\x25\x64\xba"
> > +xa+="\x10\x06\x1e\xec\xde\xb7\x36\x9c\xa5\x37\x49\x9e\x04\xb0\x36\xe9"
> > +xa+="\x7f\x44\x5a\x95\x6f\x63\x69\xae\x6e\x63\xfd\x27\xea\xe3\xe3\x47"
> > +xa+="\x85\x54\x47\xd3\xba\xc1\xc6\x0c\x10\xe7\x35\x07\x72\xc6\xc0\xc6"
> > +xa+="\xfb\xf9\xca\x3e\x38\xf0\xe8\x65\x88\x25\xd3\xb2\x0f\x1f\x02\x8f"
> > +xa+="\x35\xe3\x4d\x12\x35\x10\x3d\xf2\x33\x9b\x5b\x09\x9d\x3f\xe3\xe5"
> > +xa+="\x34\x6a\x69\x16\x42\xba\xc5\xb0\xbb\x03\xcd\x5d\x04\xd7\x56\x26"
> > +xa+="\x21\x49\x3f\xf1\xc4\x27\x3b\x6a\x45\xc5\xec\xb0\xb5\xe9\x08\xa0"
> > +xa+="\xf9\xf5\x62\x28\x2e\x85\x3e\xfc\x9a\x7e\xa1\x12\xe9\x47\x4f\xf6"
> > +xa+="\x94\x18\xf7\xc4\x7a\xe9\x66\xd4\x52\x4c\xa1\x70\x1b\x60\xa4\xbe"
> > +xa+="\x15\xc7\x5e\x27\xb4\x05\x80\x64\x68\x15\x6e\x02\xcb\xc5\x8f\xf4"
> > +xa+="\x66\x3c\x96\xac\x0c\x87\x36\x81\x35\xfa\x9b\x0b\xb6\x33\x7a\xe2"
> > +xa+="\x58\x52\x1d\x7d\x60\xc2\xa9\x1b\x4e\xd7\x72\xad\x65\x03\x40\x49"
> > +xa+="\x97\xf6\x79\x9d\xf6\x63\xa8\x99\x9c\xfd\x74\x7f\xa0\x67\xb9\x05"
> > +xa+="\x8a\xb3\x3b\xc1\x45\x94\x36\x6f\x28\xf5\xa2\xd9\x00\xb6\x46\x7a"
> > +
> > +# Z
> > +shared="0fdbd9a2 ebf50cba 489b4e4d 7cd6924a 42ee6324 a26988b2 22bc38e6
> > 9cc445f1\n" +shared+="eb47c1a4 62eca39f 39bcd7b8 19dede51 30bc38da
> > ec99c16f 40a4e5c1 9c97b796\n" +shared+="8b41823d a0650e37 13c73e6f
> > 5f2a9dff 2e67dbf5 40ee66f4 e694c28f ba1d604b\n" +shared+="71b57b8a
> > eeb67a35 ba425a38 490b6fb9 f713db22 6f893b7a 8962f426 ba3046fb\n"
> > +shared+="cff8538c 16f583e8 ae947672 0ba55ff9 75b440d0 c4565cc7 5837d23a
> > fea61a39\n" +shared+="e0b7f6c4 e24c2154 7eb19fce f8dbed10 b06a9cce
> > 971c0f0f ba7c1d5c b5035eaa\n" +shared+="4fddd3ba fe757339 e3321e3e
> > 4ebfe9e7 9c6c0401 4df63cf9 28d0a2c0 5b2d5521\n" +shared+="030c35f1
> > c84c97fe 64cad509 8012a003 d52d24c4 1a1f9348 b7575251 3facb02f\n" +
> > +# OI
> > +otherinfo="\xa1\xb2\xc3\xd4\xe5\x43\x41\x56\x53\x69\x64\x0d\x64\xc1\xb2"
> > +otherinfo+="\x33\x61\xb2\x61\xde\x78\x68\x8e\xa8\x65\xfc\xff\x11\x3c\x84"
> > +
> > +# DKM
> > +derived="8284e313 02c8a26b 393ec52d 9f9e0882\n"
> > +
> > +pcreate_key "-e $prime" user dh:prime @s
> > +expect_keyid primeid
> > +
> > +pcreate_key "-e $xa" user dh:xa @s
> > +expect_keyid xaid
> > +
> > +pcreate_key "-e $private" user dh:private @s
> > +expect_keyid privateid
> > +
> > +marker "COMPUTE DH SHARED SECRET"
> > +dh_compute $privateid $primeid $xaid
> > +expect_payload payload $shared
> 
> As I mentioned at the top, we'll need an 'expect_multiline' function that
> handles values like $shared.

Ok.
> 
> > +
> > +marker "COMPUTE DERIVED KEY FROM DH SHARED SECRET (SHA-256)"
> > +dh_compute $privateid $primeid $xaid 16 "kdf_ctr(sha256)" "$(echo -e -n
> > $otherinfo)" +expect_payload payload $derived
> 
> Have you checked that expect_payload works correctly in this case? Seems
> like it should, since the output fits on one line.

I just tested it and the code does NOT catch the error. I.e. when changing 
$derived, the harness still returns a PASS even though the returned data does 
not match the expected data.
> 
> > +
> > echo "++++ FINISHED TEST: $result" >>$OUTPUTFILE
> > 
> > # --- then report the results in the database ---
> 
> --
> Mat Martineau
> Intel OTC


Ciao
Stephan

Re: [PATCH] DH support: add KDF handling support

[Jeffrey Walton]

> Note, as shared secrets potentially post-processed by a KDF usually are again
> used as key or data encryption keys, they need to be truncated/expanded to a
> specific length anyway. A KDF inherently provides the truncation support to
> any arbitrary length. Thus, I would think that the caller needs to provide
> that length but does not need to truncate the output itself.

As far as I know, there's no reduction in proof that a truncated hash
is as secure as the non-truncated one. One of the reasons to provide
the output length as a security parameter is to help avoid truncation
and related hash output attacks.

Also see Kelsey's work on the subject;
http://www.google.com/search?q=nist+kelsey+truncated+hash.

Jeff

Re: [PATCH] DH support: add KDF handling support

[Stephan Mueller]

Am Donnerstag, 14. Juli 2016, 04:00:57 schrieb Jeffrey Walton:

Hi Jeffrey,

> > Note, as shared secrets potentially post-processed by a KDF usually are
> > again used as key or data encryption keys, they need to be
> > truncated/expanded to a specific length anyway. A KDF inherently provides
> > the truncation support to any arbitrary length. Thus, I would think that
> > the caller needs to provide that length but does not need to truncate the
> > output itself.
> 
> As far as I know, there's no reduction in proof that a truncated hash
> is as secure as the non-truncated one. One of the reasons to provide
> the output length as a security parameter is to help avoid truncation
> and related hash output attacks.
> 
> Also see Kelsey's work on the subject;
> http://www.google.com/search?q=nist+kelsey+truncated+hash.

I understand that point. However, a KDF is not just a simple hash or 
truncation thereof. Furthermore, Kelsey is part of the NIST team that also 
developed SP800-108 (the KDF definition). Furthermore, the authors of 
SP800-56A (in particular Allen Roginsky who I met personally a number of 
times) work closely with Kelsey too.

So, I would not think that applying the standard KDF operation which is 
intended to "right-size" the DH output is nothing we should worry about.

I would rather worry about the size of the private key in the DH operation. 
The private key should be as small as cryptographically possible (e.g. 224 or 
256 bits) instead of half of the DH public key minus one (what TLS does) due 
to the reduced number of Sopie-Germain primes that are available in the latter 
case.

Ciao
Stephan

Re: [PATCH] DH support: add KDF handling support

[Mat Martineau]

On Thu, 14 Jul 2016, Stephan Mueller wrote:

> Am Mittwoch, 13. Juli 2016, 16:17:12 schrieb Mat Martineau:
>
> Hi Mat,
>>
>>> ---8<----
>>>
>>> Add the interface logic to support DH with KDF handling support.
>>>
>>> The dh_compute code now allows the following options:
>>>
>>> - no KDF support / output of raw DH shared secret:
>>>  dh_compute <private> <prime> <base>
>>>
>>> - KDF support without "other information" string:
>>>  dh_compute <private> <prime> <base> <output length> <KDF type>
>>
>> Why is <output length> needed? Can it be determined from <KDF type>?
>
> The KDF can be considered as a random number generator that is seeded by the
> shared secret. I.e. it can produce arbitrary string lengths. There is no
> default string length for any given KDF.

Makes sense, thanks for the explanation.

> Note, as shared secrets potentially post-processed by a KDF usually are again
> used as key or data encryption keys, they need to be truncated/expanded to a
> specific length anyway. A KDF inherently provides the truncation support to
> any arbitrary length. Thus, I would think that the caller needs to provide
> that length but does not need to truncate the output itself.
>>
>>> - KDF support with "other information string:
>>>  dh_compute <private> <prime> <base> <output length> <KDF type> <OI
>>>  string>
>>>
>>> The test to verify the code is based on a test vector used for the CAVS
>>> testing of SP800-56A.
>>>
>>> Signed-off-by: Stephan Mueller <smueller@chronox.de>
>>> ---
>>> keyctl.c                                 | 14 +++++-
>>> keyutils.c                               | 48 ++++++++++++++++++
>>> keyutils.h                               | 13 +++++
>>> tests/keyctl/dh_compute/valid/runtest.sh | 83
>>> ++++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 2
>>> deletions(-)
>>>
>>> diff --git a/keyctl.c b/keyctl.c
>>> index edb03de..32478b3 100644
>>> --- a/keyctl.c
>>> +++ b/keyctl.c
>>> @@ -1638,14 +1638,24 @@ static void act_keyctl_dh_compute(int argc, char
>>> *argv[])>
>>> 	char *p;
>>> 	int ret, sep, col;
>>>
>>> -	if (argc != 4)
>>> +	if (argc != 4 && argc != 6 && argc != 7)
>>>
>>> 		format();
>>>
>>> 	private = get_key_id(argv[1]);
>>> 	prime = get_key_id(argv[2]);
>>> 	base = get_key_id(argv[3]);
>>>
>>> -	ret = keyctl_dh_compute_alloc(private, prime, base, &buffer);
>>> +	if (argc == 4)
>>> +		ret = keyctl_dh_compute_alloc(private, prime, base, &buffer);
>>> +	else if (argc == 6)
>>> +		ret = keyctl_dh_compute_kdf(private, prime, base, argv[4],
>>> +					    argv[5], NULL, &buffer);
>>> +	else if (argc == 7)
>>> +		ret = keyctl_dh_compute_kdf(private, prime, base, argv[4],
>>> +					    argv[5], argv[6], &buffer);
>>> +	else
>>> +		error("dh_compute: unknown number of arguments");
>>> +
>>>
>>> 	if (ret < 0)
>>>
>>> 		error("keyctl_dh_compute_alloc");
>>>
>>> diff --git a/keyutils.c b/keyutils.c
>>> index 2a69304..ffdd622 100644
>>> --- a/keyutils.c
>>> +++ b/keyutils.c
>>> @@ -386,6 +386,54 @@ int keyctl_dh_compute_alloc(key_serial_t private,
>>> key_serial_t prime, }
>>>
>>> /*
>>> + * fetch DH computation results processed by a KDF into an
>>> + * allocated buffer
>>> + * - resulting buffer has an extra NUL added to the end
>>> + * - returns count (not including extraneous NUL)
>>> + */
>>> +int keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
>>> +			  key_serial_t base, char *len, char *kdfname,
>>> +			  char *otherinfo, void **_buffer)
>>
>> All of the other keyctl_* wrappers (without the _alloc) just do the
>> syscall, with some packing/unpacking of structs where needed. The
>> allocation behavior below is only found in the *_alloc functions (in the
>> "utilities" section of keyutils.h) - I think it's worthwhile to have both
>> keyctl_dh_compute_kdf() (for pre-allocated buffers) and
>> keyctl_dh_compute_kdf_alloc().
>
> Thank you for the note. I will change the code accordingly.
>
> Though, shall I stuff the wrapper code back into the existing dh_compute
> functions or can I leave them as separate functions?

I'm not sure. In the existing code there's one keyctl wrapper per keyctl 
command. A combined wrapper would need some extra logic to decide 
whether kdfparams is passed in or not, which is different from existing 
code.

>>
>>> +{
>>> +	char *buf;
>>> +	unsigned long buflen;
>>> +	int ret;
>>> +	struct keyctl_dh_params params = { .private = private,
>>> +					   .prime = prime,
>>> +					   .base = base };
>>> +	struct keyctl_kdf_params kdfparams;
>>> +
>>> +	buflen = strtoul(len, NULL, 10);
>>
>> The string to integer conversion needs to be done in
>> act_keyctl_dh_compute(). Length can be passed in as a size_t if it's
>> needed.
>>
>
> Ok, will do.
>
>>> +	if (buflen > KEYCTL_KDF_MAX_OUTPUTLEN)
>>> +		return -1;
>>> +
>>> +	buf = malloc(buflen + 1);
>>
>> The other _alloc functions exist because the buffer length isn't known to
>> the caller in advance. If the buffer length is known in advance, the
>> caller should be allocating the buffer.
>
> With the implementation of the _alloc and "non-alloc" function, I would assume
> that this comment should be covered.
>>
>> keyctl_dh_compute makes two syscalls: one to determine the buffer length,
>> and one to do the DH calculations.
>
> I am aware of that, but as explained above, in case of a KDF, there is no
> specifically required buffer size. Any buffer size the caller provides is
> supported and will be filled with data. Thus, in the KDF case, we can skip the
> first call.

Ok.

>>
>>> +	if (!buf)
>>> +		return -1;
>>> +
>>> +	if (otherinfo) {
>>> +		kdfparams.kdfname = kdfname;
>>> +		kdfparams.kdfnamelen = strlen(kdfname);
>>
>> If kdfname is a null-terminated string, then kdfnamelen seems
>> unneccessary. I haven't reviewed the kernel side yet, but may comment more
>> there. There are other examples of null-terminated string usage in the
>> keyctl API, I'll comment more on this in the kernel patches.
>
> Please let me know on the kernel side, how you would like to handle it. Note,
> we only need that length information to ensure copy_from_user copies a maximum
> buffer length anyway.

I'll comment on that code as well, but look for use of strndup_user() in 
the kernel's keyctl.c for examples.

>
> The string is used to find the proper crypto implementation via the kernel
> crypto API. The kernel crypto API will limit the maximum number of parsed
> bytes to 64 already.
>>
>>> +		kdfparams.otherinfo = otherinfo;
>>> +		kdfparams.otherinfolen = strlen(otherinfo);
>>
>> Same for otherinfolen.
>
> Sure. But note, otherinfo must support binary data. Thus, I think that the
> kernel side must support a length parameter.
>
> Here, for user space keyctl support, surely we have some limitation regarding
> the support for binary data (i.e. the binary data must not contain a \0 as
> strlen would return the wrong size).

If there may be binary data, then a length is definitely required. Keep in 
mind that this code base is both for the keyctl command line tool and 
libkeyutils. This function may be called directly by other code, so binary 
data is just an array of bytes (including \0). To deal with binary data 
from the command line, look at "keyctl add" vs "keyctl padd". The first 
takes the key payload from a command line arg, the second accepts binary 
data from a pipe or redirect.

>>
>>> +	} else {
>>> +		kdfparams.kdfname = kdfname;
>>> +		kdfparams.kdfnamelen = strlen(kdfname);
>>> +		kdfparams.otherinfo = NULL;
>>> +		kdfparams.otherinfolen = 0;
>>> +	}
>>> +	ret = keyctl(KEYCTL_DH_COMPUTE, &params, buf, buflen, &kdfparams);
>>> +	if (ret < 0) {
>>> +		free(buf);
>>> +		return -1;
>>> +	}
>>> +
>>> +	buf[ret] = 0;
>>> +	*_buffer = buf;
>>> +	return ret;
>>> +}
>>> +
>>> +/*
>>>
>>>  * Depth-first recursively apply a function over a keyring tree
>>>  */
>>>
>>> static int recursive_key_scan_aux(key_serial_t parent, key_serial_t key,
>>> diff --git a/keyutils.h b/keyutils.h
>>> index b321aa8..5026270 100644
>>> --- a/keyutils.h
>>> +++ b/keyutils.h
>>> @@ -108,6 +108,16 @@ struct keyctl_dh_params {
>>>
>>> 	key_serial_t base;
>>>
>>> };
>>>
>>> +struct keyctl_kdf_params {
>>> +#define KEYCTL_KDF_MAX_OUTPUTLEN        1024    /* max length of KDF
>>> output */ +#define KEYCTL_KDF_MAX_STRING_LEN       64      /* maximum
>>> length of strings */
>> It's better to leave this information out of the userspace as it may
>> change depending on the kernel version. Let the kernel return an error if
>> the lengths exceed limits.
>
> Will do.
>>
>>> +	char *kdfname;
>>> +	uint32_t kdfnamelen;
>>> +	char *otherinfo;
>>> +	uint32_t otherinfolen;
>>> +	uint32_t flags;
>>> +};
>>> +
>>> /*
>>>
>>>  * syscall wrappers
>>>  */
>>>
>>> @@ -172,6 +182,9 @@ extern int keyctl_read_alloc(key_serial_t id, void
>>> **_buffer); extern int keyctl_get_security_alloc(key_serial_t id, char
>>> **_buffer); extern int keyctl_dh_compute_alloc(key_serial_t private,
>>> key_serial_t prime,>
>>> 				   key_serial_t base, void **_buffer);
>>>
>>> +int keyctl_dh_compute_kdf(key_serial_t private, key_serial_t prime,
>>> +			  key_serial_t base, char *len, char *kdfname,
>>> +			  char *otherinfo, void **_buffer);
>>>
>>> typedef int (*recursive_key_scanner_t)(key_serial_t parent, key_serial_t
>>> key,>
>>> 				       char *desc, int desc_len, void *data);
>>>
>>> diff --git a/tests/keyctl/dh_compute/valid/runtest.sh
>>> b/tests/keyctl/dh_compute/valid/runtest.sh index 40ec387..1c77268 100644
>>> --- a/tests/keyctl/dh_compute/valid/runtest.sh
>>> +++ b/tests/keyctl/dh_compute/valid/runtest.sh
>>> @@ -80,6 +80,89 @@ marker "COMPUTE DH PUBLIC KEY"
>>> dh_compute $privateid $primeid $generatorid
>>> expect_payload payload $public
>>>
>>> +
>>> +################################################################
>>> +# Testing DH compute with KDF according to SP800-56A
>>> +#
>>> +# test vectors from
>>> http://csrc.nist.gov/groups/STM/cavp/documents/keymgmt/KASTestVectorsFFC2
>>> 014.zip +################################################################
>>> +
>>> +# SHA-256
>>> +
>>> +# XephemCAVS
>>> +private="\x81\xb2\xc6\x5f\x5c\xba\xc0\x0b\x13\x53\xac\x38\xbd\x77\xa2\x5a
>>> "
>>> +private+="\x86\x50\xed\x48\x5e\x41\x3e\xac\x1d\x6c\x48\x85"
>>> +
>>> +# P
>>> +prime="\xa3\xcc\x62\x23\xe5\x0c\x6e\x3f\x7b\xb0\x58\x1d\xcb\x9e\x9f\xf0"
>>> +prime+="\x2c\x58\x07\x68\x32\x8a\x15\x20\x7b\x1c\x32\x31\x7f\xb7\x84\x96"
>>> +prime+="\x81\x5e\x3c\xf7\xf9\xd0\x9c\xcb\x9f\xa8\x40\xff\x47\x98\x51\x1a"
>>> +prime+="\x17\xb5\x59\x28\x72\x1e\x5d\xfb\xcc\xc5\x41\x47\xe0\xf0\x5f\x85"
>>> +prime+="\xb3\xac\x41\x0b\x6a\xe3\xf5\x9b\x79\x6f\x3f\xea\xc7\xfc\x52\x49"
>>> +prime+="\x21\x7e\xb2\xa0\x45\x88\x29\x3a\x5a\xde\x22\x78\x79\xf4\x6c\xeb"
>>> +prime+="\x56\x45\x7b\x5c\x43\x12\x93\xe5\xe1\x04\xd1\xb9\x64\xbd\x2c\xdf"
>>> +prime+="\xde\xff\xa0\x40\x49\xa9\x1e\x67\xee\x8c\x86\xe9\x44\xf0\x4f\x94"
>>> +prime+="\x4a\x30\xe3\x61\xf8\xd1\x5d\x17\xe5\x01\x0c\xab\xb4\xef\x40\xc0"
>>> +prime+="\xeb\xa5\xf4\xa2\x52\xd4\xfd\x6c\xf9\xda\xe6\x0e\x86\xe4\xb3\x00"
>>> +prime+="\x9b\x1d\xfc\x92\x66\x70\x35\x72\x61\x58\x7a\xd0\x5c\x00\xa6\xc6"
>>> +prime+="\xf0\x10\x6c\xec\x8f\xc5\x91\x31\x51\x50\x84\xa8\x70\x59\x41\x65"
>>> +prime+="\xb4\x93\x90\xdb\x2d\x00\xe7\x53\x8f\x23\x0d\x53\x2f\x4a\x4e\xca"
>>> +prime+="\x83\x09\xd7\x07\xc0\xb3\x83\x5c\xee\x04\xf3\xca\x55\x8a\x22\xc6"
>>> +prime+="\xb5\x20\xfe\x25\xde\x6f\xfa\x90\xef\xda\x49\x27\xd0\x18\x59\x4c"
>>> +prime+="\x0c\x0b\x77\x06\x73\x93\xb7\xf1\xe0\xfc\x7c\xf2\x16\xaf\xf3\x9f"
>>> +
>>> +# YephemIUT
>>> +xa="\x9a\x70\x82\x2d\x3f\x06\x12\x3d\x0e\x51\x8e\xe1\x16\x51\xe5\xf6"
>>> +xa+="\xb1\x19\xdc\x3b\x97\xd5\xb1\xc0\xa2\xa6\xf6\xde\x94\x25\x64\xba"
>>> +xa+="\x10\x06\x1e\xec\xde\xb7\x36\x9c\xa5\x37\x49\x9e\x04\xb0\x36\xe9"
>>> +xa+="\x7f\x44\x5a\x95\x6f\x63\x69\xae\x6e\x63\xfd\x27\xea\xe3\xe3\x47"
>>> +xa+="\x85\x54\x47\xd3\xba\xc1\xc6\x0c\x10\xe7\x35\x07\x72\xc6\xc0\xc6"
>>> +xa+="\xfb\xf9\xca\x3e\x38\xf0\xe8\x65\x88\x25\xd3\xb2\x0f\x1f\x02\x8f"
>>> +xa+="\x35\xe3\x4d\x12\x35\x10\x3d\xf2\x33\x9b\x5b\x09\x9d\x3f\xe3\xe5"
>>> +xa+="\x34\x6a\x69\x16\x42\xba\xc5\xb0\xbb\x03\xcd\x5d\x04\xd7\x56\x26"
>>> +xa+="\x21\x49\x3f\xf1\xc4\x27\x3b\x6a\x45\xc5\xec\xb0\xb5\xe9\x08\xa0"
>>> +xa+="\xf9\xf5\x62\x28\x2e\x85\x3e\xfc\x9a\x7e\xa1\x12\xe9\x47\x4f\xf6"
>>> +xa+="\x94\x18\xf7\xc4\x7a\xe9\x66\xd4\x52\x4c\xa1\x70\x1b\x60\xa4\xbe"
>>> +xa+="\x15\xc7\x5e\x27\xb4\x05\x80\x64\x68\x15\x6e\x02\xcb\xc5\x8f\xf4"
>>> +xa+="\x66\x3c\x96\xac\x0c\x87\x36\x81\x35\xfa\x9b\x0b\xb6\x33\x7a\xe2"
>>> +xa+="\x58\x52\x1d\x7d\x60\xc2\xa9\x1b\x4e\xd7\x72\xad\x65\x03\x40\x49"
>>> +xa+="\x97\xf6\x79\x9d\xf6\x63\xa8\x99\x9c\xfd\x74\x7f\xa0\x67\xb9\x05"
>>> +xa+="\x8a\xb3\x3b\xc1\x45\x94\x36\x6f\x28\xf5\xa2\xd9\x00\xb6\x46\x7a"
>>> +
>>> +# Z
>>> +shared="0fdbd9a2 ebf50cba 489b4e4d 7cd6924a 42ee6324 a26988b2 22bc38e6
>>> 9cc445f1\n" +shared+="eb47c1a4 62eca39f 39bcd7b8 19dede51 30bc38da
>>> ec99c16f 40a4e5c1 9c97b796\n" +shared+="8b41823d a0650e37 13c73e6f
>>> 5f2a9dff 2e67dbf5 40ee66f4 e694c28f ba1d604b\n" +shared+="71b57b8a
>>> eeb67a35 ba425a38 490b6fb9 f713db22 6f893b7a 8962f426 ba3046fb\n"
>>> +shared+="cff8538c 16f583e8 ae947672 0ba55ff9 75b440d0 c4565cc7 5837d23a
>>> fea61a39\n" +shared+="e0b7f6c4 e24c2154 7eb19fce f8dbed10 b06a9cce
>>> 971c0f0f ba7c1d5c b5035eaa\n" +shared+="4fddd3ba fe757339 e3321e3e
>>> 4ebfe9e7 9c6c0401 4df63cf9 28d0a2c0 5b2d5521\n" +shared+="030c35f1
>>> c84c97fe 64cad509 8012a003 d52d24c4 1a1f9348 b7575251 3facb02f\n" +
>>> +# OI
>>> +otherinfo="\xa1\xb2\xc3\xd4\xe5\x43\x41\x56\x53\x69\x64\x0d\x64\xc1\xb2"
>>> +otherinfo+="\x33\x61\xb2\x61\xde\x78\x68\x8e\xa8\x65\xfc\xff\x11\x3c\x84"
>>> +
>>> +# DKM
>>> +derived="8284e313 02c8a26b 393ec52d 9f9e0882\n"
>>> +
>>> +pcreate_key "-e $prime" user dh:prime @s
>>> +expect_keyid primeid
>>> +
>>> +pcreate_key "-e $xa" user dh:xa @s
>>> +expect_keyid xaid
>>> +
>>> +pcreate_key "-e $private" user dh:private @s
>>> +expect_keyid privateid
>>> +
>>> +marker "COMPUTE DH SHARED SECRET"
>>> +dh_compute $privateid $primeid $xaid
>>> +expect_payload payload $shared
>>
>> As I mentioned at the top, we'll need an 'expect_multiline' function that
>> handles values like $shared.
>
> Ok.
>>
>>> +
>>> +marker "COMPUTE DERIVED KEY FROM DH SHARED SECRET (SHA-256)"
>>> +dh_compute $privateid $primeid $xaid 16 "kdf_ctr(sha256)" "$(echo -e -n
>>> $otherinfo)" +expect_payload payload $derived
>>
>> Have you checked that expect_payload works correctly in this case? Seems
>> like it should, since the output fits on one line.
>
> I just tested it and the code does NOT catch the error. I.e. when changing
> $derived, the harness still returns a PASS even though the returned data does
> not match the expected data.

When I was implementing expect_multiline, I realized I also had a quoting 
problem in my use of expect_payload. Look over the patchset I posted to 
the keyrings list today, with that you should use:

expect_multiline payload "$shared"

Note that you'll also have to update your assignment to the 'shared' 
variable to make sure the newlines are preserved, like my change to the 
'public' variable assignment.

>>
>>> +
>>> echo "++++ FINISHED TEST: $result" >>$OUTPUTFILE
>>>
>>> # --- then report the results in the database ---

Regards,

--
Mat Martineau
Intel OTC

Re: [RFC PATCH] KEYS: add SP800-56A KDF support for DH

[Mat Martineau]

On Tue, 12 Jul 2016, Stephan Mueller wrote:

> Hi Mat, David,
>
> This patch is based on the cryptodev-2.6 tree with the patch
> 4693fc734d675c5518ea9bd4c9623db45bc37402 ("KEYS: Add placeholder
> for KDF usage with DH") from Linus' tree added on top.
>
> I am aware of the fact that the compat code is not present. This
> patch is intended for review to verify that the user space
> interface follows the general approach for the keys subsystem.
>
> The patch to add KDF to the kernel crypto API will be appended to
> this patch.
>
> The patch for the keyutils user space extension will also be added.
>
> Ciao
> Stephan
>
> ---8<---
>
> SP800-56A define the use of DH with key derivation function based on a
> counter. The input to the KDF is defined as (DH shared secret || other
> information). The value for the "other information" is to be provided by
> the caller.
>
> The KDF is provided by the kernel crypto API. The SP800-56A KDF is equal
> to the SP800-108 counter KDF. However, the caller is allowed to specify
> the KDF type that he wants to use to derive the key material allowing
> the use of the other KDFs provided with the kernel crypto API.
>
> As the KDF implements the proper truncation of the DH shared secret to
> the requested size, this patch fills the caller buffer up to its size.
>
> The patch is tested with a new test added to the keyutils user space
> code which uses a CAVS test vector testing the compliance with
> SP800-56A.
>
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
> include/uapi/linux/keyctl.h | 10 +++++
> security/keys/Kconfig       |  1 +
> security/keys/dh.c          | 98 ++++++++++++++++++++++++++++++++++++++++-----
> security/keys/internal.h    |  5 ++-
> security/keys/keyctl.c      |  2 +-
> 5 files changed, 103 insertions(+), 13 deletions(-)

Be sure to update Documentation/security/keys.txt once the interface is 
settled on.

>
> diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
> index 86eddd6..cc4ce7c 100644
> --- a/include/uapi/linux/keyctl.h
> +++ b/include/uapi/linux/keyctl.h
> @@ -68,4 +68,14 @@ struct keyctl_dh_params {
> 	__s32 base;
> };
>
> +struct keyctl_kdf_params {
> +#define KEYCTL_KDF_MAX_OUTPUTLEN	1024	/* max length of KDF output */
> +#define KEYCTL_KDF_MAX_STRING_LEN	64	/* maximum length of strings */

I think these limits should be in the internal headers rather than uapi.

> +	char *kdfname;
> +	__u32 kdfnamelen;

As noted in the userspace patch, if kdfname is a null-terminated string 
then kdfnamelen isn't needed.

> +	char *otherinfo;
> +	__u32 otherinfolen;
> +	__u32 flags;

Looks like flags aren't used anywhere. Do you have a use planned? You 
could add some spare capacity like the keyctl_pkey_* structs instead (see 
https://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h=keys-next&id=dd7e2ea418b4f8851a4eb976b9431fbc03d2ebaf 
)

> +};
> +
> #endif /*  _LINUX_KEYCTL_H */
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> index f826e87..56491fe 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -90,6 +90,7 @@ config KEY_DH_OPERATIONS
>        bool "Diffie-Hellman operations on retained keys"
>        depends on KEYS
>        select MPILIB
> +       select CRYPTO_KDF
>        help
> 	 This option provides support for calculating Diffie-Hellman
> 	 public keys and shared secrets using values stored as keys
> diff --git a/security/keys/dh.c b/security/keys/dh.c
> index 531ed2e..4c93969 100644
> --- a/security/keys/dh.c
> +++ b/security/keys/dh.c
> @@ -77,14 +77,74 @@ error:
> 	return ret;
> }
>
> +static int keyctl_dh_compute_kdf(struct keyctl_kdf_params *kdfcopy,
> +				 char __user *buffer, size_t buflen,
> +				 uint8_t *kbuf, size_t resultlen)
> +{

Minor point: this function name made me think it was a replacement for 
keyctl_dh_compute at first (like the userspace counterpart).

> +	char kdfname[CRYPTO_MAX_ALG_NAME] = { 0 };
> +	struct crypto_rng *tfm;
> +	uint8_t *outbuf = NULL;
> +	int ret;
> +
> +	BUILD_BUG_ON(CRYPTO_MAX_ALG_NAME != KEYCTL_KDF_MAX_STRING_LEN);

If this is a requirement, why define KEYCTL_KDF_MAX_STRING_LEN? Use 
CRYPTO_MAX_ALG_NAME directly.

> +	if (!kdfcopy->kdfnamelen)
> +		return -EFAULT;
> +	if (copy_from_user(&kdfname, kdfcopy->kdfname,
> +			   kdfcopy->kdfnamelen) != 0)

strndup_user works nicely for strings.

> +		return -EFAULT;
> +

It would be best to validate all of the userspace input before the DH 
computation is done.

> +	/*
> +	 * Concatenate otherinfo past DH shared secret -- the
> +	 * input to the KDF is (DH shared secret || otherinfo)
> +	 */
> +	if (kdfcopy->otherinfo &&
> +	    copy_from_user(kbuf + resultlen, kdfcopy->otherinfo,
> +			   kdfcopy->otherinfolen)
> +	    != 0)
> +		return -EFAULT;
> +
> +	tfm = crypto_alloc_rng(kdfname, 0, 0);
> +	if (IS_ERR(tfm))
> +		return PTR_ERR(tfm);
> +
> +#if 0
> +	/* we do not support HMAC currently */
> +	ret = crypto_rng_reset(tfm, xx, xxlen);
> +	if (ret) {
> +		crypto_free_rng(tfm);
> +		goto error5;
> +	}
> +#endif
> +
> +	outbuf = kmalloc(buflen, GFP_KERNEL);
> +	if (!outbuf) {
> +		ret = -ENOMEM;
> +		goto err;
> +	}
> +
> +	ret = crypto_rng_generate(tfm, kbuf, resultlen + kdfcopy->otherinfolen,
> +				  outbuf, buflen);
> +	if (ret)
> +		goto err;
> +
> +	ret = buflen;
> +	if (copy_to_user(buffer, outbuf, buflen) != 0)
> +		ret = -EFAULT;
> +
> +err:
> +	kzfree(outbuf);
> +	crypto_free_rng(tfm);
> +	return ret;
> +}
> long keyctl_dh_compute(struct keyctl_dh_params __user *params,
> 		       char __user *buffer, size_t buflen,
> -		       void __user *reserved)
> +		       struct keyctl_kdf_params __user *kdf)
> {
> 	long ret;
> 	MPI base, private, prime, result;
> 	unsigned nbytes;
> 	struct keyctl_dh_params pcopy;
> +	struct keyctl_kdf_params kdfcopy;
> 	uint8_t *kbuf;
> 	ssize_t keylen;
> 	size_t resultlen;
> @@ -98,12 +158,24 @@ long keyctl_dh_compute(struct keyctl_dh_params __user *params,
> 		goto out;
> 	}
>
> -	if (reserved) {
> -		ret = -EINVAL;
> -		goto out;
> +	if (kdf) {
> +		if (copy_from_user(&kdfcopy, kdf, sizeof(kdfcopy)) != 0) {
> +			ret = -EFAULT;
> +			goto out;
> +		}
> +		if (buflen > KEYCTL_KDF_MAX_OUTPUTLEN ||
> +		    kdfcopy.otherinfolen > KEYCTL_KDF_MAX_STRING_LEN ||
> +		    kdfcopy.kdfnamelen > KEYCTL_KDF_MAX_STRING_LEN) {
> +			ret = -EMSGSIZE;
> +			goto out;
> +		}
> 	}
>
> -	keylen = mpi_from_key(pcopy.prime, buflen, &prime);
> +	/*
> +	 * If the caller requests postprocessing with a KDF, allow an
> +	 * arbitrary output buffer size since the KDF ensures proper truncation.
> +	 */
> +	keylen = mpi_from_key(pcopy.prime, kdf ? SIZE_MAX : buflen, &prime);
> 	if (keylen < 0 || !prime) {
> 		/* buflen == 0 may be used to query the required buffer size,
> 		 * which is the prime key length.
> @@ -133,7 +205,8 @@ long keyctl_dh_compute(struct keyctl_dh_params __user *params,
> 		goto error3;
> 	}
>
> -	kbuf = kmalloc(resultlen, GFP_KERNEL);
> +	kbuf = kmalloc(kdf ? (resultlen + kdfcopy.otherinfolen) : resultlen,
> +		       GFP_KERNEL);
> 	if (!kbuf) {
> 		ret = -ENOMEM;
> 		goto error4;
> @@ -147,12 +220,17 @@ long keyctl_dh_compute(struct keyctl_dh_params __user *params,
> 	if (ret != 0)
> 		goto error5;
>
> -	ret = nbytes;
> -	if (copy_to_user(buffer, kbuf, nbytes) != 0)
> -		ret = -EFAULT;
> +	if (kdf) {
> +		ret = keyctl_dh_compute_kdf(&kdfcopy, buffer, buflen,
> +					    kbuf, resultlen);
> +	} else {
> +		ret = nbytes;
> +		if (copy_to_user(buffer, kbuf, nbytes) != 0)
> +			ret = -EFAULT;
> +	}
>
> error5:
> -	kfree(kbuf);
> +	kzfree(kbuf);

Thanks for adjusting this.

> error4:
> 	mpi_free(result);
> error3:
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index a705a7d..35a8d11 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -259,12 +259,13 @@ static inline long keyctl_get_persistent(uid_t uid, key_serial_t destring)
> #endif
>
> #ifdef CONFIG_KEY_DH_OPERATIONS
> +#include <crypto/rng.h>
> extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char __user *,
> -			      size_t, void __user *);
> +			      size_t, struct keyctl_kdf_params __user *);
> #else
> static inline long keyctl_dh_compute(struct keyctl_dh_params __user *params,
> 				     char __user *buffer, size_t buflen,
> -				     void __user *reserved)
> +				     struct keyctl_kdf_params __user *kdf)
> {
> 	return -EOPNOTSUPP;
> }
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index d580ad0..b106898 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -1689,7 +1689,7 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long, arg2, unsigned long, arg3,
> 	case KEYCTL_DH_COMPUTE:
> 		return keyctl_dh_compute((struct keyctl_dh_params __user *) arg2,
> 					 (char __user *) arg3, (size_t) arg4,
> -					 (void __user *) arg5);
> +					 (struct keyctl_kdf_params __user *) arg5);
>
> 	default:
> 		return -EOPNOTSUPP;


Regards,

--
Mat Martineau
Intel OTC

Re: [RFC PATCH] KEYS: add SP800-56A KDF support for DH

[Stephan Mueller]

Am Donnerstag, 14. Juli 2016, 17:45:59 schrieb Mat Martineau:

Hi Mat,

> > Signed-off-by: Stephan Mueller <smueller@chronox.de>
> > ---
> > include/uapi/linux/keyctl.h | 10 +++++
> > security/keys/Kconfig       |  1 +
> > security/keys/dh.c          | 98
> > ++++++++++++++++++++++++++++++++++++++++----- security/keys/internal.h   
> > |  5 ++-
> > security/keys/keyctl.c      |  2 +-
> > 5 files changed, 103 insertions(+), 13 deletions(-)
> 
> Be sure to update Documentation/security/keys.txt once the interface is
> settled on.

Thanks for the reminder
> 
> > diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
> > index 86eddd6..cc4ce7c 100644
> > --- a/include/uapi/linux/keyctl.h
> > +++ b/include/uapi/linux/keyctl.h
> > @@ -68,4 +68,14 @@ struct keyctl_dh_params {
> > 
> > 	__s32 base;
> > 
> > };
> > 
> > +struct keyctl_kdf_params {
> > +#define KEYCTL_KDF_MAX_OUTPUTLEN	1024	/* max length of KDF output */
> > +#define KEYCTL_KDF_MAX_STRING_LEN	64	/* maximum length of strings 
*/
> 
> I think these limits should be in the internal headers rather than uapi.

Ok
> 
> > +	char *kdfname;
> > +	__u32 kdfnamelen;
> 
> As noted in the userspace patch, if kdfname is a null-terminated string
> then kdfnamelen isn't needed.

Ok
> 
> > +	char *otherinfo;
> > +	__u32 otherinfolen;
> > +	__u32 flags;
> 
> Looks like flags aren't used anywhere. Do you have a use planned? You
> could add some spare capacity like the keyctl_pkey_* structs instead (see
> https://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h
> =keys-next&id=dd7e2ea418b4f8851a4eb976b9431fbc03d2ebaf )

I am not sure what to do here: I see the profileration of new syscalls which 
just differ from existing syscalls by a new flags field because the initial 
implementation simply missed such thing.

I want to avoid something like this happening here.

I am open for any suggestions.
> 
> > +};
> > +
> > #endif /*  _LINUX_KEYCTL_H */
> > diff --git a/security/keys/Kconfig b/security/keys/Kconfig
> > index f826e87..56491fe 100644
> > --- a/security/keys/Kconfig
> > +++ b/security/keys/Kconfig
> > @@ -90,6 +90,7 @@ config KEY_DH_OPERATIONS
> > 
> >        bool "Diffie-Hellman operations on retained keys"
> >        depends on KEYS
> >        select MPILIB
> > 
> > +       select CRYPTO_KDF
> > 
> >        help
> > 	 
> > 	 This option provides support for calculating Diffie-Hellman
> > 	 public keys and shared secrets using values stored as keys
> > 
> > diff --git a/security/keys/dh.c b/security/keys/dh.c
> > index 531ed2e..4c93969 100644
> > --- a/security/keys/dh.c
> > +++ b/security/keys/dh.c
> > 
> > @@ -77,14 +77,74 @@ error:
> > 	return ret;
> > 
> > }
> > 
> > +static int keyctl_dh_compute_kdf(struct keyctl_kdf_params *kdfcopy,
> > +				 char __user *buffer, size_t buflen,
> > +				 uint8_t *kbuf, size_t resultlen)
> > +{
> 
> Minor point: this function name made me think it was a replacement for
> keyctl_dh_compute at first (like the userspace counterpart).

Well, initially I had it part of dh_compute, but then extracted it to make the 
code nicer and less distracting.

> 
> > +	char kdfname[CRYPTO_MAX_ALG_NAME] = { 0 };
> > +	struct crypto_rng *tfm;
> > +	uint8_t *outbuf = NULL;
> > +	int ret;
> > +
> > +	BUILD_BUG_ON(CRYPTO_MAX_ALG_NAME != KEYCTL_KDF_MAX_STRING_LEN);
> 
> If this is a requirement, why define KEYCTL_KDF_MAX_STRING_LEN? Use
> CRYPTO_MAX_ALG_NAME directly.

Ok, I was not sure if I am allowed to add a crypto API header to key header 
files.
> 
> > +	if (!kdfcopy->kdfnamelen)
> > +		return -EFAULT;
> > +	if (copy_from_user(&kdfname, kdfcopy->kdfname,
> > +			   kdfcopy->kdfnamelen) != 0)
> 
> strndup_user works nicely for strings.

yes.
> 
> > +		return -EFAULT;
> > +
> 
> It would be best to validate all of the userspace input before the DH
> computation is done.

Uh, that means I cannot have a separate dh_compute_kdf function. But, ok, no 
problem.
> 
> > +	/*
> > +	 * Concatenate otherinfo past DH shared secret -- the
> > +	 * input to the KDF is (DH shared secret || otherinfo)
> > +	 */
> > +	if (kdfcopy->otherinfo &&
> > +	    copy_from_user(kbuf + resultlen, kdfcopy->otherinfo,
> > +			   kdfcopy->otherinfolen)
> > +	    != 0)
> > +		return -EFAULT;
> > +
> > +	tfm = crypto_alloc_rng(kdfname, 0, 0);
> > +	if (IS_ERR(tfm))
> > +		return PTR_ERR(tfm);
> > +
> > +#if 0
> > +	/* we do not support HMAC currently */
> > +	ret = crypto_rng_reset(tfm, xx, xxlen);
> > +	if (ret) {
> > +		crypto_free_rng(tfm);
> > +		goto error5;
> > +	}
> > +#endif
> > +
> > +	outbuf = kmalloc(buflen, GFP_KERNEL);
> > +	if (!outbuf) {
> > +		ret = -ENOMEM;
> > +		goto err;
> > +	}
> > +
> > +	ret = crypto_rng_generate(tfm, kbuf, resultlen + kdfcopy-
>otherinfolen,
> > +				  outbuf, buflen);
> > +	if (ret)
> > +		goto err;
> > +
> > +	ret = buflen;
> > +	if (copy_to_user(buffer, outbuf, buflen) != 0)
> > +		ret = -EFAULT;
> > +
> > +err:
> > +	kzfree(outbuf);
> > +	crypto_free_rng(tfm);
> > +	return ret;
> > +}
> > long keyctl_dh_compute(struct keyctl_dh_params __user *params,
> > 
> > 		       char __user *buffer, size_t buflen,
> > 
> > -		       void __user *reserved)
> > +		       struct keyctl_kdf_params __user *kdf)
> > {
> > 
> > 	long ret;
> > 	MPI base, private, prime, result;
> > 	unsigned nbytes;
> > 	struct keyctl_dh_params pcopy;
> > 
> > +	struct keyctl_kdf_params kdfcopy;
> > 
> > 	uint8_t *kbuf;
> > 	ssize_t keylen;
> > 	size_t resultlen;
> > 
> > @@ -98,12 +158,24 @@ long keyctl_dh_compute(struct keyctl_dh_params __user
> > *params,> 
> > 		goto out;
> > 	
> > 	}
> > 
> > -	if (reserved) {
> > -		ret = -EINVAL;
> > -		goto out;
> > +	if (kdf) {
> > +		if (copy_from_user(&kdfcopy, kdf, sizeof(kdfcopy)) != 0) {
> > +			ret = -EFAULT;
> > +			goto out;
> > +		}
> > +		if (buflen > KEYCTL_KDF_MAX_OUTPUTLEN ||
> > +		    kdfcopy.otherinfolen > KEYCTL_KDF_MAX_STRING_LEN ||
> > +		    kdfcopy.kdfnamelen > KEYCTL_KDF_MAX_STRING_LEN) {
> > +			ret = -EMSGSIZE;
> > +			goto out;
> > +		}
> > 
> > 	}
> > 
> > -	keylen = mpi_from_key(pcopy.prime, buflen, &prime);
> > +	/*
> > +	 * If the caller requests postprocessing with a KDF, allow an
> > +	 * arbitrary output buffer size since the KDF ensures proper 
truncation.
> > +	 */
> > +	keylen = mpi_from_key(pcopy.prime, kdf ? SIZE_MAX : buflen, &prime);
> > 
> > 	if (keylen < 0 || !prime) {
> > 	
> > 		/* buflen == 0 may be used to query the required buffer size,
> > 		
> > 		 * which is the prime key length.
> > 
> > @@ -133,7 +205,8 @@ long keyctl_dh_compute(struct keyctl_dh_params __user
> > *params,> 
> > 		goto error3;
> > 	
> > 	}
> > 
> > -	kbuf = kmalloc(resultlen, GFP_KERNEL);
> > +	kbuf = kmalloc(kdf ? (resultlen + kdfcopy.otherinfolen) : resultlen,
> > +		       GFP_KERNEL);
> > 
> > 	if (!kbuf) {
> > 	
> > 		ret = -ENOMEM;
> > 		goto error4;
> > 
> > @@ -147,12 +220,17 @@ long keyctl_dh_compute(struct keyctl_dh_params
> > __user *params,> 
> > 	if (ret != 0)
> > 	
> > 		goto error5;
> > 
> > -	ret = nbytes;
> > -	if (copy_to_user(buffer, kbuf, nbytes) != 0)
> > -		ret = -EFAULT;
> > +	if (kdf) {
> > +		ret = keyctl_dh_compute_kdf(&kdfcopy, buffer, buflen,
> > +					    kbuf, resultlen);
> > +	} else {
> > +		ret = nbytes;
> > +		if (copy_to_user(buffer, kbuf, nbytes) != 0)
> > +			ret = -EFAULT;
> > +	}
> > 
> > error5:
> > -	kfree(kbuf);
> > +	kzfree(kbuf);
> 
> Thanks for adjusting this.

I hope it is ok to not have it in a separate patch.
> 
> > error4:
> > 	mpi_free(result);
> > 
> > error3:
> > diff --git a/security/keys/internal.h b/security/keys/internal.h
> > index a705a7d..35a8d11 100644
> > --- a/security/keys/internal.h
> > +++ b/security/keys/internal.h
> > @@ -259,12 +259,13 @@ static inline long keyctl_get_persistent(uid_t uid,
> > key_serial_t destring) #endif
> > 
> > #ifdef CONFIG_KEY_DH_OPERATIONS
> > +#include <crypto/rng.h>
> > extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char
> > __user *, -			      size_t, void __user *);
> > +			      size_t, struct keyctl_kdf_params __user *);
> > #else
> > static inline long keyctl_dh_compute(struct keyctl_dh_params __user
> > *params,> 
> > 				     char __user *buffer, size_t buflen,
> > 
> > -				     void __user *reserved)
> > +				     struct keyctl_kdf_params __user *kdf)
> > {
> > 
> > 	return -EOPNOTSUPP;
> > 
> > }
> > diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> > index d580ad0..b106898 100644
> > --- a/security/keys/keyctl.c
> > +++ b/security/keys/keyctl.c
> > @@ -1689,7 +1689,7 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long,
> > arg2, unsigned long, arg3,> 
> > 	case KEYCTL_DH_COMPUTE:
> > 		return keyctl_dh_compute((struct keyctl_dh_params __user *) 
arg2,
> > 		
> > 					 (char __user *) arg3, (size_t) arg4,
> > 
> > -					 (void __user *) arg5);
> > +					 (struct keyctl_kdf_params __user *) 
arg5);
> > 
> > 	default:
> > 		return -EOPNOTSUPP;
> 
> Regards,
> 
> --
> Mat Martineau
> Intel OTC


Ciao
Stephan

Re: [RFC PATCH] KEYS: add SP800-56A KDF support for DH

[Mat Martineau]

Stephan,

On Fri, 15 Jul 2016, Stephan Mueller wrote:

> Am Donnerstag, 14. Juli 2016, 17:45:59 schrieb Mat Martineau:
>
> Hi Mat,
>
>>> Signed-off-by: Stephan Mueller <smueller@chronox.de>
>>> ---
>>> include/uapi/linux/keyctl.h | 10 +++++
>>> security/keys/Kconfig       |  1 +
>>> security/keys/dh.c          | 98
>>> ++++++++++++++++++++++++++++++++++++++++----- security/keys/internal.h
>>> |  5 ++-
>>> security/keys/keyctl.c      |  2 +-
>>> 5 files changed, 103 insertions(+), 13 deletions(-)
>>
>> Be sure to update Documentation/security/keys.txt once the interface is
>> settled on.
>
> Thanks for the reminder
>>
>>> diff --git a/include/uapi/linux/keyctl.h b/include/uapi/linux/keyctl.h
>>> index 86eddd6..cc4ce7c 100644
>>> --- a/include/uapi/linux/keyctl.h
>>> +++ b/include/uapi/linux/keyctl.h
>>> @@ -68,4 +68,14 @@ struct keyctl_dh_params {
>>>
>>> 	__s32 base;
>>>
>>> };
>>>
>>> +struct keyctl_kdf_params {
>>> +#define KEYCTL_KDF_MAX_OUTPUTLEN	1024	/* max length of KDF output */
>>> +#define KEYCTL_KDF_MAX_STRING_LEN	64	/* maximum length of strings
> */
>>
>> I think these limits should be in the internal headers rather than uapi.
>
> Ok
>>
>>> +	char *kdfname;
>>> +	__u32 kdfnamelen;
>>
>> As noted in the userspace patch, if kdfname is a null-terminated string
>> then kdfnamelen isn't needed.
>
> Ok
>>
>>> +	char *otherinfo;
>>> +	__u32 otherinfolen;
>>> +	__u32 flags;
>>
>> Looks like flags aren't used anywhere. Do you have a use planned? You
>> could add some spare capacity like the keyctl_pkey_* structs instead (see
>> https://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h
>> =keys-next&id=dd7e2ea418b4f8851a4eb976b9431fbc03d2ebaf )
>
> I am not sure what to do here: I see the profileration of new syscalls which
> just differ from existing syscalls by a new flags field because the initial
> implementation simply missed such thing.
>
> I want to avoid something like this happening here.
>
> I am open for any suggestions.

David's approach in the structs I referenced is to add a spare array of 
__u32's:

         __u32 __spare[8];

That way future additions can be added to the space currently used by 
__spare, and that array is made smaller so the overall struct size stays 
the same and the older members are not moved around.

>>
>>> +};
>>> +
>>> #endif /*  _LINUX_KEYCTL_H */
>>> diff --git a/security/keys/Kconfig b/security/keys/Kconfig
>>> index f826e87..56491fe 100644
>>> --- a/security/keys/Kconfig
>>> +++ b/security/keys/Kconfig
>>> @@ -90,6 +90,7 @@ config KEY_DH_OPERATIONS
>>>
>>>        bool "Diffie-Hellman operations on retained keys"
>>>        depends on KEYS
>>>        select MPILIB
>>>
>>> +       select CRYPTO_KDF
>>>
>>>        help
>>>
>>> 	 This option provides support for calculating Diffie-Hellman
>>> 	 public keys and shared secrets using values stored as keys
>>>
>>> diff --git a/security/keys/dh.c b/security/keys/dh.c
>>> index 531ed2e..4c93969 100644
>>> --- a/security/keys/dh.c
>>> +++ b/security/keys/dh.c
>>>
>>> @@ -77,14 +77,74 @@ error:
>>> 	return ret;
>>>
>>> }
>>>
>>> +static int keyctl_dh_compute_kdf(struct keyctl_kdf_params *kdfcopy,
>>> +				 char __user *buffer, size_t buflen,
>>> +				 uint8_t *kbuf, size_t resultlen)
>>> +{
>>
>> Minor point: this function name made me think it was a replacement for
>> keyctl_dh_compute at first (like the userspace counterpart).
>
> Well, initially I had it part of dh_compute, but then extracted it to make the
> code nicer and less distracting.

Extracting it is good - my comment was only regarding the name.

>
>>
>>> +	char kdfname[CRYPTO_MAX_ALG_NAME] = { 0 };
>>> +	struct crypto_rng *tfm;
>>> +	uint8_t *outbuf = NULL;
>>> +	int ret;
>>> +
>>> +	BUILD_BUG_ON(CRYPTO_MAX_ALG_NAME != KEYCTL_KDF_MAX_STRING_LEN);
>>
>> If this is a requirement, why define KEYCTL_KDF_MAX_STRING_LEN? Use
>> CRYPTO_MAX_ALG_NAME directly.
>
> Ok, I was not sure if I am allowed to add a crypto API header to key header
> files.

I don't think you need to include the crypto header in any key headers, 
just here in dh.c. dh.c will be converted to use the DH implementation 
recently added to the crypto subsystem, by the way.

>>
>>> +	if (!kdfcopy->kdfnamelen)
>>> +		return -EFAULT;
>>> +	if (copy_from_user(&kdfname, kdfcopy->kdfname,
>>> +			   kdfcopy->kdfnamelen) != 0)
>>
>> strndup_user works nicely for strings.
>
> yes.
>>
>>> +		return -EFAULT;
>>> +
>>
>> It would be best to validate all of the userspace input before the DH
>> computation is done.
>
> Uh, that means I cannot have a separate dh_compute_kdf function. But, ok, no
> problem.
>>
>>> +	/*
>>> +	 * Concatenate otherinfo past DH shared secret -- the
>>> +	 * input to the KDF is (DH shared secret || otherinfo)
>>> +	 */
>>> +	if (kdfcopy->otherinfo &&
>>> +	    copy_from_user(kbuf + resultlen, kdfcopy->otherinfo,
>>> +			   kdfcopy->otherinfolen)
>>> +	    != 0)
>>> +		return -EFAULT;
>>> +
>>> +	tfm = crypto_alloc_rng(kdfname, 0, 0);
>>> +	if (IS_ERR(tfm))
>>> +		return PTR_ERR(tfm);
>>> +
>>> +#if 0
>>> +	/* we do not support HMAC currently */
>>> +	ret = crypto_rng_reset(tfm, xx, xxlen);
>>> +	if (ret) {
>>> +		crypto_free_rng(tfm);
>>> +		goto error5;
>>> +	}
>>> +#endif
>>> +
>>> +	outbuf = kmalloc(buflen, GFP_KERNEL);
>>> +	if (!outbuf) {
>>> +		ret = -ENOMEM;
>>> +		goto err;
>>> +	}
>>> +
>>> +	ret = crypto_rng_generate(tfm, kbuf, resultlen + kdfcopy-
>> otherinfolen,
>>> +				  outbuf, buflen);
>>> +	if (ret)
>>> +		goto err;
>>> +
>>> +	ret = buflen;
>>> +	if (copy_to_user(buffer, outbuf, buflen) != 0)
>>> +		ret = -EFAULT;
>>> +
>>> +err:
>>> +	kzfree(outbuf);
>>> +	crypto_free_rng(tfm);
>>> +	return ret;
>>> +}
>>> long keyctl_dh_compute(struct keyctl_dh_params __user *params,
>>>
>>> 		       char __user *buffer, size_t buflen,
>>>
>>> -		       void __user *reserved)
>>> +		       struct keyctl_kdf_params __user *kdf)
>>> {
>>>
>>> 	long ret;
>>> 	MPI base, private, prime, result;
>>> 	unsigned nbytes;
>>> 	struct keyctl_dh_params pcopy;
>>>
>>> +	struct keyctl_kdf_params kdfcopy;
>>>
>>> 	uint8_t *kbuf;
>>> 	ssize_t keylen;
>>> 	size_t resultlen;
>>>
>>> @@ -98,12 +158,24 @@ long keyctl_dh_compute(struct keyctl_dh_params __user
>>> *params,>
>>> 		goto out;
>>>
>>> 	}
>>>
>>> -	if (reserved) {
>>> -		ret = -EINVAL;
>>> -		goto out;
>>> +	if (kdf) {
>>> +		if (copy_from_user(&kdfcopy, kdf, sizeof(kdfcopy)) != 0) {
>>> +			ret = -EFAULT;
>>> +			goto out;
>>> +		}
>>> +		if (buflen > KEYCTL_KDF_MAX_OUTPUTLEN ||
>>> +		    kdfcopy.otherinfolen > KEYCTL_KDF_MAX_STRING_LEN ||
>>> +		    kdfcopy.kdfnamelen > KEYCTL_KDF_MAX_STRING_LEN) {
>>> +			ret = -EMSGSIZE;
>>> +			goto out;
>>> +		}
>>>
>>> 	}
>>>
>>> -	keylen = mpi_from_key(pcopy.prime, buflen, &prime);
>>> +	/*
>>> +	 * If the caller requests postprocessing with a KDF, allow an
>>> +	 * arbitrary output buffer size since the KDF ensures proper truncation.
>>> +	 */
>>> +	keylen = mpi_from_key(pcopy.prime, kdf ? SIZE_MAX : buflen, &prime);
>>>
>>> 	if (keylen < 0 || !prime) {
>>>
>>> 		/* buflen == 0 may be used to query the required buffer size,
>>>
>>> 		 * which is the prime key length.
>>>
>>> @@ -133,7 +205,8 @@ long keyctl_dh_compute(struct keyctl_dh_params __user
>>> *params,>
>>> 		goto error3;
>>>
>>> 	}
>>>
>>> -	kbuf = kmalloc(resultlen, GFP_KERNEL);
>>> +	kbuf = kmalloc(kdf ? (resultlen + kdfcopy.otherinfolen) : resultlen,
>>> +		       GFP_KERNEL);
>>>
>>> 	if (!kbuf) {
>>>
>>> 		ret = -ENOMEM;
>>> 		goto error4;
>>>
>>> @@ -147,12 +220,17 @@ long keyctl_dh_compute(struct keyctl_dh_params
>>> __user *params,>
>>> 	if (ret != 0)
>>>
>>> 		goto error5;
>>>
>>> -	ret = nbytes;
>>> -	if (copy_to_user(buffer, kbuf, nbytes) != 0)
>>> -		ret = -EFAULT;
>>> +	if (kdf) {
>>> +		ret = keyctl_dh_compute_kdf(&kdfcopy, buffer, buflen,
>>> +					    kbuf, resultlen);
>>> +	} else {
>>> +		ret = nbytes;
>>> +		if (copy_to_user(buffer, kbuf, nbytes) != 0)
>>> +			ret = -EFAULT;
>>> +	}
>>>
>>> error5:
>>> -	kfree(kbuf);
>>> +	kzfree(kbuf);
>>
>> Thanks for adjusting this.
>
> I hope it is ok to not have it in a separate patch.
>>
>>> error4:
>>> 	mpi_free(result);
>>>
>>> error3:
>>> diff --git a/security/keys/internal.h b/security/keys/internal.h
>>> index a705a7d..35a8d11 100644
>>> --- a/security/keys/internal.h
>>> +++ b/security/keys/internal.h
>>> @@ -259,12 +259,13 @@ static inline long keyctl_get_persistent(uid_t uid,
>>> key_serial_t destring) #endif
>>>
>>> #ifdef CONFIG_KEY_DH_OPERATIONS
>>> +#include <crypto/rng.h>
>>> extern long keyctl_dh_compute(struct keyctl_dh_params __user *, char
>>> __user *, -			      size_t, void __user *);
>>> +			      size_t, struct keyctl_kdf_params __user *);
>>> #else
>>> static inline long keyctl_dh_compute(struct keyctl_dh_params __user
>>> *params,>
>>> 				     char __user *buffer, size_t buflen,
>>>
>>> -				     void __user *reserved)
>>> +				     struct keyctl_kdf_params __user *kdf)
>>> {
>>>
>>> 	return -EOPNOTSUPP;
>>>
>>> }
>>> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
>>> index d580ad0..b106898 100644
>>> --- a/security/keys/keyctl.c
>>> +++ b/security/keys/keyctl.c
>>> @@ -1689,7 +1689,7 @@ SYSCALL_DEFINE5(keyctl, int, option, unsigned long,
>>> arg2, unsigned long, arg3,>
>>> 	case KEYCTL_DH_COMPUTE:
>>> 		return keyctl_dh_compute((struct keyctl_dh_params __user *) arg2,
>>>
>>> 					 (char __user *) arg3, (size_t) arg4,
>>>
>>> -					 (void __user *) arg5);
>>> +					 (struct keyctl_kdf_params __user *) arg5);
>>>
>>> 	default:
>>> 		return -EOPNOTSUPP;

--
Mat Martineau
Intel OTC

Re: [PATCH v3 1/4] crypto: add template handling for RNGs

[Herbert Xu]

Stephan Mueller <smueller@chronox.de> wrote:
> This patch adds the ability to register templates for RNGs. RNGs are
> "meta" mechanisms using raw cipher primitives. Thus, RNGs can now be
> implemented as templates to allow the complete flexibility the kernel
> crypto API provides.
> 
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
> crypto/rng.c         | 31 +++++++++++++++++++++++++++++++
> include/crypto/rng.h | 39 +++++++++++++++++++++++++++++++++++++++
> 2 files changed, 70 insertions(+)
> 
> diff --git a/crypto/rng.c b/crypto/rng.c
> index b81cffb..92cc02a 100644
> --- a/crypto/rng.c
> +++ b/crypto/rng.c
> @@ -232,5 +232,36 @@ void crypto_unregister_rngs(struct rng_alg *algs, int count)
> }
> EXPORT_SYMBOL_GPL(crypto_unregister_rngs);
> 
> +void rng_free_instance(struct crypto_instance *inst)
> +{
> +       crypto_drop_spawn(crypto_instance_ctx(inst));
> +       kfree(rng_instance(inst));
> +}

Please use the new free interface, i.e.,

void rng_free_instance(struct rng_instance *inst)

and then

inst->free = rng_free_instance;

> +static inline struct rng_alg *__crypto_rng_alg(struct crypto_alg *alg)
> +{
> +       return container_of(alg, struct rng_alg, base);
> +}
> +
> +static inline struct rng_instance *rng_instance(
> +       struct crypto_instance *inst)
> +{
> +       return container_of(__crypto_rng_alg(&inst->alg),
> +                           struct rng_instance, alg);
> +}

These two can then be deleted.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH v3 1/4] crypto: add template handling for RNGs

[Stephan Mueller]

Am Montag, 18. Juli 2016, 15:14:17 schrieb Herbert Xu:

Hi Herbert,
> > 
> > diff --git a/crypto/rng.c b/crypto/rng.c
> > index b81cffb..92cc02a 100644
> > --- a/crypto/rng.c
> > +++ b/crypto/rng.c
> > @@ -232,5 +232,36 @@ void crypto_unregister_rngs(struct rng_alg *algs, int
> > count) }
> > EXPORT_SYMBOL_GPL(crypto_unregister_rngs);
> > 
> > +void rng_free_instance(struct crypto_instance *inst)
> > +{
> > +       crypto_drop_spawn(crypto_instance_ctx(inst));
> > +       kfree(rng_instance(inst));
> > +}
> 
> Please use the new free interface, i.e.,
> 
> void rng_free_instance(struct rng_instance *inst)
> 
> and then
> 
> inst->free = rng_free_instance;
> 
> > +static inline struct rng_alg *__crypto_rng_alg(struct crypto_alg *alg)
> > +{
> > +       return container_of(alg, struct rng_alg, base);
> > +}
> > +
> > +static inline struct rng_instance *rng_instance(
> > +       struct crypto_instance *inst)
> > +{
> > +       return container_of(__crypto_rng_alg(&inst->alg),
> > +                           struct rng_instance, alg);
> > +}
> 
> These two can then be deleted.

Thanks. I will add that to the next round.
> 
> Thanks,


Ciao
Stephan

Re: [PATCH v3 1/4] crypto: add template handling for RNGs

[Sandy Harris]

On Mon, Jul 18, 2016 at 3:14 AM, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> Stephan Mueller <smueller@chronox.de> wrote:

>> This patch adds the ability to register templates for RNGs. RNGs are
>> "meta" mechanisms using raw cipher primitives. Thus, RNGs can now be
>> implemented as templates to allow the complete flexibility the kernel
>> crypto API provides.

I do not see why this might be desirable, let alone necessary.
Security-critical code should be kept as simple as possible.
Don't we need just one good RNG?

Re: [PATCH v3 1/4] crypto: add template handling for RNGs

[Stephan Mueller]

Am Montag, 18. Juli 2016, 11:23:26 schrieb Sandy Harris:

Hi Sandy,

> On Mon, Jul 18, 2016 at 3:14 AM, Herbert Xu <herbert@gondor.apana.org.au> 
wrote:
> > Stephan Mueller <smueller@chronox.de> wrote:
> >> This patch adds the ability to register templates for RNGs. RNGs are
> >> "meta" mechanisms using raw cipher primitives. Thus, RNGs can now be
> >> implemented as templates to allow the complete flexibility the kernel
> >> crypto API provides.
> 
> I do not see why this might be desirable, let alone necessary.
> Security-critical code should be kept as simple as possible.
> Don't we need just one good RNG?

Well, why do we have multiple symmetric ciphers, hashes, or asymmetric 
ciphers? This allows different users to pick the cipher he likes. Maybe there 
is an issue found in one of them and having multiple at the disposal, allows 
an immediate fixing of issues.

Furthermore, the kernel crypto API supports hardware implementations. So, on 
one arch, you may have an accelerated SHA256 whereas on another you have an 
accelerated SHA512. And accelerated implementations are not only useful for 
speed improvements, but for security purposes as well (like cache-attack 
resistance).

This applies to RNGs too. Furthermore, I think an SP800-90A DRBG is a good 
one. But many people find this standard smelly just because of the Dual EC 
fiasco. Or it is smelly because it was developed by the US organization called 
NIST.

At the level of the kernel crypto API, no policies should be enforced. 
Policies (such as what is the "right" cipher) should be defined outside the 
kernel.

Furthermore, the RNG approach shall be usable for mechanisms that act RNG-like 
but are no real RNGs. The prime example as given with the patch set is the 
KDF. A KDF acts like an RNG, but is not considered as one. Now, there are many 
different types of KDFs out there. SP800-108 is one (defining three types), 
SP800-56A defines more.

For the KDF implementation I did not want to hard-wire the KDF logic with a 
defined cipher like SHA-256, but allow the caller to define the used hash. 
Thus I need the plumbing code do to that.

Ciao
Stephan

Re: [PATCH] DH support: add KDF handling support

[David Howells]

Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:

> > Though, shall I stuff the wrapper code back into the existing dh_compute
> > functions or can I leave them as separate functions?
> 
> I'm not sure. In the existing code there's one keyctl wrapper per keyctl
> command. A combined wrapper would need some extra logic to decide whether
> kdfparams is passed in or not, which is different from existing code.

You shouldn't change the existing keyctl wrappers.  Feel free to add another
one with extra arguments.

David

Re: [PATCH] DH support: add KDF handling support

[Stephan Mueller]

Am Mittwoch, 27. Juli 2016, 08:55:31 CEST schrieb David Howells:

Hi David,

> Mat Martineau <mathew.j.martineau@linux.intel.com> wrote:
> > > Though, shall I stuff the wrapper code back into the existing dh_compute
> > > functions or can I leave them as separate functions?
> > 
> > I'm not sure. In the existing code there's one keyctl wrapper per keyctl
> > command. A combined wrapper would need some extra logic to decide whether
> > kdfparams is passed in or not, which is different from existing code.
> 
> You shouldn't change the existing keyctl wrappers.  Feel free to add another
> one with extra arguments.

I created dh_compute_kdf and dh_compute_kdf_oi where the latter takes the 
other information from STDIN.
> 
> David



Ciao
Stephan