Refactor DataProtection trimming

[JamesNK]

Fixes #41411
Fixes #24705

Result of the discussion with Levi and Hao about how DataProtection is used:

• Custom type names are rare
• Registry policy is also rare. Mostly used by administrators who want to override crypto settings machine-wide. Adding a config setting to opt into registry policy isn't ideal for this situation because now the machine admin also needs to know that all apps have the setting enabled.

Since custom type names are rare, this PR removes build warnings. It's possible to get runtime errors, but this change tries to make them useful.

This PR:

• Removes trimming warnings from public APIs
• Suppresses warnings at a lower level
• Provides friendly error message if a custom type name is used and trimmed
• Statically references any built-in types, e.g. deserializers, encryptors

[HaoK]

Can we add a test somewhere to verify the warning experience, i.e. a trimmed app test that references a type that fails to deserialize?

[JamesNK]

Testing trimming is difficult because you need to build a new app, publish it, then run it.

However, I can add a test that verifies a type that can't be found throws the friendly error message.

[davidfowl]

Is this netstandard 2.0? These APIs aren't trim friendly.

[JamesNK]

Yes, it targets netstandard2.0.

What specifically isn't trim-friendly? No warnings are coming from the linker.

[davidfowl]

My bad, I meant that RuntimeInformation.IsOSPlatform(OSPlatform.Windows) isn't recognized by the trimmer. Those branches will exist when this is trimmed and we're targeting linux.

[JamesNK]

Got it. I added IsOSPlatform to suppress errors from build, not for trimming.

A couple of types won't matter. I don't think introducing a net6-window target is worth it.

[davidfowl]

This might still fail in weird ways. Even if the external type is loaded, it would be missing things that it depends on that weren't preserved. We're going to let that fail randomly in that case right? (TBH, nothing can be done about that...)

[JamesNK]

Agreed.

Nice thing is these changes are internal and conservative. If people run into problems we can't revisit them in the future.

[khellang]

Hey @JamesNK! 👋🏻

I'm pretty sure these changes broke cross-version forwarding when using Azure.Extensions.AspNetCore.DataProtection.Keys. See Azure/azure-sdk-for-net#42223.

The code in XmlEncryptionExtensions used to call IActivator.CreateInstance directly. Since the KeyVault extensions register their own IActivator, which handles their own forwarding, it worked across versions, but after this change, it tries to call Type.GetType first, which results in the following System.IO.FileLoadException:

at System.RuntimeTypeHandle.GetTypeByName(String name, Boolean throwOnError, Boolean ignoreCase, Boolean reflectionOnly, StackCrawlMarkHandle stackMark, IntPtr pPrivHostBinder, Boolean loadTypeFromPartialName, ObjectHandleOnStack type)
   at System.RuntimeTypeHandle.GetTypeByName(String name, Boolean throwOnError, Boolean ignoreCase, Boolean reflectionOnly, StackCrawlMark& stackMark, IntPtr pPrivHostBinder, Boolean loadTypeFromPartialName)
   at System.RuntimeType.GetType(String typeName, Boolean throwOnError, Boolean ignoreCase, Boolean reflectionOnly, StackCrawlMark& stackMark)
   at System.Type.GetType(String typeName, Boolean throwOnError)
   at Microsoft.AspNetCore.DataProtection.XmlEncryption.XmlEncryptionExtensions.CreateDecryptor(IActivator activator, String decryptorTypeName)
   at Microsoft.AspNetCore.DataProtection.XmlEncryption.XmlEncryptionExtensions.DecryptElement(XElement element, IActivator activator)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.XmlKeyManager.Microsoft.AspNetCore.DataProtection.KeyManagement.Internal.IInternalXmlKeyManager.DeserializeDescriptorFromKeyElement(XElement keyElement)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.DeferredKey.<>c__DisplayClass1_0.<GetLazyDescriptorDelegate>g__GetLazyDescriptorDelegate|0()
   at System.Lazy`1.CreateValue()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Lazy`1.get_Value()
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyBase.get_Descriptor()
   at Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.CngGcmAuthenticatedEncryptorFactory.CreateEncryptorInstance(IKey key)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyBase.CreateEncryptor()
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRing.KeyHolder.GetEncryptorInstance(Boolean& isRevoked)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRing.GetAuthenticatedEncryptorByKeyId(Guid keyId, Boolean& isRevoked)
   at Microsoft.AspNetCore.DataProtection.KeyManagement.KeyRingBasedDataProtector.UnprotectCore(Byte[] protectedData, Boolean allowOperationsOnRevokedKeys, UnprotectStatus& status)

Since the old version of the decryptor cannot be found.

I'd send a PR to fix this, but I'm not 100% sure what the intention of the change in this PR was and how to work around it.

[JamesNK]

Hi

Create an issue with what you just said. PR welcome. @amcasey is working on data protection recently. Include both of us on the issue and PR.