From 0e0e46f5f73f477b8ee9682738c42129d5d60177 Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Sat, 3 Feb 2024 14:02:48 -0600 Subject: [PATCH 1/7] backport: initialize openssl's legacy provider in rust (#10323) (#10333) * initialize openssl's legacy provider in rust (#10323) * initialize openssl's legacy provider in rust as we oxidize we need to do this here to ensure it actually happens * alex is a comment format pedant * remove the memleak tests (#10322) they are fragile, haven't caught regressions, and increasingly pointless as we oxidize. --- src/_cffi_src/openssl/crypto.py | 45 -- .../hazmat/backends/openssl/backend.py | 4 +- .../bindings/_rust/openssl/__init__.pyi | 2 + .../hazmat/bindings/openssl/_conditional.py | 7 - .../hazmat/bindings/openssl/binding.py | 31 -- src/rust/src/lib.rs | 65 +++ tests/hazmat/backends/test_openssl_memleak.py | 391 ------------------ tests/hazmat/bindings/test_openssl.py | 7 - 8 files changed, 69 insertions(+), 483 deletions(-) delete mode 100644 tests/hazmat/backends/test_openssl_memleak.py diff --git a/src/_cffi_src/openssl/crypto.py b/src/_cffi_src/openssl/crypto.py index b81b5de1da27..5284f329619c 100644 --- a/src/_cffi_src/openssl/crypto.py +++ b/src/_cffi_src/openssl/crypto.py @@ -9,8 +9,6 @@ """ TYPES = """ -static const long Cryptography_HAS_MEM_FUNCTIONS; - static const int OPENSSL_VERSION; static const int OPENSSL_CFLAGS; static const int OPENSSL_BUILT_ON; @@ -26,50 +24,7 @@ void *OPENSSL_malloc(size_t); void OPENSSL_free(void *); - - -/* Signature is significantly different in LibreSSL, so expose via different - symbol name */ -int Cryptography_CRYPTO_set_mem_functions( - void *(*)(size_t, const char *, int), - void *(*)(void *, size_t, const char *, int), - void (*)(void *, const char *, int)); - -void *Cryptography_malloc_wrapper(size_t, const char *, int); -void *Cryptography_realloc_wrapper(void *, size_t, const char *, int); -void Cryptography_free_wrapper(void *, const char *, int); """ CUSTOMIZATIONS = """ -#if CRYPTOGRAPHY_IS_LIBRESSL || CRYPTOGRAPHY_IS_BORINGSSL -static const long Cryptography_HAS_MEM_FUNCTIONS = 0; -int (*Cryptography_CRYPTO_set_mem_functions)( - void *(*)(size_t, const char *, int), - void *(*)(void *, size_t, const char *, int), - void (*)(void *, const char *, int)) = NULL; - -#else -static const long Cryptography_HAS_MEM_FUNCTIONS = 1; - -int Cryptography_CRYPTO_set_mem_functions( - void *(*m)(size_t, const char *, int), - void *(*r)(void *, size_t, const char *, int), - void (*f)(void *, const char *, int) -) { - return CRYPTO_set_mem_functions(m, r, f); -} -#endif - -void *Cryptography_malloc_wrapper(size_t size, const char *path, int line) { - return malloc(size); -} - -void *Cryptography_realloc_wrapper(void *ptr, size_t size, const char *path, - int line) { - return realloc(ptr, size); -} - -void Cryptography_free_wrapper(void *ptr, const char *path, int line) { - free(ptr); -} """ diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py index 5d9eb2768dfb..5cd1103bd133 100644 --- a/src/cryptography/hazmat/backends/openssl/backend.py +++ b/src/cryptography/hazmat/backends/openssl/backend.py @@ -138,7 +138,7 @@ def __repr__(self) -> str: return "".format( self.openssl_version_text(), self._fips_enabled, - self._binding._legacy_provider_loaded, + rust_openssl._legacy_provider_loaded, ) def openssl_assert( @@ -277,7 +277,7 @@ def _register_default_ciphers(self) -> None: # we get an EVP_CIPHER * in the _CipherContext __init__, but OpenSSL 3 # will return a valid pointer even though the cipher is unavailable. if ( - self._binding._legacy_provider_loaded + rust_openssl._legacy_provider_loaded or not self._lib.CRYPTOGRAPHY_OPENSSL_300_OR_GREATER ): for mode_cls in [CBC, CFB, OFB, ECB]: diff --git a/src/cryptography/hazmat/bindings/_rust/openssl/__init__.pyi b/src/cryptography/hazmat/bindings/_rust/openssl/__init__.pyi index 9cdb4d6a5c6e..cc54647732cc 100644 --- a/src/cryptography/hazmat/bindings/_rust/openssl/__init__.pyi +++ b/src/cryptography/hazmat/bindings/_rust/openssl/__init__.pyi @@ -42,6 +42,8 @@ __all__ = [ "x25519", ] +_legacy_provider_loaded: bool + def openssl_version() -> int: ... def raise_openssl_error() -> typing.NoReturn: ... def capture_error_stack() -> list[OpenSSLError]: ... diff --git a/src/cryptography/hazmat/bindings/openssl/_conditional.py b/src/cryptography/hazmat/bindings/openssl/_conditional.py index 30cc3bfa25ef..fc13348af77f 100644 --- a/src/cryptography/hazmat/bindings/openssl/_conditional.py +++ b/src/cryptography/hazmat/bindings/openssl/_conditional.py @@ -28,12 +28,6 @@ def cryptography_has_tls_st() -> list[str]: ] -def cryptography_has_mem_functions() -> list[str]: - return [ - "Cryptography_CRYPTO_set_mem_functions", - ] - - def cryptography_has_ed448() -> list[str]: return [ "EVP_PKEY_ED448", @@ -202,7 +196,6 @@ def cryptography_has_get_extms_support() -> list[str]: "Cryptography_HAS_SET_CERT_CB": cryptography_has_set_cert_cb, "Cryptography_HAS_SSL_ST": cryptography_has_ssl_st, "Cryptography_HAS_TLS_ST": cryptography_has_tls_st, - "Cryptography_HAS_MEM_FUNCTIONS": cryptography_has_mem_functions, "Cryptography_HAS_ED448": cryptography_has_ed448, "Cryptography_HAS_SIGALGS": cryptography_has_ssl_sigalgs, "Cryptography_HAS_PSK": cryptography_has_psk, diff --git a/src/cryptography/hazmat/bindings/openssl/binding.py b/src/cryptography/hazmat/bindings/openssl/binding.py index 40814f2a58a0..209fbeb73a8f 100644 --- a/src/cryptography/hazmat/bindings/openssl/binding.py +++ b/src/cryptography/hazmat/bindings/openssl/binding.py @@ -37,17 +37,6 @@ def _openssl_assert( ) -def _legacy_provider_error(loaded: bool) -> None: - if not loaded: - raise RuntimeError( - "OpenSSL 3.0's legacy provider failed to load. This is a fatal " - "error by default, but cryptography supports running without " - "legacy algorithms by setting the environment variable " - "CRYPTOGRAPHY_OPENSSL_NO_LEGACY. If you did not expect this error," - " you have likely made a mistake with your OpenSSL configuration." - ) - - def build_conditional_library( lib: typing.Any, conditional_names: dict[str, typing.Callable[[], list[str]]], @@ -76,7 +65,6 @@ class Binding: _lib_loaded = False _init_lock = threading.Lock() _legacy_provider: typing.Any = ffi.NULL - _legacy_provider_loaded = False _default_provider: typing.Any = ffi.NULL def __init__(self) -> None: @@ -106,25 +94,6 @@ def _ensure_ffi_initialized(cls) -> None: _openssl.lib, CONDITIONAL_NAMES ) cls._lib_loaded = True - # 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: - if not os.environ.get("CRYPTOGRAPHY_OPENSSL_NO_LEGACY"): - cls._legacy_provider = cls.lib.OSSL_PROVIDER_load( - cls.ffi.NULL, b"legacy" - ) - cls._legacy_provider_loaded = ( - cls._legacy_provider != cls.ffi.NULL - ) - _legacy_provider_error(cls._legacy_provider_loaded) - - cls._default_provider = cls.lib.OSSL_PROVIDER_load( - cls.ffi.NULL, b"default" - ) - _openssl_assert(cls._default_provider != cls.ffi.NULL) @classmethod def init_static_locks(cls) -> None: diff --git a/src/rust/src/lib.rs b/src/rust/src/lib.rs index 9dd54f4b901d..c9f9285e3825 100644 --- a/src/rust/src/lib.rs +++ b/src/rust/src/lib.rs @@ -4,6 +4,11 @@ #![deny(rust_2018_idioms, clippy::undocumented_unsafe_blocks)] +use crate::error::CryptographyResult; +#[cfg(CRYPTOGRAPHY_OPENSSL_300_OR_GREATER)] +use openssl::provider; +use std::env; + mod asn1; mod backend; mod buf; @@ -15,6 +20,12 @@ mod pkcs7; pub(crate) mod types; mod x509; +#[cfg(CRYPTOGRAPHY_OPENSSL_300_OR_GREATER)] +#[pyo3::prelude::pyclass(frozen, module = "cryptography.hazmat.bindings._rust")] +struct LoadedProviders { + legacy: Option, +} + #[pyo3::prelude::pyfunction] fn openssl_version() -> i64 { openssl::version::number() @@ -25,6 +36,35 @@ fn is_fips_enabled() -> bool { cryptography_openssl::fips::is_enabled() } +#[cfg(CRYPTOGRAPHY_OPENSSL_300_OR_GREATER)] +fn _initialize_legacy_provider() -> CryptographyResult { + // 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. + let load_legacy = env::var("CRYPTOGRAPHY_OPENSSL_NO_LEGACY") + .map(|v| v.is_empty() || v == "0") + .unwrap_or(true); + let legacy = if load_legacy { + let legacy_result = provider::Provider::try_load(None, "legacy", true); + _legacy_provider_error(legacy_result.is_ok())?; + Some(legacy_result?) + } else { + None + }; + Ok(LoadedProviders { legacy }) +} + +fn _legacy_provider_error(success: bool) -> pyo3::PyResult<()> { + if !success { + return Err(pyo3::exceptions::PyRuntimeError::new_err( + "OpenSSL 3.0's legacy provider failed to load. This is a fatal error by default, but cryptography supports running without legacy algorithms by setting the environment variable CRYPTOGRAPHY_OPENSSL_NO_LEGACY. If you did not expect this error, you have likely made a mistake with your OpenSSL configuration." + )); + } + Ok(()) +} + #[pyo3::prelude::pymodule] fn _rust(py: pyo3::Python<'_>, m: &pyo3::types::PyModule) -> pyo3::PyResult<()> { m.add_function(pyo3::wrap_pyfunction!(padding::check_pkcs7_padding, m)?)?; @@ -52,6 +92,20 @@ fn _rust(py: pyo3::Python<'_>, m: &pyo3::types::PyModule) -> pyo3::PyResult<()> m.add_submodule(cryptography_cffi::create_module(py)?)?; let openssl_mod = pyo3::prelude::PyModule::new(py, "openssl")?; + cfg_if::cfg_if! { + if #[cfg(CRYPTOGRAPHY_OPENSSL_300_OR_GREATER)] { + let providers = _initialize_legacy_provider()?; + if providers.legacy.is_some() { + openssl_mod.add("_legacy_provider_loaded", true)?; + openssl_mod.add("_providers", providers)?; + } else { + openssl_mod.add("_legacy_provider_loaded", false)?; + } + } else { + // default value for non-openssl 3+ + openssl_mod.add("_legacy_provider_loaded", false)?; + } + } openssl_mod.add_function(pyo3::wrap_pyfunction!(openssl_version, m)?)?; openssl_mod.add_function(pyo3::wrap_pyfunction!(error::raise_openssl_error, m)?)?; openssl_mod.add_function(pyo3::wrap_pyfunction!(error::capture_error_stack, m)?)?; @@ -62,3 +116,14 @@ fn _rust(py: pyo3::Python<'_>, m: &pyo3::types::PyModule) -> pyo3::PyResult<()> Ok(()) } + +#[cfg(test)] +mod tests { + use super::_legacy_provider_error; + + #[test] + fn test_legacy_provider_error() { + assert!(_legacy_provider_error(true).is_ok()); + assert!(_legacy_provider_error(false).is_err()); + } +} diff --git a/tests/hazmat/backends/test_openssl_memleak.py b/tests/hazmat/backends/test_openssl_memleak.py deleted file mode 100644 index 371a7c990188..000000000000 --- a/tests/hazmat/backends/test_openssl_memleak.py +++ /dev/null @@ -1,391 +0,0 @@ -# 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. - - -import json -import os -import platform -import subprocess -import sys -import textwrap - -import pytest - -from cryptography.hazmat.bindings.openssl.binding import Binding - -MEMORY_LEAK_SCRIPT = """ -import sys - - -def main(argv): - import gc - import json - - import cffi - - from cryptography.hazmat.bindings._rust import _openssl - - heap = {} - start_heap = {} - start_heap_realloc_delta = [0] # 1-item list so callbacks can mutate it - - BACKTRACE_ENABLED = False - if BACKTRACE_ENABLED: - backtrace_ffi = cffi.FFI() - backtrace_ffi.cdef(''' - int backtrace(void **, int); - char **backtrace_symbols(void *const *, int); - ''') - backtrace_lib = backtrace_ffi.dlopen(None) - - def backtrace(): - buf = backtrace_ffi.new("void*[]", 24) - length = backtrace_lib.backtrace(buf, len(buf)) - return (buf, length) - - def symbolize_backtrace(trace): - (buf, length) = trace - symbols = backtrace_lib.backtrace_symbols(buf, length) - stack = [ - backtrace_ffi.string(symbols[i]).decode() - for i in range(length) - ] - _openssl.lib.Cryptography_free_wrapper( - symbols, backtrace_ffi.NULL, 0 - ) - return stack - else: - def backtrace(): - return None - - def symbolize_backtrace(trace): - return None - - @_openssl.ffi.callback("void *(size_t, const char *, int)") - def malloc(size, path, line): - ptr = _openssl.lib.Cryptography_malloc_wrapper(size, path, line) - heap[ptr] = (size, path, line, backtrace()) - return ptr - - @_openssl.ffi.callback("void *(void *, size_t, const char *, int)") - def realloc(ptr, size, path, line): - if ptr != _openssl.ffi.NULL: - del heap[ptr] - new_ptr = _openssl.lib.Cryptography_realloc_wrapper( - ptr, size, path, line - ) - heap[new_ptr] = (size, path, line, backtrace()) - - # It is possible that something during the test will cause a - # realloc of memory allocated during the startup phase. (This - # was observed in conda-forge Windows builds of this package with - # provider operation_bits pointers in crypto/provider_core.c.) If - # we don't pay attention to that, the realloc'ed pointer will show - # up as a leak; but we also don't want to allow this kind of realloc - # to consume large amounts of additional memory. So we track the - # realloc and the change in memory consumption. - startup_info = start_heap.pop(ptr, None) - if startup_info is not None: - start_heap[new_ptr] = heap[new_ptr] - start_heap_realloc_delta[0] += size - startup_info[0] - - return new_ptr - - @_openssl.ffi.callback("void(void *, const char *, int)") - def free(ptr, path, line): - if ptr != _openssl.ffi.NULL: - del heap[ptr] - _openssl.lib.Cryptography_free_wrapper(ptr, path, line) - - result = _openssl.lib.Cryptography_CRYPTO_set_mem_functions( - malloc, realloc, free - ) - assert result == 1 - - # Trigger a bunch of initialization stuff. - import hashlib - from cryptography.hazmat.backends.openssl.backend import backend - - hashlib.sha256() - - start_heap.update(heap) - - try: - func(*argv[1:]) - finally: - gc.collect() - gc.collect() - gc.collect() - - if _openssl.lib.CRYPTOGRAPHY_OPENSSL_300_OR_GREATER: - _openssl.lib.OSSL_PROVIDER_unload(backend._binding._legacy_provider) - _openssl.lib.OSSL_PROVIDER_unload(backend._binding._default_provider) - - _openssl.lib.OPENSSL_cleanup() - - # Swap back to the original functions so that if OpenSSL tries to free - # something from its atexit handle it won't be going through a Python - # function, which will be deallocated when this function returns - result = _openssl.lib.Cryptography_CRYPTO_set_mem_functions( - _openssl.ffi.addressof( - _openssl.lib, "Cryptography_malloc_wrapper" - ), - _openssl.ffi.addressof( - _openssl.lib, "Cryptography_realloc_wrapper" - ), - _openssl.ffi.addressof(_openssl.lib, "Cryptography_free_wrapper"), - ) - assert result == 1 - - remaining = set(heap) - set(start_heap) - - # The constant here is the number of additional bytes of memory - # consumption that are allowed in reallocs of start_heap memory. - if remaining or start_heap_realloc_delta[0] > 3072: - info = dict( - (int(_openssl.ffi.cast("size_t", ptr)), { - "size": heap[ptr][0], - "path": _openssl.ffi.string(heap[ptr][1]).decode(), - "line": heap[ptr][2], - "backtrace": symbolize_backtrace(heap[ptr][3]), - }) - for ptr in remaining - ) - info["start_heap_realloc_delta"] = start_heap_realloc_delta[0] - sys.stdout.write(json.dumps(info)) - sys.stdout.flush() - sys.exit(255) - -main(sys.argv) -""" - - -def assert_no_memory_leaks(s, argv=[]): - env = os.environ.copy() - env["PYTHONPATH"] = os.pathsep.join(sys.path) - - # When using pytest-cov it attempts to instrument subprocesses. This - # causes the memleak tests to raise exceptions. - # we don't need coverage so we remove the env vars. - env.pop("COV_CORE_CONFIG", None) - env.pop("COV_CORE_DATAFILE", None) - env.pop("COV_CORE_SOURCE", None) - - argv = [sys.executable, "-c", f"{s}\n\n{MEMORY_LEAK_SCRIPT}", *argv] - # Shell out to a fresh Python process because OpenSSL does not allow you to - # install new memory hooks after the first malloc/free occurs. - proc = subprocess.Popen( - argv, - env=env, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE, - ) - assert proc.stdout is not None - assert proc.stderr is not None - try: - proc.wait() - if proc.returncode == 255: - # 255 means there was a leak, load the info about what mallocs - # weren't freed. - out = json.loads(proc.stdout.read().decode()) - raise AssertionError(out) - elif proc.returncode != 0: - # Any exception type will do to be honest - raise ValueError(proc.stdout.read(), proc.stderr.read()) - finally: - proc.stdout.close() - proc.stderr.close() - - -def skip_if_memtesting_not_supported(): - return pytest.mark.skipif( - not Binding().lib.Cryptography_HAS_MEM_FUNCTIONS - or platform.python_implementation() == "PyPy", - reason="Requires OpenSSL memory functions (>=1.1.0) and not PyPy", - ) - - -@pytest.mark.skip_fips(reason="FIPS self-test sets allow_customize = 0") -@skip_if_memtesting_not_supported() -class TestAssertNoMemoryLeaks: - def test_no_leak_no_malloc(self): - assert_no_memory_leaks( - textwrap.dedent( - """ - def func(): - pass - """ - ) - ) - - def test_no_leak_free(self): - assert_no_memory_leaks( - textwrap.dedent( - """ - def func(): - from cryptography.hazmat.bindings.openssl.binding import Binding - b = Binding() - name = b.lib.X509_NAME_new() - b.lib.X509_NAME_free(name) - """ - ) - ) - - def test_no_leak_gc(self): - assert_no_memory_leaks( - textwrap.dedent( - """ - def func(): - from cryptography.hazmat.bindings.openssl.binding import Binding - b = Binding() - name = b.lib.X509_NAME_new() - b.ffi.gc(name, b.lib.X509_NAME_free) - """ - ) - ) - - def test_leak(self): - with pytest.raises(AssertionError): - assert_no_memory_leaks( - textwrap.dedent( - """ - def func(): - from cryptography.hazmat.bindings.openssl.binding import ( - Binding - ) - b = Binding() - b.lib.X509_NAME_new() - """ - ) - ) - - def test_errors(self): - with pytest.raises(ValueError, match="ZeroDivisionError"): - assert_no_memory_leaks( - textwrap.dedent( - """ - def func(): - raise ZeroDivisionError - """ - ) - ) - - -@pytest.mark.skip_fips(reason="FIPS self-test sets allow_customize = 0") -@skip_if_memtesting_not_supported() -class TestOpenSSLMemoryLeaks: - def test_ec_private_numbers_private_key(self): - assert_no_memory_leaks( - textwrap.dedent( - """ - def func(): - from cryptography.hazmat.backends.openssl import backend - from cryptography.hazmat.primitives.asymmetric import ec - - ec.EllipticCurvePrivateNumbers( - private_value=int( - '280814107134858470598753916394807521398239633534281633982576099083' - '35787109896602102090002196616273211495718603965098' - ), - public_numbers=ec.EllipticCurvePublicNumbers( - curve=ec.SECP384R1(), - x=int( - '10036914308591746758780165503819213553101287571902957054148542' - '504671046744460374996612408381962208627004841444205030' - ), - y=int( - '17337335659928075994560513699823544906448896792102247714689323' - '575406618073069185107088229463828921069465902299522926' - ) - ) - ).private_key(backend) - """ - ) - ) - - def test_ec_derive_private_key(self): - assert_no_memory_leaks( - textwrap.dedent( - """ - def func(): - from cryptography.hazmat.backends.openssl import backend - from cryptography.hazmat.primitives.asymmetric import ec - ec.derive_private_key(1, ec.SECP256R1(), backend) - """ - ) - ) - - def test_x25519_pubkey_from_private_key(self): - assert_no_memory_leaks( - textwrap.dedent( - """ - def func(): - from cryptography.hazmat.primitives.asymmetric import x25519 - private_key = x25519.X25519PrivateKey.generate() - private_key.public_key() - """ - ) - ) - - @pytest.mark.parametrize( - "path", - ["pkcs12/cert-aes256cbc-no-key.p12", "pkcs12/cert-key-aes256cbc.p12"], - ) - def test_load_pkcs12_key_and_certificates(self, path): - assert_no_memory_leaks( - textwrap.dedent( - """ - def func(path): - from cryptography import x509 - from cryptography.hazmat.backends.openssl import backend - from cryptography.hazmat.primitives.serialization import pkcs12 - import cryptography_vectors - - with cryptography_vectors.open_vector_file(path, "rb") as f: - pkcs12.load_key_and_certificates( - f.read(), b"cryptography", backend - ) - """ - ), - [path], - ) - - def test_write_pkcs12_key_and_certificates(self): - assert_no_memory_leaks( - textwrap.dedent( - """ - def func(): - import os - from cryptography import x509 - from cryptography.hazmat.backends.openssl import backend - from cryptography.hazmat.primitives import serialization - from cryptography.hazmat.primitives.serialization import pkcs12 - import cryptography_vectors - - path = os.path.join('x509', 'custom', 'ca', 'ca.pem') - with cryptography_vectors.open_vector_file(path, "rb") as f: - cert = x509.load_pem_x509_certificate( - f.read(), backend - ) - path2 = os.path.join('x509', 'custom', 'dsa_selfsigned_ca.pem') - with cryptography_vectors.open_vector_file(path2, "rb") as f: - cert2 = x509.load_pem_x509_certificate( - f.read(), backend - ) - path3 = os.path.join('x509', 'letsencryptx3.pem') - with cryptography_vectors.open_vector_file(path3, "rb") as f: - cert3 = x509.load_pem_x509_certificate( - f.read(), backend - ) - key_path = os.path.join("x509", "custom", "ca", "ca_key.pem") - with cryptography_vectors.open_vector_file(key_path, "rb") as f: - key = serialization.load_pem_private_key( - f.read(), None, backend - ) - encryption = serialization.NoEncryption() - pkcs12.serialize_key_and_certificates( - b"name", key, cert, [cert2, cert3], encryption) - """ - ) - ) diff --git a/tests/hazmat/bindings/test_openssl.py b/tests/hazmat/bindings/test_openssl.py index 64c3cfdec05c..ef45b304b4ef 100644 --- a/tests/hazmat/bindings/test_openssl.py +++ b/tests/hazmat/bindings/test_openssl.py @@ -8,7 +8,6 @@ from cryptography.hazmat.bindings._rust import openssl as rust_openssl from cryptography.hazmat.bindings.openssl.binding import ( Binding, - _legacy_provider_error, _openssl_assert, _verify_package_version, ) @@ -84,12 +83,6 @@ def test_version_mismatch(self): with pytest.raises(ImportError): _verify_package_version("nottherightversion") - def test_legacy_provider_error(self): - with pytest.raises(RuntimeError): - _legacy_provider_error(False) - - _legacy_provider_error(True) - def test_rust_internal_error(self): with pytest.raises(InternalError) as exc_info: rust_openssl.raise_openssl_error() From 396bcf64c5be826ec00e7d7f45838c858c049cbc Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Thu, 15 Feb 2024 19:01:07 -0800 Subject: [PATCH 2/7] fix provider loading take two (#10390) (#10395) we previously hoisted this into rust, but we used the try_load feature which supposedly retains fallbacks. Something about that doesn't behave the way we expect though and the machinery in providers is sufficiently complex that we are just going to load the default provider explicitly. this matches our behavior pre-rust. --- src/rust/src/lib.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/rust/src/lib.rs b/src/rust/src/lib.rs index c9f9285e3825..9308e0c81c17 100644 --- a/src/rust/src/lib.rs +++ b/src/rust/src/lib.rs @@ -24,6 +24,7 @@ mod x509; #[pyo3::prelude::pyclass(frozen, module = "cryptography.hazmat.bindings._rust")] struct LoadedProviders { legacy: Option, + _default: provider::Provider, } #[pyo3::prelude::pyfunction] @@ -37,7 +38,7 @@ fn is_fips_enabled() -> bool { } #[cfg(CRYPTOGRAPHY_OPENSSL_300_OR_GREATER)] -fn _initialize_legacy_provider() -> CryptographyResult { +fn _initialize_providers() -> CryptographyResult { // 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 @@ -47,13 +48,14 @@ fn _initialize_legacy_provider() -> CryptographyResult { .map(|v| v.is_empty() || v == "0") .unwrap_or(true); let legacy = if load_legacy { - let legacy_result = provider::Provider::try_load(None, "legacy", true); + let legacy_result = provider::Provider::load(None, "legacy"); _legacy_provider_error(legacy_result.is_ok())?; Some(legacy_result?) } else { None }; - Ok(LoadedProviders { legacy }) + let _default = provider::Provider::load(None, "default")?; + Ok(LoadedProviders { legacy, _default }) } fn _legacy_provider_error(success: bool) -> pyo3::PyResult<()> { @@ -94,13 +96,13 @@ fn _rust(py: pyo3::Python<'_>, m: &pyo3::types::PyModule) -> pyo3::PyResult<()> let openssl_mod = pyo3::prelude::PyModule::new(py, "openssl")?; cfg_if::cfg_if! { if #[cfg(CRYPTOGRAPHY_OPENSSL_300_OR_GREATER)] { - let providers = _initialize_legacy_provider()?; + let providers = _initialize_providers()?; if providers.legacy.is_some() { openssl_mod.add("_legacy_provider_loaded", true)?; - openssl_mod.add("_providers", providers)?; } else { openssl_mod.add("_legacy_provider_loaded", false)?; } + openssl_mod.add("_providers", providers)?; } else { // default value for non-openssl 3+ openssl_mod.add("_legacy_provider_loaded", false)?; From c49a7a5271178c6e8ef36fa1c499f62c63ec19b9 Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Thu, 15 Feb 2024 19:41:19 -0800 Subject: [PATCH 3/7] changelog and version bump for 42.0.3 (#10396) --- CHANGELOG.rst | 8 ++++++++ pyproject.toml | 2 +- src/cryptography/__about__.py | 2 +- vectors/cryptography_vectors/__about__.py | 2 +- vectors/pyproject.toml | 2 +- 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 393ba2aa8835..f895ac2eee95 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,14 @@ Changelog ========= +.. _v42-0-3: + +42.0.3 - 2024-02-15 +~~~~~~~~~~~~~~~~~~~ + +* Fixed an initialization issue that caused key loading failures for some + users. + .. _v42-0-2: 42.0.2 - 2024-01-30 diff --git a/pyproject.toml b/pyproject.toml index c9a7979bdcc4..38549872dd59 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,7 @@ build-backend = "setuptools.build_meta" [project] name = "cryptography" -version = "42.0.2" +version = "42.0.3" authors = [ {name = "The Python Cryptographic Authority and individual contributors", email = "cryptography-dev@python.org"} ] diff --git a/src/cryptography/__about__.py b/src/cryptography/__about__.py index 5b1b9783549c..b3b68cc74880 100644 --- a/src/cryptography/__about__.py +++ b/src/cryptography/__about__.py @@ -10,7 +10,7 @@ "__copyright__", ] -__version__ = "42.0.2" +__version__ = "42.0.3" __author__ = "The Python Cryptographic Authority and individual contributors" diff --git a/vectors/cryptography_vectors/__about__.py b/vectors/cryptography_vectors/__about__.py index c3ca4f6e267b..4a3177ec60eb 100644 --- a/vectors/cryptography_vectors/__about__.py +++ b/vectors/cryptography_vectors/__about__.py @@ -6,4 +6,4 @@ "__version__", ] -__version__ = "42.0.2" +__version__ = "42.0.3" diff --git a/vectors/pyproject.toml b/vectors/pyproject.toml index 69c3511e0e73..6310fb8c485b 100644 --- a/vectors/pyproject.toml +++ b/vectors/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "flit_core.buildapi" [project] name = "cryptography_vectors" -version = "42.0.2" +version = "42.0.3" authors = [ {name = "The Python Cryptographic Authority and individual contributors", email = "cryptography-dev@python.org"} ] From df314bb182bdfd661333969a94325e4680d785f6 Mon Sep 17 00:00:00 2001 From: Paul Kehrer Date: Sat, 17 Feb 2024 21:20:20 -0800 Subject: [PATCH 4/7] backport actions m1 switch to 42.0.x (#10415) * Check to see if we can use the hosted M1 runners (#10340) * Stop pretending to be x64 on M1 in CI (#10341) --------- Co-authored-by: Alex Gaynor --- .github/workflows/ci.yml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4db52079b000..8a58cdb1d3af 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -222,14 +222,14 @@ jobs: matrix: RUNNER: - {OS: 'macos-13', ARCH: 'x86_64'} - - {OS: [self-hosted, macos, ARM64, tart], ARCH: 'arm64'} + - {OS: 'macos-14', ARCH: 'arm64'} PYTHON: - {VERSION: "3.7", NOXSESSION: "tests-nocoverage"} - {VERSION: "3.12", NOXSESSION: "tests"} exclude: # We only test latest Python on arm64. py37 won't work since there's no universal2 binary - PYTHON: {VERSION: "3.7", NOXSESSION: "tests-nocoverage"} - RUNNER: {OS: [self-hosted, macos, ARM64, tart], ARCH: 'arm64'} + RUNNER: {OS: 'macos-14', ARCH: 'arm64'} timeout-minutes: 15 steps: - uses: actions/checkout@f43a0e5ff2bd294095638e18286ca9a3d1956744 # v3.6.0 @@ -246,7 +246,6 @@ jobs: uses: actions/setup-python@0a5c61591373683505ea898e09a3ea4f39ef2b9c # v5.0.0 with: python-version: ${{ matrix.PYTHON.VERSION }} - architecture: 'x64' # we force this right now so that it will install the universal2 on arm64 cache: pip cache-dependency-path: ci-constraints-requirements.txt timeout-minutes: 3 From 7a4d012991061974da5d9cb7614de65eac94f49b Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Mon, 19 Feb 2024 12:09:10 -0500 Subject: [PATCH 5/7] Fixes #10422 -- don't crash when a PKCS#12 key and cert don't match (#10423) (#10425) --- .../hazmat/backends/openssl/backend.py | 9 +++++++++ tests/hazmat/primitives/test_pkcs12.py | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py index 5cd1103bd133..66053af7f937 100644 --- a/src/cryptography/hazmat/backends/openssl/backend.py +++ b/src/cryptography/hazmat/backends/openssl/backend.py @@ -826,6 +826,15 @@ def serialize_key_and_certificates_to_pkcs12( mac_iter, 0, ) + if p12 == self._ffi.NULL: + errors = self._consume_errors() + raise ValueError( + ( + "Failed to create PKCS12 (does the key match the " + "certificate?)" + ), + errors, + ) if ( self._lib.Cryptography_HAS_PKCS12_SET_MAC diff --git a/tests/hazmat/primitives/test_pkcs12.py b/tests/hazmat/primitives/test_pkcs12.py index cd9c279ac4b0..cd7bcafc6e73 100644 --- a/tests/hazmat/primitives/test_pkcs12.py +++ b/tests/hazmat/primitives/test_pkcs12.py @@ -657,6 +657,24 @@ def test_key_serialization_encryption_set_mac_unsupported( b"name", cakey, cacert, [], algorithm ) + @pytest.mark.supported( + only_if=lambda backend: backend._lib.Cryptography_HAS_PKCS12_SET_MAC, + skip_message="Requires OpenSSL with PKCS12_set_mac", + ) + def test_set_mac_key_certificate_mismatch(self, backend): + cacert, _ = _load_ca(backend) + key = ec.generate_private_key(ec.SECP256R1()) + encryption = ( + serialization.PrivateFormat.PKCS12.encryption_builder() + .hmac_hash(hashes.SHA256()) + .build(b"password") + ) + + with pytest.raises(ValueError): + serialize_key_and_certificates( + b"name", key, cacert, [], encryption + ) + @pytest.mark.skip_fips( reason="PKCS12 unsupported in FIPS mode. So much bad crypto in it." From aaa2dd06ed470695de818405a982d4c459869803 Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 20 Feb 2024 20:53:59 -0500 Subject: [PATCH 6/7] Fix ASN.1 issues in PKCS#7 and S/MIME signing (#10373) (#10442) * Fix ASN.1 for S/MIME capabilities. The current implementation defines the SMIMECapabilities attribute so that its value is a SEQUENCE of all the algorithm OIDs that are supported. However, the S/MIME v3 spec (RFC 2633) specifies that each algorithm should be specified in its own SEQUENCE: SMIMECapabilities ::= SEQUENCE OF SMIMECapability SMIMECapability ::= SEQUENCE { capabilityID OBJECT IDENTIFIER, parameters ANY DEFINED BY capabilityID OPTIONAL } (RFC 2633, Appendix A) This commit changes the implementation so that each algorithm is inside its own SEQUENCE. This also matches the OpenSSL implementation. * Fix the RSA OID used for signing PKCS#7/SMIME The current implementation computes the algorithm identifier used in the `digest_encryption_algorithm` PKCS#7 field (or `SignatureAlgorithmIdentifier` in S/MIME) based on both the algorithm used to sign (e.g. RSA) and the digest algorithm (e.g. SHA512). This is correct for ECDSA signatures, where the OIDs used include the digest algorithm (e.g: ecdsa-with-SHA512). However, due to historical reasons, when signing with RSA the OID specified should be the one corresponding to just RSA ("1.2.840.113549.1.1.1" rsaEncryption), rather than OIDs which also include the digest algorithm (such as "1.2.840.113549.1.1.13", sha512WithRSAEncryption). This means that the logic to compute the algorithm identifier is the same except when signing with RSA, in which case the OID will always be `rsaEncryption`. This is consistent with the OpenSSL implementation, and the RFCs that define PKCS#7 and S/MIME. See RFC 3851 (section 2.2), and RFC 3370 (section 3.2) for more details. * Add tests for the changes in PKCS7 signing * PKCS7 fixes from code review * Update CHANGELOG Co-authored-by: Facundo Tuesca --- CHANGELOG.rst | 9 +++++ src/rust/src/pkcs7.rs | 28 ++++++++++++-- src/rust/src/x509/sign.rs | 5 ++- tests/hazmat/primitives/test_pkcs7.py | 54 ++++++++++++++++++++++++++- 4 files changed, 89 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index f895ac2eee95..7fb4a46047f5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,15 @@ Changelog ========= +.. _v42-0-4: + +42.0.4 - 2024-02-20 +~~~~~~~~~~~~~~~~~~~ + +* Fixed ASN.1 encoding for PKCS7/SMIME signed messages. The fields ``SMIMECapabilities`` + and ``SignatureAlgorithmIdentifier`` should now be correctly encoded according to the + definitions in :rfc:`2633` :rfc:`3370`. + .. _v42-0-3: 42.0.3 - 2024-02-15 diff --git a/src/rust/src/pkcs7.rs b/src/rust/src/pkcs7.rs index f307cf483ad7..711018793fb9 100644 --- a/src/rust/src/pkcs7.rs +++ b/src/rust/src/pkcs7.rs @@ -104,9 +104,9 @@ fn sign_and_serialize<'p>( // Subset of values OpenSSL provides: // https://github.com/openssl/openssl/blob/667a8501f0b6e5705fd611d5bb3ca24848b07154/crypto/pkcs7/pk7_smime.c#L150 // removing all the ones that are bad cryptography - AES_256_CBC_OID, - AES_192_CBC_OID, - AES_128_CBC_OID, + &asn1::SequenceOfWriter::new([AES_256_CBC_OID]), + &asn1::SequenceOfWriter::new([AES_192_CBC_OID]), + &asn1::SequenceOfWriter::new([AES_128_CBC_OID]), ]))?; let py_signers: Vec<( @@ -205,7 +205,7 @@ fn sign_and_serialize<'p>( }, digest_algorithm: digest_alg, authenticated_attributes: authenticated_attrs, - digest_encryption_algorithm: x509::sign::compute_signature_algorithm( + digest_encryption_algorithm: compute_pkcs7_signature_algorithm( py, py_private_key, py_hash_alg, @@ -262,6 +262,26 @@ fn sign_and_serialize<'p>( } } +fn compute_pkcs7_signature_algorithm<'p>( + py: pyo3::Python<'p>, + private_key: &'p pyo3::PyAny, + hash_algorithm: &'p pyo3::PyAny, + rsa_padding: &'p pyo3::PyAny, +) -> pyo3::PyResult> { + let key_type = x509::sign::identify_key_type(py, private_key)?; + let has_pss_padding = rsa_padding.is_instance(types::PSS.get(py)?)?; + // For RSA signatures (with no PSS padding), the OID is always the same no matter the + // digest algorithm. See RFC 3370 (section 3.2). + if key_type == x509::sign::KeyType::Rsa && !has_pss_padding { + Ok(common::AlgorithmIdentifier { + oid: asn1::DefinedByMarker::marker(), + params: common::AlgorithmParameters::Rsa(Some(())), + }) + } else { + x509::sign::compute_signature_algorithm(py, private_key, hash_algorithm, rsa_padding) + } +} + fn smime_canonicalize(data: &[u8], text_mode: bool) -> (Cow<'_, [u8]>, Cow<'_, [u8]>) { let mut new_data_with_header = vec![]; let mut new_data_without_header = vec![]; diff --git a/src/rust/src/x509/sign.rs b/src/rust/src/x509/sign.rs index 4d9637d1f2de..5e38af2e2e79 100644 --- a/src/rust/src/x509/sign.rs +++ b/src/rust/src/x509/sign.rs @@ -48,7 +48,10 @@ enum HashType { Sha3_512, } -fn identify_key_type(py: pyo3::Python<'_>, private_key: &pyo3::PyAny) -> pyo3::PyResult { +pub(crate) fn identify_key_type( + py: pyo3::Python<'_>, + private_key: &pyo3::PyAny, +) -> pyo3::PyResult { if private_key.is_instance(types::RSA_PRIVATE_KEY.get(py)?)? { Ok(KeyType::Rsa) } else if private_key.is_instance(types::DSA_PRIVATE_KEY.get(py)?)? { diff --git a/tests/hazmat/primitives/test_pkcs7.py b/tests/hazmat/primitives/test_pkcs7.py index 03b04cd389e5..685b7c940ad7 100644 --- a/tests/hazmat/primitives/test_pkcs7.py +++ b/tests/hazmat/primitives/test_pkcs7.py @@ -557,6 +557,50 @@ def test_sign_text(self, backend): backend, ) + def test_smime_capabilities(self, backend): + data = b"hello world" + cert, key = _load_cert_key() + builder = ( + pkcs7.PKCS7SignatureBuilder() + .set_data(data) + .add_signer(cert, key, hashes.SHA256()) + ) + + sig_binary = builder.sign(serialization.Encoding.DER, []) + + # 1.2.840.113549.1.9.15 (SMIMECapabilities) as an ASN.1 DER encoded OID + assert b"\x06\t*\x86H\x86\xf7\r\x01\t\x0f" in sig_binary + + # 2.16.840.1.101.3.4.1.42 (aes256-CBC-PAD) as an ASN.1 DER encoded OID + aes256_cbc_pad_oid = b"\x06\x09\x60\x86\x48\x01\x65\x03\x04\x01\x2A" + # 2.16.840.1.101.3.4.1.22 (aes192-CBC-PAD) as an ASN.1 DER encoded OID + aes192_cbc_pad_oid = b"\x06\x09\x60\x86\x48\x01\x65\x03\x04\x01\x16" + # 2.16.840.1.101.3.4.1.2 (aes128-CBC-PAD) as an ASN.1 DER encoded OID + aes128_cbc_pad_oid = b"\x06\x09\x60\x86\x48\x01\x65\x03\x04\x01\x02" + + # Each algorithm in SMIMECapabilities should be inside its own + # SEQUENCE. + # This is encoded as SEQUENCE_IDENTIFIER + LENGTH + ALGORITHM_OID. + # This tests that each algorithm is indeed encoded inside its own + # sequence. See RFC 2633, Appendix A for more details. + sequence_identifier = b"\x30" + for oid in [ + aes256_cbc_pad_oid, + aes192_cbc_pad_oid, + aes128_cbc_pad_oid, + ]: + len_oid = len(oid).to_bytes(length=1, byteorder="big") + assert sequence_identifier + len_oid + oid in sig_binary + + _pkcs7_verify( + serialization.Encoding.DER, + sig_binary, + None, + [cert], + [], + backend, + ) + def test_sign_no_capabilities(self, backend): data = b"hello world" cert, key = _load_cert_key() @@ -677,9 +721,15 @@ def test_rsa_pkcs_padding_options(self, pad, backend): sig.count(b"\x06\x09\x2a\x86\x48\x86\xf7\x0d\x01\x01\x08") == 1 ) else: - # This should be a pkcs1 sha512 signature + # This should be a pkcs1 RSA signature, which uses the + # `rsaEncryption` OID (1.2.840.113549.1.1.1) no matter which + # digest algorithm is used. + # See RFC 3370 section 3.2 for more details. + # This OID appears twice, once in the certificate itself and + # another in the SignerInfo data structure in the + # `digest_encryption_algorithm` field. assert ( - sig.count(b"\x06\x09\x2A\x86\x48\x86\xF7\x0D\x01\x01\x0D") == 1 + sig.count(b"\x06\x09\x2A\x86\x48\x86\xF7\x0D\x01\x01\x01") == 2 ) _pkcs7_verify( serialization.Encoding.DER, From fe18470f7d05f963e7267e34fdf985d81ea6ceea Mon Sep 17 00:00:00 2001 From: Alex Gaynor Date: Tue, 20 Feb 2024 21:48:23 -0500 Subject: [PATCH 7/7] Bump for 42.0.4 release (#10445) --- CHANGELOG.rst | 3 +++ pyproject.toml | 2 +- src/cryptography/__about__.py | 2 +- vectors/cryptography_vectors/__about__.py | 2 +- vectors/pyproject.toml | 2 +- 5 files changed, 7 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7fb4a46047f5..b32b234c5d33 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -6,6 +6,9 @@ Changelog 42.0.4 - 2024-02-20 ~~~~~~~~~~~~~~~~~~~ +* Fixed a null-pointer-dereference and segfault that could occur when creating + a PKCS#12 bundle. Credit to **Alexander-Programming** for reporting the + issue. **CVE-2024-26130** * Fixed ASN.1 encoding for PKCS7/SMIME signed messages. The fields ``SMIMECapabilities`` and ``SignatureAlgorithmIdentifier`` should now be correctly encoded according to the definitions in :rfc:`2633` :rfc:`3370`. diff --git a/pyproject.toml b/pyproject.toml index 38549872dd59..6fce9b3403e7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -12,7 +12,7 @@ build-backend = "setuptools.build_meta" [project] name = "cryptography" -version = "42.0.3" +version = "42.0.4" authors = [ {name = "The Python Cryptographic Authority and individual contributors", email = "cryptography-dev@python.org"} ] diff --git a/src/cryptography/__about__.py b/src/cryptography/__about__.py index b3b68cc74880..b66092523d7d 100644 --- a/src/cryptography/__about__.py +++ b/src/cryptography/__about__.py @@ -10,7 +10,7 @@ "__copyright__", ] -__version__ = "42.0.3" +__version__ = "42.0.4" __author__ = "The Python Cryptographic Authority and individual contributors" diff --git a/vectors/cryptography_vectors/__about__.py b/vectors/cryptography_vectors/__about__.py index 4a3177ec60eb..e2d51b38c0f4 100644 --- a/vectors/cryptography_vectors/__about__.py +++ b/vectors/cryptography_vectors/__about__.py @@ -6,4 +6,4 @@ "__version__", ] -__version__ = "42.0.3" +__version__ = "42.0.4" diff --git a/vectors/pyproject.toml b/vectors/pyproject.toml index 6310fb8c485b..48cab55c51d1 100644 --- a/vectors/pyproject.toml +++ b/vectors/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "flit_core.buildapi" [project] name = "cryptography_vectors" -version = "42.0.3" +version = "42.0.4" authors = [ {name = "The Python Cryptographic Authority and individual contributors", email = "cryptography-dev@python.org"} ]