Skip to content

Conversation

bartonjs
Copy link
Member

With this change all RSA private key operations (excluding import/export) use the EVP_PKEY APIs.

  • RSAPaddingProcessor is no longer used in conjunction with the private keys, on Linux.
  • The pal_rsa.c copy of HasPrivateKey has been removed.

Contributes to #46526.

@bartonjs bartonjs added this to the 6.0.0 milestone Mar 25, 2021
@bartonjs bartonjs self-assigned this Mar 25, 2021
@ghost
Copy link

ghost commented Mar 25, 2021

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

Issue Details

With this change all RSA private key operations (excluding import/export) use the EVP_PKEY APIs.

  • RSAPaddingProcessor is no longer used in conjunction with the private keys, on Linux.
  • The pal_rsa.c copy of HasPrivateKey has been removed.

Contributes to #46526.

Author: bartonjs
Assignees: bartonjs
Labels:

area-System.Security

Milestone: 6.0.0

int32_t destinationLen)
{
assert(pkey != NULL);
assert(hash != NULL);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I traced back to the public RSAOpenSsl.TrySignHash entry point that hash can be nullptr, since ROS<byte>.IsEmpty is never checked. Double-check me on this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm. I agree, I don't see a size check. So either there's no test with the default span as input, or I'm missing something. I'll look more into this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, looks like we're missing a test here. The assert totally trips.

The good news is, with the test case, I see that simply removing the assert makes things behave as expected:

        Interop+Crypto+OpenSslCryptographicException : error:0408E08F:rsa routines:pkey_rsa_sign:invalid digest length
        Stack Trace:
          /home/jbarton/git/bartonjs/runtime/src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EvpPkey.Rsa.cs(94,0): at Interop.Crypto.RsaSignHash(SafeEvpPKeyHandle pkey, RSASignaturePaddingMode paddingMode, IntPtr digestAlgorithm, ReadOnlySpan`1 hash, Span`1 destination)
          /home/jbarton/git/bartonjs/runtime/src/libraries/Common/src/System/Security/Cryptography/RSAOpenSsl.cs(683,0): at System.Security.Cryptography.RSAImplementation.RSAOpenSsl.TrySignHash(ReadOnlySpan`1 hash, Span`1 destination, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding, Boolean allocateSignature, Int32& bytesWritten, Byte[]& signature)
          /home/jbarton/git/bartonjs/runtime/src/libraries/Common/src/System/Security/Cryptography/RSAOpenSsl.cs(639,0): at System.Security.Cryptography.RSAImplementation.RSAOpenSsl.TrySignHash(ReadOnlySpan`1 hash, Span`1 destination, HashAlgorithmName hashAlgorithm, RSASignaturePadding padding, Int32& bytesWritten)

(That's the correct exception since (NULL, 0) is not a valid input for RSASSA-*-SHA256... which wants something of length 32.)

@bartonjs bartonjs merged commit 272f1cd into dotnet:main Apr 2, 2021
@bartonjs bartonjs deleted the rsa_evp_pkey_sign branch April 2, 2021 16:10
@ghost ghost locked as resolved and limited conversation to collaborators May 2, 2021
@bartonjs bartonjs removed their assignment Jul 26, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants