fix(security): close #125 — eliminate username enumeration via timing equalization (SEC-04)

server/src/com/mirth/connect/model/PasswordRequirements.java

@@ -31,14 +31,40 @@ public class PasswordRequirements implements Serializable {
     private int reuseLimit;
     private boolean allowUsernameEnumeration;
 
+    /**
+     * No-arg constructor — hardened defaults bundled with issue #125 (SEC-04).
+     *
+     * <p>Previously every integer field defaulted to {@code 0}, which left
+     * operators who never explicitly configured a password policy with
+     * <em>no</em> enforcement at all (zero-length passwords, no retry limit,
+     * no lockout). The hardened defaults below match the OWASP Authentication
+     * Cheat Sheet baseline:
+     * <ul>
+     *   <li>{@code minLength = 8}</li>
+     *   <li>{@code minUpper = 1}, {@code minLower = 1}, {@code minNumeric = 1}</li>
+     *   <li>{@code minSpecial = 0} (kept zero — special characters are often
+     *       blocked or normalized by upstream LDAP/AD integrations)</li>
+     *   <li>{@code retryLimit = 5}, {@code lockoutPeriod = 1} (hour) — the
+     *       field is interpreted as hours by
+     *       {@code LoginRequirementsChecker.getStrikeTimeRemaining}
+     *       ({@code Duration.standardHours(lockoutPeriod)}); the shipped
+     *       {@code server/conf/mirth.properties} ships {@code lockoutperiod=1}
+     *       and this default matches</li>
+     *   <li>{@code expiration = 0}, {@code gracePeriod = 0},
+     *       {@code reusePeriod = 0}, {@code reuseLimit = 0} (opt-in — unchanged)</li>
+     *   <li>{@code allowUsernameEnumeration = false} (unchanged — already safe)</li>
+     * </ul>
+     * Operators who relied on the old all-zero defaults to bypass policy can
+     * still call setters explicitly to restore that posture.
+     */
     public PasswordRequirements() {
-        this.minLength = 0;
-        this.minUpper = 0;
-        this.minLower = 0;
-        this.minNumeric = 0;
+        this.minLength = 8;
+        this.minUpper = 1;
+        this.minLower = 1;
+        this.minNumeric = 1;
         this.minSpecial = 0;
-        this.retryLimit = 0;
-        this.lockoutPeriod = 0;
+        this.retryLimit = 5;
+        this.lockoutPeriod = 1;
         this.expiration = 0;
         this.gracePeriod = 0;
         this.reusePeriod = 0;

server/src/com/mirth/connect/server/controllers/DefaultUserController.java

@@ -44,6 +44,49 @@ public class DefaultUserController extends UserController {
     public static final String VACUUM_LOCK_PERSON_STATEMENT_ID = "User.vacuumPersonTable";
     public static final String VACUUM_LOCK_PREFERENCES_STATEMENT_ID = "User.vacuumPersonPreferencesTable";
     private static final String INCORRECT_CREDENTIALS_MESSAGE = "Incorrect username or password.";
+
+    /**
+     * Pre-computed PBKDF2WithHmacSHA256 hash used to equalize wall-clock cost
+     * between the known-user and unknown-user authentication paths, mitigating
+     * username enumeration via response-time side-channel (CWE-208).
+     *
+     * <p>Without this constant, an unauthenticated attacker can distinguish
+     * valid usernames from invalid ones by measuring login latency: the valid
+     * path runs PBKDF2 (~100ms at 600,000 iterations) while the invalid path
+     * returns immediately. See OWASP Authentication Cheat Sheet and the
+     * Spring Security CVE-2025-22234 disclosure for the canonical fix
+     * pattern.
+     *
+     * <p>Generation parameters (byte-equivalent to the production
+     * {@link com.mirth.commons.encryption.Digester} default):
+     * <ul>
+     *   <li>Algorithm: {@code PBKDF2WithHmacSHA256}</li>
+     *   <li>Iterations: {@code 600000} (matches
+     *       {@code EncryptionSettings.DEFAULT_DIGEST_ITERATIONS} and
+     *       {@code Digester.DEFAULT_ITERATIONS})</li>
+     *   <li>Provider: BouncyCastle</li>
+     *   <li>Salt: 8 bytes (deterministic — first 8 bytes of
+     *       {@code SHA-256("dummy-placeholder-do-not-use-in-prod")})</li>
+     *   <li>Key size: 256 bits</li>
+     *   <li>Output: base64 of (salt || derivedKey) — 40 bytes raw</li>
+     *   <li>Plaintext source: {@code "dummy-placeholder-do-not-use-in-prod"}
+     *       — never a real credential</li>
+     * </ul>
+     *
+     * <p><b>Residual risk:</b> Operators who override {@code digest.iterations}
+     * to a non-default value retain a smaller-but-non-zero residual timing
+     * delta. Disclosed in PR #125 body. Dynamic computation at server startup
+     * is a documented follow-up.
+     *
+     * <p><b>Regeneration:</b> Must be regenerated if
+     * {@code EncryptionSettings.DEFAULT_DIGEST_ITERATIONS} changes. The hash
+     * is validated at boot indirectly by
+     * {@code AuthorizeUserTimingTest#dummyHashIsConsumableWithoutThrowing},
+     * which asserts {@code digester.matches(...)} returns false without
+     * throwing — guards against malformed base64 / wrong salt-size pitfalls.
+     */
+    private static final String DUMMY_HASH = "L5+whfGBNpMMKJgAWt/jWIUPX8jnOCy5CzQomGpdNu/3hTQRWNp/yg==";
+
     private Logger logger = LogManager.getLogger(this.getClass());
     private ExtensionController extensionController = null;
 
@@ -297,8 +340,13 @@ public LoginStatus authorizeUser(String username, String plainPassword, String s
             // Retrieve the matching User
             User validUser = getUser(null, username);
 
+            // Hoisted above the if/else so both branches share a single Digester
+            // reference. The unknown-user path consumes the same PBKDF2 work as
+            // the known-user path to defeat username enumeration via timing
+            // (CWE-208 — see DUMMY_HASH javadoc and AuthorizeUserTimingTest).
+            Digester digester = ControllerFactory.getFactory().createConfigurationController().getDigester();
+
             if (validUser != null) {
-                Digester digester = ControllerFactory.getFactory().createConfigurationController().getDigester();
                 loginRequirementsChecker = new LoginRequirementsChecker(validUser);
                 if (loginRequirementsChecker.isUserLockedOut()) {
                     if (passwordRequirements.getAllowUsernameEnumeration()) {
@@ -323,6 +371,12 @@ public LoginStatus authorizeUser(String username, String plainPassword, String s
                         authorized = digester.matches(plainPassword, credentials.getPassword());
                     }
                 }
+            } else {
+                // Timing equalization (CWE-208): run the same PBKDF2 work for
+                // unknown usernames so attackers cannot distinguish "user does
+                // not exist" from "wrong password" via response-time analysis.
+                // The boolean result is intentionally discarded.
+                digester.matches(plainPassword, DUMMY_HASH);
             }
 
             LoginStatus loginStatus = null;

server/src/com/mirth/connect/server/util/PasswordRequirementsChecker.java

@@ -76,20 +76,24 @@ public static PasswordRequirementsChecker getInstance() {
     }
 
     public PasswordRequirements loadPasswordRequirements(PropertiesConfiguration securityProperties) {
+        // Start with the hardened no-arg defaults from PasswordRequirements
+        // (issue #125 SEC-04). Then read each property using the hardened
+        // value as the fallback default, so absent keys retain the secure
+        // baseline instead of collapsing to zero. See WR-01 in 01-REVIEW.md.
         PasswordRequirements passwordRequirements = new PasswordRequirements();
 
-        passwordRequirements.setMinLength(securityProperties.getInt(PASSWORD_MINLENGTH, 0));
-        passwordRequirements.setMinUpper(securityProperties.getInt(PASSWORD_MIN_UPPER, 0));
-        passwordRequirements.setMinLower(securityProperties.getInt(PASSWORD_MIN_LOWER, 0));
-        passwordRequirements.setMinNumeric(securityProperties.getInt(PASSWORD_MIN_NUMERIC, 0));
-        passwordRequirements.setMinSpecial(securityProperties.getInt(PASSWORD_MIN_SPECIAL, 0));
-        passwordRequirements.setExpiration(securityProperties.getInt(PASSWORD_EXPIRATION, 0));
-        passwordRequirements.setGracePeriod(securityProperties.getInt(PASSWORD_GRACE_PERIOD, 0));
-        passwordRequirements.setRetryLimit(securityProperties.getInt(PASSWORD_RETRY_LIMIT, 0));
-        passwordRequirements.setLockoutPeriod(securityProperties.getInt(PASSWORD_LOCKOUT_PERIOD, 0));
-        passwordRequirements.setReusePeriod(securityProperties.getInt(PASSWORD_REUSE_PERIOD, 0));
-        passwordRequirements.setReuseLimit(securityProperties.getInt(PASSWORD_REUSE_LIMIT, 0));
-        passwordRequirements.setAllowUsernameEnumeration(securityProperties.getBoolean(PASSWORD_ALLOW_USERNAME_ENUMERATION, false));
+        passwordRequirements.setMinLength(securityProperties.getInt(PASSWORD_MINLENGTH, passwordRequirements.getMinLength()));
+        passwordRequirements.setMinUpper(securityProperties.getInt(PASSWORD_MIN_UPPER, passwordRequirements.getMinUpper()));
+        passwordRequirements.setMinLower(securityProperties.getInt(PASSWORD_MIN_LOWER, passwordRequirements.getMinLower()));
+        passwordRequirements.setMinNumeric(securityProperties.getInt(PASSWORD_MIN_NUMERIC, passwordRequirements.getMinNumeric()));
+        passwordRequirements.setMinSpecial(securityProperties.getInt(PASSWORD_MIN_SPECIAL, passwordRequirements.getMinSpecial()));
+        passwordRequirements.setExpiration(securityProperties.getInt(PASSWORD_EXPIRATION, passwordRequirements.getExpiration()));
+        passwordRequirements.setGracePeriod(securityProperties.getInt(PASSWORD_GRACE_PERIOD, passwordRequirements.getGracePeriod()));
+        passwordRequirements.setRetryLimit(securityProperties.getInt(PASSWORD_RETRY_LIMIT, passwordRequirements.getRetryLimit()));
+        passwordRequirements.setLockoutPeriod(securityProperties.getInt(PASSWORD_LOCKOUT_PERIOD, passwordRequirements.getLockoutPeriod()));
+        passwordRequirements.setReusePeriod(securityProperties.getInt(PASSWORD_REUSE_PERIOD, passwordRequirements.getReusePeriod()));
+        passwordRequirements.setReuseLimit(securityProperties.getInt(PASSWORD_REUSE_LIMIT, passwordRequirements.getReuseLimit()));
+        passwordRequirements.setAllowUsernameEnumeration(securityProperties.getBoolean(PASSWORD_ALLOW_USERNAME_ENUMERATION, passwordRequirements.getAllowUsernameEnumeration()));
         return passwordRequirements;
     }