Skip to content

Commit

Permalink
Restore changes around passing unsent response
Browse files Browse the repository at this point in the history 
Signed-off-by: Peter Nied <[email protected]>
  • Loading branch information
@peternied
peternied committed Oct 4, 2023
1 parent 2125f9f commit edbe2be
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 48 deletions.
9 changes: 6 additions & 3 deletions src/main/java/com/amazon/dlic/auth/http/kerberos/HTTPSpnegoAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

package com.amazon.dlic.auth.http.kerberos;

import static org.apache.http.HttpStatus.SC_UNAUTHORIZED;

import java.io.Serializable;
import java.nio.file.Files;
import java.nio.file.Path;
Expand All @@ -25,14 +27,14 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;
import java.util.Optional;
import java.util.Set;

import javax.security.auth.Subject;
import javax.security.auth.login.LoginException;

import com.google.common.base.Strings;

import org.apache.http.HttpStatus;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.ietf.jgss.GSSContext;
Expand All @@ -55,6 +57,7 @@
import org.opensearch.security.auth.HTTPAuthenticator;
import org.opensearch.security.filter.SecurityRequest;
import org.opensearch.security.filter.SecurityRequestChannel;
import org.opensearch.security.filter.SecurityResponse;
import org.opensearch.security.user.AuthCredentials;

public class HTTPSpnegoAuthenticator implements HTTPAuthenticator {
Expand Down Expand Up @@ -281,7 +284,7 @@ public GSSCredential run() throws GSSException {
}

@Override
public boolean reRequestAuthentication(final SecurityRequestChannel request, AuthCredentials creds) {
public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest request, AuthCredentials creds) {
final Map<String, String> headers = new HashMap<>();
String responseBody = "";
final String negotiateResponseBody = getNegotiateResponseBody();
Expand All @@ -296,7 +299,7 @@ public boolean reRequestAuthentication(final SecurityRequestChannel request, Aut
headers.put("WWW-Authenticate", "Negotiate " + Base64.getEncoder().encodeToString((byte[]) creds.getNativeCredentials()));
}

return request.completeWithResponse(HttpStatus.SC_UNAUTHORIZED, headers, responseBody);
return Optional.of(new SecurityResponse(SC_UNAUTHORIZED, headers, responseBody));
}

@Override
Expand Down
63 changes: 40 additions & 23 deletions src/main/java/org/opensearch/security/auth/BackendRegistry.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Optional;
import java.util.Set;
import java.util.SortedSet;
import java.util.concurrent.Callable;
Expand All @@ -54,11 +55,13 @@
import org.opensearch.core.common.transport.TransportAddress;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.auth.blocking.ClientBlockRegistry;
import org.opensearch.security.auth.internal.NoOpAuthenticationBackend;
import org.opensearch.security.configuration.AdminDNs;
import org.opensearch.security.filter.SecurityRequestChannel;
import org.opensearch.security.filter.SecurityResponse;
import org.opensearch.security.http.OnBehalfOfAuthenticator;
import org.opensearch.security.http.XFFResolver;
import org.opensearch.security.securityconf.DynamicConfigModel;
Expand All @@ -68,6 +71,8 @@
import org.opensearch.security.user.User;
import org.opensearch.threadpool.ThreadPool;

import static org.apache.http.HttpStatus.SC_UNAUTHORIZED;

public class BackendRegistry {

protected final Logger log = LogManager.getLogger(this.getClass());
Expand Down Expand Up @@ -185,7 +190,7 @@ public void onDynamicConfigModelChanged(DynamicConfigModel dcm) {
* @return The authenticated user, null means another roundtrip
* @throws OpenSearchSecurityException
*/
public void authenticate(final SecurityRequestChannel request, final ThreadContext _DO_NOT_USE) {
public boolean authenticate(final SecurityRequestChannel request) {
final boolean isDebugEnabled = log.isDebugEnabled();
final boolean isBlockedBasedOnAddress = request.getRemoteAddress()
.map(InetSocketAddress::getAddress)
Expand All @@ -196,8 +201,8 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte
log.debug("Rejecting REST request because of blocked address: {}", request.getRemoteAddress().orElse(null));
}

request.completeWithResponse(HttpStatus.SC_UNAUTHORIZED, null, "Authentication finally failed");
return;
request.completeWithResponse(new SecurityResponse(HttpStatus.SC_UNAUTHORIZED, null, "Authentication finally failed"));
return false;
}

final String sslPrincipal = (String) threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_PRINCIPAL);
Expand All @@ -206,18 +211,18 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte
// PKI authenticated REST call
threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User(sslPrincipal));
auditLog.logSucceededLogin(sslPrincipal, true, null, request);
return;
return true;
}

if (userInjector.injectUser(request)) {
// ThreadContext injected user
return;
return true;
}

if (!isInitialized()) {
log.error("Not yet initialized (you may need to run securityadmin)");
request.completeWithResponse(HttpStatus.SC_SERVICE_UNAVAILABLE, null, "OpenSearch Security not initialized.");
return;
request.completeWithResponse(new SecurityResponse(HttpStatus.SC_SERVICE_UNAVAILABLE, null, "OpenSearch Security not initialized."));
return false;
}

final TransportAddress remoteAddress = xffResolver.resolve(request);
Expand Down Expand Up @@ -282,13 +287,17 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte
continue;
}

if (authDomain.isChallenge() && httpAuthenticator.reRequestAuthentication(request, null)) {
auditLog.logFailedLogin("<NONE>", false, null, request);
if (isTraceEnabled) {
log.trace("No 'Authorization' header, send 401 and 'WWW-Authenticate Basic'");
if (authDomain.isChallenge()) {
final Optional<SecurityResponse> restResponse = httpAuthenticator.reRequestAuthentication(request, null);
if (restResponse.isPresent()) {
auditLog.logFailedLogin("<NONE>", false, null, request);
if (isTraceEnabled) {
log.trace("No 'Authorization' header, send 401 and 'WWW-Authenticate Basic'");
}
notifyIpAuthFailureListeners(request, authCredentials);
request.completeWithResponse(restResponse.get());
return false;
}
notifyIpAuthFailureListeners(request, authCredentials);
return;
} else {
// no reRequest possible
if (isTraceEnabled) {
Expand All @@ -300,9 +309,11 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte
org.apache.logging.log4j.ThreadContext.put("user", ac.getUsername());
if (!ac.isComplete()) {
// credentials found in request but we need another client challenge
if (httpAuthenticator.reRequestAuthentication(request, ac)) {
final Optional<SecurityResponse> restResponse = httpAuthenticator.reRequestAuthentication(request, ac);
if (restResponse.isPresent()) {
notifyIpAuthFailureListeners(request, ac);
return;
request.completeWithResponse(restResponse.get());
return false;
} else {
// no reRequest possible
continue;
Expand Down Expand Up @@ -339,12 +350,12 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte
if (adminDns.isAdmin(authenticatedUser)) {
log.error("Cannot authenticate rest user because admin user is not permitted to login via HTTP");
auditLog.logFailedLogin(authenticatedUser.getName(), true, null, request);
request.completeWithResponse(
request.completeWithResponse(new SecurityResponse(
HttpStatus.SC_FORBIDDEN,
null,
"Cannot authenticate user because admin user is not permitted to login via HTTP"
);
return;
));
return false;
}

final String tenant = Utils.coalesce(request.header("securitytenant"), request.header("security_tenant"));
Expand All @@ -369,7 +380,6 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte
authenticatedUser.getName(),
request
);
return;
} else {
if (isDebugEnabled) {
log.debug("User still not authenticated after checking {} auth domains", restAuthDomains.size());
Expand All @@ -385,19 +395,22 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte
if (isDebugEnabled) {
log.debug("Anonymous User is authenticated");
}
return;
return true;
}

Optional<SecurityResponse> challengeResponse = Optional.empty();

if (firstChallengingHttpAuthenticator != null) {

if (isDebugEnabled) {
log.debug("Rerequest with {}", firstChallengingHttpAuthenticator.getClass());
}

if (firstChallengingHttpAuthenticator.reRequestAuthentication(request, null)) {
challengeResponse = firstChallengingHttpAuthenticator.reRequestAuthentication(request, null);
if (challengeResponse.isPresent()) {
if (isDebugEnabled) {
log.debug("Rerequest {} failed", firstChallengingHttpAuthenticator.getClass());
}
return;
}
}

Expand All @@ -410,8 +423,12 @@ public void authenticate(final SecurityRequestChannel request, final ThreadConte

notifyIpAuthFailureListeners(request, authCredentials);

request.completeWithResponse(org.apache.http.HttpStatus.SC_UNAUTHORIZED, null, "Authentication finally failed");
request.completeWithResponse(
challengeResponse.orElseGet(() -> new SecurityResponse(SC_UNAUTHORIZED, null, "Authentication finally failed"))
);
return false;
}
return authenticated;
}

private void notifyIpAuthFailureListeners(SecurityRequestChannel request, AuthCredentials authCredentials) {
Expand Down
10 changes: 5 additions & 5 deletions src/main/java/org/opensearch/security/filter/OpenSearchRequestChannel.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ public boolean hasCompleted() {
}

@Override
public boolean completeWithResponse(int statusCode, Map<String, String> headers, String body) {
System.out.println("@32 - completeWithResponse" + statusCode);
public boolean completeWithResponse(final SecurityResponse response) {
System.out.println("@32 - completeWithResponse" + response.getStatus());
// TODO: Remove
new Exception("&&& completeWithResponse calling stack trace").printStackTrace();

Expand All @@ -42,9 +42,9 @@ public boolean completeWithResponse(int statusCode, Map<String, String> headers,
}

try {
final BytesRestResponse restResponse = new BytesRestResponse(RestStatus.fromCode(statusCode), body);
if (headers != null) {
headers.forEach(restResponse::addHeader);
final BytesRestResponse restResponse = new BytesRestResponse(RestStatus.fromCode(response.getStatus()), response.getBody());
if (response.getHeaders() != null) {
response.getHeaders().forEach(restResponse::addHeader);
}
underlyingChannel.sendResponse(restResponse);

Expand Down
28 changes: 15 additions & 13 deletions src/main/java/org/opensearch/security/filter/SecurityRestFilter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,17 +145,20 @@ public RestHandler wrap(RestHandler original, AdminDNs adminDNs) {
return;
}

if (whitelistingSettings.checkRequestIsAllowed(request, channel, client)
&& allowlistingSettings.checkRequestIsAllowed(request, channel, client)) {
authorizeRequest(original, requestChannel, user);
if (requestChannel.hasCompleted()) {
// Caller was not authorized
return;
} else {
// Caller was authorized, forward the request to the handler
original.handleRequest(request, channel, client);
}
}
if (!(whitelistingSettings.checkRequestIsAllowed(request, channel, client)
&& allowlistingSettings.checkRequestIsAllowed(request, channel, client))) {
// Request is not allowed
return;
}

authorizeRequest(original, requestChannel, user);
if (requestChannel.hasCompleted()) {
// Caller was not authorized
return;
}

// Caller was authorized, forward the request to the handler
original.handleRequest(request, channel, client);
};
}

Expand Down Expand Up @@ -260,8 +263,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t
Matcher matcher = PATTERN_PATH_PREFIX.matcher(requestChannel.path());
final String suffix = matcher.matches() ? matcher.group(2) : null;
if (requestChannel.method() != Method.OPTIONS && !(HEALTH_SUFFIX.equals(suffix)) && !(WHO_AM_I_SUFFIX.equals(suffix))) {
registry.authenticate(requestChannel, null);
if (requestChannel.hasCompleted()) {
if (!registry.authenticate(requestChannel)) {
// another roundtrip
org.apache.logging.log4j.ThreadContext.remove("user");
return;
Expand Down
6 changes: 4 additions & 2 deletions src/main/java/org/opensearch/security/http/OnBehalfOfAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import java.security.PrivilegedAction;
import java.util.Arrays;
import java.util.List;
import java.util.Optional;
import java.util.Map.Entry;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
Expand All @@ -37,6 +38,7 @@
import org.opensearch.security.authtoken.jwt.EncryptionDecryptionUtil;
import org.opensearch.security.filter.SecurityRequest;
import org.opensearch.security.filter.SecurityRequestChannel;
import org.opensearch.security.filter.SecurityResponse;
import org.opensearch.security.ssl.util.ExceptionUtils;
import org.opensearch.security.user.AuthCredentials;
import org.opensearch.security.util.KeyUtils;
Expand Down Expand Up @@ -242,8 +244,8 @@ public Boolean isRequestAllowed(final SecurityRequest request) {
}

@Override
public boolean reRequestAuthentication(final SecurityRequestChannel response, AuthCredentials creds) {
return false;
public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest response, AuthCredentials creds) {
return Optional.empty();
}

@Override
Expand Down
6 changes: 4 additions & 2 deletions src/test/java/org/opensearch/security/cache/DummyHTTPAuthenticator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,15 @@
package org.opensearch.security.cache;

import java.nio.file.Path;
import java.util.Optional;

import org.opensearch.OpenSearchSecurityException;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.security.auth.HTTPAuthenticator;
import org.opensearch.security.filter.SecurityRequest;
import org.opensearch.security.filter.SecurityRequestChannel;
import org.opensearch.security.filter.SecurityResponse;
import org.opensearch.security.user.AuthCredentials;

public class DummyHTTPAuthenticator implements HTTPAuthenticator {
Expand All @@ -40,8 +42,8 @@ public AuthCredentials extractCredentials(final SecurityRequest request, final T
}

@Override
public boolean reRequestAuthentication(SecurityRequestChannel channel, AuthCredentials credentials) {
return false;
public Optional<SecurityResponse> reRequestAuthentication(SecurityRequest channel, AuthCredentials credentials) {
return Optional.empty();
}

public static long getCount() {
Expand Down

0 comments on commit edbe2be

Please sign in to comment.