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

Improve distribution of CoseHeaderLabel hash codes #70695

Merged
merged 2 commits into from
Jun 13, 2022
Merged
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
58 changes: 58 additions & 0 deletions src/libraries/Common/src/System/HashCodeRandomization.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace System
{
// Contains helpers for calculating randomized hash codes of common types.
// Since these hash codes are randomized, callers must not persist them between
// AppDomain restarts. There's still the potential for limited collisions
// if two distinct types have the same bit pattern (e.g., string.Empty and (int)0).
// This should be acceptable because the number of practical collisions is
// limited by the number of distinct types used here, and we expect callers to
// have a small, fixed set of accepted types for any hash-based collection.
// If we really do need to address this in the future, we can use a seed per type
// rather than a global seed for the entire AppDomain.
internal static class HashCodeRandomization
{
public static int GetRandomizedOrdinalHashCode(this string value)
{
#if NETCOREAPP
// In .NET Core, string hash codes are already randomized.

return value.GetHashCode();
#else
// Downlevel, we need to perform randomization ourselves. There's still
// the potential for limited collisions ("Hello!" and "Hello!\0"), but
// this shouldn't be a problem in practice. If we need to address it,
// we can mix the string length into the accumulator before running the
// string contents through.
//
// We'll pull out pairs of chars and write 32 bits at a time.

HashCode hashCode = default;
int pair = 0;
for (int i = 0; i < value.Length; i++)
{
int ch = value[i];
if ((i & 1) == 0)
{
pair = ch << 16; // first member of pair
}
else
{
pair |= ch; // second member of pair
hashCode.Add(pair); // write pair as single unit
pair = 0;
}
}
hashCode.Add(pair); // flush any leftover data (could be 0 or 1 chars)
return hashCode.ToHashCode();
GrabYourPitchforks marked this conversation as resolved.
Show resolved Hide resolved
#endif
}

public static int GetRandomizedHashCode(this int value)
{
return HashCode.Combine(value);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
</PropertyGroup>

<ItemGroup>
<Compile Include="$(CommonPath)System\HashCodeRandomization.cs" Link="Common\System\HashCodeRandomization.cs" />
<Compile Include="$(CommonPath)System\Memory\PointerMemoryManager.cs" Link="Common\System\Memory\PointerMemoryManager.cs" />
<Compile Include="$(LibrariesProjectRoot)System.Formats.Cbor\src\System\Formats\Cbor\CborInitialByte.cs" Link="System\Formats\Cbor\CborInitialByte.cs" />
<Compile Include="System\Security\Cryptography\Cose\CoseHeaderLabel.cs" />
Expand All @@ -32,6 +33,11 @@
<ProjectReference Include="$(LibrariesProjectRoot)System.Formats.Cbor\src\System.Formats.Cbor.csproj" />
</ItemGroup>

<!-- For System.HashCode -->
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
<PackageReference Include="Microsoft.Bcl.HashCode" Version="$(MicrosoftBclHashCodeVersion)" />
</ItemGroup>

<!-- For IncrementalHash -->
<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETFramework'">
<PackageReference Include="System.Security.Cryptography.Algorithms" Version="$(SystemSecurityCryptographyAlgorithmsVersion)" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,16 @@ public bool Equals(CoseHeaderLabel other)

public override int GetHashCode()
{
// Since this type is used as a key in a dictionary (see CoseHeaderMap)
// and since the label is potentially adversary-provided, we'll need
// to randomize the hash code.

if (LabelAsString != null)
{
return LabelAsString.GetHashCode();
return LabelAsString.GetRandomizedOrdinalHashCode();
}

return LabelAsInt32.GetHashCode();
return LabelAsInt32.GetRandomizedHashCode();
}

public static bool operator ==(CoseHeaderLabel left, CoseHeaderLabel right) => left.Equals(right);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,35 @@ public class CoseHeaderLabelTests
[Fact]
public void CoseHeaderLabel_GetHashCode()
{
CoseHeaderLabel label = default;
Assert.Equal(0, label.GetHashCode()); // There's no need to call GetHashCode on integers as they are their own hashcode.
// First, construct a COSE header of (0) using several different methods.
// They should all have the same hash code, though we don't know what
// the hash code is.

label = new CoseHeaderLabel();
Assert.Equal(0, label.GetHashCode());
CoseHeaderLabel label1 = default;
CoseHeaderLabel label2 = new CoseHeaderLabel();
CoseHeaderLabel label3 = new CoseHeaderLabel(0);

label = new CoseHeaderLabel(0);
Assert.Equal(0, label.GetHashCode());
int label1HashCode = label1.GetHashCode();
Assert.Equal(label1HashCode, label2.GetHashCode());
Assert.Equal(label1HashCode, label3.GetHashCode());

label = new CoseHeaderLabel("");
Assert.Equal("".GetHashCode(), label.GetHashCode());
// Make sure the integer hash code calculation really is randomized.
// Checking 1 & 2 together rather than independently cuts the false
// positive rate down to nearly 1 in 2^64.

label = new CoseHeaderLabel(1);
Assert.Equal(1, label.GetHashCode());
bool isReturningNormalInt32HashCode =
(new CoseHeaderLabel(1).GetHashCode() == 1)
&& (new CoseHeaderLabel(2).GetHashCode() == 2);
Assert.False(isReturningNormalInt32HashCode);

label = new CoseHeaderLabel("content-type");
Assert.Equal("content-type".GetHashCode(), label.GetHashCode());
// Make sure the string hash code calculation really is randomized.
// Checking 1 & 2 together rather than independently cuts the false
// positive rate down to nearly 1 in 2^64.

bool isReturningNormalStringHashCode =
(new CoseHeaderLabel("Hello").GetHashCode() == "Hello".GetHashCode())
&& (new CoseHeaderLabel("World").GetHashCode() == "World".GetHashCode());
Assert.Equal(PlatformDetection.IsNetCore, isReturningNormalStringHashCode);
}

[Fact]
Expand Down