Non-support of PrincipalPermissionAttribute

[ericjohannsen]

As best I can determine from my own test code (against 3.0.0) and as mentioned in #17693, PrincipalPermissionAttribute exists as a type but has no effect. MSDN also states "This attribute has no effect in .NET Core."

This seems dangerous, as a user porting existing code might expect that, if the attribute compiles, that it also functions. Since the purpose of this attribute is security, the consequences of a false assumption could be significant.

Is it a well-considered opinion that the attribute should exist yet be non-functional (not even a compiler warning...) or is this a decision warranting review?

[jkotas]

The methods for creating partial trust context throw or do not exist at all in .NET Core.

For example, PermissionSet.Deny() throws PlatformNotSupportedException in .NET Core: https://github.com/dotnet/corefx/blob/d58a51f911efb3c98beca21b6cf08cc703424fdf/src/Common/src/CoreLib/System/Security/PermissionSet.cs#L30

If your ported program depends on partial trust permissions, you will get an error much earlier, long before these permission attributes would come into play.

[ericjohannsen]

This isn't about partial trust. The following is perfectly valid permission enforcement in a .NET Framework business class. It compiles just fine in .NET Core but fails the essential task of controlling access:

public class Foo
{
  [PrincipalPermission (SecurityAction.Demand, Role = "finance")]
  public IEnumerable<Salary> GetEmployeeSalaries() => ...;
}

[GrabYourPitchforks]

@jkotas This makes me think we should obsolete some types (like PrincipalPermissionAttribute) as error. Since we don't support the CAS infrastructure in Core we should fail at compilation when these types are involved, as otherwise they'll incorrectly give the illusion that they work properly.

[GrabYourPitchforks]

Quite possibly all of the CAS-related attributes, now that I think more on it. If you manually new up a permission object and call Demand you'll get PNSE as expected. But since the runtime isn't looking at the attributes at all there's nobody to trigger the PNSE.

[jkotas]

I fully support marking all CAS-related APIs as obsolete, once we get the more fine grained controls for obsolete warnings in place.

[GrabYourPitchforks]

Related: #1698 (comment), which ponders the consequences of a "fail open" policy where developers don't expect it.