Handle null signature destinations for ECDsa / DSA / RSA

Handle null signature destinations.

When RSA/ECDSA/DSA use a "null" span as a destination for a signature, a nullptr was handed to CNG. CNG would interpret this as asking for the signature length. It would set the output length and return success without actually computing the signature.

This changes the p/invokes to prevent null values for the signature destination.

src/libraries/Common/src/Interop/Windows/BCrypt/Interop.BCryptSignHash.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Runtime.InteropServices;
+using Internal.Cryptography;
 
 using Microsoft.Win32.SafeHandles;
 
@@ -30,7 +31,7 @@ internal static partial class Interop
         {
             fixed (char* pHashAlgorithmName = hashAlgorithmName)
             fixed (byte* pHash = &MemoryMarshal.GetReference(hash))
-            fixed (byte* pDest = &MemoryMarshal.GetReference(destination))
+            fixed (byte* pDest = &Helpers.GetNonNullPinnableReference(destination))
             {
                 BCRYPT_PKCS1_PADDING_INFO paddingInfo = default;
                 paddingInfo.pszAlgId = (IntPtr)pHashAlgorithmName;
@@ -56,7 +57,7 @@ internal static partial class Interop
         {
             fixed (char* pHashAlgorithmName = hashAlgorithmName)
             fixed (byte* pHash = &MemoryMarshal.GetReference(hash))
-            fixed (byte* pDest = &MemoryMarshal.GetReference(destination))
+            fixed (byte* pDest = &Helpers.GetNonNullPinnableReference(destination))
             {
                 BCRYPT_PSS_PADDING_INFO paddingInfo = default;
                 paddingInfo.pszAlgId = (IntPtr)pHashAlgorithmName;

src/libraries/Common/src/Interop/Windows/NCrypt/Interop.SignVerify.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Runtime.InteropServices;
+using Internal.Cryptography;
 using Microsoft.Win32.SafeHandles;
 
 internal static partial class Interop
@@ -10,7 +11,16 @@ internal static partial class Interop
     internal static partial class NCrypt
     {
         [LibraryImport(Libraries.NCrypt, StringMarshalling = StringMarshalling.Utf16)]
-        internal static unsafe partial ErrorCode NCryptSignHash(SafeNCryptKeyHandle hKey, void* pPaddingInfo, ReadOnlySpan<byte> pbHashValue, int cbHashValue, Span<byte> pbSignature, int cbSignature, out int pcbResult, AsymmetricPaddingMode dwFlags);
+        private static unsafe partial ErrorCode NCryptSignHash(SafeNCryptKeyHandle hKey, void* pPaddingInfo, byte* pbHashValue, int cbHashValue, byte* pbSignature, int cbSignature, out int pcbResult, AsymmetricPaddingMode dwFlags);
+
+        internal static unsafe ErrorCode NCryptSignHash(SafeNCryptKeyHandle hKey, void* pPaddingInfo, ReadOnlySpan<byte> pbHashValue, Span<byte> pbSignature, out int pcbResult, AsymmetricPaddingMode dwFlags)
+        {
+            fixed (byte* pHash = &MemoryMarshal.GetReference(pbHashValue))
+            fixed (byte* pSignature = &Helpers.GetNonNullPinnableReference(pbSignature))
+            {
+                return NCryptSignHash(hKey, pPaddingInfo, pHash, pbHashValue.Length, pSignature, pbSignature.Length, out pcbResult, dwFlags);
+            }
+        }
 
         [LibraryImport(Libraries.NCrypt, StringMarshalling = StringMarshalling.Utf16)]
         internal static unsafe partial ErrorCode NCryptVerifySignature(SafeNCryptKeyHandle hKey, void* pPaddingInfo, ReadOnlySpan<byte> pbHashValue, int cbHashValue, ReadOnlySpan<byte> pbSignature, int cbSignature, AsymmetricPaddingMode dwFlags);

src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSASignVerify.cs

@@ -391,6 +391,25 @@ namespace System.Security.Cryptography.Dsa.Tests
             }
         }
 
+        [Fact]
+        public void SignData_NullSignature_Fails()
+        {
+            using (DSA dsa = DSAFactory.Create())
+            {
+                dsa.ImportParameters(DSATestData.GetDSA1024Params());
+
+                bool result = dsa.TrySignData(
+                    "hello"u8,
+                    (Span<byte>)null,
+                    HashAlgorithmName.SHA1,
+                    DSASignatureFormat.IeeeP1363FixedFieldConcatenation,
+                    out int bytesWritten);
+
+                Assert.False(result);
+                Assert.Equal(0, bytesWritten);
+            }
+        }
+
         private void SignAndVerify(byte[] data, string hashAlgorithmName, DSAParameters dsaParameters, int expectedSignatureLength)
         {
             using (DSA dsa = DSAFactory.Create())

src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/ECDsa/ECDsaTests.cs

@@ -86,6 +86,32 @@ namespace System.Security.Cryptography.EcDsa.Tests
             AssertExtensions.Throws<ArgumentNullException>("hash", () => ecdsa.VerifyHash(null, null));
             AssertExtensions.Throws<ArgumentNullException>("signature", () => ecdsa.VerifyHash(new byte[0], null));
         }
+
+        [Theory]
+        [MemberData(nameof(RealImplementations))]
+        public void SignHash_NullSignature_Fails(ECDsa ecdsa)
+        {
+            byte[] hash = RandomNumberGenerator.GetBytes(SHA256.HashSizeInBytes);
+
+            AssertExtensions.Throws<ArgumentException>("destination", () =>
+                ecdsa.SignHash(hash, (Span<byte>)null, DSASignatureFormat.IeeeP1363FixedFieldConcatenation));
+
+            bool result = ecdsa.TrySignHash(hash, (Span<byte>)null, DSASignatureFormat.IeeeP1363FixedFieldConcatenation, out int bytesWritten);
+            Assert.False(result);
+            Assert.Equal(0, bytesWritten);
+        }
+
+        [Theory]
+        [MemberData(nameof(RealImplementations))]
+        public void SignData_NullSignature_Fails(ECDsa ecdsa)
+        {
+            AssertExtensions.Throws<ArgumentException>("destination", () =>
+                ecdsa.SignData("hello"u8, (Span<byte>)null, HashAlgorithmName.SHA256, DSASignatureFormat.IeeeP1363FixedFieldConcatenation));
+
+            bool result = ecdsa.TrySignData("hello"u8, (Span<byte>)null, HashAlgorithmName.SHA256, DSASignatureFormat.IeeeP1363FixedFieldConcatenation, out int bytesWritten);
+            Assert.False(result);
+            Assert.Equal(0, bytesWritten);
+        }
     }
 
     [SkipOnPlatform(TestPlatforms.Browser, "Not supported on Browser")]

src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/RSA/SignVerify.cs

@@ -1429,6 +1429,60 @@ namespace System.Security.Cryptography.Rsa.Tests
             }
         }
 
+        [ConditionalTheory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void SignHash_NullSignature_Fails(bool usePss)
+        {
+            if (!SupportsPss)
+            {
+                throw new SkipTestException("Platform does not support PSS");
+            }
+
+            RSASignaturePadding padding = usePss ? RSASignaturePadding.Pss : RSASignaturePadding.Pkcs1;
+
+            using (RSA rsa = RSA.Create())
+            {
+                byte[] hash = RandomNumberGenerator.GetBytes(SHA256.HashSizeInBytes);
+
+                AssertExtensions.Throws<ArgumentException>("destination", () =>
+                    rsa.SignHash(hash, (Span<byte>)null, HashAlgorithmName.SHA256, padding));
+
+                bool result = rsa.TrySignHash(
+                    hash,
+                    (Span<byte>)null,
+                    HashAlgorithmName.SHA256,
+                    padding,
+                    out int bytesWritten);
+
+                Assert.False(result);
+                Assert.Equal(0, bytesWritten);
+            }
+        }
+
+        [ConditionalTheory]
+        [InlineData(true)]
+        [InlineData(false)]
+        public void SignData_NullSignature_Fails(bool usePss)
+        {
+            if (!SupportsPss)
+            {
+                throw new SkipTestException("Platform does not support PSS");
+            }
+
+            RSASignaturePadding padding = usePss ? RSASignaturePadding.Pss : RSASignaturePadding.Pkcs1;
+
+            using (RSA rsa = RSA.Create())
+            {
+                AssertExtensions.Throws<ArgumentException>("destination", () =>
+                    rsa.SignData("hello"u8, (Span<byte>)null, HashAlgorithmName.SHA256, padding));
+
+                bool result = rsa.TrySignData("hello"u8, (Span<byte>)null, HashAlgorithmName.SHA256, padding, out int bytesWritten);
+                Assert.False(result);
+                Assert.Equal(0, bytesWritten);
+            }
+        }
+
         private void ExpectSignature(
             byte[] expectedSignature,
             byte[] data,

src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CngCommon.SignVerify.cs

@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
+using System.Diagnostics;
 using Internal.Cryptography;
 using Microsoft.Win32.SafeHandles;
 using ErrorCode = Interop.NCrypt.ErrorCode;
@@ -20,22 +21,22 @@ namespace System.Security.Cryptography
 #endif
             byte[] signature = new byte[estimatedSize];
             int numBytesNeeded;
-            ErrorCode errorCode = Interop.NCrypt.NCryptSignHash(keyHandle, pPaddingInfo, hash, hash.Length, signature, signature.Length, out numBytesNeeded, paddingMode);
+            ErrorCode errorCode = Interop.NCrypt.NCryptSignHash(keyHandle, pPaddingInfo, hash, signature, out numBytesNeeded, paddingMode);
 
             if (errorCode == ErrorCode.STATUS_UNSUCCESSFUL)
             {
-                errorCode = Interop.NCrypt.NCryptSignHash(keyHandle, pPaddingInfo, hash, hash.Length, signature, signature.Length, out numBytesNeeded, paddingMode);
+                errorCode = Interop.NCrypt.NCryptSignHash(keyHandle, pPaddingInfo, hash, signature, out numBytesNeeded, paddingMode);
             }
 
             if (errorCode.IsBufferTooSmall())
             {
                 signature = new byte[numBytesNeeded];
-                errorCode = Interop.NCrypt.NCryptSignHash(keyHandle, pPaddingInfo, hash, hash.Length, signature, signature.Length, out numBytesNeeded, paddingMode);
+                errorCode = Interop.NCrypt.NCryptSignHash(keyHandle, pPaddingInfo, hash, signature, out numBytesNeeded, paddingMode);
             }
 
             if (errorCode == ErrorCode.STATUS_UNSUCCESSFUL)
             {
-                errorCode = Interop.NCrypt.NCryptSignHash(keyHandle, pPaddingInfo, hash, hash.Length, signature, signature.Length, out numBytesNeeded, paddingMode);
+                errorCode = Interop.NCrypt.NCryptSignHash(keyHandle, pPaddingInfo, hash, signature, out numBytesNeeded, paddingMode);
             }
 
             if (errorCode != ErrorCode.ERROR_SUCCESS)
@@ -53,9 +54,7 @@ namespace System.Security.Cryptography
                     keyHandle,
                     pPaddingInfo,
                     hash,
-                    hash.Length,
                     signature,
-                    signature.Length,
                     out int numBytesNeeded,
                     paddingMode);
 
@@ -63,6 +62,7 @@ namespace System.Security.Cryptography
                 {
                     case ErrorCode.ERROR_SUCCESS:
                         bytesWritten = numBytesNeeded;
+                        Debug.Assert(bytesWritten <= signature.Length);
                         return true;
 
                     case ErrorCode code when code.IsBufferTooSmall():