OpenSSLRandom could be marked as "ThreadSafe" if NativeCrypto.RAND_bytes is thread safe

[j-bahr]

OpenJDK's SecureRandom nextBytes(...) implementation wraps the call to the underdling SecureRandomSpi in a synchronized block if the implementation is not marked as thread safe in the provider registry.

https://github.com/openjdk/jdk/blob/f804f2ce8ef7a859aae021b20cbdcd9e34f9fb94/src/java.base/share/classes/java/security/SecureRandom.java#L759-L768

if (threadSafe) {
            secureRandomSpi.engineNextBytes(bytes);
        } else {
            synchronized (this) {
                secureRandomSpi.engineNextBytes(bytes);
            }
        }

While this is generally fine, there is no need to lock if the underlying call to the NativeCrypto.RAND_bytes is non blocking, which I believe it is, at least in the current OpenSSL implementation. This could be changed easily by adding the following to the OpenSSLProvider constructor

put("SecureRandom.SHA1PRNG ThreadSafe", "true");

When the SecureRandom is initialized and the Spi is loaded, the value for threadsafe will be set by checking the registry with the following code block. Setting ThreadSafe to true in the OpenSSLProvider will ensure that it's marked thread safe to avoid locking.

https://github.com/openjdk/jdk/blob/f804f2ce8ef7a859aae021b20cbdcd9e34f9fb94/src/java.base/share/classes/java/security/SecureRandom.java#L229-L236

    private boolean getThreadSafe() {
        if (provider == null || algorithm == null) {
            return false;
        } else {
            return Boolean.parseBoolean(provider.getProperty(
                    "SecureRandom." + algorithm + " ThreadSafe", "false"));
        }
    }

This is officially documented by oracle here https://docs.oracle.com/en/java/javase/11/docs/specs/security/standard-names.html#service-attributes, which discusses the use of service-attributes.

[prbprbprb]

Thanks!

Note that Conscrypt is tied to BoringSSL whose RAND_bytes can indeed block, but generally only when the kernel entropy pool is not yet initialised. However I believe BoringSSL maintains a separate DRBG per thread to avoid locking in native code so it is probably still thread safe. @davidben ?

Also I'm not sure the Android platform has yet imported the Java 11 Service Attributes code, but adding attributes should be mostly harmless.

[davidben]

Yup, it's perfectly fine to call RAND_bytes on multiple threads without external locking.