Reputation: 6216
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?
Upvotes: 6
Views: 5980
Reputation: 8160
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);
}
}
Upvotes: 8
Reputation: 1357
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).
Upvotes: 0
Reputation: 1868
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://stackoverflow.com/a/14110071/2076
Upvotes: 0
Reputation: 171206
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.
Upvotes: 2