Allow EVP_PKEY private key objects to be created without a public component
authorJon Spillett <jon.spillett@oracle.com>
Wed, 13 Jan 2021 04:10:51 +0000 (14:10 +1000)
committerTomas Mraz <tomas@openssl.org>
Fri, 15 Jan 2021 16:24:41 +0000 (17:24 +0100)
Reviewed-by: Richard Levitte <levitte@openssl.org>
Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
(Merged from https://github.com/openssl/openssl/pull/13855)

crypto/dh/dh_backend.c
crypto/dsa/dsa_backend.c
crypto/dsa/dsa_lib.c
crypto/ec/ec_backend.c
test/evp_extra_test.c

index 660bb4845ad25680faf82c9d990c34e57c9ccf63..6e545763dc2bd9f47f5459fd20de5db68d2fd110 100644 (file)
@@ -69,15 +69,6 @@ int dh_key_fromdata(DH *dh, const OSSL_PARAM params[])
     param_priv_key = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_PRIV_KEY);
     param_pub_key = OSSL_PARAM_locate_const(params, OSSL_PKEY_PARAM_PUB_KEY);
 
-    /*
-     * DH documentation says that a public key must be present if a
-     * private key is present.
-     * We want to have at least a public key either way, so we end up
-     * requiring it unconditionally.
-     */
-    if (param_priv_key != NULL && param_pub_key == NULL)
-        return 0;
-
     if ((param_priv_key != NULL
          && !OSSL_PARAM_get_BN(param_priv_key, &priv_key))
         || (param_pub_key != NULL
index 4809b3100b3ae43caae204e96710bc872690248b..6a053611e16a7e38cbc8f62b23d9ff4cff23496a 100644 (file)
@@ -39,13 +39,6 @@ int dsa_key_fromdata(DSA *dsa, const OSSL_PARAM params[])
     if (param_priv_key == NULL && param_pub_key == NULL)
         return 1;
 
-    /*
-     * DSA documentation says that a public key must be present if a
-     * private key is present.
-     */
-    if (param_priv_key != NULL && param_pub_key == NULL)
-        return 0;
-
     if (param_pub_key != NULL && !OSSL_PARAM_get_BN(param_pub_key, &pub_key))
         goto err;
     if (param_priv_key != NULL && !OSSL_PARAM_get_BN(param_priv_key, &priv_key))
index df9dd73dfdeef8ba62b11b73ae0e430e86f23802..7488fa24512b3758dc8b3bcabf03c67aea5b4547 100644 (file)
@@ -310,13 +310,6 @@ void DSA_get0_key(const DSA *d,
 
 int DSA_set0_key(DSA *d, BIGNUM *pub_key, BIGNUM *priv_key)
 {
-    /* If the field pub_key in d is NULL, the corresponding input
-     * parameters MUST be non-NULL.  The priv_key field may
-     * be left NULL.
-     */
-    if (d->pub_key == NULL && pub_key == NULL)
-        return 0;
-
     if (pub_key != NULL) {
         BN_free(d->pub_key);
         d->pub_key = pub_key;
index dccf6a15b96d38092f8a1e6c4eb69001de21e235..f95065717390b81ad2cc58ca2a73360a215c7179 100644 (file)
@@ -245,10 +245,6 @@ int ec_key_fromdata(EC_KEY *ec, const OSSL_PARAM params[], int include_private)
     if (ctx == NULL)
         goto err;
 
-    /* OpenSSL decree: If there's a private key, there must be a public key */
-    if (param_priv_key != NULL && param_pub_key == NULL)
-        goto err;
-
     if (param_pub_key != NULL)
         if (!OSSL_PARAM_get_octet_string(param_pub_key,
                                          (void **)&pub_key, 0, &pub_key_len)
index 67e5a48c3e25c2ede3a5795dc622eaa2d3ea2fad..832989ae00f56da560bd063deb91691ca3a6661c 100644 (file)
@@ -485,6 +485,135 @@ err:
     return res;
 }
 
+#if !defined(OPENSSL_NO_DH) && !defined(OPENSSL_NO_DSA)
+/*
+ * Test combinations of private, public, missing and private + public key 
+ * params to ensure they are all accepted
+ */
+static int test_EVP_PKEY_ffc_priv_pub(char *keytype)
+{
+    OSSL_PARAM_BLD *bld = NULL;
+    OSSL_PARAM *params = NULL;
+    BIGNUM *p = NULL, *q = NULL, *g = NULL, *pub = NULL, *priv = NULL;
+    EVP_PKEY_CTX *pctx = NULL;
+    EVP_PKEY *pkey = NULL;
+    int ret = 0;
+
+    /*
+     * Setup the parameters for our pkey object. For our purposes they don't
+     * have to actually be *valid* parameters. We just need to set something.
+     */
+    if (!TEST_ptr(pctx = EVP_PKEY_CTX_new_from_name(testctx, keytype, NULL))
+        || !TEST_ptr(p = BN_new())
+        || !TEST_ptr(q = BN_new())
+        || !TEST_ptr(g = BN_new())
+        || !TEST_ptr(pub = BN_new())
+        || !TEST_ptr(priv = BN_new()))
+        goto err;
+
+    /* Test !priv and !pub */
+    if (!TEST_ptr(bld = OSSL_PARAM_BLD_new())
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_P, p))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_Q, q))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_G, g)))
+        goto err;
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+        goto err;
+
+    if (!TEST_int_gt(EVP_PKEY_key_fromdata_init(pctx), 0)
+        || !TEST_int_gt(EVP_PKEY_fromdata(pctx, &pkey, params), 0))
+        goto err;
+
+    if (!TEST_ptr(pkey))
+        goto err;
+
+    EVP_PKEY_free(pkey);
+    pkey = NULL;
+    OSSL_PARAM_BLD_free_params(params);
+    OSSL_PARAM_BLD_free(bld);
+
+    /* Test priv and !pub */
+    if (!TEST_ptr(bld = OSSL_PARAM_BLD_new())
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_P, p))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_Q, q))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_G, g))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PRIV_KEY,
+                                             priv)))
+        goto err;
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+        goto err;
+
+    if (!TEST_int_gt(EVP_PKEY_key_fromdata_init(pctx), 0)
+        || !TEST_int_gt(EVP_PKEY_fromdata(pctx, &pkey, params), 0))
+        goto err;
+
+    if (!TEST_ptr(pkey))
+        goto err;
+
+    EVP_PKEY_free(pkey);
+    pkey = NULL;
+    OSSL_PARAM_BLD_free_params(params);
+    OSSL_PARAM_BLD_free(bld);
+
+    /* Test !priv and pub */
+    if (!TEST_ptr(bld = OSSL_PARAM_BLD_new())
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_P, p))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_Q, q))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_G, g))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PUB_KEY,
+                                             pub)))
+        goto err;
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+        goto err;
+
+    if (!TEST_int_gt(EVP_PKEY_key_fromdata_init(pctx), 0)
+        || !TEST_int_gt(EVP_PKEY_fromdata(pctx, &pkey, params), 0))
+        goto err;
+
+    if (!TEST_ptr(pkey))
+        goto err;
+
+    EVP_PKEY_free(pkey);
+    pkey = NULL;
+    OSSL_PARAM_BLD_free_params(params);
+    OSSL_PARAM_BLD_free(bld);
+
+    /* Test priv and pub */
+    if (!TEST_ptr(bld = OSSL_PARAM_BLD_new())
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_P, p))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_Q, q))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_FFC_G, g))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PUB_KEY,
+                                             pub))
+        || !TEST_true(OSSL_PARAM_BLD_push_BN(bld, OSSL_PKEY_PARAM_PRIV_KEY,
+                                             priv)))
+        goto err;
+    if (!TEST_ptr(params = OSSL_PARAM_BLD_to_param(bld)))
+        goto err;
+
+    if (!TEST_int_gt(EVP_PKEY_key_fromdata_init(pctx), 0)
+        || !TEST_int_gt(EVP_PKEY_fromdata(pctx, &pkey, params), 0))
+        goto err;
+
+    if (!TEST_ptr(pkey))
+        goto err;
+
+    ret = 1;
+ err:
+    EVP_PKEY_free(pkey);
+    EVP_PKEY_CTX_free(pctx);
+    OSSL_PARAM_BLD_free_params(params);
+    OSSL_PARAM_BLD_free(bld);
+    BN_free(p);
+    BN_free(q);
+    BN_free(g);
+    BN_free(pub);
+    BN_free(priv);
+
+    return ret;
+}
+#endif /* !OPENSSL_NO_DH && !OPENSSL_NO_DSA */
+
 static int test_EVP_Enveloped(void)
 {
     int ret = 0;
@@ -1718,7 +1847,17 @@ static int test_DSA_get_set_params(void)
 
     return ret;
 }
-#endif
+
+/*
+ * Test combinations of private, public, missing and private + public key 
+ * params to ensure they are all accepted
+ */
+static int test_DSA_priv_pub(void)
+{
+    return test_EVP_PKEY_ffc_priv_pub("DSA");
+}
+
+#endif /* !OPENSSL_NO_DSA */
 
 static int test_RSA_get_set_params(void)
 {
@@ -1833,7 +1972,17 @@ static int test_decrypt_null_chunks(void)
 }
 #endif /* !defined(OPENSSL_NO_CHACHA) && !defined(OPENSSL_NO_POLY1305) */
 
-#if !defined(OPENSSL_NO_DH) && !defined(OPENSSL_NO_DEPRECATED_3_0)
+#ifndef OPENSSL_NO_DH
+/*
+ * Test combinations of private, public, missing and private + public key 
+ * params to ensure they are all accepted
+ */
+static int test_DH_priv_pub(void)
+{
+    return test_EVP_PKEY_ffc_priv_pub("DH");
+}
+
+# ifndef OPENSSL_NO_DEPRECATED_3_0
 static int test_EVP_PKEY_set1_DH(void)
 {
     DH *x942dh = NULL, *noqdh = NULL;
@@ -1878,7 +2027,8 @@ static int test_EVP_PKEY_set1_DH(void)
 
     return ret;
 }
-#endif
+# endif /* !OPENSSL_NO_DEPRECATED_3_0 */
+#endif /* !OPENSSL_NO_DH */
 
 /*
  * We test what happens with an empty template.  For the sake of this test,
@@ -2181,13 +2331,17 @@ int setup_tests(void)
 #endif
 #ifndef OPENSSL_NO_DSA
     ADD_TEST(test_DSA_get_set_params);
+    ADD_TEST(test_DSA_priv_pub);
 #endif
     ADD_TEST(test_RSA_get_set_params);
 #if !defined(OPENSSL_NO_CHACHA) && !defined(OPENSSL_NO_POLY1305)
     ADD_TEST(test_decrypt_null_chunks);
 #endif
-#if !defined(OPENSSL_NO_DH) && !defined(OPENSSL_NO_DEPRECATED_3_0)
+#ifndef OPENSSL_NO_DH
+    ADD_TEST(test_DH_priv_pub);
+# ifndef OPENSSL_NO_DEPRECATED_3_0
     ADD_TEST(test_EVP_PKEY_set1_DH);
+# endif
 #endif
     ADD_ALL_TESTS(test_keygen_with_empty_template, 2);
     ADD_ALL_TESTS(test_pkey_ctx_fail_without_provider, 2);