Reputation: 51927
I'm generating an anti-CSRF token with this code:
TokenCSRF = new Random(Guid.NewGuid().GetHashCode()).Next(1, 9999).ToString();
Is this token guessable or truly random as expected?
Edit: I replaced the token generator with this:
byte[] byt = new byte[sizeof(Int32)];
RNGCryptoServiceProvider rngCrypto = new RNGCryptoServiceProvider();
rngCrypto.GetBytes(byt);
return BitConverter.ToInt32(byt, 0);
Upvotes: 1
Views: 2087
Reputation: 659956
Is this token guessable or truly random as expected?
That is the wrong question to ask.
Mike z's comment is correct. Guids are guaranteed to be unique, not random. There are different guid generation techniques, and some of them are more random than others. In particular, a guid generator is allowed to generate sequential guids. Most do not, but if you are using a guid for something other than uniqueness, you are using it off-label. I do not like doing anything off-label when security is on the line.
In particular, we have no evidence that any security professional has reviewed the code in either the guid generator or the hash generator to ensure that it has sufficient entropy to defeat an attacker. You should base your security upon tools which have been reviewed by experts.
While we are at it: the code itself is bizarre. It presumes that the hash of a guid has sufficient entropy to seed a random number generator, and then you generate a single 5 decimal digit number from that RNG. By assumption you already have a 32 bit random number in hand; why are you not using it as your random number, if it's already a source of randomness good enough to be a seed? You have a random seed that is more than large enough to be the random number you seek! Feeding it into Random is not going to make it more random.
That said, you should not be using it at all.
The right question to ask is:
What is the correct way to generate a random number for a security purpose?
Use a crypto-strength randomness generator whose on-label use is precisely that. There is one available in the .NET runtime library; use it!
Further reading:
https://blogs.msdn.microsoft.com/ericlippert/tag/guids/
Part 3 is in particular germane to your question.
Upvotes: 9
Reputation: 11301
GUID is guessable because part of it is identified by the machine on which it is generated, and part is the timestamp. It is nothing better than seeding current timestamp instead, which is again guessable.
If you want to generate secure tokens, you may wish to use System.Security.Cryptography.RNGCryptoServiceProvider class, or some similar method.
On a related note, do not limit your tokens to 10K values or anything that small. You would open yourself for brute force attacks. Use something like 64-bit at least and then you will be safe. If you rely on cryptographic RNG, you may wish to generate 8 or 16 bytes, then turn them to string with base-64 encoding. That sounds pretty safe to me.
Upvotes: 1