All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH] crypto: DH - add public key verification test
@ 2018-06-26 10:17 Stephan Müller
  2018-06-26 15:13 ` Stephan Mueller
  2018-06-27  6:15 ` [PATCH v2] " Stephan Müller
  0 siblings, 2 replies; 10+ messages in thread
From: Stephan Müller @ 2018-06-26 10:17 UTC (permalink / raw)
  To: herbert; +Cc: linux-crypto, keyrings

Hi,

This patch is an RFC to discuss the following: step 1 in
dh_is_pubkey_valid requires the public key to be in the range of
1 < y < p - 1. The currently implemented check is 1 < y < p since the
calculation of p - 1 requires the presence of mpi_sub or mpi_sub_ui.
Both functions are currently not implemented and I am not sure we
really need to add them for just this check.

I personally think that the check of 1 < y < p would suffice for now
considering that the verification of step 2 using the modular
exponentiation is implemented as well.

In case mpi_sub is added for another reason, we can update the check
code to be accurate.

The NIST ACVP testing passes with the code provided below.

---8<---

According to SP800-56A section 5.6.2.1, the public key to be processed
for the DH operation shall be checked for appropriateness. The check
shall covers the full verification test in case the domain parameter Q
is provided as defined in SP800-56A section 5.6.2.3.1. If Q is not
provided, the partial check according to SP800-56A section 5.6.2.3.2 is
performed.

The full verification test requires the presence of the domain parameter
Q. Thus, the patch adds the support to handle Q. It is permissible to
not provide the Q value as part of the domain parameters. This implies
that the interface is still backwards-compatible where so far only P and
G are to be provided. However, if Q is provided, it is imported.

Without the test, the NIST ACVP testing fails. After adding this check,
the NIST ACVP testing passes. Testing without providing the Q domain
parameter has been performed to verify the interface has not changed.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/dh.c         | 61 ++++++++++++++++++++++++++++++++++++++++++---
 crypto/dh_helper.c  | 15 ++++++++---
 include/crypto/dh.h |  4 +++
 3 files changed, 74 insertions(+), 6 deletions(-)

diff --git a/crypto/dh.c b/crypto/dh.c
index 5659fe7f446d..5d6d397c20ee 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -16,14 +16,16 @@
 #include <linux/mpi.h>
 
 struct dh_ctx {
-	MPI p;
-	MPI g;
-	MPI xa;
+	MPI p;	/* Value is guaranteed to be set. */
+	MPI q;	/* Value is optional. */
+	MPI g;	/* Value is guaranteed to be set. */
+	MPI xa;	/* Value is guaranteed to be set. */
 };
 
 static void dh_clear_ctx(struct dh_ctx *ctx)
 {
 	mpi_free(ctx->p);
+	mpi_free(ctx->q);
 	mpi_free(ctx->g);
 	mpi_free(ctx->xa);
 	memset(ctx, 0, sizeof(*ctx));
@@ -60,6 +62,12 @@ static int dh_set_params(struct dh_ctx *ctx, struct dh *params)
 	if (!ctx->p)
 		return -EINVAL;
 
+	if (params->q && params->q_size) {
+		ctx->q = mpi_read_raw_data(params->q, params->q_size);
+		if (!ctx->q)
+			return -EINVAL;
+	}
+
 	ctx->g = mpi_read_raw_data(params->g, params->g_size);
 	if (!ctx->g)
 		return -EINVAL;
@@ -93,6 +101,50 @@ static int dh_set_secret(struct crypto_kpp *tfm, const void *buf,
 	return -EINVAL;
 }
 
+/*
+ * SP800-56A public key verification:
+ *
+ * * If Q is provided as part of the domain paramenters, a full validation
+ *   according to SP800-56A section 5.6.2.3.1 is performed.
+ *
+ * * If Q is not provided, a partial validation according to SP800-56A section
+ *   5.6.2.3.2 is performed.
+ */
+static int dh_is_pubkey_valid(struct dh_ctx *ctx, MPI y)
+{
+	if (unlikely(!ctx->p))
+		return -EINVAL;
+
+	/*
+	 * Step 1: Verify that 2 <= y <= p - 2.
+	 *
+	 * The upper limit check is actually y < p instead of y < p - 1
+	 * as the mpi_sub_ui function is yet missing.
+	 */
+	if (mpi_cmp_ui(y, 1) < 1 || mpi_cmp(y, ctx->p) >= 0)
+		return -EINVAL;
+
+	/* Step 2: Verify that 1 = y^q mod p */
+	if (ctx->q) {
+		MPI val = mpi_alloc(0);
+		int ret = mpi_powm(val, y, ctx->q, ctx->p);
+
+		if (ret) {
+			mpi_free(val);
+			return ret;
+		}
+
+		ret = mpi_cmp_ui(val, 1);
+
+		mpi_free(val);
+
+		if (ret != 0)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int dh_compute_value(struct kpp_request *req)
 {
 	struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
@@ -115,6 +167,9 @@ static int dh_compute_value(struct kpp_request *req)
 			ret = -EINVAL;
 			goto err_free_val;
 		}
+		ret = dh_is_pubkey_valid(ctx, base);
+		if (ret)
+			goto err_free_val;
 	} else {
 		base = ctx->g;
 	}
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index 24fdb2ecaa85..a7de3d9ce5ac 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -30,7 +30,7 @@ static inline const u8 *dh_unpack_data(void *dst, const void *src, size_t size)
 
 static inline unsigned int dh_data_size(const struct dh *p)
 {
-	return p->key_size + p->p_size + p->g_size;
+	return p->key_size + p->p_size + p->q_size + p->g_size;
 }
 
 unsigned int crypto_dh_key_len(const struct dh *p)
@@ -56,9 +56,11 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const struct dh *params)
 	ptr = dh_pack_data(ptr, &secret, sizeof(secret));
 	ptr = dh_pack_data(ptr, &params->key_size, sizeof(params->key_size));
 	ptr = dh_pack_data(ptr, &params->p_size, sizeof(params->p_size));
+	ptr = dh_pack_data(ptr, &params->q_size, sizeof(params->q_size));
 	ptr = dh_pack_data(ptr, &params->g_size, sizeof(params->g_size));
 	ptr = dh_pack_data(ptr, params->key, params->key_size);
 	ptr = dh_pack_data(ptr, params->p, params->p_size);
+	ptr = dh_pack_data(ptr, params->q, params->q_size);
 	dh_pack_data(ptr, params->g, params->g_size);
 
 	return 0;
@@ -79,6 +81,7 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 
 	ptr = dh_unpack_data(&params->key_size, ptr, sizeof(params->key_size));
 	ptr = dh_unpack_data(&params->p_size, ptr, sizeof(params->p_size));
+	ptr = dh_unpack_data(&params->q_size, ptr, sizeof(params->q_size));
 	ptr = dh_unpack_data(&params->g_size, ptr, sizeof(params->g_size));
 	if (secret.len != crypto_dh_key_len(params))
 		return -EINVAL;
@@ -88,7 +91,7 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	 * some drivers assume otherwise.
 	 */
 	if (params->key_size > params->p_size ||
-	    params->g_size > params->p_size)
+	    params->g_size > params->p_size || params->q_size > params->p_size)
 		return -EINVAL;
 
 	/* Don't allocate memory. Set pointers to data within
@@ -96,7 +99,9 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	 */
 	params->key = (void *)ptr;
 	params->p = (void *)(ptr + params->key_size);
-	params->g = (void *)(ptr + params->key_size + params->p_size);
+	params->q = (void *)(ptr + params->key_size + params->p_size);
+	params->g = (void *)(ptr + params->key_size + params->p_size +
+			     params->q_size);
 
 	/*
 	 * Don't permit 'p' to be 0.  It's not a prime number, and it's subject
@@ -106,6 +111,10 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	if (memchr_inv(params->p, 0, params->p_size) = NULL)
 		return -EINVAL;
 
+	/* It is permissible to not provide Q. */
+	if (params->q_size = 0)
+		params->q = NULL;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(crypto_dh_decode_key);
diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index 71e1bb24d79f..7e0dad94cb2b 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -29,17 +29,21 @@
  *
  * @key:	Private DH key
  * @p:		Diffie-Hellman parameter P
+ * @q:		Diffie-Hellman parameter Q
  * @g:		Diffie-Hellman generator G
  * @key_size:	Size of the private DH key
  * @p_size:	Size of DH parameter P
+ * @q_size:	Size of DH parameter Q
  * @g_size:	Size of DH generator G
  */
 struct dh {
 	void *key;
 	void *p;
+	void *q;
 	void *g;
 	unsigned int key_size;
 	unsigned int p_size;
+	unsigned int q_size;
 	unsigned int g_size;
 };
 
-- 
2.17.1





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

* Re: [RFC PATCH] crypto: DH - add public key verification test
  2018-06-26 10:17 [RFC PATCH] crypto: DH - add public key verification test Stephan Müller
@ 2018-06-26 15:13 ` Stephan Mueller
  2018-06-27  6:15 ` [PATCH v2] " Stephan Müller
  1 sibling, 0 replies; 10+ messages in thread
From: Stephan Mueller @ 2018-06-26 15:13 UTC (permalink / raw)
  To: keyrings

Am Dienstag, 26. Juni 2018, 12:17:42 CEST schrieb Stephan Müller:

Hi,

> +		MPI val = mpi_alloc(0);

I just saw that I did not check for val after allocation. That will be fixed 
in another installment of the patch.

Ciao
Stephan



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

* [PATCH v2] crypto: DH - add public key verification test
  2018-06-26 10:17 [RFC PATCH] crypto: DH - add public key verification test Stephan Müller
  2018-06-26 15:13 ` Stephan Mueller
@ 2018-06-27  6:15 ` Stephan Müller
  2018-07-08 16:44   ` Herbert Xu
                     ` (2 more replies)
  1 sibling, 3 replies; 10+ messages in thread
From: Stephan Müller @ 2018-06-27  6:15 UTC (permalink / raw)
  To: Stephan Müller; +Cc: herbert, linux-crypto, keyrings

Hi,

Changes v2:
* addition of a check that mpi_alloc succeeds.

---8<---

According to SP800-56A section 5.6.2.1, the public key to be processed
for the DH operation shall be checked for appropriateness. The check
shall covers the full verification test in case the domain parameter Q
is provided as defined in SP800-56A section 5.6.2.3.1. If Q is not
provided, the partial check according to SP800-56A section 5.6.2.3.2 is
performed.

The full verification test requires the presence of the domain parameter
Q. Thus, the patch adds the support to handle Q. It is permissible to
not provide the Q value as part of the domain parameters. This implies
that the interface is still backwards-compatible where so far only P and
G are to be provided. However, if Q is provided, it is imported.

Without the test, the NIST ACVP testing fails. After adding this check,
the NIST ACVP testing passes. Testing without providing the Q domain
parameter has been performed to verify the interface has not changed.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/dh.c         | 66 ++++++++++++++++++++++++++++++++++++++++++---
 crypto/dh_helper.c  | 15 ++++++++---
 include/crypto/dh.h |  4 +++
 3 files changed, 79 insertions(+), 6 deletions(-)

diff --git a/crypto/dh.c b/crypto/dh.c
index 5659fe7f446d..8f79269db2b7 100644
--- a/crypto/dh.c
+++ b/crypto/dh.c
@@ -16,14 +16,16 @@
 #include <linux/mpi.h>
 
 struct dh_ctx {
-	MPI p;
-	MPI g;
-	MPI xa;
+	MPI p;	/* Value is guaranteed to be set. */
+	MPI q;	/* Value is optional. */
+	MPI g;	/* Value is guaranteed to be set. */
+	MPI xa;	/* Value is guaranteed to be set. */
 };
 
 static void dh_clear_ctx(struct dh_ctx *ctx)
 {
 	mpi_free(ctx->p);
+	mpi_free(ctx->q);
 	mpi_free(ctx->g);
 	mpi_free(ctx->xa);
 	memset(ctx, 0, sizeof(*ctx));
@@ -60,6 +62,12 @@ static int dh_set_params(struct dh_ctx *ctx, struct dh *params)
 	if (!ctx->p)
 		return -EINVAL;
 
+	if (params->q && params->q_size) {
+		ctx->q = mpi_read_raw_data(params->q, params->q_size);
+		if (!ctx->q)
+			return -EINVAL;
+	}
+
 	ctx->g = mpi_read_raw_data(params->g, params->g_size);
 	if (!ctx->g)
 		return -EINVAL;
@@ -93,6 +101,55 @@ static int dh_set_secret(struct crypto_kpp *tfm, const void *buf,
 	return -EINVAL;
 }
 
+/*
+ * SP800-56A public key verification:
+ *
+ * * If Q is provided as part of the domain paramenters, a full validation
+ *   according to SP800-56A section 5.6.2.3.1 is performed.
+ *
+ * * If Q is not provided, a partial validation according to SP800-56A section
+ *   5.6.2.3.2 is performed.
+ */
+static int dh_is_pubkey_valid(struct dh_ctx *ctx, MPI y)
+{
+	if (unlikely(!ctx->p))
+		return -EINVAL;
+
+	/*
+	 * Step 1: Verify that 2 <= y <= p - 2.
+	 *
+	 * The upper limit check is actually y < p instead of y < p - 1
+	 * as the mpi_sub_ui function is yet missing.
+	 */
+	if (mpi_cmp_ui(y, 1) < 1 || mpi_cmp(y, ctx->p) >= 0)
+		return -EINVAL;
+
+	/* Step 2: Verify that 1 = y^q mod p */
+	if (ctx->q) {
+		MPI val = mpi_alloc(0);
+		int ret;
+
+		if (!val)
+			return -ENOMEM;
+
+		ret = mpi_powm(val, y, ctx->q, ctx->p);
+
+		if (ret) {
+			mpi_free(val);
+			return ret;
+		}
+
+		ret = mpi_cmp_ui(val, 1);
+
+		mpi_free(val);
+
+		if (ret != 0)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int dh_compute_value(struct kpp_request *req)
 {
 	struct crypto_kpp *tfm = crypto_kpp_reqtfm(req);
@@ -115,6 +172,9 @@ static int dh_compute_value(struct kpp_request *req)
 			ret = -EINVAL;
 			goto err_free_val;
 		}
+		ret = dh_is_pubkey_valid(ctx, base);
+		if (ret)
+			goto err_free_val;
 	} else {
 		base = ctx->g;
 	}
diff --git a/crypto/dh_helper.c b/crypto/dh_helper.c
index 24fdb2ecaa85..a7de3d9ce5ac 100644
--- a/crypto/dh_helper.c
+++ b/crypto/dh_helper.c
@@ -30,7 +30,7 @@ static inline const u8 *dh_unpack_data(void *dst, const void *src, size_t size)
 
 static inline unsigned int dh_data_size(const struct dh *p)
 {
-	return p->key_size + p->p_size + p->g_size;
+	return p->key_size + p->p_size + p->q_size + p->g_size;
 }
 
 unsigned int crypto_dh_key_len(const struct dh *p)
@@ -56,9 +56,11 @@ int crypto_dh_encode_key(char *buf, unsigned int len, const struct dh *params)
 	ptr = dh_pack_data(ptr, &secret, sizeof(secret));
 	ptr = dh_pack_data(ptr, &params->key_size, sizeof(params->key_size));
 	ptr = dh_pack_data(ptr, &params->p_size, sizeof(params->p_size));
+	ptr = dh_pack_data(ptr, &params->q_size, sizeof(params->q_size));
 	ptr = dh_pack_data(ptr, &params->g_size, sizeof(params->g_size));
 	ptr = dh_pack_data(ptr, params->key, params->key_size);
 	ptr = dh_pack_data(ptr, params->p, params->p_size);
+	ptr = dh_pack_data(ptr, params->q, params->q_size);
 	dh_pack_data(ptr, params->g, params->g_size);
 
 	return 0;
@@ -79,6 +81,7 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 
 	ptr = dh_unpack_data(&params->key_size, ptr, sizeof(params->key_size));
 	ptr = dh_unpack_data(&params->p_size, ptr, sizeof(params->p_size));
+	ptr = dh_unpack_data(&params->q_size, ptr, sizeof(params->q_size));
 	ptr = dh_unpack_data(&params->g_size, ptr, sizeof(params->g_size));
 	if (secret.len != crypto_dh_key_len(params))
 		return -EINVAL;
@@ -88,7 +91,7 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	 * some drivers assume otherwise.
 	 */
 	if (params->key_size > params->p_size ||
-	    params->g_size > params->p_size)
+	    params->g_size > params->p_size || params->q_size > params->p_size)
 		return -EINVAL;
 
 	/* Don't allocate memory. Set pointers to data within
@@ -96,7 +99,9 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	 */
 	params->key = (void *)ptr;
 	params->p = (void *)(ptr + params->key_size);
-	params->g = (void *)(ptr + params->key_size + params->p_size);
+	params->q = (void *)(ptr + params->key_size + params->p_size);
+	params->g = (void *)(ptr + params->key_size + params->p_size +
+			     params->q_size);
 
 	/*
 	 * Don't permit 'p' to be 0.  It's not a prime number, and it's subject
@@ -106,6 +111,10 @@ int crypto_dh_decode_key(const char *buf, unsigned int len, struct dh *params)
 	if (memchr_inv(params->p, 0, params->p_size) = NULL)
 		return -EINVAL;
 
+	/* It is permissible to not provide Q. */
+	if (params->q_size = 0)
+		params->q = NULL;
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(crypto_dh_decode_key);
diff --git a/include/crypto/dh.h b/include/crypto/dh.h
index 71e1bb24d79f..7e0dad94cb2b 100644
--- a/include/crypto/dh.h
+++ b/include/crypto/dh.h
@@ -29,17 +29,21 @@
  *
  * @key:	Private DH key
  * @p:		Diffie-Hellman parameter P
+ * @q:		Diffie-Hellman parameter Q
  * @g:		Diffie-Hellman generator G
  * @key_size:	Size of the private DH key
  * @p_size:	Size of DH parameter P
+ * @q_size:	Size of DH parameter Q
  * @g_size:	Size of DH generator G
  */
 struct dh {
 	void *key;
 	void *p;
+	void *q;
 	void *g;
 	unsigned int key_size;
 	unsigned int p_size;
+	unsigned int q_size;
 	unsigned int g_size;
 };
 
-- 
2.17.1





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

* Re: [PATCH v2] crypto: DH - add public key verification test
  2018-06-27  6:15 ` [PATCH v2] " Stephan Müller
@ 2018-07-08 16:44   ` Herbert Xu
  2018-07-11  4:10   ` Eric Biggers
  2018-07-11 15:47   ` [PATCH v2] crypto: DH - add public key verification test Stephan Müller
  2 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2018-07-08 16:44 UTC (permalink / raw)
  To: keyrings

On Wed, Jun 27, 2018 at 08:15:31AM +0200, Stephan Müller wrote:
> Hi,
> 
> Changes v2:
> * addition of a check that mpi_alloc succeeds.
> 
> ---8<---
> 
> According to SP800-56A section 5.6.2.1, the public key to be processed
> for the DH operation shall be checked for appropriateness. The check
> shall covers the full verification test in case the domain parameter Q
> is provided as defined in SP800-56A section 5.6.2.3.1. If Q is not
> provided, the partial check according to SP800-56A section 5.6.2.3.2 is
> performed.
> 
> The full verification test requires the presence of the domain parameter
> Q. Thus, the patch adds the support to handle Q. It is permissible to
> not provide the Q value as part of the domain parameters. This implies
> that the interface is still backwards-compatible where so far only P and
> G are to be provided. However, if Q is provided, it is imported.
> 
> Without the test, the NIST ACVP testing fails. After adding this check,
> the NIST ACVP testing passes. Testing without providing the Q domain
> parameter has been performed to verify the interface has not changed.
> 
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
>  crypto/dh.c         | 66 ++++++++++++++++++++++++++++++++++++++++++---
>  crypto/dh_helper.c  | 15 ++++++++---
>  include/crypto/dh.h |  4 +++
>  3 files changed, 79 insertions(+), 6 deletions(-)

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

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

* Re: [PATCH v2] crypto: DH - add public key verification test
  2018-06-27  6:15 ` [PATCH v2] " Stephan Müller
  2018-07-08 16:44   ` Herbert Xu
@ 2018-07-11  4:10   ` Eric Biggers
  2018-07-11 18:35     ` [PATCH 1/2] crypto: DH - update test for public key verification Stephan Müller
  2018-07-11 18:36     ` [PATCH 2/2] crypto: ECDH - fix typo of P-192 b value Stephan Müller
  2018-07-11 15:47   ` [PATCH v2] crypto: DH - add public key verification test Stephan Müller
  2 siblings, 2 replies; 10+ messages in thread
From: Eric Biggers @ 2018-07-11  4:10 UTC (permalink / raw)
  To: keyrings

Hi Stephan,

On Wed, Jun 27, 2018 at 08:15:31AM +0200, Stephan Müller wrote:
> Hi,
> 
> Changes v2:
> * addition of a check that mpi_alloc succeeds.
> 
> ---8<---
> 
> According to SP800-56A section 5.6.2.1, the public key to be processed
> for the DH operation shall be checked for appropriateness. The check
> shall covers the full verification test in case the domain parameter Q
> is provided as defined in SP800-56A section 5.6.2.3.1. If Q is not
> provided, the partial check according to SP800-56A section 5.6.2.3.2 is
> performed.
> 
> The full verification test requires the presence of the domain parameter
> Q. Thus, the patch adds the support to handle Q. It is permissible to
> not provide the Q value as part of the domain parameters. This implies
> that the interface is still backwards-compatible where so far only P and
> G are to be provided. However, if Q is provided, it is imported.
> 
> Without the test, the NIST ACVP testing fails. After adding this check,
> the NIST ACVP testing passes. Testing without providing the Q domain
> parameter has been performed to verify the interface has not changed.

You forgot to update the self-tests in the kernel, so they're failing now, as
you *did* change the interface (the "key" is encoded differently now).

- Eric

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

* Re: [PATCH v2] crypto: DH - add public key verification test
  2018-06-27  6:15 ` [PATCH v2] " Stephan Müller
  2018-07-08 16:44   ` Herbert Xu
  2018-07-11  4:10   ` Eric Biggers
@ 2018-07-11 15:47   ` Stephan Müller
  2 siblings, 0 replies; 10+ messages in thread
From: Stephan Müller @ 2018-07-11 15:47 UTC (permalink / raw)
  To: keyrings

Am Mittwoch, 11. Juli 2018, 06:10:59 CEST schrieb Eric Biggers:

Hi Eric,

> You forgot to update the self-tests in the kernel, so they're failing now,
> as you *did* change the interface (the "key" is encoded differently now).

I was actually looking for failing self tests. But accidentally I disabled the 
self tests in my test bed which I just saw now. :-(

I will provide a fix shortly.

Ciao
Stephan



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

* [PATCH 1/2] crypto: DH - update test for public key verification
  2018-07-11  4:10   ` Eric Biggers
@ 2018-07-11 18:35     ` Stephan Müller
  2018-07-20  5:54       ` Herbert Xu
  2018-07-11 18:36     ` [PATCH 2/2] crypto: ECDH - fix typo of P-192 b value Stephan Müller
  1 sibling, 1 reply; 10+ messages in thread
From: Stephan Müller @ 2018-07-11 18:35 UTC (permalink / raw)
  To: Eric Biggers; +Cc: herbert, linux-crypto, keyrings

By adding a zero byte-length for the DH parameter Q value, the public
key verification test is disabled for the given test.

Reported-by: Eric Biggers <ebiggers3@gmail.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/testmgr.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/crypto/testmgr.h b/crypto/testmgr.h
index b6362169771a..759462d65f41 100644
--- a/crypto/testmgr.h
+++ b/crypto/testmgr.h
@@ -644,12 +644,14 @@ static const struct kpp_testvec dh_tv_template[] = {
 	"\x11\x02" /* len */
 	"\x00\x01\x00\x00" /* key_size */
 	"\x00\x01\x00\x00" /* p_size */
+	"\x00\x00\x00\x00" /* q_size */
 	"\x01\x00\x00\x00" /* g_size */
 #else
 	"\x00\x01" /* type */
 	"\x02\x11" /* len */
 	"\x00\x00\x01\x00" /* key_size */
 	"\x00\x00\x01\x00" /* p_size */
+	"\x00\x00\x00\x00" /* q_size */
 	"\x00\x00\x00\x01" /* g_size */
 #endif
 	/* xa */
@@ -751,12 +753,14 @@ static const struct kpp_testvec dh_tv_template[] = {
 	"\x11\x02" /* len */
 	"\x00\x01\x00\x00" /* key_size */
 	"\x00\x01\x00\x00" /* p_size */
+	"\x00\x00\x00\x00" /* q_size */
 	"\x01\x00\x00\x00" /* g_size */
 #else
 	"\x00\x01" /* type */
 	"\x02\x11" /* len */
 	"\x00\x00\x01\x00" /* key_size */
 	"\x00\x00\x01\x00" /* p_size */
+	"\x00\x00\x00\x00" /* q_size */
 	"\x00\x00\x00\x01" /* g_size */
 #endif
 	/* xa */
-- 
2.17.1





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

* [PATCH 2/2] crypto: ECDH - fix typo of P-192 b value
  2018-07-11  4:10   ` Eric Biggers
  2018-07-11 18:35     ` [PATCH 1/2] crypto: DH - update test for public key verification Stephan Müller
@ 2018-07-11 18:36     ` Stephan Müller
  2018-07-20  5:55       ` Herbert Xu
  1 sibling, 1 reply; 10+ messages in thread
From: Stephan Müller @ 2018-07-11 18:36 UTC (permalink / raw)
  To: Eric Biggers; +Cc: herbert, linux-crypto, keyrings

Fix the b value to be compliant with FIPS 186-4 D.1.2.1. This fix is
required to make sure the SP800-56A public key test passes for P-192.

Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
 crypto/ecc_curve_defs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/ecc_curve_defs.h b/crypto/ecc_curve_defs.h
index 94e883a9403f..336ab1805639 100644
--- a/crypto/ecc_curve_defs.h
+++ b/crypto/ecc_curve_defs.h
@@ -27,7 +27,7 @@ static u64 nist_p192_p[] = { 0xFFFFFFFFFFFFFFFFull, 0xFFFFFFFFFFFFFFFEull,
 static u64 nist_p192_n[] = { 0x146BC9B1B4D22831ull, 0xFFFFFFFF99DEF836ull,
 				0xFFFFFFFFFFFFFFFFull };
 static u64 nist_p192_a[] = { 0xFFFFFFFFFFFFFFFCull, 0xFFFFFFFFFFFFFFFEull,
-				0xFFFFFFFFFFFFFFFEull };
+				0xFFFFFFFFFFFFFFFFull };
 static u64 nist_p192_b[] = { 0xFEB8DEECC146B9B1ull, 0x0FA7E9AB72243049ull,
 				0x64210519E59C80E7ull };
 static struct ecc_curve nist_p192 = {
-- 
2.17.1





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

* Re: [PATCH 1/2] crypto: DH - update test for public key verification
  2018-07-11 18:35     ` [PATCH 1/2] crypto: DH - update test for public key verification Stephan Müller
@ 2018-07-20  5:54       ` Herbert Xu
  0 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2018-07-20  5:54 UTC (permalink / raw)
  To: keyrings

On Wed, Jul 11, 2018 at 08:35:49PM +0200, Stephan Müller wrote:
> By adding a zero byte-length for the DH parameter Q value, the public
> key verification test is disabled for the given test.
> 
> Reported-by: Eric Biggers <ebiggers3@gmail.com>
> Signed-off-by: Stephan Mueller <smueller@chronox.de>

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

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

* Re: [PATCH 2/2] crypto: ECDH - fix typo of P-192 b value
  2018-07-11 18:36     ` [PATCH 2/2] crypto: ECDH - fix typo of P-192 b value Stephan Müller
@ 2018-07-20  5:55       ` Herbert Xu
  0 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2018-07-20  5:55 UTC (permalink / raw)
  To: keyrings

On Wed, Jul 11, 2018 at 08:36:23PM +0200, Stephan Müller wrote:
> Fix the b value to be compliant with FIPS 186-4 D.1.2.1. This fix is
> required to make sure the SP800-56A public key test passes for P-192.
> 
> Signed-off-by: Stephan Mueller <smueller@chronox.de>

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

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

end of thread, other threads:[~2018-07-20  5:55 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-06-26 10:17 [RFC PATCH] crypto: DH - add public key verification test Stephan Müller
2018-06-26 15:13 ` Stephan Mueller
2018-06-27  6:15 ` [PATCH v2] " Stephan Müller
2018-07-08 16:44   ` Herbert Xu
2018-07-11  4:10   ` Eric Biggers
2018-07-11 18:35     ` [PATCH 1/2] crypto: DH - update test for public key verification Stephan Müller
2018-07-20  5:54       ` Herbert Xu
2018-07-11 18:36     ` [PATCH 2/2] crypto: ECDH - fix typo of P-192 b value Stephan Müller
2018-07-20  5:55       ` Herbert Xu
2018-07-11 15:47   ` [PATCH v2] crypto: DH - add public key verification test Stephan Müller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.