Adapt our decoder implementations to the new way to indicate succes / failure

This includes the special decoder used in our STOREMGMT 'file:' implementation

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14834)

4 files changed, 77 insertions, 44 deletions

diff --git a/providers/implementations/encode_decode/decode_der2key.c b/providers/implementations/encode_decode/decode_der2key.c
index f50fca3896..73acf527c1 100644
--- a/providers/implementations/encode_decode/decode_der2key.c
+++ b/providers/implementations/encode_decode/decode_der2key.c
@@ -36,26 +36,6 @@
 #include "prov/implementations.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_GET_REASON(err) == ASN1_R_NOT_ENOUGH_DATA))      \
-            ERR_pop_to_mark();                                          \
-        else                                                            \
-            ERR_clear_last_mark();                                      \
-    } while(0)
-#define RESET_ERR_MARK()                                                \
-    do {                                                                \
-        CLEAR_ERR_MARK();                                               \
-        SET_ERR_MARK();                                                 \
-    } while(0)
-
 struct der2key_ctx_st;           /* Forward declaration */
 typedef int check_key_fn(void *, struct der2key_ctx_st *ctx);
 typedef void adjust_key_fn(void *, struct der2key_ctx_st *ctx);
@@ -143,6 +123,7 @@ static void *der2key_decode_p8(const unsigned char **input_der,
     void *key = NULL;
 
     ctx->flag_fatal = 0;
+
     if ((p8 = d2i_X509_SIG(NULL, input_der, input_der_len)) != NULL) {
         char pbuf[PEM_BUFSIZE];
         size_t plen = 0;
@@ -162,6 +143,7 @@ static void *der2key_decode_p8(const unsigned char **input_der,
         && OBJ_obj2nid(alg->algorithm) == ctx->desc->evp_type)
         key = key_from_pkcs8(p8inf, PROV_LIBCTX_OF(ctx->provctx), NULL);
     PKCS8_PRIV_KEY_INFO_free(p8inf);
+
     return key;
 }
 
@@ -284,12 +266,13 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
         return 0;
     }
 
-    SET_ERR_MARK();
-    if (!read_der(ctx->provctx, cin, &der, &der_len))
+    ok = read_der(ctx->provctx, cin, &der, &der_len);
+    if (!ok)
         goto next;
 
+    ok = 0;                      /* Assume that we fail */
+
     if ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0) {
-        RESET_ERR_MARK();
         derp = der;
         if (ctx->desc->d2i_PKCS8 != NULL) {
             key = ctx->desc->d2i_PKCS8(NULL, &derp, der_len, ctx,
@@ -303,7 +286,6 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
             goto next;
     }
     if (key == NULL && (selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0) {
-        RESET_ERR_MARK();
         derp = der;
         if (ctx->desc->d2i_PUBKEY != NULL)
             key = ctx->desc->d2i_PUBKEY(NULL, &derp, der_len);
@@ -313,19 +295,25 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
             goto next;
     }
     if (key == NULL && (selection & OSSL_KEYMGMT_SELECT_ALL_PARAMETERS) != 0) {
-        RESET_ERR_MARK();
         derp = der;
         if (ctx->desc->d2i_key_params != NULL)
             key = ctx->desc->d2i_key_params(NULL, &derp, der_len);
         if (key == NULL && orig_selection != 0)
             goto next;
     }
-    RESET_ERR_MARK();
+
+    /*
+     * Last minute check to see if this was the correct type of key.  This
+     * should never lead to a fatal error, i.e. the decoding itself was
+     * correct, it was just an unexpected key type.  This is generally for
+     * classes of key types that have subtle variants, like RSA-PSS keys as
+     * opposed to plain RSA keys.
+     */
     if (key != NULL
         && ctx->desc->check_key != NULL
         && !ctx->desc->check_key(key, ctx)) {
-        CLEAR_ERR_MARK();
-        goto end;
+        ctx->desc->free_key(key);
+        key = NULL;
     }
 
     if (key != NULL && ctx->desc->adjust_key != NULL)
@@ -333,11 +321,10 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
 
  next:
     /*
-     * Prune low-level ASN.1 parse errors from error queue, assuming
-     * that this is called by decoder_process() in a loop trying several
-     * formats.
+     * Indicated that we successfully decoded something, or not at all.
+     * Ending up "empty handed" is not an error.
      */
-    CLEAR_ERR_MARK();
+    ok = 1;
 
     /*
      * We free memory here so it's not held up during the callback, because
diff --git a/providers/implementations/encode_decode/decode_msblob2key.c b/providers/implementations/encode_decode/decode_msblob2key.c
index f47d06f59d..84b259591b 100644
--- a/providers/implementations/encode_decode/decode_msblob2key.c
+++ b/providers/implementations/encode_decode/decode_msblob2key.c
@@ -116,30 +116,35 @@ static int msblob2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
 
     if (BIO_read(in, hdr_buf, 16) != 16) {
         ERR_raise(ERR_LIB_PEM, PEM_R_KEYBLOB_TOO_SHORT);
-        goto err;
+        goto next;
     }
+    ERR_set_mark();
     p = hdr_buf;
-    if (ossl_do_blob_header(&p, 16, &magic, &bitlen, &isdss, &ispub) <= 0)
-        goto err;
+    ok = ossl_do_blob_header(&p, 16, &magic, &bitlen, &isdss, &ispub) > 0;
+    ERR_pop_to_mark();
+    if (!ok)
+        goto next;
+
+    ok = 0;                      /* Assume that we fail */
 
     if ((isdss && ctx->desc->type != EVP_PKEY_DSA)
         || (!isdss && ctx->desc->type != EVP_PKEY_RSA))
-        goto err;
+        goto next;
 
     length = ossl_blob_length(bitlen, isdss, ispub);
     if (length > BLOB_MAX_LENGTH) {
         ERR_raise(ERR_LIB_PEM, PEM_R_HEADER_TOO_LONG);
-        goto err;
+        goto next;
     }
     buf = OPENSSL_malloc(length);
     if (buf == NULL) {
         ERR_raise(ERR_LIB_PEM, ERR_R_MALLOC_FAILURE);
-        goto err;
+        goto end;
     }
     p = buf;
     if (BIO_read(in, buf, length) != (int)length) {
         ERR_raise(ERR_LIB_PEM, PEM_R_KEYBLOB_TOO_SHORT);
-        goto err;
+        goto next;
     }
 
     if ((selection == 0
@@ -150,7 +155,7 @@ static int msblob2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
 
         memset(&pwdata, 0, sizeof(pwdata));
         if (!ossl_pw_set_ossl_passphrase_cb(&pwdata, pw_cb, pw_cbarg))
-            goto err;
+            goto end;
         p = buf;
         key = ctx->desc->read_private_key(&p, bitlen, ispub);
         if (selection != 0 && key == NULL)
@@ -171,6 +176,12 @@ static int msblob2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
 
  next:
     /*
+     * Indicated that we successfully decoded something, or not at all.
+     * Ending up "empty handed" is not an error.
+     */
+    ok = 1;
+
+    /*
      * We free resources here so it's not held up during the callback, because
      * we know the process is recursive and the allocated chunks of memory
      * add up.
@@ -198,7 +209,7 @@ static int msblob2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
         ok = data_cb(params, data_cbarg);
     }
 
- err:
+ end:
     BIO_free(in);
     OPENSSL_free(buf);
     ctx->desc->free_key(key);
diff --git a/providers/implementations/encode_decode/decode_pem2der.c b/providers/implementations/encode_decode/decode_pem2der.c
index fe6839965d..4249ce9cc7 100644
--- a/providers/implementations/encode_decode/decode_pem2der.c
+++ b/providers/implementations/encode_decode/decode_pem2der.c
@@ -145,9 +145,11 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
     int objtype = OSSL_OBJECT_UNKNOWN;
     const char *data_structure = NULL;
 
-    if (read_pem(ctx->provctx, cin, &pem_name, &pem_header,
-                 &der, &der_len) <= 0)
-        return 0;
+    ok = read_pem(ctx->provctx, cin, &pem_name, &pem_header,
+                  &der, &der_len) > 0;
+    /* We return "empty handed".  This is not an error. */
+    if (!ok)
+        return 1;
 
     /*
      * 10 is the number of characters in "Proc-Type:", which
@@ -159,6 +161,7 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
         EVP_CIPHER_INFO cipher;
         struct pem2der_pass_data_st pass_data;
 
+        ok = 0;                  /* Assume that we fail */
         pass_data.cb = pw_cb;
         pass_data.cbarg = pw_cbarg;
         if (!PEM_get_EVP_CIPHER_INFO(pem_header, &cipher)
@@ -168,6 +171,12 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
     }
 
     /*
+     * Indicated that we successfully decoded something, or not at all.
+     * Ending up "empty handed" is not an error.
+     */
+    ok = 1;
+
+    /*
      * Peal off certain strings from the end of |pem_name|, as they serve
      * no further purpose.
      */
diff --git a/providers/implementations/encode_decode/decode_pvk2key.c b/providers/implementations/encode_decode/decode_pvk2key.c
index 3f2c80abdc..702c89a928 100644
--- a/providers/implementations/encode_decode/decode_pvk2key.c
+++ b/providers/implementations/encode_decode/decode_pvk2key.c
@@ -20,6 +20,7 @@
 #include <openssl/core_object.h>
 #include <openssl/crypto.h>
 #include <openssl/params.h>
+#include <openssl/err.h>
 #include <openssl/pem.h>         /* For public PVK functions */
 #include <openssl/x509.h>
 #include "internal/passphrase.h"
@@ -111,11 +112,30 @@ static int pvk2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
          || (selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0)
         && ctx->desc->read_private_key != NULL) {
         struct ossl_passphrase_data_st pwdata;
+        int err, lib, reason;
 
         memset(&pwdata, 0, sizeof(pwdata));
         if (!ossl_pw_set_ossl_passphrase_cb(&pwdata, pw_cb, pw_cbarg))
             goto end;
+
         key = ctx->desc->read_private_key(in, ossl_pw_pem_password, &pwdata);
+
+        /*
+         * Because the PVK API doesn't have a separate decrypt call, we need
+         * to check the error queue for certain well known errors that are
+         * considered fatal and which we pass through, while the rest gets
+         * thrown away.
+         */
+        err = ERR_peek_last_error();
+        lib = ERR_GET_LIB(err);
+        reason = ERR_GET_REASON(err);
+        if (lib == ERR_LIB_PEM
+            && (reason == PEM_R_BAD_PASSWORD_READ
+                || reason == PEM_R_BAD_DECRYPT)) {
+            ERR_clear_last_mark();
+            goto end;
+        }
+
         if (selection != 0 && key == NULL)
             goto next;
     }
@@ -125,6 +145,12 @@ static int pvk2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection,
 
  next:
     /*
+     * Indicated that we successfully decoded something, or not at all.
+     * Ending up "empty handed" is not an error.
+     */
+    ok = 1;
+
+    /*
      * We free resources here so it's not held up during the callback, because
      * we know the process is recursive and the allocated chunks of memory
      * add up.