Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make OpenSSLDRBG thread-safe #30

Merged
merged 2 commits into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/main/java/com/canonical/openssl/cipher/OpenSSLCipher.java
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@
import javax.crypto.IllegalBlockSizeException;
import javax.crypto.BadPaddingException;

/* This implementation will be exercised by the user through the
* javax.crypto.Cipher API which isn't marked thread-safe.
* This implementation is also NOT thread-safe and applications need
* handle thread-safety concerns if need be.
*/

abstract public class OpenSSLCipher extends CipherSpi {

static {
Expand Down
92 changes: 60 additions & 32 deletions src/main/java/com/canonical/openssl/drbg/OpenSSLDrbg.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
import java.security.DrbgParameters;
import java.security.Provider;

/* This implementation will be exercised by the user through the
* java.security.SecureRandom API which is marked thread-safe.
* This implementation is also thread-safe.
*/

public class OpenSSLDrbg extends SecureRandomSpi {

public static int DEFAULT_STRENGTH = 128;
Expand All @@ -34,10 +39,10 @@ public class OpenSSLDrbg extends SecureRandomSpi {
long drbgContext;
SecureRandomParameters params;

private static class DRBGState implements Runnable {
private static class NativeDRBG implements Runnable {
private long nativeHandle;

DRBGState(long handle) {
NativeDRBG(long handle) {
this.nativeHandle = handle;
}

Expand All @@ -50,11 +55,20 @@ public void run() {
private Cleaner cleaner = NativeMemoryCleaner.cleaner;
private Cleaner.Cleanable cleanable;

// mutex lock for nextBytes(), borrowed from openjdk's NativePRNG
private final Object LOCK_GET_BYTES = new Object();

// mutex lock for generateSeed(), borrowed from openjdk's NativePRNG
private final Object LOCK_GET_SEED = new Object();

// mutex lock for setSeed(), borrowed from openjdk's NativePRNG
private final Object LOCK_SET_SEED = new Object();

private OpenSSLDrbg() { }

protected OpenSSLDrbg(String name) {
drbgContext = init(name, DEFAULT_STRENGTH, false, false, null);
cleanable = cleaner.register(this, new DRBGState(drbgContext));
cleanable = cleaner.register(this, new NativeDRBG(drbgContext));
}

protected OpenSSLDrbg(String name, SecureRandomParameters params) throws IllegalArgumentException {
Expand All @@ -70,7 +84,7 @@ protected OpenSSLDrbg(String name, SecureRandomParameters params) throws Illegal
} else {
this.drbgContext = init(name, DEFAULT_STRENGTH, false, false, null);
}
cleanable = cleaner.register(this, new DRBGState(drbgContext));
cleanable = cleaner.register(this, new NativeDRBG(drbgContext));
}

boolean isInitialized() {
Expand All @@ -84,58 +98,72 @@ protected SecureRandomParameters engineGetParameters() {

@Override
protected byte[] engineGenerateSeed(int numBytes) {
return generateSeed0(numBytes);
synchronized (LOCK_GET_SEED) {
return generateSeed0(numBytes);
}
}

@Override
protected void engineNextBytes(byte[] bytes) {
nextBytes0(bytes, DEFAULT_STRENGTH, false, null);
synchronized (LOCK_GET_BYTES) {
nextBytes0(bytes, DEFAULT_STRENGTH, false, null);
}
}

protected void engineNextBytes(byte[] bytes, SecureRandomParameters params) throws IllegalArgumentException {
if (params == null) {
engineNextBytes(bytes);
return;
synchronized (LOCK_GET_BYTES) {
if (params == null) {
engineNextBytes(bytes);
return;
}

if (!(params instanceof DrbgParameters.NextBytes)) {
throw new IllegalArgumentException("Parameters of type DrbgParameters.NextByte expected, passed " + params.getClass());
}

DrbgParameters.NextBytes nb = (DrbgParameters.NextBytes)params;
nextBytes0(bytes, nb.getStrength(), nb.getPredictionResistance(), nb.getAdditionalInput());
}

if (!(params instanceof DrbgParameters.NextBytes)) {
throw new IllegalArgumentException("Parameters of type DrbgParameters.NextByte expected, passed " + params.getClass());
}

DrbgParameters.NextBytes nb = (DrbgParameters.NextBytes)params;
nextBytes0(bytes, nb.getStrength(), nb.getPredictionResistance(), nb.getAdditionalInput());
}

protected void engineReseed() {
reseed0(null, false, null);
synchronized (LOCK_SET_SEED) {
reseed0(null, false, null);
}
}

@Override
protected void engineReseed(SecureRandomParameters params) throws IllegalArgumentException {
if (params == null) {
engineReseed();
return;
}
synchronized (LOCK_SET_SEED) {
if (params == null) {
engineReseed();
return;
}

if (!(params instanceof DrbgParameters.Reseed)) {
throw new IllegalArgumentException("Parameters of type DrbgParameters.Reseed expected, passed " + params.getClass());
if (!(params instanceof DrbgParameters.Reseed)) {
throw new IllegalArgumentException("Parameters of type DrbgParameters.Reseed expected, passed " + params.getClass());
}
DrbgParameters.Reseed rs = (DrbgParameters.Reseed)params;
reseed0(null, rs.getPredictionResistance(), rs.getAdditionalInput());
}
DrbgParameters.Reseed rs = (DrbgParameters.Reseed)params;
reseed0(null, rs.getPredictionResistance(), rs.getAdditionalInput());
}

protected void engineSetSeed(byte[] seed) {
reseed0(seed, false, null);
synchronized (LOCK_SET_SEED) {
reseed0(seed, false, null);
}
}

protected void engineSetSeed(long seed) {
byte [] seedBytes = new byte[8];
for (int i = 0; i < 8; i++) {
seedBytes[i] = (byte)(seed & (long)0xff);
seed = seed >> 8;
synchronized (LOCK_SET_SEED) {
byte [] seedBytes = new byte[8];
for (int i = 0; i < 8; i++) {
seedBytes[i] = (byte)(seed & (long)0xff);
seed = seed >> 8;
}

engineSetSeed(seedBytes);
}

engineSetSeed(seedBytes);
}

private static void cleanupNativeMemory(long handle) {
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/canonical/openssl/kdf/OpenSSLPBKDF2.java
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@
* can be represented by class PBEKeySpec. As a result, only PBKDF2
* is implemented in this prototype.
*/

/* This implementation will be exercised by the user through the
* javax.crypto.SecretKeyFactory API which isn't marked thread-safe.
* This implementation is also NOT thread-safe and applications need
* handle any concerns regarding the same.
*/
public class OpenSSLPBKDF2 extends SecretKeyFactorySpi {

static {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@
import javax.crypto.SecretKey;
import java.util.Base64;

/* This implementation will be exercised by the user through the
* javax.crypto.KeyAgreement API which isn't marked thread-safe.
* This implementation is also NOT thread-safe and applications need
* handle thread-safety concerns if need be.
*/

abstract public class OpenSSLKeyAgreement extends KeyAgreementSpi {
static {
NativeLibraryLoader.load();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,12 @@
import java.security.interfaces.RSAPublicKey;
import javax.crypto.spec.SecretKeySpec;

/* This implementation will be exercised by the user through the
* javax.crypto.KEM API which isn't marked thread-safe.
* This implementation is also NOT thread-safe and applications need
* handle thread-safety concerns if need be.
*/

final public class OpenSSLKEMRSA implements KEMSpi {

static {
Expand Down
6 changes: 6 additions & 0 deletions src/main/java/com/canonical/openssl/mac/OpenSSLMAC.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@
import java.security.spec.AlgorithmParameterSpec;
import javax.xml.crypto.dsig.spec.HMACParameterSpec;

/* This implementation will be exercised by the user through the
* javax.crypto.Mac API which isn't marked thread-safe.
* This implementation is also NOT thread-safe and applications need
* handle thread-safety concerns if need be.
*/

public abstract class OpenSSLMAC extends MacSpi {

static {
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/com/canonical/openssl/md/OpenSSLMD.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@
import java.util.Arrays;
import java.util.Set;

/* This implementation will be exercised by the user through the
* java.security.MessageDigest API which isn't marked thread-safe.
* This implementation is also NOT thread-safe and applications need
* handle thread-safety concerns if need be.
*/
public abstract class OpenSSLMD extends MessageDigestSpi {

static {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@
import java.security.SignatureException;
import java.security.SignatureSpi;

/* This implementation will be exercised by the user through the
* java.security.Signature API which isn't marked thread-safe.
* This implementation is also NOT thread-safe and applications need
* handle thread-safety concerns if need be.
*/
public abstract class OpenSSLSignature extends SignatureSpi {

static {
Expand Down
Loading