Skip to content

Commit

Permalink
fetch: convert a NULL property query to ""
Browse files Browse the repository at this point in the history 
Previously, a NULL property query was never cached and this lead to a
performance degregation.  Now, such a query is converted to an empty string
and cached.

Fixes openssl#17752
Fixes https://github.openssl.org/openssl/openssl/issues/26

Reviewed-by: Tomas Mraz <[email protected]>
Reviewed-by: Matt Caswell <[email protected]>
(Merged from openssl#17769)
  • Loading branch information
@paulidale
paulidale committed Feb 28, 2022
1 parent 98b7b74 commit af788ad
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 18 deletions.
7 changes: 4 additions & 3 deletions crypto/encode_decode/decoder_meth.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ inner_ossl_decoder_fetch(struct decoder_data_st *methdata, int id,
{
OSSL_METHOD_STORE *store = get_decoder_store(methdata->libctx);
OSSL_NAMEMAP *namemap = ossl_namemap_stored(methdata->libctx);
const char *const propq = properties != NULL ? properties : "";
void *method = NULL;
int unsupported = 0;

Expand Down Expand Up @@ -367,7 +368,7 @@ inner_ossl_decoder_fetch(struct decoder_data_st *methdata, int id,
unsupported = 1;

if (id == 0
|| !ossl_method_store_cache_get(store, NULL, id, properties, &method)) {
|| !ossl_method_store_cache_get(store, NULL, id, propq, &method)) {
OSSL_METHOD_CONSTRUCT_METHOD mcm = {
get_tmp_decoder_store,
get_decoder_from_store,
Expand All @@ -379,7 +380,7 @@ inner_ossl_decoder_fetch(struct decoder_data_st *methdata, int id,

methdata->id = id;
methdata->names = name;
methdata->propquery = properties;
methdata->propquery = propq;
methdata->flag_construct_error_occurred = 0;
if ((method = ossl_method_construct(methdata->libctx, OSSL_OP_DECODER,
&prov, 0 /* !force_cache */,
Expand All @@ -393,7 +394,7 @@ inner_ossl_decoder_fetch(struct decoder_data_st *methdata, int id,
if (id == 0 && name != NULL)
id = ossl_namemap_name2num(namemap, name);
if (id != 0)
ossl_method_store_cache_set(store, prov, id, properties, method,
ossl_method_store_cache_set(store, prov, id, propq, method,
up_ref_decoder, free_decoder);
}

Expand Down
7 changes: 4 additions & 3 deletions crypto/encode_decode/encoder_meth.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ inner_ossl_encoder_fetch(struct encoder_data_st *methdata, int id,
{
OSSL_METHOD_STORE *store = get_encoder_store(methdata->libctx);
OSSL_NAMEMAP *namemap = ossl_namemap_stored(methdata->libctx);
const char *const propq = properties != NULL ? properties : "";
void *method = NULL;
int unsupported = 0;

Expand Down Expand Up @@ -377,7 +378,7 @@ inner_ossl_encoder_fetch(struct encoder_data_st *methdata, int id,
unsupported = 1;

if (id == 0
|| !ossl_method_store_cache_get(store, NULL, id, properties, &method)) {
|| !ossl_method_store_cache_get(store, NULL, id, propq, &method)) {
OSSL_METHOD_CONSTRUCT_METHOD mcm = {
get_tmp_encoder_store,
get_encoder_from_store,
Expand All @@ -389,7 +390,7 @@ inner_ossl_encoder_fetch(struct encoder_data_st *methdata, int id,

methdata->id = id;
methdata->names = name;
methdata->propquery = properties;
methdata->propquery = propq;
methdata->flag_construct_error_occurred = 0;
if ((method = ossl_method_construct(methdata->libctx, OSSL_OP_ENCODER,
&prov, 0 /* !force_cache */,
Expand All @@ -402,7 +403,7 @@ inner_ossl_encoder_fetch(struct encoder_data_st *methdata, int id,
*/
if (id == 0)
id = ossl_namemap_name2num(namemap, name);
ossl_method_store_cache_set(store, prov, id, properties, method,
ossl_method_store_cache_set(store, prov, id, propq, method,
up_ref_encoder, free_encoder);
}

Expand Down
8 changes: 4 additions & 4 deletions crypto/evp/evp_fetch.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,6 +247,7 @@ inner_evp_generic_fetch(struct evp_method_data_st *methdata,
{
OSSL_METHOD_STORE *store = get_evp_method_store(methdata->libctx);
OSSL_NAMEMAP *namemap = ossl_namemap_stored(methdata->libctx);
const char *const propq = properties != NULL ? properties : "";
uint32_t meth_id = 0;
void *method = NULL;
int unsupported = 0;
Expand Down Expand Up @@ -299,8 +300,7 @@ inner_evp_generic_fetch(struct evp_method_data_st *methdata,
unsupported = 1;

if (meth_id == 0
|| !ossl_method_store_cache_get(store, prov, meth_id, properties,
&method)) {
|| !ossl_method_store_cache_get(store, prov, meth_id, propq, &method)) {
OSSL_METHOD_CONSTRUCT_METHOD mcm = {
get_tmp_evp_method_store,
get_evp_method_from_store,
Expand All @@ -312,7 +312,7 @@ inner_evp_generic_fetch(struct evp_method_data_st *methdata,
methdata->operation_id = operation_id;
methdata->name_id = name_id;
methdata->names = name;
methdata->propquery = properties;
methdata->propquery = propq;
methdata->method_from_algorithm = new_method;
methdata->refcnt_up_method = up_ref_method;
methdata->destruct_method = free_method;
Expand All @@ -330,7 +330,7 @@ inner_evp_generic_fetch(struct evp_method_data_st *methdata,
name_id = ossl_namemap_name2num(namemap, name);
meth_id = evp_method_id(name_id, operation_id);
if (name_id != 0)
ossl_method_store_cache_set(store, prov, meth_id, properties,
ossl_method_store_cache_set(store, prov, meth_id, propq,
method, up_ref_method, free_method);
}

Expand Down
8 changes: 3 additions & 5 deletions crypto/property/property.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,7 @@ int ossl_method_store_cache_get(OSSL_METHOD_STORE *store, OSSL_PROVIDER *prov,
QUERY elem, *r;
int res = 0;

if (nid <= 0 || store == NULL)
if (nid <= 0 || store == NULL || prop_query == NULL)
return 0;

if (!ossl_property_read_lock(store))
Expand All @@ -605,7 +605,7 @@ int ossl_method_store_cache_get(OSSL_METHOD_STORE *store, OSSL_PROVIDER *prov,
if (alg == NULL)
goto err;

elem.query = prop_query != NULL ? prop_query : "";
elem.query = prop_query;
elem.provider = prov;
r = lh_QUERY_retrieve(alg->cache, &elem);
if (r == NULL)
Expand All @@ -629,10 +629,8 @@ int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, OSSL_PROVIDER *prov,
size_t len;
int res = 1;

if (nid <= 0 || store == NULL)
if (nid <= 0 || store == NULL || prop_query == NULL)
return 0;
if (prop_query == NULL)
return 1;

if (!ossl_assert(prov != NULL))
return 0;
Expand Down
7 changes: 4 additions & 3 deletions crypto/store/store_meth.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -280,6 +280,7 @@ inner_loader_fetch(struct loader_data_st *methdata, int id,
{
OSSL_METHOD_STORE *store = get_loader_store(methdata->libctx);
OSSL_NAMEMAP *namemap = ossl_namemap_stored(methdata->libctx);
const char *const propq = properties != NULL ? properties : "";
void *method = NULL;
int unsupported = 0;

Expand Down Expand Up @@ -309,7 +310,7 @@ inner_loader_fetch(struct loader_data_st *methdata, int id,
unsupported = 1;

if (id == 0
|| !ossl_method_store_cache_get(store, NULL, id, properties, &method)) {
|| !ossl_method_store_cache_get(store, NULL, id, propq, &method)) {
OSSL_METHOD_CONSTRUCT_METHOD mcm = {
get_tmp_loader_store,
get_loader_from_store,
Expand All @@ -321,7 +322,7 @@ inner_loader_fetch(struct loader_data_st *methdata, int id,

methdata->scheme_id = id;
methdata->scheme = scheme;
methdata->propquery = properties;
methdata->propquery = propq;
methdata->flag_construct_error_occurred = 0;
if ((method = ossl_method_construct(methdata->libctx, OSSL_OP_STORE,
&prov, 0 /* !force_cache */,
Expand All @@ -333,7 +334,7 @@ inner_loader_fetch(struct loader_data_st *methdata, int id,
*/
if (id == 0)
id = ossl_namemap_name2num(namemap, scheme);
ossl_method_store_cache_set(store, prov, id, properties, method,
ossl_method_store_cache_set(store, prov, id, propq, method,
up_ref_loader, free_loader);
}

Expand Down
6 changes: 6 additions & 0 deletions doc/internal/man3/OSSL_METHOD_STORE.pod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,12 @@ The I<method_up_ref> function is called to increment the
reference count of the method and the I<method_destruct> function is called
to decrement it.

=head1 NOTES

The I<prop_query> argument to ossl_method_store_cache_get() and
ossl_method_store_cache_set() is not allowed to be NULL. Use "" for an
empty property definition or query.

=head1 RETURN VALUES

ossl_method_store_new() returns a new method store object or NULL on failure.
Expand Down

0 comments on commit af788ad

Please sign in to comment.