How can I use RNGCryptoServiceProvider to create a random password based on my preferred characters?

Question

What I am currently doing to create a random password is like this:

    public static int getRandomNumber(int maxNumber)
    {
        if (maxNumber < 1)
            throw new System.Exception("The maxNumber value should be greater than 1");
        byte[] b = new byte[4];
        new System.Security.Cryptography.RNGCryptoServiceProvider().GetBytes(b);
        int seed = (b[0] & 0x7f) << 24 | b[1] << 16 | b[2] << 8 | b[3];
        System.Random r = new System.Random(seed);
        return r.Next(1, maxNumber);
    }

Some possible problems? It's a static function that has some strange seeding patterns which may not be secure and still uses System.Random().

Using the above Random Number generator, I create a string, in this inefficient way:

  validCharacters = "abcdefghjkmnoxyz023456789#!@";

then looping through and getting a "createPassword(length)" type string using the valid array (note the use of a character set that doesn't include easily mistaken characters like 1 i etc).

Is this how to do it, or is there an easier, more secure, more effective way?

Bittern

Answer 1

For generating the random numbers, you can return the remainder of seed divided by maxNumber:

public static int getRandomNumber(int maxNumber)
{
    if (maxNumber < 1)
        throw new System.Exception("The maxNumber value should be greater than 1");
    byte[] b = new byte[4];
    new System.Security.Cryptography.RNGCryptoServiceProvider().GetBytes(b);
    int seed = (b[0] & 0x7f) << 24 | b[1] << 16 | b[2] << 8 | b[3];
    return seed % maxNumber;
}

maxNumber is exclusive in this case, which is how Random.Next(maxNumber) works too.

EDIT

The comment from @Servy was very interesting and lead me to an article by Stephen Toub and Shawn Farkas titled "Tales from the CryptoRandom" in the September 2007 issue of MSDN Magazine, available for download here, which has an example of reimplementing Random using RNGCryptoServiceProvider that has a work-around for the bias. I've included their implementation here, since the formatting at the source is pretty nasty, but the article is worth reading for the reasoning behind it.

public class CryptoRandom : Random
{
    private RNGCryptoServiceProvider _rng = new RNGCryptoServiceProvider();
    private byte[] _uint32Buffer = new byte[4];
    public CryptoRandom() { }
    public CryptoRandom(Int32 ignoredSeed) { }
    public override Int32 Next()
    {
        _rng.GetBytes(_uint32Buffer);
        return BitConverter.ToInt32(_uint32Buffer, 0) & 0x7FFFFFFF;
    }
    public override Int32 Next(Int32 maxValue)
    {
        if (maxValue < 0) throw new ArgumentOutOfRangeException("maxValue");
        return Next(0, maxValue);
    }
    public override Int32 Next(Int32 minValue, Int32 maxValue)
    {
        if (minValue > maxValue) throw new ArgumentOutOfRangeException("minValue");
        if (minValue == maxValue) return minValue;
        Int64 diff = maxValue - minValue;
        while (true)
        {
            _rng.GetBytes(_uint32Buffer);
            UInt32 rand = BitConverter.ToUInt32(_uint32Buffer, 0);
            Int64 max = (1 + (Int64)UInt32.MaxValue);
            Int64 remainder = max % diff;
            if (rand < max - remainder)
            {
                return (Int32)(minValue + (rand % diff));
            }
        }
    }
    public override double NextDouble()
    {
        _rng.GetBytes(_uint32Buffer);
        UInt32 rand = BitConverter.ToUInt32(_uint32Buffer, 0);
        return rand / (1.0 + UInt32.MaxValue);
    }
    public override void NextBytes(byte[] buffer)
    {
        if (buffer == null) throw new ArgumentNullException("buffer");
        _rng.GetBytes(buffer);
    }
}

Teratogenic

You don't have a strictly uniform probability distribution here. Here's an article for a detailed explanation. – Smalltime

Thank you, @Smalltime - very good information. I've expanded the answer to include a better implementation from an MSDN article that (hopefully) avoids this bias in the numbers generated. – Teratogenic

Answer 2

Your use of System.Random makes asserting security properties very hard because its output is not cryptographically strong. There are only 4 billion passwords now, because there are only 4 billion possible seeds.

Very unsafe. Debian password security was compromised because of a similar problem.

Use RNGCryptoServiceProvider directly to generate totally random ints as input for your password algorithm.

Foam

That's what I'm asking... How?? If I just get bytes then I get % signs and other weird characters, how do I limit it to the characters I want? – Bittern

Create a full-range int, then use the % operator to restrict the range. randomInt % maxCount. Random.Next does the same thing. – Foam

@Foam That's not quite true. If maxCount is not a factor of randomInt then there is not a strictly even probability distribution there. For more info – Smalltime

@Smalltime yeah that's true and good to realize. For 35 chars and 2 billion possible ints the problem should be irrelevant even for security purposes. – Foam

Unfortunately, the BCL is very weak with random number generation anyway. It does not help with such things at all. It also does not solve the seeding problem, which causes about 1 question on SO per day and is completely solvable even without developer cooperation. Just a disappointing API. – Foam

Answer 3

You could inherit from System.Random using a RNGCryptoServiceProvider to get the numbers, but you'll have to take some care because you'll need to override quite a few methods:

/*
[MSDN]

Notes to Inheritors:

In the .NET Framework versions 1.0 and 1.1, a minimum implementation of
a class derived from Random required overriding the Sample method
to define a new or modified algorithm for generating random numbers.
The derived class could then rely on the base class implementation
of the Random.Next(), Random.Next(Int32), Random.Next(Int32, Int32),
NextBytes, and NextDouble methods to call the derived class implementation of the Sample method.

In the .NET Framework version 2.0 and later, the behavior of the Random.Next(),
Random.Next(Int32, Int32), and NextBytes methods have changed
so that these methods do not necessarily call the derived class implementation
of the Sample method.
As a result, classes derived from Random that target the .NET Framework 2.0
and later should also override these three methods.
*/
class CryptoRandom : System.Random {
    private RandomNumberGenerator rng;
    public CryptoRandom() {
        rng = new RNGCryptoServiceProvider();
    }
    public override int Next() {
        byte[] bytes = new byte[4];
        rng.GetBytes(bytes);
        return int.MaxValue & BitConverter.ToInt32(bytes, 0);
    }
    public override void NextBytes(byte[] b)
    {
        rng.GetBytes(b);
    }
    public override int Next(int lo, int hi){
        throw new Exception("TODO override (arc4random_uniform is beautiful)");
    }
    protected override double Sample()
    {
        throw new Exception("TODO override");
    }
}

Or, since you'll be using all this just to get an index into a small (length<=255) string, you could use the bytes directly to get an index into it (avoiding modulo bias, which is why I wrote this answer in the first place -- see arc4random_uniform at http://BXR.SU/OpenBSD/lib/libc/crypt/arc4random_uniform.c#arc4random_uniform).

Combined

Answer 4

You could just take the raw bytes and base-28 encode them using the validCharacters string as the "digits" of the base-28 number. You can use the code here https://mcmap.net/q/744767/-base-n-encoding-of-a-byte-array

Preposition