API proposal: Obsolete RNGCryptoServiceProvider

[GrabYourPitchforks]

Background and Motivation

The RNGCryptoServiceProvider class is a relic from the old Windows CAPI days of yore. The original Win32 API is no longer recommended, preferring CNG for all new work.

In .NET Core, the RNGCryptoServiceProvider type is marked [EditorBrowsable(Never)], and the implementation ignores all provided constructor parameters and delegates to the underlying preferred OS implementation anyway.

There's no reason for an application targeting .NET 6.0+ to use this API. Apps should instead use RandomNumberGenerator.Create(). For AOT and linker trimming scenarios, this could also help eliminate the app's dependency on the package which contains the RNGCryptoServiceProvider type, reducing overall memory usage and disk footprint.

Proposed API

namespace System.Security.Cryptography
{
    [EditorBrowsable(EditorBrowsableState.Never)] // existing attribute
    [Obsolete("This type is obsolete. Use RandomNumberGenerator.Create() instead.")] // new attribute
    public sealed class RNGCryptoServiceProvider : RandomNumberGenerator
    { /* ... */ }
}

This could be accompanied by a fixer with two behaviors:

• All calls to RNGCryptoServiceProvider ctors become calls to the parameterless overload RandomNumberGenerator.Create().
• All fields / locals / parameters of type RNGCryptoServiceProvider instead become type RandomNumberGenerator.

The obsoletion would not affect apps targeting netstandard or .NET versions prior to 6.0, as the reference assemblies would not contain these annotations. However, the fixer could apply to all target frameworks.

Tagging subscribers to this area: @bartonjs, @vcsjones, @krwq
See info in area-owners.md if you want to be subscribed.

[vcsjones]

On the topic of RandomNumberGenerator, maybe we should steer people to not using Create + GetBytes at all, but instead use the static Fill member. Perhaps that can be part of the fixer, or at least a suggestion in the documentation.

[GrabYourPitchforks]

If we really wanted, we could also have RandomNumberGenerator.Create return a singleton whose dispose method no-ops.

[vcsjones]

If we really wanted, we could also have RandomNumberGenerator.Create return a singleton

Hm, I'll play around with that idea. What problems / concerns arise from calling GC.SuppressFinalize repeatedly? The RNG uses that dispose pattern where Dispose() is non-virtual and calls Dispose(bool disposing) for inheritors to override, then the parameterless one suppresses finalization.

type is marked [EditorBrowsable(Never)]

There are many other crypto classes marked as such, SHA256Managed and other Managed friends, etc. Others are constructors that don't make sense in Core like HMACSHA1(byte[] key, bool useManagedSha1).

Does it make sense to broaden this issue to include them?

[GrabYourPitchforks]

Does it make sense to broaden this issue to include them?

Possibly. We could try making the changes to all callers within the dotnet/* repos before marking things as obsolete, just to see how much churn it might cause.

[SteveSyfuhs]

Would the call to Create() seed the RNG differently than what Fill() ends up using, or are they sourced from the same seed?

[bartonjs]

Would the call to Create() seed the RNG differently than what Fill() ends up using, or are they sourced from the same seed?

There are no per-instance seeds. All of our CSPRNGs use stateless functions (or, at least, functions that only use hidden state).

https://github.com/dotnet/runtime/blob/master/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/RandomNumberGeneratorImplementation.OSX.cs

https://github.com/dotnet/runtime/blob/master/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/RandomNumberGeneratorImplementation.Unix.cs

https://github.com/dotnet/runtime/blob/master/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/RandomNumberGeneratorImplementation.Windows.cs

https://github.com/dotnet/runtime/blob/master/src/libraries/System.Security.Cryptography.Algorithms/src/Internal/Cryptography/RandomNumberGeneratorImplementation.Browser.cs

[marek-safar]

@GrabYourPitchforks @terrajobst is this something what will be done for 6.0? It'd help us to limit browser crypto implementation.

[bartonjs]

@marek-safar I don't think the timeline on marking it Obsolete helps browser. The RNGCryptoServiceProvider class is fake, it just wraps an instance of RandomNumberGenerator.Create: https://github.com/dotnet/runtime/blob/master/src/libraries/System.Security.Cryptography.Csp/src/System/Security/Cryptography/RNGCryptoServiceProvider.cs

So there shouldn't be any special work for Browser to do.