[openssl] master update

Richard Levitte levitte at openssl.org
Tue Sep 8 04:28:03 UTC 2020


The branch master has been updated
       via  08497fc64f688a91d421de74a8498aff33573485 (commit)
       via  20d56d6d62d98c3b2649afd2d20e0c2cc39afce1 (commit)
       via  509144964ba69b69a90269da52a2dc3acb3149e6 (commit)
      from  884baafba4a5fec6502b828a73188d7133b9179b (commit)


- Log -----------------------------------------------------------------
commit 08497fc64f688a91d421de74a8498aff33573485
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Sep 4 10:52:20 2020 +0200

    Fix test/evp_extra_test.c
    
    Because EVP_PKEY_CTX_new_from_name() could return a non-NULL context
    with no value in it, the lack of legacy implementation when OpenSSL
    was configured with 'no-ec' went through undetected.  This adds the
    necessary guards to skip a test of SM2 in that case.
    
    Reviewed-by: Paul Yang <kaishen.yy at antfin.com>
    (Merged from https://github.com/openssl/openssl/pull/12785)

commit 20d56d6d62d98c3b2649afd2d20e0c2cc39afce1
Author: Richard Levitte <levitte at openssl.org>
Date:   Thu Sep 3 12:42:43 2020 +0200

    EVP: Don't shadow EVP_PKEY_CTX_new* error records
    
    There are places that add an ERR_R_MALLOC_FAILURE record when any of
    EVP_PKEY_CTX_new*() return NULL, which is 1) inaccurate, and 2)
    shadows the more accurate error record generated when trying to create
    the EVP_PKEY_CTX.
    
    Reviewed-by: Paul Yang <kaishen.yy at antfin.com>
    (Merged from https://github.com/openssl/openssl/pull/12785)

commit 509144964ba69b69a90269da52a2dc3acb3149e6
Author: Richard Levitte <levitte at openssl.org>
Date:   Wed Sep 2 09:30:42 2020 +0200

    EVP: Preserve the EVP_PKEY id in a few more spots
    
    As long as there are internal legacy keys for EVP_PKEY, we need to preserve
    the EVP_PKEY numeric identity when generating a key, and when creating the
    EVP_PKEY_CTX.
    
    For added consistency, the EVP_PKEY_CTX contructor tries a little
    harder to find a EVP_PKEY_METHOD.  Otherwise, we may run into
    situations where the EVP_PKEY_CTX ends up having no associated methods
    at all.
    
    Reviewed-by: Paul Yang <kaishen.yy at antfin.com>
    (Merged from https://github.com/openssl/openssl/pull/12785)

-----------------------------------------------------------------------

Summary of changes:
 crypto/evp/p_lib.c     |  95 ++++++++++++++++++----------------
 crypto/evp/pmeth_gn.c  |   6 +++
 crypto/evp/pmeth_lib.c | 136 ++++++++++++++++++++++++++++++++-----------------
 include/crypto/evp.h   |   3 ++
 test/evp_extra_test.c  |  52 ++++++++++++++-----
 5 files changed, 189 insertions(+), 103 deletions(-)

diff --git a/crypto/evp/p_lib.c b/crypto/evp/p_lib.c
index fd2a6c5abc..fec4e2d43b 100644
--- a/crypto/evp/p_lib.c
+++ b/crypto/evp/p_lib.c
@@ -606,10 +606,8 @@ static EVP_PKEY *new_cmac_key_int(const unsigned char *priv, size_t len,
     }
 
     ctx = EVP_PKEY_CTX_new_from_name(libctx, "CMAC", propq);
-    if (ctx == NULL) {
-        EVPerr(0, ERR_R_MALLOC_FAILURE);
+    if (ctx == NULL)
         goto err;
-    }
 
     if (!EVP_PKEY_key_fromdata_init(ctx)) {
         EVPerr(0, EVP_R_KEY_SETUP_FAILED);
@@ -988,51 +986,62 @@ int EVP_PKEY_base_id(const EVP_PKEY *pkey)
     return EVP_PKEY_type(pkey->type);
 }
 
+#ifndef FIPS_MODULE
+int evp_pkey_name2type(const char *name)
+{
+    /*
+     * These hard coded cases are pure hackery to get around the fact
+     * that names in crypto/objects/objects.txt are a mess.  There is
+     * no "EC", and "RSA" leads to the NID for 2.5.8.1.1, an OID that's
+     * fallen out in favor of { pkcs-1 1 }, i.e. 1.2.840.113549.1.1.1,
+     * the NID of which is used for EVP_PKEY_RSA.  Strangely enough,
+     * "DSA" is accurate...  but still, better be safe and hard-code
+     * names that we know.
+     * On a similar topic, EVP_PKEY_type(EVP_PKEY_SM2) will result in
+     * EVP_PKEY_EC, because of aliasing.
+     * TODO Clean this away along with all other #legacy support.
+     */
+    int type = NID_undef;
+
+    if (strcasecmp(name, "RSA") == 0)
+        type = EVP_PKEY_RSA;
+    else if (strcasecmp(name, "RSA-PSS") == 0)
+        type = EVP_PKEY_RSA_PSS;
+    else if (strcasecmp(name, "EC") == 0)
+        type = EVP_PKEY_EC;
+    else if (strcasecmp(name, "ED25519") == 0)
+        type = EVP_PKEY_ED25519;
+    else if (strcasecmp(name, "ED448") == 0)
+        type = EVP_PKEY_ED448;
+    else if (strcasecmp(name, "X25519") == 0)
+        type = EVP_PKEY_X25519;
+    else if (strcasecmp(name, "X448") == 0)
+        type = EVP_PKEY_X448;
+    else if (strcasecmp(name, "SM2") == 0)
+        type = EVP_PKEY_SM2;
+    else if (strcasecmp(name, "DH") == 0)
+        type = EVP_PKEY_DH;
+    else if (strcasecmp(name, "X9.42 DH") == 0)
+        type = EVP_PKEY_DHX;
+    else if (strcasecmp(name, "DSA") == 0)
+        type = EVP_PKEY_DSA;
+
+    if (type == NID_undef)
+        type = EVP_PKEY_type(OBJ_sn2nid(name));
+    if (type == NID_undef)
+        type = EVP_PKEY_type(OBJ_ln2nid(name));
+
+    return type;
+}
+#endif
+
 int EVP_PKEY_is_a(const EVP_PKEY *pkey, const char *name)
 {
 #ifndef FIPS_MODULE
     if (pkey->keymgmt == NULL) {
-        /*
-         * These hard coded cases are pure hackery to get around the fact
-         * that names in crypto/objects/objects.txt are a mess.  There is
-         * no "EC", and "RSA" leads to the NID for 2.5.8.1.1, an OID that's
-         * fallen out in favor of { pkcs-1 1 }, i.e. 1.2.840.113549.1.1.1,
-         * the NID of which is used for EVP_PKEY_RSA.  Strangely enough,
-         * "DSA" is accurate...  but still, better be safe and hard-code
-         * names that we know.
-         * TODO Clean this away along with all other #legacy support.
-         */
-        int type;
+        int type = evp_pkey_name2type(name);
 
-        if (strcasecmp(name, "RSA") == 0)
-            type = EVP_PKEY_RSA;
-        else if (strcasecmp(name, "RSA-PSS") == 0)
-            type = EVP_PKEY_RSA_PSS;
-#ifndef OPENSSL_NO_EC
-        else if (strcasecmp(name, "EC") == 0)
-            type = EVP_PKEY_EC;
-        else if (strcasecmp(name, "ED25519") == 0)
-            type = EVP_PKEY_ED25519;
-        else if (strcasecmp(name, "ED448") == 0)
-            type = EVP_PKEY_ED448;
-        else if (strcasecmp(name, "X25519") == 0)
-            type = EVP_PKEY_X25519;
-        else if (strcasecmp(name, "X448") == 0)
-            type = EVP_PKEY_X448;
-#endif
-#ifndef OPENSSL_NO_DH
-        else if (strcasecmp(name, "DH") == 0)
-            type = EVP_PKEY_DH;
-        else if (strcasecmp(name, "X9.42 DH") == 0)
-            type = EVP_PKEY_DHX;
-#endif
-#ifndef OPENSSL_NO_DSA
-        else if (strcasecmp(name, "DSA") == 0)
-            type = EVP_PKEY_DSA;
-#endif
-        else
-            type = EVP_PKEY_type(OBJ_sn2nid(name));
-        return EVP_PKEY_type(pkey->type) == type;
+        return pkey->type == type;
     }
 #endif
     return EVP_KEYMGMT_is_a(pkey->keymgmt, name);
diff --git a/crypto/evp/pmeth_gn.c b/crypto/evp/pmeth_gn.c
index 3096828678..b8dad20abd 100644
--- a/crypto/evp/pmeth_gn.c
+++ b/crypto/evp/pmeth_gn.c
@@ -212,6 +212,12 @@ int EVP_PKEY_gen(EVP_PKEY_CTX *ctx, EVP_PKEY **ppkey)
         evp_pkey_free_legacy(*ppkey);
 #endif
 
+    /*
+     * Because we still have legacy keys, and evp_pkey_downgrade()
+     * TODO remove this #legacy internal keys are gone
+     */
+    (*ppkey)->type = ctx->legacy_keytype;
+
 /* TODO remove when SM2 key have been cleanly separated from EC keys */
 #ifdef TMP_SM2_HACK
     /*
diff --git a/crypto/evp/pmeth_lib.c b/crypto/evp/pmeth_lib.c
index aef7c39a20..7f144b0afc 100644
--- a/crypto/evp/pmeth_lib.c
+++ b/crypto/evp/pmeth_lib.c
@@ -121,22 +121,36 @@ EVP_PKEY_METHOD *EVP_PKEY_meth_new(int id, int flags)
     pmeth->flags = flags | EVP_PKEY_FLAG_DYNAMIC;
     return pmeth;
 }
+
+static void help_get_legacy_alg_type_from_keymgmt(const char *keytype,
+                                                  void *arg)
+{
+    int *type = arg;
+
+    if (*type == NID_undef)
+        *type = evp_pkey_name2type(keytype);
+}
+
+static int get_legacy_alg_type_from_keymgmt(const EVP_KEYMGMT *keymgmt)
+{
+    int type = NID_undef;
+
+    EVP_KEYMGMT_names_do_all(keymgmt, help_get_legacy_alg_type_from_keymgmt,
+                             &type);
+    return type;
+}
 #endif /* FIPS_MODULE */
 
 static int is_legacy_alg(int id, const char *keytype)
 {
 #ifndef FIPS_MODULE
     /* Certain EVP_PKEY keytypes are only available in legacy form */
-    if (id == -1) {
-        id = OBJ_sn2nid(keytype);
-        if (id == NID_undef)
-            id = OBJ_ln2nid(keytype);
-        if (id == NID_undef)
-            return  0;
-    }
+    if (id == -1)
+        id = evp_pkey_name2type(keytype);
+
     switch (id) {
     /*
-     * TODO(3.0): Remove SM2 and DHX when they are converted to have provider
+     * TODO(3.0): Remove SM2 when they are converted to have provider
      * support
      */
     case EVP_PKEY_SM2:
@@ -155,19 +169,12 @@ static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
                                  int id)
 
 {
-    EVP_PKEY_CTX *ret;
+    EVP_PKEY_CTX *ret = NULL;
     const EVP_PKEY_METHOD *pmeth = NULL;
     EVP_KEYMGMT *keymgmt = NULL;
 
     /*
-     * When using providers, the context is bound to the algo implementation
-     * later.
-     */
-    if (pkey == NULL && e == NULL && id == -1)
-        goto common;
-
-    /*
-     * If the internal key is provided, we extract the keytype from its
+     * If the given |pkey| is provided, we extract the keytype from its
      * keymgmt and skip over the legacy code.
      */
     if (pkey != NULL && evp_pkey_is_provided(pkey)) {
@@ -177,14 +184,24 @@ static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
         keytype = evp_first_name(pkey->keymgmt->prov, pkey->keymgmt->name_id);
         goto common;
     }
+
 #ifndef FIPS_MODULE
-    /* TODO(3.0) Legacy code should be removed when all is provider based */
+    /*
+     * TODO(3.0) This legacy code section should be removed when we stop
+     * supporting engines
+     */
     /* BEGIN legacy */
     if (id == -1) {
-        if (pkey == NULL)
-            return NULL;
-        id = pkey->type;
+        if (pkey != NULL)
+            id = pkey->type;
+        else if (keytype != NULL)
+            id = evp_pkey_name2type(keytype);
+        if (id == NID_undef)
+            id = -1;
     }
+    /* If no ID was found here, we can only resort to find a keymgmt */
+    if (id == -1)
+        goto common;
 
     /*
      * Here, we extract what information we can for the purpose of
@@ -219,24 +236,11 @@ static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
      * If an ENGINE handled this method look it up. Otherwise use internal
      * tables.
      */
-    if (e != NULL) {
+    if (e != NULL)
         pmeth = ENGINE_get_pkey_meth(e, id);
-        /*
-         * We are supposed to use an engine, so no point in looking for a
-         * provided implementation. If pmeth is NULL here we just fail.
-         */
-        if (pmeth == NULL) {
-            ENGINE_finish(e);
-            EVPerr(EVP_F_INT_CTX_NEW, EVP_R_UNSUPPORTED_ALGORITHM);
-            return NULL;
-        }
-    } else
+    else
 # endif
         pmeth = EVP_PKEY_meth_find(id);
-        /*
-         * if pmeth is NULL here we can keep trying to see if we have a provided
-         * implementation below.
-         */
 
     /* END legacy */
 #endif /* FIPS_MODULE */
@@ -248,33 +252,71 @@ static EVP_PKEY_CTX *int_ctx_new(OPENSSL_CTX *libctx,
     if (e == NULL && keytype != NULL) {
         int legacy = is_legacy_alg(id, keytype);
 
-        if (legacy) {
-            /* This could fail so ignore errors */
+        /* This could fail so ignore errors */
+        if (legacy)
             ERR_set_mark();
-        }
 
         keymgmt = EVP_KEYMGMT_fetch(libctx, keytype, propquery);
-        if (legacy) {
+        if (legacy)
             ERR_pop_to_mark();
-        } else if (keymgmt == NULL) {
-            EVPerr(EVP_F_INT_CTX_NEW, EVP_R_FETCH_FAILED);
-            return NULL;
+        else if (keymgmt == NULL)
+            return NULL;   /* EVP_KEYMGMT_fetch() recorded an error */
+
+#ifndef FIPS_MODULE
+        /*
+         * Chase down the legacy NID, as that might be needed for diverse
+         * purposes, such as ensure that EVP_PKEY_type() can return sensible
+         * values, or that there's a better chance to "downgrade" a key when
+         * needed.  We go through all keymgmt names, because the keytype
+         * that's passed to this function doesn't necessarily translate
+         * directly.
+         * TODO: Remove this when #legacy keys are gone.
+         */
+        if (keymgmt != NULL) {
+            int tmp_id = get_legacy_alg_type_from_keymgmt(keymgmt);
+
+            if (tmp_id != NID_undef) {
+                if (id == -1) {
+                    id = tmp_id;
+                } else {
+                    /*
+                     * It really really shouldn't differ.  If it still does,
+                     * something is very wrong.
+                     */
+                    if (!ossl_assert(id == tmp_id)) {
+                        EVPerr(EVP_F_INT_CTX_NEW, ERR_R_INTERNAL_ERROR);
+                        EVP_KEYMGMT_free(keymgmt);
+                        return NULL;
+                    }
+                }
+            }
         }
+#endif
+    }
+
+    if (pmeth == NULL && keymgmt == NULL) {
+        EVPerr(EVP_F_INT_CTX_NEW, EVP_R_UNSUPPORTED_ALGORITHM);
+    } else {
+        ret = OPENSSL_zalloc(sizeof(*ret));
+        if (ret == NULL)
+            EVPerr(EVP_F_INT_CTX_NEW, ERR_R_MALLOC_FAILURE);
     }
 
-    ret = OPENSSL_zalloc(sizeof(*ret));
-    if (ret == NULL) {
-        EVP_KEYMGMT_free(keymgmt);
 #if !defined(OPENSSL_NO_ENGINE) && !defined(FIPS_MODULE)
+    if ((ret == NULL || pmeth == NULL) && e != NULL)
         ENGINE_finish(e);
 #endif
-        EVPerr(EVP_F_INT_CTX_NEW, ERR_R_MALLOC_FAILURE);
+
+    if (ret == NULL) {
+        EVP_KEYMGMT_free(keymgmt);
         return NULL;
     }
+
     ret->libctx = libctx;
     ret->propquery = propquery;
     ret->keytype = keytype;
     ret->keymgmt = keymgmt;
+    ret->legacy_keytype = id;   /* TODO: Remove when #legacy key are gone */
     ret->engine = e;
     ret->pmeth = pmeth;
     ret->operation = EVP_PKEY_OP_UNDEFINED;
diff --git a/include/crypto/evp.h b/include/crypto/evp.h
index b00634234c..43ecc79f52 100644
--- a/include/crypto/evp.h
+++ b/include/crypto/evp.h
@@ -62,6 +62,8 @@ struct evp_pkey_ctx_st {
 
     /* Legacy fields below */
 
+    /* EVP_PKEY identity */
+    int legacy_keytype;
     /* Method associated with this operation */
     const EVP_PKEY_METHOD *pmeth;
     /* Engine that implements this method or NULL if builtin */
@@ -766,6 +768,7 @@ int evp_pkey_ctx_get_params_strict(EVP_PKEY_CTX *ctx, OSSL_PARAM *params);
 EVP_MD_CTX *evp_md_ctx_new_with_libctx(EVP_PKEY *pkey,
                                        const ASN1_OCTET_STRING *id,
                                        OPENSSL_CTX *libctx, const char *propq);
+int evp_pkey_name2type(const char *name);
 #endif /* !defined(FIPS_MODULE) */
 void evp_method_store_flush(OPENSSL_CTX *libctx);
 int evp_set_default_properties_int(OPENSSL_CTX *libctx, const char *propq,
diff --git a/test/evp_extra_test.c b/test/evp_extra_test.c
index f62e26c290..94b95eeac8 100644
--- a/test/evp_extra_test.c
+++ b/test/evp_extra_test.c
@@ -1803,14 +1803,19 @@ static int test_keygen_with_empty_template(int n)
 
 /*
  * Test that we fail if we attempt to use an algorithm that is not available
- * in the current library context (unless we are using an algorithm that should
- * be made available via legacy codepaths).
+ * in the current library context (unless we are using an algorithm that
+ * should be made available via legacy codepaths).
+ *
+ * 0:   RSA
+ * 1:   SM2
  */
 static int test_pkey_ctx_fail_without_provider(int tst)
 {
     OPENSSL_CTX *tmpctx = OPENSSL_CTX_new();
     OSSL_PROVIDER *nullprov = NULL;
     EVP_PKEY_CTX *pctx = NULL;
+    const char *keytype = NULL;
+    int expect_null = 0;
     int ret = 0;
 
     if (!TEST_ptr(tmpctx))
@@ -1820,21 +1825,42 @@ static int test_pkey_ctx_fail_without_provider(int tst)
     if (!TEST_ptr(nullprov))
         goto err;
 
-    pctx = EVP_PKEY_CTX_new_from_name(tmpctx, tst == 0 ? "RSA" : "SM2", "");
-
-    /* RSA is not available via any provider so we expect this to fail */
-    if (tst == 0 && !TEST_ptr_null(pctx))
-        goto err;
-
     /*
-     * SM2 is always available because it is implemented via legacy codepaths
-     * and not in a provider at all. We expect this to pass.
-     * TODO(3.0): This can be removed once there are no more algorithms
-     * available via legacy codepaths
+     * We check for certain algos in the null provider.
+     * If an algo is expected to have a provider keymgmt, contructing an
+     * EVP_PKEY_CTX is expected to fail (return NULL).
+     * Otherwise, if it's expected to have legacy support, contructing an
+     * EVP_PKEY_CTX is expected to succeed (return non-NULL).
      */
-    if (tst == 1 && !TEST_ptr(pctx))
+    switch (tst) {
+    case 0:
+        keytype = "RSA";
+        expect_null = 1;
+        break;
+    case 1:
+        keytype = "SM2";
+        expect_null = 0; /* TODO: change to 1 when we have a SM2 keymgmt */
+#ifdef OPENSSL_NO_EC
+        TEST_info("EC disable, skipping SM2 check...");
+        goto end;
+#endif
+#ifdef OPENSSL_NO_SM2
+        TEST_info("SM2 disable, skipping SM2 check...");
+        goto end;
+#endif
+        break;
+    default:
+        TEST_error("No test for case %d", tst);
+        goto err;
+    }
+
+    pctx = EVP_PKEY_CTX_new_from_name(tmpctx, keytype, "");
+    if (expect_null ? !TEST_ptr_null(pctx) : !TEST_ptr(pctx))
         goto err;
 
+#if defined(OPENSSL_NO_EC) || defined(OPENSSL_NO_SM2)
+ end:
+#endif
     ret = 1;
 
  err:


More information about the openssl-commits mailing list