[FEATURE] Broaden SecureSettingsFactory to include http transports

CHANGELOG.md

@@ -115,6 +115,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
 

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 <a href="https://github.com/opensearch-project/security/blob/d526c9f6c2a438c14db8b413148204510b9fe2e2/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java">SecuritySSLNettyHttpServerTransport</a>
  */
 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<HttpServerTransport> decompressorProvider;
 
     public SecureNetty4HttpServerTransport(
         final Settings settings,
@@ -72,7 +85,7 @@
         final Dispatcher dispatcher,
         final ClusterSettings clusterSettings,
         final SharedGroupFactory sharedGroupFactory,
-        final SecureTransportSettingsProvider secureTransportSettingsProvider,
+        final SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider,
         final Tracer tracer
     ) {
         super(
@@ -86,9 +99,45 @@
             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<ChannelInboundHandlerAdapter> 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<TransportAdapterProvider<HttpServerTransport>> 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<HttpServerTransport>() {
+            @Override
+            public String name() {
+                return REQUEST_DECOMPRESSOR;
+            }
+
+            @Override
+            public <C> Optional<C> create(Settings settings, HttpServerTransport transport, Class<C> adapterClass) {
+                return Optional.empty();
+            }
+        });
     }
 
     @Override
@@ -152,7 +201,7 @@
         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 @@
             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);
+    }
 }

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<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
         NetworkService networkService,
         HttpServerTransport.Dispatcher dispatcher,
         ClusterSettings clusterSettings,
-        SecureTransportSettingsProvider secureTransportSettingsProvider,
+        SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider,
         Tracer tracer
     ) {
         return Collections.singletonMap(
@@ -174,7 +175,7 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
                 dispatcher,
                 clusterSettings,
                 getSharedGroupFactory(settings),
-                secureTransportSettingsProvider,
+                secureHttpTransportSettingsProvider,
                 tracer
             )
         );

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

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<ServerExceptionHandler> buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) {
-                return Optional.empty();
-            }
-
-            @Override
-            public Optional<ServerExceptionHandler> buildServerTransportExceptionHandler(Settings settings, TcpTransport transport) {
+            public Optional<TransportExceptionHandler> buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) {
                 return Optional.empty();
             }
 
@@ -146,22 +140,6 @@ public Optional<SSLEngine> buildSecureHttpServerEngine(Settings settings, HttpSe
                     throw new SSLException(ex);
                 }
             }
-
-            @Override
-            public Optional<SSLEngine> buildSecureServerTransportEngine(Settings settings, TcpTransport transport) throws SSLException {
-                return Optional.empty();
-            }
-
-            @Override
-            public Optional<SSLEngine> 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
             )
         ) {

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<ServerExceptionHandler> buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) {
+            public Optional<TransportExceptionHandler> buildServerTransportExceptionHandler(Settings settings, Transport transport) {
                 return Optional.empty();
             }
 
             @Override
-            public Optional<ServerExceptionHandler> buildServerTransportExceptionHandler(Settings settings, TcpTransport transport) {
-                return Optional.empty();
-            }
-
-            @Override
-            public Optional<SSLEngine> 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<SSLEngine> buildSecureServerTransportEngine(Settings settings, TcpTransport transport) throws SSLException {
+            public Optional<SSLEngine> buildSecureServerTransportEngine(Settings settings, Transport transport) throws SSLException {
                 try {
                     final KeyStore keyStore = KeyStore.getInstance("PKCS12");
                     keyStore.load(