diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a18660a2d84c..de832cdc5c52f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -116,6 +116,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Changed - [BWC and API enforcement] Enforcing the presence of API annotations at build time ([#12872](https://github.com/opensearch-project/OpenSearch/pull/12872)) +- Improve built-in secure transports support ([#12907](https://github.com/opensearch-project/OpenSearch/pull/12907)) ### Deprecated diff --git a/modules/transport-netty4/src/main/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransport.java b/modules/transport-netty4/src/main/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransport.java index 51a76903e284d..978c92870bd75 100644 --- a/modules/transport-netty4/src/main/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransport.java +++ b/modules/transport-netty4/src/main/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransport.java @@ -37,19 +37,27 @@ import org.opensearch.core.xcontent.NamedXContentRegistry; import org.opensearch.http.HttpChannel; import org.opensearch.http.HttpHandlingSettings; +import org.opensearch.http.HttpServerTransport; import org.opensearch.http.netty4.Netty4HttpChannel; import org.opensearch.http.netty4.Netty4HttpServerTransport; -import org.opensearch.plugins.SecureTransportSettingsProvider; +import org.opensearch.plugins.SecureHttpTransportSettingsProvider; +import org.opensearch.plugins.TransportExceptionHandler; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.SharedGroupFactory; +import org.opensearch.transport.TransportAdapterProvider; import org.opensearch.transport.netty4.ssl.SslUtils; import javax.net.ssl.SSLEngine; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; + import io.netty.channel.Channel; import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; +import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.handler.codec.DecoderException; import io.netty.handler.ssl.ApplicationProtocolNames; import io.netty.handler.ssl.ApplicationProtocolNegotiationHandler; @@ -59,9 +67,14 @@ * @see SecuritySSLNettyHttpServerTransport */ public class SecureNetty4HttpServerTransport extends Netty4HttpServerTransport { + public static final String REQUEST_HEADER_VERIFIER = "HeaderVerifier"; + public static final String REQUEST_DECOMPRESSOR = "RequestDecompressor"; + private static final Logger logger = LogManager.getLogger(SecureNetty4HttpServerTransport.class); - private final SecureTransportSettingsProvider secureTransportSettingsProvider; - private final SecureTransportSettingsProvider.ServerExceptionHandler exceptionHandler; + private final SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider; + private final TransportExceptionHandler exceptionHandler; + private final ChannelInboundHandlerAdapter headerVerifier; + private final TransportAdapterProvider decompressorProvider; public SecureNetty4HttpServerTransport( final Settings settings, @@ -72,7 +85,7 @@ public SecureNetty4HttpServerTransport( final Dispatcher dispatcher, final ClusterSettings clusterSettings, final SharedGroupFactory sharedGroupFactory, - final SecureTransportSettingsProvider secureTransportSettingsProvider, + final SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider, final Tracer tracer ) { super( @@ -86,9 +99,45 @@ public SecureNetty4HttpServerTransport( sharedGroupFactory, tracer ); - this.secureTransportSettingsProvider = secureTransportSettingsProvider; - this.exceptionHandler = secureTransportSettingsProvider.buildHttpServerExceptionHandler(settings, this) - .orElse(SecureTransportSettingsProvider.ServerExceptionHandler.NOOP); + + this.secureHttpTransportSettingsProvider = secureHttpTransportSettingsProvider; + this.exceptionHandler = secureHttpTransportSettingsProvider.buildHttpServerExceptionHandler(settings, this) + .orElse(TransportExceptionHandler.NOOP); + + final List headerVerifiers = secureHttpTransportSettingsProvider.getHttpTransportAdapterProviders( + settings + ) + .stream() + .filter(p -> REQUEST_HEADER_VERIFIER.equalsIgnoreCase(p.name())) + .map(p -> p.create(settings, this, ChannelInboundHandlerAdapter.class)) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(Collectors.toList()); + + if (headerVerifiers.size() > 1) { + throw new IllegalArgumentException("Cannot have more than one header verifier configured, supplied " + headerVerifiers.size()); + } + + final Optional> decompressorProviderOpt = secureHttpTransportSettingsProvider + .getHttpTransportAdapterProviders(settings) + .stream() + .filter(p -> REQUEST_DECOMPRESSOR.equalsIgnoreCase(p.name())) + .findFirst(); + // There could be multiple request decompressor providers configured, using the first one + decompressorProviderOpt.ifPresent(p -> logger.debug("Using request decompressor provider: {}", p)); + + this.headerVerifier = headerVerifiers.isEmpty() ? null : headerVerifiers.get(0); + this.decompressorProvider = decompressorProviderOpt.orElseGet(() -> new TransportAdapterProvider() { + @Override + public String name() { + return REQUEST_DECOMPRESSOR; + } + + @Override + public Optional create(Settings settings, HttpServerTransport transport, Class adapterClass) { + return Optional.empty(); + } + }); } @Override @@ -152,7 +201,7 @@ protected SslHttpChannelHandler(final Netty4HttpServerTransport transport, final protected void initChannel(Channel ch) throws Exception { super.initChannel(ch); - final SSLEngine sslEngine = secureTransportSettingsProvider.buildSecureHttpServerEngine( + final SSLEngine sslEngine = secureHttpTransportSettingsProvider.buildSecureHttpServerEngine( settings, SecureNetty4HttpServerTransport.this ).orElseGet(SslUtils::createDefaultServerSSLEngine); @@ -166,4 +215,17 @@ protected void configurePipeline(Channel ch) { ch.pipeline().addLast(new Http2OrHttpHandler()); } } + + protected ChannelInboundHandlerAdapter createHeaderVerifier() { + if (headerVerifier != null) { + return headerVerifier; + } else { + return super.createHeaderVerifier(); + } + } + + @Override + protected ChannelInboundHandlerAdapter createDecompressor() { + return decompressorProvider.create(settings, this, ChannelInboundHandlerAdapter.class).orElseGet(super::createDecompressor); + } } diff --git a/modules/transport-netty4/src/main/java/org/opensearch/transport/Netty4ModulePlugin.java b/modules/transport-netty4/src/main/java/org/opensearch/transport/Netty4ModulePlugin.java index 56163c18949a4..e2c84ab5d339a 100644 --- a/modules/transport-netty4/src/main/java/org/opensearch/transport/Netty4ModulePlugin.java +++ b/modules/transport-netty4/src/main/java/org/opensearch/transport/Netty4ModulePlugin.java @@ -49,6 +49,7 @@ 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.SecureTransportSettingsProvider; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; @@ -160,7 +161,7 @@ public Map> getSecureHttpTransports( NetworkService networkService, HttpServerTransport.Dispatcher dispatcher, ClusterSettings clusterSettings, - SecureTransportSettingsProvider secureTransportSettingsProvider, + SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider, Tracer tracer ) { return Collections.singletonMap( @@ -174,7 +175,7 @@ public Map> getSecureHttpTransports( dispatcher, clusterSettings, getSharedGroupFactory(settings), - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, tracer ) ); diff --git a/modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java b/modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java index 9c63a1ab9161b..977121346dcc3 100644 --- a/modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java +++ b/modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java @@ -42,6 +42,7 @@ import org.opensearch.core.common.io.stream.NamedWriteableRegistry; import org.opensearch.core.indices.breaker.CircuitBreakerService; import org.opensearch.plugins.SecureTransportSettingsProvider; +import org.opensearch.plugins.TransportExceptionHandler; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.SharedGroupFactory; @@ -72,7 +73,7 @@ public class SecureNetty4Transport extends Netty4Transport { private static final Logger logger = LogManager.getLogger(SecureNetty4Transport.class); private final SecureTransportSettingsProvider secureTransportSettingsProvider; - private final SecureTransportSettingsProvider.ServerExceptionHandler exceptionHandler; + private final TransportExceptionHandler exceptionHandler; public SecureNetty4Transport( final Settings settings, @@ -100,7 +101,7 @@ public SecureNetty4Transport( this.secureTransportSettingsProvider = secureTransportSettingsProvider; this.exceptionHandler = secureTransportSettingsProvider.buildServerTransportExceptionHandler(settings, this) - .orElse(SecureTransportSettingsProvider.ServerExceptionHandler.NOOP); + .orElse(TransportExceptionHandler.NOOP); } @Override diff --git a/modules/transport-netty4/src/test/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransportConfigurationTests.java b/modules/transport-netty4/src/test/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransportConfigurationTests.java new file mode 100644 index 0000000000000..1ab1ae4f5ddfd --- /dev/null +++ b/modules/transport-netty4/src/test/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransportConfigurationTests.java @@ -0,0 +1,242 @@ +/* + * 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.http.netty4.ssl; + +import org.opensearch.common.network.NetworkService; +import org.opensearch.common.settings.ClusterSettings; +import org.opensearch.common.settings.Settings; +import org.opensearch.common.util.MockBigArrays; +import org.opensearch.common.util.MockPageCacheRecycler; +import org.opensearch.core.indices.breaker.NoneCircuitBreakerService; +import org.opensearch.http.HttpServerTransport; +import org.opensearch.http.NullDispatcher; +import org.opensearch.plugins.SecureHttpTransportSettingsProvider; +import org.opensearch.plugins.TransportExceptionHandler; +import org.opensearch.telemetry.tracing.noop.NoopTracer; +import org.opensearch.test.OpenSearchTestCase; +import org.opensearch.threadpool.TestThreadPool; +import org.opensearch.threadpool.ThreadPool; +import org.opensearch.transport.SharedGroupFactory; +import org.opensearch.transport.TransportAdapterProvider; +import org.junit.After; +import org.junit.Before; + +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLException; + +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Optional; + +import io.netty.channel.ChannelInboundHandlerAdapter; + +import static org.hamcrest.Matchers.equalTo; + +/** + * Tests for the {@link SecureNetty4HttpServerTransport} class. + */ +public class SecureNetty4HttpServerTransportConfigurationTests extends OpenSearchTestCase { + + private NetworkService networkService; + private ThreadPool threadPool; + private MockBigArrays bigArrays; + private ClusterSettings clusterSettings; + + private static class ConfigurableSecureHttpTransportSettingsProvider implements SecureHttpTransportSettingsProvider { + private final List> transportAdapterProviders; + + public ConfigurableSecureHttpTransportSettingsProvider( + List> transportAdapterProviders + ) { + this.transportAdapterProviders = transportAdapterProviders; + } + + @Override + public Collection> getHttpTransportAdapterProviders(Settings settings) { + return transportAdapterProviders; + } + + @Override + public Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) { + return Optional.empty(); + } + + @Override + public Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException { + return Optional.empty(); + } + } + + @Before + public void setup() throws Exception { + networkService = new NetworkService(Collections.emptyList()); + threadPool = new TestThreadPool("test"); + bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService()); + clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); + } + + @After + public void shutdown() throws Exception { + if (threadPool != null) { + threadPool.shutdownNow(); + } + threadPool = null; + networkService = null; + bigArrays = null; + clusterSettings = null; + } + + public void testRequestHeaderVerifier() throws InterruptedException { + final TransportAdapterProvider transportAdapterProvider = 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 ChannelInboundHandlerAdapter()); + } + + }; + + try ( + final SecureNetty4HttpServerTransport transport = new SecureNetty4HttpServerTransport( + Settings.EMPTY, + networkService, + bigArrays, + threadPool, + xContentRegistry(), + new NullDispatcher(), + clusterSettings, + new SharedGroupFactory(Settings.EMPTY), + new ConfigurableSecureHttpTransportSettingsProvider(List.of(transportAdapterProvider)), + NoopTracer.INSTANCE + ) + ) { + + } + } + + public void testMultipleRequestHeaderVerifiers() throws InterruptedException { + final TransportAdapterProvider transportAdapterProvider = 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 ChannelInboundHandlerAdapter()); + } + + }; + + final IllegalArgumentException ex = assertThrows( + IllegalArgumentException.class, + () -> new SecureNetty4HttpServerTransport( + Settings.EMPTY, + networkService, + bigArrays, + threadPool, + xContentRegistry(), + new NullDispatcher(), + clusterSettings, + new SharedGroupFactory(Settings.EMPTY), + new ConfigurableSecureHttpTransportSettingsProvider(List.of(transportAdapterProvider, transportAdapterProvider)), + NoopTracer.INSTANCE + ) + ); + + assertThat(ex.getMessage(), equalTo("Cannot have more than one header verifier configured, supplied 2")); + } + + public void testRequestDecompressor() throws InterruptedException { + final TransportAdapterProvider transportAdapterProvider = new TransportAdapterProvider() { + @Override + public String name() { + return SecureNetty4HttpServerTransport.REQUEST_DECOMPRESSOR; + } + + @SuppressWarnings("unchecked") + @Override + public Optional create(Settings settings, HttpServerTransport transport, Class adapterClass) { + return Optional.of((C) new ChannelInboundHandlerAdapter()); + } + + }; + + try ( + final SecureNetty4HttpServerTransport transport = new SecureNetty4HttpServerTransport( + Settings.EMPTY, + networkService, + bigArrays, + threadPool, + xContentRegistry(), + new NullDispatcher(), + clusterSettings, + new SharedGroupFactory(Settings.EMPTY), + new ConfigurableSecureHttpTransportSettingsProvider(List.of(transportAdapterProvider)), + NoopTracer.INSTANCE + ) + ) { + + } + } + + public void testRequestDecompressorAndRequestHeaderVerifier() throws InterruptedException { + final TransportAdapterProvider requestDecompressor = new TransportAdapterProvider() { + @Override + public String name() { + return SecureNetty4HttpServerTransport.REQUEST_DECOMPRESSOR; + } + + @SuppressWarnings("unchecked") + @Override + public Optional create(Settings settings, HttpServerTransport transport, Class adapterClass) { + return Optional.of((C) new ChannelInboundHandlerAdapter()); + } + + }; + + final TransportAdapterProvider requestHeaderVerifier = 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 ChannelInboundHandlerAdapter()); + } + + }; + + try ( + final SecureNetty4HttpServerTransport transport = new SecureNetty4HttpServerTransport( + Settings.EMPTY, + networkService, + bigArrays, + threadPool, + xContentRegistry(), + new NullDispatcher(), + clusterSettings, + new SharedGroupFactory(Settings.EMPTY), + new ConfigurableSecureHttpTransportSettingsProvider(List.of(requestDecompressor, requestHeaderVerifier)), + NoopTracer.INSTANCE + ) + ) { + + } + } +} diff --git a/modules/transport-netty4/src/test/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransportTests.java b/modules/transport-netty4/src/test/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransportTests.java index 9ea49d0b24d44..e79a066ad8f63 100644 --- a/modules/transport-netty4/src/test/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransportTests.java +++ b/modules/transport-netty4/src/test/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransportTests.java @@ -29,7 +29,8 @@ import org.opensearch.http.HttpTransportSettings; import org.opensearch.http.NullDispatcher; import org.opensearch.http.netty4.Netty4HttpClient; -import org.opensearch.plugins.SecureTransportSettingsProvider; +import org.opensearch.plugins.SecureHttpTransportSettingsProvider; +import org.opensearch.plugins.TransportExceptionHandler; import org.opensearch.rest.BytesRestResponse; import org.opensearch.rest.RestChannel; import org.opensearch.rest.RestRequest; @@ -40,7 +41,6 @@ import org.opensearch.threadpool.ThreadPool; import org.opensearch.transport.NettyAllocator; import org.opensearch.transport.SharedGroupFactory; -import org.opensearch.transport.TcpTransport; import org.opensearch.transport.netty4.ssl.TrustAllManager; import org.junit.After; import org.junit.Before; @@ -83,7 +83,6 @@ import io.netty.handler.codec.http.HttpResponseStatus; import io.netty.handler.codec.http.HttpUtil; import io.netty.handler.codec.http.HttpVersion; -import io.netty.handler.ssl.ClientAuth; import io.netty.handler.ssl.SslContextBuilder; import static org.opensearch.core.rest.RestStatus.BAD_REQUEST; @@ -104,7 +103,7 @@ public class SecureNetty4HttpServerTransportTests extends OpenSearchTestCase { private ThreadPool threadPool; private MockBigArrays bigArrays; private ClusterSettings clusterSettings; - private SecureTransportSettingsProvider secureTransportSettingsProvider; + private SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider; @Before public void setup() throws Exception { @@ -113,14 +112,9 @@ public void setup() throws Exception { bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService()); clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS); - secureTransportSettingsProvider = new SecureTransportSettingsProvider() { + secureHttpTransportSettingsProvider = new SecureHttpTransportSettingsProvider() { @Override - public Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) { - return Optional.empty(); - } - - @Override - public Optional buildServerTransportExceptionHandler(Settings settings, TcpTransport transport) { + public Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) { return Optional.empty(); } @@ -146,22 +140,6 @@ public Optional buildSecureHttpServerEngine(Settings settings, HttpSe throw new SSLException(ex); } } - - @Override - public Optional buildSecureServerTransportEngine(Settings settings, TcpTransport transport) throws SSLException { - return Optional.empty(); - } - - @Override - public Optional buildSecureClientTransportEngine(Settings settings, String hostname, int port) throws SSLException { - return Optional.of( - SslContextBuilder.forClient() - .clientAuth(ClientAuth.NONE) - .trustManager(TrustAllManager.INSTANCE) - .build() - .newEngine(NettyAllocator.getAllocator()) - ); - } }; } @@ -241,7 +219,7 @@ public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext, dispatcher, clusterSettings, new SharedGroupFactory(settings), - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, NoopTracer.INSTANCE ) ) { @@ -292,7 +270,7 @@ public void testBindUnavailableAddress() { new NullDispatcher(), clusterSettings, new SharedGroupFactory(Settings.EMPTY), - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, NoopTracer.INSTANCE ) ) { @@ -312,7 +290,7 @@ public void testBindUnavailableAddress() { new NullDispatcher(), clusterSettings, new SharedGroupFactory(settings), - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, NoopTracer.INSTANCE ) ) { @@ -366,7 +344,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th dispatcher, clusterSettings, new SharedGroupFactory(settings), - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, NoopTracer.INSTANCE ) ) { @@ -430,7 +408,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th dispatcher, clusterSettings, new SharedGroupFactory(Settings.EMPTY), - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, NoopTracer.INSTANCE ) ) { @@ -487,7 +465,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th dispatcher, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), new SharedGroupFactory(settings), - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, NoopTracer.INSTANCE ) ) { @@ -562,7 +540,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th dispatcher, new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), new SharedGroupFactory(settings), - secureTransportSettingsProvider, + secureHttpTransportSettingsProvider, NoopTracer.INSTANCE ) ) { diff --git a/modules/transport-netty4/src/test/java/org/opensearch/transport/netty4/ssl/SimpleSecureNetty4TransportTests.java b/modules/transport-netty4/src/test/java/org/opensearch/transport/netty4/ssl/SimpleSecureNetty4TransportTests.java index 0cae58b8efa2a..df3b005f40903 100644 --- a/modules/transport-netty4/src/test/java/org/opensearch/transport/netty4/ssl/SimpleSecureNetty4TransportTests.java +++ b/modules/transport-netty4/src/test/java/org/opensearch/transport/netty4/ssl/SimpleSecureNetty4TransportTests.java @@ -20,8 +20,8 @@ import org.opensearch.core.common.io.stream.NamedWriteableRegistry; import org.opensearch.core.common.transport.TransportAddress; import org.opensearch.core.indices.breaker.NoneCircuitBreakerService; -import org.opensearch.http.HttpServerTransport; import org.opensearch.plugins.SecureTransportSettingsProvider; +import org.opensearch.plugins.TransportExceptionHandler; import org.opensearch.telemetry.tracing.noop.NoopTracer; import org.opensearch.test.transport.MockTransportService; import org.opensearch.test.transport.StubbableTransport; @@ -69,40 +69,12 @@ protected Transport build(Settings settings, final Version version, ClusterSetti NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry(Collections.emptyList()); final SecureTransportSettingsProvider 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) { - return Optional.empty(); - } - - @Override - public Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException { - try { - final KeyStore keyStore = KeyStore.getInstance("PKCS12"); - keyStore.load( - SimpleSecureNetty4TransportTests.class.getResourceAsStream("/netty4-secure.jks"), - "password".toCharArray() - ); - - final KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance("SunX509"); - keyManagerFactory.init(keyStore, "password".toCharArray()); - - SSLEngine engine = SslContextBuilder.forServer(keyManagerFactory) - .trustManager(TrustAllManager.INSTANCE) - .build() - .newEngine(NettyAllocator.getAllocator()); - return Optional.of(engine); - } catch (final IOException | NoSuchAlgorithmException | UnrecoverableKeyException | KeyStoreException - | CertificateException ex) { - throw new SSLException(ex); - } - } - - @Override - public Optional buildSecureServerTransportEngine(Settings settings, TcpTransport transport) throws SSLException { + public Optional buildSecureServerTransportEngine(Settings settings, Transport transport) throws SSLException { try { final KeyStore keyStore = KeyStore.getInstance("PKCS12"); keyStore.load( diff --git a/server/src/main/java/org/opensearch/common/network/NetworkModule.java b/server/src/main/java/org/opensearch/common/network/NetworkModule.java index d0f5dd9e4581d..bb8da190a6f35 100644 --- a/server/src/main/java/org/opensearch/common/network/NetworkModule.java +++ b/server/src/main/java/org/opensearch/common/network/NetworkModule.java @@ -55,6 +55,8 @@ import org.opensearch.http.HttpServerTransport; import org.opensearch.index.shard.PrimaryReplicaSyncer.ResyncTask; import org.opensearch.plugins.NetworkPlugin; +import org.opensearch.plugins.SecureHttpTransportSettingsProvider; +import org.opensearch.plugins.SecureSettingsFactory; import org.opensearch.plugins.SecureTransportSettingsProvider; import org.opensearch.ratelimitting.admissioncontrol.enums.AdmissionControlActionType; import org.opensearch.tasks.RawTaskStatus; @@ -74,7 +76,9 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.function.Supplier; +import java.util.stream.Collectors; /** * A module to handle registering and binding all network related classes. @@ -173,13 +177,31 @@ public NetworkModule( ClusterSettings clusterSettings, Tracer tracer, List transportInterceptors, - Collection secureTransportSettingsProvider + Collection secureSettingsFactories ) { this.settings = settings; - if (secureTransportSettingsProvider.size() > 1) { + final Collection secureTransportSettingsProviders = secureSettingsFactories.stream() + .map(p -> p.getSecureTransportSettingsProvider(settings)) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(Collectors.toList()); + + if (secureTransportSettingsProviders.size() > 1) { + throw new IllegalArgumentException( + "there is more than one secure transport settings provider: " + secureTransportSettingsProviders + ); + } + + final Collection secureHttpTransportSettingsProviders = secureSettingsFactories.stream() + .map(p -> p.getSecureHttpTransportSettingsProvider(settings)) + .filter(Optional::isPresent) + .map(Optional::get) + .collect(Collectors.toList()); + + if (secureHttpTransportSettingsProviders.size() > 1) { throw new IllegalArgumentException( - "there is more than one secure transport settings provider: " + secureTransportSettingsProvider + "there is more than one secure HTTP transport settings provider: " + secureHttpTransportSettingsProviders ); } @@ -213,9 +235,9 @@ public NetworkModule( registerTransport(entry.getKey(), entry.getValue()); } - // Register any secure transports if available - if (secureTransportSettingsProvider.isEmpty() == false) { - final SecureTransportSettingsProvider secureSettingProvider = secureTransportSettingsProvider.iterator().next(); + // Register any HTTP secure transports if available + if (secureHttpTransportSettingsProviders.isEmpty() == false) { + final SecureHttpTransportSettingsProvider secureSettingProvider = secureHttpTransportSettingsProviders.iterator().next(); final Map> secureHttpTransportFactory = plugin.getSecureHttpTransports( settings, @@ -233,6 +255,11 @@ public NetworkModule( for (Map.Entry> entry : secureHttpTransportFactory.entrySet()) { registerHttpTransport(entry.getKey(), entry.getValue()); } + } + + // Register any secure transports if available + if (secureTransportSettingsProviders.isEmpty() == false) { + final SecureTransportSettingsProvider secureSettingProvider = secureTransportSettingsProviders.iterator().next(); final Map> secureTransportFactory = plugin.getSecureTransports( settings, diff --git a/server/src/main/java/org/opensearch/node/Node.java b/server/src/main/java/org/opensearch/node/Node.java index 8348b8f02d342..7fa2b6c8ff497 100644 --- a/server/src/main/java/org/opensearch/node/Node.java +++ b/server/src/main/java/org/opensearch/node/Node.java @@ -203,7 +203,7 @@ import org.opensearch.plugins.ScriptPlugin; import org.opensearch.plugins.SearchPipelinePlugin; import org.opensearch.plugins.SearchPlugin; -import org.opensearch.plugins.SecureTransportSettingsProvider; +import org.opensearch.plugins.SecureSettingsFactory; import org.opensearch.plugins.SystemIndexPlugin; import org.opensearch.plugins.TelemetryPlugin; import org.opensearch.ratelimitting.admissioncontrol.AdmissionControlService; @@ -950,9 +950,9 @@ protected Node( admissionControlService ); - final Collection secureTransportSettingsProviders = pluginsService.filterPlugins(Plugin.class) + final Collection secureSettingsFactories = pluginsService.filterPlugins(Plugin.class) .stream() - .map(p -> p.getSecureSettingFactory(settings).flatMap(f -> f.getSecureTransportSettingsProvider(settings))) + .map(p -> p.getSecureSettingFactory(settings)) .filter(Optional::isPresent) .map(Optional::get) .collect(Collectors.toList()); @@ -972,7 +972,7 @@ protected Node( clusterService.getClusterSettings(), tracer, transportInterceptors, - secureTransportSettingsProviders + secureSettingsFactories ); Collection>> indexTemplateMetadataUpgraders = pluginsService.filterPlugins( diff --git a/server/src/main/java/org/opensearch/plugins/NetworkPlugin.java b/server/src/main/java/org/opensearch/plugins/NetworkPlugin.java index 679833c9f6e0d..138ef6f71280d 100644 --- a/server/src/main/java/org/opensearch/plugins/NetworkPlugin.java +++ b/server/src/main/java/org/opensearch/plugins/NetworkPlugin.java @@ -139,7 +139,7 @@ default Map> getSecureHttpTransports( NetworkService networkService, HttpServerTransport.Dispatcher dispatcher, ClusterSettings clusterSettings, - SecureTransportSettingsProvider secureTransportSettingsProvider, + SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider, Tracer tracer ) { return Collections.emptyMap(); diff --git a/server/src/main/java/org/opensearch/plugins/SecureHttpTransportSettingsProvider.java b/server/src/main/java/org/opensearch/plugins/SecureHttpTransportSettingsProvider.java new file mode 100644 index 0000000000000..ff86cbc04e240 --- /dev/null +++ b/server/src/main/java/org/opensearch/plugins/SecureHttpTransportSettingsProvider.java @@ -0,0 +1,55 @@ +/* + * 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.plugins; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.common.settings.Settings; +import org.opensearch.http.HttpServerTransport; +import org.opensearch.transport.TransportAdapterProvider; + +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLException; + +import java.util.Collection; +import java.util.Collections; +import java.util.Optional; + +/** + * A provider for security related settings for HTTP transports. + * + * @opensearch.experimental + */ +@ExperimentalApi +public interface SecureHttpTransportSettingsProvider { + /** + * Collection of additional {@link TransportAdapterProvider}s that are specific to particular HTTP transport + * @param settings settings + * @return a collection of additional {@link TransportAdapterProvider}s + */ + default Collection> getHttpTransportAdapterProviders(Settings settings) { + return Collections.emptyList(); + } + + /** + * If supported, builds the {@link TransportExceptionHandler} instance for {@link HttpServerTransport} instance + * @param settings settings + * @param transport {@link HttpServerTransport} instance + * @return if supported, builds the {@link TransportExceptionHandler} instance + */ + Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport); + + /** + * If supported, builds the {@link SSLEngine} instance for {@link HttpServerTransport} instance + * @param settings settings + * @param transport {@link HttpServerTransport} instance + * @return if supported, builds the {@link SSLEngine} instance + * @throws SSLException throws SSLException if the {@link SSLEngine} instance cannot be built + */ + Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException; +} diff --git a/server/src/main/java/org/opensearch/plugins/SecureSettingsFactory.java b/server/src/main/java/org/opensearch/plugins/SecureSettingsFactory.java index b98d9cf51c129..ec2276ecc62ef 100644 --- a/server/src/main/java/org/opensearch/plugins/SecureSettingsFactory.java +++ b/server/src/main/java/org/opensearch/plugins/SecureSettingsFactory.java @@ -26,4 +26,11 @@ public interface SecureSettingsFactory { * @return optionally, the instance of the {@link SecureTransportSettingsProvider} */ Optional getSecureTransportSettingsProvider(Settings settings); + + /** + * Creates (or provides pre-created) instance of the {@link SecureHttpTransportSettingsProvider} + * @param settings settings + * @return optionally, the instance of the {@link SecureHttpTransportSettingsProvider} + */ + Optional getSecureHttpTransportSettingsProvider(Settings settings); } diff --git a/server/src/main/java/org/opensearch/plugins/SecureTransportSettingsProvider.java b/server/src/main/java/org/opensearch/plugins/SecureTransportSettingsProvider.java index 6d038ed30c8ff..5b7402a01f82d 100644 --- a/server/src/main/java/org/opensearch/plugins/SecureTransportSettingsProvider.java +++ b/server/src/main/java/org/opensearch/plugins/SecureTransportSettingsProvider.java @@ -10,12 +10,14 @@ import org.opensearch.common.annotation.ExperimentalApi; import org.opensearch.common.settings.Settings; -import org.opensearch.http.HttpServerTransport; -import org.opensearch.transport.TcpTransport; +import org.opensearch.transport.Transport; +import org.opensearch.transport.TransportAdapterProvider; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; +import java.util.Collection; +import java.util.Collections; import java.util.Optional; /** @@ -26,57 +28,30 @@ @ExperimentalApi public interface SecureTransportSettingsProvider { /** - * An exception handler for errors that might happen while secure transport handle the requests. - * - * @see SslExceptionHandler - * - * @opensearch.experimental - */ - @ExperimentalApi - @FunctionalInterface - interface ServerExceptionHandler { - static ServerExceptionHandler NOOP = t -> {}; - - /** - * Handler for errors happening during the server side processing of the requests - * @param t the error - */ - void onError(Throwable t); - } - - /** - * If supported, builds the {@link ServerExceptionHandler} instance for {@link HttpServerTransport} instance - * @param settings settings - * @param transport {@link HttpServerTransport} instance - * @return if supported, builds the {@link ServerExceptionHandler} instance - */ - Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport); - - /** - * If supported, builds the {@link ServerExceptionHandler} instance for {@link TcpTransport} instance + * Collection of additional {@link TransportAdapterProvider}s that are specific to particular transport * @param settings settings - * @param transport {@link TcpTransport} instance - * @return if supported, builds the {@link ServerExceptionHandler} instance + * @return a collection of additional {@link TransportAdapterProvider}s */ - Optional buildServerTransportExceptionHandler(Settings settings, TcpTransport transport); + default Collection> getTransportAdapterProviders(Settings settings) { + return Collections.emptyList(); + } /** - * If supported, builds the {@link SSLEngine} instance for {@link HttpServerTransport} instance + * If supported, builds the {@link TransportExceptionHandler} instance for {@link Transport} instance * @param settings settings - * @param transport {@link HttpServerTransport} instance - * @return if supported, builds the {@link SSLEngine} instance - * @throws SSLException throws SSLException if the {@link SSLEngine} instance cannot be built + * @param transport {@link Transport} instance + * @return if supported, builds the {@link TransportExceptionHandler} instance */ - Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException; + Optional buildServerTransportExceptionHandler(Settings settings, Transport transport); /** - * If supported, builds the {@link SSLEngine} instance for {@link TcpTransport} instance + * If supported, builds the {@link SSLEngine} instance for {@link Transport} instance * @param settings settings - * @param transport {@link TcpTransport} instance + * @param transport {@link Transport} instance * @return if supported, builds the {@link SSLEngine} instance * @throws SSLException throws SSLException if the {@link SSLEngine} instance cannot be built */ - Optional buildSecureServerTransportEngine(Settings settings, TcpTransport transport) throws SSLException; + Optional buildSecureServerTransportEngine(Settings settings, Transport transport) throws SSLException; /** * If supported, builds the {@link SSLEngine} instance for client transport instance diff --git a/server/src/main/java/org/opensearch/plugins/TransportExceptionHandler.java b/server/src/main/java/org/opensearch/plugins/TransportExceptionHandler.java new file mode 100644 index 0000000000000..a6b935a6b97bc --- /dev/null +++ b/server/src/main/java/org/opensearch/plugins/TransportExceptionHandler.java @@ -0,0 +1,30 @@ +/* + * 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.plugins; + +import org.opensearch.common.annotation.ExperimentalApi; + +/** + * An exception handler for errors that might happen while secure transport handle the requests. + * + * @see SslExceptionHandler + * + * @opensearch.experimental + */ +@ExperimentalApi +@FunctionalInterface +public interface TransportExceptionHandler { + static TransportExceptionHandler NOOP = t -> {}; + + /** + * Handler for errors happening during the server side processing of the requests + * @param t the error + */ + void onError(Throwable t); +} diff --git a/server/src/main/java/org/opensearch/transport/TransportAdapterProvider.java b/server/src/main/java/org/opensearch/transport/TransportAdapterProvider.java new file mode 100644 index 0000000000000..36dbd5a699b40 --- /dev/null +++ b/server/src/main/java/org/opensearch/transport/TransportAdapterProvider.java @@ -0,0 +1,40 @@ +/* + * 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.transport; + +import org.opensearch.common.annotation.ExperimentalApi; +import org.opensearch.common.settings.Settings; + +import java.util.Optional; + +/** + * Transport specific adapter providers which could be injected into the transport processing chain. The transport adapters + * are transport specific and do not have any common abstraction on top. + * @param transport type + * + * @opensearch.experimental + */ +@ExperimentalApi +public interface TransportAdapterProvider { + /** + * The name of this transport adapter provider (and essentially are freestyle). + * @return the name of this transport adapter provider + */ + String name(); + + /** + * Provides a new transport adapter of required transport adapter class and transport instance. + * @param transport adapter class + * @param settings settings + * @param transport HTTP transport instance + * @param adapterClass required transport adapter class + * @return the non-empty {@link Optional} if the transport adapter could be created, empty one otherwise + */ + Optional create(Settings settings, T transport, Class adapterClass); +} diff --git a/server/src/test/java/org/opensearch/common/network/NetworkModuleTests.java b/server/src/test/java/org/opensearch/common/network/NetworkModuleTests.java index 1c607ca0dc98b..447377e372e61 100644 --- a/server/src/test/java/org/opensearch/common/network/NetworkModuleTests.java +++ b/server/src/test/java/org/opensearch/common/network/NetworkModuleTests.java @@ -47,13 +47,15 @@ import org.opensearch.http.HttpStats; import org.opensearch.http.NullDispatcher; import org.opensearch.plugins.NetworkPlugin; +import org.opensearch.plugins.SecureHttpTransportSettingsProvider; +import org.opensearch.plugins.SecureSettingsFactory; import org.opensearch.plugins.SecureTransportSettingsProvider; +import org.opensearch.plugins.TransportExceptionHandler; import org.opensearch.telemetry.tracing.Tracer; import org.opensearch.telemetry.tracing.noop.NoopTracer; import org.opensearch.test.OpenSearchTestCase; import org.opensearch.threadpool.TestThreadPool; import org.opensearch.threadpool.ThreadPool; -import org.opensearch.transport.TcpTransport; import org.opensearch.transport.Transport; import org.opensearch.transport.TransportInterceptor; import org.opensearch.transport.TransportRequest; @@ -73,38 +75,60 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; +import static org.hamcrest.CoreMatchers.startsWith; + public class NetworkModuleTests extends OpenSearchTestCase { private ThreadPool threadPool; - private SecureTransportSettingsProvider secureTransportSettingsProvider; + private SecureSettingsFactory secureSettingsFactory; @Override public void setUp() throws Exception { super.setUp(); threadPool = new TestThreadPool(NetworkModuleTests.class.getName()); - secureTransportSettingsProvider = new SecureTransportSettingsProvider() { - @Override - public Optional buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) { - return Optional.empty(); - } - - @Override - public Optional buildServerTransportExceptionHandler(Settings settings, TcpTransport transport) { - return Optional.empty(); - } - - @Override - public Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException { - return Optional.empty(); - } + secureSettingsFactory = new SecureSettingsFactory() { @Override - public Optional buildSecureServerTransportEngine(Settings settings, TcpTransport transport) throws SSLException { - return Optional.empty(); + public Optional getSecureTransportSettingsProvider(Settings settings) { + return Optional.of(new SecureTransportSettingsProvider() { + @Override + public Optional buildServerTransportExceptionHandler( + Settings settings, + Transport transport + ) { + return Optional.empty(); + } + + @Override + public Optional buildSecureServerTransportEngine(Settings settings, Transport transport) + throws SSLException { + return Optional.empty(); + } + + @Override + public Optional buildSecureClientTransportEngine(Settings settings, String hostname, int port) + throws SSLException { + return Optional.empty(); + } + }); } @Override - public Optional buildSecureClientTransportEngine(Settings settings, String hostname, int port) throws SSLException { - return Optional.empty(); + public Optional getSecureHttpTransportSettingsProvider(Settings settings) { + return Optional.of(new SecureHttpTransportSettingsProvider() { + @Override + public Optional buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) + throws SSLException { + return Optional.empty(); + } + + @Override + public Optional buildHttpServerExceptionHandler( + Settings settings, + HttpServerTransport transport + ) { + return Optional.empty(); + } + }); } }; } @@ -211,7 +235,7 @@ public Map> getSecureTransports( return Collections.singletonMap("custom-secure", custom); } }; - NetworkModule module = newNetworkModule(settings, null, List.of(secureTransportSettingsProvider), plugin); + NetworkModule module = newNetworkModule(settings, null, List.of(secureSettingsFactory), plugin); assertSame(custom, module.getTransportSupplier()); } @@ -222,7 +246,7 @@ public void testRegisterSecureHttpTransport() { .build(); Supplier custom = FakeHttpTransport::new; - NetworkModule module = newNetworkModule(settings, null, List.of(secureTransportSettingsProvider), new NetworkPlugin() { + NetworkModule module = newNetworkModule(settings, null, List.of(secureSettingsFactory), new NetworkPlugin() { @Override public Map> getSecureHttpTransports( Settings settings, @@ -234,7 +258,7 @@ public Map> getSecureHttpTransports( NetworkService networkService, HttpServerTransport.Dispatcher requestDispatcher, ClusterSettings clusterSettings, - SecureTransportSettingsProvider secureTransportSettingsProvider, + SecureHttpTransportSettingsProvider secureTransportSettingsProvider, Tracer tracer ) { return Collections.singletonMap("custom-secure", custom); @@ -595,7 +619,7 @@ private NetworkModule newNetworkModule( private NetworkModule newNetworkModule( Settings settings, List coreTransportInterceptors, - List secureTransportSettingsProviders, + List secureSettingsFactories, NetworkPlugin... plugins ) { return new NetworkModule( @@ -612,7 +636,33 @@ private NetworkModule newNetworkModule( new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS), NoopTracer.INSTANCE, coreTransportInterceptors, - secureTransportSettingsProviders + secureSettingsFactories + ); + } + + public void testRegisterSecureTransportMultipleProviers() { + Settings settings = Settings.builder().put(NetworkModule.TRANSPORT_TYPE_KEY, "custom-secure").build(); + Supplier custom = () -> null; // content doesn't matter we check reference equality + NetworkPlugin plugin = new NetworkPlugin() { + @Override + public Map> getSecureTransports( + Settings settings, + ThreadPool threadPool, + PageCacheRecycler pageCacheRecycler, + CircuitBreakerService circuitBreakerService, + NamedWriteableRegistry namedWriteableRegistry, + NetworkService networkService, + SecureTransportSettingsProvider secureTransportSettingsProvider, + Tracer tracer + ) { + return Collections.singletonMap("custom-secure", custom); + } + }; + + final IllegalArgumentException ex = assertThrows( + IllegalArgumentException.class, + () -> newNetworkModule(settings, null, List.of(secureSettingsFactory, secureSettingsFactory), plugin) ); + assertThat(ex.getMessage(), startsWith("there is more than one secure transport settings provider")); } }