From 695d195bbb81f8ed4027468fe1c480f958e846c8 Mon Sep 17 00:00:00 2001 From: Richard Levitte Date: Thu, 23 May 2019 03:36:21 +0200 Subject: [PATCH] Replumbing: make it possible for providers to specify multiple names This modifies the treatment of algorithm name strings to allow multiple names separated with colons. Reviewed-by: Tomas Mraz (Merged from https://github.com/openssl/openssl/pull/8985) --- crypto/core_algorithm.c | 2 +- crypto/core_fetch.c | 6 +- crypto/core_namemap.c | 32 ++++++-- crypto/err/openssl.txt | 2 + crypto/evp/evp_err.c | 3 + crypto/evp/evp_fetch.c | 102 +++++++++++++++++++++---- doc/internal/man3/ossl_namemap_new.pod | 23 ++++-- include/internal/namemap.h | 4 + include/openssl/core.h | 6 +- include/openssl/evperr.h | 2 + 10 files changed, 150 insertions(+), 32 deletions(-) diff --git a/crypto/core_algorithm.c b/crypto/core_algorithm.c index f88a0458ec..2973b37604 100644 --- a/crypto/core_algorithm.c +++ b/crypto/core_algorithm.c @@ -44,7 +44,7 @@ static int algorithm_do_this(OSSL_PROVIDER *provider, void *cbdata) break; ok = 1; /* As long as we've found *something* */ - while (map->algorithm_name != NULL) { + while (map->algorithm_names != NULL) { const OSSL_ALGORITHM *thismap = map++; data->fn(provider, thismap, no_store, data->data); diff --git a/crypto/core_fetch.c b/crypto/core_fetch.c index 1e0d82fb61..ed50bb87d5 100644 --- a/crypto/core_fetch.c +++ b/crypto/core_fetch.c @@ -31,7 +31,7 @@ static void ossl_method_construct_this(OSSL_PROVIDER *provider, struct construct_data_st *data = cbdata; void *method = NULL; - if ((method = data->mcm->construct(algo->algorithm_name, + if ((method = data->mcm->construct(algo->algorithm_names, algo->implementation, provider, data->mcm_data)) == NULL) return; @@ -53,12 +53,12 @@ static void ossl_method_construct_this(OSSL_PROVIDER *provider, * add to the global store */ data->mcm->put(data->libctx, NULL, method, provider, - data->operation_id, algo->algorithm_name, + data->operation_id, algo->algorithm_names, algo->property_definition, data->mcm_data); } data->mcm->put(data->libctx, data->store, method, provider, - data->operation_id, algo->algorithm_name, + data->operation_id, algo->algorithm_names, algo->property_definition, data->mcm_data); /* refcnt-- because we're dropping the reference */ diff --git a/crypto/core_namemap.c b/crypto/core_namemap.c index cb26b429b7..71b70ff5aa 100644 --- a/crypto/core_namemap.c +++ b/crypto/core_namemap.c @@ -148,7 +148,8 @@ void ossl_namemap_doall_names(const OSSL_NAMEMAP *namemap, int number, CRYPTO_THREAD_unlock(namemap->lock); } -int ossl_namemap_name2num(const OSSL_NAMEMAP *namemap, const char *name) +int ossl_namemap_name2num_n(const OSSL_NAMEMAP *namemap, + const char *name, size_t name_len) { NAMENUM_ENTRY *namenum_entry, namenum_tmpl; int number = 0; @@ -161,7 +162,8 @@ int ossl_namemap_name2num(const OSSL_NAMEMAP *namemap, const char *name) if (namemap == NULL) return 0; - namenum_tmpl.name = (char *)name; + if ((namenum_tmpl.name = OPENSSL_strndup(name, name_len)) == NULL) + return 0; namenum_tmpl.number = 0; CRYPTO_THREAD_read_lock(namemap->lock); namenum_entry = @@ -169,10 +171,19 @@ int ossl_namemap_name2num(const OSSL_NAMEMAP *namemap, const char *name) if (namenum_entry != NULL) number = namenum_entry->number; CRYPTO_THREAD_unlock(namemap->lock); + OPENSSL_free(namenum_tmpl.name); return number; } +int ossl_namemap_name2num(const OSSL_NAMEMAP *namemap, const char *name) +{ + if (name == NULL) + return 0; + + return ossl_namemap_name2num_n(namemap, name, strlen(name)); +} + struct num2name_data_st { size_t idx; /* Countdown */ const char *name; /* Result */ @@ -199,7 +210,8 @@ const char *ossl_namemap_num2name(const OSSL_NAMEMAP *namemap, int number, return data.name; } -int ossl_namemap_add(OSSL_NAMEMAP *namemap, int number, const char *name) +int ossl_namemap_add_n(OSSL_NAMEMAP *namemap, int number, + const char *name, size_t name_len) { NAMENUM_ENTRY *namenum = NULL; int tmp_number; @@ -209,16 +221,16 @@ int ossl_namemap_add(OSSL_NAMEMAP *namemap, int number, const char *name) namemap = ossl_namemap_stored(NULL); #endif - if (name == NULL || namemap == NULL) + if (name == NULL || name_len == 0 || namemap == NULL) return 0; - if ((tmp_number = ossl_namemap_name2num(namemap, name)) != 0) + if ((tmp_number = ossl_namemap_name2num_n(namemap, name, name_len)) != 0) return tmp_number; /* Pretend success */ CRYPTO_THREAD_write_lock(namemap->lock); if ((namenum = OPENSSL_zalloc(sizeof(*namenum))) == NULL - || (namenum->name = OPENSSL_strdup(name)) == NULL) + || (namenum->name = OPENSSL_strndup(name, name_len)) == NULL) goto err; namenum->number = tmp_number = @@ -238,3 +250,11 @@ int ossl_namemap_add(OSSL_NAMEMAP *namemap, int number, const char *name) CRYPTO_THREAD_unlock(namemap->lock); return 0; } + +int ossl_namemap_add(OSSL_NAMEMAP *namemap, int number, const char *name) +{ + if (name == NULL) + return 0; + + return ossl_namemap_add_n(namemap, number, name, strlen(name)); +} diff --git a/crypto/err/openssl.txt b/crypto/err/openssl.txt index 520ac1f9a4..973b85c969 100644 --- a/crypto/err/openssl.txt +++ b/crypto/err/openssl.txt @@ -2394,6 +2394,7 @@ ESS_R_ESS_SIGNING_CERT_ADD_ERROR:100:ess signing cert add error ESS_R_ESS_SIGNING_CERT_V2_ADD_ERROR:101:ess signing cert v2 add error EVP_R_AES_KEY_SETUP_FAILED:143:aes key setup failed EVP_R_ARIA_KEY_SETUP_FAILED:176:aria key setup failed +EVP_R_BAD_ALGORITHM_NAME:200:bad algorithm name EVP_R_BAD_DECRYPT:100:bad decrypt EVP_R_BAD_KEY_LENGTH:195:bad key length EVP_R_BUFFER_TOO_SMALL:155:buffer too small @@ -2403,6 +2404,7 @@ EVP_R_CANNOT_SET_PARAMETERS:198:cannot set parameters EVP_R_CIPHER_NOT_GCM_MODE:184:cipher not gcm mode EVP_R_CIPHER_PARAMETER_ERROR:122:cipher parameter error EVP_R_COMMAND_NOT_SUPPORTED:147:command not supported +EVP_R_CONFLICTING_ALGORITHM_NAME:201:conflicting algorithm name EVP_R_COPY_ERROR:173:copy error EVP_R_CTRL_NOT_IMPLEMENTED:132:ctrl not implemented EVP_R_CTRL_OPERATION_NOT_IMPLEMENTED:133:ctrl operation not implemented diff --git a/crypto/evp/evp_err.c b/crypto/evp/evp_err.c index b74b87e4f5..62ca87c683 100644 --- a/crypto/evp/evp_err.c +++ b/crypto/evp/evp_err.c @@ -18,6 +18,7 @@ static const ERR_STRING_DATA EVP_str_reasons[] = { "aes key setup failed"}, {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_ARIA_KEY_SETUP_FAILED), "aria key setup failed"}, + {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_BAD_ALGORITHM_NAME), "bad algorithm name"}, {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_BAD_DECRYPT), "bad decrypt"}, {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_BAD_KEY_LENGTH), "bad key length"}, {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_BUFFER_TOO_SMALL), "buffer too small"}, @@ -33,6 +34,8 @@ static const ERR_STRING_DATA EVP_str_reasons[] = { "cipher parameter error"}, {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_COMMAND_NOT_SUPPORTED), "command not supported"}, + {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_CONFLICTING_ALGORITHM_NAME), + "conflicting algorithm name"}, {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_COPY_ERROR), "copy error"}, {ERR_PACK(ERR_LIB_EVP, 0, EVP_R_CTRL_NOT_IMPLEMENTED), "ctrl not implemented"}, diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c index e15b8e26be..2404dfca30 100644 --- a/crypto/evp/evp_fetch.c +++ b/crypto/evp/evp_fetch.c @@ -20,6 +20,8 @@ #include "crypto/evp.h" /* evp_local.h needs it */ #include "evp_local.h" +#define NAME_SEPARATOR ':' + static void default_method_store_free(void *vstore) { ossl_method_store_free(vstore); @@ -42,7 +44,7 @@ struct method_data_st { OSSL_METHOD_CONSTRUCT_METHOD *mcm; int operation_id; /* For get_method_from_store() */ int name_id; /* For get_method_from_store() */ - const char *name; /* For get_method_from_store() */ + const char *names; /* For get_method_from_store() */ const char *propquery; /* For get_method_from_store() */ void *(*method_from_dispatch)(int name_id, const OSSL_DISPATCH *, OSSL_PROVIDER *, void *); @@ -51,6 +53,69 @@ struct method_data_st { void (*destruct_method)(void *method); }; +static int add_names_to_namemap(OSSL_NAMEMAP *namemap, + const char *names) +{ + const char *p, *q; + size_t l; + int id = 0; + + /* Check that we have a namemap and that there is at least one name */ + if (namemap == NULL) { + ERR_raise(ERR_LIB_EVP, ERR_R_PASSED_NULL_PARAMETER); + return 0; + } + + /* + * Check that no name is an empty string, and that all names have at + * most one numeric identity together. + */ + for (p = names; *p != '\0'; p = (q == NULL ? p + l : q + 1)) { + int this_id; + + if ((q = strchr(p, NAME_SEPARATOR)) == NULL) + l = strlen(p); /* offset to \0 */ + else + l = q - p; /* offset to the next separator */ + + this_id = ossl_namemap_name2num_n(namemap, p, l); + + if (*p == '\0' || *p == NAME_SEPARATOR) { + ERR_raise(ERR_LIB_EVP, EVP_R_BAD_ALGORITHM_NAME); + return 0; + } + if (id == 0) + id = this_id; + else if (this_id != 0 && this_id != id) { + ERR_raise_data(ERR_LIB_EVP, EVP_R_CONFLICTING_ALGORITHM_NAME, + "\"%.*s\" has an existing different identity %d (from \"%s\")", + l, p, this_id, names); + return 0; + } + } + + /* Now that we have checked, register all names */ + for (p = names; *p != '\0'; p = (q == NULL ? p + l : q + 1)) { + int this_id; + + if ((q = strchr(p, NAME_SEPARATOR)) == NULL) + l = strlen(p); /* offset to \0 */ + else + l = q - p; /* offset to the next separator */ + + this_id = ossl_namemap_add_n(namemap, id, p, l); + if (id == 0) + id = this_id; + else if (this_id != id) { + ERR_raise_data(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR, + "Got id %d when expecting %d", this_id, id); + return 0; + } + } + + return id; +} + /* * Generic routines to fetch / create EVP methods with ossl_method_construct() */ @@ -105,10 +170,13 @@ static void *get_method_from_store(OPENSSL_CTX *libctx, void *store, */ if ((name_id = methdata->name_id) == 0) { OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx); + const char *names = methdata->names; + const char *q = strchr(names, NAME_SEPARATOR); + size_t l = (q == NULL ? strlen(names) : (size_t)(q - names)); if (namemap == 0) return NULL; - name_id = ossl_namemap_name2num(namemap, methdata->name); + name_id = ossl_namemap_name2num_n(namemap, names, l); } if (name_id == 0 @@ -131,21 +199,30 @@ static void *get_method_from_store(OPENSSL_CTX *libctx, void *store, static int put_method_in_store(OPENSSL_CTX *libctx, void *store, void *method, const OSSL_PROVIDER *prov, - int operation_id, const char *name, + int operation_id, const char *names, const char *propdef, void *data) { struct method_data_st *methdata = data; OSSL_NAMEMAP *namemap; int name_id; uint32_t meth_id; + size_t l = 0; /* * put_method_in_store() is only called with a method that was * successfully created by construct_method() below, which means - * the name should already be stored in the namemap, so just use it. + * that all the names should already be stored in the namemap with + * the same numeric identity, so just use the first to get that + * identity. */ + if (names != NULL) { + const char *q = strchr(names, NAME_SEPARATOR); + + l = (q == NULL ? strlen(names) : (size_t)(q - names)); + } + if ((namemap = ossl_namemap_stored(libctx)) == NULL - || (name_id = ossl_namemap_name2num(namemap, name)) == 0 + || (name_id = ossl_namemap_name2num_n(namemap, names, l)) == 0 || (meth_id = method_id(operation_id, name_id)) == 0) return 0; @@ -162,7 +239,7 @@ static int put_method_in_store(OPENSSL_CTX *libctx, void *store, * The core fetching functionality passes the name of the implementation. * This function is responsible to getting an identity number for it. */ -static void *construct_method(const char *name, const OSSL_DISPATCH *fns, +static void *construct_method(const char *names, const OSSL_DISPATCH *fns, OSSL_PROVIDER *prov, void *data) { /* @@ -170,17 +247,14 @@ static void *construct_method(const char *name, const OSSL_DISPATCH *fns, * NULL, so it's safe to say that of all the spots to create a new * namemap entry, this is it. Should the name already exist there, we * know that ossl_namemap_add() will return its corresponding number. - * - * TODO(3.0): If this function gets an array of names instead of just - * one, we need to check through all the names to see if at least one - * of them has an associated number, and use that. If several names - * have associated numbers that differ from each other, it's an error. */ struct method_data_st *methdata = data; OPENSSL_CTX *libctx = ossl_provider_library_context(prov); OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx); - int name_id = ossl_namemap_add(namemap, 0, name); + int name_id = add_names_to_namemap(namemap, names); + if (name_id == 0) + return NULL; return methdata->method_from_dispatch(name_id, fns, prov, methdata->method_data); } @@ -255,7 +329,7 @@ static void *inner_generic_fetch(OPENSSL_CTX *libctx, int operation_id, mcmdata.libctx = libctx; mcmdata.operation_id = operation_id; mcmdata.name_id = name_id; - mcmdata.name = name; + mcmdata.names = name; mcmdata.propquery = properties; mcmdata.method_from_dispatch = new_method; mcmdata.destruct_method = free_method; @@ -346,7 +420,7 @@ static void do_one(OSSL_PROVIDER *provider, const OSSL_ALGORITHM *algo, struct do_all_data_st *data = vdata; OPENSSL_CTX *libctx = ossl_provider_library_context(provider); OSSL_NAMEMAP *namemap = ossl_namemap_stored(libctx); - int name_id = ossl_namemap_add(namemap, 0, algo->algorithm_name); + int name_id = add_names_to_namemap(namemap, algo->algorithm_names); void *method = NULL; if (name_id != 0) diff --git a/doc/internal/man3/ossl_namemap_new.pod b/doc/internal/man3/ossl_namemap_new.pod index b82d47a5bf..2bcf21386d 100644 --- a/doc/internal/man3/ossl_namemap_new.pod +++ b/doc/internal/man3/ossl_namemap_new.pod @@ -3,7 +3,9 @@ =head1 NAME ossl_namemap_new, ossl_namemap_free, ossl_namemap_stored, -ossl_namemap_add, ossl_namemap_name2num, ossl_namemap_doall_names +ossl_namemap_add, ossl_namemap_add_n, +ossl_namemap_name2num, ossl_namemap_name2num_n, +ossl_namemap_doall_names - internal number E-E name map =head1 SYNOPSIS @@ -16,8 +18,12 @@ ossl_namemap_add, ossl_namemap_name2num, ossl_namemap_doall_names void ossl_namemap_free(OSSL_NAMEMAP *namemap); int ossl_namemap_add(OSSL_NAMEMAP *namemap, int number, const char *name); + int ossl_namemap_add_n(OSSL_NAMEMAP *namemap, int number, + const char *name, size_t name_len); int ossl_namemap_name2num(const OSSL_NAMEMAP *namemap, const char *name); + int ossl_namemap_name2num_n(const OSSL_NAMEMAP *namemap, + const char *name, size_t name_len); void ossl_namemap_doall_names(const OSSL_NAMEMAP *namemap, int number, void (*fn)(const char *name, void *data), void *data); @@ -49,6 +55,12 @@ names already associated with that number. ossl_namemap_name2num() finds the number corresponding to the given I. +ossl_namemap_add_n() and ossl_namemap_name2num_n() do the same thing +as ossl_namemap_add() and ossl_namemap_name2num(), but take a string +length I as well, allowing the caller to use a fragment of +a string as a name. + + ossl_namemap_doall_names() walks through all names associated with I in the given I and calls the function I for each of them. @@ -60,15 +72,16 @@ pass extra data for that function to use. ossl_namemap_new() and ossl_namemap_stored() return the pointer to a B, or NULL on error. -ossl_namemap_add() returns the number associated with the added -string, or zero on error. +ossl_namemap_add() and ossl_namemap_add_n() return the number associated +with the added string, or zero on error. ossl_namemap_num2names() returns a pointer to a NULL-terminated list of pointers to the names corresponding to the given number, or NULL if it's undefined in the given B. -ossl_namemap_name2num() returns the number corresponding to the given -name, or 0 if it's undefined in the given B. +ossl_namemap_name2num() and ossl_namemap_name2num_n() return the number +corresponding to the given name, or 0 if it's undefined in the given +B. =head1 NOTES diff --git a/include/internal/namemap.h b/include/internal/namemap.h index ee69388f11..73163a42cb 100644 --- a/include/internal/namemap.h +++ b/include/internal/namemap.h @@ -17,6 +17,8 @@ OSSL_NAMEMAP *ossl_namemap_new(void); void ossl_namemap_free(OSSL_NAMEMAP *namemap); int ossl_namemap_add(OSSL_NAMEMAP *namemap, int number, const char *name); +int ossl_namemap_add_n(OSSL_NAMEMAP *namemap, int number, + const char *name, size_t name_len); /* * The number<->name relationship is 1<->many @@ -24,6 +26,8 @@ int ossl_namemap_add(OSSL_NAMEMAP *namemap, int number, const char *name); * number->name mapping is an iterator. */ int ossl_namemap_name2num(const OSSL_NAMEMAP *namemap, const char *name); +int ossl_namemap_name2num_n(const OSSL_NAMEMAP *namemap, + const char *name, size_t name_len); const char *ossl_namemap_num2name(const OSSL_NAMEMAP *namemap, int number, size_t idx); void ossl_namemap_doall_names(const OSSL_NAMEMAP *namemap, int number, diff --git a/include/openssl/core.h b/include/openssl/core.h index 2377169a33..74a3bfb91e 100644 --- a/include/openssl/core.h +++ b/include/openssl/core.h @@ -55,13 +55,13 @@ struct ossl_item_st { }; /* - * Type to tie together algorithm name, property definition string and + * Type to tie together algorithm names, property definition string and * the algorithm implementation in the form of a dispatch table. * - * An array of these is always terminated by algorithm_name == NULL + * An array of these is always terminated by algorithm_names == NULL */ struct ossl_algorithm_st { - const char *algorithm_name; /* key */ + const char *algorithm_names; /* key */ const char *property_definition; /* key */ const OSSL_DISPATCH *implementation; }; diff --git a/include/openssl/evperr.h b/include/openssl/evperr.h index fefbfb593e..7da9083ae1 100644 --- a/include/openssl/evperr.h +++ b/include/openssl/evperr.h @@ -167,6 +167,7 @@ int ERR_load_EVP_strings(void); */ # define EVP_R_AES_KEY_SETUP_FAILED 143 # define EVP_R_ARIA_KEY_SETUP_FAILED 176 +# define EVP_R_BAD_ALGORITHM_NAME 200 # define EVP_R_BAD_DECRYPT 100 # define EVP_R_BAD_KEY_LENGTH 195 # define EVP_R_BUFFER_TOO_SMALL 155 @@ -176,6 +177,7 @@ int ERR_load_EVP_strings(void); # define EVP_R_CIPHER_NOT_GCM_MODE 184 # define EVP_R_CIPHER_PARAMETER_ERROR 122 # define EVP_R_COMMAND_NOT_SUPPORTED 147 +# define EVP_R_CONFLICTING_ALGORITHM_NAME 201 # define EVP_R_COPY_ERROR 173 # define EVP_R_CTRL_NOT_IMPLEMENTED 132 # define EVP_R_CTRL_OPERATION_NOT_IMPLEMENTED 133 -- 2.25.1