OpenSSLRandom could be marked as "ThreadSafe" if NativeCrypto.RAND_bytes is thread safe
- 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
Assessment
This issue has not been assessed yet.