Skip to content

Commit

Permalink
add unsafe_skip_rsa_key_validation
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
reaperhulk committed Oct 1, 2022
1 parent 9ad715a commit 79d8c65
Show file tree
Hide file tree
Showing 13 changed files with 138 additions and 48 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
5 changes: 5 additions & 0 deletions docs/glossary.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
20 changes: 17 additions & 3 deletions docs/hazmat/primitives/asymmetric/rsa.rst
Original file line number Diff line number Diff line change
Expand Up @@ -473,10 +473,24 @@ 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)

:returns: An instance of
:class:`~cryptography.hazmat.primitives.asymmetric.rsa.RSAPrivateKey`.
: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`.

Handling partial RSA private keys
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Expand Down
32 changes: 29 additions & 3 deletions docs/hazmat/primitives/asymmetric/serialization.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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`,
Expand Down Expand Up @@ -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

Expand All @@ -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`,
Expand Down
53 changes: 40 additions & 13 deletions src/cryptography/hazmat/backends/openssl/backend.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
) -> PRIVATE_KEY_TYPES:
"""
Return the appropriate type of PrivateKey given an evp_pkey cdata
pointer.
Expand All @@ -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
Expand Down Expand Up @@ -932,13 +943,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: typing.Optional[bool] = None,
) -> 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:
Expand Down Expand Up @@ -996,7 +1010,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: typing.Optional[bool] = None,
) -> PRIVATE_KEY_TYPES:
# OpenSSL has a function called d2i_AutoPrivateKey that in theory
# handles this automatically, however it doesn't handle encrypted
Expand All @@ -1005,15 +1022,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):
Expand Down Expand Up @@ -1146,7 +1165,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 *")
Expand Down Expand Up @@ -1192,7 +1213,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()
Expand Down Expand Up @@ -2191,7 +2214,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)
Expand Down
9 changes: 7 additions & 2 deletions src/cryptography/hazmat/backends/openssl/rsa.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
11 changes: 9 additions & 2 deletions src/cryptography/hazmat/primitives/asymmetric/rsa.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
12 changes: 10 additions & 2 deletions src/cryptography/hazmat/primitives/serialization/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand Down
9 changes: 0 additions & 9 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 3 additions & 1 deletion tests/hazmat/backends/test_openssl.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading

0 comments on commit 79d8c65

Please sign in to comment.