From f12a66b342567a5f4ac8aead60f3d7fab1a478cb Mon Sep 17 00:00:00 2001 From: Peter Nied Date: Fri, 6 Oct 2023 20:36:40 -0500 Subject: [PATCH] [Backport 1.3] Add early rejection from RestHandler for unauthorized requests (#3418) (#3495) Backport of 6b0b682da from #3418 Previously unauthorized requests were fully processed and rejected once they reached the RestHandler. This allocations more memory and resources for these requests that might not be useful if they are already detected as unauthorized. Using the headerVerifer and decompressor customization from [1], perform an early authorization check when only the headers are available, save an 'early response' for transmission and do not perform the decompression on the request to speed up closing out the connection. - Resolves https://github.com/opensearch-project/OpenSearch/issues/10260 Signed-off-by: Peter Nied Signed-off-by: Craig Perkins Signed-off-by: Craig Perkins Co-authored-by: Craig Perkins Signed-off-by: Peter Nied --- .../http/saml/AuthTokenProcessorHandler.java | 2 +- .../auth/http/saml/HTTPSamlAuthenticator.java | 2 +- .../security/OpenSearchSecurityPlugin.java | 5 +- .../security/auth/BackendRegistry.java | 18 ++- .../security/filter/NettyAttribute.java | 49 +++++++ .../security/filter/NettyRequest.java | 100 +++++++++++++ .../security/filter/NettyRequestChannel.java | 54 +++++++ .../filter/OpenSearchRequestChannel.java | 45 ------ .../filter/SecurityRequestChannel.java | 7 +- .../filter/SecurityRequestFactory.java | 7 + .../security/filter/SecurityResponse.java | 33 +++++ .../security/filter/SecurityRestFilter.java | 48 ++++-- .../security/filter/SecurityRestUtils.java | 12 ++ .../http/SecurityHttpServerTransport.java | 48 +++++- .../SecurityNonSslHttpServerTransport.java | 49 ++++++- .../opensearch/security/http/XFFResolver.java | 13 +- .../ssl/OpenSearchSecuritySSLPlugin.java | 33 ++++- .../netty/Netty4ConditionalDecompressor.java | 37 +++++ .../Netty4HttpRequestHeaderVerifier.java | 137 ++++++++++++++++++ .../SecuritySSLNettyHttpServerTransport.java | 56 +++++-- .../integration/BasicAuditlogTest.java | 13 +- .../filter/SecurityRestFilterUnitTests.java | 109 ++++++++++++++ .../helper/cluster/ClusterConfiguration.java | 2 +- .../test/plugin/UserInjectorPlugin.java | 116 --------------- 24 files changed, 752 insertions(+), 243 deletions(-) create mode 100644 src/main/java/org/opensearch/security/filter/NettyAttribute.java create mode 100644 src/main/java/org/opensearch/security/filter/NettyRequest.java create mode 100644 src/main/java/org/opensearch/security/filter/NettyRequestChannel.java create mode 100644 src/main/java/org/opensearch/security/filter/SecurityRestUtils.java create mode 100644 src/main/java/org/opensearch/security/ssl/http/netty/Netty4ConditionalDecompressor.java create mode 100644 src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java create mode 100644 src/test/java/org/opensearch/security/filter/SecurityRestFilterUnitTests.java delete mode 100644 src/test/java/org/opensearch/security/test/plugin/UserInjectorPlugin.java 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 6157853324..3bab0343cb 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 @@ -240,7 +240,7 @@ private Optional handleLowLevel(RestRequest restRequest) throw return Optional.of(new SecurityResponse(HttpStatus.SC_OK, SecurityResponse.CONTENT_TYPE_APP_JSON, responseBodyString)); } 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")); + return Optional.of(new SecurityResponse(HttpStatus.SC_BAD_REQUEST, new Exception("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 f37ea522f2..51b58b71c6 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 @@ -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); diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 4a1f5f5227..bfb6826c6f 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -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; @@ -722,13 +721,13 @@ public Map> 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(); diff --git a/src/main/java/org/opensearch/security/auth/BackendRegistry.java b/src/main/java/org/opensearch/security/auth/BackendRegistry.java index 2300b424dc..3b108fbdaa 100644 --- a/src/main/java/org/opensearch/security/auth/BackendRegistry.java +++ b/src/main/java/org/opensearch/security/auth/BackendRegistry.java @@ -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; @@ -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); @@ -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 */ @@ -368,15 +370,17 @@ public boolean authenticate(final SecurityRequestChannel request) { log.debug("Rejecting REST request because of blocked address: {}", request.getRemoteAddress().orElse(null)); } - request.queueForSending(new SecurityResponse(SC_UNAUTHORIZED, null, "Authentication finally failed")); + request.queueForSending(new SecurityResponse(SC_UNAUTHORIZED, new Exception("Authentication finally failed"))); 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; } @@ -388,7 +392,7 @@ public boolean authenticate(final SecurityRequestChannel request) { if (!isInitialized()) { log.error("Not yet initialized (you may need to run securityadmin)"); - request.queueForSending(new SecurityResponse(SC_SERVICE_UNAVAILABLE, null, "OpenSearch Security not initialized.")); + request.queueForSending(new SecurityResponse(SC_SERVICE_UNAVAILABLE, new Exception("OpenSearch Security not initialized."))); return false; } diff --git a/src/main/java/org/opensearch/security/filter/NettyAttribute.java b/src/main/java/org/opensearch/security/filter/NettyAttribute.java new file mode 100644 index 0000000000..685e94e199 --- /dev/null +++ b/src/main/java/org/opensearch/security/filter/NettyAttribute.java @@ -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 Optional popFrom(final RestRequest request, final AttributeKey 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 Optional popFrom(final ChannelHandlerContext ctx, final AttributeKey attribute) { + return Optional.ofNullable(ctx.channel().attr(attribute).getAndSet(null)); + } + + /** + * Gets an attribute value from the channel handler context + */ + public static Optional peekFrom(final ChannelHandlerContext ctx, final AttributeKey attribute) { + return Optional.ofNullable(ctx.channel().attr(attribute).get()); + } + + /** + * Clears an attribute value from the channel handler context + */ + public static void clearAttribute(final RestRequest request, final AttributeKey attribute) { + if (request.getHttpChannel() instanceof Netty4HttpChannel) { + Channel nettyChannel = ((Netty4HttpChannel) request.getHttpChannel()).getNettyChannel(); + nettyChannel.attr(attribute).set(null); + } + } + +} diff --git a/src/main/java/org/opensearch/security/filter/NettyRequest.java b/src/main/java/org/opensearch/security/filter/NettyRequest.java new file mode 100644 index 0000000000..4ef17b9dc7 --- /dev/null +++ b/src/main/java/org/opensearch/security/filter/NettyRequest.java @@ -0,0 +1,100 @@ +/* + * 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 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> getHeaders() { + final Map> headers = new TreeMap<>(String.CASE_INSENSITIVE_ORDER); + underlyingRequest.headers().forEach(h -> headers.put(h.getKey(), List.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 getRemoteAddress() { + return Optional.ofNullable(this.underlyingChannel.getRemoteAddress()); + } + + @Override + public String uri() { + return underlyingRequest.uri(); + } + + @Override + public Map params() { + return params(underlyingRequest.uri()); + } + + private static Map 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 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; + } +} diff --git a/src/main/java/org/opensearch/security/filter/NettyRequestChannel.java b/src/main/java/org/opensearch/security/filter/NettyRequestChannel.java new file mode 100644 index 0000000000..a83ecdea8a --- /dev/null +++ b/src/main/java/org/opensearch/security/filter/NettyRequestChannel.java @@ -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 responseRef = new AtomicReference(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 getQueuedResponse() { + return Optional.ofNullable(responseRef.get()); + } +} diff --git a/src/main/java/org/opensearch/security/filter/OpenSearchRequestChannel.java b/src/main/java/org/opensearch/security/filter/OpenSearchRequestChannel.java index 4f2026cd93..24c90488cb 100644 --- a/src/main/java/org/opensearch/security/filter/OpenSearchRequestChannel.java +++ b/src/main/java/org/opensearch/security/filter/OpenSearchRequestChannel.java @@ -12,22 +12,14 @@ package org.opensearch.security.filter; import java.util.Optional; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicReference; -import org.apache.logging.log4j.LogManager; -import org.apache.logging.log4j.Logger; -import org.opensearch.rest.RestStatus; -import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; public class OpenSearchRequestChannel extends OpenSearchRequest implements SecurityRequestChannel { - private final Logger log = LogManager.getLogger(OpenSearchRequest.class); - private final AtomicReference responseRef = new AtomicReference(null); - private final AtomicBoolean hasCompleted = new AtomicBoolean(false); private final RestChannel underlyingChannel; OpenSearchRequestChannel(final RestRequest request, final RestChannel channel) { @@ -46,10 +38,6 @@ public void queueForSending(final SecurityResponse response) { 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"); } @@ -61,37 +49,4 @@ public void queueForSending(final SecurityResponse response) { public Optional getQueuedResponse() { return Optional.ofNullable(responseRef.get()); } - - @Override - public boolean sendResponse() { - 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("No response has been associated with this channel"); - } - - final SecurityResponse response = responseRef.get(); - - try { - final BytesRestResponse restResponse = new BytesRestResponse(RestStatus.fromCode(response.getStatus()), response.getBody()); - if (response.getHeaders() != null) { - response.getHeaders().forEach(restResponse::addHeader); - } - underlyingChannel.sendResponse(restResponse); - - return true; - } catch (final Exception e) { - log.error("Error when attempting to send response", e); - throw new RuntimeException(e); - } finally { - hasCompleted.set(true); - } - - } } diff --git a/src/main/java/org/opensearch/security/filter/SecurityRequestChannel.java b/src/main/java/org/opensearch/security/filter/SecurityRequestChannel.java index 1eec754c08..66744d01dd 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityRequestChannel.java +++ b/src/main/java/org/opensearch/security/filter/SecurityRequestChannel.java @@ -19,11 +19,8 @@ public interface SecurityRequestChannel extends SecurityRequest { /** Associate a response with this channel */ - public void queueForSending(final SecurityResponse response); + void queueForSending(final SecurityResponse response); /** Acess the queued response */ - public Optional getQueuedResponse(); - - /** Send the response through the channel */ - public boolean sendResponse(); + Optional getQueuedResponse(); } diff --git a/src/main/java/org/opensearch/security/filter/SecurityRequestFactory.java b/src/main/java/org/opensearch/security/filter/SecurityRequestFactory.java index de74df01ff..0b64d0220d 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityRequestFactory.java +++ b/src/main/java/org/opensearch/security/filter/SecurityRequestFactory.java @@ -11,6 +11,8 @@ package org.opensearch.security.filter; +import io.netty.handler.codec.http.HttpRequest; +import org.opensearch.http.netty4.Netty4HttpChannel; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; @@ -24,6 +26,11 @@ public static SecurityRequest from(final RestRequest request) { return new OpenSearchRequest(request); } + /** Creates a security request from a netty HttpRequest object */ + public static SecurityRequestChannel from(HttpRequest request, Netty4HttpChannel channel) { + return new NettyRequestChannel(request, channel); + } + /** Creates a security request channel from a RestRequest & RestChannel */ public static SecurityRequestChannel from(final RestRequest request, final RestChannel channel) { return new OpenSearchRequestChannel(request, channel); diff --git a/src/main/java/org/opensearch/security/filter/SecurityResponse.java b/src/main/java/org/opensearch/security/filter/SecurityResponse.java index 56bf733a1d..009a1c3769 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityResponse.java +++ b/src/main/java/org/opensearch/security/filter/SecurityResponse.java @@ -11,9 +11,14 @@ package org.opensearch.security.filter; +import java.io.IOException; import java.util.Map; import org.apache.http.HttpHeaders; +import org.opensearch.common.xcontent.XContentFactory; +import org.opensearch.rest.RestStatus; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestResponse; import com.google.common.collect.ImmutableMap; @@ -25,6 +30,12 @@ public class SecurityResponse { private final Map headers; private final String body; + public SecurityResponse(final int status, final Exception e) { + this.status = status; + this.headers = CONTENT_TYPE_APP_JSON; + this.body = generateFailureMessage(e); + } + public SecurityResponse(final int status, final Map headers, final String body) { this.status = status; this.headers = headers; @@ -43,4 +54,26 @@ public String getBody() { return body; } + public RestResponse asRestResponse() { + final RestResponse restResponse = new BytesRestResponse(RestStatus.fromCode(getStatus()), getBody()); + if (getHeaders() != null) { + getHeaders().forEach(restResponse::addHeader); + } + return restResponse; + } + + protected String generateFailureMessage(final Exception e) { + try { + return XContentFactory.jsonBuilder() + .startObject() + .startObject("error") + .field("status", "error") + .field("reason", e.getMessage()) + .endObject() + .endObject() + .toString(); + } catch (final IOException ioe) { + throw new RuntimeException(ioe); + } + } } diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java index 62a9ab92db..6ab2ea1b57 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java @@ -52,20 +52,23 @@ import org.opensearch.security.ssl.util.SSLRequestHelper; import org.opensearch.security.support.ConfigConstants; import org.opensearch.security.support.HTTPHelper; +import org.opensearch.security.user.User; +import org.opensearch.tasks.Task; import org.opensearch.threadpool.ThreadPool; import org.opensearch.security.ssl.util.SSLRequestHelper.SSLInfo; import org.opensearch.security.auth.BackendRegistry; -import org.opensearch.security.user.User; import org.greenrobot.eventbus.Subscribe; -import java.util.NoSuchElementException; import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX; +import static org.opensearch.security.http.SecurityHttpServerTransport.CONTEXT_TO_RESTORE; +import static org.opensearch.security.http.SecurityHttpServerTransport.EARLY_RESPONSE; +import static org.opensearch.security.http.SecurityHttpServerTransport.IS_AUTHENTICATED; public class SecurityRestFilter { @@ -80,9 +83,9 @@ public class SecurityRestFilter { private WhitelistingSettings whitelistingSettings; - private static final String HEALTH_SUFFIX = "health"; + public static final String HEALTH_SUFFIX = "health"; private static final String REGEX_PATH_PREFIX = "/("+ LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" +"(.*)"; - private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX); + public static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX); public SecurityRestFilter(final BackendRegistry registry, final AuditLog auditLog, @@ -114,13 +117,33 @@ public SecurityRestFilter(final BackendRegistry registry, final AuditLog auditLo */ public RestHandler wrap(RestHandler original, AdminDNs adminDNs) { return (request, channel, client) -> { - org.apache.logging.log4j.ThreadContext.clearAll(); + + final Optional maybeSavedResponse = NettyAttribute.popFrom(request, EARLY_RESPONSE); + if (maybeSavedResponse.isPresent()) { + NettyAttribute.clearAttribute(request, CONTEXT_TO_RESTORE); + NettyAttribute.clearAttribute(request, IS_AUTHENTICATED); + channel.sendResponse(maybeSavedResponse.get().asRestResponse()); + return; + } + + NettyAttribute.popFrom(request, CONTEXT_TO_RESTORE).ifPresent(storedContext -> { + // X_OPAQUE_ID will be overritten on restore - save to apply after restoring the saved context + final String xOpaqueId = threadContext.getHeader(Task.X_OPAQUE_ID); + storedContext.restore(); + if (xOpaqueId != null) { + threadContext.putHeader(Task.X_OPAQUE_ID, xOpaqueId); + } + }); + final SecurityRequestChannel requestChannel = SecurityRequestFactory.from(request, channel); // Authenticate request - checkAndAuthenticateRequest(requestChannel); + if (!NettyAttribute.popFrom(request, IS_AUTHENTICATED).orElse(false)) { + // we aren't authenticated so we should skip this step + checkAndAuthenticateRequest(requestChannel); + } if (requestChannel.getQueuedResponse().isPresent()) { - requestChannel.sendResponse(); + channel.sendResponse(requestChannel.getQueuedResponse().get().asRestResponse()); return; } @@ -135,8 +158,7 @@ public RestHandler wrap(RestHandler original, AdminDNs adminDNs) { final Optional deniedResponse = whitelistingSettings.checkRequestIsAllowed(requestChannel); if (deniedResponse.isPresent()) { - requestChannel.queueForSending(deniedResponse.orElseThrow(NoSuchElementException::new)); - requestChannel.sendResponse(); + channel.sendResponse(deniedResponse.get().asRestResponse()); return; } @@ -148,7 +170,7 @@ public RestHandler wrap(RestHandler original, AdminDNs adminDNs) { /** * Checks if a given user is a SuperAdmin */ - private boolean userIsSuperAdmin(User user, AdminDNs adminDNs) { + boolean userIsSuperAdmin(User user, AdminDNs adminDNs) { return user != null && adminDNs.isAdmin(user); } @@ -160,7 +182,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t log.error(exception.toString()); auditLog.logBadHeaders(requestChannel); - requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString())); + requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, exception)); return; } @@ -169,7 +191,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t log.error(exception.toString()); auditLog.logBadHeaders(requestChannel); - requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, exception.toString())); + requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, exception)); return; } @@ -189,7 +211,7 @@ public void checkAndAuthenticateRequest(SecurityRequestChannel requestChannel) t } catch (SSLPeerUnverifiedException e) { log.error("No ssl info", e); auditLog.logSSLException(requestChannel, e); - requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, null, null)); + requestChannel.queueForSending(new SecurityResponse(HttpStatus.SC_FORBIDDEN, new Exception("No ssl info"))); return; } diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestUtils.java b/src/main/java/org/opensearch/security/filter/SecurityRestUtils.java new file mode 100644 index 0000000000..1599346b90 --- /dev/null +++ b/src/main/java/org/opensearch/security/filter/SecurityRestUtils.java @@ -0,0 +1,12 @@ +package org.opensearch.security.filter; + +public class SecurityRestUtils { + public static String path(final String uri) { + final int index = uri.indexOf('?'); + if (index >= 0) { + return uri.substring(0, index); + } else { + return uri; + } + } +} diff --git a/src/main/java/org/opensearch/security/http/SecurityHttpServerTransport.java b/src/main/java/org/opensearch/security/http/SecurityHttpServerTransport.java index 10a626ced5..57200045e6 100644 --- a/src/main/java/org/opensearch/security/http/SecurityHttpServerTransport.java +++ b/src/main/java/org/opensearch/security/http/SecurityHttpServerTransport.java @@ -30,24 +30,56 @@ package org.opensearch.security.http; -import org.opensearch.security.ssl.http.netty.SecuritySSLNettyHttpServerTransport; +import io.netty.util.AttributeKey; import org.opensearch.common.network.NetworkService; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.common.util.BigArrays; +import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.common.xcontent.NamedXContentRegistry; -import org.opensearch.threadpool.ThreadPool; -import org.opensearch.transport.SharedGroupFactory; - +import org.opensearch.security.filter.SecurityResponse; +import org.opensearch.security.filter.SecurityRestFilter; import org.opensearch.security.ssl.SecurityKeyStore; import org.opensearch.security.ssl.SslExceptionHandler; +import org.opensearch.security.ssl.http.netty.SecuritySSLNettyHttpServerTransport; import org.opensearch.security.ssl.http.netty.ValidatingDispatcher; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.SharedGroupFactory; public class SecurityHttpServerTransport extends SecuritySSLNettyHttpServerTransport { - public SecurityHttpServerTransport(final Settings settings, final NetworkService networkService, - final BigArrays bigArrays, final ThreadPool threadPool, final SecurityKeyStore odsks, - final SslExceptionHandler sslExceptionHandler, final NamedXContentRegistry namedXContentRegistry, final ValidatingDispatcher dispatcher, final ClusterSettings clusterSettings, SharedGroupFactory sharedGroupFactory) { - super(settings, networkService, bigArrays, threadPool, odsks, namedXContentRegistry, dispatcher, sslExceptionHandler, clusterSettings, sharedGroupFactory); + public static final AttributeKey EARLY_RESPONSE = AttributeKey.newInstance("opensearch-http-early-response"); + public static final AttributeKey CONTEXT_TO_RESTORE = AttributeKey.newInstance( + "opensearch-http-request-thread-context" + ); + public static final AttributeKey SHOULD_DECOMPRESS = AttributeKey.newInstance("opensearch-http-should-decompress"); + public static final AttributeKey IS_AUTHENTICATED = AttributeKey.newInstance("opensearch-http-is-authenticated"); + + public SecurityHttpServerTransport( + final Settings settings, + final NetworkService networkService, + final BigArrays bigArrays, + final ThreadPool threadPool, + final SecurityKeyStore odsks, + final SslExceptionHandler sslExceptionHandler, + final NamedXContentRegistry namedXContentRegistry, + final ValidatingDispatcher dispatcher, + final ClusterSettings clusterSettings, + SharedGroupFactory sharedGroupFactory, + SecurityRestFilter restFilter + ) { + super( + settings, + networkService, + bigArrays, + threadPool, + odsks, + namedXContentRegistry, + dispatcher, + sslExceptionHandler, + clusterSettings, + sharedGroupFactory, + restFilter + ); } } diff --git a/src/main/java/org/opensearch/security/http/SecurityNonSslHttpServerTransport.java b/src/main/java/org/opensearch/security/http/SecurityNonSslHttpServerTransport.java index fba77fb2ac..481384f290 100644 --- a/src/main/java/org/opensearch/security/http/SecurityNonSslHttpServerTransport.java +++ b/src/main/java/org/opensearch/security/http/SecurityNonSslHttpServerTransport.java @@ -30,6 +30,10 @@ package org.opensearch.security.http; +import io.netty.channel.Channel; +import io.netty.channel.ChannelHandler; + +import io.netty.channel.ChannelInboundHandlerAdapter; import org.opensearch.common.network.NetworkService; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; @@ -37,19 +41,40 @@ import org.opensearch.common.xcontent.NamedXContentRegistry; import org.opensearch.http.HttpHandlingSettings; import org.opensearch.http.netty4.Netty4HttpServerTransport; +import org.opensearch.security.filter.SecurityRestFilter; +import org.opensearch.security.ssl.http.netty.Netty4ConditionalDecompressor; +import org.opensearch.security.ssl.http.netty.Netty4HttpRequestHeaderVerifier; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.SharedGroupFactory; -import io.netty.channel.Channel; -import io.netty.channel.ChannelHandler; - public class SecurityNonSslHttpServerTransport extends Netty4HttpServerTransport { + private final ChannelInboundHandlerAdapter headerVerifier; + private final ChannelInboundHandlerAdapter conditionalDecompressor; - public SecurityNonSslHttpServerTransport(final Settings settings, final NetworkService networkService, final BigArrays bigArrays, - final ThreadPool threadPool, final NamedXContentRegistry namedXContentRegistry, final Dispatcher dispatcher, - ClusterSettings clusterSettings, SharedGroupFactory sharedGroupFactory) { - super(settings, networkService, bigArrays, threadPool, namedXContentRegistry, dispatcher, clusterSettings, sharedGroupFactory); + public SecurityNonSslHttpServerTransport( + final Settings settings, + final NetworkService networkService, + final BigArrays bigArrays, + final ThreadPool threadPool, + final NamedXContentRegistry namedXContentRegistry, + final Dispatcher dispatcher, + final ClusterSettings clusterSettings, + final SharedGroupFactory sharedGroupFactory, + final SecurityRestFilter restFilter + ) { + super( + settings, + networkService, + bigArrays, + threadPool, + namedXContentRegistry, + dispatcher, + clusterSettings, + sharedGroupFactory + ); + headerVerifier = new Netty4HttpRequestHeaderVerifier(restFilter, threadPool, settings); + conditionalDecompressor = new Netty4ConditionalDecompressor(); } @Override @@ -68,4 +93,14 @@ protected void initChannel(Channel ch) throws Exception { super.initChannel(ch); } } + + @Override + protected ChannelInboundHandlerAdapter createHeaderVerifier() { + return headerVerifier; + } + + @Override + protected ChannelInboundHandlerAdapter createDecompressor() { + return conditionalDecompressor; + } } diff --git a/src/main/java/org/opensearch/security/http/XFFResolver.java b/src/main/java/org/opensearch/security/http/XFFResolver.java index 6b2cbbc7ee..8f4fb0c869 100644 --- a/src/main/java/org/opensearch/security/http/XFFResolver.java +++ b/src/main/java/org/opensearch/security/http/XFFResolver.java @@ -36,11 +36,8 @@ import org.apache.logging.log4j.LogManager; import org.opensearch.OpenSearchSecurityException; import org.opensearch.common.transport.TransportAddress; -import org.opensearch.http.netty4.Netty4HttpChannel; -import org.opensearch.rest.RestRequest; import org.opensearch.common.util.concurrent.ThreadContext; import org.opensearch.security.filter.SecurityRequest; -import org.opensearch.security.filter.OpenSearchRequest; import org.opensearch.security.securityconf.DynamicConfigModel; import org.opensearch.security.support.ConfigConstants; import org.opensearch.threadpool.ThreadPool; @@ -63,16 +60,8 @@ public TransportAddress resolve(final SecurityRequest request) throws OpenSearch if (isTraceEnabled) { log.trace("resolve {}", request.getRemoteAddress().orElse(null)); } - - boolean requestFromNetty = false; - if (request instanceof OpenSearchRequest) { - final OpenSearchRequest securityRequestChannel = (OpenSearchRequest) request; - final RestRequest restRequest = securityRequestChannel.breakEncapsulationForRequest(); - - requestFromNetty = restRequest.getHttpChannel() instanceof Netty4HttpChannel; - } - if (enabled && request.getRemoteAddress().isPresent() && requestFromNetty) { + if (enabled && request.getRemoteAddress().isPresent()) { final InetSocketAddress remoteAddress = request.getRemoteAddress().get(); final InetSocketAddress isa = new InetSocketAddress(detector.detect(request, threadContext), remoteAddress.getPort()); diff --git a/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java b/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java index b1042b3fd2..a12da4eafc 100644 --- a/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java +++ b/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java @@ -74,6 +74,8 @@ import org.opensearch.rest.RestController; import org.opensearch.rest.RestHandler; import org.opensearch.script.ScriptService; +import org.opensearch.security.filter.SecurityRestFilter; +import org.opensearch.security.ssl.http.netty.ValidatingDispatcher; import org.opensearch.security.ssl.rest.SecuritySSLInfoAction; import org.opensearch.security.ssl.transport.*; import org.opensearch.security.ssl.util.SSLConfigConstants; @@ -83,7 +85,6 @@ import org.opensearch.transport.TransportInterceptor; import org.opensearch.watcher.ResourceWatcherService; -import org.opensearch.security.ssl.http.netty.ValidatingDispatcher; import org.opensearch.security.ssl.transport.PrincipalExtractor; import org.opensearch.security.ssl.transport.SecuritySSLTransportInterceptor; @@ -93,12 +94,13 @@ public class OpenSearchSecuritySSLPlugin extends Plugin implements SystemIndexPl private static boolean USE_NETTY_DEFAULT_ALLOCATOR = Booleans.parseBoolean(System.getProperty("opensearch.unsafe.use_netty_default_allocator"), false); public static final boolean OPENSSL_SUPPORTED = (PlatformDependent.javaVersion() < 12) && USE_NETTY_DEFAULT_ALLOCATOR; protected final Logger log = LogManager.getLogger(this.getClass()); - protected static final String CLIENT_TYPE = "client.type"; + public static final String CLIENT_TYPE = "client.type"; protected final boolean client; protected final boolean httpSSLEnabled; protected final boolean transportSSLEnabled; protected final boolean extendedKeyUsageEnabled; protected final Settings settings; + protected volatile SecurityRestFilter securityRestHandler; protected final SharedGroupFactory sharedGroupFactory; protected final SecurityKeyStore sks; protected PrincipalExtractor principalExtractor; @@ -225,12 +227,27 @@ public Map> getHttpTransports(Settings set NetworkService networkService, Dispatcher dispatcher, ClusterSettings clusterSettings) { if (!client && httpSSLEnabled) { - - final ValidatingDispatcher validatingDispatcher = new ValidatingDispatcher(threadPool.getThreadContext(), dispatcher, settings, configPath, NOOP_SSL_EXCEPTION_HANDLER); - final SecuritySSLNettyHttpServerTransport sgsnht = - new SecuritySSLNettyHttpServerTransport(settings, networkService, bigArrays, threadPool, - sks, xContentRegistry, validatingDispatcher, NOOP_SSL_EXCEPTION_HANDLER, clusterSettings, - sharedGroupFactory); + + final ValidatingDispatcher validatingDispatcher = new ValidatingDispatcher( + threadPool.getThreadContext(), + dispatcher, + settings, + configPath, + NOOP_SSL_EXCEPTION_HANDLER + ); + final SecuritySSLNettyHttpServerTransport sgsnht = new SecuritySSLNettyHttpServerTransport( + settings, + networkService, + bigArrays, + threadPool, + sks, + xContentRegistry, + validatingDispatcher, + NOOP_SSL_EXCEPTION_HANDLER, + clusterSettings, + sharedGroupFactory, + securityRestHandler + ); return Collections.singletonMap("org.opensearch.security.ssl.http.netty.SecuritySSLNettyHttpServerTransport", () -> sgsnht); diff --git a/src/main/java/org/opensearch/security/ssl/http/netty/Netty4ConditionalDecompressor.java b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4ConditionalDecompressor.java new file mode 100644 index 0000000000..c8059fad5d --- /dev/null +++ b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4ConditionalDecompressor.java @@ -0,0 +1,37 @@ +/* + * 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.ssl.http.netty; + +import io.netty.channel.ChannelHandler.Sharable; +import io.netty.channel.embedded.EmbeddedChannel; +import io.netty.handler.codec.http.HttpContentDecompressor; + +import static org.opensearch.security.http.SecurityHttpServerTransport.EARLY_RESPONSE; +import static org.opensearch.security.http.SecurityHttpServerTransport.SHOULD_DECOMPRESS; + +import org.opensearch.security.filter.NettyAttribute; + +@Sharable +public class Netty4ConditionalDecompressor extends HttpContentDecompressor { + + @Override + protected EmbeddedChannel newContentDecoder(String contentEncoding) throws Exception { + final boolean hasAnEarlyReponse = NettyAttribute.peekFrom(ctx, EARLY_RESPONSE).isPresent(); + final boolean shouldDecompress = NettyAttribute.popFrom(ctx, SHOULD_DECOMPRESS).orElse(false); + if (hasAnEarlyReponse || !shouldDecompress) { + // If there was an error prompting an early response,... don't decompress + // If there is no explicit decompress flag,... don't decompress + // If there is a decompress flag and it is false,... don't decompress + return super.newContentDecoder("identity"); + } + + // Decompresses the content based on its encoding + return super.newContentDecoder(contentEncoding); + } +} diff --git a/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java new file mode 100644 index 0000000000..2a9a9ac273 --- /dev/null +++ b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java @@ -0,0 +1,137 @@ +/* + * 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.ssl.http.netty; + +import io.netty.channel.SimpleChannelInboundHandler; +import io.netty.handler.codec.http.DefaultHttpRequest; +import io.netty.handler.codec.http.HttpMethod; +import io.netty.handler.codec.http.HttpRequest; +import io.netty.util.ReferenceCountUtil; +import org.opensearch.ExceptionsHelper; +import org.opensearch.common.util.concurrent.ThreadContext; + +import io.netty.channel.ChannelHandler.Sharable; +import io.netty.channel.ChannelHandlerContext; +import org.opensearch.http.netty4.Netty4HttpChannel; +import org.opensearch.http.netty4.Netty4HttpServerTransport; +import org.opensearch.rest.RestUtils; +import org.opensearch.security.filter.SecurityRequestChannel; +import org.opensearch.security.filter.SecurityRequestChannelUnsupported; +import org.opensearch.security.filter.SecurityRequestFactory; +import org.opensearch.security.filter.SecurityResponse; +import org.opensearch.security.filter.SecurityRestFilter; +import org.opensearch.security.filter.SecurityRestUtils; +import org.opensearch.security.ssl.transport.SSLConfig; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.security.support.ConfigConstants; +import org.opensearch.security.ssl.OpenSearchSecuritySSLPlugin; +import org.opensearch.common.settings.Settings; +import org.opensearch.OpenSearchSecurityException; + +import java.util.regex.Matcher; + +import static com.amazon.dlic.auth.http.saml.HTTPSamlAuthenticator.API_AUTHTOKEN_SUFFIX; +import static org.opensearch.security.filter.SecurityRestFilter.HEALTH_SUFFIX; +import static org.opensearch.security.filter.SecurityRestFilter.PATTERN_PATH_PREFIX; +import static org.opensearch.security.http.SecurityHttpServerTransport.CONTEXT_TO_RESTORE; +import static org.opensearch.security.http.SecurityHttpServerTransport.EARLY_RESPONSE; +import static org.opensearch.security.http.SecurityHttpServerTransport.SHOULD_DECOMPRESS; +import static org.opensearch.security.http.SecurityHttpServerTransport.IS_AUTHENTICATED; + +@Sharable +public class Netty4HttpRequestHeaderVerifier extends SimpleChannelInboundHandler { + private final SecurityRestFilter restFilter; + private final ThreadPool threadPool; + private final SSLConfig sslConfig; + private final boolean injectUserEnabled; + private final boolean passthrough; + + public Netty4HttpRequestHeaderVerifier(SecurityRestFilter restFilter, ThreadPool threadPool, Settings settings) { + this.restFilter = restFilter; + this.threadPool = threadPool; + + this.injectUserEnabled = settings.getAsBoolean(ConfigConstants.SECURITY_UNSUPPORTED_INJECT_USER_ENABLED, false); + boolean disabled = settings.getAsBoolean(ConfigConstants.SECURITY_DISABLED, false); + if (disabled) { + sslConfig = new SSLConfig(false, false); + } else { + sslConfig = new SSLConfig(settings); + } + boolean client = !"node".equals(settings.get(OpenSearchSecuritySSLPlugin.CLIENT_TYPE)); + this.passthrough = client || disabled || sslConfig.isSslOnlyMode(); + } + + @Override + public void channelRead0(ChannelHandlerContext ctx, DefaultHttpRequest msg) throws Exception { + // DefaultHttpRequest should always be first and contain headers + ReferenceCountUtil.retain(msg); + + if (passthrough) { + ctx.fireChannelRead(msg); + return; + } + + // Start by setting this value to false, only requests that meet all the criteria will be decompressed + ctx.channel().attr(SHOULD_DECOMPRESS).set(Boolean.FALSE); + ctx.channel().attr(IS_AUTHENTICATED).set(Boolean.FALSE); + + final Netty4HttpChannel httpChannel = ctx.channel().attr(Netty4HttpServerTransport.HTTP_CHANNEL_KEY).get(); + String rawPath = SecurityRestUtils.path(msg.uri()); + String path = RestUtils.decodeComponent(rawPath); + Matcher matcher = PATTERN_PATH_PREFIX.matcher(path); + final String suffix = matcher.matches() ? matcher.group(2) : null; + if (API_AUTHTOKEN_SUFFIX.equals(suffix)) { + ctx.fireChannelRead(msg); + return; + } + + final SecurityRequestChannel requestChannel = SecurityRequestFactory.from(msg, httpChannel); + ThreadContext threadContext = threadPool.getThreadContext(); + try (ThreadContext.StoredContext ignore = threadPool.getThreadContext().stashContext()) { + injectUser(msg, threadContext); + + boolean shouldSkipAuthentication = HttpMethod.OPTIONS.equals(msg.method()) + || HEALTH_SUFFIX.equals(suffix); + + if (!shouldSkipAuthentication) { + // If request channel is completed and a response is sent, then there was a failure during authentication + restFilter.checkAndAuthenticateRequest(requestChannel); + } + + ThreadContext.StoredContext contextToRestore = threadPool.getThreadContext().newStoredContext(false); + ctx.channel().attr(CONTEXT_TO_RESTORE).set(contextToRestore); + + requestChannel.getQueuedResponse().ifPresent(response -> ctx.channel().attr(EARLY_RESPONSE).set(response)); + + boolean shouldDecompress = !shouldSkipAuthentication && requestChannel.getQueuedResponse().isEmpty(); + + if (requestChannel.getQueuedResponse().isEmpty() || shouldSkipAuthentication) { + // Only allow decompression on authenticated requests that also aren't one of those ^ + ctx.channel().attr(SHOULD_DECOMPRESS).set(Boolean.valueOf(shouldDecompress)); + ctx.channel().attr(IS_AUTHENTICATED).set(Boolean.TRUE); + } + } catch (final OpenSearchSecurityException e) { + final SecurityResponse earlyResponse = new SecurityResponse(ExceptionsHelper.status(e).getStatus(), e); + ctx.channel().attr(EARLY_RESPONSE).set(earlyResponse); + } catch (final SecurityRequestChannelUnsupported srcu) { + // Use defaults for unsupported channels + } finally { + ctx.fireChannelRead(msg); + } + } + + private void injectUser(HttpRequest request, ThreadContext threadContext) { + if (this.injectUserEnabled) { + threadContext.putTransient( + ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, + request.headers().get(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER) + ); + } + } +} diff --git a/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java b/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java index 197c587f5e..a35a4550c3 100644 --- a/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java +++ b/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java @@ -17,8 +17,13 @@ package org.opensearch.security.ssl.http.netty; -import org.apache.logging.log4j.Logger; +import io.netty.channel.Channel; +import io.netty.channel.ChannelHandler; +import io.netty.channel.ChannelInboundHandlerAdapter; +import io.netty.handler.codec.DecoderException; +import io.netty.handler.ssl.SslHandler; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.opensearch.common.network.NetworkService; import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; @@ -27,28 +32,47 @@ import org.opensearch.http.HttpChannel; import org.opensearch.http.HttpHandlingSettings; import org.opensearch.http.netty4.Netty4HttpServerTransport; +import org.opensearch.security.filter.SecurityRestFilter; import org.opensearch.security.ssl.SecurityKeyStore; import org.opensearch.security.ssl.SslExceptionHandler; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.SharedGroupFactory; -import io.netty.channel.Channel; -import io.netty.channel.ChannelHandler; -import io.netty.handler.codec.DecoderException; -import io.netty.handler.ssl.SslHandler; - public class SecuritySSLNettyHttpServerTransport extends Netty4HttpServerTransport { private static final Logger logger = LogManager.getLogger(SecuritySSLNettyHttpServerTransport.class); private final SecurityKeyStore sks; private final SslExceptionHandler errorHandler; - - public SecuritySSLNettyHttpServerTransport(final Settings settings, final NetworkService networkService, final BigArrays bigArrays, - final ThreadPool threadPool, final SecurityKeyStore sks, final NamedXContentRegistry namedXContentRegistry, final ValidatingDispatcher dispatcher, - final SslExceptionHandler errorHandler, ClusterSettings clusterSettings, SharedGroupFactory sharedGroupFactory) { - super(settings, networkService, bigArrays, threadPool, namedXContentRegistry, dispatcher, clusterSettings, sharedGroupFactory); + private final ChannelInboundHandlerAdapter headerVerifier; + private final ChannelInboundHandlerAdapter conditionalDecompressor; + + public SecuritySSLNettyHttpServerTransport( + final Settings settings, + final NetworkService networkService, + final BigArrays bigArrays, + final ThreadPool threadPool, + final SecurityKeyStore sks, + final NamedXContentRegistry namedXContentRegistry, + final ValidatingDispatcher dispatcher, + final SslExceptionHandler errorHandler, + ClusterSettings clusterSettings, + SharedGroupFactory sharedGroupFactory, + SecurityRestFilter restFilter + ) { + super( + settings, + networkService, + bigArrays, + threadPool, + namedXContentRegistry, + dispatcher, + clusterSettings, + sharedGroupFactory + ); this.sks = sks; this.errorHandler = errorHandler; + headerVerifier = new Netty4HttpRequestHeaderVerifier(restFilter, threadPool, settings); + conditionalDecompressor = new Netty4ConditionalDecompressor(); } @Override @@ -84,4 +108,14 @@ protected void initChannel(Channel ch) throws Exception { ch.pipeline().addFirst("ssl_http", sslHandler); } } + + @Override + protected ChannelInboundHandlerAdapter createHeaderVerifier() { + return headerVerifier; + } + + @Override + protected ChannelInboundHandlerAdapter createDecompressor() { + return conditionalDecompressor; + } } diff --git a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java index 779e528cf5..01be96dc12 100644 --- a/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java +++ b/src/test/java/org/opensearch/security/auditlog/integration/BasicAuditlogTest.java @@ -530,13 +530,16 @@ public void testUpdateSettings() throws Exception { "}"+ "}"; + String expectedRequestBodyLog = + "{\\\"persistent_settings\\\":{\\\"indices\\\":{\\\"recovery\\\":{\\\"*\\\":null}}},\\\"transient_settings\\\":{\\\"indices\\\":{\\\"recovery\\\":{\\\"*\\\":null}}}}"; + HttpResponse response = rh.executePutRequest("_cluster/settings", json, encodeBasicHeader("admin", "admin")); Assert.assertEquals(HttpStatus.SC_OK, response.getStatusCode()); - System.out.println(TestAuditlogImpl.sb.toString()); - Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("AUTHENTICATED")); - Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("cluster:admin/settings/update")); - Assert.assertTrue(TestAuditlogImpl.sb.toString().contains("indices.recovery.*")); - //may vary because we log may hit cluster manager directly or not + String auditLogImpl = TestAuditlogImpl.sb.toString(); + Assert.assertTrue(auditLogImpl.contains("AUTHENTICATED")); + Assert.assertTrue(auditLogImpl.contains("cluster:admin/settings/update")); + Assert.assertTrue(auditLogImpl.contains(expectedRequestBodyLog)); + // may vary because we log may hit cluster manager directly or not Assert.assertTrue(TestAuditlogImpl.messages.size() > 1); Assert.assertTrue(validateMsgs(TestAuditlogImpl.messages)); } diff --git a/src/test/java/org/opensearch/security/filter/SecurityRestFilterUnitTests.java b/src/test/java/org/opensearch/security/filter/SecurityRestFilterUnitTests.java new file mode 100644 index 0000000000..a1f1efd38e --- /dev/null +++ b/src/test/java/org/opensearch/security/filter/SecurityRestFilterUnitTests.java @@ -0,0 +1,109 @@ +package org.opensearch.security.filter; + +import org.junit.Before; +import org.junit.Ignore; +import org.junit.Test; +import org.opensearch.client.node.NodeClient; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.concurrent.ThreadContext; +import org.opensearch.common.bytes.BytesArray; +import org.opensearch.rest.RestStatus; +import org.opensearch.common.xcontent.NamedXContentRegistry; +import org.opensearch.rest.BytesRestResponse; +import org.opensearch.rest.RestChannel; +import org.opensearch.rest.RestHandler; +import org.opensearch.rest.RestRequest; +import org.opensearch.security.auditlog.AuditLog; +import org.opensearch.security.auth.BackendRegistry; +import org.opensearch.security.configuration.AdminDNs; +import org.opensearch.security.configuration.CompatConfig; +import org.opensearch.security.ssl.transport.PrincipalExtractor; +import org.opensearch.security.util.FakeRestRequest; +import org.opensearch.threadpool.ThreadPool; + +import java.nio.file.Path; +import java.util.List; +import java.util.Map; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; + +public class SecurityRestFilterUnitTests { + + SecurityRestFilter sf; + RestHandler testRestHandler; + + class TestRestHandler implements RestHandler { + + @Override + public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { + channel.sendResponse(new BytesRestResponse(RestStatus.OK, BytesRestResponse.TEXT_CONTENT_TYPE, BytesArray.EMPTY)); + } + } + + @Before + public void setUp() throws NoSuchMethodException { + testRestHandler = new TestRestHandler(); + + ThreadPool tp = spy(new ThreadPool(Settings.builder().put("node.name", "mock").build())); + doReturn(new ThreadContext(Settings.EMPTY)).when(tp).getThreadContext(); + + sf = new SecurityRestFilter( + mock(BackendRegistry.class), + mock(AuditLog.class), + tp, + mock(PrincipalExtractor.class), + Settings.EMPTY, + mock(Path.class), + mock(CompatConfig.class) + ); + } + + @Ignore + @Test + public void testDoesCallDelegateOnSuccessfulAuthorization() throws Exception { + SecurityRestFilter filterSpy = spy(sf); + AdminDNs adminDNs = mock(AdminDNs.class); + + RestHandler testRestHandlerSpy = spy(testRestHandler); + RestHandler wrappedRestHandler = filterSpy.wrap(testRestHandlerSpy, adminDNs); + + doReturn(false).when(filterSpy).userIsSuperAdmin(any(), any()); + // doReturn(true).when(filterSpy).authorizeRequest(any(), any(), any()); + + FakeRestRequest fakeRequest = new FakeRestRequest.Builder().withPath("/test") + .withMethod(RestRequest.Method.POST) + .withHeaders(Map.of("Content-Type", "application/json")) + .build(); + + wrappedRestHandler.handleRequest(fakeRequest, mock(RestChannel.class), mock(NodeClient.class)); + + verify(testRestHandlerSpy).handleRequest(any(), any(), any()); + } + + @Ignore + @Test + public void testDoesNotCallDelegateOnUnauthorized() throws Exception { + SecurityRestFilter filterSpy = spy(sf); + AdminDNs adminDNs = mock(AdminDNs.class); + + RestHandler testRestHandlerSpy = spy(testRestHandler); + RestHandler wrappedRestHandler = filterSpy.wrap(testRestHandlerSpy, adminDNs); + + doReturn(false).when(filterSpy).userIsSuperAdmin(any(), any()); + // doReturn(false).when(filterSpy).authorizeRequest(any(), any(), any()); + + FakeRestRequest fakeRequest = new FakeRestRequest.Builder().withPath("/test") + .withMethod(RestRequest.Method.POST) + .withHeaders(Map.of("Content-Type", "application/json")) + .build(); + + wrappedRestHandler.handleRequest(fakeRequest, mock(RestChannel.class), mock(NodeClient.class)); + + verify(testRestHandlerSpy, never()).handleRequest(any(), any(), any()); + } +} diff --git a/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java b/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java index 2546b4e26f..8b254a8d72 100644 --- a/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java +++ b/src/test/java/org/opensearch/security/test/helper/cluster/ClusterConfiguration.java @@ -37,6 +37,7 @@ import java.util.stream.Collectors; import org.opensearch.security.OpenSearchSecurityPlugin; +import org.opensearch.security.TransportUserInjectorIntegTest.UserInjectorPlugin; import org.opensearch.index.reindex.ReindexPlugin; import org.opensearch.join.ParentJoinPlugin; import org.opensearch.percolator.PercolatorPlugin; @@ -45,7 +46,6 @@ import org.opensearch.search.aggregations.matrix.MatrixAggregationPlugin; import org.opensearch.transport.Netty4Plugin; -import org.opensearch.security.test.plugin.UserInjectorPlugin; import com.google.common.collect.Lists; public enum ClusterConfiguration { diff --git a/src/test/java/org/opensearch/security/test/plugin/UserInjectorPlugin.java b/src/test/java/org/opensearch/security/test/plugin/UserInjectorPlugin.java deleted file mode 100644 index 43e4e8869f..0000000000 --- a/src/test/java/org/opensearch/security/test/plugin/UserInjectorPlugin.java +++ /dev/null @@ -1,116 +0,0 @@ -/* - * Copyright 2015-2018 _floragunn_ GmbH - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -/* - * Portions Copyright OpenSearch Contributors - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package org.opensearch.security.test.plugin; - -import java.nio.file.Path; -import java.util.Map; -import java.util.function.Supplier; - -import org.opensearch.common.network.NetworkService; -import org.opensearch.common.settings.ClusterSettings; -import org.opensearch.common.settings.Settings; -import org.opensearch.common.util.BigArrays; -import org.opensearch.common.util.PageCacheRecycler; -import org.opensearch.common.util.concurrent.ThreadContext; -import org.opensearch.common.xcontent.NamedXContentRegistry; -import org.opensearch.http.HttpServerTransport; -import org.opensearch.http.HttpServerTransport.Dispatcher; -import org.opensearch.http.netty4.Netty4HttpServerTransport; -import org.opensearch.indices.breaker.CircuitBreakerService; -import org.opensearch.plugins.NetworkPlugin; -import org.opensearch.plugins.Plugin; -import org.opensearch.rest.RestChannel; -import org.opensearch.rest.RestRequest; -import org.opensearch.security.support.ConfigConstants; -import org.opensearch.threadpool.ThreadPool; -import org.opensearch.transport.SharedGroupFactory; - -import com.google.common.collect.ImmutableMap; - -/** - * Mimics the behavior of system integrators that run their own plugins (i.e. server transports) - * in front of OpenSearch Security. This transport just copies the user string from the - * REST headers to the ThreadContext to test user injection. - * @author jkressin - */ -public class UserInjectorPlugin extends Plugin implements NetworkPlugin { - - Settings settings; - private final SharedGroupFactory sharedGroupFactory; - ThreadPool threadPool; - - public UserInjectorPlugin(final Settings settings, final Path configPath) { - this.settings = settings; - sharedGroupFactory = new SharedGroupFactory(settings); - } - - @Override - public Map> getHttpTransports(Settings settings, ThreadPool threadPool, BigArrays bigArrays, - PageCacheRecycler pageCacheRecycler, CircuitBreakerService circuitBreakerService, NamedXContentRegistry xContentRegistry, - NetworkService networkService, Dispatcher dispatcher, ClusterSettings clusterSettings) { - - final UserInjectingDispatcher validatingDispatcher = new UserInjectingDispatcher(dispatcher); - return ImmutableMap.of("org.opensearch.security.http.UserInjectingServerTransport", - () -> new UserInjectingServerTransport(settings, networkService, bigArrays, threadPool, xContentRegistry, validatingDispatcher, clusterSettings, sharedGroupFactory)); - } - - class UserInjectingServerTransport extends Netty4HttpServerTransport { - - public UserInjectingServerTransport(final Settings settings, final NetworkService networkService, final BigArrays bigArrays, - final ThreadPool threadPool, final NamedXContentRegistry namedXContentRegistry, final Dispatcher dispatcher, ClusterSettings clusterSettings, SharedGroupFactory sharedGroupFactory) { - super(settings, networkService, bigArrays, threadPool, namedXContentRegistry, dispatcher, clusterSettings, sharedGroupFactory); - } - } - - class UserInjectingDispatcher implements Dispatcher { - - private Dispatcher originalDispatcher; - - public UserInjectingDispatcher(final Dispatcher originalDispatcher) { - super(); - this.originalDispatcher = originalDispatcher; - } - - @Override - public void dispatchRequest(RestRequest request, RestChannel channel, ThreadContext threadContext) { - threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, request.header(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER)); - originalDispatcher.dispatchRequest(request, channel, threadContext); - - } - - @Override - public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, Throwable cause) { - threadContext.putTransient(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER, channel.request().header(ConfigConstants.OPENDISTRO_SECURITY_INJECTED_USER)); - originalDispatcher.dispatchBadRequest(channel, threadContext, cause); - } - } - -}