Move all MACs to the providers by levitte · Pull Request #8877 · openssl/openssl

levitte · 2019-05-03T17:15:09Z

Because EVP_MAC is a new API, there is no legacy code to keep around, and the API is entirely adapted to support provider based implementations. This affects code in all kinds of places, all the way up to how openssl list -mac-algorithms does its work.

It's recommended to read the commit messages for explanation of some of the changes.

This currently depends on #9303

crypto/evp/p_lib.c

levitte · 2019-06-04T17:33:15Z

I've put down quite a lot of work in this, and there are multiple parts this change affects.

The EVP_MAC API itself

Because the EVP_MAC API is completely new and unreleased, I chose to throw away all the legacy stuff and make this completely provider based.

Most notably, this means that MACs are always fetched and that control functions are replaced with parameter passing functions.
It also means that MACs have no presence in the OBJ databases, except for the cases where a legacy EVP_PKEY implementation exists.

Information services

openssl list -mac-algorithms needed new code to be able to display information on existing MAC algorithms, and since the OBJ based functionality is now gone, a re-implementation of EVP_MAC_do_all and EVP_MAC_do_all_sorted is now needed.

TODO

~~Parameter names are currently scattered all over the code and need to be collected as macros in include/openssl/core_names.h.~~
~~Some functions and libcrypto<->provider interfaces are not yet implemented.~~

levitte · 2019-06-04T17:36:27Z

A note on these functions:

const OSSL_PARAM *EVP_MAC_CTX_get_param_types(const EVP_MAC *mac);
const OSSL_PARAM *EVP_MAC_CTX_set_param_types(const EVP_MAC *mac);

According to design discussions, these should return OSSL_ITEM arrays. However, that type is poor in information, especially regarding expected value sizes, and rather than creating yet another type, I thought that re-using OSSL_PARAM for parameter declaration and discovery purposes could be a viable idea. Special semantics apply to the data_size field, which means "arbitrary length" in this use.

I submit this as a proposal of a common manner to figure out available parameters.

levitte · 2019-06-05T07:03:15Z

I would appreciate some commentary on this

beldmit · 2019-06-05T11:57:05Z

Current TLS implementation refers to MAC NIDs. How can we change it to avoid this reference for future algorithms?

levitte · 2019-06-05T12:10:47Z

Current TLS implementation refers to MAC NIDs. How can we change it to avoid this reference for future algorithms?

Refer to them by name and use EVP_MAC_fetch()

beldmit · 2019-06-05T14:50:27Z

I mean places similar to ssl_cipher_get_evp() and ssl_load_ciphers(). So for TLS < 1.3 we either have to refactor this code or register NIDs for new primitives. It seems not a problem but should be taken into account.

apps/mac.c

util/libcrypto.num

test/evp_test.c

providers/default/defltprov.c

crypto/evp/evp_lib.c

providers/common/macs/gmac.c

providers/common/macs/hmac.c

levitte · 2019-06-06T09:14:19Z

TODO

~~Figure out OPENSSL_CTX passing (will be another PR, as this should be solved on a wider scale)~~
~~Figure out passing of properties (just pass as a utf8 string parameter, I presume)~~

levitte · 2019-06-19T07:15:48Z

OPENSSL_CTX passing was pretty much solved in #9160. I need to work a little bit more on this PR to integrate that, but it's getting close.

It currently builds, at least on my machine, and tests pretty fine, except for the BLAKE2 MACs...

levitte · 2019-06-24T07:13:21Z

This is no longer a draft

levitte · 2019-06-24T07:44:14Z

Argh, of course, the OSSL_PARAM return size change makes it all go belly up...

It might take a week before I get to this...

paulidale · 2019-06-24T08:03:00Z

Apologies :(

levitte · 2019-06-24T08:11:42Z

I approved it, so it's as much on me.

This avoids getting them confused with the MAC implementations.

For information processing.

levitte · 2019-08-15T12:11:47Z

I did a larger squashing job and pushed here to ensure that CIs are still happy. Feel free to have a new look through to see if I messed up... I don't think I did. If nothing goes wrong and no one complains, I'll use the approvals made so far.

levitte · 2019-08-15T20:13:23Z

Merged.

e74bd29 Prepare EVP_MAC infrastructure for moving all MACs to providers
55a0a11 Move BLAKE2 MACs to the providers
2e5db6a Move CMAC to providers
d33313b Move GMAC to providers
5183ebd Move HMAC to providers
e23cda0 Move KMAC to providers
4657693 Move SipHash to providers
ae0b6b9 Move Poly1305 to providers
6a4f9cd Remove init of MACs from EVP
d747fb2 Adapt apps/mac.c to use provider based MACs
776796e Adapt diverse code to provider based MACs.
25446a6 Adapt the MAC tests, and tests for other things that use EVP_MAC
f73eb73 Adjust some provider reason codes
810a1d0 OSSL_PARAM_construct_from_text(): handle non-hex octet string input
bb31895 Rename the hash implementations KMAC{128,256} to KECCAK_KMAC{128,256}
7dd0f29 Add EVP_MAC_provider()
d1cafb0 Implement EVP_MAC_do_all_ex()
467b8e5 Re-implement 'openssl list -mac-algorithms'

Quite a few adaptations are needed, most prominently the added code to allow provider based MACs. As part of this, all the old information functions are gone, except for EVP_MAC_name(). Some of them will reappear later, for example EVP_MAC_do_all() in some form. MACs by EVP_PKEY was particularly difficult to deal with, as they need to allocate and deallocate EVP_MAC_CTXs "under the hood", and thereby implicitly fetch the corresponding EVP_MAC. This means that EVP_MACs can't be constant in a EVP_MAC_CTX, as their reference count may need to be incremented and decremented as part of the allocation or deallocation of the EVP_MAC_CTX. It may be that other provider based EVP operation types may need to be handled in a similar manner. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Shane Lontis <shane.lontis@oracle.com> (Merged from #8877)

This also moves the remaining parts of BLAKE2 digests to the default provider, and removes the legacy EVP implementation. Reviewed-by: Matt Caswell <matt@openssl.org> Reviewed-by: Shane Lontis <shane.lontis@oracle.com> (Merged from #8877)