runtime: Annotate unsupported APIs in System.Security.*

The constructor is… harder?

Yeah. But then it feels weird to have a constructor that takes the key blob and no not-obsolete way to do anything with it.

Since it’s a protected constructor on a class that no one is deriving from I would wager we could take a swing at obsoleting it with less than perfect wording.

OK, the landscape here is complicated, but in general it looks like @bartonjs’s wager holds up, particularly through the lens of “ecosystem code exposed to such a change”. I definitely found types that inherit from System.Security.Cryptography.ECDiffieHellmanPublicKey, but almost universally these are types in our own shipped assemblies (System.Core, System.Security.Cng/Algorithms/OpenSsl). These assemblies are in our own packages, as well as redistributed in other packages at a pretty high rate. None of those really represent problematic scenarios WRT deprecation. The few that were genuinely non-product types are in assemblies that are not published on NuGet.org, and instead come from other locations like internal test trees/tools.

I need to figure out how to publish a few NuGet packages, then.

Had there been an actual “eat my hat” type consequence I definitely would have thrown in a disclaimer like “(offer valid only for non-prerelease packages already present on nuget.org at the original time of this post)”. I may not be smart enough to remember to look in partial files, but I’m not that naive 😄.

I’ll tackle the rest of these over the next day or two.

• Rfc2898DeriveBytes.CryptDeriveKey obsolete
  • Agreed
• System.Security.SecurityContext obsolete
  • Agreed (merged!)
• CmsSigner.CmsSigner(CspParameters) obsolete
  • Agreed (double checked that we didn’t make it work on Windows)
• SignerInfo.ComputeCounterSignature() obsolete
  • Agreed (double checked that we didn’t make it work on Windows)
• RC2.Create() see below

For RC2-browser we seem to have marked the whole type, which was probably a bit of an overreach, because someone /could/ provide a working implementation of RC2. RC2.Create() on android I’m pretty sure succeeds, but returns an object that fails if you try to actually use it. Part of me thinks it’d be better to just put the PNSE there, but maybe there’s a legit use case I can think of for the dummy RC2 instance.

So… yeah, marking RC2.Create as Unsupported-android is probably right. With a comment that says a more formal version of “yeah, it returns, but the object is going to throw if you call CreateEncryptor or CreateDecryptor… so it’s the same as not working”

As for other things in @buyaa-n 's tables:

• ECDiffieHellmanCng.ToXmlString/FromXmlString
  • Obsolete.
  • We never added them in Core, no one ever complained (to my recollection). The base class version literally never worked (even in NetFX when it was added), and the derived type is Windows-only.
• ECDiffieHellmanCngPublicKey.ToXmlString/FromXmlString
  • Obsolete (same as above, probably same diagnostic ID)
• ECDsaCng.ToXmlString/FromXmlString
  • Obsolete (same as above, probably same diagnostic ID)
• ECDiffieHellmanPublicKey.ToXmlString
  • Obsolete
  • We’ve never had a derived type in Core that didn’t throw. Let’s just obsolete the base type’s version. Maybe the same diagnostic ID as above.
  • Wasn’t directly on the table, but addresses the ECDiffieHellmanOpenSslPublicKey case 😄

I think that only leaves ECDiffieHellmanPublicKey.ToByteArray and ECDiffieHellmanPublicKey…ctor(byte[]). I’d support Obsoleting these. I… thought… we had added SPKI export on this type, but I don’t see it. So maybe we need to hold this one back one release. Using inbox types the ToByteArray only works on Windows, but since it’s a virtual I don’t like putting a SupportedOSAttribute on it.

Adding last findings

Just to summarize findings that I think are actionable:

• Rfc2898DeriveBytes.CryptDeriveKey obsolete: https://github.com/dotnet/runtime/pull/57002
• System.Security.SecurityContext obsolete https://github.com/dotnet/runtime/pull/57035
• CmsSigner.CmsSigner(CspParameters) obsolete https://github.com/dotnet/runtime/pull/57301
• SignerInfo.ComputeCounterSignature() obsolete https://github.com/dotnet/runtime/pull/57301
• RC2.Create() add [UnsupportedOSPlatform(“android”)] https://github.com/dotnet/runtime/pull/57289

It would still be useful to have someone else validate this.

We could take these 3 obsoletions without going through API Review–just code review from @bartonjs and @GrabYourPitchforks. I’m comfortable taking them before the August 17th cutoff for .NET 6, especially since it’s not a breaking change.

‘ECDsa.ECDsaImplementation.ECDsaSecurity Transforms.ExportExplicitParameters(bool)’

I’m not sure adding an UnsupportedOSPlatform here would be ergonomic. The issue is that the public API ExportExplicitParameters is accessible via the virtual ECDsa.ExportExplicitParameters. We would have to decorate the virtual with the UOSP attribute, which would not interact well with derived classes (our own or developers’ own). To illustrate:

using System.Runtime.Versioning;

Foo f = new Bar();
f.Do(); // This is actually a `Bar` but since it's called through the base type, a compiler warning appears.

public class Foo
{
    [UnsupportedOSPlatform("iOS")]
    public virtual object Do() => throw new PlatformNotSupportedException();
}

public class Bar : Foo
{
    public override object Do() => new();
}

Similar concerns lie with ToXmlString, FromXmlString, and ToByteArray. It seems a little odd to obsolete an override when a user derived type could override one of these from the base class and implement it.

Some of these do make sense to me to obsolete:

1. Rfc2898DeriveBytes.CryptDeriveKey - it’s unimplemented and can’t be implemented xplat since it’s a Windows thing. I’m unaware of demand to bring it back.
2. CmsSigner.CmsSigner(CspParameters) seems weird. It manufactures a certificate on the fly based on those CspParameters. It makes sense to obsolete since it can only be implemented on Windows. If we really, really wanted “make me a cert on the fly” behavior, it probably makes sense to implement a new API that accepted an AsymmetricAlgorithm or something.
3. SignerInfo.ComputeCounterSignature() There’s an overload that accepts the CmsSigner, so I think it makes sense to obsolete.

I don’t see any immediately actionable things for 6.0. Presumably the obsoletions need to go through API review, and I don’t know if we’re taking any more obsoletions for 6.0 (granted all of them throw, so it’s not exactly a breaking change).

DSA.CreateCore()

The public entry, Create is already marked with UnsupportedOSPlatform:

https://github.com/dotnet/runtime/blob/a2f2a629328a136c2cd18f2cd729e3854de2aeb4/src/libraries/System.Security.Cryptography.Algorithms/src/System/Security/Cryptography/DSA.cs#L38

So I think that one is already taken care of.

Some of these APIs that “unconditionally throw” are virtual and are expected to be overridden in derived classes, so while the base class throws, they aren’t necessarily obsolete because derived classes override them and some functionality depend on these classes. I think this would be the case for all of the methods on the HMAC class. So I do not think those should be obsolete or otherwise annotated.