diff options
author | Richard Levitte <levitte@openssl.org> | 2021-01-28 08:48:55 +0100 |
---|---|---|
committer | Richard Levitte <levitte@openssl.org> | 2021-03-19 16:46:39 +0100 |
commit | 65ef000ec26e6ec2fab2c31f74be1b76275bbbe5 (patch) | |
tree | 80fce9e3c46205f44c69007f2090a559848e6a20 /providers/implementations | |
parent | 2d101b0f493a3066c5ea7152c00c44d70fcea4d8 (diff) |
PROV: Refactor DER->key decoder
The decoding of DER into keys with keytype specific routines depended
entirely on the absence of the generic algo specific C type from
EVP_PKEYs. That is not necessary, and may even prove to be a bit of a
disadvantage, depending on what libcrypto has to offer in terms of
type specific d2i functionality for different kinds of input
structures.
To remedy, we try with all available type specific functions first,
and only turn to the general d2i functions (those that return an
EVP_PKEY) as a last resort.
Furthermore, there are cases where the decoder might not get the key
type it expected. This may happen when certain key types that share
the same OpenSSL structure may be mixed up somehow. The known cases
are EC vs SM2 and RSA vs RSA-PSS.
To remedy, we add the possibility to specify a checking function that
can check if the key that was decoded meets decoder expectations.
Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14314)
Diffstat (limited to 'providers/implementations')
-rw-r--r-- | providers/implementations/encode_decode/decode_der2key.c | 181 |
1 files changed, 125 insertions, 56 deletions
diff --git a/providers/implementations/encode_decode/decode_der2key.c b/providers/implementations/encode_decode/decode_der2key.c index 359116a8a9..47fe2bad6b 100644 --- a/providers/implementations/encode_decode/decode_der2key.c +++ b/providers/implementations/encode_decode/decode_der2key.c @@ -44,7 +44,8 @@ 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) == ERR_R_NESTED_ASN1_ERROR \ + || ERR_GET_REASON(err) == ASN1_R_NOT_ENOUGH_DATA)) \ ERR_pop_to_mark(); \ else \ ERR_clear_last_mark(); \ @@ -113,9 +114,10 @@ static OSSL_FUNC_decoder_decode_fn der2key_decode; static OSSL_FUNC_decoder_export_object_fn der2key_export_object; struct der2key_ctx_st; /* Forward declaration */ -typedef void *(extract_key_fn)(EVP_PKEY *); -typedef void (adjust_key_fn)(void *, struct der2key_ctx_st *ctx); -typedef void (free_key_fn)(void *); +typedef void *extract_key_fn(EVP_PKEY *); +typedef int check_key_fn(void *, struct der2key_ctx_st *ctx); +typedef void adjust_key_fn(void *, struct der2key_ctx_st *ctx); +typedef void free_key_fn(void *); struct keytype_desc_st { const char *keytype_name; const OSSL_DISPATCH *fns; /* Keymgmt (to pilfer functions from) */ @@ -143,6 +145,14 @@ struct keytype_desc_st { * For PKCS#8 decoders, we use EVP_PKEY extractors, EVP_PKEY_get1_{TYPE}() */ extract_key_fn *extract_key; + + /* + * For any key, we may need to check that the key meets expectations. + * This is useful when the same functions can decode several variants + * of a key. + */ + check_key_fn *check_key; + /* * For any key, we may need to make provider specific adjustments, such * as ensure the key carries the correct library context. @@ -260,7 +270,6 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, EVP_PKEY *pkey = NULL; void *key = NULL; int orig_selection = selection; - int dec_err; int ok = 0; /* @@ -280,54 +289,54 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, SET_ERR_MARK(); if (!read_der(ctx->provctx, cin, &der, &der_len)) - goto end; + goto next; - if (ctx->desc->extract_key == NULL) { - /* - * There's no EVP_PKEY extractor, so we use the type specific - * functions. - */ + /* We try the typs specific functions first, if available */ + if (ctx->desc->d2i_private_key != NULL + && (selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0) { + RESET_ERR_MARK(); derp = der; - if ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0) { - key = ctx->desc->d2i_private_key(NULL, &derp, der_len); - if (key == NULL && orig_selection != 0) - goto end; - } - if (key == NULL - && (selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0) { - key = ctx->desc->d2i_public_key(NULL, &derp, der_len); - if (key == NULL && orig_selection != 0) - goto end; - } - if (key == NULL - && (selection & OSSL_KEYMGMT_SELECT_ALL_PARAMETERS) != 0) { - key = ctx->desc->d2i_key_params(NULL, &derp, der_len); - } - } else { + key = ctx->desc->d2i_private_key(NULL, &derp, der_len); + if (key == NULL && orig_selection != 0) + goto next; + } + if (key == NULL + && ctx->desc->d2i_public_key != NULL + && (selection & OSSL_KEYMGMT_SELECT_PUBLIC_KEY) != 0) { + RESET_ERR_MARK(); + derp = der; + key = ctx->desc->d2i_public_key(NULL, &derp, der_len); + if (key == NULL && orig_selection != 0) + goto next; + } + if (key == NULL + && ctx->desc->d2i_key_params != NULL + && (selection & OSSL_KEYMGMT_SELECT_ALL_PARAMETERS) != 0) { + RESET_ERR_MARK(); + derp = der; + key = ctx->desc->d2i_key_params(NULL, &derp, der_len); + } + if (key == NULL + && ctx->desc->extract_key != NULL) { /* * There is a EVP_PKEY extractor, so we use the more generic * EVP_PKEY functions, since they know how to unpack PKCS#8 and * SubjectPublicKeyInfo. */ - /* - * Opportunistic attempt to decrypt. If it doesn't work, we try - * to decode our input unencrypted. - */ - if (der_from_p8(&new_der, &new_der_len, der, der_len, - pw_cb, pw_cbarg)) { - OPENSSL_free(der); - der = new_der; - der_len = new_der_len; - } - /* decryption errors are fatal and should be reported */ - dec_err = ERR_peek_last_error(); - if (ERR_GET_LIB(dec_err) == ERR_LIB_PROV - && ERR_GET_REASON(dec_err) == PROV_R_BAD_DECRYPT) - goto end; - - RESET_ERR_MARK(); if ((selection & OSSL_KEYMGMT_SELECT_PRIVATE_KEY) != 0) { + /* + * Opportunistic attempt to decrypt. If it doesn't work, we try + * to decode our input unencrypted. + */ + if (der_from_p8(&new_der, &new_der_len, der, der_len, + pw_cb, pw_cbarg)) { + OPENSSL_free(der); + der = new_der; + der_len = new_der_len; + } + RESET_ERR_MARK(); + derp = der; pkey = evp_privatekey_from_binary(ctx->desc->evp_type, NULL, &derp, der_len, libctx, NULL); @@ -361,10 +370,17 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, } } + if (key != NULL + && ctx->desc->check_key != NULL + && !ctx->desc->check_key(key, ctx)) { + CLEAR_ERR_MARK(); + goto end; + } + if (key != NULL && ctx->desc->adjust_key != NULL) ctx->desc->adjust_key(key, ctx); - end: + next: /* * Prune low-level ASN.1 parse errors from error queue, assuming * that this is called by decoder_process() in a loop trying several @@ -372,7 +388,13 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, */ CLEAR_ERR_MARK(); + /* + * We free memory 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. + */ OPENSSL_free(der); + der = NULL; if (key != NULL) { OSSL_PARAM params[4]; @@ -392,7 +414,10 @@ static int der2key_decode(void *vctx, OSSL_CORE_BIO *cin, int selection, ok = data_cb(params, data_cbarg); } + + end: ctx->desc->free_key(key); + OPENSSL_free(der); return ok; } @@ -425,6 +450,7 @@ static int der2key_export_object(void *vctx, # define dh_d2i_public_key NULL # define dh_d2i_key_params (d2i_of_void *)d2i_DHparams # define dh_free (free_key_fn *)DH_free +# define dh_check NULL static void dh_adjust(void *key, struct der2key_ctx_st *ctx) { @@ -437,6 +463,7 @@ static void dh_adjust(void *key, struct der2key_ctx_st *ctx) # define dhx_d2i_public_key NULL # define dhx_d2i_key_params (d2i_of_void *)d2i_DHxparams # define dhx_free (free_key_fn *)DH_free +# define dhx_check NULL # define dhx_adjust dh_adjust #endif @@ -449,6 +476,7 @@ static void dh_adjust(void *key, struct der2key_ctx_st *ctx) # define dsa_d2i_public_key (d2i_of_void *)d2i_DSAPublicKey # define dsa_d2i_key_params (d2i_of_void *)d2i_DSAparams # define dsa_free (free_key_fn *)DSA_free +# define dsa_check NULL static void dsa_adjust(void *key, struct der2key_ctx_st *ctx) { @@ -466,6 +494,15 @@ static void dsa_adjust(void *key, struct der2key_ctx_st *ctx) # define ec_d2i_key_params (d2i_of_void *)d2i_ECParameters # define ec_free (free_key_fn *)EC_KEY_free +static int ec_check(void *key, struct der2key_ctx_st *ctx) +{ + /* We're trying to be clever by comparing two truths */ + + int sm2 = (EC_KEY_get_flags(key) & EC_FLAG_SM2_RANGE) != 0; + + return sm2 == (ctx->desc->evp_type == EVP_PKEY_SM2); +} + static void ec_adjust(void *key, struct der2key_ctx_st *ctx) { ossl_ec_key_set0_libctx(key, PROV_LIBCTX_OF(ctx->provctx)); @@ -487,6 +524,7 @@ static void ecx_key_adjust(void *key, struct der2key_ctx_st *ctx) # define ed25519_d2i_public_key NULL # define ed25519_d2i_key_params NULL # define ed25519_free (free_key_fn *)ossl_ecx_key_free +# define ed25519_check NULL # define ed25519_adjust ecx_key_adjust # define ed448_evp_type EVP_PKEY_ED448 @@ -495,6 +533,7 @@ static void ecx_key_adjust(void *key, struct der2key_ctx_st *ctx) # define ed448_d2i_public_key NULL # define ed448_d2i_key_params NULL # define ed448_free (free_key_fn *)ossl_ecx_key_free +# define ed448_check NULL # define ed448_adjust ecx_key_adjust # define x25519_evp_type EVP_PKEY_X25519 @@ -503,6 +542,7 @@ static void ecx_key_adjust(void *key, struct der2key_ctx_st *ctx) # define x25519_d2i_public_key NULL # define x25519_d2i_key_params NULL # define x25519_free (free_key_fn *)ossl_ecx_key_free +# define x25519_check NULL # define x25519_adjust ecx_key_adjust # define x448_evp_type EVP_PKEY_X448 @@ -511,6 +551,7 @@ static void ecx_key_adjust(void *key, struct der2key_ctx_st *ctx) # define x448_d2i_public_key NULL # define x448_d2i_key_params NULL # define x448_free (free_key_fn *)ossl_ecx_key_free +# define x448_check NULL # define x448_adjust ecx_key_adjust # ifndef OPENSSL_NO_SM2 @@ -520,6 +561,7 @@ static void ecx_key_adjust(void *key, struct der2key_ctx_st *ctx) # define sm2_d2i_public_key NULL # define sm2_d2i_key_params (d2i_of_void *)d2i_ECParameters # define sm2_free (free_key_fn *)EC_KEY_free +# define sm2_check ec_check # define sm2_adjust ec_adjust # endif #endif @@ -533,6 +575,19 @@ static void ecx_key_adjust(void *key, struct der2key_ctx_st *ctx) #define rsa_d2i_key_params NULL #define rsa_free (free_key_fn *)RSA_free +static int rsa_check(void *key, struct der2key_ctx_st *ctx) +{ + switch (RSA_test_flags(key, RSA_FLAG_TYPE_MASK)) { + case RSA_FLAG_TYPE_RSA: + return ctx->desc->evp_type == EVP_PKEY_RSA; + case RSA_FLAG_TYPE_RSASSAPSS: + return ctx->desc->evp_type == EVP_PKEY_RSA_PSS; + } + + /* Currently unsupported RSA key type */ + return 0; +} + static void rsa_adjust(void *key, struct der2key_ctx_st *ctx) { ossl_rsa_set0_libctx(key, PROV_LIBCTX_OF(ctx->provctx)); @@ -544,6 +599,7 @@ static void rsa_adjust(void *key, struct der2key_ctx_st *ctx) #define rsapss_d2i_public_key (d2i_of_void *)d2i_RSAPublicKey #define rsapss_d2i_key_params NULL #define rsapss_free (free_key_fn *)RSA_free +#define rsapss_check rsa_check #define rsapss_adjust rsa_adjust /* ---------------------------------------------------------------------- */ @@ -553,63 +609,69 @@ static void rsa_adjust(void *key, struct der2key_ctx_st *ctx) * for each kind of object we want to decode. */ #define DO_type_specific_keypair(keytype) \ - "type-specific", 0, \ + "type-specific", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_KEYPAIR ), \ keytype##_d2i_private_key, \ keytype##_d2i_public_key, \ NULL, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free #define DO_type_specific_pub(keytype) \ - "type-specific", 0, \ + "type-specific", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_PUBLIC_KEY ), \ NULL, \ keytype##_d2i_public_key, \ NULL, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free #define DO_type_specific_priv(keytype) \ - "type-specific", 0, \ + "type-specific", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_PRIVATE_KEY ), \ keytype##_d2i_private_key, \ NULL, \ NULL, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free #define DO_type_specific_params(keytype) \ - "type-specific", 0, \ + "type-specific", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_ALL_PARAMETERS ), \ NULL, \ NULL, \ keytype##_d2i_key_params, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free #define DO_type_specific(keytype) \ - "type-specific", 0, \ + "type-specific", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_ALL ), \ keytype##_d2i_private_key, \ keytype##_d2i_public_key, \ keytype##_d2i_key_params, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free #define DO_type_specific_no_pub(keytype) \ - "type-specific", 0, \ + "type-specific", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_PRIVATE_KEY \ | OSSL_KEYMGMT_SELECT_ALL_PARAMETERS ), \ keytype##_d2i_private_key, \ NULL, \ keytype##_d2i_key_params, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free @@ -620,6 +682,7 @@ static void rsa_adjust(void *key, struct der2key_ctx_st *ctx) NULL, \ NULL, \ keytype##_evp_extract, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free @@ -630,57 +693,63 @@ static void rsa_adjust(void *key, struct der2key_ctx_st *ctx) NULL, \ NULL, \ keytype##_evp_extract, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free #define DO_DH(keytype) \ - "DH", 0, \ + "DH", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_ALL_PARAMETERS ), \ NULL, \ NULL, \ keytype##_d2i_key_params, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free #define DO_DHX(keytype) \ - "DHX", 0, \ + "DHX", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_ALL_PARAMETERS ), \ NULL, \ NULL, \ keytype##_d2i_key_params, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free #define DO_DSA(keytype) \ - "DSA", 0, \ + "DSA", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_ALL ), \ keytype##_d2i_private_key, \ keytype##_d2i_public_key, \ keytype##_d2i_key_params, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free #define DO_EC(keytype) \ - "EC", 0, \ + "EC", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_PRIVATE_KEY \ | OSSL_KEYMGMT_SELECT_ALL_PARAMETERS ), \ keytype##_d2i_private_key, \ NULL, \ keytype##_d2i_key_params, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free #define DO_RSA(keytype) \ - "RSA", 0, \ + "RSA", keytype##_evp_type, \ ( OSSL_KEYMGMT_SELECT_KEYPAIR ), \ keytype##_d2i_private_key, \ keytype##_d2i_public_key, \ NULL, \ NULL, \ + keytype##_check, \ keytype##_adjust, \ keytype##_free |