diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index a59d1f531d..6e3c22e695 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -107,6 +107,7 @@ import org.opensearch.extensions.ExtensionsManager; import org.opensearch.http.HttpServerTransport; import org.opensearch.http.HttpServerTransport.Dispatcher; +import org.opensearch.http.netty4.ssl.SecureNetty4HttpServerTransport; import org.opensearch.identity.Subject; import org.opensearch.identity.noop.NoopSubject; import org.opensearch.index.IndexModule; @@ -117,6 +118,7 @@ import org.opensearch.plugins.ExtensionAwarePlugin; import org.opensearch.plugins.IdentityPlugin; import org.opensearch.plugins.MapperPlugin; +import org.opensearch.plugins.SecureHttpTransportSettingsProvider; import org.opensearch.plugins.SecureSettingsFactory; import org.opensearch.plugins.SecureTransportSettingsProvider; import org.opensearch.repositories.RepositoriesService; @@ -156,7 +158,6 @@ import org.opensearch.security.filter.SecurityFilter; import org.opensearch.security.filter.SecurityRestFilter; import org.opensearch.security.http.NonSslHttpServerTransport; -import org.opensearch.security.http.SecureHttpServerTransport; import org.opensearch.security.http.XFFResolver; import org.opensearch.security.identity.SecurityTokenManager; import org.opensearch.security.privileges.PrivilegesEvaluator; @@ -239,7 +240,6 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin private volatile PrivilegesEvaluator evaluator; private volatile UserService userService; private volatile RestLayerPrivilegesEvaluator restLayerEvaluator; - private volatile ThreadPool threadPool; private volatile ConfigurationRepository cr; private volatile AdminDNs adminDns; private volatile ClusterService cs; @@ -927,7 +927,7 @@ public Map> getSecureHttpTransports( NetworkService networkService, Dispatcher dispatcher, ClusterSettings clusterSettings, - SecureTransportSettingsProvider secureTransportSettingsProvider, + SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider, Tracer tracer ) { @@ -942,7 +942,7 @@ public Map> getSecureHttpTransports( networkService, dispatcher, clusterSettings, - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, tracer ); } @@ -958,7 +958,7 @@ public Map> getSecureHttpTransports( evaluateSslExceptionHandler() ); // TODO close odshst - final SecureHttpServerTransport odshst = new SecureHttpServerTransport( + final SecureNetty4HttpServerTransport odshst = new SecureNetty4HttpServerTransport( migrateSettings(settings), networkService, bigArrays, @@ -967,9 +967,8 @@ public Map> getSecureHttpTransports( validatingDispatcher, clusterSettings, sharedGroupFactory, - secureTransportSettingsProvider, - tracer, - securityRestHandler + secureHttpTransportSettingsProvider, + tracer ); return Collections.singletonMap("org.opensearch.security.http.SecurityHttpServerTransport", () -> odshst); @@ -985,9 +984,8 @@ public Map> getSecureHttpTransports( dispatcher, clusterSettings, sharedGroupFactory, - secureTransportSettingsProvider, - tracer, - securityRestHandler + secureHttpTransportSettingsProvider, + tracer ) ); } @@ -2032,7 +2030,7 @@ public SecurityTokenManager getTokenManager() { @Override public Optional getSecureSettingFactory(Settings settings) { - return Optional.of(new OpenSearchSecureSettingsFactory(settings, sks, sslExceptionHandler)); + return Optional.of(new OpenSearchSecureSettingsFactory(threadPool, sks, sslExceptionHandler, securityRestHandler)); } public static class GuiceHolder implements LifecycleComponent { diff --git a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java index 8ccaa3041c..420211f29b 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityRestFilter.java @@ -55,6 +55,7 @@ import org.opensearch.security.privileges.RestLayerPrivilegesEvaluator; import org.opensearch.security.securityconf.impl.AllowlistingSettings; import org.opensearch.security.securityconf.impl.WhitelistingSettings; +import org.opensearch.security.ssl.http.netty.Netty4HttpRequestHeaderVerifier; import org.opensearch.security.ssl.transport.PrincipalExtractor; import org.opensearch.security.ssl.util.ExceptionUtils; import org.opensearch.security.ssl.util.SSLRequestHelper; @@ -69,10 +70,6 @@ import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX; import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX; -import static org.opensearch.security.http.SecureHttpServerTransport.CONTEXT_TO_RESTORE; -import static org.opensearch.security.http.SecureHttpServerTransport.EARLY_RESPONSE; -import static org.opensearch.security.http.SecureHttpServerTransport.IS_AUTHENTICATED; -import static org.opensearch.security.http.SecureHttpServerTransport.UNCONSUMED_PARAMS; public class SecurityRestFilter { @@ -128,15 +125,18 @@ public AuthczRestHandler(RestHandler original, AdminDNs adminDNs) { @Override public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception { - final Optional maybeSavedResponse = NettyAttribute.popFrom(request, EARLY_RESPONSE); + final Optional maybeSavedResponse = NettyAttribute.popFrom( + request, + Netty4HttpRequestHeaderVerifier.EARLY_RESPONSE + ); if (maybeSavedResponse.isPresent()) { - NettyAttribute.clearAttribute(request, CONTEXT_TO_RESTORE); - NettyAttribute.clearAttribute(request, IS_AUTHENTICATED); + NettyAttribute.clearAttribute(request, Netty4HttpRequestHeaderVerifier.CONTEXT_TO_RESTORE); + NettyAttribute.clearAttribute(request, Netty4HttpRequestHeaderVerifier.IS_AUTHENTICATED); channel.sendResponse(maybeSavedResponse.get().asRestResponse()); return; } - NettyAttribute.popFrom(request, CONTEXT_TO_RESTORE).ifPresent(storedContext -> { + NettyAttribute.popFrom(request, Netty4HttpRequestHeaderVerifier.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(); @@ -145,7 +145,7 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c } }); - NettyAttribute.popFrom(request, UNCONSUMED_PARAMS).ifPresent(unconsumedParams -> { + NettyAttribute.popFrom(request, Netty4HttpRequestHeaderVerifier.UNCONSUMED_PARAMS).ifPresent(unconsumedParams -> { for (String unconsumedParam : unconsumedParams) { // Consume the parameter on the RestRequest request.param(unconsumedParam); @@ -155,7 +155,7 @@ public void handleRequest(RestRequest request, RestChannel channel, NodeClient c final SecurityRequestChannel requestChannel = SecurityRequestFactory.from(request, channel); // Authenticate request - if (!NettyAttribute.popFrom(request, IS_AUTHENTICATED).orElse(false)) { + if (!NettyAttribute.popFrom(request, Netty4HttpRequestHeaderVerifier.IS_AUTHENTICATED).orElse(false)) { // we aren't authenticated so we should skip this step checkAndAuthenticateRequest(requestChannel); } diff --git a/src/main/java/org/opensearch/security/http/NonSslHttpServerTransport.java b/src/main/java/org/opensearch/security/http/NonSslHttpServerTransport.java index c97d872aca..d0d1d9f09f 100644 --- a/src/main/java/org/opensearch/security/http/NonSslHttpServerTransport.java +++ b/src/main/java/org/opensearch/security/http/NonSslHttpServerTransport.java @@ -34,22 +34,15 @@ import org.opensearch.http.HttpHandlingSettings; import org.opensearch.http.netty4.Netty4HttpServerTransport; import org.opensearch.http.netty4.ssl.SecureNetty4HttpServerTransport; -import org.opensearch.plugins.SecureTransportSettingsProvider; -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.plugins.SecureHttpTransportSettingsProvider; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.SharedGroupFactory; import io.netty.channel.Channel; import io.netty.channel.ChannelHandler; -import io.netty.channel.ChannelInboundHandlerAdapter; public class NonSslHttpServerTransport extends SecureNetty4HttpServerTransport { - - private final ChannelInboundHandlerAdapter headerVerifier; - public NonSslHttpServerTransport( final Settings settings, final NetworkService networkService, @@ -59,9 +52,8 @@ public NonSslHttpServerTransport( final Dispatcher dispatcher, final ClusterSettings clusterSettings, final SharedGroupFactory sharedGroupFactory, - final SecureTransportSettingsProvider secureTransportSettingsProvider, - final Tracer tracer, - final SecurityRestFilter restFilter + final SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider, + final Tracer tracer ) { super( settings, @@ -72,10 +64,9 @@ public NonSslHttpServerTransport( dispatcher, clusterSettings, sharedGroupFactory, - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, tracer ); - headerVerifier = new Netty4HttpRequestHeaderVerifier(restFilter, threadPool, settings); } @Override @@ -94,14 +85,4 @@ protected void initChannel(Channel ch) throws Exception { super.initChannel(ch); } } - - @Override - protected ChannelInboundHandlerAdapter createHeaderVerifier() { - return headerVerifier; - } - - @Override - protected ChannelInboundHandlerAdapter createDecompressor() { - return new Netty4ConditionalDecompressor(); - } } diff --git a/src/main/java/org/opensearch/security/http/SecureHttpServerTransport.java b/src/main/java/org/opensearch/security/http/SecureHttpServerTransport.java deleted file mode 100644 index 170f39ffd6..0000000000 --- a/src/main/java/org/opensearch/security/http/SecureHttpServerTransport.java +++ /dev/null @@ -1,101 +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. - */ - -/* - * 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.http; - -import java.util.Set; - -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.core.xcontent.NamedXContentRegistry; -import org.opensearch.http.netty4.ssl.SecureNetty4HttpServerTransport; -import org.opensearch.plugins.SecureTransportSettingsProvider; -import org.opensearch.security.filter.SecurityResponse; -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.security.ssl.http.netty.ValidatingDispatcher; -import org.opensearch.telemetry.tracing.Tracer; -import org.opensearch.threadpool.ThreadPool; -import org.opensearch.transport.SharedGroupFactory; - -import io.netty.channel.ChannelInboundHandlerAdapter; -import io.netty.util.AttributeKey; - -public class SecureHttpServerTransport extends SecureNetty4HttpServerTransport { - - public static final AttributeKey EARLY_RESPONSE = AttributeKey.newInstance("opensearch-http-early-response"); - public static final AttributeKey> UNCONSUMED_PARAMS = AttributeKey.newInstance("opensearch-http-request-consumed-params"); - 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"); - - private final ChannelInboundHandlerAdapter headerVerifier; - - public SecureHttpServerTransport( - final Settings settings, - final NetworkService networkService, - final BigArrays bigArrays, - final ThreadPool threadPool, - final NamedXContentRegistry namedXContentRegistry, - final ValidatingDispatcher dispatcher, - final ClusterSettings clusterSettings, - SharedGroupFactory sharedGroupFactory, - final SecureTransportSettingsProvider secureTransportSettingsProvider, - Tracer tracer, - SecurityRestFilter restFilter - ) { - super( - settings, - networkService, - bigArrays, - threadPool, - namedXContentRegistry, - dispatcher, - clusterSettings, - sharedGroupFactory, - secureTransportSettingsProvider, - tracer - ); - - headerVerifier = new Netty4HttpRequestHeaderVerifier(restFilter, threadPool, settings); - } - - @Override - protected ChannelInboundHandlerAdapter createHeaderVerifier() { - return headerVerifier; - } - - @Override - protected ChannelInboundHandlerAdapter createDecompressor() { - return new Netty4ConditionalDecompressor(); - } -} diff --git a/src/main/java/org/opensearch/security/ssl/OpenSearchSecureSettingsFactory.java b/src/main/java/org/opensearch/security/ssl/OpenSearchSecureSettingsFactory.java index d85f490d0c..5351eea57e 100644 --- a/src/main/java/org/opensearch/security/ssl/OpenSearchSecureSettingsFactory.java +++ b/src/main/java/org/opensearch/security/ssl/OpenSearchSecureSettingsFactory.java @@ -11,63 +11,124 @@ package org.opensearch.security.ssl; +import java.util.Collection; +import java.util.List; import java.util.Optional; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; import org.opensearch.common.settings.Settings; import org.opensearch.http.HttpServerTransport; +import org.opensearch.http.netty4.ssl.SecureNetty4HttpServerTransport; +import org.opensearch.plugins.SecureHttpTransportSettingsProvider; import org.opensearch.plugins.SecureSettingsFactory; import org.opensearch.plugins.SecureTransportSettingsProvider; -import org.opensearch.transport.TcpTransport; +import org.opensearch.plugins.TransportExceptionHandler; +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.Transport; +import org.opensearch.transport.TransportAdapterProvider; + +import io.netty.channel.ChannelInboundHandlerAdapter; public class OpenSearchSecureSettingsFactory implements SecureSettingsFactory { - private final Settings settings; + private final ThreadPool threadPool; private final SecurityKeyStore sks; private final SslExceptionHandler sslExceptionHandler; + private final SecurityRestFilter restFilter; - public OpenSearchSecureSettingsFactory(Settings settings, SecurityKeyStore sks, SslExceptionHandler sslExceptionHandler) { - this.settings = settings; + public OpenSearchSecureSettingsFactory( + ThreadPool threadPool, + SecurityKeyStore sks, + SslExceptionHandler sslExceptionHandler, + SecurityRestFilter restFilter + ) { + this.threadPool = threadPool; this.sks = sks; this.sslExceptionHandler = sslExceptionHandler; + this.restFilter = restFilter; } @Override public Optional getSecureTransportSettingsProvider(Settings settings) { return Optional.of(new SecureTransportSettingsProvider() { @Override - public Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) { - return Optional.of(new ServerExceptionHandler() { + public Optional buildServerTransportExceptionHandler(Settings settings, Transport transport) { + return Optional.of(new TransportExceptionHandler() { @Override public void onError(Throwable t) { - sslExceptionHandler.logError(t, true); + sslExceptionHandler.logError(t, false); } }); } @Override - public Optional buildServerTransportExceptionHandler(Settings settings, TcpTransport transport) { - return Optional.of(new ServerExceptionHandler() { - @Override - public void onError(Throwable t) { - sslExceptionHandler.logError(t, false); - } - }); + public Optional buildSecureServerTransportEngine(Settings settings, Transport transport) throws SSLException { + return Optional.of(sks.createServerTransportSSLEngine()); } @Override - public Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException { - return Optional.of(sks.createHTTPSSLEngine()); + public Optional buildSecureClientTransportEngine(Settings settings, String hostname, int port) throws SSLException { + return Optional.of(sks.createClientTransportSSLEngine(hostname, port)); } + }); + } + @Override + public Optional getSecureHttpTransportSettingsProvider(Settings settings) { + return Optional.of(new SecureHttpTransportSettingsProvider() { @Override - public Optional buildSecureServerTransportEngine(Settings settings, TcpTransport transport) throws SSLException { - return Optional.of(sks.createServerTransportSSLEngine()); + public Collection> getHttpTransportAdapterProviders(Settings settings) { + return List.of(new TransportAdapterProvider() { + @Override + public String name() { + return SecureNetty4HttpServerTransport.REQUEST_DECOMPRESSOR; + } + + @SuppressWarnings("unchecked") + @Override + public Optional create(Settings settings, HttpServerTransport transport, Class adapterClass) { + if (transport instanceof SecureNetty4HttpServerTransport + && ChannelInboundHandlerAdapter.class.isAssignableFrom(adapterClass)) { + return Optional.of((C) new Netty4ConditionalDecompressor()); + } else { + return Optional.empty(); + } + } + }, new TransportAdapterProvider() { + @Override + public String name() { + return SecureNetty4HttpServerTransport.REQUEST_HEADER_VERIFIER; + } + + @SuppressWarnings("unchecked") + @Override + public Optional create(Settings settings, HttpServerTransport transport, Class adapterClass) { + if (transport instanceof SecureNetty4HttpServerTransport + && ChannelInboundHandlerAdapter.class.isAssignableFrom(adapterClass)) { + return Optional.of((C) new Netty4HttpRequestHeaderVerifier(restFilter, threadPool, settings)); + } else { + return Optional.empty(); + } + } + }); } @Override - public Optional buildSecureClientTransportEngine(Settings settings, String hostname, int port) throws SSLException { - return Optional.of(sks.createClientTransportSSLEngine(hostname, port)); + public Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) { + return Optional.of(new TransportExceptionHandler() { + @Override + public void onError(Throwable t) { + sslExceptionHandler.logError(t, true); + } + }); + } + + @Override + public Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException { + return Optional.of(sks.createHTTPSSLEngine()); } }); } diff --git a/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java b/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java index 3acbce21cf..073193e9d4 100644 --- a/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java +++ b/src/main/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPlugin.java @@ -61,8 +61,10 @@ import org.opensearch.env.NodeEnvironment; import org.opensearch.http.HttpServerTransport; import org.opensearch.http.HttpServerTransport.Dispatcher; +import org.opensearch.http.netty4.ssl.SecureNetty4HttpServerTransport; import org.opensearch.plugins.NetworkPlugin; import org.opensearch.plugins.Plugin; +import org.opensearch.plugins.SecureHttpTransportSettingsProvider; import org.opensearch.plugins.SecureSettingsFactory; import org.opensearch.plugins.SecureTransportSettingsProvider; import org.opensearch.plugins.SystemIndexPlugin; @@ -73,7 +75,6 @@ import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.NonValidatingObjectMapper; import org.opensearch.security.filter.SecurityRestFilter; -import org.opensearch.security.http.SecureHttpServerTransport; import org.opensearch.security.ssl.http.netty.ValidatingDispatcher; import org.opensearch.security.ssl.rest.SecuritySSLInfoAction; import org.opensearch.security.ssl.transport.DefaultPrincipalExtractor; @@ -131,6 +132,7 @@ public class OpenSearchSecuritySSLPlugin extends Plugin implements SystemIndexPl private final static SslExceptionHandler NOOP_SSL_EXCEPTION_HANDLER = new SslExceptionHandler() { }; protected final SSLConfig SSLConfig; + protected volatile ThreadPool threadPool; // public OpenSearchSecuritySSLPlugin(final Settings settings, final Path configPath) { // this(settings, configPath, false); @@ -266,7 +268,7 @@ public Map> getSecureHttpTransports( NetworkService networkService, Dispatcher dispatcher, ClusterSettings clusterSettings, - SecureTransportSettingsProvider secureTransportSettingsProvider, + SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider, Tracer tracer ) { @@ -279,7 +281,7 @@ public Map> getSecureHttpTransports( configPath, NOOP_SSL_EXCEPTION_HANDLER ); - final SecureHttpServerTransport sgsnht = new SecureHttpServerTransport( + final SecureNetty4HttpServerTransport sgsnht = new SecureNetty4HttpServerTransport( migrateSettings(settings), networkService, bigArrays, @@ -288,9 +290,8 @@ public Map> getSecureHttpTransports( validatingDispatcher, clusterSettings, sharedGroupFactory, - secureTransportSettingsProvider, - tracer, - securityRestHandler + secureHttpTransportSettingsProvider, + tracer ); return Collections.singletonMap("org.opensearch.security.ssl.http.netty.SecuritySSLNettyHttpServerTransport", () -> sgsnht); @@ -381,6 +382,7 @@ public Collection createComponents( Supplier repositoriesServiceSupplier ) { + this.threadPool = threadPool; final List components = new ArrayList<>(1); if (client) { @@ -676,7 +678,7 @@ public List getSettingsFilter() { @Override public Optional getSecureSettingFactory(Settings settings) { - return Optional.of(new OpenSearchSecureSettingsFactory(settings, sks, NOOP_SSL_EXCEPTION_HANDLER)); + return Optional.of(new OpenSearchSecureSettingsFactory(threadPool, sks, NOOP_SSL_EXCEPTION_HANDLER, securityRestHandler)); } protected Settings migrateSettings(Settings settings) { 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 index 8b2d4bb1d2..b36a12da48 100644 --- a/src/main/java/org/opensearch/security/ssl/http/netty/Netty4ConditionalDecompressor.java +++ b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4ConditionalDecompressor.java @@ -13,15 +13,12 @@ import io.netty.channel.embedded.EmbeddedChannel; import io.netty.handler.codec.http.HttpContentDecompressor; -import static org.opensearch.security.http.SecureHttpServerTransport.EARLY_RESPONSE; -import static org.opensearch.security.http.SecureHttpServerTransport.SHOULD_DECOMPRESS; - 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); + final boolean hasAnEarlyReponse = NettyAttribute.peekFrom(ctx, Netty4HttpRequestHeaderVerifier.EARLY_RESPONSE).isPresent(); + final boolean shouldDecompress = NettyAttribute.popFrom(ctx, Netty4HttpRequestHeaderVerifier.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 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 index 052f1961e2..9afd6b0e22 100644 --- a/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java +++ b/src/main/java/org/opensearch/security/ssl/http/netty/Netty4HttpRequestHeaderVerifier.java @@ -8,6 +8,8 @@ package org.opensearch.security.ssl.http.netty; +import java.util.Set; + import org.opensearch.ExceptionsHelper; import org.opensearch.OpenSearchSecurityException; import org.opensearch.common.settings.Settings; @@ -30,16 +32,19 @@ import io.netty.channel.SimpleChannelInboundHandler; import io.netty.handler.codec.http.DefaultHttpRequest; import io.netty.handler.codec.http.HttpRequest; +import io.netty.util.AttributeKey; import io.netty.util.ReferenceCountUtil; -import static org.opensearch.security.http.SecureHttpServerTransport.CONTEXT_TO_RESTORE; -import static org.opensearch.security.http.SecureHttpServerTransport.EARLY_RESPONSE; -import static org.opensearch.security.http.SecureHttpServerTransport.IS_AUTHENTICATED; -import static org.opensearch.security.http.SecureHttpServerTransport.SHOULD_DECOMPRESS; -import static org.opensearch.security.http.SecureHttpServerTransport.UNCONSUMED_PARAMS; - @Sharable public class Netty4HttpRequestHeaderVerifier extends SimpleChannelInboundHandler { + public static final AttributeKey IS_AUTHENTICATED = AttributeKey.newInstance("opensearch-http-is-authenticated"); + public static final AttributeKey SHOULD_DECOMPRESS = AttributeKey.newInstance("opensearch-http-should-decompress"); + public static final AttributeKey CONTEXT_TO_RESTORE = AttributeKey.newInstance( + "opensearch-http-request-thread-context" + ); + public static final AttributeKey> UNCONSUMED_PARAMS = AttributeKey.newInstance("opensearch-http-request-consumed-params"); + public static final AttributeKey EARLY_RESPONSE = AttributeKey.newInstance("opensearch-http-early-response"); + private final SecurityRestFilter restFilter; private final ThreadPool threadPool; private final SSLConfig sslConfig; @@ -72,8 +77,8 @@ public void channelRead0(ChannelHandlerContext ctx, DefaultHttpRequest msg) thro } // 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); + ctx.channel().attr(Netty4HttpRequestHeaderVerifier.SHOULD_DECOMPRESS).set(Boolean.FALSE); + ctx.channel().attr(Netty4HttpRequestHeaderVerifier.IS_AUTHENTICATED).set(Boolean.FALSE); final Netty4HttpChannel httpChannel = ctx.channel().attr(Netty4HttpServerTransport.HTTP_CHANNEL_KEY).get(); @@ -85,24 +90,25 @@ public void channelRead0(ChannelHandlerContext ctx, DefaultHttpRequest msg) thro // If request channel is completed and a response is sent, then there was a failure during authentication restFilter.checkAndAuthenticateRequest(requestChannel); - ctx.channel().attr(UNCONSUMED_PARAMS).set(requestChannel.getUnconsumedParams()); + ctx.channel().attr(Netty4HttpRequestHeaderVerifier.UNCONSUMED_PARAMS).set(requestChannel.getUnconsumedParams()); ThreadContext.StoredContext contextToRestore = threadPool.getThreadContext().newStoredContext(false); - ctx.channel().attr(CONTEXT_TO_RESTORE).set(contextToRestore); + ctx.channel().attr(Netty4HttpRequestHeaderVerifier.CONTEXT_TO_RESTORE).set(contextToRestore); - requestChannel.getQueuedResponse().ifPresent(response -> ctx.channel().attr(EARLY_RESPONSE).set(response)); + requestChannel.getQueuedResponse() + .ifPresent(response -> ctx.channel().attr(Netty4HttpRequestHeaderVerifier.EARLY_RESPONSE).set(response)); boolean shouldSkipAuthentication = SecurityRestUtils.shouldSkipAuthentication(requestChannel); 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); + ctx.channel().attr(Netty4HttpRequestHeaderVerifier.SHOULD_DECOMPRESS).set(Boolean.valueOf(shouldDecompress)); + ctx.channel().attr(Netty4HttpRequestHeaderVerifier.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); + ctx.channel().attr(Netty4HttpRequestHeaderVerifier.EARLY_RESPONSE).set(earlyResponse); } catch (final SecurityRequestChannelUnsupported srcu) { // Use defaults for unsupported channels } finally { diff --git a/src/test/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPluginTest.java b/src/test/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPluginTest.java index 03488fe17c..aefb12c0db 100644 --- a/src/test/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPluginTest.java +++ b/src/test/java/org/opensearch/security/ssl/OpenSearchSecuritySSLPluginTest.java @@ -13,6 +13,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Supplier; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; @@ -26,16 +27,22 @@ import org.opensearch.common.settings.ClusterSettings; import org.opensearch.common.settings.Settings; import org.opensearch.http.HttpServerTransport; +import org.opensearch.http.netty4.ssl.SecureNetty4HttpServerTransport; +import org.opensearch.plugins.SecureHttpTransportSettingsProvider; import org.opensearch.plugins.SecureTransportSettingsProvider; +import org.opensearch.plugins.TransportExceptionHandler; import org.opensearch.security.ssl.util.SSLConfigConstants; import org.opensearch.security.support.SecuritySettings; import org.opensearch.security.test.AbstractSecurityUnitTest; import org.opensearch.security.test.helper.file.FileHelper; import org.opensearch.telemetry.tracing.noop.NoopTracer; -import org.opensearch.transport.TcpTransport; import org.opensearch.transport.Transport; +import org.opensearch.transport.TransportAdapterProvider; + +import io.netty.channel.ChannelInboundHandlerAdapter; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.nullValue; import static org.hamcrest.MatcherAssert.assertThat; @@ -44,6 +51,7 @@ public class OpenSearchSecuritySSLPluginTest extends AbstractSecurityUnitTest { private Settings settings; + private SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider; private SecureTransportSettingsProvider secureTransportSettingsProvider; private ClusterSettings clusterSettings; @@ -76,27 +84,29 @@ public void setUp() { secureTransportSettingsProvider = new SecureTransportSettingsProvider() { @Override - public Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) { + public Optional buildServerTransportExceptionHandler(Settings settings, Transport transport) { return Optional.empty(); } @Override - public Optional buildServerTransportExceptionHandler(Settings settings, TcpTransport transport) { + public Optional buildSecureServerTransportEngine(Settings settings, Transport transport) throws SSLException { return Optional.empty(); } @Override - public Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException { + public Optional buildSecureClientTransportEngine(Settings settings, String hostname, int port) throws SSLException { return Optional.empty(); } + }; + secureHttpTransportSettingsProvider = new SecureHttpTransportSettingsProvider() { @Override - public Optional buildSecureServerTransportEngine(Settings settings, TcpTransport transport) throws SSLException { + public Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) { return Optional.empty(); } @Override - public Optional buildSecureClientTransportEngine(Settings settings, String hostname, int port) throws SSLException { + public Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException { return Optional.empty(); } }; @@ -117,10 +127,14 @@ public void testRegisterSecureHttpTransport() throws IOException { null, null, clusterSettings, - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, NoopTracer.INSTANCE ); assertThat(transports, hasKey("org.opensearch.security.ssl.http.netty.SecuritySSLNettyHttpServerTransport")); + assertThat( + transports.get("org.opensearch.security.ssl.http.netty.SecuritySSLNettyHttpServerTransport").get(), + not(nullValue()) + ); } } @@ -138,6 +152,7 @@ public void testRegisterSecureTransport() throws IOException { NoopTracer.INSTANCE ); assertThat(transports, hasKey("org.opensearch.security.ssl.http.netty.SecuritySSLNettyTransport")); + assertThat(transports.get("org.opensearch.security.ssl.http.netty.SecuritySSLNettyTransport").get(), not(nullValue())); } } @@ -243,4 +258,69 @@ public void testRegisterSecureTransportWithDuplicateSettings() throws IOExceptio } } } + + @Test + public void testRegisterSecureHttpTransportWithRequestHeaderVerifier() throws IOException { + final AtomicBoolean created = new AtomicBoolean(false); + + class LocalHeaderVerifier extends ChannelInboundHandlerAdapter { + public LocalHeaderVerifier() { + created.set(true); + } + } + + final SecureHttpTransportSettingsProvider provider = new SecureHttpTransportSettingsProvider() { + @Override + public Collection> getHttpTransportAdapterProviders(Settings settings) { + return List.of(new TransportAdapterProvider() { + + @Override + public String name() { + return SecureNetty4HttpServerTransport.REQUEST_HEADER_VERIFIER; + } + + @SuppressWarnings("unchecked") + @Override + public Optional create(Settings settings, HttpServerTransport transport, Class adapterClass) { + return Optional.of((C) new LocalHeaderVerifier()); + } + + }); + } + + @Override + public Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) { + return Optional.empty(); + } + + @Override + public Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException { + return Optional.empty(); + } + }; + + try (OpenSearchSecuritySSLPlugin plugin = new OpenSearchSecuritySSLPlugin(settings, null, false)) { + final Map> transports = plugin.getSecureHttpTransports( + settings, + MOCK_POOL, + null, + null, + null, + null, + null, + null, + clusterSettings, + provider, + NoopTracer.INSTANCE + ); + assertThat(transports, hasKey("org.opensearch.security.ssl.http.netty.SecuritySSLNettyHttpServerTransport")); + + assertThat( + transports.get("org.opensearch.security.ssl.http.netty.SecuritySSLNettyHttpServerTransport").get(), + not(nullValue()) + ); + + assertThat(created.get(), is(true)); + } + } }