add unsafe_skip_rsa_key_validation (#7667)

* add unsafe_skip_rsa_key_validation

This allows users to skip RSA key validation when calling
load_pem_private_key, load_der_private_key, and
RSAPrivateNumbers.private_key. This is a significant performance
improvement but is **only safe if you know the key is valid**. If you
use this when the key is invalid OpenSSL makes no guarantees about what
might happen. Infinite loops, crashes, and all manner of terrible things
become possible if that occurs. Beware, beware, beware.

* Apply suggestions from code review

Co-authored-by: Alex Gaynor <alex.gaynor@gmail.com>

* remove unneeded variable

Co-authored-by: Alex Gaynor <alex.gaynor@gmail.com>

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<legacy-provider>`.
+* 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:
 

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

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 keyword-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`.

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 <cryptography.x509.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 keyword-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 keyword-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`,

src/cryptography/hazmat/backends/openssl/backend.py

@@ -186,7 +186,6 @@ def __init__(self):
  self._binding = binding.Binding()
  self._ffi = self._binding.ffi
  self._lib = self._binding.lib
- self._rsa_skip_check_key = False
  self._fips_enabled = self._is_fips_enabled()
 
  self._cipher_registry = {}
@@ -542,8 +541,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 +556,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 +590,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 +658,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 +673,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 +695,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 +944,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 +1011,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 +1023,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 +1166,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 +1214,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 +2215,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)

src/cryptography/hazmat/backends/openssl/rsa.py

@@ -367,15 +367,20 @@ 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
  # primality checking. In normal use this is unlikely to be a problem
  # 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()

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):

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(

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

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,
  )
  ),
  )