-
Notifications
You must be signed in to change notification settings - Fork 485
Can we remove all Code Access Security (CAS) transparency attributes from this project? #477
Description
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:
-
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). -
Microsoft has obsoleted large parts of CAS, and .NET Core no longer supports it at all. There's simply no more future for CAS.
-
The attributes are applied only sporadically, and as far as I can tell, in a haphazard, non-systematic manner.
-
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
privatemethods 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.
- It is often
-
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 ofDOTNET40minimized, 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.