Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use name identity instead of name in diverse methods #9897

Closed
wants to merge 6 commits into from

Conversation

@levitte
Copy link
Member

levitte commented Sep 14, 2019

In method structures such as EVP_MD, EVP_CIPHER, etc when it's a provider implementation, we currently save away the name... however, the namemap already supports having a set of names associated with each different method, and saving copies of all the names would be fairly dumb, and the idea of a "canonical" name is still being disputed.

As an alternative, which avoids both the unnecessary saving and the dispute on "canonical" names is to save away the number associate with the name instead, and adapt the rest of the code around it. That's what this PR does.

This also adds a mechanism to check if a given method is a specific algorithm. EVP_CIPHER_is_a() and EVP_MAC_is_a work the same way, they take a method pointer and a string and check of that method is an algorithm that can be identified with the given string.


This is related to the discussions going on in, among others, #8984 and #8985

@levitte levitte added this to In progress in 3.0 New Core + FIPS via automation Sep 14, 2019
@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Sep 16, 2019

(even though this is a draft, it would be nice to get comments on the concept)

@levitte levitte force-pushed the levitte:nameid-instead-of-name branch 3 times, most recently from 8843ce4 to 11307d1 Sep 16, 2019
@levitte levitte marked this pull request as ready for review Sep 17, 2019
@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Sep 17, 2019

This is no longer a draft.

@levitte levitte changed the title WIP: Use name identity instead of name in diverse methods Use name identity instead of name in diverse methods Sep 17, 2019
@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Sep 17, 2019

... and out of WIP too

@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Sep 17, 2019

Ping.

crypto/evp/evp_fetch.c Outdated Show resolved Hide resolved
crypto/evp/evp_fetch.c Outdated Show resolved Hide resolved
doc/man3/EVP_EncryptInit.pod Outdated Show resolved Hide resolved
doc/man3/EVP_MAC.pod Show resolved Hide resolved
crypto/evp/evp_locl.h Show resolved Hide resolved
crypto/core_namemap.c Show resolved Hide resolved
3.0 New Core + FIPS automation moved this from In progress to Needs review Sep 18, 2019
crypto/evp/evp_fetch.c Outdated Show resolved Hide resolved
levitte added 5 commits Sep 14, 2019
…tring

Multiple names per implementation is already supported in the namemap,
but hasn't been used yet.  However, as soon as we have multiple names,
we will get an issue with what name should be saved in the method.

The solution is to not save the name itself, but rather the number
it's associated with.  This number is supposed to be unique for each
set of names, and we assume that algorithm names are globally unique,
i.e. there can be no name overlap between different algorithm types.

Incidently, it was also found that the 'get' function used by
ossl_construct_method() doesn't need all the parameters it was given;
most of what it needs, it can now get through the data structure given
by the caller of ossl_construct_method().  As a consequence,
ossl_construct_method() itself doesn't need all the parameters it was
given either.

There are some added internal functions that are expected to disappear
as soon as legacy code is removed, such as evp_first_name() and
ossl_namemap_num2name().
With some provider implementations, there are underlying ciphers,
digests and macs.  For some of them, the name was retrieved from the
method, but since the methods do not store those any more, we add
different mechanics.

For code that needs to pass on the name of a cipher or diges via
parameters, we simply locally store the name that was used when
fetching said cipher or digest.  This will ensure that any underlying
code that needs to fetch that same cipher or digest does so with the
exact same name instead of any random name from the set of names
associated with the algorithm.

For code that needs to check what kind of algorithm was passed, we
provide EVP_{type}_is_a(), that returns true if the given method has
the given name as one of its names.
@levitte levitte force-pushed the levitte:nameid-instead-of-name branch from ceffe2c to bd5618f Sep 19, 2019
@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Sep 19, 2019

Ping

3.0 New Core + FIPS automation moved this from Needs review to Reviewer approved Sep 19, 2019
levitte added a commit that referenced this pull request Sep 19, 2019
…tring

Multiple names per implementation is already supported in the namemap,
but hasn't been used yet.  However, as soon as we have multiple names,
we will get an issue with what name should be saved in the method.

The solution is to not save the name itself, but rather the number
it's associated with.  This number is supposed to be unique for each
set of names, and we assume that algorithm names are globally unique,
i.e. there can be no name overlap between different algorithm types.

Incidently, it was also found that the 'get' function used by
ossl_construct_method() doesn't need all the parameters it was given;
most of what it needs, it can now get through the data structure given
by the caller of ossl_construct_method().  As a consequence,
ossl_construct_method() itself doesn't need all the parameters it was
given either.

There are some added internal functions that are expected to disappear
as soon as legacy code is removed, such as evp_first_name() and
ossl_namemap_num2name().

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #9897)
levitte added a commit that referenced this pull request Sep 19, 2019
With some provider implementations, there are underlying ciphers,
digests and macs.  For some of them, the name was retrieved from the
method, but since the methods do not store those any more, we add
different mechanics.

For code that needs to pass on the name of a cipher or diges via
parameters, we simply locally store the name that was used when
fetching said cipher or digest.  This will ensure that any underlying
code that needs to fetch that same cipher or digest does so with the
exact same name instead of any random name from the set of names
associated with the algorithm.

For code that needs to check what kind of algorithm was passed, we
provide EVP_{type}_is_a(), that returns true if the given method has
the given name as one of its names.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #9897)
@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Sep 19, 2019

Merged.

f7c16d4 In provider implemented methods, save the name number, not the name string
7cfa171 Modify providers that keep track of underlying algorithms

@levitte levitte closed this Sep 19, 2019
3.0 New Core + FIPS automation moved this from Reviewer approved to Done Sep 19, 2019
@levitte

This comment has been minimized.

Copy link
Member Author

levitte commented Sep 19, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
2 participants
You can’t perform that action at this time.