Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Backport 1.3] Add early rejection from RestHandler for unauthorized requests (#3418) #3675

Merged
merged 25 commits into from
Nov 27, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ab88c77
Pull in unmodified integration test folder
peternied Nov 3, 2023
87323d5
Executable integration test
peternied Nov 3, 2023
bc0882e
Get CI fully passing
peternied Nov 8, 2023
fa687d7
Disable resource tests while waiting for the fix to be backported
peternied Nov 8, 2023
640a439
[Backport 1.3] Expanding Authentication with SecurityRequest Abstract…
peternied Oct 6, 2023
3d005bd
Fix missing Map.of(...)
peternied Nov 8, 2023
4f70185
Fix compile issues on jdk 8
peternied Nov 8, 2023
7dc969a
Handle orElseThrow(...) missing API
peternied Nov 8, 2023
303b8e3
Add missing overrides
peternied Nov 8, 2023
0468305
Playing with test
peternied Nov 9, 2023
1b4eee9
Merge remote-tracking branch 'origin/1.3' into security-request-abstr…
peternied Nov 9, 2023
5190af3
Remove whitespace change
peternied Nov 9, 2023
b6df98a
Remove misc changes
peternied Nov 9, 2023
916a2e8
Remove minor whitespace change
peternied Nov 9, 2023
114d7a8
Don't exit until response has been queued
peternied Nov 9, 2023
f12a66b
[Backport 1.3] Add early rejection from RestHandler for unauthorized …
peternied Oct 7, 2023
b1558fc
Remove the ignore flag on the resource tests
peternied Nov 9, 2023
b06ce7b
Fix jdk8 incompaitable changes
peternied Nov 10, 2023
355fb12
Fix some test failures
peternied Nov 10, 2023
b6eb22c
Merge remote-tracking branch 'origin/1.3' into early-rejection
peternied Nov 21, 2023
066d69f
Merge remote-tracking branch 'peternied/early-rejection' into early-r…
peternied Nov 21, 2023
609bb2e
[Backport 2.11] Use BytesRestResponse constructor with contentType in…
cwperks Nov 16, 2023
71f73d0
[Backport 2.11] Use new instance of Decompressor on channel initializ…
opensearch-trigger-bot[bot] Oct 26, 2023
7408813
Fix test import for RestStatus
peternied Nov 21, 2023
97aacef
Fix jdk8 issues
peternied Nov 21, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import org.apache.logging.log4j.Logger;
import org.junit.BeforeClass;
import org.junit.ClassRule;
import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.opensearch.action.index.IndexRequest;
Expand All @@ -49,7 +48,6 @@

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;

@Ignore("Waiting on backport for https://github.com/opensearch-project/security/pull/3418")
@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
public class ResourceFocusedTests {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.common.xcontent.XContentBuilder;
import org.opensearch.common.xcontent.XContentFactory;
import org.opensearch.common.xcontent.XContentType;
import org.opensearch.env.Environment;
import org.ietf.jgss.GSSContext;
import org.ietf.jgss.GSSCredential;
Expand Down Expand Up @@ -281,10 +282,12 @@ public GSSCredential run() throws GSSException {
public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest request, AuthCredentials creds) {
final Map<String, String> headers = new HashMap<>();
String responseBody = "";
String contentType = null;
SecurityResponse response;
final String negotiateResponseBody = getNegotiateResponseBody();
if (negotiateResponseBody != null) {
responseBody = negotiateResponseBody;
headers.putAll(SecurityResponse.CONTENT_TYPE_APP_JSON);
contentType = XContentType.JSON.mediaType();
}

if (creds == null || creds.getNativeCredentials() == null) {
Expand All @@ -293,7 +296,12 @@ public Optional<SecurityResponse> reRequestAuthentication(final SecurityRequest
headers.put("WWW-Authenticate", "Negotiate " + Base64.getEncoder().encodeToString((byte[]) creds.getNativeCredentials()));
}

return Optional.of(new SecurityResponse(SC_UNAUTHORIZED, headers, responseBody));
if (contentType != null) {
response = new SecurityResponse(SC_UNAUTHORIZED, headers, responseBody, contentType);
} else {
response = new SecurityResponse(SC_UNAUTHORIZED, headers, responseBody);
}
return Optional.of(response);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ private Optional<SecurityResponse> handleLowLevel(RestRequest restRequest) throw

String responseBodyString = DefaultObjectMapper.objectMapper.writeValueAsString(responseBody);

return Optional.of(new SecurityResponse(HttpStatus.SC_OK, SecurityResponse.CONTENT_TYPE_APP_JSON, responseBodyString));
return Optional.of(new SecurityResponse(HttpStatus.SC_OK, null, responseBodyString, XContentType.JSON.mediaType()));
} catch (JsonProcessingException e) {
log.warn("Error while parsing JSON for /_opendistro/_security/api/authtoken", e);
return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, null, "JSON could not be parsed"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public class HTTPSamlAuthenticator implements HTTPAuthenticator, Destroyable {
public static final String IDP_METADATA_FILE = "idp.metadata_file";
public static final String IDP_METADATA_CONTENT = "idp.metadata_content";

private static final String API_AUTHTOKEN_SUFFIX = "api/authtoken";
public static final String API_AUTHTOKEN_SUFFIX = "api/authtoken";
private static final String AUTHINFO_SUFFIX = "authinfo";
private static final String REGEX_PATH_PREFIX = "/(" + LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" +"(.*)";
private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,6 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
public static final String PLUGINS_PREFIX = "_plugins/_security";

private boolean sslCertReloadEnabled;
private volatile SecurityRestFilter securityRestHandler;
private volatile SecurityInterceptor si;
private volatile PrivilegesEvaluator evaluator;
private volatile ThreadPool threadPool;
Expand Down Expand Up @@ -722,13 +721,13 @@ public Map<String, Supplier<HttpServerTransport>> getHttpTransports(Settings set
settings, configPath, evaluateSslExceptionHandler());
//TODO close odshst
final SecurityHttpServerTransport odshst = new SecurityHttpServerTransport(settings, networkService, bigArrays,
threadPool, sks, evaluateSslExceptionHandler(), xContentRegistry, validatingDispatcher, clusterSettings, sharedGroupFactory);
threadPool, sks, evaluateSslExceptionHandler(), xContentRegistry, validatingDispatcher, clusterSettings, sharedGroupFactory, securityRestHandler);

return Collections.singletonMap("org.opensearch.security.http.SecurityHttpServerTransport",
() -> odshst);
} else if (!client) {
return Collections.singletonMap("org.opensearch.security.http.SecurityHttpServerTransport",
() -> new SecurityNonSslHttpServerTransport(settings, networkService, bigArrays, threadPool, xContentRegistry, dispatcher, clusterSettings, sharedGroupFactory));
() -> new SecurityNonSslHttpServerTransport(settings, networkService, bigArrays, threadPool, xContentRegistry, dispatcher, clusterSettings, sharedGroupFactory, securityRestHandler));
}
}
return Collections.emptyMap();
Expand Down
22 changes: 11 additions & 11 deletions src/main/java/org/opensearch/security/auth/BackendRegistry.java
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
import org.opensearch.common.settings.Settings;
import org.opensearch.common.transport.TransportAddress;
import org.opensearch.rest.RestStatus;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.auth.blocking.ClientBlockRegistry;
import org.opensearch.security.auth.internal.NoOpAuthenticationBackend;
Expand Down Expand Up @@ -187,6 +188,8 @@ public BackendRegistry(final Settings settings, final AdminDNs adminDns,
this.auditLog = auditLog;
this.threadPool = threadPool;
this.userInjector = new UserInjector(settings, threadPool, auditLog, xffResolver);
this.restAuthDomains = Collections.emptySortedSet();
this.ipAuthFailureListeners = Collections.emptyList();


this.ttlInMin = settings.getAsInt(ConfigConstants.SECURITY_CACHE_TTL_MINUTES, 60);
Expand Down Expand Up @@ -353,7 +356,6 @@ public User authenticate(final TransportRequest request, final String sslPrincip
/**
*
* @param request
* @param channel
* @return The authenticated user, null means another roundtrip
* @throws OpenSearchSecurityException
*/
Expand All @@ -372,11 +374,13 @@ public boolean authenticate(final SecurityRequestChannel request) {
return false;
}

final String sslPrincipal = (String) threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_PRINCIPAL);
ThreadContext threadContext = this.threadPool.getThreadContext();

if(adminDns.isAdminDN(sslPrincipal)) {
//PKI authenticated REST call
threadPool.getThreadContext().putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User(sslPrincipal));
final String sslPrincipal = (String) threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_SSL_PRINCIPAL);

if (adminDns.isAdminDN(sslPrincipal)) {
// PKI authenticated REST call
threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, new User(sslPrincipal));
auditLog.logSucceededLogin(sslPrincipal, true, null, request);
return true;
}
Expand Down Expand Up @@ -505,11 +509,7 @@ public boolean authenticate(final SecurityRequestChannel request) {
log.error("Cannot authenticate rest user because admin user is not permitted to login via HTTP");
auditLog.logFailedLogin(authenticatedUser.getName(), true, null, request);
request.queueForSending(
new SecurityResponse(
SC_FORBIDDEN,
null,
"Cannot authenticate user because admin user is not permitted to login via HTTP"
)
new SecurityResponse(SC_FORBIDDEN, "Cannot authenticate user because admin user is not permitted to login via HTTP")
);
return false;
}
Expand Down Expand Up @@ -577,7 +577,7 @@ public boolean authenticate(final SecurityRequestChannel request) {
notifyIpAuthFailureListeners(request, authCredenetials);

request.queueForSending(
challengeResponse.orElseGet(() -> new SecurityResponse(SC_UNAUTHORIZED, null, "Authentication finally failed"))
challengeResponse.orElseGet(() -> new SecurityResponse(SC_UNAUTHORIZED, "Authentication finally failed"))
);
return false;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package org.opensearch.security.filter;

import java.util.Optional;

import org.opensearch.http.netty4.Netty4HttpChannel;
import org.opensearch.rest.RestRequest;

import io.netty.channel.Channel;
import io.netty.channel.ChannelHandlerContext;
import io.netty.util.AttributeKey;

public class NettyAttribute {

/**
* Gets an attribute value from the request context and clears it from that context
*/
public static <T> Optional<T> popFrom(final RestRequest request, final AttributeKey<T> attribute) {
if (request.getHttpChannel() instanceof Netty4HttpChannel) {
Channel nettyChannel = ((Netty4HttpChannel) request.getHttpChannel()).getNettyChannel();
return Optional.ofNullable(nettyChannel.attr(attribute).getAndSet(null));
}
return Optional.empty();
}

/**
* Gets an attribute value from the channel handler context and clears it from that context
*/
public static <T> Optional<T> popFrom(final ChannelHandlerContext ctx, final AttributeKey<T> attribute) {
return Optional.ofNullable(ctx.channel().attr(attribute).getAndSet(null));
}

/**
* Gets an attribute value from the channel handler context
*/
public static <T> Optional<T> peekFrom(final ChannelHandlerContext ctx, final AttributeKey<T> attribute) {
return Optional.ofNullable(ctx.channel().attr(attribute).get());
}

/**
* Clears an attribute value from the channel handler context
*/
public static <T> void clearAttribute(final RestRequest request, final AttributeKey<T> attribute) {
if (request.getHttpChannel() instanceof Netty4HttpChannel) {
Channel nettyChannel = ((Netty4HttpChannel) request.getHttpChannel()).getNettyChannel();
nettyChannel.attr(attribute).set(null);
}
}

}
102 changes: 102 additions & 0 deletions src/main/java/org/opensearch/security/filter/NettyRequest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/*
* 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.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.security.filter;

import java.net.InetSocketAddress;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.TreeMap;

import javax.net.ssl.SSLEngine;

import io.netty.handler.ssl.SslHandler;
import org.opensearch.http.netty4.Netty4HttpChannel;
import org.opensearch.rest.RestRequest.Method;

import com.google.common.collect.ImmutableList;

import io.netty.handler.codec.http.HttpRequest;
import org.opensearch.rest.RestUtils;

/**
* Wraps the functionality of HttpRequest for use in the security plugin
*/
public class NettyRequest implements SecurityRequest {

protected final HttpRequest underlyingRequest;
protected final Netty4HttpChannel underlyingChannel;

NettyRequest(final HttpRequest request, final Netty4HttpChannel channel) {
this.underlyingRequest = request;
this.underlyingChannel = channel;
}

@Override
public Map<String, List<String>> getHeaders() {
final Map<String, List<String>> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER);
underlyingRequest.headers().forEach(h -> headers.put(h.getKey(), ImmutableList.of(h.getValue())));
return headers;
}

@Override
public SSLEngine getSSLEngine() {
// We look for Ssl_handler called `ssl_http` in the outbound pipeline of Netty channel first, and if its not
// present we look for it in inbound channel. If its present in neither we return null, else we return the sslHandler.
SslHandler sslhandler = (SslHandler) underlyingChannel.getNettyChannel().pipeline().get("ssl_http");
return sslhandler != null ? sslhandler.engine() : null;
}

@Override
public String path() {
String rawPath = SecurityRestUtils.path(underlyingRequest.uri());
return RestUtils.decodeComponent(rawPath);
}

@Override
public Method method() {
return Method.valueOf(underlyingRequest.method().name());
}

@Override
public Optional<InetSocketAddress> getRemoteAddress() {
return Optional.ofNullable(this.underlyingChannel.getRemoteAddress());
}

@Override
public String uri() {
return underlyingRequest.uri();
}

@Override
public Map<String, String> params() {
return params(underlyingRequest.uri());
}

private static Map<String, String> params(String uri) {
// Sourced from
// https://github.com/opensearch-project/OpenSearch/blob/main/server/src/main/java/org/opensearch/http/AbstractHttpServerTransport.java#L419-L422
final Map<String, String> params = new HashMap<>();
final int index = uri.indexOf(63);
if (index >= 0) {
try {
RestUtils.decodeQueryString(uri, index + 1, params);
} catch (IllegalArgumentException var4) {
return Collections.emptyMap();
}
}

return params;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
/*
* 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.
*
* Modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

package org.opensearch.security.filter;

import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicReference;

import io.netty.handler.codec.http.HttpRequest;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.http.netty4.Netty4HttpChannel;

public class NettyRequestChannel extends NettyRequest implements SecurityRequestChannel {
private final Logger log = LogManager.getLogger(NettyRequestChannel.class);

private AtomicBoolean hasCompleted = new AtomicBoolean(false);
private final AtomicReference<SecurityResponse> responseRef = new AtomicReference<SecurityResponse>(null);

NettyRequestChannel(final HttpRequest request, Netty4HttpChannel channel) {
super(request, channel);
}

@Override
public void queueForSending(SecurityResponse response) {
if (underlyingChannel == null) {
throw new UnsupportedOperationException("Channel was not defined");
}

if (hasCompleted.get()) {
throw new UnsupportedOperationException("This channel has already completed");
}

if (getQueuedResponse().isPresent()) {
throw new UnsupportedOperationException("Another response was already queued");
}

responseRef.set(response);
}

@Override
public Optional<SecurityResponse> getQueuedResponse() {
return Optional.ofNullable(responseRef.get());
}
}
Loading
Loading