[openssl] master update

Richard Levitte levitte at openssl.org
Fri Apr 23 18:22:58 UTC 2021


The branch master has been updated
       via  a70936a8453a307992820f2a9d3e252f6c4f9ad6 (commit)
       via  3d80b5e611f112fd004a4320cb5ecce93c73b7d4 (commit)
       via  521a0bf6a11c4cdaef331934e93581d06ce834e1 (commit)
       via  e36a4dc476448a2ef212d774be48ce38ea6eb6df (commit)
      from  f58f7ec9397de7b752aa547e2677933559a657db (commit)


- Log -----------------------------------------------------------------
commit a70936a8453a307992820f2a9d3e252f6c4f9ad6
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Apr 23 15:52:02 2021 +0200

    TEST: correct test/recipes/30-test_evp_data/evppkey_ecdh.txt
    
    Some keys with groups that aren't supported by FIPS were still used
    for Derive stanzas, even when testing with the FIPS provider.
    This was due to the flaw in evp_keymgmt_util_try_import() that meant
    that even though the key was invalid for FIPS, it could still come
    through, because the imported keydata wasn't cleared on import error.
    With that flaw corrected, these few Derive stanzas start failing.
    
    We mitigate this by making of "offending" Derive stanzas only
    available with the default provider.
    
    Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
    (Merged from https://github.com/openssl/openssl/pull/15008)

commit 3d80b5e611f112fd004a4320cb5ecce93c73b7d4
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Apr 23 15:47:59 2021 +0200

    STORE: Simplify error filtering in der2obj_decode()
    
    We do here like in all other decoder implementations, drop all errors
    that were caused by a failing asn1_d2i_read_bio(), as it's most likely
    to mean that the input isn't DER, and another decoder implementation,
    if there is any left, should have a go.
    
    Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
    (Merged from https://github.com/openssl/openssl/pull/15008)

commit 521a0bf6a11c4cdaef331934e93581d06ce834e1
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Apr 23 15:44:39 2021 +0200

    crypto/store/ossl_result.c: Better filtering of errors
    
    The diverse variants of try_XXX() were filtering errors independently
    of each other.  It's better done in ossl_store_handle_load_result()
    itself, where we have control over the overall success and failure of
    the attempts.
    
    Fixes #14973
    
    Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
    (Merged from https://github.com/openssl/openssl/pull/15008)

commit e36a4dc476448a2ef212d774be48ce38ea6eb6df
Author: Richard Levitte <levitte at openssl.org>
Date:   Fri Apr 23 15:40:30 2021 +0200

    EVP: evp_keymgmt_util_try_import() should clean up on failed import
    
    If evp_keymgmt_util_try_import() allocated keydata, and the import
    itself fails, it should deallocate keydata.
    
    Reviewed-by: Dmitry Belyavskiy <beldmit at gmail.com>
    (Merged from https://github.com/openssl/openssl/pull/15008)

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

Summary of changes:
 crypto/evp/keymgmt_lib.c                           | 25 +++++----
 crypto/store/store_result.c                        | 63 ++++++++--------------
 .../implementations/storemgmt/file_store_der2obj.c | 24 +++------
 test/recipes/30-test_evp_data/evppkey_ecdh.txt     |  8 +++
 4 files changed, 53 insertions(+), 67 deletions(-)

diff --git a/crypto/evp/keymgmt_lib.c b/crypto/evp/keymgmt_lib.c
index f3118a76c9..301e1a8a2f 100644
--- a/crypto/evp/keymgmt_lib.c
+++ b/crypto/evp/keymgmt_lib.c
@@ -31,12 +31,15 @@ static int match_type(const EVP_KEYMGMT *keymgmt1, const EVP_KEYMGMT *keymgmt2)
 int evp_keymgmt_util_try_import(const OSSL_PARAM params[], void *arg)
 {
     struct evp_keymgmt_util_try_import_data_st *data = arg;
+    int delete_on_error = 0;
 
     /* Just in time creation of keydata */
-    if (data->keydata == NULL
-        && (data->keydata = evp_keymgmt_newdata(data->keymgmt)) == NULL) {
-        ERR_raise(ERR_LIB_EVP, ERR_R_MALLOC_FAILURE);
-        return 0;
+    if (data->keydata == NULL) {
+        if ((data->keydata = evp_keymgmt_newdata(data->keymgmt)) == NULL) {
+            ERR_raise(ERR_LIB_EVP, ERR_R_MALLOC_FAILURE);
+            return 0;
+        }
+        delete_on_error = 1;
     }
 
     /*
@@ -46,8 +49,14 @@ int evp_keymgmt_util_try_import(const OSSL_PARAM params[], void *arg)
     if (params[0].key == NULL)
         return 1;
 
-    return evp_keymgmt_import(data->keymgmt, data->keydata, data->selection,
-                              params);
+    if (evp_keymgmt_import(data->keymgmt, data->keydata, data->selection,
+                           params))
+        return 1;
+    if (delete_on_error) {
+        evp_keymgmt_freedata(data->keymgmt, data->keydata);
+        data->keydata = NULL;
+    }
+    return 0;
 }
 
 int evp_keymgmt_util_assign_pkey(EVP_PKEY *pkey, EVP_KEYMGMT *keymgmt,
@@ -149,11 +158,9 @@ void *evp_keymgmt_util_export_to_provider(EVP_PKEY *pk, EVP_KEYMGMT *keymgmt)
      * which does the import for us.  If successful, we're done.
      */
     if (!evp_keymgmt_util_export(pk, OSSL_KEYMGMT_SELECT_ALL,
-                                 &evp_keymgmt_util_try_import, &import_data)) {
+                                 &evp_keymgmt_util_try_import, &import_data))
         /* If there was an error, bail out */
-        evp_keymgmt_freedata(keymgmt, import_data.keydata);
         return NULL;
-    }
 
     if (!CRYPTO_THREAD_write_lock(pk->lock)) {
         evp_keymgmt_freedata(keymgmt, import_data.keydata);
diff --git a/crypto/store/store_result.c b/crypto/store/store_result.c
index f69045994d..82ec046763 100644
--- a/crypto/store/store_result.c
+++ b/crypto/store/store_result.c
@@ -126,19 +126,33 @@ int ossl_store_handle_load_result(const OSSL_PARAM params[], void *arg)
      * The helper functions return 0 on actual errors, otherwise 1, even if
      * they didn't fill out |*v|.
      */
-    if (!try_name(&helper_data, v))
+    ERR_set_mark();
+    if (*v == NULL && !try_name(&helper_data, v))
         goto err;
-    if (!try_key(&helper_data, v, ctx, provider, libctx, propq))
+    ERR_pop_to_mark();
+    ERR_set_mark();
+    if (*v == NULL && !try_key(&helper_data, v, ctx, provider, libctx, propq))
         goto err;
-    if (!try_cert(&helper_data, v, libctx, propq))
+    ERR_pop_to_mark();
+    ERR_set_mark();
+    if (*v == NULL && !try_cert(&helper_data, v, libctx, propq))
         goto err;
-    if (!try_crl(&helper_data, v, libctx, propq))
+    ERR_pop_to_mark();
+    ERR_set_mark();
+    if (*v == NULL && !try_crl(&helper_data, v, libctx, propq))
         goto err;
-    if (!try_pkcs12(&helper_data, v, ctx, libctx, propq))
+    ERR_pop_to_mark();
+    ERR_set_mark();
+    if (*v == NULL && !try_pkcs12(&helper_data, v, ctx, libctx, propq))
         goto err;
+    ERR_pop_to_mark();
+
+    if (*v == NULL)
+        ERR_raise(ERR_LIB_OSSL_STORE, ERR_R_UNSUPPORTED);
 
     return (*v != NULL);
  err:
+    ERR_clear_last_mark();
     return 0;
 }
 
@@ -282,13 +296,8 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
     /* Try PUBKEY first, that's a real easy target */
     if (ctx->expected_type == 0
         || ctx->expected_type == OSSL_STORE_INFO_PUBKEY) {
-        /* Discard DER decoding errors, it only means we return "empty handed" */
-        if (ctx->expected_type == 0)
-            ERR_set_mark();
         derp = der;
         pk = d2i_PUBKEY_ex(NULL, &derp, der_len, libctx, propq);
-        if (ctx->expected_type == 0)
-            ERR_pop_to_mark();
 
         if (pk != NULL)
             *store_info_new = OSSL_STORE_INFO_new_PUBKEY;
@@ -302,16 +311,9 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
         X509_SIG *p8 = NULL;
         PKCS8_PRIV_KEY_INFO *p8info = NULL;
 
-        /*
-         * See if it's an encrypted PKCS#8 and decrypt it.  Discard DER
-         * decoding errors, a failed decode only means we return "empty handed"
-         */
-        if (ctx->expected_type == 0)
-            ERR_set_mark();
+        /* See if it's an encrypted PKCS#8 and decrypt it. */
         derp = der;
         p8 = d2i_X509_SIG(NULL, &derp, der_len);
-        if (ctx->expected_type == 0)
-            ERR_pop_to_mark();
 
         if (p8 != NULL) {
             char pbuf[PEM_BUFSIZE];
@@ -344,15 +346,9 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
          * |der| is NULL
          */
         if (der != NULL) {
-            /*
-             * Try to unpack an unencrypted PKCS#8, that's easy. Discard DER
-             * decoding errors, a failed decode only means we return "empty
-             * handed"
-             */
-            ERR_set_mark();
+            /* Try to unpack an unencrypted PKCS#8, that's easy */
             derp = der;
             p8info = d2i_PKCS8_PRIV_KEY_INFO(NULL, &derp, der_len);
-            ERR_pop_to_mark();
 
             if (p8info != NULL) {
                 pk = EVP_PKCS82PKEY_ex(p8info, libctx, propq);
@@ -399,8 +395,8 @@ static int try_key(struct extracted_param_data_st *data, OSSL_STORE_INFO **v,
 
             /*
              * Desperate last maneuver, in case the decoders don't support
-             * the data we have, then we try on our own to at least get a
-             * legacy key.
+             * the data we have, then we try on our own to at least get an
+             * engine provided legacy key.
              * This is the same as der2key_decode() does, but in a limited
              * way and within the walls of libcrypto.
              *
@@ -464,16 +460,11 @@ static int try_cert(struct extracted_param_data_st *data, OSSL_STORE_INFO **v,
             && (strcasecmp(data->data_type, PEM_STRING_X509_TRUSTED) == 0))
             ignore_trusted = 0;
 
-        /* Discard DER decoding errors, it only means we return "empty handed" */
-        if (data->object_type == OSSL_OBJECT_UNKNOWN)
-            ERR_set_mark();
         cert = d2i_X509_AUX(NULL, (const unsigned char **)&data->octet_data,
                             data->octet_data_size);
         if (cert == NULL && ignore_trusted)
             cert = d2i_X509(NULL, (const unsigned char **)&data->octet_data,
                             data->octet_data_size);
-        if (data->object_type == OSSL_OBJECT_UNKNOWN)
-            ERR_pop_to_mark();
 
         if (cert != NULL)
             /* We determined the object type */
@@ -500,13 +491,8 @@ static int try_crl(struct extracted_param_data_st *data, OSSL_STORE_INFO **v,
         || data->object_type == OSSL_OBJECT_CRL) {
         X509_CRL *crl;
 
-        /* Discard DER decoding errors, it only means we return "empty handed" */
-        if (data->object_type == OSSL_OBJECT_UNKNOWN)
-            ERR_set_mark();
         crl = d2i_X509_CRL(NULL, (const unsigned char **)&data->octet_data,
                            data->octet_data_size);
-        if (data->object_type == OSSL_OBJECT_UNKNOWN)
-            ERR_pop_to_mark();
 
         if (crl != NULL)
             /* We determined the object type */
@@ -537,11 +523,8 @@ static int try_pkcs12(struct extracted_param_data_st *data, OSSL_STORE_INFO **v,
         /* Initial parsing */
         PKCS12 *p12;
 
-        /* Discard DER decoding errors, it only means we return "empty handed" */
-        ERR_set_mark();
         p12 = d2i_PKCS12(NULL, (const unsigned char **)&data->octet_data,
                          data->octet_data_size);
-        ERR_pop_to_mark();
 
         if (p12 != NULL) {
             char *pass = NULL;
diff --git a/providers/implementations/storemgmt/file_store_der2obj.c b/providers/implementations/storemgmt/file_store_der2obj.c
index 2ecf20bac7..4f90535842 100644
--- a/providers/implementations/storemgmt/file_store_der2obj.c
+++ b/providers/implementations/storemgmt/file_store_der2obj.c
@@ -87,29 +87,18 @@ static int der2obj_decode(void *provctx, OSSL_CORE_BIO *cin, int selection,
      */
     BIO *in = ossl_bio_new_from_core_bio(provctx, cin);
     BUF_MEM *mem = NULL;
-    int err, ok;
+    int ok;
 
     if (in == NULL)
         return 0;
 
     ERR_set_mark();
     ok = (asn1_d2i_read_bio(in, &mem) >= 0);
-    /*
-     * Prune low-level ASN.1 parse errors from error queue, assuming that
-     * this is called by decoder_process() in a loop trying several formats.
-     */
-    if (!ok) {
-        err = ERR_peek_last_error();
-        if (ERR_GET_LIB(err) == ERR_LIB_ASN1
-            && (ERR_GET_REASON(err) == ASN1_R_HEADER_TOO_LONG
-                || ERR_GET_REASON(err) == ASN1_R_UNSUPPORTED_TYPE
-                || ERR_GET_REASON(err) == ERR_R_NESTED_ASN1_ERROR
-                || ERR_GET_REASON(err) == ASN1_R_NOT_ENOUGH_DATA)) {
-            ERR_pop_to_mark();
-        } else {
-            ERR_clear_last_mark();
-            goto end;
-        }
+    ERR_pop_to_mark();
+    if (!ok && mem != NULL) {
+        OPENSSL_free(mem->data);
+        OPENSSL_free(mem);
+        mem = NULL;
     }
 
     ok = 1;
@@ -128,7 +117,6 @@ static int der2obj_decode(void *provctx, OSSL_CORE_BIO *cin, int selection,
         OPENSSL_free(mem->data);
         OPENSSL_free(mem);
     }
- end:
     BIO_free(in);
     return ok;
 }
diff --git a/test/recipes/30-test_evp_data/evppkey_ecdh.txt b/test/recipes/30-test_evp_data/evppkey_ecdh.txt
index 9d3ef6c292..d50b2d166e 100644
--- a/test/recipes/30-test_evp_data/evppkey_ecdh.txt
+++ b/test/recipes/30-test_evp_data/evppkey_ecdh.txt
@@ -947,12 +947,14 @@ PrivPubKeyPair = BOB_sect163r1:BOB_sect163r1_PUB
 
 # ECDH Alice with Bob peer
 
+Availablein=default
 Derive=ALICE_sect163r1
 PeerKey=BOB_sect163r1_PUB
 SharedSecret=02355c765bbc07fcc44bb1496e490912f6df56e6d4
 
 # ECDH Bob with Alice peer
 
+Availablein=default
 Derive=BOB_sect163r1
 PeerKey=ALICE_sect163r1_PUB
 SharedSecret=02355c765bbc07fcc44bb1496e490912f6df56e6d4
@@ -993,12 +995,14 @@ PrivPubKeyPair = BOB_sect193r1:BOB_sect193r1_PUB
 
 # ECDH Alice with Bob peer
 
+Availablein=default
 Derive=ALICE_sect193r1
 PeerKey=BOB_sect193r1_PUB
 SharedSecret=00458b4c5ad122de5a377bea0adf1ab87bcb961b24ed764f47
 
 # ECDH Bob with Alice peer
 
+Availablein=default
 Derive=BOB_sect193r1
 PeerKey=ALICE_sect193r1_PUB
 SharedSecret=00458b4c5ad122de5a377bea0adf1ab87bcb961b24ed764f47
@@ -1039,12 +1043,14 @@ PrivPubKeyPair = BOB_sect193r2:BOB_sect193r2_PUB
 
 # ECDH Alice with Bob peer
 
+Availablein=default
 Derive=ALICE_sect193r2
 PeerKey=BOB_sect193r2_PUB
 SharedSecret=019d1f316d204a9cd1b9632cebb4accddb204158be3e435891
 
 # ECDH Bob with Alice peer
 
+Availablein=default
 Derive=BOB_sect193r2
 PeerKey=ALICE_sect193r2_PUB
 SharedSecret=019d1f316d204a9cd1b9632cebb4accddb204158be3e435891
@@ -1085,12 +1091,14 @@ PrivPubKeyPair = BOB_sect239k1:BOB_sect239k1_PUB
 
 # ECDH Alice with Bob peer
 
+Availablein=default
 Derive=ALICE_sect239k1
 PeerKey=BOB_sect239k1_PUB
 SharedSecret=4d1c9a8ae73f754d0a593d6e426114f4f67d7c8082ccc4e04a72b0d2aff8
 
 # ECDH Bob with Alice peer
 
+Availablein=default
 Derive=BOB_sect239k1
 PeerKey=ALICE_sect239k1_PUB
 SharedSecret=4d1c9a8ae73f754d0a593d6e426114f4f67d7c8082ccc4e04a72b0d2aff8


More information about the openssl-commits mailing list