Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include System.Security.Cryptography.RandomNumberGenerator.GetInt32 #1101

Closed
khellang opened this Issue Mar 14, 2019 · 4 comments

Comments

Projects
None yet
2 participants
@khellang
Copy link

khellang commented Mar 14, 2019

I'd like to propose the inclusion of the new System.Security.Cryptography.RandomNumberGenerator.GetInt32 APIs, added in dotnet/corefx#31243.

As noted in dotnet/corefx#30873, this is often incorrectly implemented, which can lead to security vulnerabilities and bias. It would be nice if it was part of the standard, to have a convenient API for this across all platforms.

The implementation doesn't have a lot of dependencies and could probably be (more or less) copied to other platforms.

diff --git a/src/netstandard/ref/System.Security.Cryptography.cs b/src/netstandard/ref/System.Security.Cryptography.cs
index ec34484..2b7d68b 100644
--- a/src/netstandard/ref/System.Security.Cryptography.cs
+++ b/src/netstandard/ref/System.Security.Cryptography.cs
@@ -810,6 +810,8 @@ namespace System.Security.Cryptography
         public virtual void GetBytes(System.Span<byte> data) { }
         public virtual void GetNonZeroBytes(byte[] data) { }
         public virtual void GetNonZeroBytes(System.Span<byte> data) { }
+        public static int GetInt32(int fromInclusive, int toExclusive) { throw null; }
+        public static int GetInt32(int toExclusive) { throw null; }
     }
     [System.ComponentModel.EditorBrowsableAttribute(System.ComponentModel.EditorBrowsableState.Never)]
     public abstract partial class RC2 : System.Security.Cryptography.SymmetricAlgorithm

// @terrajobst

terrajobst added a commit to terrajobst/standard that referenced this issue Mar 15, 2019

@Drawaes

This comment has been minimized.

Copy link

Drawaes commented Mar 18, 2019

Why would you make it an int32 seems like for any decent crypto area it should be an int64 ?

@khellang

This comment has been minimized.

Copy link
Author

khellang commented Mar 18, 2019

Why would you make it an int32 seems like for any decent crypto area it should be an int64 ?

I don't think this is the place to discuss this. The API was approved in dotnet/corefx#30873, implemented in dotnet/corefx#31243 and now included in the standard. It was considered in the API review, though:

We also considered GetInt64() versions but given that these values are mostly used for indexing this seems equally over the top.

@Drawaes

This comment has been minimized.

Copy link

Drawaes commented Mar 18, 2019

Indexing what?

@khellang

This comment has been minimized.

Copy link
Author

khellang commented Mar 18, 2019

Indexing what?

Indexing indexers 😜

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.
You signed in with another tab or window. Reload to refresh your session. You signed out in another tab or window. Reload to refresh your session.