DataProtection isn't linker friendly

[davidfowl]

Here are the warnings:

C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\RegistryPolicyResolver.cs(40,47): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.RegistryPolicyResolver.PopulateOptions(Object,RegistryKey): The return value of method 'System.Object.GetType()' with dynamically accessed member kinds 'None' is passed into the implicit 'this' parameter of method 'System.Type.GetProperties()' which requires dynamically accessed member kinds 'PublicProperties'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'PublicProperties'. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\RegistryPolicyResolver.cs(57,29): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.RegistryPolicyResolver.PopulateOptions(Object,RegistryKey): Reflection call 'System.Type.GetType(String,Boolean)' inside 'Microsoft.AspNetCore.DataProtection.RegistryPolicyResolver.PopulateOptions(Object,RegistryKey)' was detected with unknown value for the type name. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\RegistryPolicyResolver.cs(82,25): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.RegistryPolicyResolver.ReadKeyEscrowSinks(RegistryKey): Reflection call 'System.Type.GetType(String,Boolean)' inside 'Microsoft.AspNetCore.DataProtection.RegistryPolicyResolver.ReadKeyEscrowSinks(RegistryKey)' was detected with unknown value for the type name. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\SimpleActivator.cs(31,13): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.SimpleActivator.CreateInstance(Type,String): Reflection call 'System.Type.GetType(String,Boolean)' inside 'Microsoft.AspNetCore.DataProtection.SimpleActivator.CreateInstance(Type,String)' was detected with unknown value for the type name. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\SimpleActivator.cs(37,17): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.SimpleActivator.CreateInstance(Type,String): The return value of method 'System.Type.GetType(String,Boolean)' with dynamically accessed member kinds 'None' is passed into the implicit 'this' parameter of method 'System.Type.GetConstructor(Type[])' which requires dynamically accessed member kinds 'PublicParameterlessConstructor'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'PublicParameterlessConstructor'. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\SimpleActivator.cs(40,21): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.SimpleActivator.CreateInstance(Type,String): The return value of method 'System.Type.GetType(String,Boolean)' with dynamically accessed member kinds 'None' is passed into the parameter #0 of method 'System.Object System.Activator::CreateInstance(System.Type)' which requires dynamically accessed member kinds 'PublicParameterlessConstructor'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'PublicParameterlessConstructor'. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\SimpleActivator.cs(45,13): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.SimpleActivator.CreateInstance(Type,String): The return value of method 'System.Type.GetType(String,Boolean)' with dynamically accessed member kinds 'None' is passed into the implicit 'this' parameter of method 'System.Type.GetConstructor(Type[])' which requires dynamically accessed member kinds 'PublicConstructors'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'PublicConstructors'. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\SimpleActivator.cs(53,13): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.SimpleActivator.CreateInstance(Type,String): The return value of method 'System.Type.GetType(String,Boolean)' with dynamically accessed member kinds 'None' is passed into the parameter #0 of method 'System.Object System.Activator::CreateInstance(System.Type)' which requires dynamically accessed member kinds 'PublicParameterlessConstructor'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'PublicParameterlessConstructor'. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\TypeForwardingActivator.cs(54,17): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.TypeForwardingActivator.CreateInstance(Type,String,Boolean&): Reflection call 'System.Type.GetType(String,Boolean)' inside 'Microsoft.AspNetCore.DataProtection.TypeForwardingActivator.CreateInstance(Type,String,Boolean&)' was detected with unknown value for the type name. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\TypeForwardingActivator.cs(54,17): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.TypeForwardingActivator.CreateInstance(Type,String,Boolean&): Reflection call 'System.Type.GetType(String,Boolean)' inside 'Microsoft.AspNetCore.DataProtection.TypeForwardingActivator.CreateInstance(Type,String,Boolean&)' was detected with unknown value for the type name. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\AuthenticatedEncryption\ManagedAuthenticatedEncryptorFactory.cs(128,51): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.ManagedAuthenticatedEncryptorFactory.AlgorithmActivator.AlgorithmActivatorCore<T>.AlgorithmActivatorCore`1(): The generic parameter 'T' from 'Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.ManagedAuthenticatedEncryptorFactory.AlgorithmActivator.AlgorithmActivatorCore<T>' with dynamically accessed member kinds 'None' is passed into the generic parameter 'T' from 'System.Activator.CreateInstance<T>()' which requires dynamically accessed member kinds 'PublicParameterlessConstructor'. To fix this add DynamicallyAccessedMembersAttribute to it and specify at least these member kinds 'PublicParameterlessConstructor'. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]
C:\dev\git\aspnetcore\src\DataProtection\DataProtection\src\AuthenticatedEncryption\ConfigurationModel\ManagedAuthenticatedEncryptorDescriptorDeserializer.cs(73,17): Trim analysis warning IL2006: Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.ConfigurationModel.ManagedAuthenticatedEncryptorDescriptorDeserializer.FriendlyNameToType(String): Reflection call 'System.Type.GetType(String,Boolean)' inside 'Microsoft.AspNetCore.DataProtection.AuthenticatedEncryption.ConfigurationModel.ManagedAuthenticatedEncryptorDescriptorDeserializer.FriendlyNameToType(String)' was detected with unknown value for the type name. [C:\Users\davifowl\source\repos\WebApplication60\WebApplication60\WebApplication60.csproj]

Generally, DataProtection tries to load types from configuration, but these types are part of data protection itself so we can preserve these types so that loading works. Funnily enough, if we used the DI system, this wouldn't be a problem 😄

Components with dependencies on DataProtection:

• Microsoft.AspNetCore.Authentication.Cookies
• Microsoft.AspNetCore.Session
• Microsoft.AspNetCore.Authentication
• Microsoft.AspNetCore.Components.Web.Extensions
• Microsoft.AspNetCore.Antiforgery
• Microsoft.AspNetCore.Mvc.ViewFeatures
• Microsoft.AspNetCore.Components.Server

[blowdart]

Well that's what you get for needing back compat to systems where DI doesn't exist :)

[davidfowl]

Well that's what you get for needing back compat to systems where DI doesn't exist :)

Hah, I was looking at the code and its unfortunately some of the most reflection heavy code in the System. That never occurred to me before...

[blowdart]

It was also written when, well DI was crap, and if you look at some the issues, the response was roll your own reflection.

However setting that side it's also policy based, where an admin outside of an application can place restrictions on the algorithms used and other key decisions that affect control flow. This is needed for some regulated environments. This type of external control does not lend itself to trimming, or indeed interop with other systems. Say for example, you trimmed app 1 to only support SHA256 because that's all trimming detected, but it also talks to app2 which sends SHA512. You'd going to have unknown and probably very hard to trace errors from that. Given it's data protection it has to fail closed/safe.

Data Protection is probably not a target for trimming, not without a real concerted effort and careful design.

[davidfowl]

I don't know what you mean by when the DI system was crap, but maybe you meant that it can't create things based on name which it still can't do. I think what you said is reasonable but the way it's designed today it sounds like an easy thing to do would be to keep all the implementations, which sounds fine as a first step.

If data protection is untrimmable a designed today then we'll need to look at the cost of the assembly an determine if it worth it and if so maybe decouple other components from the implementation here

[ghost]

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[davidfowl]

@HaoK this is assigned to you for preview4 but we should move it and discuss it more before you start.

[HaoK]

Sure preview5? Do you want me to setup a meeting to discuss this work?

[davidfowl]

Yep!

[HaoK]

Do we need Levi for this meeting or just Barry?

[davidfowl]

Both would be ideal

[GrabYourPitchforks]

Honestly this seems doable to me. Might have to invent some tricks to help get the ball rolling, but nothing here immediately stands out as being impossible to trim.

[blowdart]

Linking #31821

[davidfowl]

How is that related?

[HaoK]

Discussion notes:

High level options

• Leave everything
• Leave a subset of algorithms
• Custom protectors is covered (by startup configuration)

Next steps

• @davidfowl Figure out how to test things
• Figure out which classes to pin
• Consider changing platform checks to OS platform checks
• fallback to managed codepath (SP800) 10x slower (@HaoK try deleting the code for size difference)

DataProtection -> Span in, Array out

[HaoK]

Update: I tried deleting all the code related to the windows implementations of SP800 so

• Anything CngCbc and CngGcm related, the factories, the config, the deserlization:

This resulted in the following differences in Microsoft.AspNetCore.DataProtection.dll size:
net6.0 (before 154KB after 143 KB) = -11KB
net461 (before 151 kb after 140 KB) -11KB
netstandard (before 151 kb after 140 KB) -11KB

So looks like it saves around 11 KB, or around 7%

[ghost]

Thanks for contacting us.

We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.