fix(security): close #135 — replace SHA1PRNG with default SecureRandom (SEC-02)#157
Open
NSchatz wants to merge 4 commits into
Open
Conversation
…care#135 - Adds SecureRandomAlgorithmTest with four JUnit 4 cases: 1. new SecureRandom().getAlgorithm() != "SHA1PRNG" on JDK 17 2. Digester salt generator algorithm != "SHA1PRNG" after initialize() 3. PBEEncryptor salt generator algorithm != "SHA1PRNG" after initialize() 4. Probabilistic randomness sanity (two instances → different bytes) - Tests 2 and 3 are EXPECTED TO FAIL before the production swap in 2.2 because the current Digester.java:179 and PBEEncryptor.java:83 pin the salt generator to SecureRandom.getInstance("SHA1PRNG") (FIPS-rejected). - Uses iteration count 1000 (test-speed; not asserting on iteration count). - Follows DigesterTest.java pattern for BouncyCastleProvider setup.
…ult SecureRandom in Digester and PBEEncryptor
Replaces SecureRandom.getInstance("SHA1PRNG") with new SecureRandom() at
the two salt-generator call sites in com.mirth.commons.encryption. On
FIPS-mode JVM 17 (RHEL 9 / AlmaLinux 9 with `update-crypto-policies --set
FIPS`), the JVM rejects SHA1PRNG and Digester.initialize() /
PBEEncryptor.initialize() throw NoSuchAlgorithmException at server start,
preventing BridgeLink from running.
The no-arg new SecureRandom() resolves to a provider-appropriate algorithm
(NativePRNG on non-FIPS Linux, a FIPS-approved DRBG on FIPS JVMs) per
OpenJDK SecureRandom javadoc and OpenLiberty FIPS issue #24469.
- server/src/com/mirth/commons/encryption/Digester.java:179 — swap.
- server/src/com/mirth/commons/encryption/PBEEncryptor.java:83 — swap.
Surrounding try/catch blocks are intentionally preserved (CONTEXT.md
locks "two one-line changes only" — minimal diff). The NoSuchAlgorithmException
catch in Digester.java becomes unreachable but harmless; future refactor
tracked under threat T-02-03.
After this commit, all four cases in SecureRandomAlgorithmTest (commit
7768cc9) move from RED → GREEN.
Closes Innovar-Healthcare#135
After the swap from SecureRandom.getInstance("SHA1PRNG") to
new SecureRandom() the surrounding catch (NoSuchAlgorithmException)
catches an exception the body can no longer throw. SpotBugs /
SonarQube will flag the dead catch.
Drop the try/catch and the now-unused import. The EncryptionException
declaration is retained on initialize()'s signature for forward
compatibility with subclass overrides.
Without setProvider() the test threw IllegalArgumentException: missing provider when PBEEncryptor.initialize() called SecretKeyFactory.getInstance. Mirror the existing pattern from digesterSaltGeneratorIsNotSHA1PRNGAfterInitialize. Verified locally: ant test-compile + targeted JUnit run — all 4 tests pass plus all 9 existing DigesterTest tests. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #135.
Replaces
SecureRandom.getInstance("SHA1PRNG")withnew SecureRandom()at the two vendoredcom.mirth.commons.encryptionsalt-generator call sites. On FIPS-mode JVM 17 (RHEL 9 / AlmaLinux 9 withupdate-crypto-policies --set FIPS), the JVM rejects SHA1PRNG and the server cannot start —Digester.initialize()andPBEEncryptor.initialize()throwNoSuchAlgorithmExceptionat startup. The default no-argSecureRandomresolves to a JVM-default provider that is FIPS-approved (per OpenJDK SecureRandom javadoc and OpenLiberty FIPS issue #24469).Changes
server/src/com/mirth/commons/encryption/Digester.java:179—SecureRandom.getInstance("SHA1PRNG")→new SecureRandom().server/src/com/mirth/commons/encryption/PBEEncryptor.java:83— same replacement.server/test/com/mirth/commons/encryption/test/SecureRandomAlgorithmTest.java(new) — four JUnit 4 tests:new SecureRandom().getAlgorithm()is not"SHA1PRNG".Digester.initialize()does not throw; its salt generator algorithm is not"SHA1PRNG".PBEEncryptor.initialize()does not throw; its salt generator algorithm is not"SHA1PRNG".new SecureRandom()instances produce different byte arrays (probabilistic randomness sanity per threat T-02-02).Surrounding
try/catchblocks are intentionally preserved (minimal-diff lock — CONTEXT.md "two one-line changes only"). TheNoSuchAlgorithmExceptioncatch inDigester.javais now unreachable but harmless; future refactor tracked under threat T-02-03.Test plan
ant -f server/build.xml test -Dtest=SecureRandomAlgorithmTest— all four tests pass (CI).ant -f server/build.xml test -Dtest=DigesterTest— existing tests pass, no regression (CI).grep -rn "SHA1PRNG" server/src donkey/src client/src custom-extensions/ --include='*.java'— zero production hits (verified locally).Rationale
Per OpenJDK SecureRandom javadoc (Java 17) and OpenLiberty FIPS issue #24469, FIPS-mode JVMs reject the
SHA1PRNGalgorithm name and select a FIPS-approved DRBG when constructed via the no-arg constructor. The phase plan (.planning/phases/01-security-cluster/01-CONTEXT.md§#135) explicitly locks unconditionalnew SecureRandom()— no DRBG try/catch, no Tomcat-style runtime detection, nobc-fipsmigration (SEC-V2-02 deferred).Local test verification (2026-05-13)
Ant 1.10.14 installed via tarball into
~/.local/opt/ant. OpenJDK 17.0.18 used.ant -f server/build.xml compile— BUILD SUCCESSFUL (35s)ant -f server/build.xml test-compile— BUILD SUCCESSFUL (23s)java org.junit.runner.JUnitCore com.mirth.commons.encryption.test.SecureRandomAlgorithmTest com.mirth.commons.encryption.test.DigesterTest— OK (13 tests) in 18.6snewSecureRandomDoesNotResolveToSHA1PRNG,digesterSaltGeneratorIsNotSHA1PRNGAfterInitialize,pbeEncryptorSaltGeneratorIsNotSHA1PRNGAfterInitialize,twoCallsToNewSecureRandomProduceDifferentBytesgrep -rn "SHA1PRNG" server/src donkey/src client/src custom-extensions/ --include='*.java'returns zero matches.Follow-up commit in this verification cycle
5fcc16c65—fix(135): set BouncyCastleProvider in PBEEncryptor test. The original test omittedencryptor.setProvider(new BouncyCastleProvider())before callingencryptor.initialize(), causingIllegalArgumentException: missing providerfromSecretKeyFactory.getInstance(...). Mirrors the existing pattern indigesterSaltGeneratorIsNotSHA1PRNGAfterInitialize. Production code unchanged.