From patchwork Tue Jun 26 10:17:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephan Mueller X-Patchwork-Id: 10488591 X-Patchwork-Delegate: herbert@gondor.apana.org.au Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 27F2A602B3 for ; Tue, 26 Jun 2018 10:17:57 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 2043F2887D for ; Tue, 26 Jun 2018 10:17:57 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 12B7F28881; Tue, 26 Jun 2018 10:17:57 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 7B4112887D for ; Tue, 26 Jun 2018 10:17:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934573AbeFZKRv (ORCPT ); Tue, 26 Jun 2018 06:17:51 -0400 Received: from mo4-p00-ob.smtp.rzone.de ([81.169.146.217]:34859 "EHLO mo4-p00-ob.smtp.rzone.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933509AbeFZKRt (ORCPT ); Tue, 26 Jun 2018 06:17:49 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; t=1530008264; s=strato-dkim-0002; d=chronox.de; h=Message-ID:Date:Subject:Cc:To:From:X-RZG-CLASS-ID:X-RZG-AUTH:From: Subject:Sender; bh=ykkqce+j8u73yBEKTBZmw1x/aZQhJ9vEsu+e9cra1fo=; b=SHQe4Se7Y3sJ4bkReH8w6yEOErtjGh2B89qdBNRXy9REstWia3hwcvo/X9ei8gZkpJ FxHP6RNQ2SAv1EVAEq1aelDBC1AjdZDtZVUGyDw0z6TYvmcs8PpftZlrJ6hz3maeVbNN 76gaJMb6OqJmgF5k1lFXHkWXZVOdN82Hwg7PfBhb4cCPm+BJ8ftqN+NJZ8xjiWflimot EgKN0Rb1h17+4UxnSzxR07PkR+zivznrqft31COhdP3tQB+nrXW8RlCihSaumA1BlhSj o7exFIspvMlY07XgY9EKfaEwhVmTQSR000UG6dQ28at/5NjkthRfPZ6klADd0r/bLXCi 1QKg== X-RZG-AUTH: ":P2ERcEykfu11Y98lp/T7+hdri+uKZK8TKWEqNyiHySGSa9k9zW4DNhHoQE+naq/TxnB4G17At7r7wShWoSPbxur/SZaB+xKVHcWbtg==" X-RZG-CLASS-ID: mo00 Received: from positron.chronox.de by smtp.strato.de (RZmta 43.10 AUTH) with ESMTPSA id j06b90u5QAHgnI1 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (curve secp521r1 with 521 ECDH bits, eq. 15360 bits RSA)) (Client did not present a certificate); Tue, 26 Jun 2018 12:17:42 +0200 (CEST) From: Stephan =?ISO-8859-1?Q?M=FCller?= To: herbert@gondor.apana.org.au Cc: linux-crypto@vger.kernel.org, keyrings@vger.kernel.org Subject: [RFC PATCH] crypto: DH - add public key verification test Date: Tue, 26 Jun 2018 12:17:42 +0200 Message-ID: <3594538.6iHA3aaNKK@positron.chronox.de> MIME-Version: 1.0 Sender: linux-crypto-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP 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 --- 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 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, ¶ms->key_size, sizeof(params->key_size)); ptr = dh_pack_data(ptr, ¶ms->p_size, sizeof(params->p_size)); + ptr = dh_pack_data(ptr, ¶ms->q_size, sizeof(params->q_size)); ptr = dh_pack_data(ptr, ¶ms->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(¶ms->key_size, ptr, sizeof(params->key_size)); ptr = dh_unpack_data(¶ms->p_size, ptr, sizeof(params->p_size)); + ptr = dh_unpack_data(¶ms->q_size, ptr, sizeof(params->q_size)); ptr = dh_unpack_data(¶ms->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; };