This is an automated email from the ASF dual-hosted git repository.
suneet pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 58f2e51 Do not echo back username on auth failure (#10097)
58f2e51 is described below
commit 58f2e5116126fee868b39e268341db0e60c8425f
Author: Suneet Saldanha <[email protected]>
AuthorDate: Fri Jul 10 12:19:10 2020 -0700
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
---
.../authentication/BasicHTTPAuthenticator.java | 5 +---
.../validator/LDAPCredentialsValidator.java | 2 +-
.../MetadataStoreCredentialsValidator.java | 2 +-
.../authentication/BasicHTTPAuthenticatorTest.java | 6 ++--
.../validator/DBCredentialsValidatorTest.java | 2 +-
.../security/ITBasicAuthConfigurationTest.java | 33 ++++++++++++++++++----
6 files changed, 34 insertions(+), 16 deletions(-)
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.HttpServletRequest;
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 @@ public class BasicHTTPAuthenticator implements Authenticator
}
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 @@ public class LDAPCredentialsValidator implements
CredentialsValidator
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 @@ public class MetadataStoreCredentialsValidator implements
CredentialsValidator
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 @@ public class BasicHTTPAuthenticatorTest
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 @@ public class BasicHTTPAuthenticatorTest
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 @@ public class BasicHTTPAuthenticatorTest
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 @@ public class DBCredentialsValidatorTest
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 @@ public class ITBasicAuthConfigurationTest
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 @@ public class ITBasicAuthConfigurationTest
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 @@ public class ITBasicAuthConfigurationTest
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 @@ public class ITBasicAuthConfigurationTest
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 @@ public class ITBasicAuthConfigurationTest
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;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
