From 6d4e6009d27712a405e1e3a4c33fb8a8566f134a Mon Sep 17 00:00:00 2001 From: Pauli Date: Thu, 26 Mar 2020 09:28:01 +1000 Subject: Param build: make structures opaque. Since this is public, it is best to make the underlying structure opaque. This means converting from stack allocation to dynamic allocation for all usages. Reviewed-by: Nicola Tuveri (Merged from https://github.com/openssl/openssl/pull/11390) --- crypto/dh/dh_ameth.c | 35 +++++++++++++++++---------------- crypto/dsa/dsa_ameth.c | 32 +++++++++++++++++-------------- crypto/ec/ec_ameth.c | 19 ++++++++++-------- crypto/ec/ecx_meth.c | 14 ++++++++------ crypto/param_build.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++--- crypto/rsa/rsa_ameth.c | 31 +++++++++++++----------------- crypto/rsa/rsa_lib.c | 14 +++++++++----- 7 files changed, 127 insertions(+), 70 deletions(-) (limited to 'crypto') diff --git a/crypto/dh/dh_ameth.c b/crypto/dh/dh_ameth.c index 505211054f..86e78aaf6c 100644 --- a/crypto/dh/dh_ameth.c +++ b/crypto/dh/dh_ameth.c @@ -494,13 +494,13 @@ static int dh_pkey_export_to(const EVP_PKEY *from, void *to_keydata, EVP_KEYMGMT *to_keymgmt) { DH *dh = from->pkey.dh; - OSSL_PARAM_BLD tmpl; + OSSL_PARAM_BLD *tmpl; const BIGNUM *p = DH_get0_p(dh), *g = DH_get0_g(dh), *q = DH_get0_q(dh); const BIGNUM *pub_key = DH_get0_pub_key(dh); const BIGNUM *priv_key = DH_get0_priv_key(dh); - OSSL_PARAM *params; + OSSL_PARAM *params = NULL; int selection = 0; - int rv; + int rv = 0; /* * If the DH method is foreign, then we can't be sure of anything, and @@ -512,35 +512,38 @@ static int dh_pkey_export_to(const EVP_PKEY *from, void *to_keydata, if (p == NULL || g == NULL) return 0; - OSSL_PARAM_BLD_init(&tmpl); - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_FFC_P, p) - || !OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_FFC_G, g)) + tmpl = OSSL_PARAM_BLD_new(); + if (tmpl == NULL) return 0; + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_FFC_P, p) + || !OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_FFC_G, g)) + goto err; if (q != NULL) { - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_FFC_Q, q)) - return 0; + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_FFC_Q, q)) + goto err; } selection |= OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS; if (pub_key != NULL) { - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_PUB_KEY, pub_key)) - return 0; + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_PUB_KEY, pub_key)) + goto err; selection |= OSSL_KEYMGMT_SELECT_PUBLIC_KEY; } if (priv_key != NULL) { - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_PRIV_KEY, + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_PRIV_KEY, priv_key)) - return 0; + goto err; selection |= OSSL_KEYMGMT_SELECT_PRIVATE_KEY; } - if ((params = OSSL_PARAM_BLD_to_param(&tmpl)) == NULL) - return 0; + if ((params = OSSL_PARAM_BLD_to_param(tmpl)) == NULL) + goto err; /* We export, the provider imports */ rv = evp_keymgmt_import(to_keymgmt, to_keydata, selection, params); - OSSL_PARAM_BLD_free(params); - + OSSL_PARAM_BLD_free_params(params); +err: + OSSL_PARAM_BLD_free(tmpl); return rv; } diff --git a/crypto/dsa/dsa_ameth.c b/crypto/dsa/dsa_ameth.c index 1de5a2da9b..cc72189cdb 100644 --- a/crypto/dsa/dsa_ameth.c +++ b/crypto/dsa/dsa_ameth.c @@ -523,13 +523,13 @@ static int dsa_pkey_export_to(const EVP_PKEY *from, void *to_keydata, EVP_KEYMGMT *to_keymgmt) { DSA *dsa = from->pkey.dsa; - OSSL_PARAM_BLD tmpl; + OSSL_PARAM_BLD *tmpl; const BIGNUM *p = DSA_get0_p(dsa), *g = DSA_get0_g(dsa); const BIGNUM *q = DSA_get0_q(dsa), *pub_key = DSA_get0_pub_key(dsa); const BIGNUM *priv_key = DSA_get0_priv_key(dsa); OSSL_PARAM *params; int selection = 0; - int rv; + int rv = 0; /* * If the DSA method is foreign, then we can't be sure of anything, and @@ -541,33 +541,37 @@ static int dsa_pkey_export_to(const EVP_PKEY *from, void *to_keydata, if (p == NULL || q == NULL || g == NULL) return 0; - OSSL_PARAM_BLD_init(&tmpl); - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_FFC_P, p) - || !OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_FFC_Q, q) - || !OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_FFC_G, g)) + tmpl = OSSL_PARAM_BLD_new(); + if (tmpl == NULL) return 0; + + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_FFC_P, p) + || !OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_FFC_Q, q) + || !OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_FFC_G, g)) + goto err; selection |= OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS; if (pub_key != NULL) { - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_PUB_KEY, + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_PUB_KEY, pub_key)) - return 0; + goto err; selection |= OSSL_KEYMGMT_SELECT_PUBLIC_KEY; } if (priv_key != NULL) { - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_PRIV_KEY, + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_PRIV_KEY, priv_key)) - return 0; + goto err; selection |= OSSL_KEYMGMT_SELECT_PRIVATE_KEY; } - if ((params = OSSL_PARAM_BLD_to_param(&tmpl)) == NULL) - return 0; + if ((params = OSSL_PARAM_BLD_to_param(tmpl)) == NULL) + goto err; /* We export, the provider imports */ rv = evp_keymgmt_import(to_keymgmt, to_keydata, selection, params); - OSSL_PARAM_BLD_free(params); - + OSSL_PARAM_BLD_free_params(params); +err: + OSSL_PARAM_BLD_free(tmpl); return rv; } diff --git a/crypto/ec/ec_ameth.c b/crypto/ec/ec_ameth.c index 85427cf456..65af8cc3c5 100644 --- a/crypto/ec/ec_ameth.c +++ b/crypto/ec/ec_ameth.c @@ -626,7 +626,7 @@ int ec_pkey_export_to(const EVP_PKEY *from, void *to_keydata, const EC_GROUP *ecg = NULL; unsigned char *pub_key_buf = NULL; size_t pub_key_buflen; - OSSL_PARAM_BLD tmpl; + OSSL_PARAM_BLD *tmpl; OSSL_PARAM *params = NULL; const BIGNUM *priv_key = NULL; const EC_POINT *pub_point = NULL; @@ -645,10 +645,12 @@ int ec_pkey_export_to(const EVP_PKEY *from, void *to_keydata, if (EC_KEY_get_method(eckey) != EC_KEY_OpenSSL()) return 0; - OSSL_PARAM_BLD_init(&tmpl); + tmpl = OSSL_PARAM_BLD_new(); + if (tmpl == NULL) + return 0; /* export the domain parameters */ - if (!ecparams_to_params(eckey, &tmpl)) + if (!ecparams_to_params(eckey, tmpl)) goto err; selection |= OSSL_KEYMGMT_SELECT_DOMAIN_PARAMETERS; @@ -660,7 +662,7 @@ int ec_pkey_export_to(const EVP_PKEY *from, void *to_keydata, if ((pub_key_buflen = EC_POINT_point2buf(ecg, pub_point, POINT_CONVERSION_COMPRESSED, &pub_key_buf, NULL)) == 0 - || !OSSL_PARAM_BLD_push_octet_string(&tmpl, + || !OSSL_PARAM_BLD_push_octet_string(tmpl, OSSL_PKEY_PARAM_PUB_KEY, pub_key_buf, pub_key_buflen)) @@ -711,7 +713,7 @@ int ec_pkey_export_to(const EVP_PKEY *from, void *to_keydata, goto err; sz = (ecbits + 7 ) / 8; - if (!OSSL_PARAM_BLD_push_BN_pad(&tmpl, + if (!OSSL_PARAM_BLD_push_BN_pad(tmpl, OSSL_PKEY_PARAM_PRIV_KEY, priv_key, sz)) goto err; @@ -726,20 +728,21 @@ int ec_pkey_export_to(const EVP_PKEY *from, void *to_keydata, (EC_KEY_get_flags(eckey) & EC_FLAG_COFACTOR_ECDH) ? 1 : 0; /* Export the ECDH_COFACTOR_MODE parameter */ - if (!OSSL_PARAM_BLD_push_int(&tmpl, + if (!OSSL_PARAM_BLD_push_int(tmpl, OSSL_PKEY_PARAM_USE_COFACTOR_ECDH, ecdh_cofactor_mode)) goto err; selection |= OSSL_KEYMGMT_SELECT_OTHER_PARAMETERS; } - params = OSSL_PARAM_BLD_to_param(&tmpl); + params = OSSL_PARAM_BLD_to_param(tmpl); /* We export, the provider imports */ rv = evp_keymgmt_import(to_keymgmt, to_keydata, selection, params); err: - OSSL_PARAM_BLD_free(params); + OSSL_PARAM_BLD_free(tmpl); + OSSL_PARAM_BLD_free_params(params); OPENSSL_free(pub_key_buf); return rv; } diff --git a/crypto/ec/ecx_meth.c b/crypto/ec/ecx_meth.c index 8a48b28f38..c142552b29 100644 --- a/crypto/ec/ecx_meth.c +++ b/crypto/ec/ecx_meth.c @@ -409,34 +409,36 @@ static int ecx_pkey_export_to(const EVP_PKEY *from, void *to_keydata, EVP_KEYMGMT *to_keymgmt) { const ECX_KEY *key = from->pkey.ecx; - OSSL_PARAM_BLD tmpl; + OSSL_PARAM_BLD *tmpl = OSSL_PARAM_BLD_new(); OSSL_PARAM *params = NULL; int selection = 0; int rv = 0; - OSSL_PARAM_BLD_init(&tmpl); + if (tmpl == NULL) + return 0; /* A key must at least have a public part */ - if (!OSSL_PARAM_BLD_push_octet_string(&tmpl, OSSL_PKEY_PARAM_PUB_KEY, + if (!OSSL_PARAM_BLD_push_octet_string(tmpl, OSSL_PKEY_PARAM_PUB_KEY, key->pubkey, key->keylen)) goto err; selection |= OSSL_KEYMGMT_SELECT_PUBLIC_KEY; if (key->privkey != NULL) { - if (!OSSL_PARAM_BLD_push_octet_string(&tmpl, + if (!OSSL_PARAM_BLD_push_octet_string(tmpl, OSSL_PKEY_PARAM_PRIV_KEY, key->privkey, key->keylen)) goto err; selection |= OSSL_KEYMGMT_SELECT_PRIVATE_KEY; } - params = OSSL_PARAM_BLD_to_param(&tmpl); + params = OSSL_PARAM_BLD_to_param(tmpl); /* We export, the provider imports */ rv = evp_keymgmt_import(to_keymgmt, to_keydata, selection, params); err: - OSSL_PARAM_BLD_free(params); + OSSL_PARAM_BLD_free(tmpl); + OSSL_PARAM_BLD_free_params(params); return rv; } diff --git a/crypto/param_build.c b/crypto/param_build.c index ca4fc4af7e..11986d999b 100644 --- a/crypto/param_build.c +++ b/crypto/param_build.c @@ -12,11 +12,47 @@ #include #include #include +#include #include "internal/cryptlib.h" #include "openssl/param_build.h" +/* + * The number of OSSL_PARAM elements a builder will allow. + */ +#define OSSL_PARAM_BLD_MAX 25 + +/* + * Special internal param type to indicate the end of an allocate OSSL_PARAM + * array. + */ #define OSSL_PARAM_ALLOCATED_END 127 +typedef struct { + const char *key; + int type; + int secure; + size_t size; + size_t alloc_blocks; + const BIGNUM *bn; + const void *string; + union { + /* + * These fields are never directly addressed, but their sizes are + * imporant so that all native types can be copied here without overrun. + */ + ossl_intmax_t i; + ossl_uintmax_t u; + double d; + } num; +} OSSL_PARAM_BLD_DEF; + +struct ossl_param_bld_st { + size_t curr; + size_t total_blocks; + size_t secure_blocks; + OSSL_PARAM_BLD_DEF params[OSSL_PARAM_BLD_MAX]; +}; + typedef union { OSSL_UNION_ALIGN; } OSSL_PARAM_BLD_BLOCK; @@ -66,9 +102,14 @@ static int param_push_num(OSSL_PARAM_BLD *bld, const char *key, return 1; } -void OSSL_PARAM_BLD_init(OSSL_PARAM_BLD *bld) +OSSL_PARAM_BLD *OSSL_PARAM_BLD_new(void) { - memset(bld, 0, sizeof(*bld)); + return OPENSSL_zalloc(sizeof(OSSL_PARAM_BLD)); +} + +void OSSL_PARAM_BLD_free(OSSL_PARAM_BLD *bld) +{ + OPENSSL_free(bld); } int OSSL_PARAM_BLD_push_int(OSSL_PARAM_BLD *bld, const char *key, int num) @@ -301,12 +342,14 @@ OSSL_PARAM *OSSL_PARAM_BLD_to_param(OSSL_PARAM_BLD *bld) if (s == NULL) { CRYPTOerr(CRYPTO_F_OSSL_PARAM_BLD_TO_PARAM, CRYPTO_R_SECURE_MALLOC_FAILURE); + OPENSSL_free(bld); return NULL; } } params = OPENSSL_malloc(total); if (params == NULL) { CRYPTOerr(CRYPTO_F_OSSL_PARAM_BLD_TO_PARAM, ERR_R_MALLOC_FAILURE); + OPENSSL_free(bld); OPENSSL_secure_free(s); return NULL; } @@ -315,10 +358,13 @@ OSSL_PARAM *OSSL_PARAM_BLD_to_param(OSSL_PARAM_BLD *bld) last->data_size = ss; last->data = s; last->data_type = OSSL_PARAM_ALLOCATED_END; + + /* Reset for reuse */ + memset(bld, 0, sizeof(*bld)); return params; } -void OSSL_PARAM_BLD_free(OSSL_PARAM *params) +void OSSL_PARAM_BLD_free_params(OSSL_PARAM *params) { if (params != NULL) { OSSL_PARAM *p; diff --git a/crypto/rsa/rsa_ameth.c b/crypto/rsa/rsa_ameth.c index ba82f6ccb7..ec8df4a718 100644 --- a/crypto/rsa/rsa_ameth.c +++ b/crypto/rsa/rsa_ameth.c @@ -1084,7 +1084,7 @@ static int rsa_pkey_export_to(const EVP_PKEY *from, void *to_keydata, EVP_KEYMGMT *to_keymgmt) { RSA *rsa = from->pkey.rsa; - OSSL_PARAM_BLD tmpl; + OSSL_PARAM_BLD *tmpl = OSSL_PARAM_BLD_new(); const BIGNUM *n = RSA_get0_n(rsa), *e = RSA_get0_e(rsa); const BIGNUM *d = RSA_get0_d(rsa); STACK_OF(BIGNUM_const) *primes = NULL, *exps = NULL, *coeffs = NULL; @@ -1093,23 +1093,23 @@ static int rsa_pkey_export_to(const EVP_PKEY *from, void *to_keydata, int selection = 0; int rv = 0; + if (tmpl == NULL) + return 0; /* * If the RSA method is foreign, then we can't be sure of anything, and * can therefore not export or pretend to export. */ if (RSA_get_method(rsa) != RSA_PKCS1_OpenSSL()) - return 0; + goto err; /* Public parameters must always be present */ if (n == NULL || e == NULL) goto err; - OSSL_PARAM_BLD_init(&tmpl); - /* |e| and |n| are always present */ - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_RSA_E, e)) + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_RSA_E, e)) goto err; - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_RSA_N, n)) + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_RSA_N, n)) goto err; selection |= OSSL_KEYMGMT_SELECT_PUBLIC_KEY; @@ -1138,20 +1138,14 @@ static int rsa_pkey_export_to(const EVP_PKEY *from, void *to_keydata, && (numprimes < 2 || numexps < 2 || numcoeffs < 1)) goto err; - /* assert that an OSSL_PARAM_BLD has enough space. */ - if (!ossl_assert(/* n, e */ 2 + /* d */ 1 + /* numprimes */ 1 - + numprimes + numexps + numcoeffs - <= OSSL_PARAM_BLD_MAX)) - goto err; - - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_RSA_D, d)) + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_RSA_D, d)) goto err; selection |= OSSL_KEYMGMT_SELECT_PRIVATE_KEY; for (i = 0; i < numprimes; i++) { const BIGNUM *num = sk_BIGNUM_const_value(primes, i); - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_RSA_FACTOR, + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_RSA_FACTOR, num)) goto err; } @@ -1159,7 +1153,7 @@ static int rsa_pkey_export_to(const EVP_PKEY *from, void *to_keydata, for (i = 0; i < numexps; i++) { const BIGNUM *num = sk_BIGNUM_const_value(exps, i); - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_RSA_EXPONENT, + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_RSA_EXPONENT, num)) goto err; } @@ -1167,13 +1161,13 @@ static int rsa_pkey_export_to(const EVP_PKEY *from, void *to_keydata, for (i = 0; i < numcoeffs; i++) { const BIGNUM *num = sk_BIGNUM_const_value(coeffs, i); - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_RSA_COEFFICIENT, + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_RSA_COEFFICIENT, num)) goto err; } } - if ((params = OSSL_PARAM_BLD_to_param(&tmpl)) == NULL) + if ((params = OSSL_PARAM_BLD_to_param(tmpl)) == NULL) goto err; /* We export, the provider imports */ @@ -1183,7 +1177,8 @@ static int rsa_pkey_export_to(const EVP_PKEY *from, void *to_keydata, sk_BIGNUM_const_free(primes); sk_BIGNUM_const_free(exps); sk_BIGNUM_const_free(coeffs); - OSSL_PARAM_BLD_free(params); + OSSL_PARAM_BLD_free_params(params); + OSSL_PARAM_BLD_free(tmpl); return rv; } diff --git a/crypto/rsa/rsa_lib.c b/crypto/rsa/rsa_lib.c index e65e303735..e9a5b48fbc 100644 --- a/crypto/rsa/rsa_lib.c +++ b/crypto/rsa/rsa_lib.c @@ -1296,7 +1296,7 @@ int EVP_PKEY_CTX_set_rsa_keygen_bits(EVP_PKEY_CTX *ctx, int bits) int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp) { - OSSL_PARAM_BLD tmpl; + OSSL_PARAM_BLD *tmpl; OSSL_PARAM *params; int ret; @@ -1315,13 +1315,17 @@ int EVP_PKEY_CTX_set_rsa_keygen_pubexp(EVP_PKEY_CTX *ctx, BIGNUM *pubexp) return EVP_PKEY_CTX_ctrl(ctx, EVP_PKEY_RSA, EVP_PKEY_OP_KEYGEN, EVP_PKEY_CTRL_RSA_KEYGEN_PUBEXP, 0, pubexp); - OSSL_PARAM_BLD_init(&tmpl); - if (!OSSL_PARAM_BLD_push_BN(&tmpl, OSSL_PKEY_PARAM_RSA_E, pubexp) - || (params = OSSL_PARAM_BLD_to_param(&tmpl)) == NULL) + if ((tmpl = OSSL_PARAM_BLD_new()) == NULL) return 0; + if (!OSSL_PARAM_BLD_push_BN(tmpl, OSSL_PKEY_PARAM_RSA_E, pubexp) + || (params = OSSL_PARAM_BLD_to_param(tmpl)) == NULL) { + OSSL_PARAM_BLD_free(tmpl); + return 0; + } + OSSL_PARAM_BLD_free(tmpl); ret = EVP_PKEY_CTX_set_params(ctx, params); - OSSL_PARAM_BLD_free(params); + OSSL_PARAM_BLD_free_params(params); return ret; } -- cgit v1.2.3