Massive performance regression in RSA key loading in cryptography 37.0.x

[jaroslawr]

Cryptography 37.0.x is nearly 10 times slower at loading RSA keys than cryptography 36.0.x.

I generate an RSA key:

 openssl genrsa -out key.pem 2048

Install cryptography 36:

pip3 install cryptography==36.0.0

Run this benchmark:

from cryptography.hazmat.primitives import serialization

for _ in range(1000):
    with open("key.pem", "rb") as key_file:
        private_key = serialization.load_pem_private_key(key_file.read(), password=None)

time python3 cryptography_test.py gives 4.10s.

Now install cryptography 37.0.2:

pip3 install cryptography==37.0.2

time python3 cryptography_test.py gives over 44s.

I suspect the performance regression is due to:
0724c5f

[alex]

The cause of this is almost certainly our upgrade to OpenSSL 3.0, which has a new algorithm for checking the validity of RSA private keys, which is known to be much slower for valid RSA keys.

Unfortunately we're in between a rock and a hard place and don't have a way to avoid this cost without silently accepting invalid keys which can have all sorts of negative consequences.

I don't have a better suggestion than "try to avoid loading the same key multiple times".

[reaperhulk]

@jaroslawr Our APIs assume all keys may be loaded in an adversarial context and OpenSSL 3's key tests are much slower than previous versions. As Alex said, one potential solution is to simply avoid repeatedly loading keys, but that may not be feasible for you.

Are all keys you load trusted? As in, there's no possibility of attacker controlled key material being loaded? If that's the case then we have an extremely private flag you can modify at runtime (https://github.com/pyca/cryptography/blob/main/src/cryptography/hazmat/backends/openssl/backend.py#L186) which will disable key checks. We could hypothetically discuss adding an unsafe_no_key_check kwarg to our loader APIs as well to make this a per-load option, but that carries significant concern for misuse so I'm reluctant to add it without a compelling reason.

[jeblair]

I'm not the OP, but since you're looking for feedback: disabling the check would be useful for our use case in Zuul. Even making that flag on the backend public or some similar way of establishing it in the API would be great.

More detail: Zuul loads private RSA keys for every project in its system at startup, and there could be thousands of those. They are self-generated and securely stored, so the validation is not worth the time cost for our use case. I'm seeing 0.7 seconds to load a key with checking, and 0.005 seconds to load it without checking.

I plan on using the super-secret flag for now, but making that a reliable option would be useful for us.

Thanks for the solution!

[alex]

Thanks for sharing your use case.

I think we'd like to find a way to help address this. The wrinkle is that the implication of skipping these checks are... unclear. When you really know the keys are trusted, it probably doesn't matter, but the exact danger of using this to speed up "probably safe" keys are unclear: it's not just not getting a clear exception at key load time, various OpenSSL algorithms for operating on keys may have undefined behavior for unsafe keys.

So what the exact API should be here is unclear.

[jgb]

Following this topic. I spent a few hours today on figuring out why one of our pytest series went from needing 10 minutes to complete to 50 minutes.
Turns out it was cryptography v37.
Going back to v36 solves the performance problem.

[ronf]

I ran into this issue with AsyncSSH and ended up making the change at ronf/asyncssh@aa465c8 in my unit test setup code to get back the original performance. It basically does the following:

from cryptography.hazmat.backends.openssl import backend

# Disable RSA key blinding to speed up unit tests
backend._rsa_skip_check_key = True