mirror/openssl
2
0
Fork 0
mirror of https://github.com/openssl/openssl.git synced 2025-03-19 19:50:42 +08:00
Code Issues Packages Projects Releases Wiki Activity

Refactor how KEYMGMT methods get associated with other methods

Browse Source 
KEYMGMT methods were attached to other methods after those were fully
created and registered, thereby creating a potential data race, if two
threads tried to create the exact same method at the same time.

Instead of this, we change the method creating function to take an
extra data parameter, passed all the way from the public fetching
function.  In the case of EVP_KEYEXCH, we pass all the necessary data
that evp_keyexch_from_dispatch() needs to be able to fetch the
appropriate KEYMGMT method on the fly.

Fixes 

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/9678)
This commit is contained in:
Richard Levitte 2019-08-23 14:03:28 +02:00
parent 7964e3709a
commit 3ca9d210c9
11 changed files with 86 additions and 55 deletions

1
crypto/err/openssl.txt

@ -2484,6 +2484,7 @@ EVP_R_NOT_XOF_OR_INVALID_LENGTH:178:not XOF or invalid length
EVP_R_NO_CIPHER_SET:131:no cipher set
EVP_R_NO_DEFAULT_DIGEST:158:no default digest
EVP_R_NO_DIGEST_SET:139:no digest set
EVP_R_NO_KEYMGMT_AVAILABLE:199:no keymgmt available
EVP_R_NO_KEYMGMT_PRESENT:196:no keymgmt present
EVP_R_NO_KEY_SET:154:no key set
EVP_R_NO_OPERATION_SET:149:no operation set

6
crypto/evp/digest.c

@ -617,7 +617,7 @@ int EVP_MD_CTX_ctrl(EVP_MD_CTX *ctx, int cmd, int p1, void *p2)
}
static void *evp_md_from_dispatch(const char *name, const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov)
OSSL_PROVIDER *prov, void *unused)
{
EVP_MD *md = NULL;
int fncnt = 0;
@ -744,7 +744,7 @@ EVP_MD *EVP_MD_fetch(OPENSSL_CTX *ctx, const char *algorithm,
{
EVP_MD *md =
evp_generic_fetch(ctx, OSSL_OP_DIGEST, algorithm, properties,
evp_md_from_dispatch, evp_md_up_ref,
evp_md_from_dispatch, NULL, evp_md_up_ref,
evp_md_free);
return md;
@ -756,5 +756,5 @@ void EVP_MD_do_all_ex(OPENSSL_CTX *libctx,
{
evp_generic_do_all(libctx, OSSL_OP_DIGEST,
(void (*)(void *, void *))fn, arg,
evp_md_from_dispatch, evp_md_free);
evp_md_from_dispatch, NULL, evp_md_free);
}

7
crypto/evp/evp_enc.c

@ -1246,7 +1246,8 @@ int EVP_CIPHER_CTX_copy(EVP_CIPHER_CTX *out, const EVP_CIPHER_CTX *in)
static void *evp_cipher_from_dispatch(const char *name,
const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov)
OSSL_PROVIDER *prov,
void *unused)
{
EVP_CIPHER *cipher = NULL;
int fnciphcnt = 0, fnctxcnt = 0;
@ -1386,7 +1387,7 @@ EVP_CIPHER *EVP_CIPHER_fetch(OPENSSL_CTX *ctx, const char *algorithm,
{
EVP_CIPHER *cipher =
evp_generic_fetch(ctx, OSSL_OP_CIPHER, algorithm, properties,
evp_cipher_from_dispatch, evp_cipher_up_ref,
evp_cipher_from_dispatch, NULL, evp_cipher_up_ref,
evp_cipher_free);
return cipher;
@ -1398,5 +1399,5 @@ void EVP_CIPHER_do_all_ex(OPENSSL_CTX *libctx,
{
evp_generic_do_all(libctx, OSSL_OP_CIPHER,
(void (*)(void *, void *))fn, arg,
evp_cipher_from_dispatch, evp_cipher_free);
evp_cipher_from_dispatch, NULL, evp_cipher_free);
}

2
crypto/evp/evp_err.c

@ -99,6 +99,8 @@ static const ERR_STRING_DATA EVP_str_reasons[] = {
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_CIPHER_SET), "no cipher set"},
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_DEFAULT_DIGEST), "no default digest"},
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_DIGEST_SET), "no digest set"},
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_KEYMGMT_AVAILABLE),
"no keymgmt available"},
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_KEYMGMT_PRESENT), "no keymgmt present"},
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_KEY_SET), "no key set"},
{ERR_PACK(ERR_LIB_EVP, 0, EVP_R_NO_OPERATION_SET), "no operation set"},

21
crypto/evp/evp_fetch.c

@ -41,7 +41,8 @@ struct method_data_st {
const char *name;
OSSL_METHOD_CONSTRUCT_METHOD *mcm;
void *(*method_from_dispatch)(const char *, const OSSL_DISPATCH *,
OSSL_PROVIDER *);
OSSL_PROVIDER *, void *);
void *method_data;
int (*refcnt_up_method)(void *method);
void (*destruct_method)(void *method);
};
@ -142,7 +143,8 @@ static void *construct_method(const char *name, const OSSL_DISPATCH *fns,
{
struct method_data_st *methdata = data;
return methdata->method_from_dispatch(name, fns, prov);
return methdata->method_from_dispatch(name, fns, prov,
methdata->method_data);
}
static void destruct_method(void *method, void *data)
@ -156,7 +158,9 @@ void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
const char *name, const char *properties,
void *(*new_method)(const char *name,
const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov),
OSSL_PROVIDER *prov,
void *method_data),
void *method_data,
int (*up_ref_method)(void *),
void (*free_method)(void *))
{
@ -205,6 +209,7 @@ void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
mcmdata.destruct_method = free_method;
mcmdata.refcnt_up_method = up_ref_method;
mcmdata.destruct_method = free_method;
mcmdata.method_data = method_data;
if ((method = ossl_method_construct(libctx, operation_id, name,
properties, 0 /* !force_cache */,
&mcm, &mcmdata)) != NULL) {
@ -239,7 +244,7 @@ struct do_all_data_st {
void (*user_fn)(void *method, void *arg);
void *user_arg;
void *(*new_method)(const char *name, const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov);
OSSL_PROVIDER *prov, void *method_data);
void (*free_method)(void *);
};
@ -248,7 +253,7 @@ static void do_one(OSSL_PROVIDER *provider, const OSSL_ALGORITHM *algo,
{
struct do_all_data_st *data = vdata;
void *method = data->new_method(algo->algorithm_name,
algo->implementation, provider);
algo->implementation, provider, NULL);
if (method != NULL) {
data->user_fn(method, data->user_arg);
@ -261,7 +266,9 @@ void evp_generic_do_all(OPENSSL_CTX *libctx, int operation_id,
void *user_arg,
void *(*new_method)(const char *name,
const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov),
OSSL_PROVIDER *prov,
void *method_data),
void *method_data,
void (*free_method)(void *))
{
struct do_all_data_st data;
@ -270,5 +277,5 @@ void evp_generic_do_all(OPENSSL_CTX *libctx, int operation_id,
data.free_method = free_method;
data.user_fn = user_fn;
data.user_arg = user_arg;
ossl_algorithm_do_all(libctx, operation_id, NULL, do_one, &data);
ossl_algorithm_do_all(libctx, operation_id, method_data, do_one, &data);
}

8
crypto/evp/evp_locl.h

@ -141,7 +141,9 @@ void *evp_generic_fetch(OPENSSL_CTX *ctx, int operation_id,
const char *algorithm, const char *properties,
void *(*new_method)(const char *name,
const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov),
OSSL_PROVIDER *prov,
void *method_data),
void *method_data,
int (*up_ref_method)(void *),
void (*free_method)(void *));
void evp_generic_do_all(OPENSSL_CTX *libctx, int operation_id,
@ -149,7 +151,9 @@ void evp_generic_do_all(OPENSSL_CTX *libctx, int operation_id,
void *user_arg,
void *(*new_method)(const char *name,
const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov),
OSSL_PROVIDER *prov,
void *method_data),
void *method_data,
void (*free_method)(void *));
/* Helper functions to avoid duplicating code */

72
crypto/evp/exchange.c

@ -32,20 +32,45 @@ static EVP_KEYEXCH *evp_keyexch_new(OSSL_PROVIDER *prov)
return exchange;
}
struct keymgmt_data_st {
OPENSSL_CTX *ctx;
const char *properties;
};
static void *evp_keyexch_from_dispatch(const char *name,
const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov)
OSSL_PROVIDER *prov,
void *vkeymgmt_data)
{
/*
* Key exchange cannot work without a key, and key management
* from the same provider to manage its keys. We therefore fetch
* a key management method using the same algorithm and properties
* and pass that down to evp_generic_fetch to be passed on to our
* evp_keyexch_from_dispatch, which will attach the key management
* method to the newly created key exchange method as long as the
* provider matches.
*/
struct keymgmt_data_st *keymgmt_data = vkeymgmt_data;
EVP_KEYMGMT *keymgmt = EVP_KEYMGMT_fetch(keymgmt_data->ctx, name,
keymgmt_data->properties);
EVP_KEYEXCH *exchange = NULL;
int fncnt = 0;
if (keymgmt == NULL || EVP_KEYMGMT_provider(keymgmt) != prov) {
ERR_raise(ERR_LIB_EVP, EVP_R_NO_KEYMGMT_AVAILABLE);
goto err;
}
if ((exchange = evp_keyexch_new(prov)) == NULL
|| (exchange->name = OPENSSL_strdup(name)) == NULL) {
EVP_KEYEXCH_free(exchange);
EVPerr(0, ERR_R_MALLOC_FAILURE);
return NULL;
ERR_raise(ERR_LIB_EVP, ERR_R_MALLOC_FAILURE);
goto err;
}
exchange->keymgmt = keymgmt;
keymgmt = NULL; /* avoid double free on failure below */
for (; fns->function_id != 0; fns++) {
switch (fns->function_id) {
case OSSL_FUNC_KEYEXCH_NEWCTX:
@ -96,13 +121,17 @@ static void *evp_keyexch_from_dispatch(const char *name,
* and freectx. The dupctx, set_peer and set_params functions are
* optional.
*/
EVP_KEYEXCH_free(exchange);
EVPerr(EVP_F_EVP_KEYEXCH_FROM_DISPATCH,
EVP_R_INVALID_PROVIDER_FUNCTIONS);
return NULL;
goto err;
}
return exchange;
err:
EVP_KEYEXCH_free(exchange);
EVP_KEYMGMT_free(keymgmt);
return NULL;
}
void EVP_KEYEXCH_free(EVP_KEYEXCH *exchange)
@ -137,31 +166,16 @@ OSSL_PROVIDER *EVP_KEYEXCH_provider(const EVP_KEYEXCH *exchange)
EVP_KEYEXCH *EVP_KEYEXCH_fetch(OPENSSL_CTX *ctx, const char *algorithm,
const char *properties)
{
/*
* Key exchange cannot work without a key, and we key management
* from the same provider to manage its keys.
*/
EVP_KEYEXCH *keyexch =
evp_generic_fetch(ctx, OSSL_OP_KEYEXCH, algorithm, properties,
evp_keyexch_from_dispatch,
(int (*)(void *))EVP_KEYEXCH_up_ref,
(void (*)(void *))EVP_KEYEXCH_free);
EVP_KEYEXCH *keyexch = NULL;
struct keymgmt_data_st keymgmt_data;
/* If the method is newly created, there's no keymgmt attached */
if (keyexch->keymgmt == NULL) {
EVP_KEYMGMT *keymgmt = EVP_KEYMGMT_fetch(ctx, algorithm, properties);
keymgmt_data.ctx = ctx;
keymgmt_data.properties = properties;
keyexch = evp_generic_fetch(ctx, OSSL_OP_KEYEXCH, algorithm, properties,
evp_keyexch_from_dispatch, &keymgmt_data,
(int (*)(void *))EVP_KEYEXCH_up_ref,
(void (*)(void *))EVP_KEYEXCH_free);
if (keymgmt == NULL
|| (EVP_KEYEXCH_provider(keyexch)
!= EVP_KEYMGMT_provider(keymgmt))) {
EVP_KEYEXCH_free(keyexch);
EVP_KEYMGMT_free(keymgmt);
EVPerr(EVP_F_EVP_KEYEXCH_FETCH, EVP_R_NO_KEYMGMT_PRESENT);
return NULL;
}
keyexch->keymgmt = keymgmt;
}
return keyexch;
}

4
crypto/evp/keymgmt_meth.c

@ -34,7 +34,7 @@ static void *keymgmt_new(void)
}
static void *keymgmt_from_dispatch(const char *name, const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov)
OSSL_PROVIDER *prov, void *unused)
{
EVP_KEYMGMT *keymgmt = NULL;
@ -156,7 +156,7 @@ EVP_KEYMGMT *EVP_KEYMGMT_fetch(OPENSSL_CTX *ctx, const char *algorithm,
{
EVP_KEYMGMT *keymgmt =
evp_generic_fetch(ctx, OSSL_OP_KEYMGMT, algorithm, properties,
keymgmt_from_dispatch,
keymgmt_from_dispatch, NULL,
(int (*)(void *))EVP_KEYMGMT_up_ref,
(void (*)(void *))EVP_KEYMGMT_free);

6
crypto/evp/mac_meth.c

@ -48,7 +48,7 @@ static void *evp_mac_new(void)
}
static void *evp_mac_from_dispatch(const char *name, const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov)
OSSL_PROVIDER *prov, void *unused)
{
EVP_MAC *mac = NULL;
int fnmaccnt = 0, fnctxcnt = 0;
@ -154,7 +154,7 @@ EVP_MAC *EVP_MAC_fetch(OPENSSL_CTX *libctx, const char *algorithm,
const char *properties)
{
return evp_generic_fetch(libctx, OSSL_OP_MAC, algorithm, properties,
evp_mac_from_dispatch, evp_mac_up_ref,
evp_mac_from_dispatch, NULL, evp_mac_up_ref,
evp_mac_free);
}
@ -205,5 +205,5 @@ void EVP_MAC_do_all_ex(OPENSSL_CTX *libctx,
{
evp_generic_do_all(libctx, OSSL_OP_MAC,
(void (*)(void *, void *))fn, arg,
evp_mac_from_dispatch, evp_mac_free);
evp_mac_from_dispatch, NULL, evp_mac_free);
}

9
doc/internal/man3/evp_generic_fetch.pod

@ -11,8 +11,11 @@ evp_generic_fetch - generic algorithm fetcher and method creator for EVP
void *evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
const char *name, const char *properties,
void *(*new_method)(const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov),
void *(*new_method)(const char *name,
const OSSL_DISPATCH *fns,
OSSL_PROVIDER *prov,
void *method_data),
void *method_data,
int (*up_ref_method)(void *),
void (*free_method)(void *));
@ -31,6 +34,8 @@ The three functions are supposed to:
creates an internal method from function pointers found in the
dispatch table C<fns>.
The algorithm I<name>, provider I<prov>, and I<method_data> are
also passed to be used as new_method() sees fit.
=item up_ref_method()

5
include/openssl/evperr.h

@ -41,10 +41,6 @@ int ERR_load_EVP_strings(void);
# define EVP_F_ARIA_GCM_INIT_KEY 0
# define EVP_F_ARIA_INIT_KEY 0
# define EVP_F_B64_NEW 0
# define EVP_F_BLAKE2B_MAC_CTRL 0
# define EVP_F_BLAKE2B_MAC_INIT 0
# define EVP_F_BLAKE2S_MAC_CTRL 0
# define EVP_F_BLAKE2S_MAC_INIT 0
# define EVP_F_CAMELLIA_INIT_KEY 0
# define EVP_F_CHACHA20_POLY1305_CTRL 0
# define EVP_F_CMLL_T4_INIT_KEY 0
@ -218,6 +214,7 @@ int ERR_load_EVP_strings(void);
# define EVP_R_NO_CIPHER_SET 131
# define EVP_R_NO_DEFAULT_DIGEST 158
# define EVP_R_NO_DIGEST_SET 139
# define EVP_R_NO_KEYMGMT_AVAILABLE 199
# define EVP_R_NO_KEYMGMT_PRESENT 196
# define EVP_R_NO_KEY_SET 154
# define EVP_R_NO_OPERATION_SET 149