Skip to content

Can we remove all Code Access Security (CAS) transparency attributes from this project? #477

@stakx

Description

@stakx

TL;DR:

May I submit a PR to remove all applications of CAS security transparency-related attributes, i.e. [SecurityCritical] and [SecuritySafeCritical] and [assembly: SecurityRules(SecurityRuleSet.Level2)]?

Reasons:

  1. The attributes in question have zero effect without [assembly: AllowPartiallyTrustedCallers], which got removed back in Move builds from TeamCity to AppVeyor and Travis CI #241 (see also Sort out whether AllowPartiallyTrustedCallers gets defined Windsor#248 and APTCA consistency Windsor#300).

  2. Microsoft has obsoleted large parts of CAS, and .NET Core no longer supports it at all. There's simply no more future for CAS.

  3. The attributes are applied only sporadically, and as far as I can tell, in a haphazard, non-systematic manner.

  4. Some of these attributes are applied in apparently pointless places, making me suspect we never really cared enough about CAS to think it through & get it right:

    • It is often private methods that are marked [SecurityCritical], but who could call these methods except other methods in the very same class? Does it make sense that some sibling methods may call them, and others may not? Security auditing at such a fine-grained level doesn't make much sense, IMHO.
    • In one instance, a type (static) constructor is marked with one of these attributes. According to MS documentation, this won't have any effect at all, as type constructors are called by the runtime, which is always fully-trusted and therefore not constrained by these attributes.
  5. By removing these, we could get rid of a lot of conditional code guards #if FEATURE_SECURITY_PERMISSIONS && DOTNET40. I'd like to especially see usage of DOTNET40 minimized, for the reason given below.

Some further background re: (5), DOTNET40

I'd love to see the next minor version of the Castle.Core NuGet package acquire two framework targets: netcoreapp2.0 and netstandard2.1. (1) This could greatly simplify the transitive dependency graphs of .NET projects relying (directly or indirectly) on Castle.Core. Our existing netstandard1.x targets will bring in a lot of dependencies even when downstream libraries such as Moq, FakeItEasy, NSubstitute etc. already target netstandard2.x.

To support these new targets, we'd need to adjust some of our conditional compilation symbols (#if FEATURE_*) as the APIs of these target frameworks are once again different from all of the target frameworks we currently support (.NET Framework 3.5, 4.x, and .NET Standard 1.x).

I've been trying to determine the set of conditional compilation symbols that would work for these two new targets. Many of the existing symbols can be used without any changes; some will require changes. One symbol that's especially problematic is DOTNET40, it is a kitchen sink for several different things. It would be good to minimize usages of DOTNET40 and replace it with more specific symbols wherever possible, so that these can be cleanly mapped to the various target frameworks.

One of the things that falls under the DOTNET40 umbrella is Code Access Security transparency level 2; look for code guarded by #if FEATURE_SECURITY_PERMISSIONS && DOTNET40). Since the code thus guarded doesn't really make sense anymore (see reasons given above), I believe it would be easiest to just remove it rather than introducing a new FEATURE_SECURITY_TRANSPARENCY_LEVEL2 symbol.


(1) I'd also like to see some existing target frameworks dropped—net35, netstandard1.x, netstandard1.5, for reasons I'll happily lay out if requested— but that would be a breaking change and cannot be done in a minor version.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions