Set implicit rejection of invalid RSA PKCS#1 v1.5 padding

[qmuntal]

OpenSSL 3.2 changed the EVP_PKEY_decrypt behavior to not return an error when the padding is invalid. Instead, it returns a random value. See openssl/openssl#13817.

This is a security improvement, but it breaks compatibility with rsa.DecryptPKCS1v15#DecryptPKCS1v15, which is documented to return an error when the padding is invalid.

To maintain compatibility, we need to enable implicit rejection of invalid padding.

[qmuntal]

@derekparker note if this is merged then this patch can be removed from the RedHat Go fork: https://github.com/golang-fips/go/blob/556abee0da00b1c2ad965590d7793cd8d741b423/patches/012-fixes.patch#L8.

[ueno]

If we go with this, shouldn't we also provide a safer alternative with implicit rejection that allows us to implement DecryptPKCS1v15SessionKey?

[qmuntal]

If we go with this, shouldn't we also provide a safer alternative with implicit rejection that allows us to implement DecryptPKCS1v15SessionKey?

DecryptPKCS1v15SessionKey is already backed by OpenSSL via openssl.DecryptRSANoPadding, see https://cs.opensource.google/go/go/+/refs/tags/go1.22.2:src/crypto/rsa/pkcs1v15.go;l=197. Is it enough?

[ueno]

DecryptPKCS1v15SessionKey is already backed by OpenSSL via openssl.DecryptRSANoPadding, see https://cs.opensource.google/go/go/+/refs/tags/go1.22.2:src/crypto/rsa/pkcs1v15.go;l=197. Is it enough?

My assumption is that both RSA decryption and unpadding should be done in the same boundary of a cryptographic module from the FIPS perspective. @simo5 @tomato42 what do you think?

[tomato42]

From FIPS perspective, the whole discussion is moot: If you use PKCS#1v1.5 padding, then you can't be FIPS compliant. This padding mode is completely forbidden since 1st of Jan 2024.

From security perspective: the PKCS#1v1.5 algorithm is very dangerous because it is very hard to use safely in practice, like recent 30+ CVEs have shown. Thus the change to OpenSSL and implementation of implicit rejection. What implicit rejection does, it it keeps the information relevant to the Bleichenbacher attack internal to the cryptographic library. Which means that user code doesn't need to be side-channel secure. Note that the OpenSSL API will still return errors when the ciphertext is externally invalid, when it has numerical value larger than the modulus of the used private key. Also, PKCS#1v1.5 doesn't guarantee that a modified ciphertext (be it due to transmission error or a deliberate attack) will not decrypt to a random message.

[simo5]

DecryptPKCS1v15SessionKey is already backed by OpenSSL via openssl.DecryptRSANoPadding, see https://cs.opensource.google/go/go/+/refs/tags/go1.22.2:src/crypto/rsa/pkcs1v15.go;l=197. Is it enough?

My assumption is that both RSA decryption and unpadding should be done in the same boundary of a cryptographic module from the FIPS perspective. @simo5 @tomato42 what do you think?

As Hubert said at this point the whole algorithm should not be used anymore, however if it has to be used, for whatever reason, even if unapproved, then I think the thing should be deferred wholesale to openssl, it is very easy to introduce side-channels otherwise.

I do not think Go provides any guarantees in term of compilation that would allow to create side-channel free depadding, and the depadding operation itself is very critical in term of side-channels.

[tomato42]

Actually, reading the code more, the API used:

func DecryptRSAPKCS1(priv *PrivateKeyRSA, ciphertext []byte) ([]byte, error)

is inherently non-side channel safe. So by disabling implicit rejection in OpenSSL you're making the code vulnerable.

[tomato42]

I do not think Go provides any guarantees in term of compilation that would allow to create side-channel free depadding, and the depadding operation itself is very critical in term of side-channels.

We did verify CVE-2023-45287 is actually fixed... I haven't looked how it was done, but I'm pretty sure that it, as used in the TLS server code, is done in side-channel free way.

[qmuntal]

Thanks everyone for the insights. Based on this discussion, I'm retracting this PR.