Support Rfc3279 signature format for DSA and EcDSA #1612

krwq · 2020-01-10T18:34:15Z

Fixes: #31548

Please see the issue for more details.

bartonjs

Partial review, will resume later.

src/libraries/Common/src/Internal/Cryptography/AsymmetricAlgorithmHelpers.Der.cs

src/libraries/Common/src/Interop/Unix/System.Security.Cryptography.Native/Interop.EcDsa.cs

src/libraries/Common/src/System/Security/Cryptography/DSACng.SignVerify.cs

src/libraries/Common/src/System/Security/Cryptography/DSAOpenSsl.cs

src/libraries/Common/src/System/Security/Cryptography/ECDsaCng.SignVerify.cs

src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/Helpers.cs

src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/DSA.cs

bartonjs · 2020-01-10T22:43:47Z

src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/DSA.cs

+
+        protected virtual bool TryCreateSignatureCore(ReadOnlySpan<byte> hash, Span<byte> destination, DSASignatureFormat signatureFormat, out int bytesWritten)
+        {
+            // This method is expected to be overriden with better implementation


When we update the docs for this class will we recommend falling back to this implementation for an unknown signature format, since we'd theoretically handle it in ConvertIeeeP1363Signature for them?

Alternatively, do we want to add something so a derived type can report whether or not it directly handles a format, so that we understand when to apply an 1363-to-destination-scheme strategy?

(Maybe it's moot since there're only the two, but I like to think ahead)

src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/DSA.cs

...stem.Security.Cryptography.Algorithms/src/System/Security/Cryptography/DSASignatureFormat.cs

src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/ECDsa.cs

src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/DSA.cs

krwq · 2020-01-15T00:09:15Z

Marking as NO REVIEW until I figure out all the failures and fix build issues on non-Windows and fix all feedback so far

src/libraries/System.Security.Cryptography.Algorithms/tests/DsaEcDsaSignatureFormatTests.cs

...aries/System.Security.Cryptography.Algorithms/ref/System.Security.Cryptography.Algorithms.cs

This accounts for: * macOS not supporting DSA key generation * macOS not supporting FIPS 186-3 DSA * DSACryptoServiceProvider not supporting FIPS 186-3 DSA * Testing the signature format tests on the concrete types, as well as the opaque ones.

src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/DSA.cs

The existing methods should all be unchanged, and the new methods written in terms of the existing ones.

GrabYourPitchforks

Some questions about edge cases and error handling, but otherwise seems solid. :)

src/libraries/Common/src/Internal/Cryptography/AsymmetricAlgorithmHelpers.Der.cs

src/libraries/Common/src/Internal/Cryptography/Helpers.cs

src/libraries/Common/src/System/Security/Cryptography/DSACng.SignVerify.cs

src/libraries/Common/src/System/Security/Cryptography/ECDsaCng.SignVerify.cs

src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/DSA.cs

src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/ECDsa.cs

Support DSASignatureFormat

d12272e

krwq requested a review from bartonjs January 10, 2020 18:34