diff --git a/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionTests.java b/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionTests.java index bb16e0be1b..34e79613f6 100644 --- a/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionTests.java +++ b/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionTests.java @@ -12,7 +12,6 @@ import java.util.concurrent.TimeUnit; import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; -import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -36,8 +35,8 @@ @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) @ThreadLeakScope(ThreadLeakScope.Scope.NONE) public class IpBruteForceAttacksPreventionTests { - private static final User USER_1 = new User("simple-user-1").roles(ALL_ACCESS); - private static final User USER_2 = new User("simple-user-2").roles(ALL_ACCESS); + static final User USER_1 = new User("simple-user-1").roles(ALL_ACCESS); + static final User USER_2 = new User("simple-user-2").roles(ALL_ACCESS); public static final int ALLOWED_TRIES = 3; public static final int TIME_WINDOW_SECONDS = 3; @@ -51,7 +50,7 @@ public class IpBruteForceAttacksPreventionTests { public static final String CLIENT_IP_8 = "127.0.0.8"; public static final String CLIENT_IP_9 = "127.0.0.9"; - private static final AuthFailureListeners listener = new AuthFailureListeners().addRateLimit( + static final AuthFailureListeners listener = new AuthFailureListeners().addRateLimit( new RateLimiting("internal_authentication_backend_limiting").type("ip") .allowedTries(ALLOWED_TRIES) .timeWindowSeconds(TIME_WINDOW_SECONDS) @@ -60,13 +59,17 @@ public class IpBruteForceAttacksPreventionTests { .maxTrackedClients(500) ); - @ClassRule - public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) - .anonymousAuth(false) - .authFailureListeners(listener) - .authc(AUTHC_HTTPBASIC_INTERNAL_WITHOUT_CHALLENGE) - .users(USER_1, USER_2) - .build(); + @Rule + public LocalCluster cluster = createCluster(); + + public LocalCluster createCluster() { + return new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) + .anonymousAuth(false) + .authFailureListeners(listener) + .authc(AUTHC_HTTPBASIC_INTERNAL_WITHOUT_CHALLENGE) + .users(USER_1, USER_2) + .build(); + } @Rule public LogsRule logsRule = new LogsRule("org.opensearch.security.auth.BackendRegistry"); @@ -151,7 +154,7 @@ public void shouldReleaseIpAddressLock() throws InterruptedException { } } - private static void authenticateUserWithIncorrectPassword(String sourceIpAddress, User user, int numberOfRequests) { + void authenticateUserWithIncorrectPassword(String sourceIpAddress, User user, int numberOfRequests) { var clientConfiguration = new TestRestClientConfiguration().username(user.getName()) .password("incorrect password") .sourceInetAddress(sourceIpAddress); diff --git a/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionWithDomainChallengeTests.java b/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionWithDomainChallengeTests.java new file mode 100644 index 0000000000..6159599119 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/IpBruteForceAttacksPreventionWithDomainChallengeTests.java @@ -0,0 +1,32 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + */ + +package org.opensearch.security; + +import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; +import org.junit.runner.RunWith; +import org.opensearch.test.framework.cluster.ClusterManager; +import org.opensearch.test.framework.cluster.LocalCluster; + +import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; + +@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) +@ThreadLeakScope(ThreadLeakScope.Scope.NONE) +public class IpBruteForceAttacksPreventionWithDomainChallengeTests extends IpBruteForceAttacksPreventionTests { + @Override + public LocalCluster createCluster() { + return new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE) + .anonymousAuth(false) + .authFailureListeners(listener) + .authc(AUTHC_HTTPBASIC_INTERNAL) + .users(USER_1, USER_2) + .build(); + } +} diff --git a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java index c09127e592..77890a4645 100644 --- a/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java +++ b/src/integrationTest/java/org/opensearch/test/framework/cluster/LocalOpenSearchCluster.java @@ -344,7 +344,7 @@ public String toString() { String clusterManagerNodes = nodeByTypeToString(CLUSTER_MANAGER); String dataNodes = nodeByTypeToString(DATA); String clientNodes = nodeByTypeToString(CLIENT); - return "\nES Cluster " + return "\nOS Cluster " + clusterName + "\ncluster manager nodes: " + clusterManagerNodes diff --git a/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java b/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java index 3fee4a9444..7210ed5950 100644 --- a/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java +++ b/src/main/java/com/amazon/dlic/auth/http/saml/AuthTokenProcessorHandler.java @@ -23,7 +23,6 @@ import java.util.regex.Pattern; import java.util.stream.Collectors; -import javax.xml.parsers.ParserConfigurationException; import javax.xml.xpath.XPathExpressionException; import com.fasterxml.jackson.core.JsonParseException; @@ -32,7 +31,6 @@ import com.fasterxml.jackson.databind.node.ObjectNode; import com.google.common.base.Strings; import com.onelogin.saml2.authn.SamlResponse; -import com.onelogin.saml2.exception.SettingsException; import com.onelogin.saml2.exception.ValidationError; import com.onelogin.saml2.settings.Saml2Settings; import com.onelogin.saml2.util.Util; @@ -49,7 +47,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.joda.time.DateTime; -import org.xml.sax.SAXException; import org.opensearch.OpenSearchSecurityException; import org.opensearch.SpecialPermission; @@ -57,7 +54,6 @@ import org.opensearch.common.settings.Settings; import org.opensearch.common.xcontent.XContentType; import org.opensearch.rest.BytesRestResponse; -import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; import org.opensearch.core.rest.RestStatus; @@ -122,7 +118,7 @@ class AuthTokenProcessorHandler { } @SuppressWarnings("removal") - boolean handle(RestRequest restRequest, RestChannel restChannel) throws Exception { + BytesRestResponse handle(RestRequest restRequest) throws Exception { try { final SecurityManager sm = System.getSecurityManager(); @@ -130,11 +126,10 @@ boolean handle(RestRequest restRequest, RestChannel restChannel) throws Exceptio sm.checkPermission(new SpecialPermission()); } - return AccessController.doPrivileged(new PrivilegedExceptionAction() { + return AccessController.doPrivileged(new PrivilegedExceptionAction() { @Override - public Boolean run() throws XPathExpressionException, SamlConfigException, IOException, ParserConfigurationException, - SAXException, SettingsException { - return handleLowLevel(restRequest, restChannel); + public BytesRestResponse run() throws SamlConfigException, IOException { + return handleLowLevel(restRequest); } }); } catch (PrivilegedActionException e) { @@ -147,13 +142,11 @@ public Boolean run() throws XPathExpressionException, SamlConfigException, IOExc } private AuthTokenProcessorAction.Response handleImpl( - RestRequest restRequest, - RestChannel restChannel, String samlResponseBase64, String samlRequestId, String acsEndpoint, Saml2Settings saml2Settings - ) throws XPathExpressionException, ParserConfigurationException, SAXException, IOException, SettingsException { + ) { if (token_log.isDebugEnabled()) { try { token_log.debug( @@ -188,8 +181,7 @@ private AuthTokenProcessorAction.Response handleImpl( } } - private boolean handleLowLevel(RestRequest restRequest, RestChannel restChannel) throws SamlConfigException, IOException, - XPathExpressionException, ParserConfigurationException, SAXException, SettingsException { + private BytesRestResponse handleLowLevel(RestRequest restRequest) throws SamlConfigException, IOException { try { if (restRequest.getMediaType() != XContentType.JSON) { @@ -234,31 +226,18 @@ private boolean handleLowLevel(RestRequest restRequest, RestChannel restChannel) acsEndpoint = getAbsoluteAcsEndpoint(((ObjectNode) jsonRoot).get("acsEndpoint").textValue()); } - AuthTokenProcessorAction.Response responseBody = this.handleImpl( - restRequest, - restChannel, - samlResponseBase64, - samlRequestId, - acsEndpoint, - saml2Settings - ); + AuthTokenProcessorAction.Response responseBody = this.handleImpl(samlResponseBase64, samlRequestId, acsEndpoint, saml2Settings); if (responseBody == null) { - return false; + return null; } String responseBodyString = DefaultObjectMapper.objectMapper.writeValueAsString(responseBody); - BytesRestResponse authenticateResponse = new BytesRestResponse(RestStatus.OK, "application/json", responseBodyString); - restChannel.sendResponse(authenticateResponse); - - return true; + return new BytesRestResponse(RestStatus.OK, "application/json", responseBodyString); } catch (JsonProcessingException e) { log.warn("Error while parsing JSON for /_opendistro/_security/api/authtoken", e); - - BytesRestResponse authenticateResponse = new BytesRestResponse(RestStatus.BAD_REQUEST, "JSON could not be parsed"); - restChannel.sendResponse(authenticateResponse); - return true; + return new BytesRestResponse(RestStatus.BAD_REQUEST, "JSON could not be parsed"); } } diff --git a/src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java b/src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java index 707a3c791f..8e7f3aaa13 100644 --- a/src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java +++ b/src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java @@ -204,7 +204,7 @@ public boolean reRequestAuthentication(final SecurityRequestChannel request, fin ); } catch (Exception e) { log.error("Error in reRequestAuthentication()", e); - return false; + return null; } } diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 7446149a37..7b0c2e2faa 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -232,7 +232,7 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte User authenticatedUser = null; - AuthCredentials authCredenetials = null; + AuthCredentials authCredentials = null; HTTPAuthenticator firstChallengingHttpAuthenticator = null; @@ -274,7 +274,7 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte continue; } - authCredenetials = ac; + authCredentials = ac; if (ac == null) { // no credentials found in request @@ -297,6 +297,7 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte } else { org.apache.logging.log4j.ThreadContext.put("user", ac.getUsername()); if (!ac.isComplete()) { + final BytesRestResponse restResponse = httpAuthenticator.reRequestAuthentication(request, ac); // credentials found in request but we need another client challenge if (httpAuthenticator.reRequestAuthentication(request, ac)) { // auditLog.logFailedLogin(ac.getUsername()+" ", request); --noauditlog @@ -373,7 +374,7 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte log.debug("User still not authenticated after checking {} auth domains", restAuthDomains.size()); } - if (authCredenetials == null && anonymousAuthEnabled) { + if (authCredentials == null && anonymousAuthEnabled) { final String tenant = Utils.coalesce(request.header("securitytenant"), request.header("security_tenant")); User anonymousUser = new User(User.ANONYMOUS.getName(), new HashSet(User.ANONYMOUS.getRoles()), null); anonymousUser.setRequestedTenant(tenant); @@ -385,6 +386,7 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte } return; } + BytesRestResponse challengeResponse = null; if (firstChallengingHttpAuthenticator != null) { @@ -409,12 +411,12 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte log.warn( "Authentication finally failed for {} from {}", - authCredenetials == null ? null : authCredenetials.getUsername(), + authCredentials == null ? null : authCredentials.getUsername(), remoteAddress ); - auditLog.logFailedLogin(authCredenetials == null ? null : authCredenetials.getUsername(), false, null, request); + auditLog.logFailedLogin(authCredentials == null ? null : authCredentials.getUsername(), false, null, request); - notifyIpAuthFailureListeners(request, authCredenetials); + notifyIpAuthFailureListeners(request, authCredentials); request.completeWithResponse(org.apache.http.HttpStatus.SC_UNAUTHORIZED, null, "Authentication finally failed"); } diff --git a/src/main/java/org/opensearch/security/auth/HTTPAuthenticator.java b/src/main/java/org/opensearch/security/auth/HTTPAuthenticator.java index ad1742b17f..4582d38651 100644 --- a/src/main/java/org/opensearch/security/auth/HTTPAuthenticator.java +++ b/src/main/java/org/opensearch/security/auth/HTTPAuthenticator.java @@ -72,15 +72,14 @@ AuthCredentials extractCredentials(final SecurityRequestChannel request, final T /** * If the {@code extractCredentials()} call was not successful or the authentication flow needs another roundtrip this method - * will be called. If the custom HTTP authenticator does not support this method is a no-op and false should be returned. - * + * will be called. If the custom HTTP authenticator does not support this method is a no-op and null response should be returned. * If the custom HTTP authenticator does support re-request authentication or supports authentication flows with multiple roundtrips - * then the response should be sent (through the channel) and true must be returned. + * then the response will be returned which can then be sent via response channel. * * @param channel The channel to sent back the response * @param credentials The credentials from the prior authentication attempt - * @return false if re-request is not supported/necessary, true otherwise. - * If true is returned {@code channel.sendResponse()} must be called so that the request completes. + * @return null if re-request is not supported/necessary, response object otherwise. + * If an object is returned {@code channel.sendResponse()} must be called so that the request completes. */ boolean reRequestAuthentication(final SecurityRequestChannel channel, AuthCredentials credentials); } diff --git a/src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java b/src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java index fecc8d834c..02ea9eea62 100644 --- a/src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java +++ b/src/test/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticatorTest.java @@ -47,6 +47,7 @@ import org.opensearch.common.settings.Settings; import org.opensearch.core.xcontent.MediaType; import org.opensearch.core.xcontent.XContentBuilder; +import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; import org.opensearch.rest.RestRequest.Method; diff --git a/src/test/java/org/opensearch/security/auth/limiting/AddressBasedRateLimiterTest.java b/src/test/java/org/opensearch/security/auth/limiting/AddressBasedRateLimiterTest.java index 827bfa24b6..69ddc5c03a 100644 --- a/src/test/java/org/opensearch/security/auth/limiting/AddressBasedRateLimiterTest.java +++ b/src/test/java/org/opensearch/security/auth/limiting/AddressBasedRateLimiterTest.java @@ -20,28 +20,27 @@ import org.junit.Test; import org.opensearch.common.settings.Settings; -import org.opensearch.security.user.AuthCredentials; + +import java.net.InetAddress; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class AddressBasedRateLimiterTest { - private final static byte[] PASSWORD = new byte[] { '1', '2', '3' }; - @Test public void simpleTest() throws Exception { Settings settings = Settings.builder().put("allowed_tries", 3).build(); - UserNameBasedRateLimiter rateLimiter = new UserNameBasedRateLimiter(settings, null); + AddressBasedRateLimiter rateLimiter = new AddressBasedRateLimiter(settings, null); - assertFalse(rateLimiter.isBlocked("a")); - rateLimiter.onAuthFailure(null, new AuthCredentials("a", PASSWORD), null); - assertFalse(rateLimiter.isBlocked("a")); - rateLimiter.onAuthFailure(null, new AuthCredentials("a", PASSWORD), null); - assertFalse(rateLimiter.isBlocked("a")); - rateLimiter.onAuthFailure(null, new AuthCredentials("a", PASSWORD), null); - assertTrue(rateLimiter.isBlocked("a")); + assertFalse(rateLimiter.isBlocked(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }))); + rateLimiter.onAuthFailure(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }), null, null); + assertFalse(rateLimiter.isBlocked(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }))); + rateLimiter.onAuthFailure(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }), null, null); + assertFalse(rateLimiter.isBlocked(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }))); + rateLimiter.onAuthFailure(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }), null, null); + assertTrue(rateLimiter.isBlocked(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }))); } } diff --git a/src/test/java/org/opensearch/security/auth/limiting/UserNameBasedRateLimiterTest.java b/src/test/java/org/opensearch/security/auth/limiting/UserNameBasedRateLimiterTest.java index e42d2bd1b8..a8285c42a7 100644 --- a/src/test/java/org/opensearch/security/auth/limiting/UserNameBasedRateLimiterTest.java +++ b/src/test/java/org/opensearch/security/auth/limiting/UserNameBasedRateLimiterTest.java @@ -17,30 +17,31 @@ package org.opensearch.security.auth.limiting; -import java.net.InetAddress; - import org.junit.Test; import org.opensearch.common.settings.Settings; +import org.opensearch.security.user.AuthCredentials; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class UserNameBasedRateLimiterTest { + private final static byte[] PASSWORD = new byte[] { '1', '2', '3' }; + @Test public void simpleTest() throws Exception { Settings settings = Settings.builder().put("allowed_tries", 3).build(); - AddressBasedRateLimiter rateLimiter = new AddressBasedRateLimiter(settings, null); + UserNameBasedRateLimiter rateLimiter = new UserNameBasedRateLimiter(settings, null); - assertFalse(rateLimiter.isBlocked(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }))); - rateLimiter.onAuthFailure(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }), null, null); - assertFalse(rateLimiter.isBlocked(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }))); - rateLimiter.onAuthFailure(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }), null, null); - assertFalse(rateLimiter.isBlocked(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }))); - rateLimiter.onAuthFailure(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }), null, null); - assertTrue(rateLimiter.isBlocked(InetAddress.getByAddress(new byte[] { 1, 2, 3, 4 }))); + assertFalse(rateLimiter.isBlocked("a")); + rateLimiter.onAuthFailure(null, new AuthCredentials("a", PASSWORD), null); + assertFalse(rateLimiter.isBlocked("a")); + rateLimiter.onAuthFailure(null, new AuthCredentials("a", PASSWORD), null); + assertFalse(rateLimiter.isBlocked("a")); + rateLimiter.onAuthFailure(null, new AuthCredentials("a", PASSWORD), null); + assertTrue(rateLimiter.isBlocked("a")); } }