From db6c106db9d77886d534aef6c3bbb8dfc3e06194 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 1 May 2020 12:22:44 -0400 Subject: [PATCH 1/4] Improve DSA digest handling on macOS --- .../Cryptography/DSASecurityTransforms.cs | 9 ++++++++- .../DSA/DSASignVerify.cs | 20 +++++++++++++++++++ .../src/Resources/Strings.resx | 3 +++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs b/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs index 0744b81a6e3672..9b5f63048b6403 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs @@ -261,9 +261,16 @@ public override unsafe void ImportSubjectPublicKeyInfo( public override byte[] CreateSignature(byte[] rgbHash) { + const int Sha1DigestSizeBytes = 20; + if (rgbHash == null) throw new ArgumentNullException(nameof(rgbHash)); + // MacOS only supports FIPS-186-2, which only defines SHA1 + // as a supported algorithm. + if (rgbHash.Length != Sha1DigestSizeBytes) + throw new CryptographicException(SR.Cryptography_HashSizeInvalid); + SecKeyPair keys = GetKeys(); if (keys.PrivateKey == null) @@ -277,7 +284,7 @@ public override byte[] CreateSignature(byte[] rgbHash) // are always 160 bits / 20 bytes (the size of SHA-1, and the only legal length for Q). byte[] ieeeFormatSignature = AsymmetricAlgorithmHelpers.ConvertDerToIeee1363( derFormatSignature.AsSpan(0, derFormatSignature.Length), - fieldSizeBits: 160); + fieldSizeBits: Sha1DigestSizeBytes * 8); return ieeeFormatSignature; } diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSASignVerify.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSASignVerify.cs index 37c6438fa5eb8e..170a962fb0cad0 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSASignVerify.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSASignVerify.cs @@ -374,6 +374,25 @@ public void Verify2048WithSha1() } } + [ConditionalTheory(nameof(DoesNotSupportFips186_3))] + [InlineData(128)] + [InlineData(256)] + [InlineData(384)] + [InlineData(512)] + public void SignHash_DigestAlgorithmUnsupported(int hashSizeBits) + { + byte[] hash = new byte[hashSizeBits / 8]; + RandomNumberGenerator.Fill(hash); + + using (DSA dsa = DSAFactory.Create()) + { + DSATestData.GetDSA1024_186_2(out DSAParameters parameters, out _, out _); + dsa.ImportParameters(parameters); + CryptographicException ce = Assert.Throws(() => dsa.CreateSignature(hash)); + Assert.Contains("hash's size is not supported", ce.Message); + } + } + private void SignAndVerify(byte[] data, string hashAlgorithmName, DSAParameters dsaParameters, int expectedSignatureLength) { using (DSA dsa = DSAFactory.Create()) @@ -394,5 +413,6 @@ internal static bool SupportsFips186_3 } } public static bool SupportsKeyGeneration => DSAFactory.SupportsKeyGeneration; + public static bool DoesNotSupportFips186_3 => !SupportsFips186_3; } } diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx index 76aed28615e9ae..c572a34f518749 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx @@ -162,6 +162,9 @@ The hash algorithm name cannot be null or empty. + + The specified hash's size is not supported on this platform. + Object identifier (OID) is unknown. From 6d61a705bc77c45229989b75aa5ddffaae4d8dca Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 1 May 2020 12:55:49 -0400 Subject: [PATCH 2/4] Fix build. --- .../src/Resources/Strings.resx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx index 37fc7642786bc6..31ac681514e42b 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx @@ -345,6 +345,9 @@ Removing the requested certificate would modify admin trust settings, and has been denied. + + The specified hash's size is not supported on this platform. + Specified key is not a valid size for this algorithm. From 4d8b2a0a7b99969e6311ff351660e290005831ed Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 1 May 2020 16:54:50 -0400 Subject: [PATCH 3/4] Fix exception message; Linux appears capable of FIPS-186-3. --- .../System/Security/Cryptography/DSASecurityTransforms.cs | 2 +- .../AlgorithmImplementations/DSA/DSASignVerify.cs | 2 +- .../src/Resources/Strings.resx | 4 ++-- .../tests/DSACryptoServiceProviderProvider.cs | 2 +- .../src/Resources/Strings.resx | 6 +++--- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs b/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs index 9b5f63048b6403..cf69f85de20649 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/DSASecurityTransforms.cs @@ -269,7 +269,7 @@ public override byte[] CreateSignature(byte[] rgbHash) // MacOS only supports FIPS-186-2, which only defines SHA1 // as a supported algorithm. if (rgbHash.Length != Sha1DigestSizeBytes) - throw new CryptographicException(SR.Cryptography_HashSizeInvalid); + throw new CryptographicException(SR.Format(SR.Cryptography_InvalidHashSize, "SHA1", Sha1DigestSizeBytes)); SecKeyPair keys = GetKeys(); diff --git a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSASignVerify.cs b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSASignVerify.cs index 170a962fb0cad0..44429c37b171e0 100644 --- a/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSASignVerify.cs +++ b/src/libraries/Common/tests/System/Security/Cryptography/AlgorithmImplementations/DSA/DSASignVerify.cs @@ -389,7 +389,7 @@ public void SignHash_DigestAlgorithmUnsupported(int hashSizeBits) DSATestData.GetDSA1024_186_2(out DSAParameters parameters, out _, out _); dsa.ImportParameters(parameters); CryptographicException ce = Assert.Throws(() => dsa.CreateSignature(hash)); - Assert.Contains("hash's size is not supported", ce.Message); + Assert.Equal("SHA1 algorithm hash size is 20 bytes.", ce.Message); } } diff --git a/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx index c572a34f518749..1fd5f39a7c162f 100644 --- a/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.Algorithms/src/Resources/Strings.resx @@ -162,8 +162,8 @@ The hash algorithm name cannot be null or empty. - - The specified hash's size is not supported on this platform. + + {0} algorithm hash size is {1} bytes. Object identifier (OID) is unknown. diff --git a/src/libraries/System.Security.Cryptography.Csp/tests/DSACryptoServiceProviderProvider.cs b/src/libraries/System.Security.Cryptography.Csp/tests/DSACryptoServiceProviderProvider.cs index 2a9bcefeb785d4..ff3b1e42c65da4 100644 --- a/src/libraries/System.Security.Cryptography.Csp/tests/DSACryptoServiceProviderProvider.cs +++ b/src/libraries/System.Security.Cryptography.Csp/tests/DSACryptoServiceProviderProvider.cs @@ -16,7 +16,7 @@ public DSA Create(int keySize) return new DSACryptoServiceProvider(keySize); } - public bool SupportsFips186_3 => false; + public bool SupportsFips186_3 => PlatformDetection.IsLinux; public bool SupportsKeyGeneration => !PlatformDetection.IsOSX; } diff --git a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx index 31ac681514e42b..59b616b7f4f459 100644 --- a/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx +++ b/src/libraries/System.Security.Cryptography.X509Certificates/src/Resources/Strings.resx @@ -74,7 +74,7 @@ Invalid type. - + Non-negative number required. @@ -345,8 +345,8 @@ Removing the requested certificate would modify admin trust settings, and has been denied. - - The specified hash's size is not supported on this platform. + + {0} algorithm hash size is {1} bytes. Specified key is not a valid size for this algorithm. From 3a7b190aaf3772a0b651d51d4185f90254ca6ac6 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Fri, 1 May 2020 17:20:04 -0400 Subject: [PATCH 4/4] Move hash size check for Unix DSA CSP --- .../Cryptography/DSACryptoServiceProvider.Unix.cs | 13 ++++++++++--- .../tests/DSACryptoServiceProviderProvider.cs | 2 +- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Security.Cryptography.Csp/src/System/Security/Cryptography/DSACryptoServiceProvider.Unix.cs b/src/libraries/System.Security.Cryptography.Csp/src/System/Security/Cryptography/DSACryptoServiceProvider.Unix.cs index 52fdfcd17c56a0..f7a1828328866a 100644 --- a/src/libraries/System.Security.Cryptography.Csp/src/System/Security/Cryptography/DSACryptoServiceProvider.Unix.cs +++ b/src/libraries/System.Security.Cryptography.Csp/src/System/Security/Cryptography/DSACryptoServiceProvider.Unix.cs @@ -49,7 +49,16 @@ public DSACryptoServiceProvider(CspParameters parameters) } [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Security", "CA5351", Justification = "This is the implementation of DSACryptoServiceProvider")] - public override byte[] CreateSignature(byte[] rgbHash) => _impl.CreateSignature(rgbHash); + public override byte[] CreateSignature(byte[] rgbHash) + { + if (rgbHash == null) + throw new ArgumentNullException(nameof(rgbHash)); + + if (rgbHash.Length != SHA1_HASHSIZE) + throw new CryptographicException(SR.Format(SR.Cryptography_InvalidHashSize, "SHA1", SHA1_HASHSIZE)); + + return _impl.CreateSignature(rgbHash); + } public override bool TryCreateSignature(ReadOnlySpan hash, Span destination, out int bytesWritten) => _impl.TryCreateSignature(hash, destination, out bytesWritten); @@ -212,8 +221,6 @@ public byte[] SignHash(byte[] rgbHash, string str) throw new ArgumentNullException(nameof(rgbHash)); if (PublicOnly) throw new CryptographicException(SR.Cryptography_CSP_NoPrivateKey); - if (rgbHash.Length != SHA1_HASHSIZE) - throw new CryptographicException(SR.Format(SR.Cryptography_InvalidHashSize, "SHA1", SHA1_HASHSIZE)); // Only SHA1 allowed; the default value is SHA1 if (str != null && !string.Equals(str, "SHA1", StringComparison.OrdinalIgnoreCase)) diff --git a/src/libraries/System.Security.Cryptography.Csp/tests/DSACryptoServiceProviderProvider.cs b/src/libraries/System.Security.Cryptography.Csp/tests/DSACryptoServiceProviderProvider.cs index ff3b1e42c65da4..2a9bcefeb785d4 100644 --- a/src/libraries/System.Security.Cryptography.Csp/tests/DSACryptoServiceProviderProvider.cs +++ b/src/libraries/System.Security.Cryptography.Csp/tests/DSACryptoServiceProviderProvider.cs @@ -16,7 +16,7 @@ public DSA Create(int keySize) return new DSACryptoServiceProvider(keySize); } - public bool SupportsFips186_3 => PlatformDetection.IsLinux; + public bool SupportsFips186_3 => false; public bool SupportsKeyGeneration => !PlatformDetection.IsOSX; }