As the ->q in struct dh_ctx gets never set anywhere, the code in dh_is_pubkey_valid() for doing the full public key validation in accordance to SP800-56Arev3 is effectively dead. However, for safe-prime groups Q = (P - 1)/2 by definition and as the safe-prime groups are the only possible groups in FIPS mode (via those ffdheXYZ() templates), this enables dh_is_pubkey_valid() to calculate Q on the fly for these. Implement this. With this change, the last code accessing struct dh_ctx's ->q is now gone. Remove this member from struct dh_ctx. Signed-off-by: Nicolai Stange <nstange@xxxxxxx> --- crypto/dh.c | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/crypto/dh.c b/crypto/dh.c index d0d24f615b2d..cca289477485 100644 --- a/crypto/dh.c +++ b/crypto/dh.c @@ -15,7 +15,6 @@ struct dh_ctx { 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. */ }; @@ -23,7 +22,6 @@ struct dh_ctx { 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)); @@ -99,11 +97,12 @@ static int dh_set_secret(struct crypto_kpp *tfm, const void *buf, /* * 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. + * * For the safe-prime groups in FIPS mode, Q can be computed + * trivially from P and 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. + * * For all other sets of group parameters, only 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) { @@ -114,21 +113,40 @@ static int dh_is_pubkey_valid(struct dh_ctx *ctx, MPI y) * 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. + * in order to save one mpi_sub_ui() invocation here. Note that + * p - 1 is the non-trivial element of the subgroup of order 2 and + * thus, the check on y^q below would fail if y == p - 1. */ 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); + /* + * Step 2: Verify that 1 = y^q mod p + * + * For the safe-prime groups q = (p - 1)/2. + */ + if (fips_enabled) { + MPI val, q; int ret; + val = mpi_alloc(0); if (!val) return -ENOMEM; - ret = mpi_powm(val, y, ctx->q, ctx->p); + q = mpi_alloc(mpi_get_nlimbs(ctx->p)); + if (!q) { + mpi_free(val); + return -ENOMEM; + } + /* + * ->p is odd, so no need to explicitly subtract one + * from it before shifting to the right. + */ + mpi_rshift(q, ctx->p, 1); + + ret = mpi_powm(val, y, q, ctx->p); + mpi_free(q); if (ret) { mpi_free(val); return ret; -- 2.26.2