RNGCryptoServiceProvider - Random Number Review

Question

While looking for best attempts at generating truly random numbers, I stumbled upon this code example.

Looking for opinions on this snippet.

using System;
using System.Security.Cryptography;

private static int NextInt(int min, int max)
{
    RNGCryptoServiceProvider rng = new RNGCryptoServiceProvider();
    byte[] buffer = new byte[4];
    
    rng.GetBytes(buffer);
    int result = BitConverter.ToInt32(buffer, 0);

    return new Random(result).Next(min, max);
}

Source: http://www.vcskicks.com/code-snippet/rng-int.php

Would this be preferred over using a tick count seed such as:

Random rand = new Random(Environment.TickCount); 
rand.Next(min, max);

Note:

I am not looking for third party random data providers such as Random.org, as such a dependency is not realistic to the application.

Solution 1

Well, using RNGCryptoServiceProvider gives you an unguessable crypto-strength seed whereas Environment.TickCount is, in theory, predictable.

Another crucial difference would be evident when calling your NextInt method several times in quick succession. Using RNGCryptoServiceProvider will seed the Random object with a different crypto-strength number each time, meaning that it will go on to return a different random number for each call. Using TickCount risks seeding the Random object with the same number each time (if the method is called several times during the same "tick"), meaning that it will go on to return the same (supposedly random) number for each call.

If you genuinely need truly random numbers then you shouldn't be using a computer to generate them at all: you should be measuring radioactive decay or something similarly, genuinely unpredictable.

Solution 2

Don't use your code. Your solution is wrong and generates poor random numbers. I suggest my solution, which generates cryptographically strong random numbers:

public class SecureRandom : RandomNumberGenerator
{
    private readonly RandomNumberGenerator rng = new RNGCryptoServiceProvider();


    public int Next()
    {
        var data = new byte[sizeof(int)];
        rng.GetBytes(data);
        return BitConverter.ToInt32(data, 0) & (int.MaxValue - 1);
    }

    public int Next(int maxValue)
    {
        return Next(0, maxValue);
    }

    public int Next(int minValue, int maxValue)
    {
        if (minValue > maxValue)
        {
            throw new ArgumentOutOfRangeException();
        }
        return (int)Math.Floor((minValue + ((double)maxValue - minValue) * NextDouble()));
    }

    public double NextDouble()
    {
        var data = new byte[sizeof(uint)];
        rng.GetBytes(data);
        var randUint = BitConverter.ToUInt32(data, 0);
        return randUint / (uint.MaxValue + 1.0);
    }

    public override void GetBytes(byte[] data)
    {
        rng.GetBytes(data);
    }

    public override void GetNonZeroBytes(byte[] data)
    {
        rng.GetNonZeroBytes(data);
    }
}

Solution 3

I really do not suggest using provided example. Although RNGCryptoServiceProvider returns truly good random (or at least it should), but the same is not true for Random. Moreover - it is not known if Random(value) creates true bijection against value returned by Next(...). Moreover - it is not guaranteed that Next(min, max) returns the value in a truly random manner (meaning equal chances for number to hit each value).

I would first tear down the problem to getting a number in the interval 0 - max (exclusive). Then, I would use nearest power of 2 to get a random value in the range 0 - (2^n - 1). Now, one thing you MUST never do here is use modulo to get a number in the preferred range, like rand(0 - (2^n - 1)) % max, because by doing, so you are actually increasing chances of getting number in lower range.

Example: max = 3, n = 2 (0 - (2^2 - 1)) % 2, numbers (0, 1, 2, 3), corresponding values after modulo (0, 1, 2, 0). See that we hit 0 twice, which is really bad randomness.

So, the solution would be to use crypto random to get a value to nearest power of two, and in case the value is outside maximum range, repeat procedure (get another crypto random) until the value is inside the given range. This would be a much better algorithm.

Solution 4

Ok, so I'm a little late to the party, but I really wanted a full implementation of System.Random that can be called multiple times during the same timer tic and yield different results. After much agonizing over different implementations, I settled on the simplest one that I came up with, which provides a default constructor that supplies a random key to the base System.Random constructor:

/// <summary> An implementation of System.Random whose default constructor uses a random seed value rather than the system time. </summary>
public class RandomEx : Random
{
    /// <summary> Initializes a new CryptoRandom instance using a random seed value. </summary>
    public RandomEx()
        : base(_GetSeed())
    { }

    /// <summary> Initializes a new CryptoRandom instance using the specified seed value. </summary>
    /// <param name="seed"> The seed value. </param>
    public RandomEx(int seed)
        : base(seed)
    { }

    // The static (shared by all callers!) RandomNumberGenerator instance
    private static RandomNumberGenerator _rng = null;

    /// <summary> Static method that returns a random integer. </summary>
    private static int _GetSeed()
    {
        var seed = new byte[sizeof(int)];

        lock (typeof(RandomEx)) {
            // Initialize the RandomNumberGenerator instance if necessary
            if (_rng == null) _rng = new RNGCryptoServiceProvider();

            // Get the random bytes
            _rng.GetBytes(seed);
        }

        // Convert the bytes to an int
        return BitConverter.ToInt32(seed, 0);
    }
}

Along the way, I also wrote and tested an implementation that overrides the methods necessary to use RNGCryptoServiceProvider to provide ALL of the random values (rather than relying on whatever random number generator is baked into the System.Random class). But I have no idea how cryptographically strong the results are by the time you take my random Sample() values and push 'em through the transformations to produce integer values. Anyway, here is the code if anyone wants it:

/// <summary> An implementation of System.Random that uses RNGCryptoServiceProvider to provide random values. </summary>
public class CryptoRandom : Random, IDisposable
{
    // Class data
    RandomNumberGenerator _csp = new RNGCryptoServiceProvider();

    /// <summary> Returns a random number between 0.0 (inclusive) and 1.0 (exclusive). </summary>
    protected override double Sample()
    {
        // Get a nonnegative random Int64
        byte[] bytes = new byte[sizeof(long)];
        _csp.GetBytes(bytes);
        long value = BitConverter.ToInt64(bytes, 0) & long.MaxValue;

        // Scale it to 0->1
        return (double)value / (((double)Int64.MaxValue) + 1025.0d);
    }

    /// <summary> Fills the elements of the specified array of bytes with random numbers. </summary>
    /// <param name="buffer"> An array of bytes to contain random numbers. </param>
    public override void NextBytes(byte[] buffer)
    {
        _csp.GetBytes(buffer);
    }

    /// <summary> Returns a nonnegative random integer. </summary>
    /// <returns> A 32-bit signed integer greater than or equal to zero. </returns>
    public override int Next()
    {
        byte[] data = new byte[4];
        _csp.GetBytes(data);
        data[3] &= 0x7f;
        return BitConverter.ToInt32(data, 0);
    }

    /// <summary> Returns a random integer that is within a specified range. </summary>
    /// <param name="minValue"> The inclusive lower bound of the random number returned. </param>
    /// <param name="maxValue"> The exclusive upper bound of the random number returned. maxValue must be greater than or equal to minValue. </param>
    /// <returns> A 32-bit signed integer greater than or equal to minValue and less than maxValue; that is, the range of return values includes minValue but not maxValue. If minValue equals maxValue, minValue is returned. </returns>
    public override int Next(int minValue, int maxValue)
    {
        // Special case
        if (minValue == maxValue) return minValue;

        double sample = Sample();
        double range = (double)maxValue - (double)minValue;
        return (int)((sample * (double)range) + (double)minValue);
    }

    #region IDisposible implementation

    /// <summary> Disposes the CryptoRandom instance and all of its allocated resources. </summary>
    public void Dispose()
    {
        // Do the actual work
        Dispose(true);

        // This object will be cleaned up by the Dispose method. Call GC.SupressFinalize to 
        // take this object off the finalization queue and prevent finalization code for this object 
        // from executing a second time.
        GC.SuppressFinalize(this);
    }

    // Dispose(bool disposing) executes in two distinct scenarios:
    //
    // If disposing is true, the method has been called directly or indirectly by a user's code and both
    // managed and unmanaged resources can be disposed. 
    //
    // If disposing is false, the method has been called by the runtime from inside the finalizer.
    // In this case, only unmanaged resources can be disposed. 
    protected virtual void Dispose(bool disposing)
    {
        if (disposing) {
            // The method has been called directly or indirectly by a user's code; dispose managed resources (if any)
            if (_csp != null) {
                _csp.Dispose();
                _csp = null;
            }

            // Dispose unmanaged resources (if any)
        }
    }

    #endregion
}

Solution 5

I felt like this could be a lot simpler and achieved in fewer lines of code so I'll throw my effort in - this will always return a positive number between 0 and 1:

public double Random()
{
    using var csp = new RNGCryptoServiceProvider();

    byte[] b = new byte[8];
    csp.GetBytes(b);
    var lng = BitConverter.ToInt64(b, 0);
    var dbl = (double)(lng < 0 ? ~lng : lng);

    // Convert to a random number between 0 and 1
    return dbl / long.MaxValue;
}

Instead of creating a new RNGCryptoServiceProvider each time, a simple static field would do fine.

To return a random positive integer between two numbers this works fine - though you need to check min is less than max, and that both are positive:

public long RandomInt64(long min = 0, long max = long.MaxValue)
{
    // Check arguments
    if (min >= max)
    {
        throw new ArgumentOutOfRangeException(nameof(min), min, "Minimium value must be less than the maximum value.");
    }

    if (min < 0)
    {
        throw new ArgumentException("Minimum value must be at least 0.", nameof(min));
    }

    // Get the range between the specified minimum and maximum values
    var range = max - min;

    // Now add a random amount of the range to the minimum value - it will never exceed maximum value
    var add = Math.Round(range * Random());
    return (long)(min + add);
}

View more solutions

Comments

[Henk Holterman]

It's not the seed that's so vulnerable, but having a few pseudo-random values that makes it feasible to 'guess' the next.

[LukeH]

@Henk: True, but in the OP's example each Random instance is single-use so I don't think that's an issue. Seed the RNG with a crypto-strength number; generate a single integer; discard the RNG; repeat as required. It's not an ideal setup but should be reasonably secure. (Of course, if truly crypto-strength numbers are required then the OP should consult a crypto expert.)

[Jodrell]

Off on a tangent here but, surely radioactive decay is fairly predictable.

[LukeH]

@Jodrell: Radioactive decay is only predictable at a statistical level. For a given sample of "stuff", the half-life predicts statistically how long it will take before half of the sample has undergone radioactive decay. You can't predict when any individual atom will decay.

[Jodrell]

true of course. How easy is it to detect when an individial atom decays?

[Chorel]

So solution you've provided is good or poor, as I'm confused with your info.

[JustAMartin]

Why not take the number directly from the "result = BitConverter.ToInt32(buffer, 0)" ? Does calling one more Random with that seed makes it more random than just returning the result of RNGCryptoServiceProvider?

[LukeH]

@Martin: Seeding Random with the output of RNGCryptoServiceProvider isn't any more random. Presumably, the OP does it so that they can then use the Next(min, max) method.

[Bob.at.Indigo.Health]

I like the concept, but the implementation is buggy. Calling new SecureRandom ().Next(int.MinValue, int.MaxValue); always returns the same value.

[markthewizard1234]

Radioactive decay is predictable

[Stefan]

This answer is really good. It outlines what might be the problem in the provided code and then proceeds to provide a correct solution. Thank you!

[trees_are_great]

Using Random several times in the same tick, meant that Random really wasn't very random at all in my experience.

[Puterdo Borato]

I'd expect NextDouble to return values [0.0, 1.0] but this implementation can return negative numbers.

[Piotr Krankiewicz]

The signed number generations make no sense because they return positive number always. Regarding MaxValue i would recommend using modulo operator, return NextULong() % MaxValue, instead of using loops

[Theodor Zoulias]

The absolute of long.MinValue (-9223372036854775808) is larger than long.MaxValue (9223372036854775807), so the expression dbl / long.MaxValue * (dbl < 0 ? -1 : 1) can produce a double larger than 1.

[bfren]

If you do double min = long.MinValue; and double max = long.MaxValue; then (min / max) == -1?

[Theodor Zoulias]

Try (double)min / (double)max, otherwise the result if the division will be an integer.

[bfren]

double min = long.MinValue; is the same as var min = (double)long.MinValue; so they are already cast to double. If you actually try you'll see it returns 1. I don't know why because you're right about the absolute values.

[Theodor Zoulias]

Yeap, you are right, it returns 1. It seems that the double runs out of significant digits, and the result is rounded. I revoked my downvote, but still this approach seems smelly to me.

[bfren]

Agreed - an alternative could be Math.Abs(dbl) / long.MinValue * -1 which means it never should be mathematically over 1, just looks a bit messy!

[Theodor Zoulias]

I would prefer to get rid of the sign bit of the original long number, by using the bitwise complement operator (~) in case the number was negative. This would guarantee a number in the range 0 - long.MaxValue.

[bytedev]

Looping until you get the sort of value you desire is definitely not more efficient.

[Synctrex]

The major downside here is RandomNumberGenerator doesn't provide the same functionality that Random() does, specifically the ability to generate a random number between two values. That functionality doesn't seem to reappear until .NET 5 docs.microsoft.com/en-us/dotnet/api/…