-
Notifications
You must be signed in to change notification settings - Fork 5.1k
104080 custom rsapss salt length #119255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
104080 custom rsapss salt length #119255
Changes from all commits
e0b1dae
05ed28b
21135d5
d705126
3c6288c
4d3bd37
d19d017
20be62b
c0d0ce0
81432d5
a016382
9cde2fa
a6dd48a
71325e3
e2407be
7b65140
623199c
b14906e
9dcb4ea
fee0f09
a386bac
430a846
e0eb704
dd64636
d97581c
e99f10d
f066e97
de11a87
79bb2de
6e64ad1
aeab113
d70ef9b
76065fe
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -735,7 +735,10 @@ public override byte[] SignHash(byte[] hash, HashAlgorithmName hashAlgorithm, RS | |
int bytesRequired = Interop.Crypto.GetEvpPKeySizeBytes(key); | ||
byte[] signature = new byte[bytesRequired]; | ||
|
||
int written = Interop.Crypto.RsaSignHash(key, padding.Mode, hashAlgorithm, hash, signature); | ||
int pssSaltLength = padding.Mode == RSASignaturePaddingMode.Pss | ||
? RsaPaddingProcessor.CalculatePssSaltLength(padding.PssSaltLength, KeySize, hashAlgorithm) | ||
: 0; | ||
int written = Interop.Crypto.RsaSignHash(key, padding.Mode, pssSaltLength, hashAlgorithm, hash, signature); | ||
|
||
if (written != signature.Length) | ||
{ | ||
|
@@ -766,7 +769,10 @@ public override bool TrySignHash( | |
return false; | ||
} | ||
|
||
bytesWritten = Interop.Crypto.RsaSignHash(key, padding.Mode, hashAlgorithm, hash, destination); | ||
int pssSaltLength = padding.Mode == RSASignaturePaddingMode.Pss | ||
? RsaPaddingProcessor.CalculatePssSaltLength(padding.PssSaltLength, KeySize, hashAlgorithm) | ||
: 0; | ||
bytesWritten = Interop.Crypto.RsaSignHash(key, padding.Mode, pssSaltLength, hashAlgorithm, hash, destination); | ||
Debug.Assert(bytesWritten == bytesRequired); | ||
return true; | ||
} | ||
|
@@ -791,9 +797,13 @@ public override bool VerifyHash(ReadOnlySpan<byte> hash, ReadOnlySpan<byte> sign | |
|
||
SafeEvpPKeyHandle key = GetKey(); | ||
|
||
int pssSaltLength = padding.Mode == RSASignaturePaddingMode.Pss | ||
? RsaPaddingProcessor.CalculatePssSaltLength(padding.PssSaltLength, KeySize, hashAlgorithm) | ||
: 0; | ||
return Interop.Crypto.RsaVerifyHash( | ||
key, | ||
padding.Mode, | ||
pssSaltLength, | ||
hashAlgorithm, | ||
hash, | ||
signature); | ||
|
@@ -853,20 +863,17 @@ private static void ValidatePadding(RSASignaturePadding padding) | |
{ | ||
ArgumentNullException.ThrowIfNull(padding); | ||
|
||
// RSASignaturePadding currently only has the mode property, so | ||
// there's no need for a runtime check that PKCS#1 doesn't use | ||
// nonsensical options like with RSAEncryptionPadding. | ||
// | ||
// This would change if we supported PSS with an MGF other than MGF-1, | ||
// or with a custom salt size, or with a different MGF digest algorithm | ||
// than the data digest algorithm. | ||
// PKCS#1 does not currently have anything to validate. | ||
if (padding.Mode == RSASignaturePaddingMode.Pkcs1) | ||
{ | ||
Debug.Assert(padding == RSASignaturePadding.Pkcs1); | ||
} | ||
else if (padding.Mode == RSASignaturePaddingMode.Pss) | ||
{ | ||
Debug.Assert(padding == RSASignaturePadding.Pss); | ||
if (padding.PssSaltLength < RSASignaturePadding.PssSaltLengthMax) | ||
{ | ||
throw PaddingModeNotSupported(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The exception says the padding "mode" is invalid. Which isn't the case here. The "mode" is PSS, which is valid. So we probably want a new message for the salt size being gibberish (and yet a different one for "that doesn't work here") There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, an assert here is fine given the throw there. |
||
} | ||
} | ||
else | ||
{ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is tricky. Right now, .NET 10 is "done". Technically this work will go in for .NET 11. However, the runtime and SDK are not "bumped" to .NET 11 yet so
#if NET11_0_OR_GREATER
doesn't exist. We might need to wait for this work until #118583 is complete.