SignedXml does not work by default in trimmed applications

Open
#97,274 6 comments 4 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
5/5
Estimated time
Over a week
Newbie friendliness
25/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
csharp

Research direction

Start with SignedXml's use of CryptoConfig and reproduce the reported CryptographicException in a trimmed application. Compare the available options for preserving hash algorithms and signature formatters or improving the diagnostic; done should mean the trimmed SignedXml scenario no longer fails unexpectedly or clearly explains the required roots.

Written by the indexing model from the issue text.

Description

area-System.Security linkable-framework

If you use SignedXml within a trimmed application, the out-of-the-box experience is going to be a CryptographicException.

This is because SignedXml uses CryptoConfig to resolve hash algorithm identifiers (URIs) to an appropriate hash implementation. The trimmer cannot see through this, and even if it could, it could not handle scenarios where the algorithm is part of the document input. The trimmer ends up removing various pieces from System.Security.Cryptography that SignedXml actually needs. This includes types like RSAPKCS1SignatureFormatter and RSAPKCS1SignatureDeformatter, SHA1Managed, etc.

The error you get in this situation is very difficult to resolve.

System.Security.Cryptography.CryptographicException: Could not create hash algorithm object. If the application has been trimmed, ensure the required algorithm implementations are preserved.

What hash algorithm? It actually needs to be the *Managed ones because that's what CryptoConfig resolves for legacy purposes. If you just add S.S.C.SHA1 to your root list, it's not going to work. Without a detailed knowledge of how CryptoConfig and the runtime works, it's difficult to solve this problem on your own. How is someone supposed to know they need to add an obsolete class to their roots?

I am sure we can fix CryptoConfig to resolve things to better algorithms in some situations, or we can improve the error message. You fix one, then you move on to the next error about signature formatters. That presents as a null-ref.

One option is we add DynamicDependencyAttribute to SignedXml for common things that get end up getting resolved by CryptoConfig.

Something like:

partial class SignedXml {
    [DynamicallyAccessedMemberTypes.All, "System.Security.Cryptography.SHA1Managed", "System.Security.Cryptography.Algorithms")]
    [DynamicallyAccessedMemberTypes.All, "System.Security.Cryptography.SHA256Managed", "System.Security.Cryptography.Algorithms")]
    [DynamicallyAccessedMemberTypes.All, "System.Security.Cryptography.RSAPKCS1SignatureFormatter", "System.Security.Cryptography.Algorithms")]
    [DynamicallyAccessedMemberTypes.All, "System.Security.Cryptography.RSAPKCS1SignatureDeformatter", "System.Security.Cryptography.Algorithms")]
    // SHA-384 and SHA-512 too, I suppose.
    public SignedXml(XmlDocument doc);

    // Repeat for other constructors or methods
}

The knee-jerk reaction here might be to say "But this ends up rooting types that might not be used" which is correct. If you never use SHA-1, SHA-1 is rooted for no good reason. It's on the "small" side, we aren't rooting an actual implementation of SHA-1, just a public shim around an internal algorithm that dispatches by name.

This is a common issue with other cryptographic stacks. OIDs in various things like CMS have the same problem. We don't know the hash algorithm in a document until we try to process the document at runtime.

You could also say, "But if someone changes their CryptoConfig to use something else, we've rooted these things for nothing" which, yes, is also correct. But I believe this to be an uncommon scenario.

I also believe that if we were building S.S.C.Xml from scratch without CryptoConfig, it would look something like this:

HashAlgorithm alg = hasher switch {
    "http://www.w3.org/2000/09/xmldsig#sha1" => SHA1.Create(),
    "http://www.w3.org/2001/04/xmlenc#sha256" => SHA256.Create(),
    // etc
    _ => throw new CryptographicException(SR.IDontKnowWhatHashYouAreTalkingAbout),
};

Which leads us back to rooting hash algorithms.


An alternative thing to do is to handle null better when we get back from CryptoConfig, like GetSha1FromCryptoConfig() ?? SHA1.Create() but this is going to end up rooting common algorithms anyway through the factory method. Likewise for the signature formatters.

A final alternative option is to not fix this - but instead (significantly) improve the error along with a link to documentation explaining which things they need to manually add to their roots.

Dominant language
C#
Stars
18.3k
Forks
5.6k
Avg merge
2d 19h
Merged PRs (30d)
589

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from dotnet/runtime

All issues in dotnet/runtime

Similar issues

More C# issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.