Skip to content

Make RSA decryption API safe to use with PKCS#1 v1.5 padding#13817

Closed
tomato42 wants to merge 7 commits into
openssl:masterfrom
tomato42:bleichenbacher-workaround
Closed

Make RSA decryption API safe to use with PKCS#1 v1.5 padding#13817
tomato42 wants to merge 7 commits into
openssl:masterfrom
tomato42:bleichenbacher-workaround

Conversation

@tomato42

@tomato42 tomato42 commented Jan 8, 2021

Copy link
Copy Markdown
Contributor

fixes #13421

The code is functional (behaves the same as the implementation in NSS and tlslite-ng).

We need to decide if we want to cache the d_hash value or not.

Checklist
  • documentation is added or updated
  • tests are added or updated
  • decide if the the hash of the private exponent should live in the RSA structure or not
  • make sure we still handle the mismatch between buffer size and key size the same way

Comment thread crypto/rsa/rsa_ossl.c Outdated
Comment thread crypto/rsa/rsa_ossl.c Outdated
Comment thread crypto/rsa/rsa_pk1.c Outdated
@tomato42 tomato42 force-pushed the bleichenbacher-workaround branch from dfc4db8 to fe1cb2c Compare January 8, 2021 20:00
@tomato42

tomato42 commented Jan 8, 2021

Copy link
Copy Markdown
Contributor Author

@beldmit thanks, updated

@beldmit

beldmit commented Jan 8, 2021

Copy link
Copy Markdown
Member

Build failures seems relevant

Comment thread crypto/rsa/rsa_ossl.c Outdated
Comment thread crypto/rsa/rsa_pk1.c Outdated
@tomato42 tomato42 force-pushed the bleichenbacher-workaround branch from fe1cb2c to 87dcfa0 Compare January 11, 2021 17:16
@tomato42

Copy link
Copy Markdown
Contributor Author

@beldmit updated

@beldmit

beldmit commented Jan 11, 2021

Copy link
Copy Markdown
Member

@tomato42 build failures still persist :(

Comment thread crypto/rsa/rsa_ossl.c Outdated
@tomato42 tomato42 force-pushed the bleichenbacher-workaround branch 2 times, most recently from 25f9b41 to 565fc7e Compare January 11, 2021 17:42
Comment thread crypto/rsa/rsa_ossl.c Outdated
Comment thread crypto/rsa/rsa_ossl.c Outdated
Comment thread crypto/rsa/rsa_ossl.c Outdated
@tomato42 tomato42 force-pushed the bleichenbacher-workaround branch 2 times, most recently from b04e04f to 5e8c1e7 Compare January 12, 2021 14:46
@tomato42 tomato42 marked this pull request as ready for review January 12, 2021 16:22
Comment thread crypto/rsa/rsa_pk1.c Outdated
Comment thread crypto/rsa/rsa_ossl.c Outdated
Comment thread crypto/rsa/rsa_pk1.c Outdated
Comment thread crypto/rsa/rsa_pk1.c Outdated
Comment thread crypto/rsa/rsa_pk1.c Outdated
Comment thread crypto/rsa/rsa_pk1.c Outdated
@tomato42 tomato42 force-pushed the bleichenbacher-workaround branch from 5e8c1e7 to 71a2484 Compare January 13, 2021 14:46
@tomato42

Copy link
Copy Markdown
Contributor Author

@beldmit @t8m PR updated

@tomato42 tomato42 force-pushed the bleichenbacher-workaround branch from 71a2484 to 04dc864 Compare January 13, 2021 15:00
Comment thread crypto/rsa/rsa_pk1.c Outdated
Comment thread crypto/rsa/rsa_pk1.c Outdated
Comment thread crypto/rsa/rsa_pk1.c Outdated
@tomato42 tomato42 force-pushed the bleichenbacher-workaround branch from 04dc864 to 5d1b481 Compare January 13, 2021 15:14
Comment thread crypto/rsa/rsa_pk1.c Outdated
@tomato42 tomato42 force-pushed the bleichenbacher-workaround branch from 5d1b481 to 843b27b Compare January 13, 2021 15:23
@t8m

t8m commented Jan 13, 2021

Copy link
Copy Markdown
Member

This looks good to me now, however given this is an API break we should discuss it within OTC at least.

@t8m t8m added this to the 3.0.0 beta1 milestone Jan 13, 2021
@junaruga

junaruga commented Mar 6, 2024

Copy link
Copy Markdown
Contributor

@t8m do you plan to merge it to 3.1.0 later?

There is currently no plan to merge it to 3.1 branch.

@t8m I am not sure if you remember this old pull-request. But may I ask you why there is currently no plan to merge it to 3.1 branch? What prevented you from backporting this PR to the OpenSSL 3.1/3.0?

There is already a patch for this pull-request included in the downstream OpenSSL 3.1 RPM package on Fedora Linux 39.
https://src.fedoraproject.org/rpms/openssl/blob/f39/f/0079-RSA-PKCS15-implicit-rejection.patch

And there is also already a patch for this pull-request included in the downstrem OpenSSL 3.0 RPM package on Feodra Linux 38.
https://src.fedoraproject.org/rpms/openssl/blob/f38/f/0100-RSA-PKCS15-implicit-rejection.patch

So, I assume preparing the patches for the OpenSSL 3.1/3.0 is relatively easy. For example, if @tomato42 will send the pull-requests into both the openssl-3.1 and openssl-3.0 branches, could you help to review the pull-requests?

@t8m

t8m commented Mar 13, 2024

Copy link
Copy Markdown
Member

This is clearly a feature and thus it is not eligible to be merged to stable branches.

@junaruga

junaruga commented Mar 14, 2024

Copy link
Copy Markdown
Contributor

This is clearly a feature and thus it is not eligible to be merged to stable branches.

@t8m Thanks for your reply. If you think this PR is about a feature, not backporting to the stable branches makes sense to me.

While this PR adds the rsa_pkcs1_implicit_rejection, I was thinking this PR is about a security fix. If a change is about a security fix, you would generally backport it to the stable branches, right?

While I can see the CVE-2020-25659 for python-cryptography and CVE-2020-25657 for m2crypto in CHANGES.md of this PR, does not OpenSSL project have the own CVE ID?

@t8m

t8m commented Mar 14, 2024

Copy link
Copy Markdown
Member

No, we do not regard this issue as a security issue and we did not assign a CVE to it.

@neverpanic

Copy link
Copy Markdown
Contributor

FYI, Ubuntu has backported this change.

@junaruga

junaruga commented Apr 16, 2024

Copy link
Copy Markdown
Contributor

No, we do not regard this issue as a security issue and we did not assign a CVE to it.

@t8m OK. Why do you think you don't regard this issue as a security issue?

Perhaps, do you think that in OpenSSL 3.1 or older versions, using the RSA decryption API safely is a responsibility of users or applications using OpenSSL C APIs? If you think so, could you create a document in this repository or somewhere to guide users about how to use the RSA decryption API safely in OpenSSL 3.1 or earlier versions? Or Is there already such a document?

@tomato42

tomato42 commented Apr 16, 2024

Copy link
Copy Markdown
Contributor Author

https://www.openssl.org/docs/man3.0/man3/RSA_private_decrypt.html

WARNINGS

Decryption failures in the RSA_PKCS1_PADDING mode leak information
which can potentially be used to mount a Bleichenbacher padding oracle
attack. This is an inherent weakness in the PKCS #1 v1.5 padding design.
Prefer RSA_PKCS1_OAEP_PADDING.

The newer man page is much more explicit:
https://www.openssl.org/docs/man3.2/man3/EVP_PKEY_decrypt.html

In OpenSSL versions before 3.2.0, when used in PKCS#1 v1.5 padding,
both the return value from the EVP_PKEY_decrypt() and the outlen
provided information useful in mounting a Bleichenbacher attack
against the used private key. They had to processed in a side-channel
free way.

@junaruga

Copy link
Copy Markdown
Contributor

@tomato42 Thanks for providing the info. OK. OpenSSL project already has such a document.

@tomato42

Copy link
Copy Markdown
Contributor Author

@junaruga I've proposed a PR to make the documentation even more explicit: #24159, feel free to comment on that.

@mbartosch

Copy link
Copy Markdown

I am currently in the process of debugging a problem that seems to happen in conjunction with this patch.

This came to my attention due to a problem that occurred after upgrading OpenSSL to openssl-1:1.1.1k-12.el8_9.x86_64 on a Rocky Linux 8 system. The same happens on a RedHat RHEL 8 system. According to the RedHat changelogs, this patch has been backported into RedHat's OpenSSL.

Problem description:
When performing an openssl cms -decrypt using an RSA key in a PKCS#11 token, the decryption fails with a hard error:

openssl cms -decrypt -in rocky-ciphertext-softhsm.cms -inform pem -recip softhsm-cert.pem -engine pkcs11 -keyform engine -inkey "pkcs11:model=SoftHSM%20v2;manufacturer=SoftHSM%20project;serial=064c221662be4c79;token=Testtoken;id=%D8%A8%DB%E0%D6%37%1F%BE%DE%C2%B2%08%FF%6F%A2%E3%8C%61%62%D0;object=rsatest1"
engine "pkcs11" set.
Enter PKCS#11 token PIN for Testtoken:
Error decrypting CMS structure
140457840785216:error:06065064:digital envelope routines:EVP_DecryptFinal_ex:bad decrypt:crypto/evp/evp_enc.c:643:

I have verified that the same happens with a Nitrokey HSM2 and an nCipher Edge HSM, so I am pretty sure that the problem is not related to a particular HSM type or PKCS#11 library.

After downgrading RedHat openssl to a previous version that does not include this patch, the above command works without problem.

@tomato42

tomato42 commented Aug 7, 2024

Copy link
Copy Markdown
Contributor Author

@mbartosch this should be filed as a separate issue, preferably with a complete stand-alone reproducer

that being said, use of -recip should be unaffected if the ciphertext matches the key...

@jiajia123-wind

Copy link
Copy Markdown

@tomato42 , does the modification in [this pull request](#23832) mean that the issue will no longer exist in OpenSSL versions after 3.4.0? [CVE-2023-50781](https://nvd.nist.gov/vuln/detail/CVE-2023-50781)

@tomato42

tomato42 commented Jan 17, 2025

Copy link
Copy Markdown
Contributor Author

@tomato42 , does the modification in [this pull request](#23832) mean that the issue will no longer exist in OpenSSL versions after 3.4.0? [CVE-2023-50781](https://nvd.nist.gov/vuln/detail/CVE-2023-50781)

I'm confused... #23832 is about Pairwise Consistency Tests... those have little to do with implicit rejection (this PR)

CVE-2023-50781 is a bug in m2crypto... that this PR mitigates if, and only if, the default provider is in use (e.g. won't help with pkcs11 provider)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: ready to merge The 24 hour grace period has passed, ready to merge branch: master Applies to master branch severity: fips change The pull request changes FIPS provider sources tests: present The PR has suitable tests present triaged: feature The issue/pr requests/adds a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

RSA decryption API is very hard to use safely