3

I am trying to use a SecureRandom to generate random numbers in my java project. But I am a little confused as how to keep my object for SecureRandom. Should it be a static class member. I dont intend to call this from outside. Below is my current implementation :

Class MyClass {

    private static final SecureRandom secureRandom = new SecureRandom();

    private long calculate(int noOfRetry){
        final long value = someValueCalculationWith-noOfRetry;
        final float randomNo = secureRandom().nextFloat() + 1;
        return (long) (value*randomNo);
    }
}

Is this the correct way to use SecureRandom in java ?

Maarten Bodewes
  • 90,524
  • 13
  • 150
  • 263
Som
  • 1,522
  • 1
  • 15
  • 48
  • 1
    Depends on the context, see here for some very good info re entropy issues https://stackoverflow.com/questions/27622625/securerandom-with-nativeprng-vs-sha1prng/27638413 – Nigel Savage Jul 07 '21 at 13:06
  • 3
    Beware that `float` and `double` have very interesting properties w.r.t. precision, rounding and randomness. When I see a `float` or `double` being used in a security application I always feel a little shudder - something may be out of kilt. Use e.g. `nextInt(int max)` instead. – Maarten Bodewes Jul 07 '21 at 13:43
  • Oh, and this seems to be a random timeout or whatever, beware too that may successfully hides the calculation time of one calculation, but statistics can still provide interesting details. E.g. if you add the minimum additional time to the minimum running time, you'd leak the latter, even for that specific instance. Having a *constant time* implementation may be better at hiding stuff. – Maarten Bodewes Jul 07 '21 at 13:49

2 Answers2

2

No, don't make it static. If you want you can make it an instance field, but making it a class field is not optimal. E.g. see the note on thread-safety on the Random class that it has been derived from:

Instances of java.util.Random are threadsafe. However, the concurrent use of the same java.util.Random instance across threads may encounter contention and consequent poor performance. Consider instead using ThreadLocalRandom in multithreaded designs.

Beware though that the ThreadLocalRandom is not cryptographically secure, and therefore not a good option for you. In general, you should try and avoid using static class fields, especially when the instances are stateful.

If you only require the random instance in one or a few methods that are not in a tight loop then making it a local instance is perfectly fine (just using var rng = new SecureRandom() in other words, or even just new SecureRandom() if you have a single method call that requires it).

Maarten Bodewes
  • 90,524
  • 13
  • 150
  • 263
  • Thanks Mate ... I got your point. So in the same method I can call the new SecureRandom() multiple times like this : `final float randomNo = new SecureRandom().nextFloat() + 1;` – Som Jul 07 '21 at 13:43
  • Just a note here. According to this link SecureRandom is guaranteed to be thread safe https://bugs.openjdk.java.net/browse/JDK-8165115 Specially there seems to be no need of synchronizing the access to nextFloat and the only synchronized methods are related to the seed and generating byte arrays. – avd Jul 07 '21 at 13:50
  • 1
    Well, in the same method I would just create one instance and use a variable. The initialization time for `SecureRandom` is not that high, but having it once is always better than multiple times. – Maarten Bodewes Jul 07 '21 at 13:51
  • @avd Of course, that should be the case because - as a child class - it needs to adhere to the specifications for `Random`. That's why I included a quote from the JavaDoc of that class. That implementation just allows for the `SecureRandomSpi`to rely on `SecureRandom` for thread safety, it seems. Maybe some external providers didn't read the `Random` class documentation :) – Maarten Bodewes Jul 07 '21 at 13:52
  • I do not see anything in your writeup that justifies describing static use as "dangerous". Potentially slow maybe, but not dangerous. – President James K. Polk Jul 07 '21 at 16:50
  • Yeah, good point, I'll adjust it. It could be a problem for Java 8 in case a provider implementation is not thread safe, but that's about it. – Maarten Bodewes Jul 07 '21 at 22:06
0

I totally agree with Maartens's answer. However one can notice that java.util classes create statics for SecureRandom themselfes.

public final class UUID implements java.io.Serializable, Comparable<UUID> {

    ...

    /*
     * The random number generator used by this class to create random
     * based UUIDs. In a holder class to defer initialization until needed.
     */
    private static class Holder {
        static final SecureRandom numberGenerator = new SecureRandom();
    }
user17477134
  • 1