From a0bece343e38d73d038d4f3a62c2a9638608ac9c Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Thu, 22 Apr 2021 19:16:38 -0500 Subject: [PATCH 2/4] [WIP] 3.0.0 support (#5250) * 3.0.0 support * almost...there... * make mypy happy --- .github/workflows/ci.yml | 7 ++-- src/_cffi_src/build_openssl.py | 1 + src/_cffi_src/openssl/cryptography.py | 3 ++ src/_cffi_src/openssl/err.py | 6 +++ src/_cffi_src/openssl/fips.py | 2 +- src/_cffi_src/openssl/provider.py | 40 ++++++++++++++++++ .../hazmat/backends/openssl/backend.py | 42 ++++++++++++++++--- .../hazmat/backends/openssl/ciphers.py | 15 ++++++- .../hazmat/bindings/openssl/_conditional.py | 11 +++++ .../hazmat/bindings/openssl/binding.py | 20 +++++++++ tests/hazmat/backends/test_openssl_memleak.py | 6 ++- tests/hazmat/bindings/test_openssl.py | 4 +- tests/hazmat/primitives/test_dh.py | 24 ++++++++++- 13 files changed, 167 insertions(+), 14 deletions(-) create mode 100644 src/_cffi_src/openssl/provider.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index cd967a3a..747f84c1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -18,9 +18,10 @@ jobs: - {VERSION: "3.9", TOXENV: "flake,rust,docs", COVERAGE: "false"} - {VERSION: "pypy3", TOXENV: "pypy3"} - {VERSION: "3.9", TOXENV: "py39", OPENSSL: {TYPE: "openssl", VERSION: "1.1.0l"}} - - {VERSION: "3.9", TOXENV: "py39", OPENSSL: {TYPE: "openssl", VERSION: "1.1.1i"}} - - {VERSION: "3.9", TOXENV: "py39-ssh", OPENSSL: {TYPE: "openssl", VERSION: "1.1.1i"}} - - {VERSION: "3.9", TOXENV: "py39", OPENSSL: {TYPE: "openssl", VERSION: "1.1.1i", CONFIG_FLAGS: "no-engine no-rc2 no-srtp no-ct"}} + - {VERSION: "3.9", TOXENV: "py39", OPENSSL: {TYPE: "openssl", VERSION: "1.1.1j"}} + - {VERSION: "3.9", TOXENV: "py39-ssh", OPENSSL: {TYPE: "openssl", VERSION: "1.1.1j"}} + - {VERSION: "3.9", TOXENV: "py39", OPENSSL: {TYPE: "openssl", VERSION: "1.1.1j", CONFIG_FLAGS: "no-engine no-rc2 no-srtp no-ct"}} + - {VERSION: "3.9", TOXENV: "py39", OPENSSL: {TYPE: "openssl", VERSION: "3.0.0-alpha15"}} - {VERSION: "3.9", TOXENV: "py39", OPENSSL: {TYPE: "libressl", VERSION: "2.9.2"}} - {VERSION: "3.9", TOXENV: "py39", OPENSSL: {TYPE: "libressl", VERSION: "3.0.2"}} - {VERSION: "3.9", TOXENV: "py39", OPENSSL: {TYPE: "libressl", VERSION: "3.1.5"}} diff --git a/src/_cffi_src/build_openssl.py b/src/_cffi_src/build_openssl.py index 08499d66..557296ed 100644 --- a/src/_cffi_src/build_openssl.py +++ b/src/_cffi_src/build_openssl.py @@ -104,6 +104,7 @@ ffi = build_ffi_for_binding( "osrandom_engine", "pem", "pkcs12", + "provider", "rand", "rsa", "ssl", diff --git a/src/_cffi_src/openssl/cryptography.py b/src/_cffi_src/openssl/cryptography.py index e2b5a132..06d1e778 100644 --- a/src/_cffi_src/openssl/cryptography.py +++ b/src/_cffi_src/openssl/cryptography.py @@ -34,6 +34,8 @@ INCLUDES = """ #define CRYPTOGRAPHY_OPENSSL_110F_OR_GREATER \ (OPENSSL_VERSION_NUMBER >= 0x1010006f && !CRYPTOGRAPHY_IS_LIBRESSL) +#define CRYPTOGRAPHY_OPENSSL_300_OR_GREATER \ + (OPENSSL_VERSION_NUMBER >= 0x30000000 && !CRYPTOGRAPHY_IS_LIBRESSL) #define CRYPTOGRAPHY_OPENSSL_LESS_THAN_110J \ (OPENSSL_VERSION_NUMBER < 0x101000af || CRYPTOGRAPHY_IS_LIBRESSL) @@ -53,6 +55,7 @@ INCLUDES = """ TYPES = """ static const int CRYPTOGRAPHY_OPENSSL_110F_OR_GREATER; +static const int CRYPTOGRAPHY_OPENSSL_300_OR_GREATER; static const int CRYPTOGRAPHY_OPENSSL_LESS_THAN_111; static const int CRYPTOGRAPHY_OPENSSL_LESS_THAN_111B; diff --git a/src/_cffi_src/openssl/err.py b/src/_cffi_src/openssl/err.py index 0634b656..8cfeaf5b 100644 --- a/src/_cffi_src/openssl/err.py +++ b/src/_cffi_src/openssl/err.py @@ -18,6 +18,7 @@ static const int EVP_R_UNKNOWN_PBE_ALGORITHM; static const int ERR_LIB_EVP; static const int ERR_LIB_PEM; +static const int ERR_LIB_PROV; static const int ERR_LIB_ASN1; static const int ERR_LIB_PKCS12; @@ -45,4 +46,9 @@ int ERR_GET_REASON(unsigned long); """ CUSTOMIZATIONS = """ +/* This define is tied to provider support and is conditionally + removed if Cryptography_HAS_PROVIDERS is false */ +#ifndef ERR_LIB_PROV +#define ERR_LIB_PROV 0 +#endif """ diff --git a/src/_cffi_src/openssl/fips.py b/src/_cffi_src/openssl/fips.py index b9d0d64d..23c10af9 100644 --- a/src/_cffi_src/openssl/fips.py +++ b/src/_cffi_src/openssl/fips.py @@ -17,7 +17,7 @@ int FIPS_mode(void); """ CUSTOMIZATIONS = """ -#if CRYPTOGRAPHY_IS_LIBRESSL +#if CRYPTOGRAPHY_IS_LIBRESSL || CRYPTOGRAPHY_OPENSSL_300_OR_GREATER static const long Cryptography_HAS_FIPS = 0; int (*FIPS_mode_set)(int) = NULL; int (*FIPS_mode)(void) = NULL; diff --git a/src/_cffi_src/openssl/provider.py b/src/_cffi_src/openssl/provider.py new file mode 100644 index 00000000..d7d659ea --- /dev/null +++ b/src/_cffi_src/openssl/provider.py @@ -0,0 +1,40 @@ +# This file is dual licensed under the terms of the Apache License, Version +# 2.0, and the BSD License. See the LICENSE file in the root of this repository +# for complete details. + + +INCLUDES = """ +#if CRYPTOGRAPHY_OPENSSL_300_OR_GREATER +#include +#include +#endif +""" + +TYPES = """ +static const long Cryptography_HAS_PROVIDERS; + +typedef ... OSSL_PROVIDER; +typedef ... OSSL_LIB_CTX; + +static const long PROV_R_BAD_DECRYPT; +static const long PROV_R_WRONG_FINAL_BLOCK_LENGTH; +""" + +FUNCTIONS = """ +OSSL_PROVIDER *OSSL_PROVIDER_load(OSSL_LIB_CTX *, const char *); +int OSSL_PROVIDER_unload(OSSL_PROVIDER *prov); +""" + +CUSTOMIZATIONS = """ +#if CRYPTOGRAPHY_OPENSSL_300_OR_GREATER +static const long Cryptography_HAS_PROVIDERS = 1; +#else +static const long Cryptography_HAS_PROVIDERS = 0; +typedef void OSSL_PROVIDER; +typedef void OSSL_LIB_CTX; +static const long PROV_R_BAD_DECRYPT = 0; +static const long PROV_R_WRONG_FINAL_BLOCK_LENGTH = 0; +OSSL_PROVIDER *(*OSSL_PROVIDER_load)(OSSL_LIB_CTX *, const char *) = NULL; +int (*OSSL_PROVIDER_unload)(OSSL_PROVIDER *) = NULL; +#endif +""" diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py index a96d08d8..86e8f0a8 100644 --- a/src/cryptography/hazmat/backends/openssl/backend.py +++ b/src/cryptography/hazmat/backends/openssl/backend.py @@ -1281,6 +1281,11 @@ class Backend(object): def _evp_pkey_from_der_traditional_key(self, bio_data, password): key = self._lib.d2i_PrivateKey_bio(bio_data.bio, self._ffi.NULL) if key != self._ffi.NULL: + # In OpenSSL 3.0.0-alpha15 there exist scenarios where the key will + # successfully load but errors are still put on the stack. Tracked + # as https://github.com/openssl/openssl/issues/14996 + self._consume_errors() + key = self._ffi.gc(key, self._lib.EVP_PKEY_free) if password is not None: raise TypeError( @@ -1448,6 +1453,11 @@ class Backend(object): else: self._handle_key_loading_error() + # In OpenSSL 3.0.0-alpha15 there exist scenarios where the key will + # successfully load but errors are still put on the stack. Tracked + # as https://github.com/openssl/openssl/issues/14996 + self._consume_errors() + evp_pkey = self._ffi.gc(evp_pkey, self._lib.EVP_PKEY_free) if password is not None and userdata.called == 0: @@ -1470,11 +1480,22 @@ class Backend(object): "incorrect format or it may be encrypted with an unsupported " "algorithm." ) - elif errors[0]._lib_reason_match( - self._lib.ERR_LIB_EVP, self._lib.EVP_R_BAD_DECRYPT - ) or errors[0]._lib_reason_match( - self._lib.ERR_LIB_PKCS12, - self._lib.PKCS12_R_PKCS12_CIPHERFINAL_ERROR, + + elif ( + errors[0]._lib_reason_match( + self._lib.ERR_LIB_EVP, self._lib.EVP_R_BAD_DECRYPT + ) + or errors[0]._lib_reason_match( + self._lib.ERR_LIB_PKCS12, + self._lib.PKCS12_R_PKCS12_CIPHERFINAL_ERROR, + ) + or ( + self._lib.Cryptography_HAS_PROVIDERS + and errors[0]._lib_reason_match( + self._lib.ERR_LIB_PROV, + self._lib.PROV_R_BAD_DECRYPT, + ) + ) ): raise ValueError("Bad decrypt. Incorrect password?") @@ -2520,7 +2541,16 @@ class Backend(object): if sk_x509_ptr[0] != self._ffi.NULL: sk_x509 = self._ffi.gc(sk_x509_ptr[0], self._lib.sk_X509_free) num = self._lib.sk_X509_num(sk_x509_ptr[0]) - for i in range(num): + + # In OpenSSL < 3.0.0 PKCS12 parsing reverses the order of the + # certificates. + indices: typing.Iterable[int] + if self._lib.CRYPTOGRAPHY_OPENSSL_300_OR_GREATER: + indices = range(num) + else: + indices = reversed(range(num)) + + for i in indices: x509 = self._lib.sk_X509_value(sk_x509, i) self.openssl_assert(x509 != self._ffi.NULL) x509 = self._ffi.gc(x509, self._lib.X509_free) diff --git a/src/cryptography/hazmat/backends/openssl/ciphers.py b/src/cryptography/hazmat/backends/openssl/ciphers.py index 0f96795f..a2dd6894 100644 --- a/src/cryptography/hazmat/backends/openssl/ciphers.py +++ b/src/cryptography/hazmat/backends/openssl/ciphers.py @@ -145,7 +145,13 @@ class _CipherContext(object): res = self._backend._lib.EVP_CipherUpdate( self._ctx, outbuf, outlen, inbuf, inlen ) - self._backend.openssl_assert(res != 0) + if res == 0 and isinstance(self._mode, modes.XTS): + raise ValueError( + "In XTS mode you must supply at least a full block in the " + "first update call. For AES this is 16 bytes." + ) + else: + self._backend.openssl_assert(res != 0) data_processed += inlen total_out += outlen[0] @@ -174,6 +180,13 @@ class _CipherContext(object): errors[0]._lib_reason_match( self._backend._lib.ERR_LIB_EVP, self._backend._lib.EVP_R_DATA_NOT_MULTIPLE_OF_BLOCK_LENGTH, + ) + or ( + self._backend._lib.Cryptography_HAS_PROVIDERS + and errors[0]._lib_reason_match( + self._backend._lib.ERR_LIB_PROV, + self._backend._lib.PROV_R_WRONG_FINAL_BLOCK_LENGTH, + ) ), errors=errors, ) diff --git a/src/cryptography/hazmat/bindings/openssl/_conditional.py b/src/cryptography/hazmat/bindings/openssl/_conditional.py index 86548357..1f42c7be 100644 --- a/src/cryptography/hazmat/bindings/openssl/_conditional.py +++ b/src/cryptography/hazmat/bindings/openssl/_conditional.py @@ -270,6 +270,16 @@ def cryptography_has_get_proto_version(): ] +def cryptography_has_providers(): + return [ + "OSSL_PROVIDER_load", + "OSSL_PROVIDER_unload", + "ERR_LIB_PROV", + "PROV_R_WRONG_FINAL_BLOCK_LENGTH", + "PROV_R_BAD_DECRYPT", + ] + + # This is a mapping of # {condition: function-returning-names-dependent-on-that-condition} so we can # loop over them and delete unsupported names at runtime. It will be removed @@ -318,4 +328,5 @@ CONDITIONAL_NAMES = { "Cryptography_HAS_VERIFIED_CHAIN": cryptography_has_verified_chain, "Cryptography_HAS_SRTP": cryptography_has_srtp, "Cryptography_HAS_GET_PROTO_VERSION": cryptography_has_get_proto_version, + "Cryptography_HAS_PROVIDERS": cryptography_has_providers, } diff --git a/src/cryptography/hazmat/bindings/openssl/binding.py b/src/cryptography/hazmat/bindings/openssl/binding.py index a2bc36a8..6dcec26a 100644 --- a/src/cryptography/hazmat/bindings/openssl/binding.py +++ b/src/cryptography/hazmat/bindings/openssl/binding.py @@ -113,6 +113,8 @@ class Binding(object): ffi = ffi _lib_loaded = False _init_lock = threading.Lock() + _legacy_provider: typing.Any = None + _default_provider: typing.Any = None def __init__(self): self._ensure_ffi_initialized() @@ -140,6 +142,24 @@ class Binding(object): # adds all ciphers/digests for EVP cls.lib.OpenSSL_add_all_algorithms() cls._register_osrandom_engine() + # As of OpenSSL 3.0.0 we must register a legacy cipher provider + # to get RC2 (needed for junk asymmetric private key + # serialization), RC4, Blowfish, IDEA, SEED, etc. These things + # are ugly legacy, but we aren't going to get rid of them + # any time soon. + if cls.lib.CRYPTOGRAPHY_OPENSSL_300_OR_GREATER: + cls._legacy_provider = cls.lib.OSSL_PROVIDER_load( + cls.ffi.NULL, b"legacy" + ) + _openssl_assert( + cls.lib, cls._legacy_provider != cls.ffi.NULL + ) + cls._default_provider = cls.lib.OSSL_PROVIDER_load( + cls.ffi.NULL, b"default" + ) + _openssl_assert( + cls.lib, cls._default_provider != cls.ffi.NULL + ) @classmethod def init_static_locks(cls): diff --git a/tests/hazmat/backends/test_openssl_memleak.py b/tests/hazmat/backends/test_openssl_memleak.py index 0c96516f..0316b5d9 100644 --- a/tests/hazmat/backends/test_openssl_memleak.py +++ b/tests/hazmat/backends/test_openssl_memleak.py @@ -82,7 +82,7 @@ def main(argv): assert result == 1 # Trigger a bunch of initialization stuff. - import cryptography.hazmat.backends.openssl + from cryptography.hazmat.backends.openssl.backend import backend start_heap = set(heap) @@ -91,6 +91,10 @@ def main(argv): gc.collect() gc.collect() + if lib.CRYPTOGRAPHY_OPENSSL_300_OR_GREATER: + lib.OSSL_PROVIDER_unload(backend._binding._legacy_provider) + lib.OSSL_PROVIDER_unload(backend._binding._default_provider) + if lib.Cryptography_HAS_OPENSSL_CLEANUP: lib.OPENSSL_cleanup() diff --git a/tests/hazmat/bindings/test_openssl.py b/tests/hazmat/bindings/test_openssl.py index fb9a1e36..4d1e3b55 100644 --- a/tests/hazmat/bindings/test_openssl.py +++ b/tests/hazmat/bindings/test_openssl.py @@ -91,7 +91,9 @@ class TestOpenSSL(object): _openssl_assert(b.lib, False) error = exc_info.value.err_code[0] - assert error.code == 101183626 + # As of 3.0.0 OpenSSL sets func codes to 0, so the combined + # code is a different value + assert error.code in (101183626, 50331786) assert error.lib == b.lib.ERR_LIB_EVP assert error.func == b.lib.EVP_F_EVP_ENCRYPTFINAL_EX assert error.reason == b.lib.EVP_R_DATA_NOT_MULTIPLE_OF_BLOCK_LENGTH diff --git a/tests/hazmat/primitives/test_dh.py b/tests/hazmat/primitives/test_dh.py index 131807fc..bb29919f 100644 --- a/tests/hazmat/primitives/test_dh.py +++ b/tests/hazmat/primitives/test_dh.py @@ -180,7 +180,23 @@ class TestDH(object): params = dh.DHParameterNumbers(p, int(vector["g"])) param = params.parameters(backend) key = param.generate_private_key() - assert key.private_numbers().public_numbers.parameter_numbers == params + # In OpenSSL 3.0.0 OpenSSL maps to known groups. This results in + # a scenario where loading a known group with p and g returns a + # re-serialized form that has q as well (the Sophie Germain prime of + # that group). This makes a naive comparison of the parameter numbers + # objects fail, so we have to be a bit smarter + serialized_params = ( + key.private_numbers().public_numbers.parameter_numbers + ) + if serialized_params.q is None: + # This is the path OpenSSL < 3.0 takes + assert serialized_params == params + else: + assert serialized_params.p == params.p + assert serialized_params.g == params.g + # p = 2q + 1 since it is a Sophie Germain prime, so we can compute + # what we expect OpenSSL to have done here. + assert serialized_params.q == (params.p - 1) // 2 @pytest.mark.skip_fips(reason="non-FIPS parameters") @pytest.mark.parametrize( @@ -382,6 +398,12 @@ class TestDH(object): assert symkey1 != symkey2 @pytest.mark.skip_fips(reason="key_size too small for FIPS") + @pytest.mark.supported( + only_if=lambda backend: ( + not backend._lib.CRYPTOGRAPHY_OPENSSL_300_OR_GREATER + ), + skip_message="256-bit DH keys are not supported in OpenSSL 3.0.0+", + ) def test_load_256bit_key_from_pkcs8(self, backend): data = load_vectors_from_file( os.path.join("asymmetric", "DH", "dh_key_256.pem"), -- 2.31.1