[openssl] master update

dev at ddvo.net dev at ddvo.net
Wed Sep 30 18:50:28 UTC 2020


The branch master has been updated
       via  4a24d6050bee3cafd3e1eb42b800ece23bdba6b5 (commit)
       via  66066e1bba041459c2f879666b79e4a2158f5905 (commit)
       via  9032c2c11b2f14dcdbd253b470abc595a07a6c51 (commit)
      from  e1f5a92df4b612de8eac7ca538ef44f4b1deec5a (commit)


- Log -----------------------------------------------------------------
commit 4a24d6050bee3cafd3e1eb42b800ece23bdba6b5
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Tue Sep 29 10:33:22 2020 +0200

    EC_GROUP_new_by_curve_name_with_libctx(): Add name of unknown group to error output
    
    Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
    (Merged from https://github.com/openssl/openssl/pull/13023)

commit 66066e1bba041459c2f879666b79e4a2158f5905
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Mon Sep 28 16:14:14 2020 +0200

    Prune low-level ASN.1 parse errors from error queue in der2key_decode() etc.
    
    Also adds error output tests on loading key files with unsupported algorithms to 30-test_evp.t
    
    Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
    (Merged from https://github.com/openssl/openssl/pull/13023)

commit 9032c2c11b2f14dcdbd253b470abc595a07a6c51
Author: Dr. David von Oheimb <David.von.Oheimb at siemens.com>
Date:   Mon Sep 28 19:44:49 2020 +0200

    25-test_x509.t: Add test for suitable error report loading unsupported sm2 cert
    
    Reviewed-by: Tomas Mraz <tmraz at fedoraproject.org>
    (Merged from https://github.com/openssl/openssl/pull/13023)

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

Summary of changes:
 crypto/ec/ec_ameth.c                               | 17 +++-------
 crypto/ec/ec_curve.c                               |  4 +++
 crypto/encode_decode/decoder_lib.c                 | 15 ++++++++-
 crypto/evp/evp_pkey.c                              |  4 +--
 crypto/store/store_result.c                        |  1 +
 crypto/x509/x_pubkey.c                             | 12 +++----
 .../implementations/encode_decode/decode_der2key.c | 34 ++++++++++++++-----
 test/certs/server-dsa-pubkey.pem                   | 20 ++++++++++++
 test/recipes/25-test_x509.t                        | 15 ++++++---
 test/recipes/30-test_evp.t                         | 38 +++++++++++++++++++++-
 10 files changed, 124 insertions(+), 36 deletions(-)
 create mode 100644 test/certs/server-dsa-pubkey.pem

diff --git a/crypto/ec/ec_ameth.c b/crypto/ec/ec_ameth.c
index b586a43539..3312faa336 100644
--- a/crypto/ec/ec_ameth.c
+++ b/crypto/ec/ec_ameth.c
@@ -172,10 +172,8 @@ static int eckey_pub_decode(EVP_PKEY *pkey, const X509_PUBKEY *pubkey)
 
     eckey = eckey_type2param(ptype, pval, libctx, propq);
 
-    if (!eckey) {
-        ECerr(EC_F_ECKEY_PUB_DECODE, ERR_R_EC_LIB);
+    if (!eckey)
         return 0;
-    }
 
     /* We have parameters now set public key */
     if (!o2i_ECPublicKey(&eckey, &p, pklen)) {
@@ -224,22 +222,19 @@ static int eckey_priv_decode_with_libctx(EVP_PKEY *pkey,
     X509_ALGOR_get0(NULL, &ptype, &pval, palg);
 
     eckey = eckey_type2param(ptype, pval, libctx, propq);
-
     if (eckey == NULL)
-        goto ecliberr;
+        goto err;
 
     /* We have parameters now set private key */
     if (!d2i_ECPrivateKey(&eckey, &p, pklen)) {
         ECerr(0, EC_R_DECODE_ERROR);
-        goto ecerr;
+        goto err;
     }
 
     EVP_PKEY_assign_EC_KEY(pkey, eckey);
     return 1;
 
- ecliberr:
-    ECerr(0, ERR_R_EC_LIB);
- ecerr:
+ err:
     EC_KEY_free(eckey);
     return 0;
 }
@@ -472,10 +467,8 @@ static int old_ec_priv_decode(EVP_PKEY *pkey,
 {
     EC_KEY *ec;
 
-    if ((ec = d2i_ECPrivateKey(NULL, pder, derlen)) == NULL) {
-        ECerr(EC_F_OLD_EC_PRIV_DECODE, EC_R_DECODE_ERROR);
+    if ((ec = d2i_ECPrivateKey(NULL, pder, derlen)) == NULL)
         return 0;
-    }
     EVP_PKEY_assign_EC_KEY(pkey, ec);
     return 1;
 }
diff --git a/crypto/ec/ec_curve.c b/crypto/ec/ec_curve.c
index bf02c261f7..a63a8535c3 100644
--- a/crypto/ec/ec_curve.c
+++ b/crypto/ec/ec_curve.c
@@ -18,6 +18,7 @@
 #include "ec_local.h"
 #include <openssl/err.h>
 #include <openssl/obj_mac.h>
+#include <openssl/objects.h>
 #include <openssl/opensslconf.h>
 #include "internal/nelem.h"
 #include "e_os.h" /* strcasecmp required by windows */
@@ -3298,6 +3299,9 @@ EC_GROUP *EC_GROUP_new_by_curve_name_with_libctx(OPENSSL_CTX *libctx,
     if ((curve = ec_curve_nid2curve(nid)) == NULL
         || (ret = ec_group_new_from_data(libctx, propq, *curve)) == NULL) {
         ECerr(0, EC_R_UNKNOWN_GROUP);
+#ifndef FIPS_MODULE
+        ERR_add_error_data(2, "name=", OBJ_nid2sn(nid));
+#endif
         return NULL;
     }
 
diff --git a/crypto/encode_decode/decoder_lib.c b/crypto/encode_decode/decoder_lib.c
index 0bc772e43b..0411da41f4 100644
--- a/crypto/encode_decode/decoder_lib.c
+++ b/crypto/encode_decode/decoder_lib.c
@@ -11,6 +11,9 @@
 #include <openssl/bio.h>
 #include <openssl/params.h>
 #include <openssl/provider.h>
+#include <openssl/evperr.h>
+#include <openssl/ecerr.h>
+#include <openssl/x509err.h>
 #include "internal/passphrase.h"
 #include "crypto/decoder.h"
 #include "encoder_local.h"
@@ -424,7 +427,7 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
     BIO *bio = data->bio;
     long loc;
     size_t i;
-    int ok = 0;
+    int err, ok = 0;
     /* For recursions */
     struct decoder_process_data_st new_data;
 
@@ -532,6 +535,16 @@ static int decoder_process(const OSSL_PARAM params[], void *arg)
                                  &new_data.ctx->pwdata);
         if (ok)
             break;
+        err = ERR_peek_last_error();
+        if ((ERR_GET_LIB(err) == ERR_LIB_EVP
+             && ERR_GET_REASON(err) == EVP_R_UNSUPPORTED_PRIVATE_KEY_ALGORITHM)
+#ifndef OPENSSL_NO_EC
+            || (ERR_GET_LIB(err) == ERR_LIB_EC
+                && ERR_GET_REASON(err) == EC_R_UNKNOWN_GROUP)
+#endif
+            || (ERR_GET_LIB(err) == ERR_LIB_X509
+                && ERR_GET_REASON(err) == X509_R_UNSUPPORTED_ALGORITHM))
+            break; /* fatal error; preserve it on the error queue and stop */
     }
 
  end:
diff --git a/crypto/evp/evp_pkey.c b/crypto/evp/evp_pkey.c
index f31d1d68f8..45666a2c42 100644
--- a/crypto/evp/evp_pkey.c
+++ b/crypto/evp/evp_pkey.c
@@ -41,10 +41,8 @@ EVP_PKEY *EVP_PKCS82PKEY_with_libctx(const PKCS8_PRIV_KEY_INFO *p8,
     }
 
     if (pkey->ameth->priv_decode_with_libctx != NULL) {
-        if (!pkey->ameth->priv_decode_with_libctx(pkey, p8, libctx, propq)) {
-            EVPerr(0, EVP_R_PRIVATE_KEY_DECODE_ERROR);
+        if (!pkey->ameth->priv_decode_with_libctx(pkey, p8, libctx, propq))
             goto error;
-        }
     } else if (pkey->ameth->priv_decode != NULL) {
         if (!pkey->ameth->priv_decode(pkey, p8)) {
             EVPerr(0, EVP_R_PRIVATE_KEY_DECODE_ERROR);
diff --git a/crypto/store/store_result.c b/crypto/store/store_result.c
index c3f21eedad..363d25adbf 100644
--- a/crypto/store/store_result.c
+++ b/crypto/store/store_result.c
@@ -88,6 +88,7 @@ static int try_pkcs12(struct extracted_param_data_st *, OSSL_STORE_INFO **,
                                                                         \
         if (ERR_GET_LIB(err) == ERR_LIB_ASN1                            \
             && (ERR_GET_REASON(err) == ASN1_R_UNKNOWN_PUBLIC_KEY_TYPE   \
+                || ERR_GET_REASON(err) == ASN1_R_NO_MATCHING_CHOICE_TYPE \
                 || ERR_GET_REASON(err) == ERR_R_NESTED_ASN1_ERROR))     \
             ERR_pop_to_mark();                                          \
         else                                                            \
diff --git a/crypto/x509/x_pubkey.c b/crypto/x509/x_pubkey.c
index a4d3c9fa5e..d63a33e301 100644
--- a/crypto/x509/x_pubkey.c
+++ b/crypto/x509/x_pubkey.c
@@ -41,12 +41,12 @@ static int x509_pubkey_decode(EVP_PKEY **pk, const X509_PUBKEY *key);
 static int pubkey_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
                      void *exarg)
 {
+    X509_PUBKEY *pubkey = (X509_PUBKEY *)*pval;
+
     if (operation == ASN1_OP_FREE_POST) {
-        X509_PUBKEY *pubkey = (X509_PUBKEY *)*pval;
         EVP_PKEY_free(pubkey->pkey);
     } else if (operation == ASN1_OP_D2I_POST) {
         /* Attempt to decode public key and cache in pubkey structure. */
-        X509_PUBKEY *pubkey = (X509_PUBKEY *)*pval;
         EVP_PKEY_free(pubkey->pkey);
         pubkey->pkey = NULL;
         /*
@@ -55,8 +55,10 @@ static int pubkey_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
          * will return an appropriate error.
          */
         ERR_set_mark();
-        if (x509_pubkey_decode(&pubkey->pkey, pubkey) == -1)
+        if (x509_pubkey_decode(&pubkey->pkey, pubkey) == -1) {
+            ERR_clear_last_mark();
             return 0;
+        }
         ERR_pop_to_mark();
     }
     return 1;
@@ -180,10 +182,8 @@ static int x509_pubkey_decode(EVP_PKEY **ppkey, const X509_PUBKEY *key)
          * future we could have different return codes for decode
          * errors and fatal errors such as malloc failure.
          */
-        if (!pkey->ameth->pub_decode(pkey, key)) {
-            X509err(X509_F_X509_PUBKEY_DECODE, X509_R_PUBLIC_KEY_DECODE_ERROR);
+        if (!pkey->ameth->pub_decode(pkey, key))
             goto error;
-        }
     } else {
         X509err(X509_F_X509_PUBKEY_DECODE, X509_R_METHOD_NOT_SUPPORTED);
         goto error;
diff --git a/providers/implementations/encode_decode/decode_der2key.c b/providers/implementations/encode_decode/decode_der2key.c
index 64b085673a..0b6debf506 100644
--- a/providers/implementations/encode_decode/decode_der2key.c
+++ b/providers/implementations/encode_decode/decode_der2key.c
@@ -30,6 +30,25 @@
 #include "prov/providercommonerr.h"
 #include "endecoder_local.h"
 
+#define SET_ERR_MARK() ERR_set_mark()
+#define CLEAR_ERR_MARK()                                                \
+    do {                                                                \
+        int 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_pop_to_mark();                                          \
+        else                                                            \
+            ERR_clear_last_mark();                                      \
+    } while(0)
+#define RESET_ERR_MARK()                                                \
+    do {                                                                \
+        CLEAR_ERR_MARK();                                               \
+        SET_ERR_MARK();                                                 \
+    } while(0)
+
 static int read_der(PROV_CTX *provctx, OSSL_CORE_BIO *cin,
                     unsigned char **data, long *len)
 {
@@ -165,9 +184,9 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin,
     long new_der_len;
     EVP_PKEY *pkey = NULL;
     void *key = NULL;
-    int err, ok = 0;
+    int ok = 0;
 
-    ERR_set_mark();
+    SET_ERR_MARK();
     if (!read_der(ctx->provctx, cin, &der, &der_len))
         goto err;
 
@@ -180,16 +199,19 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin,
         der = new_der;
         der_len = new_der_len;
     }
+    RESET_ERR_MARK();
 
     derp = der;
     pkey = d2i_PrivateKey_ex(ctx->desc->type, NULL, &derp, der_len,
                              libctx, NULL);
     if (pkey == NULL) {
+        RESET_ERR_MARK();
         derp = der;
         pkey = d2i_PUBKEY_ex(NULL, &derp, der_len, libctx, NULL);
     }
 
     if (pkey == NULL) {
+        RESET_ERR_MARK();
         derp = der;
         pkey = d2i_KeyParams(ctx->desc->type, NULL, &derp, der_len);
     }
@@ -198,13 +220,7 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin,
      * Prune low-level ASN.1 parse errors from error queue, assuming that
      * this is called by decoder_process() in a loop trying several formats.
      */
-    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) == ERR_R_NESTED_ASN1_ERROR))
-        ERR_pop_to_mark();
-    else
-        ERR_clear_last_mark();
+    CLEAR_ERR_MARK();
 
     if (pkey != NULL) {
         /*
diff --git a/test/certs/server-dsa-pubkey.pem b/test/certs/server-dsa-pubkey.pem
new file mode 100644
index 0000000000..e5b5e6a5af
--- /dev/null
+++ b/test/certs/server-dsa-pubkey.pem
@@ -0,0 +1,20 @@
+-----BEGIN PUBLIC KEY-----
+MIIDSDCCAjoGByqGSM44BAEwggItAoIBAQD+P3LcpaA+AYu9M1gSsHi8fixl7VPC
+sKK96oaH7/ZJqvOD0TdASkn+4Td8SPvkc+KG2bBbmp39FCxGpa4d8CRLKVbIHAFt
+aKHIDFuMlPuFnsiaU0uWN/s3lROhAHWrTiODhehFM+NiPrAOJmtXQURBoeQ07t4H
+oyKz7sUyTF2qotw1JDvBRb6JXw+13Z2a1iZGJopLZN3RicvoHee3rYEsM5AHMS3c
+ntYX2NhQUHjiQ451iL2OkFJtVeaUoX5JV6KYSzz4lzNlYwJfF/Tzac/+l1aFA1ND
+bNFcQ1UC0JXscKeT/J2Wo8kRwpx042UKaayw5jkOv3GndgKCOaCe29UrAiEAh8hM
+JV/kKTLolNr6kV87KV8eTaJfrnSRS2E3ToOhWH0CggEBAOd/YKl8svYqvJtThaOs
+mVETeXwEvz/MLqpj4hZr029Oqps7z6OmeZ2er7aldxC5+BKMxCfPlhFo0iQ9XITp
++J7UqS3qrRZqAnxMjd6VmEGXKWOoeAc0CpEzR1QNkjKodzgstQj5oYbiiPG0SgCt
+BV4I1b/IuKzkjcLxQaF+8Rob/lzLBwA6pFjZNa6FcDjthmtH2pC+zI760sv05rbZ
+GcXDj8G0SLsvbkrfiRIn/8LkgBpoTWpKfa8BmvYtt9WI/CYkbeQYIwM9sXUPwRSD
+1VONSg5bXTW3Sxmzy3Yfy9RYt+suMKzi78oSv81e5BoL1D2HtfxSAFQbiJU3kipx
+vhsDggEGAAKCAQEArDidnkCegHb/itBTFeyGsebv+I8Z93V3jGcKPOs3s1wqB/+H
+RL5ERlhQOq/lfYPigUFKhfC8tlCVAM+MtUDqXCzqAkomw0yX8oVkp9plswxHKlqj
+zKr6PWLOJGp/NDBAL1ZcUzHB1omvmkUHy9pYiapVVNUuUdL2Z5EvDze8jQoiR0k9
+zgMKiH+MyCfV0tLo8W8djFJPlIM9Ypa7DH4fazcEfRuzq1jvK/uX4+HWmg3Nswdh
+5eysb++RqtJSUBtGT3tAQY59WjBf2nXMG0nkZGkT7TCJ6icvNdbSl1AlAGMV/nZN
+3PFsFH17L8uMUYS7V5PWiqQTxe5COHqpGumo9A==
+-----END PUBLIC KEY-----
diff --git a/test/recipes/25-test_x509.t b/test/recipes/25-test_x509.t
index 4b37ee6464..f5b4245960 100644
--- a/test/recipes/25-test_x509.t
+++ b/test/recipes/25-test_x509.t
@@ -16,7 +16,7 @@ use OpenSSL::Test qw/:DEFAULT srctop_file/;
 
 setup("test_x509");
 
-plan tests => 14;
+plan tests => 15;
 
 require_ok(srctop_file('test','recipes','tconversion.pl'));
 
@@ -105,9 +105,10 @@ sub test_errors { # actually tests diagnostics of OSSL_STORE
     my ($expected, $cert, @opts) = @_;
     my $infile = srctop_file('test', 'certs', $cert);
     my @args = qw(openssl x509 -in);
-    push(@args, "$infile", @opts);
+    push(@args, $infile, @opts);
     my $tmpfile = 'out.txt';
-    my $res = !run(app([@args], stderr => $tmpfile));
+    my $res =  grep(/-text/, @opts) ? run(app([@args], stdout => $tmpfile))
+                                    : !run(app([@args], stderr => $tmpfile));
     my $found = 0;
     open(my $in, '<', $tmpfile) or die "Could not open file $tmpfile";
     while(<$in>) {
@@ -116,7 +117,7 @@ sub test_errors { # actually tests diagnostics of OSSL_STORE
         $found = 1 if m/$expected/; # output must include $expected
     }
     close $in;
-    unlink $tmpfile;
+    # $tmpfile is kept to help with investigation in case of failure
     return $res && $found;
 }
 
@@ -124,3 +125,9 @@ ok(test_errors("Can't open any-dir/", "root-cert.pem", '-out', 'any-dir/'),
    "load root-cert errors");
 ok(test_errors("RC2-40-CBC", "v3-certs-RC2.p12", '-passin', 'pass:v3-certs'),
    "load v3-certs-RC2 no asn1 errors");
+SKIP: {
+    skip "sm2 not disabled", 1 if !disabled("sm2");
+
+    ok(test_errors("unknown group|unsupported algorithm", "sm2.pem", '-text'),
+       "error loading unsupported sm2 cert");
+}
diff --git a/test/recipes/30-test_evp.t b/test/recipes/30-test_evp.t
index 17e2d17007..9a8bb74bb6 100644
--- a/test/recipes/30-test_evp.t
+++ b/test/recipes/30-test_evp.t
@@ -110,7 +110,8 @@ push @defltfiles, qw(evppkey_sm2.txt) unless $no_sm2;
 plan tests =>
     ($no_fips ? 0 : 1)          # FIPS install test
     + (scalar(@configs) * scalar(@files))
-    + scalar(@defltfiles);
+    + scalar(@defltfiles)
+    + 3; # error output tests
 
 unless ($no_fips) {
     my $infile = bldtop_file('providers', platform->dso('fips'));
@@ -139,3 +140,38 @@ foreach my $f ( @defltfiles ) {
                  data_file("$f")])),
        "running evp_test -config $conf $f");
 }
+
+sub test_errors { # actually tests diagnostics of OSSL_STORE
+    my ($expected, $key, @opts) = @_;
+    my $infile = srctop_file('test', 'certs', $key);
+    my @args = qw(openssl pkey -in);
+    push(@args, $infile, @opts);
+    my $tmpfile = 'out.txt';
+    my $res = !run(app([@args], stderr => $tmpfile));
+    my $found = 0;
+    open(my $in, '<', $tmpfile) or die "Could not open file $tmpfile";
+    while(<$in>) {
+        print; # this may help debugging
+        $res &&= !m/asn1 encoding/; # output must not include ASN.1 parse errors
+        $found = 1 if m/$expected/; # output must include $expected
+    }
+    close $in;
+    # $tmpfile is kept to help with investigation in case of failure
+    return $res && $found;
+}
+
+SKIP: {
+    skip "DSA not disabled", 2 if !disabled("dsa");
+
+    ok(test_errors("unsupported algorithm", "server-dsa-key.pem"),
+       "error loading unsupported dsa private key");
+    ok(test_errors("unsupported algorithm", "server-dsa-pubkey.pem", "-pubin"),
+       "error loading unsupported dsa public key");
+}
+
+SKIP: {
+    skip "sm2 not disabled", 1 if !disabled("sm2");
+
+    ok(test_errors("unknown group|unsupported algorithm", "sm2.key"),
+       "error loading unsupported sm2 private key");
+}


More information about the openssl-commits mailing list