wpf
wpf copied to clipboard
dotnet
Remove SafeReflectionInvoker Part 2
Follow up to dotnet/wpf#6693
Description
Removes the remaining methods from SafeReflectionInvoker which was reverted in dotnet/wpf#6693 because of this comment: https://github.com/dotnet/wpf/pull/6693#issuecomment-1385285388
@dipeshmsft Let me know if/when you have more details about whether or not this is safe to do. My personal opinion is that this is a reasonable change to do because it is not part of the public API.
Customer Impact
See "Risk".
Regression
No.
Testing
Local build + CI.
Risk
Low. There is a risk that an application uses this internal API which could be considered a breaking change that falls in Bucket 4 in the dotnet/runtime documentation.
Microsoft Reviewers: Open in CodeFlow
Just putting some context on why this is a purely CAS thing, now a dead code (even in NetFX after at least 4.6):
IsInSystemXaml / IsSystemXamlNonPublic:
/// Critical: Used to detect luring attack described in class-level comments.
/// Safe: Gets the information from reflection.
/// The MethodInfo (and MemberInfo) have an InheritanceDemand so derived class
/// spoofing in PT is not an issue.
Class-level comments:
/// <SecurityNote>
/// All invocation of user-provided reflection objects inside System.Xaml should be routed through this class.
///
/// Invoking a reflection object directly from this class (or any class in System.Xaml)
/// could potentially expose internal types or methods in System.Xaml, because
/// mscorlib sees the invocation as coming from System.Xaml. Instead, we do
/// the invocation from a dynamic assembly created for this purpose. Because
/// the wrapper methods live in a separate assembly (with no internals) and are
/// marked as security-transparent, the security checks in mscorlib will treat
/// attempts to use internals of System.xaml the same as it treats attempts to
/// use internals of any other assembly; i.e. the caller (that is, the user's
/// application) must have the appropriate permissions.
///
/// This class exposes proxy methods for each of the reflection methods we need.
/// The static constructor creates the dynamic assembly and populates it with
/// the wrapper methods that actually call into reflection code. The proxy
/// methods in this class merely call the corresponding dynamic wrapper methods.
///
/// The dynamic assembly technique turns out to have perf implications - it loads
/// parts of mscorlib and clr that are otherwise unneeded (for Reflection.Emit et al.),
/// and requires JIT compilation of the resulting IL. We don't need the elaborate
/// techinique in full-trust scenarios (a malicious full-trust app can already
/// invoke internals just by calling reflection directly). So in full-trust we
/// use the old code.
/// </SecurityNote>
and from XamlMemberInvoker (which further confirms a check like this is no longer ever needed):
/// This class makes the assumption that any internal ShouldSerialize methods in System.Xaml are safe
/// for public invocation. If this becomes untrue, then ShouldSerializeValue needs an IsSystemXamlNonPublic
/// check, just like GetValue and SetValue have.
