From 9cc97ddf3c8c3c6ef30b0505ad2559d3734c685d Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Mon, 12 Apr 2021 12:20:20 +0200 Subject: 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 (Merged from https://github.com/openssl/openssl/pull/14834) --- .../implementations/encode_decode/decode_der2key.c | 51 ++++++++-------------- .../encode_decode/decode_msblob2key.c | 29 ++++++++---- .../implementations/encode_decode/decode_pem2der.c | 15 +++++-- .../implementations/encode_decode/decode_pvk2key.c | 26 +++++++++++ 4 files changed, 77 insertions(+), 44 deletions(-) (limited to 'providers/implementations/encode_decode') 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) @@ -170,6 +175,12 @@ static int msblob2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, ctx->desc->adjust_key(key, ctx); 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 @@ -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) @@ -167,6 +170,12 @@ static int pem2der_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, goto end; } + /* + * 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 #include #include +#include #include /* For public PVK functions */ #include #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; } @@ -124,6 +144,12 @@ static int pvk2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, ctx->desc->adjust_key(key, ctx); 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 -- cgit v1.2.3