From 2677f26455f2b91f13e8c453b91f43d9c08f0527 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Dec 2022 08:46:39 -0500 Subject: [PATCH 1/3] news fragment --- newsfragments/3914.minor | 0 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 newsfragments/3914.minor diff --git a/newsfragments/3914.minor b/newsfragments/3914.minor new file mode 100644 index 000000000..e69de29bb From 05c7450376bbbfc4cfe0fb977265b9a1365cf588 Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Dec 2022 08:47:05 -0500 Subject: [PATCH 2/3] Try to use an upcoming python-cryptography feature to avoid some costs If the key is the wrong number of bits then we don't care about any other validation results because we're just going to reject it. So, check that before applying other validation, if possible. This is untested since the version of python-cryptography that supports it is not released yet and I don't feel like setting up a Rust build tool chain at the moment. --- src/allmydata/crypto/rsa.py | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index 95cf01413..96885cfa1 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -72,20 +72,39 @@ def create_signing_keypair_from_string(private_key_der): :returns: 2-tuple of (private_key, public_key) """ - priv_key = load_der_private_key( + load = partial( + load_der_private_key, private_key_der, password=None, backend=default_backend(), ) - if not isinstance(priv_key, rsa.RSAPrivateKey): + + try: + # Load it once without the potentially expensive OpenSSL validation + # checks. These have superlinear complexity. We *will* run them just + # below - but first we'll apply our own constant-time checks. + unsafe_priv_key = load(unsafe_skip_rsa_key_validation=True) + except TypeError: + # cryptography<39 does not support this parameter, so just load the + # key with validation... + unsafe_priv_key = load() + # But avoid *reloading* it since that will run the expensive + # validation *again*. + load = lambda: unsafe_priv_key + + if not isinstance(unsafe_priv_key, rsa.RSAPrivateKey): raise ValueError( "Private Key did not decode to an RSA key" ) - if priv_key.key_size != 2048: + if unsafe_priv_key.key_size != 2048: raise ValueError( "Private Key must be 2048 bits" ) - return priv_key, priv_key.public_key() + + # Now re-load it with OpenSSL's validation applied. + safe_priv_key = load() + + return safe_priv_key, safe_priv_key.public_key() def der_string_from_signing_key(private_key): From c014ad55b1aa42295db7295f7a7d99092fee39fd Mon Sep 17 00:00:00 2001 From: Jean-Paul Calderone Date: Wed, 14 Dec 2022 08:48:02 -0500 Subject: [PATCH 3/3] remove Python 2 boilerplate --- src/allmydata/crypto/rsa.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/allmydata/crypto/rsa.py b/src/allmydata/crypto/rsa.py index 96885cfa1..cdd9a6035 100644 --- a/src/allmydata/crypto/rsa.py +++ b/src/allmydata/crypto/rsa.py @@ -12,14 +12,9 @@ on any of their methods. Ported to Python 3. """ -from __future__ import absolute_import -from __future__ import division -from __future__ import print_function -from __future__ import unicode_literals +from __future__ import annotations -from future.utils import PY2 -if PY2: - from builtins import filter, map, zip, ascii, chr, hex, input, next, oct, open, pow, round, super, bytes, dict, list, object, range, str, max, min # noqa: F401 +from functools import partial from cryptography.exceptions import InvalidSignature from cryptography.hazmat.backends import default_backend