google / google/conscrypt

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

Open
#1,097 2 comments 0 reactions 1 assignee Claimed by @prbprbprb View on GitHub
Dominant language
Java
Stars
1.4k
Forks
326
Avg merge
16h 22m
Merged PRs (30d)
17

Description

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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.