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 <[email protected]>
---
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
On 2/21/22 13:11, Nicolai Stange wrote:
> 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 <[email protected]>
> ---
> crypto/dh.c | 40 +++++++++++++++++++++++++++++-----------
> 1 file changed, 29 insertions(+), 11 deletions(-)
>
Reviewed-by: Hannes Reinecke <[email protected]>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
[email protected] +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), GF: Felix Imendörffer