From 51056be22c66730aaf3a42a38593fac3e83fd325 Mon Sep 17 00:00:00 2001 From: Kevin Jones Date: Tue, 1 Feb 2022 15:10:59 -0500 Subject: [PATCH] Use hash one-shots in RSA/DSACryptoServiceProvider The CryptoServiceProviders were not disposing of hash instances that they created. Rather than fix the disposal, they were converted to one-shots. --- .../Common/src/Internal/Cryptography/Helpers.cs | 8 ++++ .../Security/Cryptography/HashOneShotHelpers.cs | 55 +++++++++++++++++++++- .../Security/Cryptography/CapiHelper.Windows.cs | 11 +++++ .../DSACryptoServiceProvider.Windows.cs | 18 ++++--- .../RSACryptoServiceProvider.Windows.cs | 16 +++---- 5 files changed, 89 insertions(+), 19 deletions(-) diff --git a/src/libraries/Common/src/Internal/Cryptography/Helpers.cs b/src/libraries/Common/src/Internal/Cryptography/Helpers.cs index cb87338..c9edc11 100644 --- a/src/libraries/Common/src/Internal/Cryptography/Helpers.cs +++ b/src/libraries/Common/src/Internal/Cryptography/Helpers.cs @@ -42,6 +42,14 @@ namespace Internal.Cryptography public static bool IsRC2Supported => true; #endif + [UnsupportedOSPlatformGuard("browser")] + internal static bool HasMD5 { get; } = +#if NET5_0_OR_GREATER + !OperatingSystem.IsBrowser(); +#else + true; +#endif + [return: NotNullIfNotNull("src")] public static byte[]? CloneByteArray(this byte[]? src) { diff --git a/src/libraries/Common/src/System/Security/Cryptography/HashOneShotHelpers.cs b/src/libraries/Common/src/System/Security/Cryptography/HashOneShotHelpers.cs index 22fff97..e1f5045 100644 --- a/src/libraries/Common/src/System/Security/Cryptography/HashOneShotHelpers.cs +++ b/src/libraries/Common/src/System/Security/Cryptography/HashOneShotHelpers.cs @@ -2,6 +2,7 @@ // The .NET Foundation licenses this file to you under the MIT license. using Internal.Cryptography; +using System.IO; namespace System.Security.Cryptography { @@ -9,7 +10,59 @@ namespace System.Security.Cryptography [System.Diagnostics.CodeAnalysis.SuppressMessage("Microsoft.Security", "CA5351", Justification = "Weak algorithms are used as instructed by the caller")] internal static class HashOneShotHelpers { - public static int MacData( + internal static byte[] HashData(HashAlgorithmName hashAlgorithm, ReadOnlySpan source) + { + if (hashAlgorithm == HashAlgorithmName.SHA256) + { + return SHA256.HashData(source); + } + else if (hashAlgorithm == HashAlgorithmName.SHA1) + { + return SHA1.HashData(source); + } + else if (hashAlgorithm == HashAlgorithmName.SHA512) + { + return SHA512.HashData(source); + } + else if (hashAlgorithm == HashAlgorithmName.SHA384) + { + return SHA384.HashData(source); + } + else if (Helpers.HasMD5 && hashAlgorithm == HashAlgorithmName.MD5) + { + return MD5.HashData(source); + } + + throw new CryptographicException(SR.Format(SR.Cryptography_UnknownHashAlgorithm, hashAlgorithm.Name)); + } + + internal static byte[] HashData(HashAlgorithmName hashAlgorithm, Stream source) + { + if (hashAlgorithm == HashAlgorithmName.SHA256) + { + return SHA256.HashData(source); + } + else if (hashAlgorithm == HashAlgorithmName.SHA1) + { + return SHA1.HashData(source); + } + else if (hashAlgorithm == HashAlgorithmName.SHA512) + { + return SHA512.HashData(source); + } + else if (hashAlgorithm == HashAlgorithmName.SHA384) + { + return SHA384.HashData(source); + } + else if (Helpers.HasMD5 && hashAlgorithm == HashAlgorithmName.MD5) + { + return MD5.HashData(source); + } + + throw new CryptographicException(SR.Format(SR.Cryptography_UnknownHashAlgorithm, hashAlgorithm.Name)); + } + + internal static int MacData( HashAlgorithmName hashAlgorithm, ReadOnlySpan key, ReadOnlySpan source, diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CapiHelper.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CapiHelper.Windows.cs index 2511c29..591470a 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CapiHelper.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/CapiHelper.Windows.cs @@ -1117,6 +1117,17 @@ namespace System.Security.Cryptography _ => throw new ArgumentException(SR.Argument_InvalidValue, nameof(hashAlg)), }; + internal static HashAlgorithmName AlgIdToHashAlgorithmName(int hashAlg) => + hashAlg switch + { + CapiHelper.CALG_MD5 => HashAlgorithmName.MD5, + CapiHelper.CALG_SHA1 => HashAlgorithmName.SHA1, + CapiHelper.CALG_SHA_256 => HashAlgorithmName.SHA256, + CapiHelper.CALG_SHA_384 => HashAlgorithmName.SHA384, + CapiHelper.CALG_SHA_512 => HashAlgorithmName.SHA512, + _ => throw new ArgumentException(SR.Argument_InvalidValue, nameof(hashAlg)), + }; + /// /// Convert an OID into a CAPI-1 CALG ID. /// diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/DSACryptoServiceProvider.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/DSACryptoServiceProvider.Windows.cs index 0a53d8a..9952580 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/DSACryptoServiceProvider.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/DSACryptoServiceProvider.Windows.cs @@ -15,7 +15,6 @@ namespace System.Security.Cryptography private readonly bool _randomKeyContainer; private SafeCapiKeyHandle? _safeKeyHandle; private SafeProvHandle? _safeProvHandle; - private readonly SHA1 _sha1; private static volatile CspProviderFlags s_useMachineKeyStore; private bool _disposed; @@ -79,7 +78,6 @@ namespace System.Security.Cryptography out _randomKeyContainer); _keySize = dwKeySize; - _sha1 = SHA1.Create(); // If this is not a random container we generate, create it eagerly // in the constructor so we can report any errors now. @@ -389,7 +387,7 @@ namespace System.Security.Cryptography /// The DSA signature for the specified data. public byte[] SignData(Stream inputStream) { - byte[] hashVal = _sha1.ComputeHash(inputStream); + byte[] hashVal = SHA1.HashData(inputStream); return SignHash(hashVal, null); } @@ -400,7 +398,7 @@ namespace System.Security.Cryptography /// The DSA signature for the specified data. public byte[] SignData(byte[] buffer) { - byte[] hashVal = _sha1.ComputeHash(buffer); + byte[] hashVal = SHA1.HashData(buffer); return SignHash(hashVal, null); } @@ -413,7 +411,7 @@ namespace System.Security.Cryptography /// The DSA signature for the specified data. public byte[] SignData(byte[] buffer, int offset, int count) { - byte[] hashVal = _sha1.ComputeHash(buffer, offset, count); + byte[] hashVal = SHA1.HashData(new ReadOnlySpan(buffer, offset, count)); return SignHash(hashVal, null); } @@ -425,7 +423,7 @@ namespace System.Security.Cryptography /// true if the signature verifies as valid; otherwise, false. public bool VerifyData(byte[] rgbData, byte[] rgbSignature) { - byte[] hashVal = _sha1.ComputeHash(rgbData); + byte[] hashVal = SHA1.HashData(rgbData); return VerifyHash(hashVal, null, rgbSignature); } @@ -457,7 +455,7 @@ namespace System.Security.Cryptography throw new CryptographicException(SR.Cryptography_UnknownHashAlgorithm, hashAlgorithm.Name); } - return _sha1.ComputeHash(data, offset, count); + return SHA1.HashData(new ReadOnlySpan(data, offset, count)); } protected override byte[] HashData(Stream data, HashAlgorithmName hashAlgorithm) @@ -471,7 +469,7 @@ namespace System.Security.Cryptography throw new CryptographicException(SR.Cryptography_UnknownHashAlgorithm, hashAlgorithm.Name); } - return _sha1.ComputeHash(data); + return SHA1.HashData(data); } /// @@ -489,8 +487,8 @@ namespace System.Security.Cryptography int calgHash = CapiHelper.NameOrOidToHashAlgId(str, OidGroup.HashAlgorithm); - if (rgbHash.Length != _sha1.HashSize / 8) - throw new CryptographicException(SR.Format(SR.Cryptography_InvalidHashSize, "SHA1", _sha1.HashSize / 8)); + if (rgbHash.Length != SHA1.HashSizeInBytes) + throw new CryptographicException(SR.Format(SR.Cryptography_InvalidHashSize, "SHA1", SHA1.HashSizeInBytes)); return CapiHelper.SignValue( SafeProvHandle, diff --git a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACryptoServiceProvider.Windows.cs b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACryptoServiceProvider.Windows.cs index e8706e6..ed045ea 100644 --- a/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACryptoServiceProvider.Windows.cs +++ b/src/libraries/System.Security.Cryptography/src/System/Security/Cryptography/RSACryptoServiceProvider.Windows.cs @@ -442,8 +442,8 @@ namespace System.Security.Cryptography public byte[] SignData(byte[] buffer, int offset, int count, object halg) { int calgHash = CapiHelper.ObjToHashAlgId(halg); - HashAlgorithm hash = CapiHelper.ObjToHashAlgorithm(halg); - byte[] hashVal = hash.ComputeHash(buffer, offset, count); + HashAlgorithmName hashAlgorithmName = CapiHelper.AlgIdToHashAlgorithmName(calgHash); + byte[] hashVal = HashOneShotHelpers.HashData(hashAlgorithmName, new ReadOnlySpan(buffer, offset, count)); return SignHash(hashVal, calgHash); } @@ -457,8 +457,8 @@ namespace System.Security.Cryptography public byte[] SignData(byte[] buffer, object halg) { int calgHash = CapiHelper.ObjToHashAlgId(halg); - HashAlgorithm hash = CapiHelper.ObjToHashAlgorithm(halg); - byte[] hashVal = hash.ComputeHash(buffer); + HashAlgorithmName hashAlgorithmName = CapiHelper.AlgIdToHashAlgorithmName(calgHash); + byte[] hashVal = HashOneShotHelpers.HashData(hashAlgorithmName, buffer); return SignHash(hashVal, calgHash); } @@ -472,8 +472,8 @@ namespace System.Security.Cryptography public byte[] SignData(Stream inputStream, object halg) { int calgHash = CapiHelper.ObjToHashAlgId(halg); - HashAlgorithm hash = CapiHelper.ObjToHashAlgorithm(halg); - byte[] hashVal = hash.ComputeHash(inputStream); + HashAlgorithmName hashAlgorithmName = CapiHelper.AlgIdToHashAlgorithmName(calgHash); + byte[] hashVal = HashOneShotHelpers.HashData(hashAlgorithmName, inputStream); return SignHash(hashVal, calgHash); } @@ -522,8 +522,8 @@ namespace System.Security.Cryptography public bool VerifyData(byte[] buffer, object halg, byte[] signature) { int calgHash = CapiHelper.ObjToHashAlgId(halg); - HashAlgorithm hash = CapiHelper.ObjToHashAlgorithm(halg); - byte[] hashVal = hash.ComputeHash(buffer); + HashAlgorithmName hashAlgorithmName = CapiHelper.AlgIdToHashAlgorithmName(calgHash); + byte[] hashVal = HashOneShotHelpers.HashData(hashAlgorithmName, buffer); return VerifyHash(hashVal, calgHash, signature); } -- 2.7.4