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

Public Key Token calculation is incorrect #3153

Closed
marklio opened this issue Jan 10, 2024 · 1 comment
Closed

Public Key Token calculation is incorrect #3153

marklio opened this issue Jan 10, 2024 · 1 comment
Labels

Comments

@marklio
Copy link

marklio commented Jan 10, 2024

Steps to reproduce

  1. Inspect the code at
    static HashAlgorithm GetHashAlgorithm(this MetadataReader reader)
    {
    switch (reader.GetAssemblyDefinition().HashAlgorithm)
    {
    case AssemblyHashAlgorithm.None:
    // only for multi-module assemblies?
    return SHA1.Create();
    case AssemblyHashAlgorithm.MD5:
    return MD5.Create();
    case AssemblyHashAlgorithm.Sha1:
    return SHA1.Create();
    case AssemblyHashAlgorithm.Sha256:
    return SHA256.Create();
    case AssemblyHashAlgorithm.Sha384:
    return SHA384.Create();
    case AssemblyHashAlgorithm.Sha512:
    return SHA512.Create();
    default:
    return SHA1.Create(); // default?
    }
    }
    static string CalculatePublicKeyToken(BlobHandle blob, MetadataReader reader)
    {
    // Calculate public key token:
    // 1. hash the public key using the appropriate algorithm.
    byte[] publicKeyTokenBytes = reader.GetHashAlgorithm().ComputeHash(reader.GetBlobBytes(blob));
    // 2. take the last 8 bytes
    // 3. according to Cecil we need to reverse them, other sources did not mention this.
    return publicKeyTokenBytes.TakeLast(8).Reverse().ToHexString(8);
    }

Error message shown

The implementation uses the assembly's hash algorithm id to choose which hash algorithm to use when hashing the public key. This is incorrect. public key tokens are always calculated using a SHA1 hash of the public key.

Details

Since SHA1 is the default, and it is pretty difficult to successfully create a non-SHA1 assembly, the current implementation almost always works.

Here's a straightforward implementation from .NET itself: https://github.com/dotnet/runtime/blob/cc5f1df48e5e39a6fa5ab94ca4664aac3cc36898/src/libraries/System.Private.CoreLib/src/System/Reflection/AssemblyNameHelpers.StrongName.cs#L11C1-L33C10

You can see that SHA1 is always used. Recall that assembly references can capture a public key without the ability to capture a hash algorithm, so it must be calculatable without that. Also, publishers must be able to change the hash algorithm in future releases without changing an assembly's identity.

@siegfriedpammer
Copy link
Member

Thank you for reporting this inconsistency. Note that there are two separate APIs that calculate hashes. The other API was always using SHA1. I just misinterpreted the assembly metadata fields when implementing the System.Reflection.Metadata-based API.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2024
mattsh247 pushed a commit to mattsh247/ILSpy that referenced this issue Jul 30, 2024
…g to ECMA-335, the hash algorithm stored in the assembly metadata is intended for file content verification purposes, not identification purposes.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants