Is rapidly creating BouncyCastle SecureRandom instances problematic?

Question

As noted at Random number generator only generating one random number, it's generally incorrect to create a new instance of System.Random every time that you need another random number, since System.Random is seeded based upon the clock and so multiple instances created in the same tick will yield identical random numbers. As such, one common practice (at least in single-threaded applications) is to create a single instance of Random stored in a static field that is used for all random number generation.

RNGCryptoServiceProvider, on the other hand, does not have this particular flaw... but is apparently costly to instantiate, and therefore it's again recommended to store and reuse a single instance of it.

How about Org.BouncyCastle.Security.SecureRandom? Do I similarly need to store and reuse a single instance of it, or is it basically fine to create instances on demand every time that I need another random number?

Related (since it's also about the undocumented basics of how to use this class correctly): https://stackoverflow.com/q/46792243/1709587 — Mark Amery, Oct 17 '17 at 14:18
I'd argue that it depends on how often you need a random number in practice. In one of my own projects I use `RNGCryptoServiceProvider.Create()` to generate a session token for each new user to the site and performance has been handling 1,000's visits per day. So how frequently do you need a random number? — DiskJunky, Oct 17 '17 at 15:02
From the answer linked in your comment, you would end up instantiating a new RNGCryptoServiceProvider per SecureRandom instance. Other than this performance hit it looks fine. — President James K. Polk, Oct 17 '17 at 15:42
@JamesKPolk actually that `Master` random generator (for which `RNGCryptoServiceProvider` is used) is static singleton so it is initialized one time, not every time `SecureRandom` is created.I don't think its true by the way that creating `new RNGCryptoServiceProvider()` is costly. Internally it uses unmanaged api and if you use default constructor - it will use the same unmanaged "handle", essentially making no difference between reusing one instance and creating new one every time (because they all use the same unmanaged "handle"). — Evk, Oct 17 '17 at 15:52
@Evk: Thanks for correcting me, and thanks for your answer at the other link. In that case there should be neither security nor performance issues. — President James K. Polk, Oct 17 '17 at 15:55

score 2 · Accepted Answer · answered Oct 17 '17 at 17:07

We can again (like in related question) look at source code to draw some conclusions (SecureRandom source code for reference).

All work in constructor goes for creating pseudo-random generator:

private static DigestRandomGenerator CreatePrng(string digestName, bool autoSeed)
{
    IDigest digest = DigestUtilities.GetDigest(digestName);
    if (digest == null)
        return null;
    DigestRandomGenerator prng = new DigestRandomGenerator(digest);
    if (autoSeed)
    {
        prng.AddSeedMaterial(NextCounterValue());
        prng.AddSeedMaterial(GetNextBytes(Master, digest.GetDigestSize()));
    }
    return prng;
}

Creating digest (hash) costs nothing (relative to other work). For example Sha256Digest used by default (with empty constructor) just allocates small byte[] buffer. Creating DigestRandomGenerator itself also costs nothing (couple small buffers). Major work done is here:

prng.AddSeedMaterial(GetNextBytes(Master, digest.GetDigestSize()));

It uses "master" RNG to generate seed value. Master RNG on full .NET platform is RNGCryptoServiceProvider (which for SecureRandom is stored in static field and initialized only once). So all work when creating SecureRandom goes to creating cryptographically random seed for pseudo RNG.

I'd say, it's better not create new instance every time, at least for small generation (for one-two NextInt() calls), because if you create new instance for every single generated number - you essentially double the costs (one time to generate crypto random number for seed and one to generate your target random number). Because (as far as I know), SecureRandom is thread safe - there is not much reason to not reuse one instance.

Side note - I don't think RNGCryptoServiceProvider is heavy to create as your link claims. Its constructor goes like this:

public RNGCryptoServiceProvider()
  : this((CspParameters) null)
{
}

[SecuritySafeCritical]
public RNGCryptoServiceProvider(CspParameters cspParams)
{
  if (cspParams != null)
  {
    this.m_safeProvHandle = Utils.AcquireProvHandle(cspParams);
    this.m_ownsHandle = true;
  }
  else
  {
    // we are interested in this path
    this.m_safeProvHandle = Utils.StaticProvHandle;
    this.m_ownsHandle = false;
  }
}

So when you create new instance (without providing csp) - it reuses the same Utils.StaticProvHandle, so it uses the same "unmanaged" instance of RNG provider. Which in turn means creating new instance and reusing the same instance have no difference in performance. Maybe in previous versions of .NET it was not like this, not sure.

Is rapidly creating BouncyCastle SecureRandom instances problematic?

1 Answers1

Linked