Skip to content

Commit ac162b4

Browse files
mgaffiganmgaffigan
committed
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: #240 Signed-off-by: Mitch Gaffigan <mitch.gaffigan@comcast.net>
1 parent 8e040c7 commit ac162b4

File tree

6 files changed

+44
-17
lines changed

6 files changed

+44
-17
lines changed

‎server/conf/mirth.properties‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ password.expiration = 0
2020
password.graceperiod = 0
2121
password.reuseperiod = 0
2222
password.reuselimit = 0
23+
password.allowusernameenumeration = false
2324

2425
# Only used for migration purposes, do not modify
2526
version = 4.5.2

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ public class PasswordRequirements implements Serializable {
2929
private int gracePeriod;
3030
private int reusePeriod;
3131
private int reuseLimit;
32+
private boolean allowUsernameEnumeration;
3233

3334
public PasswordRequirements() {
3435
this.minLength = 0;
@@ -42,9 +43,18 @@ public PasswordRequirements() {
4243
this.gracePeriod = 0;
4344
this.reusePeriod = 0;
4445
this.reuseLimit = 0;
46+
this.allowUsernameEnumeration = false;
4547
}
4648

49+
/**
50+
* @deprecated Use {@link #PasswordRequirements(int, int, int, int, int, int, int, int, int, int, int, boolean)} instead.
51+
*/
52+
@Deprecated
4753
public PasswordRequirements(int minLength, int minUpper, int minLower, int minNumeric, int minSpecial, int retryLimit, int lockoutPeriod, int expiration, int gracePeriod, int reusePeriod, int reuseLimit) {
54+
this(minLength, minUpper, minLower, minNumeric, minSpecial, retryLimit, lockoutPeriod, expiration, gracePeriod, reusePeriod, reuseLimit, false);
55+
}
56+
57+
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) {
4858
this.minLength = minLength;
4959
this.minUpper = minUpper;
5060
this.minLower = minLower;
@@ -56,6 +66,7 @@ public PasswordRequirements(int minLength, int minUpper, int minLower, int minNu
5666
this.gracePeriod = gracePeriod;
5767
this.reusePeriod = reusePeriod;
5868
this.reuseLimit = reuseLimit;
69+
this.allowUsernameEnumeration = allowUsernameEnumeration;
5970
}
6071

6172
public int getMinLength() {
@@ -145,4 +156,12 @@ public int getReuseLimit() {
145156
public void setReuseLimit(int reuseLimit) {
146157
this.reuseLimit = reuseLimit;
147158
}
159+
160+
public boolean getAllowUsernameEnumeration() {
161+
return allowUsernameEnumeration;
162+
}
163+
164+
public void setAllowUsernameEnumeration(boolean allowUsernameEnumeration) {
165+
this.allowUsernameEnumeration = allowUsernameEnumeration;
166+
}
148167
}

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
public class DefaultUserController extends UserController {
4444
public static final String VACUUM_LOCK_PERSON_STATEMENT_ID = "User.vacuumPersonTable";
4545
public static final String VACUUM_LOCK_PREFERENCES_STATEMENT_ID = "User.vacuumPersonPreferencesTable";
46+
private static final String INCORRECT_CREDENTIALS_MESSAGE = "Incorrect username or password.";
4647

4748
private Logger logger = LogManager.getLogger(this.getClass());
4849
private ExtensionController extensionController = null;
@@ -292,6 +293,7 @@ public LoginStatus authorizeUser(String username, String plainPassword, String s
292293
boolean authorized = false;
293294
Credentials credentials = null;
294295
LoginRequirementsChecker loginRequirementsChecker = null;
296+
PasswordRequirements passwordRequirements = ControllerFactory.getFactory().createConfigurationController().getPasswordRequirements();
295297

296298
// Retrieve the matching User
297299
User validUser = getUser(null, username);
@@ -300,7 +302,11 @@ public LoginStatus authorizeUser(String username, String plainPassword, String s
300302
Digester digester = ControllerFactory.getFactory().createConfigurationController().getDigester();
301303
loginRequirementsChecker = new LoginRequirementsChecker(validUser);
302304
if (loginRequirementsChecker.isUserLockedOut()) {
303-
return new LoginStatus(LoginStatus.Status.FAIL_LOCKED_OUT, "User account \"" + username + "\" has been locked. You may attempt to login again in " + loginRequirementsChecker.getPrintableStrikeTimeRemaining() + ".");
305+
if (passwordRequirements.getAllowUsernameEnumeration()) {
306+
return new LoginStatus(LoginStatus.Status.FAIL_LOCKED_OUT, "User account \"" + username + "\" has been locked. You may attempt to login again in " + loginRequirementsChecker.getPrintableStrikeTimeRemaining() + ".");
307+
} else {
308+
return new LoginStatus(LoginStatus.Status.FAIL, INCORRECT_CREDENTIALS_MESSAGE);
309+
}
304310
}
305311

306312
loginRequirementsChecker.resetExpiredStrikes();
@@ -320,7 +326,6 @@ public LoginStatus authorizeUser(String username, String plainPassword, String s
320326
}
321327
}
322328

323-
PasswordRequirements passwordRequirements = ControllerFactory.getFactory().createConfigurationController().getPasswordRequirements();
324329
LoginStatus loginStatus = null;
325330

326331
if (authorized) {
@@ -383,12 +388,12 @@ public LoginStatus authorizeUser(String username, String plainPassword, String s
383388
}
384389
} else {
385390
LoginStatus.Status status = LoginStatus.Status.FAIL;
386-
String failMessage = "Incorrect username or password.";
391+
String failMessage = INCORRECT_CREDENTIALS_MESSAGE;
387392

388393
if (loginRequirementsChecker != null) {
389394
loginRequirementsChecker.incrementStrikes();
390395

391-
if (loginRequirementsChecker.isLockoutEnabled()) {
396+
if (loginRequirementsChecker.isLockoutEnabled() && passwordRequirements.getAllowUsernameEnumeration()) {
392397
if (loginRequirementsChecker.isUserLockedOut()) {
393398
status = LoginStatus.Status.FAIL_LOCKED_OUT;
394399
failMessage += " User account \"" + username + "\" has been locked. You may attempt to login again in " + loginRequirementsChecker.getPrintableStrikeTimeRemaining() + ".";

‎server/src/com/mirth/connect/server/servlets/SwaggerExamplesServlet.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1165,7 +1165,7 @@ private List<MetaDataColumn> getMetaDataColumnListExample() {
11651165
}
11661166

11671167
private PasswordRequirements getPasswordRequirementsExample() {
1168-
return new PasswordRequirements(8, 1, 1, 1, 1, 3, 0, 0, 0, 0, 3);
1168+
return new PasswordRequirements(8, 1, 1, 1, 1, 3, 0, 0, 0, 0, 3, false);
11691169
}
11701170

11711171
private List<String> getPasswordRequirementListExample() {

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ public class PasswordRequirementsChecker implements Serializable {
5757
private static final String PASSWORD_LOCKOUT_PERIOD = "password.lockoutperiod";
5858
private static final String PASSWORD_REUSE_PERIOD = "password.reuseperiod";
5959
private static final String PASSWORD_REUSE_LIMIT = "password.reuselimit";
60+
private static final String PASSWORD_ALLOW_USERNAME_ENUMERATION = "password.allowusernameenumeration";
6061

6162
private static PasswordRequirementsChecker instance = null;
6263

@@ -88,6 +89,7 @@ public PasswordRequirements loadPasswordRequirements(PropertiesConfiguration sec
8889
passwordRequirements.setLockoutPeriod(securityProperties.getInt(PASSWORD_LOCKOUT_PERIOD, 0));
8990
passwordRequirements.setReusePeriod(securityProperties.getInt(PASSWORD_REUSE_PERIOD, 0));
9091
passwordRequirements.setReuseLimit(securityProperties.getInt(PASSWORD_REUSE_LIMIT, 0));
92+
passwordRequirements.setAllowUsernameEnumeration(securityProperties.getBoolean(PASSWORD_ALLOW_USERNAME_ENUMERATION, false));
9193

9294
return passwordRequirements;
9395
}

‎server/test/com/mirth/connect/server/util/PasswordRequirementsTests.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -25,57 +25,57 @@ protected void tearDown() throws Exception {
2525
}
2626

2727
public void testMinLength() throws ControllerException {
28-
PasswordRequirements req = new PasswordRequirements(10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0);
28+
PasswordRequirements req = new PasswordRequirements(10, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, false);
2929
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "test", req));
3030
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "testtesttest", req));
3131

3232
}
3333

3434
public void testMinUpper() throws ControllerException {
35-
PasswordRequirements req = new PasswordRequirements(0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0);
35+
PasswordRequirements req = new PasswordRequirements(0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 0, false);
3636
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "test", req));
3737
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "aTest", req));
3838

39-
PasswordRequirements req2 = new PasswordRequirements(0, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0);
39+
PasswordRequirements req2 = new PasswordRequirements(0, -1, 0, 0, 0, 0, 0, 0, 0, 0, 0, false);
4040
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "test", req2));
4141
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TESt", req2));
4242
}
4343

4444
public void testMinLower() throws ControllerException {
45-
PasswordRequirements req = new PasswordRequirements(0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0);
45+
PasswordRequirements req = new PasswordRequirements(0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, false);
4646
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req));
4747
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TESt", req));
4848

49-
PasswordRequirements req2 = new PasswordRequirements(0, 0, -1, 0, 0, 0, 0, 0, 0, 0, 0);
49+
PasswordRequirements req2 = new PasswordRequirements(0, 0, -1, 0, 0, 0, 0, 0, 0, 0, 0, false);
5050
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req2));
5151
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TESt", req2));
5252

53-
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "testBLAH", new PasswordRequirements(0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0)));
54-
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "testtBLAH", new PasswordRequirements(0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0)));
53+
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "testBLAH", new PasswordRequirements(0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0, false)));
54+
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "testtBLAH", new PasswordRequirements(0, 0, 5, 0, 0, 0, 0, 0, 0, 0, 0, false)));
5555
}
5656

5757
public void testMinNumeric() throws ControllerException {
58-
PasswordRequirements req = new PasswordRequirements(0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0);
58+
PasswordRequirements req = new PasswordRequirements(0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, false);
5959
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req));
6060
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST9", req));
6161

62-
PasswordRequirements req2 = new PasswordRequirements(0, 0, 0, -1, 0, 0, 0, 0, 0, 0, 0);
62+
PasswordRequirements req2 = new PasswordRequirements(0, 0, 0, -1, 0, 0, 0, 0, 0, 0, 0, false);
6363
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req2));
6464
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TESt9", req2));
6565
}
6666

6767
public void testMinSpecial() throws ControllerException {
68-
PasswordRequirements req = new PasswordRequirements(0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0);
68+
PasswordRequirements req = new PasswordRequirements(0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, false);
6969
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req));
7070
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST$TEST", req));
7171

72-
PasswordRequirements req2 = new PasswordRequirements(0, 0, 0, 0, -1, 0, 0, 0, 0, 0, 0);
72+
PasswordRequirements req2 = new PasswordRequirements(0, 0, 0, 0, -1, 0, 0, 0, 0, 0, 0, false);
7373
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TEST", req2));
7474
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "TESt$", req2));
7575
}
7676

7777
public void testAllConditions() throws ControllerException {
78-
PasswordRequirements req = new PasswordRequirements(15, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0);
78+
PasswordRequirements req = new PasswordRequirements(15, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, false);
7979
assertNotNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "test", req));
8080
assertNull(PasswordRequirementsChecker.getInstance().doesPasswordMeetRequirements(null, "Th1$isAtestTEST*#", req));
8181
}

0 commit comments

Comments
 (0)