Prune low-level ASN.1 parse errors from error queue in decoder_process()

Fixes #12840

Reviewed-by: Tomas Mraz <tmraz@fedoraproject.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/12893)

crypto/dh/dh_ameth.c

@@ -269,10 +269,8 @@ static int dh_param_decode(EVP_PKEY *pkey,
 {
     DH *dh;
 
-    if ((dh = d2i_dhp(pkey, pder, derlen)) == NULL) {
-        DHerr(DH_F_DH_PARAM_DECODE, ERR_R_DH_LIB);
+    if ((dh = d2i_dhp(pkey, pder, derlen)) == NULL)
         return 0;
-    }
     dh->dirty_cnt++;
     EVP_PKEY_assign(pkey, pkey->ameth->pkey_id, dh);
     return 1;

crypto/ec/ec_ameth.c

@@ -438,10 +438,8 @@ static int eckey_param_decode(EVP_PKEY *pkey,
 {
     EC_KEY *eckey;
 
-    if ((eckey = d2i_ECParameters(NULL, pder, derlen)) == NULL) {
-        ECerr(EC_F_ECKEY_PARAM_DECODE, ERR_R_EC_LIB);
+    if ((eckey = d2i_ECParameters(NULL, pder, derlen)) == NULL)
         return 0;
-    }
     EVP_PKEY_assign_EC_KEY(pkey, eckey);
     return 1;
 }

crypto/ec/ec_asn1.c

@@ -885,13 +885,11 @@ EC_GROUP *d2i_ECPKParameters(EC_GROUP **a, const unsigned char **in, long len)
     const unsigned char *p = *in;
 
     if ((params = d2i_ECPKPARAMETERS(NULL, &p, len)) == NULL) {
-        ECerr(EC_F_D2I_ECPKPARAMETERS, EC_R_D2I_ECPKPARAMETERS_FAILURE);
         ECPKPARAMETERS_free(params);
         return NULL;
     }
 
     if ((group = EC_GROUP_new_from_ecpkparameters(params)) == NULL) {
-        ECerr(EC_F_D2I_ECPKPARAMETERS, EC_R_PKPARAMETERS2GROUP_FAILURE);
         ECPKPARAMETERS_free(params);
         return NULL;
     }
@@ -934,10 +932,8 @@ EC_KEY *d2i_ECPrivateKey(EC_KEY **a, const unsigned char **in, long len)
     EC_PRIVATEKEY *priv_key = NULL;
     const unsigned char *p = *in;
 
-    if ((priv_key = d2i_EC_PRIVATEKEY(NULL, &p, len)) == NULL) {
-        ECerr(EC_F_D2I_ECPRIVATEKEY, ERR_R_EC_LIB);
+    if ((priv_key = d2i_EC_PRIVATEKEY(NULL, &p, len)) == NULL)
         return NULL;
-    }
 
     if (a == NULL || *a == NULL) {
         if ((ret = EC_KEY_new()) == NULL) {
@@ -1110,7 +1106,6 @@ EC_KEY *d2i_ECParameters(EC_KEY **a, const unsigned char **in, long len)
         ret = *a;
 
     if (!d2i_ECPKParameters(&ret->group, in, len)) {
-        ECerr(EC_F_D2I_ECPARAMETERS, ERR_R_EC_LIB);
         if (a == NULL || *a != ret)
              EC_KEY_free(ret);
         else

crypto/ec/ec_err.c

@@ -27,8 +27,6 @@ static const ERR_STRING_DATA EC_str_reasons[] = {
     "curve does not support ecdsa"},
     {ERR_PACK(ERR_LIB_EC, 0, EC_R_CURVE_DOES_NOT_SUPPORT_SIGNING),
     "curve does not support signing"},
-    {ERR_PACK(ERR_LIB_EC, 0, EC_R_D2I_ECPKPARAMETERS_FAILURE),
-    "d2i ecpkparameters failure"},
     {ERR_PACK(ERR_LIB_EC, 0, EC_R_DECODE_ERROR), "decode error"},
     {ERR_PACK(ERR_LIB_EC, 0, EC_R_DISCRIMINANT_IS_ZERO),
     "discriminant is zero"},
@@ -93,8 +91,6 @@ static const ERR_STRING_DATA EC_str_reasons[] = {
     {ERR_PACK(ERR_LIB_EC, 0, EC_R_PASSED_NULL_PARAMETER),
     "passed null parameter"},
     {ERR_PACK(ERR_LIB_EC, 0, EC_R_PEER_KEY_ERROR), "peer key error"},
-    {ERR_PACK(ERR_LIB_EC, 0, EC_R_PKPARAMETERS2GROUP_FAILURE),
-    "pkparameters2group failure"},
     {ERR_PACK(ERR_LIB_EC, 0, EC_R_POINT_ARITHMETIC_FAILURE),
     "point arithmetic failure"},
     {ERR_PACK(ERR_LIB_EC, 0, EC_R_POINT_AT_INFINITY), "point at infinity"},

crypto/err/openssl.txt

@@ -2412,7 +2412,6 @@ EC_R_COORDINATES_OUT_OF_RANGE:146:coordinates out of range
 EC_R_CURVE_DOES_NOT_SUPPORT_ECDH:160:curve does not support ecdh
 EC_R_CURVE_DOES_NOT_SUPPORT_ECDSA:170:curve does not support ecdsa
 EC_R_CURVE_DOES_NOT_SUPPORT_SIGNING:159:curve does not support signing
-EC_R_D2I_ECPKPARAMETERS_FAILURE:117:d2i ecpkparameters failure
 EC_R_DECODE_ERROR:142:decode error
 EC_R_DISCRIMINANT_IS_ZERO:118:discriminant is zero
 EC_R_EC_GROUP_NEW_BY_NAME_FAILURE:119:ec group new by name failure
@@ -2462,7 +2461,6 @@ EC_R_NO_PRIVATE_VALUE:154:no private value
 EC_R_OPERATION_NOT_SUPPORTED:152:operation not supported
 EC_R_PASSED_NULL_PARAMETER:134:passed null parameter
 EC_R_PEER_KEY_ERROR:149:peer key error
-EC_R_PKPARAMETERS2GROUP_FAILURE:127:pkparameters2group failure
 EC_R_POINT_ARITHMETIC_FAILURE:155:point arithmetic failure
 EC_R_POINT_AT_INFINITY:106:point at infinity
 EC_R_POINT_COORDINATES_BLIND_FAILURE:163:point coordinates blind failure
@@ -2571,7 +2569,6 @@ EVP_R_INVALID_SALT_LENGTH:186:invalid salt length
 EVP_R_INVALID_SECRET_LENGTH:223:invalid secret length
 EVP_R_INVALID_SEED_LENGTH:220:invalid seed length
 EVP_R_INVALID_VALUE:222:invalid value
-EVP_R_KEYGEN_FAILURE:120:keygen failure
 EVP_R_KEYMGMT_EXPORT_FAILURE:205:keymgmt export failure
 EVP_R_KEY_SETUP_FAILED:180:key setup failed
 EVP_R_LOCKING_NOT_SUPPORTED:213:locking not supported
@@ -2791,9 +2788,7 @@ PKCS12_R_MAC_SETUP_ERROR:110:mac setup error
 PKCS12_R_MAC_STRING_SET_ERROR:111:mac string set error
 PKCS12_R_MAC_VERIFY_FAILURE:113:mac verify failure
 PKCS12_R_PARSE_ERROR:114:parse error
-PKCS12_R_PKCS12_ALGOR_CIPHERINIT_ERROR:115:pkcs12 algor cipherinit error
 PKCS12_R_PKCS12_CIPHERFINAL_ERROR:116:pkcs12 cipherfinal error
-PKCS12_R_PKCS12_PBE_CRYPT_ERROR:117:pkcs12 pbe crypt error
 PKCS12_R_UNKNOWN_DIGEST_ALGORITHM:118:unknown digest algorithm
 PKCS12_R_UNSUPPORTED_PKCS12_MODE:119:unsupported pkcs12 mode
 PKCS7_R_CERTIFICATE_VERIFY_ERROR:117:certificate verify error

crypto/evp/evp_err.c

@@ -103,7 +103,6 @@ static const ERR_STRING_DATA EVP_str_reasons[] = {
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_INVALID_SEED_LENGTH),
     "invalid seed length"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_INVALID_VALUE), "invalid value"},
-    {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_KEYGEN_FAILURE), "keygen failure"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_KEYMGMT_EXPORT_FAILURE),
     "keymgmt export failure"},
     {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_KEY_SETUP_FAILED), "key setup failed"},

crypto/evp/evp_pbe.c

@@ -114,6 +114,7 @@ int EVP_PBE_CipherInit(ASN1_OBJECT *pbe_obj, const char *pass, int passlen,
         cipher = EVP_get_cipherbynid(cipher_nid);
         if (!cipher) {
             EVPerr(EVP_F_EVP_PBE_CIPHERINIT, EVP_R_UNKNOWN_CIPHER);
+            ERR_add_error_data(1, OBJ_nid2sn(cipher_nid));
             return 0;
         }
     }
@@ -128,11 +129,7 @@ int EVP_PBE_CipherInit(ASN1_OBJECT *pbe_obj, const char *pass, int passlen,
         }
     }
 
-    if (!keygen(ctx, pass, passlen, param, cipher, md, en_de)) {
-        EVPerr(EVP_F_EVP_PBE_CIPHERINIT, EVP_R_KEYGEN_FAILURE);
-        return 0;
-    }
-    return 1;
+    return keygen(ctx, pass, passlen, param, cipher, md, en_de);
 }
 
 DECLARE_OBJ_BSEARCH_CMP_FN(EVP_PBE_CTL, EVP_PBE_CTL, pbe2);

crypto/pkcs12/p12_decr.c

@@ -33,11 +33,8 @@ unsigned char *PKCS12_pbe_crypt(const X509_ALGOR *algor,
 
     /* Process data */
     if (!EVP_PBE_CipherInit(algor->algorithm, pass, passlen,
-                            algor->parameter, ctx, en_de)) {
-        PKCS12err(PKCS12_F_PKCS12_PBE_CRYPT,
-                  PKCS12_R_PKCS12_ALGOR_CIPHERINIT_ERROR);
+                            algor->parameter, ctx, en_de))
         goto err;
-    }
 
     /*
      * GOST algorithm specifics:
@@ -125,11 +122,8 @@ void *PKCS12_item_decrypt_d2i(const X509_ALGOR *algor, const ASN1_ITEM *it,
     int outlen = 0;
 
     if (!PKCS12_pbe_crypt(algor, pass, passlen, oct->data, oct->length,
-                          &out, &outlen, 0)) {
-        PKCS12err(PKCS12_F_PKCS12_ITEM_DECRYPT_D2I,
-                  PKCS12_R_PKCS12_PBE_CRYPT_ERROR);
+                          &out, &outlen, 0))
         return NULL;
-    }
     p = out;
     OSSL_TRACE_BEGIN(PKCS12_DECRYPT) {
         BIO_printf(trc_out, "\n");

crypto/pkcs12/p12_kiss.c

@@ -80,7 +80,11 @@ int PKCS12_parse(PKCS12 *p12, const char *pass, EVP_PKEY **pkey, X509 **cert,
     }
 
     if (!parse_pk12(p12, pass, -1, pkey, ocerts)) {
-        PKCS12err(PKCS12_F_PKCS12_PARSE, PKCS12_R_PARSE_ERROR);
+        int err = ERR_peek_last_error();
+
+        if (ERR_GET_LIB(err) != ERR_LIB_EVP
+                && ERR_GET_REASON(err) != EVP_R_UNSUPPORTED_ALGORITHM)
+            PKCS12err(0, PKCS12_R_PARSE_ERROR);
         goto err;
     }
 

crypto/pkcs12/pk12err.c

@@ -39,12 +39,8 @@ static const ERR_STRING_DATA PKCS12_str_reasons[] = {
     {ERR_PACK(ERR_LIB_PKCS12, 0, PKCS12_R_MAC_VERIFY_FAILURE),
     "mac verify failure"},
     {ERR_PACK(ERR_LIB_PKCS12, 0, PKCS12_R_PARSE_ERROR), "parse error"},
-    {ERR_PACK(ERR_LIB_PKCS12, 0, PKCS12_R_PKCS12_ALGOR_CIPHERINIT_ERROR),
-    "pkcs12 algor cipherinit error"},
     {ERR_PACK(ERR_LIB_PKCS12, 0, PKCS12_R_PKCS12_CIPHERFINAL_ERROR),
     "pkcs12 cipherfinal error"},
-    {ERR_PACK(ERR_LIB_PKCS12, 0, PKCS12_R_PKCS12_PBE_CRYPT_ERROR),
-    "pkcs12 pbe crypt error"},
     {ERR_PACK(ERR_LIB_PKCS12, 0, PKCS12_R_UNKNOWN_DIGEST_ALGORITHM),
     "unknown digest algorithm"},
     {ERR_PACK(ERR_LIB_PKCS12, 0, PKCS12_R_UNSUPPORTED_PKCS12_MODE),

crypto/rsa/rsa_ameth.c

@@ -113,10 +113,8 @@ static int rsa_pub_decode(EVP_PKEY *pkey, const X509_PUBKEY *pubkey)
 
     if (!X509_PUBKEY_get0_param(NULL, &p, &pklen, &alg, pubkey))
         return 0;
-    if ((rsa = d2i_RSAPublicKey(NULL, &p, pklen)) == NULL) {
-        RSAerr(RSA_F_RSA_PUB_DECODE, ERR_R_RSA_LIB);
+    if ((rsa = d2i_RSAPublicKey(NULL, &p, pklen)) == NULL)
         return 0;
-    }
     if (!rsa_param_decode(rsa, alg)) {
         RSA_free(rsa);
         return 0;
@@ -164,10 +162,8 @@ static int old_rsa_priv_decode(EVP_PKEY *pkey,
 {
     RSA *rsa;
 
-    if ((rsa = d2i_RSAPrivateKey(NULL, pder, derlen)) == NULL) {
-        RSAerr(RSA_F_OLD_RSA_PRIV_DECODE, ERR_R_RSA_LIB);
+    if ((rsa = d2i_RSAPrivateKey(NULL, pder, derlen)) == NULL)
         return 0;
-    }
     EVP_PKEY_assign(pkey, pkey->ameth->pkey_id, rsa);
     return 1;
 }

crypto/store/store_result.c

@@ -87,7 +87,8 @@ static int try_pkcs12(struct extracted_param_data_st *, OSSL_STORE_INFO **,
         int err = ERR_peek_last_error();                                \
                                                                         \
         if (ERR_GET_LIB(err) == ERR_LIB_ASN1                            \
-            && ERR_GET_REASON(err) == ERR_R_NESTED_ASN1_ERROR)          \
+            && (ERR_GET_REASON(err) == ASN1_R_UNKNOWN_PUBLIC_KEY_TYPE   \
+                || ERR_GET_REASON(err) == ERR_R_NESTED_ASN1_ERROR))     \
             ERR_pop_to_mark();                                          \
         else                                                            \
             ERR_clear_last_mark();                                      \
@@ -279,11 +280,13 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
     const unsigned char *der = data->octet_data, *derp;
     long der_len = (long)data->octet_data_size;
 
+    SET_ERR_MARK();
     /* Try PUBKEY first, that's a real easy target */
     derp = der;
     pk = d2i_PUBKEY_ex(NULL, &derp, der_len, libctx, propq);
     if (pk != NULL)
         *store_info_new = OSSL_STORE_INFO_new_PUBKEY;
+    RESET_ERR_MARK();
 
     /* Try private keys next */
     if (pk == NULL) {
@@ -319,6 +322,7 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
             }
             X509_SIG_free(p8);
         }
+        RESET_ERR_MARK();
 
         /*
          * If the encrypted PKCS#8 couldn't be decrypted,
@@ -328,6 +332,7 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
             /* Try to unpack an unencrypted PKCS#8, that's easy */
             derp = der;
             p8info = d2i_PKCS8_PRIV_KEY_INFO(NULL, &derp, der_len);
+            RESET_ERR_MARK();
             if (p8info != NULL) {
                 pk = EVP_PKCS82PKEY_with_libctx(p8info, libctx, propq);
                 PKCS8_PRIV_KEY_INFO_free(p8info);
@@ -344,6 +349,7 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
                 pk = d2i_PrivateKey_ex(EVP_PKEY_SM2, NULL,
                                        &derp, der_len,
                                        libctx, NULL);
+                RESET_ERR_MARK();
             }
         }
 
@@ -363,9 +369,11 @@ static EVP_PKEY *try_key_value_legacy(struct extracted_param_data_st *data,
     if (pk == NULL) {
         derp = der;
         pk = d2i_KeyParams(EVP_PKEY_SM2, NULL, &derp, der_len);
+        RESET_ERR_MARK();
         if (pk != NULL)
             *store_info_new = OSSL_STORE_INFO_new_PARAMS;
     }
+    CLEAR_ERR_MARK();
 
     return pk;
 }

include/openssl/ecerr.h

@@ -233,7 +233,6 @@ int ERR_load_EC_strings(void);
 #  define EC_R_CURVE_DOES_NOT_SUPPORT_ECDH                 160
 #  define EC_R_CURVE_DOES_NOT_SUPPORT_ECDSA                170
 #  define EC_R_CURVE_DOES_NOT_SUPPORT_SIGNING              159
-#  define EC_R_D2I_ECPKPARAMETERS_FAILURE                  117
 #  define EC_R_DECODE_ERROR                                142
 #  define EC_R_DISCRIMINANT_IS_ZERO                        118
 #  define EC_R_EC_GROUP_NEW_BY_NAME_FAILURE                119
@@ -283,7 +282,6 @@ int ERR_load_EC_strings(void);
 #  define EC_R_OPERATION_NOT_SUPPORTED                     152
 #  define EC_R_PASSED_NULL_PARAMETER                       134
 #  define EC_R_PEER_KEY_ERROR                              149
-#  define EC_R_PKPARAMETERS2GROUP_FAILURE                  127
 #  define EC_R_POINT_ARITHMETIC_FAILURE                    155
 #  define EC_R_POINT_AT_INFINITY                           106
 #  define EC_R_POINT_COORDINATES_BLIND_FAILURE             163

include/openssl/evperr.h

@@ -214,7 +214,6 @@ int ERR_load_EVP_strings(void);
 # define EVP_R_INVALID_SECRET_LENGTH                      223
 # define EVP_R_INVALID_SEED_LENGTH                        220
 # define EVP_R_INVALID_VALUE                              222
-# define EVP_R_KEYGEN_FAILURE                             120
 # define EVP_R_KEYMGMT_EXPORT_FAILURE                     205
 # define EVP_R_KEY_SETUP_FAILED                           180
 # define EVP_R_LOCKING_NOT_SUPPORTED                      213

include/openssl/pkcs12err.h

@@ -77,9 +77,7 @@ int ERR_load_PKCS12_strings(void);
 # define PKCS12_R_MAC_STRING_SET_ERROR                    111
 # define PKCS12_R_MAC_VERIFY_FAILURE                      113
 # define PKCS12_R_PARSE_ERROR                             114
-# define PKCS12_R_PKCS12_ALGOR_CIPHERINIT_ERROR           115
 # define PKCS12_R_PKCS12_CIPHERFINAL_ERROR                116
-# define PKCS12_R_PKCS12_PBE_CRYPT_ERROR                  117
 # define PKCS12_R_UNKNOWN_DIGEST_ALGORITHM                118
 # define PKCS12_R_UNSUPPORTED_PKCS12_MODE                 119
 

providers/implementations/encode_decode/decode_der2key.c

@@ -165,10 +165,11 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin,
     long new_der_len;
     EVP_PKEY *pkey = NULL;
     void *key = NULL;
-    int ok = 0;
+    int err, ok = 0;
 
+    ERR_set_mark();
     if (!read_der(ctx->provctx, cin, &der, &der_len))
-        return 0;
+        goto err;
 
     /*
      * Opportunistic attempt to decrypt.  If it doesn't work, we try to
@@ -192,6 +193,18 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin,
         derp = der;
         pkey = d2i_KeyParams(ctx->desc->type, NULL, &derp, der_len);
     }
+ err:
+    /*
+     * 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();
 
     if (pkey != NULL) {
         /*

providers/implementations/storemgmt/file_store_der2obj.c

@@ -27,6 +27,8 @@
 #include <openssl/core_object.h>
 #include <openssl/bio.h>
 #include <openssl/buffer.h>
+#include <openssl/err.h>
+#include <openssl/asn1err.h>
 #include <openssl/params.h>
 #include "internal/asn1.h"
 #include "prov/bio.h"
@@ -85,8 +87,21 @@ static int der2obj_decode(void *provctx, OSSL_CORE_BIO *cin,
      */
     BIO *in = (BIO *)cin;
     BUF_MEM *mem = NULL;
-    int ok = (asn1_d2i_read_bio(in, &mem) >= 0);
+    int err, ok;
 
+    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.
+     */
+    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();
     if (ok) {
         OSSL_PARAM params[3];
         int object_type = OSSL_OBJECT_UNKNOWN;

test/recipes/25-test_x509.t

@@ -16,7 +16,7 @@ use OpenSSL::Test qw/:DEFAULT srctop_file/;
 
 setup("test_x509");
 
-plan tests => 12;
+plan tests => 14;
 
 require_ok(srctop_file('test','recipes','tconversion.pl'));
 
@@ -100,3 +100,27 @@ sub has_doctor_id {
     close(DATA);
     return m/2.16.528.1.1003.1.3.5.5.2-1-0000006666-Z-12345678-01.015-12345678/;
 }
+
+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);
+    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;
+    unlink $tmpfile;
+    return $res && $found;
+}
+
+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");