32

I have to generate a uniform, secure random integer within a given range for a program that generates passwords. Right now I use this:

RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider();
byte[] rand = new byte[4];
rng.GetBytes(rand);
int i = BitConverter.ToUInt16(rand, 0);
int result = i%max;   // max is the range's upper bound (the lower is 0)

Is this method safe to use for cryptographic purposes? If not, how should I do it?

9
  • 1
    I notice that you're generating a 4-byte random sequence, but then converting that to a 2-byte integer (ushort / UInt16). Perhaps generate either a 2-byte sequence or convert the 4-byte sequence to uint / UInt32. I'm not sure whether the modulo affects cryptographic security in this instance.
    – wablab
    Commented Feb 23, 2017 at 21:26
  • 2
    This post seems to indicate that what you're doing is pretty good: stackoverflow.com/questions/5008804/…. Or follow this post to create a cryptographically secure double between 0 and 1, and then multiply the result by your upper max? stackoverflow.com/questions/2854438/…
    – wablab
    Commented Feb 23, 2017 at 21:31
  • 2
    Modulo doesn't lead to cryptographically secure random numbers. There are many examples over on Cryptography. This is the one I found quickly: crypto.stackexchange.com/q/22767/13022
    – Artjom B.
    Commented Feb 23, 2017 at 21:52
  • 1
    I posted an asnwer, but I realised that I was not giving any help. So I deleted it, Im sorry.
    – afonte
    Commented Feb 23, 2017 at 21:54
  • 1
    I think I posted a better answer now :)
    – afonte
    Commented Feb 23, 2017 at 22:05

5 Answers 5

16

You can have a look at the CryptoRandom class taken from niik/CryptoRandom.cs which is the original version by Stephen Toub and Shawn Farkas. In this class they implement several random generators that seem to be cryptographically secure.

I have used the following version in my projects for random integer generation.

public class RandomGenerator
{
    readonly RNGCryptoServiceProvider csp;

    public RandomGenerator()
    {
        csp = new RNGCryptoServiceProvider();
    }

    public int Next(int minValue, int maxExclusiveValue)
    {
        if (minValue >= maxExclusiveValue)
            throw new ArgumentOutOfRangeException("minValue must be lower than maxExclusiveValue");

        long diff = (long)maxExclusiveValue - minValue;
        long upperBound = uint.MaxValue / diff * diff;

        uint ui;
        do
        {
            ui = GetRandomUInt();
        } while (ui >= upperBound);
        return (int)(minValue + (ui % diff));
    }

    private uint GetRandomUInt()
    {
        var randomBytes = GenerateRandomBytes(sizeof(uint));
        return BitConverter.ToUInt32(randomBytes, 0);
    }

    private byte[] GenerateRandomBytes(int bytesNumber)
    {
        byte[] buffer = new byte[bytesNumber];
        csp.GetBytes(buffer);
        return buffer;
    }
}
15
  • 2
    Thank you. I'm not sure I understand what it does, because it uses functions defined elsewhere and is further complicated to be thread-safe, but once I understand it I'll consider using it.
    – Hey
    Commented Feb 23, 2017 at 22:10
  • 4
    Something to pay particular attention to is the way the loop gets rid of bias; it works out the largest range that could be moduloed without bias and then loops until it gets a number in that range. It costs to do those loops, but every time it has to is a time that just moduloing would have bias. Calculating the largest range that would work reduces how often that happens.
    – Jon Hanna
    Commented Feb 23, 2017 at 22:48
  • Thank you so much for your explanation.
    – afonte
    Commented Feb 24, 2017 at 1:01
  • @JonHanna But it is not particularly efficient when it comes to ranges that are slightly over $2^31$ for the value of diff, as the change of the while loop repeating is almost 50%. This can be avoided by asking for 5 bytes, converting those to a long and then performing the algorithm. Commented Feb 24, 2017 at 1:13
  • @MaartenBodewes but then that's less efficient in other cases, so it's swings and roundabouts.
    – Jon Hanna
    Commented Feb 24, 2017 at 1:17
13

There are two issues in the accepted answer.

  • It does not dispose the disposable csp correctly
  • When minvalue equals maxvalue, it throws an error (the standard random method does not)

Revised code

using System;
using System.Security.Cryptography;

namespace CovidMassTesting.Helpers
{
    /// <summary>
    /// Secure random generator
    ///
    /// <https://stackoverflow.com/questions/42426420/how-to-generate-a-cryptographically-secure-random-integer-within-a-range>
    ///
    /// </summary>
    public class RandomGenerator : IDisposable
    {
        private readonly RNGCryptoServiceProvider csp;

        /// <summary>
        /// Constructor
        /// </summary>
        public RandomGenerator()
        {
            csp = new RNGCryptoServiceProvider();
        }

        /// <summary>
        /// Get random value
        /// </summary>
        /// <param name="minValue"></param>
        /// <param name="maxExclusiveValue"></param>
        /// <returns></returns>
        public int Next(int minValue, int maxExclusiveValue)
        {
            if (minValue == maxExclusiveValue)
                return minValue;

            if (minValue > maxExclusiveValue)
            {
                throw new ArgumentOutOfRangeException($"{nameof(minValue)} must be lower than {nameof(maxExclusiveValue)}");
            }

            var diff = (long)maxExclusiveValue - minValue;
            var upperBound = uint.MaxValue / diff * diff;

            uint ui;
            do
            {
                ui = GetRandomUInt();
            } while (ui >= upperBound);

            return (int)(minValue + (ui % diff));
        }

        private uint GetRandomUInt()
        {
            var randomBytes = GenerateRandomBytes(sizeof(uint));
            return BitConverter.ToUInt32(randomBytes, 0);
        }

        private byte[] GenerateRandomBytes(int bytesNumber)
        {
            var buffer = new byte[bytesNumber];
            csp.GetBytes(buffer);
            return buffer;
        }

        private bool _disposed;

        /// <summary>
        /// Public implementation of Dispose pattern callable by consumers.
        /// </summary>
        public void Dispose()
        {
            Dispose(true);
            GC.SuppressFinalize(this);
        }

        /// <summary>
        /// Protected implementation of Dispose pattern.
        /// </summary>
        /// <param name="disposing"></param>
        protected virtual void Dispose(bool disposing)
        {
            if (_disposed)
            {
                return;
            }

            if (disposing)
            {
                // Dispose managed state (managed objects).
                csp?.Dispose();
            }

            _disposed = true;
        }
    }
}

Usage

/// <summary>
        /// Generates a random password,
        /// respecting the given strength requirements.
        /// </summary>
        /// <param name="opts">A valid PasswordOptions object
        /// containing the password strength requirements.</param>
        /// <returns>A random password</returns>
        public static string GenerateRandomPassword(PasswordOptions opts = null)
        {
            if (opts == null) opts = new PasswordOptions()
            {
                RequiredLength = 10,
                RequiredUniqueChars = 4,
                RequireDigit = true,
                RequireLowercase = true,
                RequireNonAlphanumeric = true,
                RequireUppercase = true
            };

            string[] randomChars = new[] {
                "ABCDEFGHJKLMNOPQRSTUVWXYZ",    // Uppercase
                "abcdefghijkmnopqrstuvwxyz",    // Lowercase
                "0123456789",                   // Digits
                "!@$?_-"                        // Non-alphanumeric
            };

            using RandomGenerator rand = new RandomGenerator();
            List<char> chars = new List<char>();

            if (opts.RequireUppercase)
                chars.Insert(rand.Next(0, chars.Count),
                    randomChars[0][rand.Next(0, randomChars[0].Length)]);

            if (opts.RequireLowercase)
                chars.Insert(rand.Next(0, chars.Count),
                    randomChars[1][rand.Next(0, randomChars[1].Length)]);

            if (opts.RequireDigit)
                chars.Insert(rand.Next(0, chars.Count),
                    randomChars[2][rand.Next(0, randomChars[2].Length)]);

            if (opts.RequireNonAlphanumeric)
                chars.Insert(rand.Next(0, chars.Count),
                    randomChars[3][rand.Next(0, randomChars[3].Length)]);

            for (int i = chars.Count; i < opts.RequiredLength
                || chars.Distinct().Count() < opts.RequiredUniqueChars; i++)
            {
                string rcs = randomChars[rand.Next(0, randomChars.Length)];
                chars.Insert(rand.Next(0, chars.Count),
                    rcs[rand.Next(0, rcs.Length)]);
            }

            return new string(chars.ToArray());
        }
1
  • The linked source has a restrictive license.
    – Steve P
    Commented May 21 at 15:31
12

In .NET 6, RNGCryptoServiceProvider used in the previous answers is now obsolete.

For cryptographic random numbers, simply use RandomNumberGenerator static methods, such as:

var byteArray = RandomNumberGenerator.GetBytes(24);
2
  • 2
    Yes, according to the link you provided, RNGCryptoService provider is indeed obsolete. But the example you gave doesn't work, it brings up the error CS1503 Argument 1: cannot convert from 'int' to 'byte[]'. If you just want to get a random integer, use RandomNumberGenerator.GetInt32(...) - either with one GetInt32(toMaxExclusive) or two arguments GetInt32(min, toMaxExclusive).
    – Matt
    Commented Apr 19, 2022 at 7:29
  • 1
    Is there any information about why RNGCryptoServiceProvider is obsolete? Does it have a vulnerability or something? Is it safe to use in .NET versions before 6?
    – Klesun
    Commented Sep 5, 2023 at 7:01
7

From your code I can see, you want to get a random integer number from an interval.

There is a new cryptographic random number generator included in .NET (since versions Core 3.0, Core 3.1, .NET 5, .NET 6, .NET 7 RC 1 and .NET Standard 2.1).

As jws mentioned, the formerly used class RNGCryptoServiceProvider is deprecated.

You can use this helper method. It can also easily replace the unsafe System.Random's .Next method:

/// <summary>
/// Generate a secure random number
/// </summary>
/// <param name="fromInclusive">Random number interval (min, including this number)</param>
/// <param name="toExclusive">Random number interval (max, excluding this number)</param>
/// <returns></returns>
private int RandomNumber(int fromInclusive, int toExclusive)
=> System.Security.Cryptography.RandomNumberGenerator.GetInt32(fromInclusive, toExclusive);

To simulate rolling a dice, use it like

var getNumber = RandomNumber(1, 7); // including 1, excluding 7 => 1 .. 6

If you prefer to use it "the old way" with .Next(), you can create a class like so (note that instead of a seed there are the fromInclusive and toExclusive parameters instead):

public class SecureRandom
{
    private int fromInc, toExcl;
    public SecureRandom(int toExclusive = 2) => Init(0, toExclusive);
    public SecureRandom(int fromInclusive, int toExclusive) 
           => Init(fromInclusive, toExclusive);
    private void Init(int fromInclusive, int toExclusive)
    {
        fromInc = fromInclusive; toExcl = toExclusive;
    }
    
    public int Next() => RandomNumber(fromInc, toExcl);

    public static int RandomNumber(int fromInclusive, int toExclusive)
    => System.Security.Cryptography.RandomNumberGenerator.GetInt32(fromInclusive, toExclusive);
}

Example:

// always the same interval in a loop:
var rnd = new SecureRandom(1, 7); 
for (int i = 0; i < 100; i++)
{
    Console.WriteLine(rnd.Next()); // roll the dice 100 times
}

// alternative usage (without creating an instance):
Console.WriteLine(SecureRandom.RandomNumber(1, 7));

Note:

  • This version doesn't require to get an instance from the cryptographic class any more - you just call it to get the next random number.

  • There's also an overload of GetInt32(...) which takes one argument for the maximum exclusive value, which starts from the minimum value 0. If you need that, feel free to update the code and create another overloaded method for the static function RandomNumber, like:

 public static int RandomNumber(int toExclusive)
    => System.Security.Cryptography.RandomNumberGenerator.GetInt32(0, toExclusive);
}
0

In new projects like .net core, it says RNGCryptoServiceProvider is obsolete, here's the equivalent version but using RandomNumberGenerator in System.Security.Cryptography namespace which is not obsolete and is cryptographically secure:

public class SecureRandomNumberGenerator: IDisposable
{
   private RandomNumberGenerator rng = RandomNumberGenerator.Create();

   public int GenerateRandomNumberInRange(int minValue, int maxValue)
{
    if (minValue >= maxValue)
    {
        throw new ArgumentOutOfRangeException(nameof(minValue), "minValue must be less than maxValue");
    }

    int range = maxValue - minValue + 1;
    byte[] uint32Buffer = new byte[4];

    int result;
    do
    {
        rng.GetBytes(uint32Buffer);
        uint randomUint = BitConverter.ToUInt32(uint32Buffer, 0);
        result = (int)(randomUint % range);
    } while (result < 0 || result >= range);

    return minValue + result;
}

public void Dispose()
{
    rng.Dispose();
}

}

Here's the usage:

using (var num = new SecureRandomNumberGenerator())
{
  for (int i = 0; i < 100; i++)
  {
       var number = num.GenerateRandomNumberInRange(5, 10);
       Console.WriteLine(number);
   }
}

Not the answer you're looking for? Browse other questions tagged or ask your own question.