Remove [SecurityCritical] and [SecuritySafeCritical] attributes

[danmoseley]

Security transparency will not be implemented in CoreFX, and use of these attributes can be removed throughout.

[danmoseley]

Same applies for eg [SecurityTransparent] but I don't really see use of that.

[danmoseley]

Also [ComVisible] can be removed

[morganbr]

Any assemblies that can also be used on .NET Framework need to keep their attributes or they may break.

[hughbe]

@morganbr how does one identify which assembly can be used on .NET fx?

[jkotas]

• In reference assemblies ie. ...\ref...*.cs, SecuritySafeCritical attributes can be always removed (they have no impact on the contract) and SecurityCritical attributes should always stay (I believe that some of the tooling looks at them in reference assemblies)
• In implementation assemblies ie ...\src...*.cs, the SecuritySafeCritical/SecurityCritical annotations are only needed if:
  • The files are built as OOB for desktop (the .cs files are included in the build unconditionally or for net46x TargetGroup) AND
  • The assembly is opted into security transparency. It is done via assembly level attribute like [assembly: SecurityTransparentAttribute]

(Edit) The assemblies in this category are: System.Collections.Immutable, System.IO.Compression and System.Net.Http - so we need to be careful about cleaning it up from those. It should be fine to clean it up everywhere else.

[hughbe]

@jkotas cheers, and what about ComVisible?

[jkotas]

There are a few cases where ComVisible is needed for tests, or other corner cases; but it should be fine to remove otherwise. It was scrubbed once already (dotnet/corefx#782), so there are not that many files that have it.

[morganbr]

The assembly is opted into security transparency

The assembly attributes for that are SecurityTransparent, SecurityCritical, and AllowPartiallyTrustedCallers.

[karelz]

@JeremyKuhne are you still working on it? Or should we flip it back to "up for grabs"?

[weshaggard]

When we find cases where we need to keep the attributes for at least one build configuration can we please just keep it for all configurations instead of #ifdef'ing them like we are doing in dotnet/corefx#17262 (comment)