Add FFC param/key validation

Embed libctx in dsa and dh objects and cleanup internal methods to not pass libctx (This makes it consistent with the rsa changes)

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10910)

6 files changed, 201 insertions, 100 deletions

diff --git a/crypto/dh/dh_check.c b/crypto/dh/dh_check.c
index 8bb2452..4832230 100644
--- a/crypto/dh/dh_check.c
+++ b/crypto/dh/dh_check.c
@@ -11,6 +11,7 @@
 #include "internal/cryptlib.h"
 #include <openssl/bn.h>
 #include "dh_local.h"
+#include "crypto/dh.h"
 
 /*-
  * Check that p and g are suitable enough
@@ -37,6 +38,28 @@ int DH_check_params_ex(const DH *dh)
     return errflags == 0;
 }
 
+#ifdef FIPS_MODE
+int DH_check_params(const DH *dh, int *ret)
+{
+    int nid;
+
+    *ret = 0;
+    /*
+     * SP800-56A R3 Section 5.5.2 Assurances of Domain Parameter Validity
+     * (1a) The domain parameters correspond to any approved safe prime group.
+     */
+    nid = DH_get_nid((DH *)dh);
+    if (nid != NID_undef)
+        return 1;
+    /*
+     * OR
+     * (2b) FFC domain params conform to FIPS-186-4 explicit domain param
+     * validity tests.
+     */
+    return ffc_params_FIPS186_4_validate(&dh->params, FFC_PARAM_TYPE_DH, NULL,
+                                         FFC_PARAMS_VALIDATE_ALL, ret, NULL);
+}
+#else
 int DH_check_params(const DH *dh, int *ret)
 {
     int ok = 0;
@@ -73,6 +96,7 @@ int DH_check_params(const DH *dh, int *ret)
     BN_CTX_free(ctx);
     return ok;
 }
+#endif /* FIPS_MODE */
 
 /*-
  * Check that p is a safe prime and
@@ -107,11 +131,20 @@ int DH_check_ex(const DH *dh)
     return errflags == 0;
 }
 
+/* Note: according to documentation - this only checks the params */
 int DH_check(const DH *dh, int *ret)
 {
+#ifdef FIPS_MODE
+    return DH_check_params(dh, ret);
+#else
     int ok = 0, r;
     BN_CTX *ctx = NULL;
     BIGNUM *t1 = NULL, *t2 = NULL;
+    int nid = DH_get_nid((DH *)dh);
+
+    *ret = 0;
+    if (nid != NID_undef)
+        return 1;
 
     if (!DH_check_params(dh, ret))
         return 0;
@@ -171,6 +204,7 @@ int DH_check(const DH *dh, int *ret)
     BN_CTX_end(ctx);
     BN_CTX_free(ctx);
     return ok;
+#endif /* FIPS_MODE */
 }
 
 int DH_check_pub_key_ex(const DH *dh, const BIGNUM *pub_key)
@@ -190,38 +224,83 @@ int DH_check_pub_key_ex(const DH *dh, const BIGNUM *pub_key)
     return errflags == 0;
 }
 
+/*
+ * See SP800-56Ar3 Section 5.6.2.3.1 : FFC Full public key validation.
+ */
 int DH_check_pub_key(const DH *dh, const BIGNUM *pub_key, int *ret)
 {
+    return ffc_validate_public_key(&dh->params, pub_key, ret);
+}
+
+/*
+ * See SP800-56Ar3 Section 5.6.2.3.1 : FFC Partial public key validation.
+ * To only be used with ephemeral FFC public keys generated using the approved
+ * safe-prime groups.
+ */
+int dh_check_pub_key_partial(const DH *dh, const BIGNUM *pub_key, int *ret)
+{
+    return ffc_validate_public_key_partial(&dh->params, pub_key, ret);
+}
+
+int dh_check_priv_key(const DH *dh, const BIGNUM *priv_key, int *ret)
+{
     int ok = 0;
-    BIGNUM *tmp = NULL;
-    BN_CTX *ctx = NULL;
+    BIGNUM *two_powN = NULL, *upper;
 
     *ret = 0;
-    ctx = BN_CTX_new();
-    if (ctx == NULL)
-        goto err;
-    BN_CTX_start(ctx);
-    tmp = BN_CTX_get(ctx);
-    if (tmp == NULL || !BN_set_word(tmp, 1))
-        goto err;
-    if (BN_cmp(pub_key, tmp) <= 0)
-        *ret |= DH_CHECK_PUBKEY_TOO_SMALL;
-    if (BN_copy(tmp, dh->params.p) == NULL || !BN_sub_word(tmp, 1))
+    two_powN = BN_new();
+    if (two_powN == NULL)
+        return 0;
+    if (dh->params.q == NULL)
         goto err;
-    if (BN_cmp(pub_key, tmp) >= 0)
-        *ret |= DH_CHECK_PUBKEY_TOO_LARGE;
+    upper = dh->params.q;
 
-    if (dh->params.q != NULL) {
-        /* Check pub_key^q == 1 mod p */
-        if (!BN_mod_exp(tmp, pub_key, dh->params.q, dh->params.p, ctx))
+    /* Is it from an approved Safe prime group ?*/
+    if (DH_get_nid((DH *)dh) != NID_undef) {
+        if (!BN_lshift(two_powN, BN_value_one(), dh->length))
             goto err;
-        if (!BN_is_one(tmp))
-            *ret |= DH_CHECK_PUBKEY_INVALID;
+        if (BN_cmp(two_powN, dh->params.q) < 0)
+            upper = two_powN;
     }
+    if (!ffc_validate_private_key(upper, priv_key, ret))
+        goto err;
 
     ok = 1;
- err:
-    BN_CTX_end(ctx);
-    BN_CTX_free(ctx);
+err:
+    BN_free(two_powN);
     return ok;
 }
+
+/*
+ * FFC pairwise check from SP800-56A R3.
+ *    Section 5.6.2.1.4 Owner Assurance of Pair-wise Consistency
+ */
+int dh_check_pairwise(DH *dh)
+{
+    int ret = 0;
+    BN_CTX *ctx = NULL;
+    BIGNUM *pub_key = NULL;
+
+    if (dh->params.p == NULL
+        || dh->params.g == NULL
+        || dh->priv_key == NULL
+        || dh->pub_key == NULL)
+        return 0;
+
+    ctx = BN_CTX_new_ex(dh->libctx);
+    if (ctx == NULL)
+        goto err;
+    pub_key = BN_new();
+    if (pub_key == NULL)
+        goto err;
+
+    /* recalculate the public key = (g ^ priv) mod p */
+    if (!dh_generate_public_key(ctx, dh, dh->priv_key, pub_key))
+        goto err;
+    /* check it matches the existing pubic_key */
+    ret = BN_cmp(pub_key, dh->pub_key) == 0;
+err:
+    BN_free(pub_key);
+    BN_CTX_free(ctx);
+    return ret;
+}
diff --git a/crypto/dh/dh_gen.c b/crypto/dh/dh_gen.c
index 3d3bcb2..89264e9 100644
--- a/crypto/dh/dh_gen.c
+++ b/crypto/dh/dh_gen.c
@@ -27,7 +27,7 @@ static int dh_builtin_genparams(DH *ret, int prime_len, int generator,
  * TODO(3.0): keygen should be able to use this method to do a FIPS186-4 style
  * paramgen.
  */
-int dh_generate_ffc_parameters(OPENSSL_CTX *libctx, DH *dh, int bits,
+int dh_generate_ffc_parameters(DH *dh, int bits,
                                int qbits, int gindex, BN_GENCB *cb)
 {
     int ret, res;
@@ -38,7 +38,8 @@ int dh_generate_ffc_parameters(OPENSSL_CTX *libctx, DH *dh, int bits,
         qbits = EVP_MD_size(evpmd) * 8;
     }
     dh->params.gindex = gindex;
-    ret = ffc_params_FIPS186_4_generate(libctx, &dh->params, FFC_PARAM_TYPE_DH,
+    ret = ffc_params_FIPS186_4_generate(dh->libctx, &dh->params,
+                                        FFC_PARAM_TYPE_DH,
                                         bits, qbits, NULL, &res, cb);
     if (ret > 0)
         dh->dirty_cnt++;
diff --git a/crypto/dh/dh_group_params.c b/crypto/dh/dh_group_params.c
index 4996e85..6c057d1 100644
--- a/crypto/dh/dh_group_params.c
+++ b/crypto/dh/dh_group_params.c
@@ -15,11 +15,16 @@
 #include <openssl/bn.h>
 #include <openssl/objects.h>
 #include "crypto/bn_dh.h"
+#include "crypto/dh.h"
 
-static DH *dh_param_init(int nid, const BIGNUM *p, int32_t nbits)
+#ifndef FIPS_MODE
+static DH *dh_new_by_nid_with_ctx(OPENSSL_CTX *libctx, int nid);
+
+static DH *dh_param_init(OPENSSL_CTX *libctx, int nid, const BIGNUM *p,
+                         int32_t nbits)
 {
     BIGNUM *q = NULL;
-    DH *dh = DH_new();
+    DH *dh = dh_new_with_ctx(libctx);
 
     if (dh == NULL)
         return NULL;
@@ -41,7 +46,7 @@ static DH *dh_param_init(int nid, const BIGNUM *p, int32_t nbits)
     return dh;
 }
 
-DH *DH_new_by_nid(int nid)
+static DH *dh_new_by_nid_with_ctx(OPENSSL_CTX *libctx, int nid)
 {
     /*
      * The last parameter specified in these fields is
@@ -50,35 +55,41 @@ DH *DH_new_by_nid(int nid)
      */
     switch (nid) {
     case NID_ffdhe2048:
-        return dh_param_init(nid, &_bignum_ffdhe2048_p, 225);
+        return dh_param_init(libctx, nid, &_bignum_ffdhe2048_p, 225);
     case NID_ffdhe3072:
-        return dh_param_init(nid, &_bignum_ffdhe3072_p, 275);
+        return dh_param_init(libctx, nid, &_bignum_ffdhe3072_p, 275);
     case NID_ffdhe4096:
-        return dh_param_init(nid, &_bignum_ffdhe4096_p, 325);
+        return dh_param_init(libctx, nid, &_bignum_ffdhe4096_p, 325);
     case NID_ffdhe6144:
-        return dh_param_init(nid, &_bignum_ffdhe6144_p, 375);
+        return dh_param_init(libctx, nid, &_bignum_ffdhe6144_p, 375);
     case NID_ffdhe8192:
-        return dh_param_init(nid, &_bignum_ffdhe8192_p, 400);
+        return dh_param_init(libctx, nid, &_bignum_ffdhe8192_p, 400);
 #ifndef FIPS_MODE
     case NID_modp_1536:
-        return dh_param_init(nid, &_bignum_modp_1536_p, 190);
+        return dh_param_init(libctx, nid, &_bignum_modp_1536_p, 190);
 #endif
     case NID_modp_2048:
-        return dh_param_init(nid, &_bignum_modp_2048_p, 225);
+        return dh_param_init(libctx, nid, &_bignum_modp_2048_p, 225);
     case NID_modp_3072:
-        return dh_param_init(nid, &_bignum_modp_3072_p, 275);
+        return dh_param_init(libctx, nid, &_bignum_modp_3072_p, 275);
     case NID_modp_4096:
-        return dh_param_init(nid, &_bignum_modp_4096_p, 325);
+        return dh_param_init(libctx, nid, &_bignum_modp_4096_p, 325);
     case NID_modp_6144:
-        return dh_param_init(nid, &_bignum_modp_6144_p, 375);
+        return dh_param_init(libctx, nid, &_bignum_modp_6144_p, 375);
     case NID_modp_8192:
-        return dh_param_init(nid, &_bignum_modp_8192_p, 400);
+        return dh_param_init(libctx, nid, &_bignum_modp_8192_p, 400);
     default:
-        DHerr(DH_F_DH_NEW_BY_NID, DH_R_INVALID_PARAMETER_NID);
+        DHerr(0, DH_R_INVALID_PARAMETER_NID);
         return NULL;
     }
 }
 
+DH *DH_new_by_nid(int nid)
+{
+    return dh_new_by_nid_with_ctx(NULL, nid);
+}
+#endif
+
 int DH_get_nid(DH *dh)
 {
     int nid = dh->params.nid;
diff --git a/crypto/dh/dh_key.c b/crypto/dh/dh_key.c
index 0bee75c..14d3546 100644
--- a/crypto/dh/dh_key.c
+++ b/crypto/dh/dh_key.c
@@ -20,8 +20,7 @@ static int dh_bn_mod_exp(const DH *dh, BIGNUM *r,
 static int dh_init(DH *dh);
 static int dh_finish(DH *dh);
 
-int dh_compute_key(OPENSSL_CTX *libctx, unsigned char *key,
-                   const BIGNUM *pub_key, DH *dh)
+static int compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh)
 {
     BN_CTX *ctx = NULL;
     BN_MONT_CTX *mont = NULL;
@@ -41,7 +40,7 @@ int dh_compute_key(OPENSSL_CTX *libctx, unsigned char *key,
         return 0;
     }
 
-    ctx = BN_CTX_new_ex(libctx);
+    ctx = BN_CTX_new_ex(dh->libctx);
     if (ctx == NULL)
         goto err;
     BN_CTX_start(ctx);
@@ -81,18 +80,21 @@ int dh_compute_key(OPENSSL_CTX *libctx, unsigned char *key,
     return ret;
 }
 
-static int compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh)
+int DH_compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh)
 {
-    return dh_compute_key(NULL, key, pub_key, dh);
+#ifdef FIPS_MODE
+    return compute_key(key, pub_key, dh);
+#else
+    return dh->meth->compute_key(key, pub_key, dh);
+#endif
 }
 
-int dh_compute_key_padded(OPENSSL_CTX *libctx, unsigned char *key,
-                          const BIGNUM *pub_key, DH *dh)
+int DH_compute_key_padded(unsigned char *key, const BIGNUM *pub_key, DH *dh)
 {
     int rv, pad;
 
 #ifdef FIPS_MODE
-    rv = dh_compute_key(libctx, key, pub_key, dh);
+    rv = compute_key(key, pub_key, dh);
 #else
     rv = dh->meth->compute_key(key, pub_key, dh);
 #endif
@@ -106,18 +108,6 @@ int dh_compute_key_padded(OPENSSL_CTX *libctx, unsigned char *key,
     return rv + pad;
 }
 
-#ifndef FIPS_MODE
-int DH_compute_key(unsigned char *key, const BIGNUM *pub_key, DH *dh)
-{
-    return dh->meth->compute_key(key, pub_key, dh);
-}
-
-int DH_compute_key_padded(unsigned char *key, const BIGNUM *pub_key, DH *dh)
-{
-    return dh_compute_key_padded(NULL, key, pub_key, dh);
-}
-#endif
-
 static DH_METHOD dh_ossl = {
     "OpenSSL DH Method",
     generate_key,
@@ -168,14 +158,46 @@ void DH_set_default_method(const DH_METHOD *meth)
 {
     default_DH_method = meth;
 }
+#endif /* FIPS_MODE */
 
 int DH_generate_key(DH *dh)
 {
+#ifdef FIPS_MODE
+    return generate_key(dh);
+#else
     return dh->meth->generate_key(dh);
+#endif
 }
-#endif /* FIPS_MODE */
 
-static int dh_generate_key(OPENSSL_CTX *libctx, DH *dh)
+int dh_generate_public_key(BN_CTX *ctx, DH *dh, const BIGNUM *priv_key,
+                           BIGNUM *pub_key)
+{
+    int ret = 0;
+    BIGNUM *prk = BN_new();
+    BN_MONT_CTX *mont = NULL;
+
+    if (prk == NULL)
+        return 0;
+
+    if (dh->flags & DH_FLAG_CACHE_MONT_P) {
+        mont = BN_MONT_CTX_set_locked(&dh->method_mont_p,
+                                      dh->lock, dh->params.p, ctx);
+        if (mont == NULL)
+            goto err;
+    }
+    BN_with_flags(prk, priv_key, BN_FLG_CONSTTIME);
+
+    /* pub_key = g^priv_key mod p */
+    if (!dh->meth->bn_mod_exp(dh, pub_key, dh->params.g, prk, dh->params.p,
+                              ctx, mont))
+        goto err;
+    ret = 1;
+err:
+    BN_clear_free(prk);
+    return ret;
+}
+
+static int generate_key(DH *dh)
 {
     int ok = 0;
     int generate_new_key = 0;
@@ -183,7 +205,6 @@ static int dh_generate_key(OPENSSL_CTX *libctx, DH *dh)
     unsigned l;
 #endif
     BN_CTX *ctx = NULL;
-    BN_MONT_CTX *mont = NULL;
     BIGNUM *pub_key = NULL, *priv_key = NULL;
 
     if (BN_num_bits(dh->params.p) > OPENSSL_DH_MAX_MODULUS_BITS) {
@@ -196,7 +217,7 @@ static int dh_generate_key(OPENSSL_CTX *libctx, DH *dh)
         return 0;
     }
 
-    ctx = BN_CTX_new_ex(libctx);
+    ctx = BN_CTX_new_ex(dh->libctx);
     if (ctx == NULL)
         goto err;
 
@@ -205,23 +226,17 @@ static int dh_generate_key(OPENSSL_CTX *libctx, DH *dh)
         if (priv_key == NULL)
             goto err;
         generate_new_key = 1;
-    } else
+    } else {
         priv_key = dh->priv_key;
+    }
 
     if (dh->pub_key == NULL) {
         pub_key = BN_new();
         if (pub_key == NULL)
             goto err;
-    } else
+    } else {
         pub_key = dh->pub_key;
-
-    if (dh->flags & DH_FLAG_CACHE_MONT_P) {
-        mont = BN_MONT_CTX_set_locked(&dh->method_mont_p,
-                                      dh->lock, dh->params.p, ctx);
-        if (!mont)
-            goto err;
     }
-
     if (generate_new_key) {
         /* Is it an approved safe prime ?*/
         if (DH_get_nid(dh) != NID_undef) {
@@ -274,22 +289,8 @@ static int dh_generate_key(OPENSSL_CTX *libctx, DH *dh)
         }
     }
 
-    {
-        BIGNUM *prk = BN_new();
-
-        if (prk == NULL)
-            goto err;
-        BN_with_flags(prk, priv_key, BN_FLG_CONSTTIME);
-
-        /* pub_key = g^priv_key mod p */
-        if (!dh->meth->bn_mod_exp(dh, pub_key, dh->params.g, prk, dh->params.p,
-                                  ctx, mont)) {
-            BN_clear_free(prk);
-            goto err;
-        }
-        /* We MUST free prk before any further use of priv_key */
-        BN_clear_free(prk);
-    }
+    if (!dh_generate_public_key(ctx, dh, priv_key, pub_key))
+        goto err;
 
     dh->pub_key = pub_key;
     dh->priv_key = priv_key;
@@ -307,11 +308,6 @@ static int dh_generate_key(OPENSSL_CTX *libctx, DH *dh)
     return ok;
 }
 
-static int generate_key(DH *dh)
-{
-    return dh_generate_key(NULL, dh);
-}
-
 int dh_buf2key(DH *dh, const unsigned char *buf, size_t len)
 {
     int err_reason = DH_R_BN_ERROR;
diff --git a/crypto/dh/dh_lib.c b/crypto/dh/dh_lib.c
index 0c1cccb..e6fc3ef 100644
--- a/crypto/dh/dh_lib.c
+++ b/crypto/dh/dh_lib.c
@@ -16,6 +16,8 @@
 #include "crypto/dh.h"
 #include "dh_local.h"
 
+static DH *dh_new_intern(ENGINE *engine, OPENSSL_CTX *libctx);
+
 #ifndef FIPS_MODE
 int DH_set_method(DH *dh, const DH_METHOD *meth)
 {
@@ -36,36 +38,47 @@ int DH_set_method(DH *dh, const DH_METHOD *meth)
         meth->init(dh);
     return 1;
 }
-#endif /* !FIPS_MODE */
 
 DH *DH_new(void)
 {
-    return DH_new_method(NULL);
+    return dh_new_intern(NULL, NULL);
 }
 
 DH *DH_new_method(ENGINE *engine)
 {
+    return dh_new_intern(engine, NULL);
+}
+#endif /* !FIPS_MODE */
+
+DH *dh_new_with_ctx(OPENSSL_CTX *libctx)
+{
+    return dh_new_intern(NULL, libctx);
+}
+
+static DH *dh_new_intern(ENGINE *engine, OPENSSL_CTX *libctx)
+{
     DH *ret = OPENSSL_zalloc(sizeof(*ret));
 
     if (ret == NULL) {
-        DHerr(DH_F_DH_NEW_METHOD, ERR_R_MALLOC_FAILURE);
+        DHerr(0, ERR_R_MALLOC_FAILURE);
         return NULL;
     }
 
     ret->references = 1;
     ret->lock = CRYPTO_THREAD_lock_new();
     if (ret->lock == NULL) {
-        DHerr(DH_F_DH_NEW_METHOD, ERR_R_MALLOC_FAILURE);
+        DHerr(0, ERR_R_MALLOC_FAILURE);
         OPENSSL_free(ret);
         return NULL;
     }
 
+    ret->libctx = libctx;
     ret->meth = DH_get_default_method();
 #if !defined(FIPS_MODE) && !defined(OPENSSL_NO_ENGINE)
     ret->flags = ret->meth->flags;  /* early default init */
     if (engine) {
         if (!ENGINE_init(engine)) {
-            DHerr(DH_F_DH_NEW_METHOD, ERR_R_ENGINE_LIB);
+            DHerr(0, ERR_R_ENGINE_LIB);
             goto err;
         }
         ret->engine = engine;
@@ -74,7 +87,7 @@ DH *DH_new_method(ENGINE *engine)
     if (ret->engine) {
         ret->meth = ENGINE_get_DH(ret->engine);
         if (ret->meth == NULL) {
-            DHerr(DH_F_DH_NEW_METHOD, ERR_R_ENGINE_LIB);
+            DHerr(0, ERR_R_ENGINE_LIB);
             goto err;
         }
     }
@@ -88,7 +101,7 @@ DH *DH_new_method(ENGINE *engine)
 #endif /* FIPS_MODE */
 
     if ((ret->meth->init != NULL) && !ret->meth->init(ret)) {
-        DHerr(DH_F_DH_NEW_METHOD, ERR_R_INIT_FAIL);
+        DHerr(0, ERR_R_INIT_FAIL);
         goto err;
     }
 
diff --git a/crypto/dh/dh_local.h b/crypto/dh/dh_local.h
index 57d9d34..7fefcf7 100644
--- a/crypto/dh/dh_local.h
+++ b/crypto/dh/dh_local.h
@@ -31,6 +31,7 @@ struct dh_st {
     CRYPTO_EX_DATA ex_data;
     ENGINE *engine;
 #endif
+    OPENSSL_CTX *libctx;
     const DH_METHOD *meth;
     CRYPTO_RWLOCK *lock;