Do not echo back username on auth failure (#10097)
* Do not echo back username on auth failure
* use bad username
* Remove username from exception messages
* fix tests
* fix the tests
* hopefully this time
* this time the tests work
* fixed this time
* fix
* upgrade to Jetty 9.4.30
* Unknown users echo back Unauthorized
* fix
diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/BasicHTTPAuthenticator.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/BasicHTTPAuthenticator.java
index ee2a2ef..600af93 100644
--- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/BasicHTTPAuthenticator.java
+++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/BasicHTTPAuthenticator.java
@@ -48,7 +48,6 @@
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
import java.util.EnumSet;
-import java.util.Locale;
import java.util.Map;
@JsonTypeName("basic")
@@ -218,9 +217,7 @@
}
catch (BasicSecurityAuthenticationException ex) {
LOG.info("Exception authenticating user %s - %s", user, ex.getMessage());
- httpResp.sendError(HttpServletResponse.SC_UNAUTHORIZED,
- String.format(Locale.getDefault(),
- "User authentication failed username[%s].", user));
+ httpResp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User authentication failed.");
}
}
diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java
index 7a9bc0c..1db8799 100644
--- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java
+++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java
@@ -165,7 +165,7 @@
if (!validatePassword(this.ldapConfig, userDn, password)) {
LOG.debug("Password incorrect for LDAP user %s", username);
- throw new BasicSecurityAuthenticationException("User LDAP authentication failed username[%s].", userDn.toString());
+ throw new BasicSecurityAuthenticationException("User LDAP authentication failed.");
}
byte[] salt = BasicAuthUtils.generateSalt();
diff --git a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/MetadataStoreCredentialsValidator.java b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/MetadataStoreCredentialsValidator.java
index 9f86f0d..7d75d0c 100644
--- a/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/MetadataStoreCredentialsValidator.java
+++ b/extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/MetadataStoreCredentialsValidator.java
@@ -83,7 +83,7 @@
return new AuthenticationResult(username, authorizerName, authenticatorName, null);
} else {
LOG.debug("Password incorrect for metadata store user %s", username);
- throw new BasicSecurityAuthenticationException("User metadata store authentication failed username[%s].", username);
+ throw new BasicSecurityAuthenticationException("User metadata store authentication failed.");
}
}
}
diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/BasicHTTPAuthenticatorTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/BasicHTTPAuthenticatorTest.java
index 52c6356..84bfdcf 100644
--- a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/BasicHTTPAuthenticatorTest.java
+++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/BasicHTTPAuthenticatorTest.java
@@ -174,7 +174,7 @@
EasyMock.replay(req);
HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
- resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User authentication failed username[userA].");
+ resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User authentication failed.");
EasyMock.expectLastCall().times(1);
EasyMock.replay(resp);
@@ -210,7 +210,7 @@
validator.validateCredentials(EasyMock.eq("basic"), EasyMock.eq("basic"), EasyMock.eq("userA"), EasyMock.aryEq("badpassword".toCharArray()))
)
.andThrow(
- new BasicSecurityAuthenticationException("User authentication failed username[%s].", "userA")
+ new BasicSecurityAuthenticationException("User authentication failed.")
)
.times(1);
EasyMock.replay(validator);
@@ -220,7 +220,7 @@
EasyMock.replay(req);
HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
- resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User authentication failed username[userA].");
+ resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User authentication failed.");
EasyMock.expectLastCall().times(1);
EasyMock.replay(resp);
diff --git a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/validator/DBCredentialsValidatorTest.java b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/validator/DBCredentialsValidatorTest.java
index b8e4942..a981b52 100644
--- a/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/validator/DBCredentialsValidatorTest.java
+++ b/extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/validator/DBCredentialsValidatorTest.java
@@ -143,7 +143,7 @@
String password = "badpassword";
expectedException.expect(BasicSecurityAuthenticationException.class);
- expectedException.expectMessage("User metadata store authentication failed username[userA].");
+ expectedException.expectMessage("User metadata store authentication failed.");
validator.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray());
}
}
diff --git a/integration-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthConfigurationTest.java b/integration-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthConfigurationTest.java
index d276acb..2a8506f 100644
--- a/integration-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthConfigurationTest.java
+++ b/integration-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthConfigurationTest.java
@@ -291,8 +291,8 @@
datasourceOnlyUserClient,
SYS_SCHEMA_TASKS_QUERY,
adminTasks.stream()
- .filter((taskEntry) -> "auth_test".equals(taskEntry.get("datasource")))
- .collect(Collectors.toList())
+ .filter((taskEntry) -> "auth_test".equals(taskEntry.get("datasource")))
+ .collect(Collectors.toList())
);
// as user that can read auth_test and STATE
@@ -317,7 +317,8 @@
datasourceWithStateUserClient,
SYS_SCHEMA_SERVER_SEGMENTS_QUERY,
adminServerSegments.stream()
- .filter((serverSegmentEntry) -> ((String) serverSegmentEntry.get("segment_id")).contains("auth_test"))
+ .filter((serverSegmentEntry) -> ((String) serverSegmentEntry.get("segment_id")).contains(
+ "auth_test"))
.collect(Collectors.toList())
);
@@ -326,8 +327,8 @@
datasourceWithStateUserClient,
SYS_SCHEMA_TASKS_QUERY,
adminTasks.stream()
- .filter((taskEntry) -> "auth_test".equals(taskEntry.get("datasource")))
- .collect(Collectors.toList())
+ .filter((taskEntry) -> "auth_test".equals(taskEntry.get("datasource")))
+ .collect(Collectors.toList())
);
// as user that can only read STATE
@@ -471,6 +472,26 @@
testOptionsRequests(adminClient);
}
+ @Test
+ public void testMaliciousUser()
+ {
+ String maliciousUsername = "<script>alert('hello')</script>";
+ HttpClient maliciousClient = new CredentialedHttpClient(
+ new BasicCredentials(maliciousUsername, "noPass"),
+ httpClient
+ );
+ StatusResponseHolder responseHolder = HttpUtil.makeRequestWithExpectedStatus(
+ maliciousClient,
+ HttpMethod.GET,
+ config.getBrokerUrl() + "/status",
+ null,
+ HttpResponseStatus.UNAUTHORIZED
+ );
+ String responseContent = responseHolder.getContent();
+ Assert.assertTrue(responseContent.contains("<tr><th>MESSAGE:</th><td>Unauthorized</td></tr>"));
+ Assert.assertFalse(responseContent.contains(maliciousUsername));
+ }
+
private void testOptionsRequests(HttpClient httpClient)
{
HttpUtil.makeRequest(httpClient, HttpMethod.OPTIONS, config.getCoordinatorUrl() + "/status", null);
@@ -522,7 +543,7 @@
catch (AvaticaSqlException ase) {
Assert.assertEquals(
ase.getErrorMessage(),
- "Error while executing SQL \"SELECT * FROM INFORMATION_SCHEMA.COLUMNS\": Remote driver error: BasicSecurityAuthenticationException: User metadata store authentication failed username[admin]."
+ "Error while executing SQL \"SELECT * FROM INFORMATION_SCHEMA.COLUMNS\": Remote driver error: BasicSecurityAuthenticationException: User metadata store authentication failed."
);
return;
}
