diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8a0196bff2d8..3156ee365f8e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -27,6 +27,14 @@ Changelog other X.509 builders has been removed. * Added support for :ref:`disabling the legacy provider in OpenSSL 3.0.x`. +* Added support for disabling RSA key validation checks when loading RSA + keys via + :func:`~cryptography.hazmat.primitives.serialization.load_pem_private_key`, + :func:`~cryptography.hazmat.primitives.serialization.load_der_private_key`, + and + :meth:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateNumbers.private_key`. + This speeds up key loading but is :term:`unsafe` if you are loading potentially + attacker supplied keys. .. _v38-0-1: diff --git a/docs/glossary.rst b/docs/glossary.rst index b85a61091e38..0fa40245d1b8 100644 --- a/docs/glossary.rst +++ b/docs/glossary.rst @@ -100,6 +100,11 @@ Glossary name. U-labels use unicode characters outside the ASCII range and are encoded as A-labels when stored in certificates. + unsafe + This is a term used to describe an operation where the user must + ensure that the input is correct. Failure to do so can result in + crashes, hangs, and other security issues. + .. _`hardware security module`: https://en.wikipedia.org/wiki/Hardware_security_module .. _`idna`: https://pypi.org/project/idna/ .. _`buffer protocol`: https://docs.python.org/3/c-api/buffer.html diff --git a/docs/hazmat/primitives/asymmetric/rsa.rst b/docs/hazmat/primitives/asymmetric/rsa.rst index d21cb801275f..ebdc56d18de3 100644 --- a/docs/hazmat/primitives/asymmetric/rsa.rst +++ b/docs/hazmat/primitives/asymmetric/rsa.rst @@ -473,7 +473,21 @@ is unavailable. A `Chinese remainder theorem`_ coefficient used to speed up RSA operations. Calculated as: q\ :sup:`-1` mod p - .. method:: private_key() + .. method:: private_key(*, unsafe_skip_rsa_key_validation=False) + + :param unsafe_skip_rsa_key_validation: + + .. versionadded:: 39.0.0 + + A ``kwarg`` only argument that defaults to ``False``. If ``True`` + RSA private keys will not be validated. This significantly speeds up + loading the keys, but is is :term:`unsafe` unless you are certain + the key is valid. User supplied keys should never be loaded with + this parameter set to ``True``. If you do load an invalid key this + way and attempt to use it OpenSSL may hang, crash, or otherwise + misbehave. + + :type unsafe_skip_rsa_key_validation: bool :returns: An instance of :class:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKey`. diff --git a/docs/hazmat/primitives/asymmetric/serialization.rst b/docs/hazmat/primitives/asymmetric/serialization.rst index db3271b90d3c..b1669c893bbc 100644 --- a/docs/hazmat/primitives/asymmetric/serialization.rst +++ b/docs/hazmat/primitives/asymmetric/serialization.rst @@ -125,7 +125,7 @@ all begin with ``-----BEGIN {format}-----`` and end with ``-----END extract the public key with :meth:`Certificate.public_key `. -.. function:: load_pem_private_key(data, password) +.. function:: load_pem_private_key(data, password, *, unsafe_skip_rsa_key_validation=False) .. versionadded:: 0.6 @@ -141,7 +141,20 @@ all begin with ``-----BEGIN {format}-----`` and end with ``-----END :param password: The password to use to decrypt the data. Should be ``None`` if the private key is not encrypted. - :type data: :term:`bytes-like` + :type password: :term:`bytes-like` + + :param unsafe_skip_rsa_key_validation: + + .. versionadded:: 39.0.0 + + A ``kwarg`` only argument that defaults to ``False``. If ``True`` + RSA private keys will not be validated. This significantly speeds up + loading the keys, but is is :term:`unsafe` unless you are certain the + key is valid. User supplied keys should never be loaded with this + parameter set to ``True``. If you do load an invalid key this way and + attempt to use it OpenSSL may hang, crash, or otherwise misbehave. + + :type unsafe_skip_rsa_key_validation: bool :returns: One of :class:`~cryptography.hazmat.primitives.asymmetric.ed25519.Ed25519PrivateKey`, @@ -234,7 +247,7 @@ data is binary. DER keys may be in a variety of formats, but as long as you know whether it is a public or private key the loading functions will handle the rest. -.. function:: load_der_private_key(data, password) +.. function:: load_der_private_key(data, password, *, unsafe_skip_rsa_key_validation=False) .. versionadded:: 0.8 @@ -248,6 +261,19 @@ the rest. be ``None`` if the private key is not encrypted. :type password: :term:`bytes-like` + :param unsafe_skip_rsa_key_validation: + + .. versionadded:: 39.0.0 + + A ``kwarg`` only argument that defaults to ``False``. If ``True`` + RSA private keys will not be validated. This significantly speeds up + loading the keys, but is is :term:`unsafe` unless you are certain the + key is valid. User supplied keys should never be loaded with this + parameter set to ``True``. If you do load an invalid key this way and + attempt to use it OpenSSL may hang, crash, or otherwise misbehave. + + :type unsafe_skip_rsa_key_validation: bool + :returns: One of :class:`~cryptography.hazmat.primitives.asymmetric.ed25519.Ed25519PrivateKey`, :class:`~cryptography.hazmat.primitives.asymmetric.x25519.X25519PrivateKey`, diff --git a/src/cryptography/hazmat/backends/openssl/backend.py b/src/cryptography/hazmat/backends/openssl/backend.py index 890e2f9521a9..1557ff122bef 100644 --- a/src/cryptography/hazmat/backends/openssl/backend.py +++ b/src/cryptography/hazmat/backends/openssl/backend.py @@ -542,8 +542,9 @@ def generate_rsa_private_key( self.openssl_assert(res == 1) evp_pkey = self._rsa_cdata_to_evp_pkey(rsa_cdata) + # We can skip RSA key validation here since we just generated the key return _RSAPrivateKey( - self, rsa_cdata, evp_pkey, self._rsa_skip_check_key + self, rsa_cdata, evp_pkey, unsafe_skip_rsa_key_validation=True ) def generate_rsa_parameters_supported( @@ -556,7 +557,9 @@ def generate_rsa_parameters_supported( ) def load_rsa_private_numbers( - self, numbers: rsa.RSAPrivateNumbers + self, + numbers: rsa.RSAPrivateNumbers, + unsafe_skip_rsa_key_validation: bool, ) -> rsa.RSAPrivateKey: rsa._check_private_key_components( numbers.p, @@ -588,7 +591,10 @@ def load_rsa_private_numbers( evp_pkey = self._rsa_cdata_to_evp_pkey(rsa_cdata) return _RSAPrivateKey( - self, rsa_cdata, evp_pkey, self._rsa_skip_check_key + self, + rsa_cdata, + evp_pkey, + unsafe_skip_rsa_key_validation=unsafe_skip_rsa_key_validation, ) def load_rsa_public_numbers( @@ -653,7 +659,9 @@ def _read_mem_bio(self, bio) -> bytes: bio_data = self._ffi.buffer(buf[0], buf_len)[:] return bio_data - def _evp_pkey_to_private_key(self, evp_pkey) -> PRIVATE_KEY_TYPES: + def _evp_pkey_to_private_key( + self, evp_pkey, unsafe_skip_rsa_key_validation: bool + ) -> PRIVATE_KEY_TYPES: """ Return the appropriate type of PrivateKey given an evp_pkey cdata pointer. @@ -666,7 +674,10 @@ def _evp_pkey_to_private_key(self, evp_pkey) -> PRIVATE_KEY_TYPES: self.openssl_assert(rsa_cdata != self._ffi.NULL) rsa_cdata = self._ffi.gc(rsa_cdata, self._lib.RSA_free) return _RSAPrivateKey( - self, rsa_cdata, evp_pkey, self._rsa_skip_check_key + self, + rsa_cdata, + evp_pkey, + unsafe_skip_rsa_key_validation=unsafe_skip_rsa_key_validation, ) elif ( key_type == self._lib.EVP_PKEY_RSA_PSS @@ -685,7 +696,9 @@ def _evp_pkey_to_private_key(self, evp_pkey) -> PRIVATE_KEY_TYPES: res = self._lib.i2d_RSAPrivateKey_bio(bio, rsa_cdata) self.openssl_assert(res == 1) return self.load_der_private_key( - self._read_mem_bio(bio), password=None + self._read_mem_bio(bio), + password=None, + unsafe_skip_rsa_key_validation=unsafe_skip_rsa_key_validation, ) elif key_type == self._lib.EVP_PKEY_DSA: dsa_cdata = self._lib.EVP_PKEY_get1_DSA(evp_pkey) @@ -932,13 +945,16 @@ def create_cmac_ctx(self, algorithm: BlockCipherAlgorithm) -> _CMACContext: return _CMACContext(self, algorithm) def load_pem_private_key( - self, data: bytes, password: typing.Optional[bytes] + self, + data: bytes, + password: typing.Optional[bytes], + unsafe_skip_rsa_key_validation: bool, ) -> PRIVATE_KEY_TYPES: return self._load_key( self._lib.PEM_read_bio_PrivateKey, - self._evp_pkey_to_private_key, data, password, + unsafe_skip_rsa_key_validation, ) def load_pem_public_key(self, data: bytes) -> PUBLIC_KEY_TYPES: @@ -996,7 +1012,10 @@ def load_pem_parameters(self, data: bytes) -> dh.DHParameters: self._handle_key_loading_error() def load_der_private_key( - self, data: bytes, password: typing.Optional[bytes] + self, + data: bytes, + password: typing.Optional[bytes], + unsafe_skip_rsa_key_validation: bool, ) -> PRIVATE_KEY_TYPES: # OpenSSL has a function called d2i_AutoPrivateKey that in theory # handles this automatically, however it doesn't handle encrypted @@ -1005,15 +1024,17 @@ def load_der_private_key( bio_data = self._bytes_to_bio(data) key = self._evp_pkey_from_der_traditional_key(bio_data, password) if key: - return self._evp_pkey_to_private_key(key) + return self._evp_pkey_to_private_key( + key, unsafe_skip_rsa_key_validation + ) else: # Finally we try to load it with the method that handles encrypted # PKCS8 properly. return self._load_key( self._lib.d2i_PKCS8PrivateKey_bio, - self._evp_pkey_to_private_key, data, password, + unsafe_skip_rsa_key_validation, ) def _evp_pkey_from_der_traditional_key(self, bio_data, password): @@ -1146,7 +1167,9 @@ def _check_keys_correspond(self, key1, key2): if self._lib.EVP_PKEY_cmp(key1._evp_pkey, key2._evp_pkey) != 1: raise ValueError("Keys do not correspond") - def _load_key(self, openssl_read_func, convert_func, data, password): + def _load_key( + self, openssl_read_func, data, password, unsafe_skip_rsa_key_validation + ): mem_bio = self._bytes_to_bio(data) userdata = self._ffi.new("CRYPTOGRAPHY_PASSWORD_DATA *") @@ -1192,7 +1215,9 @@ def _load_key(self, openssl_read_func, convert_func, data, password): password is not None and userdata.called == 1 ) or password is None - return convert_func(evp_pkey) + return self._evp_pkey_to_private_key( + evp_pkey, unsafe_skip_rsa_key_validation + ) def _handle_key_loading_error(self) -> typing.NoReturn: errors = self._consume_errors() @@ -2191,7 +2216,11 @@ def load_pkcs12( if evp_pkey_ptr[0] != self._ffi.NULL: evp_pkey = self._ffi.gc(evp_pkey_ptr[0], self._lib.EVP_PKEY_free) - key = self._evp_pkey_to_private_key(evp_pkey) + # We don't support turning off RSA key validation when loading + # PKCS12 keys + key = self._evp_pkey_to_private_key( + evp_pkey, unsafe_skip_rsa_key_validation=False + ) if x509_ptr[0] != self._ffi.NULL: x509 = self._ffi.gc(x509_ptr[0], self._lib.X509_free) diff --git a/src/cryptography/hazmat/backends/openssl/rsa.py b/src/cryptography/hazmat/backends/openssl/rsa.py index 31cff1620461..694829d2c5f1 100644 --- a/src/cryptography/hazmat/backends/openssl/rsa.py +++ b/src/cryptography/hazmat/backends/openssl/rsa.py @@ -367,7 +367,12 @@ class _RSAPrivateKey(RSAPrivateKey): _key_size: int def __init__( - self, backend: "Backend", rsa_cdata, evp_pkey, _skip_check_key: bool + self, + backend: "Backend", + rsa_cdata, + evp_pkey, + *, + unsafe_skip_rsa_key_validation: bool, ): res: int # RSA_check_key is slower in OpenSSL 3.0.0 due to improved @@ -375,7 +380,7 @@ def __init__( # since users don't load new keys constantly, but for TESTING we've # added an init arg that allows skipping the checks. You should not # use this in production code unless you understand the consequences. - if not _skip_check_key: + if not unsafe_skip_rsa_key_validation: res = backend._lib.RSA_check_key(rsa_cdata) if res != 1: errors = backend._consume_errors_with_text() diff --git a/src/cryptography/hazmat/primitives/asymmetric/rsa.py b/src/cryptography/hazmat/primitives/asymmetric/rsa.py index 5ffe767cde53..36d360f223df 100644 --- a/src/cryptography/hazmat/primitives/asymmetric/rsa.py +++ b/src/cryptography/hazmat/primitives/asymmetric/rsa.py @@ -354,12 +354,19 @@ def iqmp(self) -> int: def public_numbers(self) -> "RSAPublicNumbers": return self._public_numbers - def private_key(self, backend: typing.Any = None) -> RSAPrivateKey: + def private_key( + self, + backend: typing.Any = None, + *, + unsafe_skip_rsa_key_validation: bool = False, + ) -> RSAPrivateKey: from cryptography.hazmat.backends.openssl.backend import ( backend as ossl, ) - return ossl.load_rsa_private_numbers(self) + return ossl.load_rsa_private_numbers( + self, unsafe_skip_rsa_key_validation + ) def __eq__(self, other: object) -> bool: if not isinstance(other, RSAPrivateNumbers): diff --git a/src/cryptography/hazmat/primitives/serialization/base.py b/src/cryptography/hazmat/primitives/serialization/base.py index 059b6e40f46d..8a841766404f 100644 --- a/src/cryptography/hazmat/primitives/serialization/base.py +++ b/src/cryptography/hazmat/primitives/serialization/base.py @@ -16,10 +16,14 @@ def load_pem_private_key( data: bytes, password: typing.Optional[bytes], backend: typing.Any = None, + *, + unsafe_skip_rsa_key_validation: bool = False, ) -> PRIVATE_KEY_TYPES: from cryptography.hazmat.backends.openssl.backend import backend as ossl - return ossl.load_pem_private_key(data, password) + return ossl.load_pem_private_key( + data, password, unsafe_skip_rsa_key_validation + ) def load_pem_public_key( @@ -42,10 +46,14 @@ def load_der_private_key( data: bytes, password: typing.Optional[bytes], backend: typing.Any = None, + *, + unsafe_skip_rsa_key_validation: bool = False, ) -> PRIVATE_KEY_TYPES: from cryptography.hazmat.backends.openssl.backend import backend as ossl - return ossl.load_der_private_key(data, password) + return ossl.load_der_private_key( + data, password, unsafe_skip_rsa_key_validation + ) def load_der_public_key( diff --git a/tests/conftest.py b/tests/conftest.py index 9049922ba51f..a85b41ff9a0f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -46,12 +46,3 @@ def backend(request): # Ensure the error stack is clear after the test errors = openssl_backend._consume_errors_with_text() assert not errors - - -@pytest.fixture -def disable_rsa_checks(backend): - # Use this fixture to skip RSA key checks in tests that need the - # performance. - backend._rsa_skip_check_key = True - yield - backend._rsa_skip_check_key = False diff --git a/tests/hazmat/backends/test_openssl.py b/tests/hazmat/backends/test_openssl.py index 7830019cac6a..6f3a975bac89 100644 --- a/tests/hazmat/backends/test_openssl.py +++ b/tests/hazmat/backends/test_openssl.py @@ -477,7 +477,9 @@ def test_pem_password_cb_no_password(self): def test_unsupported_evp_pkey_type(self): key = backend._create_evp_pkey_gc() with raises_unsupported_algorithm(None): - backend._evp_pkey_to_private_key(key) + backend._evp_pkey_to_private_key( + key, unsafe_skip_rsa_key_validation=False + ) with raises_unsupported_algorithm(None): backend._evp_pkey_to_public_key(key) @@ -493,7 +495,9 @@ def test_very_long_pem_serialization_password(self): ), lambda pemfile: ( backend.load_pem_private_key( - pemfile.read().encode(), password + pemfile.read().encode(), + password, + unsafe_skip_rsa_key_validation=False, ) ), ) diff --git a/tests/hazmat/primitives/test_rsa.py b/tests/hazmat/primitives/test_rsa.py index 6f083cbcb541..5a9fa19f37b4 100644 --- a/tests/hazmat/primitives/test_rsa.py +++ b/tests/hazmat/primitives/test_rsa.py @@ -487,7 +487,7 @@ class TestRSASignature: ), skip_message="Does not support SHA1 signature.", ) - def test_pkcs1v15_signing(self, backend, disable_rsa_checks, subtests): + def test_pkcs1v15_signing(self, backend, subtests): vectors = _flatten_pkcs1_examples( load_vectors_from_file( os.path.join("asymmetric", "RSA", "pkcs1v15sign-vectors.txt"), @@ -506,7 +506,7 @@ def test_pkcs1v15_signing(self, backend, disable_rsa_checks, subtests): public_numbers=rsa.RSAPublicNumbers( e=private["public_exponent"], n=private["modulus"] ), - ).private_key(backend) + ).private_key(backend, unsafe_skip_rsa_key_validation=True) signature = private_key.sign( binascii.unhexlify(example["message"]), padding.PKCS1v15(), @@ -1682,9 +1682,7 @@ class TestRSADecryption: ), skip_message="Does not support PKCS1v1.5.", ) - def test_decrypt_pkcs1v15_vectors( - self, backend, disable_rsa_checks, subtests - ): + def test_decrypt_pkcs1v15_vectors(self, backend, subtests): vectors = _flatten_pkcs1_examples( load_vectors_from_file( os.path.join("asymmetric", "RSA", "pkcs1v15crypt-vectors.txt"), @@ -1703,7 +1701,7 @@ def test_decrypt_pkcs1v15_vectors( public_numbers=rsa.RSAPublicNumbers( e=private["public_exponent"], n=private["modulus"] ), - ).private_key(backend) + ).private_key(backend, unsafe_skip_rsa_key_validation=True) ciphertext = binascii.unhexlify(example["encryption"]) assert len(ciphertext) == (skey.key_size + 7) // 8 message = skey.decrypt(ciphertext, padding.PKCS1v15()) @@ -1804,9 +1802,7 @@ def test_decrypt_oaep_vectors(self, subtests, backend): "Does not support OAEP using SHA224 MGF1 and SHA224 hash." ), ) - def test_decrypt_oaep_sha2_vectors( - self, backend, disable_rsa_checks, subtests - ): + def test_decrypt_oaep_sha2_vectors(self, backend, subtests): vectors = _build_oaep_sha2_vectors() for private, public, example, mgf1_alg, hash_alg in vectors: with subtests.test(): @@ -1820,7 +1816,7 @@ def test_decrypt_oaep_sha2_vectors( public_numbers=rsa.RSAPublicNumbers( e=private["public_exponent"], n=private["modulus"] ), - ).private_key(backend) + ).private_key(backend, unsafe_skip_rsa_key_validation=True) message = skey.decrypt( binascii.unhexlify(example["encryption"]), padding.OAEP( diff --git a/tests/wycheproof/test_rsa.py b/tests/wycheproof/test_rsa.py index 7925a5bf91b8..0670e1c47c00 100644 --- a/tests/wycheproof/test_rsa.py +++ b/tests/wycheproof/test_rsa.py @@ -98,6 +98,7 @@ def test_rsa_pkcs1v15_signature_generation(backend, wycheproof): wycheproof.testgroup["privateKeyPem"].encode(), password=None, backend=backend, + unsafe_skip_rsa_key_validation=True, ) assert isinstance(key, rsa.RSAPrivateKey) digest = _DIGESTS[wycheproof.testgroup["sha"]] @@ -193,6 +194,7 @@ def test_rsa_oaep_encryption(backend, wycheproof): wycheproof.testgroup["privateKeyPem"].encode("ascii"), password=None, backend=backend, + unsafe_skip_rsa_key_validation=True, ) assert isinstance(key, rsa.RSAPrivateKey) digest = _DIGESTS[wycheproof.testgroup["sha"]] @@ -228,6 +230,7 @@ def test_rsa_pkcs1_encryption(backend, wycheproof): wycheproof.testgroup["privateKeyPem"].encode("ascii"), password=None, backend=backend, + unsafe_skip_rsa_key_validation=True, ) assert isinstance(key, rsa.RSAPrivateKey) diff --git a/tests/wycheproof/utils.py b/tests/wycheproof/utils.py index 3c18e62afa43..eebbe7ce3bf6 100644 --- a/tests/wycheproof/utils.py +++ b/tests/wycheproof/utils.py @@ -3,9 +3,7 @@ def wycheproof_tests(*paths): def wrapper(func): - def run_wycheproof( - backend, disable_rsa_checks, subtests, pytestconfig - ): + def run_wycheproof(backend, subtests, pytestconfig): wycheproof_root = pytestconfig.getoption( "--wycheproof-root", skip=True )