10

Is this piece of code safe?

 SecureRandom randomizer = new SecureRandom(String.valueOf(new Date().getTime()).getBytes());

Is this the right way to instance the seed of secure random?

Jordi P.S.
  • 3,838
  • 7
  • 36
  • 59

3 Answers3

26

No, you should avoid the SecureRandom(byte[]) constructor. It is both unsafe and non-portable.

It is non-portable because it behaves differently on Windows vs. other operating systems.

On most OSes, the default algorithm is "NativePRNG", which obtains random data from the OS (usually "/dev/random") and ignores the seed you provide.

On Windows, the default algorithm is "SHA1PRNG", which combines your seed with a counter and computes a hash of the result.

This is bad news in your example, because the input (the current UTC time in milliseconds) has a relatively small range of possible values. For example if an attacker knows that the RNG was seeded in the last 48 hours, they can narrow the seed down to less than 228 possible values, i.e. you have only 27 bits of entropy.

If on the other hand you had used the default SecureRandom() constructor on Windows, it would have called the native CryptoGenRandom function to get a 128-bit seed. So by specifying your own seed you have weakened the security.

If you really want to override the default seed (e.g. for unit testing) you should also specify the algorithm. E.g.

SecureRandom sr = SecureRandom.getInstance("SHA1PRNG");
sr.setSeed("abcdefghijklmnop".getBytes("us-ascii"));

See also How to solve performance problem with Java SecureRandom?
and this blog post: http://www.cigital.com/justice-league-blog/2009/08/14/proper-use-of-javas-securerandom/

Community
  • 1
  • 1
finnw
  • 47,861
  • 24
  • 143
  • 221
  • 1
    Note that the code you provide does initialize the pseudo random generator with just the encoded string as input. So it would create the same random each time. I think you meant to show exactly this, but you might want to make that more clear in the answer (as it is certainly not secure). It might also be different if any other provider would offer `"SHA1PRNG"`, so in that regard it is a bit dangerous. – Maarten Bodewes Sep 04 '12 at 00:07
  • @MaartenBodewes Why do you think the seed is ignored when using "NativePRNG"? – AlikElzin-kilaka Nov 29 '16 at 12:11
  • @AlikElzin-kilaka it is not possible to use a user-provided seed with `/dev/random`. The native Windows RNG can, but when I posted this answer the JRE did not take advantage of this. That was 4 years ago though so it may have changed since then. – finnw Dec 01 '16 at 18:04
2

I think it is best to let the SecureRandom seed itself. This is done by calling nextBytes immediately after it's creation (calling setSeed will prevent this).

final byte[] dummy = new byte[512];
SecureRandom sr = SecureRandom.getInstance("SHA1PRNG");
sr.nextBytes(dummy);

You want to use SHA1PRNG because it guarantees a fast non-blocking implementation even on Linux, where the default is not.

  • 3
    If the `nextBytes()` call is intended for just kicking of the seeding, I would not use 512 byes but only 1 for efficency reasons. – eckes Jan 09 '13 at 02:28
1

The code is reasonably safe because it doesn't just use the seed given to seed the randomizer.

Its not much more random than just using.

SecureRandom randomizer = new SecureRandom();
Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • 2
    Yep, probably using the default is better -- one assumes that the designers of the algorithm would pick a good seeding scheme. The scheme above probably weakens the seed slightly by converting to character representation. – Hot Licks Sep 03 '12 at 13:59
  • 6
    It is considerably *less* random than using the default constructor. – user207421 Sep 03 '12 at 22:10
  • @PeterLawrey Here you recommend using 'new SecureRandom()' but at [this](http://stackoverflow.com/a/8572501/1317865) you say to use 'System.nanoTime'. In [this](http://crypto.stackexchange.com/questions/12306/generate-random-in-secure-message-transfer) situation should I use 'new SecureRandom()' or 'System.nanoTime'. Thanks in advance – Favolas Dec 13 '13 at 18:02