Runtime: System.Diagnostics.DiagnosticSource assembly has obsolete SecurityPermissionAttribute

[markrendle]

We're trying to use System.Diagnostics.DiagnosticSource package release 4.5.1 in a secure sandboxed environment, and it's being rejected because it has this assembly attribute:

[assembly: SecurityPermission(SecurityAction.RequestMinimum, SkipVerification = true)]

According to the docs, this attribute and the SecurityAction enum are obsolete and not supported by the runtime, so there's no reason for them to be in the lib\net46\System.Diagnostics.DiagnosticSource assembly.

Having downloaded and decompiled older versions of the package, it seems like this attribute turned up in release 4.4.0, which was the release that added the Activity type, which we are using and so can't revert to 4.3.1.

4.4.0 also introduced separate build targets for net45 and netcoreapp2.0, where previously there had just been net46, netstandard1.1, netstandard1.3 and the portable... builds. Is it possible that adding the net45 target has dragged this attribute in from somewhere in the depths of the CI environment?

[vancem]

While the documentation indicates that SecurityAction is obsolete, it is because we no longer recommend using code access security at all as a security boundary. You seem to be using the code access security (CAS) sandbox, which forces the unusual code paths.

The SkipVerification is needed in the context of code access security because unsafe code was used (that is in fact why the attribute was added when activities were added, we used unsafe code there).

Thus it is not as easy as simply removing it (it DOES something with respect to code access security). In theory DiagnosticSource DOES work in a CAS environment (we add the AllowPartiallyTrustedCodeAttribute and added the SecuritySafeCritical where we think they are needed), so we think it should work, but evidentially it does not for you.

I think you will need to give more information about exactly how your scenario fails and what exactly your security configuration is.

@jorive

[markrendle]

@vancem Thank you for the response. Is there a way to discuss this out of the open? I'm working on banking systems and don't want to share too much information in a public forum.

[vancem]

Send mail to [email protected] [email protected] and [email protected]

[tommcdon]

@vancem @jorive @noahfalk is there any action needed for this issue?

[vancem]

The fundamental source of the issue is that a while back DiagnosticSource started to use unsafe code (for efficiently generating new IDs). This makes it more subtle for code access security (since it is no longer verifiable). Now the attribute @markrendle is complaining about is actually obsolete only because all of code access security is obsolete, however he seems to be using code access security, which is the issue. In theory it should still work (if he grants trust to the DiagnosticSource DLL). I have not heard back from him since his 1/22 posting above).

However I think it is fair to say that at least at the present time it does not look like there are any changes needed. The security attributes do belong there (since the code does use unsafe constructs), and it is a question of granting the System.Diagnositics.DiagnosticSource.dll permission to use unsafe code (something he needs to do in his security configuration).

The most likely resolution is that we find some work-around for him.

[markrendle]

Hi @vancem. Apologies, I tried to send an email from my work account but it got blocked as an external address. I'm re-sending from my personal account now.

[vancem]

After a bit more out of band discussion with @markrendle, I am going to close this bug.

I think the crux of this situation is whether the System.Diagnostics.DiagnosticSource package can use unsafe code or not. If it does, then it needs to be trusted, and thus anyone using code access security will need to update their security config to grant that true trust. If on the other hand the package does not use unsafe code, not changes are needed.

Now the unsafe code that started this issue you could imagine avoiding (we used it for efficiency reasons). However since that time we have added more unsafe code to assembly for other reasons that frankly cannot be removed as easily. The desire to use small amounts of unsafe code to improve performance of highly perf-sensitive areas like logging is not going to go away.

On top of all this, while there is an advantage to avoiding unsafe code to avoid bugs, there is really no strong FUTURE LOOKING reason to have 100% safe code. This is because code access security is only supported on the Desktop framework and even there our OFFICIAL guidance to transition to not using it. It was simply too broad of a security boundary to secure the standard we wanted to guarantee.

Thus there is no strong value to keeping the code 100% safe and we have a relatively strong reason to include a very small amount of unsafe code in the current code (with the expectation that we might do it again).

What this means for you is that if you wish to use System.Diagnostics.DiagnosticSource in the partial trust standbox supported on .NET Desktop, you will need to modify the configuration of the sandbox to trust the System.Diagnostics.DiagnosticSource assembly. Alternatively you could maybe simply forgo the logging in those environments (that may be preferable in your case, especially if the sandbox is being phased out).

We have reviewed the code in System.Diagnostic.DiagnositicSource and we do believe that it is safe to trust. You can of course perform your own review of the SecuritySafeCritical parts of the code (that is the parts that are allowed to do unsafe things). The amount of such code is not large.

Thus we believe that there is a reasonable work-around for you, and that this path makes sense for all involved. You should be weening yourself off of code access security, and we don't want to be constrained in our future designs for the convenience of code using features we only support as part of maintenance.