Use EVP_PKEY for RSA Decrypt

[bartonjs]

• Moves the EVP_DIGEST*/IntPtr cache from HashProviderDispenser to an Interop file so it can be shared across S.S.C.Algorithms and S.S.C.OpenSsl.
• Uses EVP_PKEY_decrypt instead of RSA_private_decrypt
  • Side benefit, stops using RSAPaddingProcessor for OAEP with SHA-2
• Creates a temporary copy of HasNoPrivateKey. The original (in pal_rsa.c) will be deleted when RSA signing moves to EVP_PKEY.

Contributes to #46526

[ghost]

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq, @GrabYourPitchforks
See info in area-owners.md if you want to be subscribed.

Issue Details

---

• Moves the EVP_DIGEST*/IntPtr cache from HashProviderDispenser to an Interop file so it can be shared across S.S.C.Algorithms and S.S.C.OpenSsl.
• Uses EVP_PKEY_decrypt instead of RSA_private_decrypt
  • Side benefit, stops using RSAPaddingProcessor for OAEP with SHA-2
• Creates a temporary copy of HasNoPrivateKey. The original (in pal_rsa.c) will be deleted when RSA signing moves to EVP_PKEY.

Contributes to #46526

Author:	bartonjs
Assignees:	bartonjs
Labels:	area-System.Security
Milestone:	6.0.0

[bartonjs]

OK, I'm officially confused.

System.Security.Cryptography.Pkcs.Tests  Total: 944, Errors: 0, Failed: 0, Skipped: 24, Time: 9.816s

...
$ uname -a
Linux jbarton002 4.15.0-1109-azure #121~16.04.1-Ubuntu SMP Thu Feb 25 13:37:23 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

[vcsjones]

@bartonjs Hmm. Let me give it a shot.

[vcsjones]

@bartonjs written needs to be initialized to the length of destination here. From OpenSSL docs:

If out is not NULL then before the call the outlen parameter should contain the length of the out buffer, if the call is successful the decrypted data is written to out and the amount of data written to outlen.

So written is an in/out, not just an out and might be garbage, which makes sense why all of the failures were in checked / release builds. OpenSSL then checks the that the outlen input is long enough to write to.

The validation call is here:

https://github.com/openssl/openssl/blob/52c587d60be67c337364b830dd3fdc15404a2f04/crypto/evp/pmeth_fn.c#L200

And if arglen (written in your case) is too small after checking it against the key size, it throws.

https://github.com/openssl/openssl/blob/52c587d60be67c337364b830dd3fdc15404a2f04/crypto/evp/pmeth_fn.c#L29-L30

If you initialize written to zero first, the error will happen consistently, just to demonstrate that the value prior to calling EVP_PKEY_decrypt matters.

[vcsjones]

I opened a PR into your PR for this here. bartonjs#1

If the PR into a PR is too weird or doesn't work you can grab a patch here and apply it directly to your branch https://github.com/bartonjs/runtime/pull/1.patch (assuming the changes are useful 😄 )

[bartonjs]

Oy, what an embarrassing RTFM moment. Thanks for the detective work.

[bartonjs]

I opened a PR into your PR for this here

Huh, the tab had gotten your first message, but not the second. I didn't ignore your PRPR, just happened to have made a pretty-darn-close-to-identical version before seeing that it existed (when catching up on email notifications).

[vcsjones]

No worries. I'm glad we pretty much came to the same change set. 😀

[vcsjones]

Yay, that seemed to do it.

[bartonjs]

The new failures aren't caused by the latest commit, going ahead and merging.