cisco / mlspp

Implementation of Messaging Layer Security
BSD 2-Clause "Simplified" License
110 stars 42 forks source link

Support OpenSSL 3.0 #259

Closed bifurcation closed 11 months ago

bifurcation commented 2 years ago

Currently, the hpke crypto library abstractions call several functions that have been deprecated in OpenSSL 3.0. Since the GitHub Actions runners appear to have upgraded to OpenSSL 3.0, this means that CI is broken until we support it.

The functions we rely on fall in basically three categories:

We should not switch over entirely to 3.0, since it's pretty new. So we will need CMake tooling to detect what version we have, set a #define configuration, and then have both 1.1 and 3.0 implementations.

Useful references:

ctjhai commented 1 year ago

Hi @bifurcation, I have an implementation that works as suggested above, i.e. let CMake detects the version of OpenSSL available in the system. However, there is a minor issue in that it may not be possible to set the flag EVP_MD_CTX_FLAG_NON_FIPS_ALLOW in FIPS mode. I think OpenSSL 3.0 doesn't support this flag anymore. It may be possible to set the flag as follows (although I am not sure if it does anything):

/* Error checking is ignored */
EVP_PKEY *key = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL, key_data, key_size);
EVP_MD_CTX *mctx = EVP_MD_CTX_new();
EVP_MD *md = EVP_sha256();
EVP_PKEY_CTX *pctx = NULL;
EVP_MD_CTX_set_flags(mctx, EVP_MD_CTX_FLAG_NON_FIPS_ALLOW); /* <-- Set the flag here */
EVP_DigestSignInit(mctx, &pctx, md, NULL, key);
EVP_DigestSignUpdate(mctx, data, data_size);
EVP_DigestSignFinal(mctx, digest, &digest_size);
...

However, the EVP_ API above does not work with an empty HMAC key, which is needed in HPKE, see: https://github.com/openssl/openssl/issues/13089.

As an alternative, I am using EVP_MAC_ API instead and throw an exception if the HMAC key is less than 112 bits, see: https://github.com/ctjhai/mlspp/tree/openssl3

Shall I raise a PR against this repo?

bifurcation commented 1 year ago

A PR would be great! Thanks for doing the work to figure all this out.

Just to confirm I understand: It seems like the combination of (a) HPKE's requirement for a zero-length MAC key, and (b) the lack of a FIPS escape hatch in OpenSSL 3 mean that HPKE and MLS won't work with OpenSSL 3 in FIPS mode? If that's the case, we should have a switch in CMakeLists.txt that forces OpenSSL 1.1, so that a consumer that requires FIPS can build that way even if OpenSSL 3 is present.

One thing that might be worth considering (probably separately from this): As of version 3.2, OpenSSL has its own support for HPKE. So one could imagine swapping that in for the custom HPKE implementation here. It might not save a ton, since you would still need a bunch of the hpke library for AEAD, signature, and safe wrapping of the OpenSSL HPKE. Likely not worth doing until we can assume 3.2 as a minimum version.

ctjhai commented 1 year ago

That's right, I believe MLS won't work with OpenSSL 3 in FIPS mode. However, it looks like OpenSSL 1.1 isn't FIPS compliant either, only versions 3.0, 1.0.2 and 1.0.1 are FIPS compliant, see: https://wiki.openssl.org/index.php/FIPS_modules. But the minimum supported version for HPKE library is 1.1, this doesn't look right. Have you tried compiling/linking against OpenSSL 1.0.2? Not saying it's a good idea as it is no longer supported.

Agreed on OpenSSL v3.2, we can revisit that in the future.

bifurcation commented 1 year ago

OpenSSL 1.1 itself isn't FIPS-compliant, but there are FIPS-validated modules derived from it (and thus API-compatible), in particular the Cisco FOM. So for Cisco in particular, a 1.1 switch would be good enough to support our FIPS module. Presumably any other people who require FIPS are either not using MLSpp right now (since we've always required 1.1), or they have modules that are compatible with 1.1.

I haven't tried linking against 1.0.2, but I seem to recall some relevant incompatibilities. In any case, as you point out, not worth it.

So yeah, if we can have OpenSSL 3 support that works now in non-FIPS mode, with a CMake option to force 1.1 for when FIPS mode is required, that seems like it would work well. Then we can delete the old code once 3.X fixes the zero-length key bug.

glhewett commented 11 months ago

@bifurcation, Is there anything else to do for this issue?

bifurcation commented 11 months ago

Nope, this is good.