Remove System.Security.Permissions from S.C.CM.

[AraHaan]

This removes System.Security.Permissions from S.C.CM for non-.NET Framework targets. For .NET Framework targets nothing changes. This is because CAS is only implemented in .NET Framework and as such should only be used in .NET Framework to remove the chance that in .NET Core/5+ and .NET Standard that System.Drawing.Common is loaded outside of a windows environment which since .NET 6 is made to throw PNSE everywhere which is not desired for people using cross platform dependencies that just so happens to depend on S.C.CM.

Contributes to #64592

Note: I consider changes in the internal namespace to be an implementation detail and any possibly breaking change being an acceptable change as it is marked internal in the name meaning that users outside of this repository should not be using it and if they are the namespace should imply that it can have breaking changes at any time. With that excluded in my mind from public ABI, I have determined this change (part of #64592) is trivial. For S.DS the change to remove S.S.P is non-trivial as it breaks public ABI and so needs to go through api-review (and approval) for that breaking change to land in .NET 8.

After that, I do not see much of any codebases in dotnet/runtime using S.S.P anymore (outside of the .NET Framework targets for them).

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[ghost]

Tagging subscribers to this area: @dotnet/area-system-configuration
See info in area-owners.md if you want to be subscribed.

Issue Details

---

This removes System.Security.Permissions from S.C.CM for non-.NET Framework targets. For .NET Framework targets nothing changes. This is because CAS is only implemented in .NET Framework and as such should only be used in .NET Framework to remove the chance that in .NET Core/5+ and .NET Standard that System.Drawing.Common is loaded outside of a windows environment which since .NET 6 is made to throw PNSE everywhere which is not desired for people using cross platform dependencies that just so happens to depend on S.C.CM.

Contributes to #64592

Note: I consider changes in the internal namespace to be an implementation detail and any possibly breaking change being an acceptable change as it is marked internal in the name meaning that users outside of this repository should not be using it and if they are the namespace should imply that it can have breaking changes at any time. With that excluded in my mind from public ABI, I have determined this change (part of #64592) is trivial. For S.DS the change to remove S.S.P is non-trivial as it breaks public ABI and so needs to go through api-review (and approval) for that breaking change to land in .NET 8.

After that, I do not see much of any codebases in dotnet/runtime using S.S.P anymore (outside of the .NET Framework targets for them).

Author:	AraHaan
Assignees:	-
Labels:	area-System.Configuration, new-api-needs-documentation, community-contribution
Milestone:	-

[AraHaan]

The suppressions I added seems to not be working for some reason.

[ghost]

Added needs-breaking-change-doc-created label because this PR has the breaking-change label.

When you commit this breaking change:

1. Create and link to this PR and the issue a matching issue in the dotnet/docs repo using the breaking change documentation template, then remove this needs-breaking-change-doc-created label.
2. Ask a committer to mail the .NET Breaking Change Notification DL.

Tagging @dotnet/compat for awareness of the breaking change.

[danmoseley]

@ericstj do you expect we'll be able to take a change of this sort? I have some interest in the issue (but not pushing on it)

[ViktorHofer]

This removes the API from both .NETCoreApp assemblies and the netstandard2.0 assembly. Is the .NETStandard change intentional?

[ericstj]

Yes, we were exploring the intentional breaking change here. It was discussed as an option here: #64592 (comment)

[ericstj]

This source file should not be included for .NETFramework at all. .NETFramework should just be type-forwards. Same goes for all the src files. Just delete the implementation. Keep the test ifdef though.

[AraHaan]

ah ok

[ericstj]

@ericstj do you expect we'll be able to take a change of this sort? I have some interest in the issue (but not pushing on it)

I think we might, but it depends on usage. We don't typically remove API like this, since it's a binary breaking change, but this might be a case where the benefit is worth the risk (assuming we've measured the risk appropriately).

Usually when we break things, we like to have a mitigation to fix someone who's broken and cannot recompile - I don't see how we can have that here so that's a higher risk. That said, we feel it may still be acceptable since the code that's being removed is very unlikely to be called since it hasn't done anything for years, using it would have already hit an obsolete warning, it's in an Internal namespace, and the architecture of DelegatingConfigHost was specifically designed to avoid folks having to refer to interface members they didn't need to change:

runtime/src/libraries/System.Configuration.ConfigurationManager/src/System/Configuration/Internal/DelegatingConfigHost.cs

Lines 9 to 23 in ff3d567

	// A public implementation of IInternalConfigHost that simply
	// delegates all members of the IInternalConfigHost interface to
	// another instance of a host. All interface members are marked virtual
	// so that a derived class can override just the ones needed to
	// implement that specific host, while all others are delegated to
	// another implementation such as InternalConfigHost.
	//
	// The advantages of this arrangement are:
	// * The IInternalConfigHost interface can be extended without
	// requiring other hosts to be updated.
	// * This class that we are making public has no implementation
	// of its own that can be exploited. All the hosts with meaningful
	// implementation can remain internal.
	// * It allows straightforward chaining of host functionality,
	// see UpdateConfigHost as an example.

All these qualifications make me feel like you won't find a ton of precedent here, @danmoseley, so if that's what you're interested in I don't expect we'll establish it.

In any case I think we should make this change early and document it as breaking. We can give folks a chance to give us feedback.

I'm supportive of this being polished and documented and going out in an early preview to .NET 8.0, assuming we have agreement from @dotnet/area-system-configuration.

[danmoseley]

All these qualifications make me feel like you won't find a ton of precedent here, @danmoseley, so if that's what you're interested in I don't expect we'll establish it.

I don't have an opinion on breakingness/precedent, just that it's come up a internally as being an impediment to picking up MSRC's as you know.

[steveharter]

@AraHaan closing this PR since replaced by #82259.

Thanks for your contribution here. It forced a deeper dive into this problem where we found that the S.S.P assembly does not actually reference S.C.CM since the PermissionSet class is in corelib, not S.C.CM (at least for non-netstandard) and allows the new PR to address the issue in a less-breaking way.