Skip to content

Commit

Permalink
Add unit tests and checks when setting DEFAULT to be ThreadSafe
Browse files Browse the repository at this point in the history
This commit improves the way SecureRandom algorithms (LibCryptoRng and its alias DEFAULT) are registered and adds unit tests.

1. Check that DEFAULT points to LibCryptoRng before setting it to be ThreadSafe. This is to mitigate risks that a future code change uses a different non-ThreadSafe algorithm for the DEFAULT alias.
2. Add unit tests checking that both LibCryptoRng and DEFAULT are ThreadSafe.
  • Loading branch information
Fabrice Benhamouda committed Apr 10, 2024
1 parent 52e926a commit 002095c
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,24 @@ private void buildServiceMap() {
singletonMap("ThreadSafe", "true"),
"DEFAULT")
.setSelfTest(LibCryptoRng.SPI.SELF_TEST);
// Following line is a workaround to ensure that the SecureRandom service
// is seens as ThreadSafe by SecureRandom, when using the alias name DEFAULT
setProperty("SecureRandom.DEFAULT ThreadSafe", "true");

// Following lines are a workaround to ensure that the SecureRandom service
// is seen as ThreadSafe by SecureRandom, when using the alias name DEFAULT
// See https://bugs.openjdk.org/browse/JDK-8329754

// We add additional tests to confirm the DEFAULT algorithm is actually ThreadSafe
// This is to prevent issues in case a future code change set DEFAULT to a non-ThreadSafe
// algorithm

// Get the name of the algorithm pointed by the alias name DEFAULT
String algorithmUsedForDEFAULT = getProperty("Alg.Alias.SecureRandom.DEFAULT");
// If this alias exists and the algorithm pointed by it is thread safe, then mark DEFAULT
// ThreadSafe
if (algorithmUsedForDEFAULT != null
&& "true"
.equals(getProperty("SecureRandom." + algorithmUsedForDEFAULT + " ThreadSafe"))) {
setProperty("SecureRandom.DEFAULT ThreadSafe", "true");
}
}

addSignatures();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,18 @@ public void testSerialization() throws Exception {
result.assertHealthy();
}
}

/**
* Check that the SecureRandom algorithm `LibCryptoRng` and its alias `DEFAULT` are both marked
* thread safe
*/
@Test
public void testSecureRandomThreadSafe() {
assertEquals(
"true",
AmazonCorrettoCryptoProvider.INSTANCE.getProperty("SecureRandom.LibCryptoRng ThreadSafe"));
assertEquals(
"true",
AmazonCorrettoCryptoProvider.INSTANCE.getProperty("SecureRandom.DEFAULT ThreadSafe"));
}
}

0 comments on commit 002095c

Please sign in to comment.