Skip to content
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

Avoid finalizable internal state for non-FIPS scenarios #67998

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Compilers/Core/Portable/CryptographicHashProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ internal static ImmutableArray<byte> ComputeSha1(byte[] bytes)

internal static ImmutableArray<byte> ComputeHash(HashAlgorithmName algorithmName, IEnumerable<Blob> bytes)
{
using (var incrementalHash = IncrementalHash.CreateHash(algorithmName))
using (var incrementalHash = RoslynIncrementalHash.CreateHash(algorithmName))
{
incrementalHash.AppendData(bytes);
return ImmutableArray.Create(incrementalHash.GetHashAndReset());
Expand All @@ -196,7 +196,7 @@ internal static ImmutableArray<byte> ComputeHash(HashAlgorithmName algorithmName

internal static ImmutableArray<byte> ComputeHash(HashAlgorithmName algorithmName, IEnumerable<ArraySegment<byte>> bytes)
{
using (var incrementalHash = IncrementalHash.CreateHash(algorithmName))
using (var incrementalHash = RoslynIncrementalHash.CreateHash(algorithmName))
{
incrementalHash.AppendData(bytes);
return ImmutableArray.Create(incrementalHash.GetHashAndReset());
Expand All @@ -206,7 +206,7 @@ internal static ImmutableArray<byte> ComputeHash(HashAlgorithmName algorithmName
internal static ImmutableArray<byte> ComputeSourceHash(ImmutableArray<byte> bytes, SourceHashAlgorithm hashAlgorithm = SourceHashAlgorithms.Default)
{
var algorithmName = GetAlgorithmName(hashAlgorithm);
using (var incrementalHash = IncrementalHash.CreateHash(algorithmName))
using (var incrementalHash = RoslynIncrementalHash.CreateHash(algorithmName))
{
incrementalHash.AppendData(bytes.ToArray());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yikes. feels like this should be better on Core as well, by using available hash functions that don't require tehse intermediary allocations.

return ImmutableArray.Create(incrementalHash.GetHashAndReset());
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Core/Portable/Emit/EmitOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,7 @@ internal void ValidateOptions(DiagnosticBag diagnostics, CommonMessageProvider m
{
try
{
IncrementalHash.CreateHash(PdbChecksumAlgorithm).Dispose();
RoslynIncrementalHash.CreateHash(PdbChecksumAlgorithm).Dispose();
}
catch
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Diagnostics;
using System.Runtime.Versioning;
using System.Security.Cryptography;

namespace Roslyn.Utilities;

/// <summary>Provides support for computing a hash value incrementally across several segments.</summary>
internal sealed class RoslynIncrementalHash : IDisposable
{
private const int NTE_BAD_ALGID = -2146893816;

private readonly HashAlgorithmName _algorithmName;
private HashAlgorithm _hash;
private bool _disposed;
private bool _resetPending;

/// <summary>Gets the name of the algorithm being performed.</summary>
/// <value>The name of the algorithm being performed.</value>
public HashAlgorithmName AlgorithmName => _algorithmName;

private RoslynIncrementalHash(HashAlgorithmName name, HashAlgorithm hash)
{
_algorithmName = name;
_hash = hash;
}

/// <summary>Appends the specified data to the data already processed in the hash or HMAC.</summary>
/// <param name="data">The data to process.</param>
/// <exception cref="ArgumentNullException"><paramref name="data"/> is <see langword="null"/>.</exception>
/// <exception cref="ObjectDisposedException">The <see cref="RoslynIncrementalHash"/> object has already been disposed.</exception>
public void AppendData(byte[] data)
{
if (data == null)
{
throw new ArgumentNullException(nameof(data));
}

AppendData(data, 0, data.Length);
}

/// <summary>Appends the specified number of bytes from the specified data, starting at the specified offset, to the
/// data already processed in the hash.</summary>
/// <param name="data">The data to process.</param>
/// <param name="offset">The offset into the byte array from which to begin using data.</param>
/// <param name="count">The number of bytes to use from <paramref name="data"/>.</param>
/// <exception cref="ArgumentNullException"><paramref name="data"/> is <see langword="null"/>.</exception>
/// <exception cref="ArgumentOutOfRangeException"><paramref name="count"/> or <paramref name="offset"/> is negative.
/// -or-
/// <paramref name="count"/> is larger than the length of <paramref name="data"/>.</exception>
/// <exception cref="ArgumentException">The sum of <paramref name="offset"/> and <paramref name="count"/> is larger than the data length.</exception>
/// <exception cref="ObjectDisposedException">The <see cref="RoslynIncrementalHash"/> object has already been disposed.</exception>
public void AppendData(byte[] data, int offset, int count)
{
if (data == null)
{
throw new ArgumentNullException(nameof(data));
}

if (offset < 0)
{
throw new ArgumentOutOfRangeException(nameof(offset), "ArgumentOutOfRange_NeedNonNegNum");
}

if (count < 0 || count > data.Length)
{
throw new ArgumentOutOfRangeException(nameof(count));
}

if (data.Length - count < offset)
{
throw new ArgumentException("Argument_InvalidOffLen");
}

if (_disposed)
{
throw new ObjectDisposedException(typeof(RoslynIncrementalHash).Name);
}

if (_resetPending)
{
_hash.Initialize();
_resetPending = false;
}

_hash.TransformBlock(data, offset, count, null, 0);
}

/// <summary>Retrieves the hash for the data accumulated from prior calls to the <see cref="AppendData(byte[])"/>
/// method, and resets the object to its initial state.</summary>
/// <returns>The computed hash or HMAC.</returns>
/// <exception cref="ObjectDisposedException">The <see cref="RoslynIncrementalHash"/> object has already been disposed.</exception>
public byte[] GetHashAndReset()
{
if (_disposed)
{
throw new ObjectDisposedException(typeof(RoslynIncrementalHash).Name);
}

if (_resetPending)
{
_hash.Initialize();
}

_hash.TransformFinalBlock(Array.Empty<byte>(), 0, 0);
byte[] hash = _hash.Hash!;
_resetPending = true;
return hash;
}

/// <summary>Releases the resources used by the current instance of the <see cref="RoslynIncrementalHash"/> class.</summary>
public void Dispose()
{
_disposed = true;
if (_hash != null)
{
_hash.Dispose();
_hash = null!;
}
}

/// <summary>Creates a <see cref="RoslynIncrementalHash"/> for the specified algorithm.</summary>
/// <param name="hashAlgorithm">The name of the hash algorithm to perform.</param>
/// <returns>An <see cref="RoslynIncrementalHash"/> instance ready to compute the hash algorithm specified by <paramref name="hashAlgorithm"/>.</returns>
/// <exception cref="ArgumentException"><paramref name="hashAlgorithm"/>.<see cref="HashAlgorithmName.Name"/> is <see langword="null"/> or an empty string.</exception>
/// <exception cref="CryptographicException"><paramref name="hashAlgorithm" /> is not a known hash algorithm.</exception>
public static RoslynIncrementalHash CreateHash(HashAlgorithmName hashAlgorithm)
{
if (string.IsNullOrEmpty(hashAlgorithm.Name))
{
throw new ArgumentException("Cryptography_HashAlgorithmNameNullOrEmpty", nameof(hashAlgorithm));
}
return new RoslynIncrementalHash(hashAlgorithm, GetHashAlgorithm(hashAlgorithm));
}

private static HashAlgorithm GetHashAlgorithm(HashAlgorithmName hashAlgorithm)
{
if (hashAlgorithm == HashAlgorithmName.MD5)
{
return MD5.Create();
}

if (hashAlgorithm == HashAlgorithmName.SHA1)
{
return SHA1.Create();
}

if (hashAlgorithm == HashAlgorithmName.SHA256)
{
return SHA256.Create();
}

if (hashAlgorithm == HashAlgorithmName.SHA384)
{
return SHA384.Create();
}

if (hashAlgorithm == HashAlgorithmName.SHA512)
{
return SHA512.Create();
}
Comment on lines +142 to +165
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 The change relative to the reference implementation is here. .NET Framework creates instances of SHA256CryptoServiceProvider instead of calling SHA256.Create() (likewise for each of the others).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 In theory, we could also address this by modifying the way we create our pool of IncrementalHash objects to use reflection to reset the _hash field.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and it's hte provider that was causing the problem before?


throw new CryptographicException(NTE_BAD_ALGID);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,28 @@
using System;
using System.Collections.Generic;
using System.Reflection.Metadata;
using System.Security.Cryptography;

namespace Roslyn.Utilities
{
internal static class IncrementalHashExtensions
internal static class RoslynIncrementalHashExtensions
{
internal static void AppendData(this IncrementalHash hash, IEnumerable<Blob> blobs)
internal static void AppendData(this RoslynIncrementalHash hash, IEnumerable<Blob> blobs)
{
foreach (var blob in blobs)
{
hash.AppendData(blob.GetBytes());
}
}

internal static void AppendData(this IncrementalHash hash, IEnumerable<ArraySegment<byte>> blobs)
internal static void AppendData(this RoslynIncrementalHash hash, IEnumerable<ArraySegment<byte>> blobs)
{
foreach (var blob in blobs)
{
hash.AppendData(blob);
}
}

internal static void AppendData(this IncrementalHash hash, ArraySegment<byte> segment)
internal static void AppendData(this RoslynIncrementalHash hash, ArraySegment<byte> segment)
{
RoslynDebug.AssertNotNull(segment.Array);
hash.AppendData(segment.Array, segment.Offset, segment.Count);
Expand Down
2 changes: 1 addition & 1 deletion src/Compilers/Core/Portable/PEWriter/SigningUtilities.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ internal static byte[] CalculateRsaSignature(IEnumerable<Blob> content, RSAParam

internal static byte[] CalculateSha1(IEnumerable<Blob> content)
{
using (var hash = IncrementalHash.CreateHash(HashAlgorithmName.SHA1))
using (var hash = RoslynIncrementalHash.CreateHash(HashAlgorithmName.SHA1))
{
hash.AppendData(content);
return hash.GetHashAndReset();
Expand Down
3 changes: 2 additions & 1 deletion src/Compilers/Test/Core/Metadata/ILValidation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
using Microsoft.CodeAnalysis.Text;
using Microsoft.DiaSymReader.Tools;
using Microsoft.Metadata.Tools;
using Roslyn.Utilities;

namespace Roslyn.Test.Utilities
{
Expand Down Expand Up @@ -145,7 +146,7 @@ private static byte[] ComputeSigningHash(
buffer[authenticodeOffset + i] = 0;
}

using (var hash = IncrementalHash.CreateHash(HashAlgorithmName.SHA1))
using (var hash = RoslynIncrementalHash.CreateHash(HashAlgorithmName.SHA1))
{
// First hash the DOS header and PE headers
hash.AppendData(buffer, 0, peHeadersSize);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ internal partial class Checksum
// https://github.com/dotnet/runtime/blob/f2db6d6093c54e5eeb9db2d8dcbe15b2db92ad8c/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/SHA256.cs#L18-L19
private const int SHA256HashSizeBytes = 256 / 8;

private static readonly ObjectPool<IncrementalHash> s_incrementalHashPool =
new(() => IncrementalHash.CreateHash(HashAlgorithmName.SHA256), size: 20);
private static readonly ObjectPool<RoslynIncrementalHash> s_incrementalHashPool =
new(() => RoslynIncrementalHash.CreateHash(HashAlgorithmName.SHA256), size: 20);

// Dedicated pools for the byte[]s we use to create checksums from two or three existing checksums. Sized to
// exactly the space needed to splat the existing checksum data into the array and then hash it.
Expand Down Expand Up @@ -234,7 +234,7 @@ public static Checksum Create(ParseOptions value, ISerializerService serializer)
return Create(stream);
}

private static void AppendData(IncrementalHash hash, byte[] buffer, string value)
private static void AppendData(RoslynIncrementalHash hash, byte[] buffer, string value)
{
var stringBytes = MemoryMarshal.AsBytes(value.AsSpan());
Debug.Assert(stringBytes.Length == value.Length * 2);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
<Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\InternalUtilities\RequiredMemberAttribute.cs" Link="InternalUtilities\RequiredMemberAttribute.cs" />
<Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\InternalUtilities\SetsRequiredMembersAttribute.cs" Link="InternalUtilities\SetsRequiredMembersAttribute.cs" />
<Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\InternalUtilities\ReferenceEqualityComparer.cs" Link="InternalUtilities\ReferenceEqualityComparer.cs" />
<Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\InternalUtilities\RoslynIncrementalHash.cs" Link="InternalUtilities\RoslynIncrementalHash.cs" />
<Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\InternalUtilities\RoslynString.cs" Link="InternalUtilities\RoslynString.cs" />
<Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\InternalUtilities\SpecializedCollections.cs" Link="InternalUtilities\SpecializedCollections.cs" />
<Compile Include="$(MSBuildThisFileDirectory)..\..\..\..\Compilers\Core\Portable\InternalUtilities\SharedStopwatch.cs" Link="InternalUtilities\SharedStopwatch.cs" />
Expand Down