From ac162b44c42fab81a975ee5049b4d0f96fb4ac6a Mon Sep 17 00:00:00 2001 From: Mitch Gaffigan Date: Sun, 18 Jan 2026 20:40:20 -0600 Subject: [PATCH] Fix information disclosure when lockout enabled When lockout is enabled, enhanced response error messages allow an attacker to enumerate valid usernames. Change adds new configuration option to disable username enumeration by returning the same error message for both invalid usernames and locked out accounts. The option is enabled by default. Issue: https://github.com/OpenIntegrationEngine/engine/issues/240 Signed-off-by: Mitch Gaffigan --- server/conf/mirth.properties | 1 + .../connect/model/PasswordRequirements.java | 19 +++++++++++++++ .../controllers/DefaultUserController.java | 13 ++++++---- .../servlets/SwaggerExamplesServlet.java | 2 +- .../util/PasswordRequirementsChecker.java | 2 ++ .../util/PasswordRequirementsTests.java | 24 +++++++++---------- 6 files changed, 44 insertions(+), 17 deletions(-) diff --git a/server/conf/mirth.properties b/server/conf/mirth.properties index e3c9719f1f..3f0749a829 100644 --- a/server/conf/mirth.properties +++ b/server/conf/mirth.properties @@ -20,6 +20,7 @@ password.expiration = 0 password.graceperiod = 0 password.reuseperiod = 0 password.reuselimit = 0 +password.allowusernameenumeration = false # Only used for migration purposes, do not modify version = 4.5.2 diff --git a/server/src/com/mirth/connect/model/PasswordRequirements.java b/server/src/com/mirth/connect/model/PasswordRequirements.java index c4276ff578..02fac11f8e 100644 --- a/server/src/com/mirth/connect/model/PasswordRequirements.java +++ b/server/src/com/mirth/connect/model/PasswordRequirements.java @@ -29,6 +29,7 @@ public class PasswordRequirements implements Serializable { private int gracePeriod; private int reusePeriod; private int reuseLimit; + private boolean allowUsernameEnumeration; public PasswordRequirements() { this.minLength = 0; @@ -42,9 +43,18 @@ public PasswordRequirements() { this.gracePeriod = 0; this.reusePeriod = 0; this.reuseLimit = 0; + this.allowUsernameEnumeration = false; } + /** + * @deprecated Use {@link #PasswordRequirements(int, int, int, int, int, int, int, int, int, int, int, boolean)} instead. + */ + @Deprecated public PasswordRequirements(int minLength, int minUpper, int minLower, int minNumeric, int minSpecial, int retryLimit, int lockoutPeriod, int expiration, int gracePeriod, int reusePeriod, int reuseLimit) { + this(minLength, minUpper, minLower, minNumeric, minSpecial, retryLimit, lockoutPeriod, expiration, gracePeriod, reusePeriod, reuseLimit, false); + } + + public PasswordRequirements(int minLength, int minUpper, int minLower, int minNumeric, int minSpecial, int retryLimit, int lockoutPeriod, int expiration, int gracePeriod, int reusePeriod, int reuseLimit, boolean allowUsernameEnumeration) { this.minLength = minLength; this.minUpper = minUpper; this.minLower = minLower; @@ -56,6 +66,7 @@ public PasswordRequirements(int minLength, int minUpper, int minLower, int minNu this.gracePeriod = gracePeriod; this.reusePeriod = reusePeriod; this.reuseLimit = reuseLimit; + this.allowUsernameEnumeration = allowUsernameEnumeration; } public int getMinLength() { @@ -145,4 +156,12 @@ public int getReuseLimit() { public void setReuseLimit(int reuseLimit) { this.reuseLimit = reuseLimit; } + + public boolean getAllowUsernameEnumeration() { + return allowUsernameEnumeration; + } + + public void setAllowUsernameEnumeration(boolean allowUsernameEnumeration) { + this.allowUsernameEnumeration = allowUsernameEnumeration; + } } diff --git a/server/src/com/mirth/connect/server/controllers/DefaultUserController.java b/server/src/com/mirth/connect/server/controllers/DefaultUserController.java index 26fef00e39..e96513118e 100644 --- a/server/src/com/mirth/connect/server/controllers/DefaultUserController.java +++ b/server/src/com/mirth/connect/server/controllers/DefaultUserController.java @@ -43,6 +43,7 @@ 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."; private Logger logger = LogManager.getLogger(this.getClass()); private ExtensionController extensionController = null; @@ -292,6 +293,7 @@ public LoginStatus authorizeUser(String username, String plainPassword, String s boolean authorized = false; Credentials credentials = null; LoginRequirementsChecker loginRequirementsChecker = null; + PasswordRequirements passwordRequirements = ControllerFactory.getFactory().createConfigurationController().getPasswordRequirements(); // Retrieve the matching User User validUser = getUser(null, username); @@ -300,7 +302,11 @@ public LoginStatus authorizeUser(String username, String plainPassword, String s Digester digester = ControllerFactory.getFactory().createConfigurationController().getDigester(); loginRequirementsChecker = new LoginRequirementsChecker(validUser); if (loginRequirementsChecker.isUserLockedOut()) { - return new LoginStatus(LoginStatus.Status.FAIL_LOCKED_OUT, "User account \"" + username + "\" has been locked. You may attempt to login again in " + loginRequirementsChecker.getPrintableStrikeTimeRemaining() + "."); + if (passwordRequirements.getAllowUsernameEnumeration()) { + return new LoginStatus(LoginStatus.Status.FAIL_LOCKED_OUT, "User account \"" + username + "\" has been locked. You may attempt to login again in " + loginRequirementsChecker.getPrintableStrikeTimeRemaining() + "."); + } else { + return new LoginStatus(LoginStatus.Status.FAIL, INCORRECT_CREDENTIALS_MESSAGE); + } } loginRequirementsChecker.resetExpiredStrikes(); @@ -320,7 +326,6 @@ public LoginStatus authorizeUser(String username, String plainPassword, String s } } - PasswordRequirements passwordRequirements = ControllerFactory.getFactory().createConfigurationController().getPasswordRequirements(); LoginStatus loginStatus = null; if (authorized) { @@ -383,12 +388,12 @@ public LoginStatus authorizeUser(String username, String plainPassword, String s } } else { LoginStatus.Status status = LoginStatus.Status.FAIL; - String failMessage = "Incorrect username or password."; + String failMessage = INCORRECT_CREDENTIALS_MESSAGE; if (loginRequirementsChecker != null) { loginRequirementsChecker.incrementStrikes(); - if (loginRequirementsChecker.isLockoutEnabled()) { + if (loginRequirementsChecker.isLockoutEnabled() && passwordRequirements.getAllowUsernameEnumeration()) { if (loginRequirementsChecker.isUserLockedOut()) { status = LoginStatus.Status.FAIL_LOCKED_OUT; failMessage += " User account \"" + username + "\" has been locked. You may attempt to login again in " + loginRequirementsChecker.getPrintableStrikeTimeRemaining() + "."; diff --git a/server/src/com/mirth/connect/server/servlets/SwaggerExamplesServlet.java b/server/src/com/mirth/connect/server/servlets/SwaggerExamplesServlet.java index 476775c82b..e70bf8dab3 100644 --- a/server/src/com/mirth/connect/server/servlets/SwaggerExamplesServlet.java +++ b/server/src/com/mirth/connect/server/servlets/SwaggerExamplesServlet.java @@ -1165,7 +1165,7 @@ private List getMetaDataColumnListExample() { } private PasswordRequirements getPasswordRequirementsExample() { - return new PasswordRequirements(8, 1, 1, 1, 1, 3, 0, 0, 0, 0, 3); + return new PasswordRequirements(8, 1, 1, 1, 1, 3, 0, 0, 0, 0, 3, false); } private List getPasswordRequirementListExample() { diff --git a/server/src/com/mirth/connect/server/util/PasswordRequirementsChecker.java b/server/src/com/mirth/connect/server/util/PasswordRequirementsChecker.java index a099588e24..609de4a70a 100644 --- a/server/src/com/mirth/connect/server/util/PasswordRequirementsChecker.java +++ b/server/src/com/mirth/connect/server/util/PasswordRequirementsChecker.java @@ -57,6 +57,7 @@ public class PasswordRequirementsChecker implements Serializable { private static final String PASSWORD_LOCKOUT_PERIOD = "password.lockoutperiod"; private static final String PASSWORD_REUSE_PERIOD = "password.reuseperiod"; private static final String PASSWORD_REUSE_LIMIT = "password.reuselimit"; + private static final String PASSWORD_ALLOW_USERNAME_ENUMERATION = "password.allowusernameenumeration"; private static PasswordRequirementsChecker instance = null; @@ -88,6 +89,7 @@ public PasswordRequirements loadPasswordRequirements(PropertiesConfiguration sec 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)); return passwordRequirements; } diff --git a/server/test/com/mirth/connect/server/util/PasswordRequirementsTests.java b/server/test/com/mirth/connect/server/util/PasswordRequirementsTests.java index e21b4a683e..11d6d4f617 100644 --- a/server/test/com/mirth/connect/server/util/PasswordRequirementsTests.java +++ b/server/test/com/mirth/connect/server/util/PasswordRequirementsTests.java @@ -25,57 +25,57 @@ protected void tearDown() throws Exception { } public void testMinLength() throws ControllerException { - PasswordRequirements req = new PasswordRequirements(10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0); + PasswordRequirements req = new PasswordRequirements(10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, false); assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "test", req)); assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "testtesttest", req)); } public void testMinUpper() throws ControllerException { - PasswordRequirements req = new PasswordRequirements(0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0); + PasswordRequirements req = new PasswordRequirements(0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, false); assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "test", req)); assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "aTest", req)); - PasswordRequirements req2 = new PasswordRequirements(0, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0); + PasswordRequirements req2 = new PasswordRequirements(0, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, false); assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "test", req2)); assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TESt", req2)); } public void testMinLower() throws ControllerException { - PasswordRequirements req = new PasswordRequirements(0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0); + PasswordRequirements req = new PasswordRequirements(0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, false); assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req)); assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TESt", req)); - PasswordRequirements req2 = new PasswordRequirements(0, 0, -1, 0, 0, 0, 0, 0, 0, 0, 0); + PasswordRequirements req2 = new PasswordRequirements(0, 0, -1, 0, 0, 0, 0, 0, 0, 0, 0, false); assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req2)); assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TESt", req2)); - assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "testBLAH", new PasswordRequirements(0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0))); - assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "testtBLAH", new PasswordRequirements(0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0))); + assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "testBLAH", new PasswordRequirements(0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0, false))); + assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "testtBLAH", new PasswordRequirements(0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0, false))); } public void testMinNumeric() throws ControllerException { - PasswordRequirements req = new PasswordRequirements(0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0); + PasswordRequirements req = new PasswordRequirements(0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, false); assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req)); assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST9", req)); - PasswordRequirements req2 = new PasswordRequirements(0, 0, 0, -1, 0, 0, 0, 0, 0, 0, 0); + PasswordRequirements req2 = new PasswordRequirements(0, 0, 0, -1, 0, 0, 0, 0, 0, 0, 0, false); assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req2)); assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TESt9", req2)); } public void testMinSpecial() throws ControllerException { - PasswordRequirements req = new PasswordRequirements(0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0); + PasswordRequirements req = new PasswordRequirements(0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, false); assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req)); assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST$TEST", req)); - PasswordRequirements req2 = new PasswordRequirements(0, 0, 0, 0, -1, 0, 0, 0, 0, 0, 0); + PasswordRequirements req2 = new PasswordRequirements(0, 0, 0, 0, -1, 0, 0, 0, 0, 0, 0, false); assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req2)); assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TESt$", req2)); } public void testAllConditions() throws ControllerException { - PasswordRequirements req = new PasswordRequirements(15, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0); + PasswordRequirements req = new PasswordRequirements(15, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, false); assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "test", req)); assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "Th1$isAtestTEST*#", req)); }