Skip to content

Commit cac3d99

Browse files
retareta
committed
[FEATURE] Improve built-in secure transports support
Signed-off-by: Andriy Redko <andriy.redko@aiven.io>
1 parent 10fc755 commit cac3d99

File tree

15 files changed

+312
-155
lines changed

15 files changed

+312
-155
lines changed

‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
112112

113113
### Changed
114114
- [BWC and API enforcement] Enforcing the presence of API annotations at build time ([#12872](https://github.com/opensearch-project/OpenSearch/pull/12872))
115+
- Improve built-in secure transports support ([#12907](https://github.com/opensearch-project/OpenSearch/pull/12907))
115116

116117
### Deprecated
117118

‎modules/transport-netty4/src/main/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransport.java

+56-8
Original file line numberDiff line numberDiff line change
@@ -37,19 +37,25 @@
3737
import org.opensearch.core.xcontent.NamedXContentRegistry;
3838
import org.opensearch.http.HttpChannel;
3939
import org.opensearch.http.HttpHandlingSettings;
40+
import org.opensearch.http.HttpServerTransport;
4041
import org.opensearch.http.netty4.Netty4HttpChannel;
4142
import org.opensearch.http.netty4.Netty4HttpServerTransport;
42-
import org.opensearch.plugins.SecureTransportSettingsProvider;
43+
import org.opensearch.plugins.SecureHttpTransportSettingsProvider;
44+
import org.opensearch.plugins.TransportExceptionHandler;
4345
import org.opensearch.telemetry.tracing.Tracer;
4446
import org.opensearch.threadpool.ThreadPool;
4547
import org.opensearch.transport.SharedGroupFactory;
48+
import org.opensearch.transport.TransportAdapterProvider;
4649
import org.opensearch.transport.netty4.ssl.SslUtils;
4750

4851
import javax.net.ssl.SSLEngine;
4952

53+
import java.util.Optional;
54+
5055
import io.netty.channel.Channel;
5156
import io.netty.channel.ChannelHandler;
5257
import io.netty.channel.ChannelHandlerContext;
58+
import io.netty.channel.ChannelInboundHandlerAdapter;
5359
import io.netty.handler.codec.DecoderException;
5460
import io.netty.handler.ssl.ApplicationProtocolNames;
5561
import io.netty.handler.ssl.ApplicationProtocolNegotiationHandler;
@@ -59,9 +65,14 @@
5965
* @see <a href="https://github.com/opensearch-project/security/blob/d526c9f6c2a438c14db8b413148204510b9fe2e2/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java">SecuritySSLNettyHttpServerTransport</a>
6066
*/
6167
public class SecureNetty4HttpServerTransport extends Netty4HttpServerTransport {
68+
public static final String HEADER_VERIFIER = "HeaderVerifier";
69+
public static final String REQUEST_DECOMPRESSOR = "RequestDecompressor";
70+
6271
private static final Logger logger = LogManager.getLogger(SecureNetty4HttpServerTransport.class);
63-
private final SecureTransportSettingsProvider secureTransportSettingsProvider;
64-
private final SecureTransportSettingsProvider.ServerExceptionHandler exceptionHandler;
72+
private final SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider;
73+
private final TransportExceptionHandler exceptionHandler;
74+
private final ChannelInboundHandlerAdapter headerVerifier;
75+
private final TransportAdapterProvider<HttpServerTransport> decompressorProvider;
6576

6677
public SecureNetty4HttpServerTransport(
6778
final Settings settings,
@@ -72,7 +83,7 @@ public SecureNetty4HttpServerTransport(
7283
final Dispatcher dispatcher,
7384
final ClusterSettings clusterSettings,
7485
final SharedGroupFactory sharedGroupFactory,
75-
final SecureTransportSettingsProvider secureTransportSettingsProvider,
86+
final SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider,
7687
final Tracer tracer
7788
) {
7889
super(
@@ -86,9 +97,33 @@ public SecureNetty4HttpServerTransport(
8697
sharedGroupFactory,
8798
tracer
8899
);
89-
this.secureTransportSettingsProvider = secureTransportSettingsProvider;
90-
this.exceptionHandler = secureTransportSettingsProvider.buildHttpServerExceptionHandler(settings, this)
91-
.orElse(SecureTransportSettingsProvider.ServerExceptionHandler.NOOP);
100+
101+
this.secureHttpTransportSettingsProvider = secureHttpTransportSettingsProvider;
102+
this.exceptionHandler = secureHttpTransportSettingsProvider.buildHttpServerExceptionHandler(settings, this)
103+
.orElse(TransportExceptionHandler.NOOP);
104+
105+
this.headerVerifier = secureHttpTransportSettingsProvider.getHttpTransportAdapterProviders(settings)
106+
.stream()
107+
.filter(p -> HEADER_VERIFIER.equalsIgnoreCase(p.name()))
108+
.findFirst()
109+
.flatMap(p -> p.create(settings, this, ChannelInboundHandlerAdapter.class))
110+
.orElse(null);
111+
112+
this.decompressorProvider = secureHttpTransportSettingsProvider.getHttpTransportAdapterProviders(settings)
113+
.stream()
114+
.filter(p -> REQUEST_DECOMPRESSOR.equalsIgnoreCase(p.name()))
115+
.findFirst()
116+
.orElseGet(() -> new TransportAdapterProvider<HttpServerTransport>() {
117+
@Override
118+
public String name() {
119+
return REQUEST_DECOMPRESSOR;
120+
}
121+
122+
@Override
123+
public <C> Optional<C> create(Settings settings, HttpServerTransport transport, Class<C> adapterClass) {
124+
return Optional.empty();
125+
}
126+
});
92127
}
93128

94129
@Override
@@ -152,7 +187,7 @@ protected SslHttpChannelHandler(final Netty4HttpServerTransport transport, final
152187
protected void initChannel(Channel ch) throws Exception {
153188
super.initChannel(ch);
154189

155-
final SSLEngine sslEngine = secureTransportSettingsProvider.buildSecureHttpServerEngine(
190+
final SSLEngine sslEngine = secureHttpTransportSettingsProvider.buildSecureHttpServerEngine(
156191
settings,
157192
SecureNetty4HttpServerTransport.this
158193
).orElseGet(SslUtils::createDefaultServerSSLEngine);
@@ -166,4 +201,17 @@ protected void configurePipeline(Channel ch) {
166201
ch.pipeline().addLast(new Http2OrHttpHandler());
167202
}
168203
}
204+
205+
protected ChannelInboundHandlerAdapter createHeaderVerifier() {
206+
if (headerVerifier != null) {
207+
return headerVerifier;
208+
} else {
209+
return super.createHeaderVerifier();
210+
}
211+
}
212+
213+
@Override
214+
protected ChannelInboundHandlerAdapter createDecompressor() {
215+
return decompressorProvider.create(settings, this, ChannelInboundHandlerAdapter.class).orElseGet(super::createDecompressor);
216+
}
169217
}

‎modules/transport-netty4/src/main/java/org/opensearch/transport/Netty4ModulePlugin.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import org.opensearch.http.netty4.ssl.SecureNetty4HttpServerTransport;
5050
import org.opensearch.plugins.NetworkPlugin;
5151
import org.opensearch.plugins.Plugin;
52+
import org.opensearch.plugins.SecureHttpTransportSettingsProvider;
5253
import org.opensearch.plugins.SecureTransportSettingsProvider;
5354
import org.opensearch.telemetry.tracing.Tracer;
5455
import org.opensearch.threadpool.ThreadPool;
@@ -160,7 +161,7 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
160161
NetworkService networkService,
161162
HttpServerTransport.Dispatcher dispatcher,
162163
ClusterSettings clusterSettings,
163-
SecureTransportSettingsProvider secureTransportSettingsProvider,
164+
SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider,
164165
Tracer tracer
165166
) {
166167
return Collections.singletonMap(
@@ -174,7 +175,7 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
174175
dispatcher,
175176
clusterSettings,
176177
getSharedGroupFactory(settings),
177-
secureTransportSettingsProvider,
178+
secureHttpTransportSettingsProvider,
178179
tracer
179180
)
180181
);

‎modules/transport-netty4/src/main/java/org/opensearch/transport/netty4/ssl/SecureNetty4Transport.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@
4242
import org.opensearch.core.common.io.stream.NamedWriteableRegistry;
4343
import org.opensearch.core.indices.breaker.CircuitBreakerService;
4444
import org.opensearch.plugins.SecureTransportSettingsProvider;
45+
import org.opensearch.plugins.TransportExceptionHandler;
4546
import org.opensearch.telemetry.tracing.Tracer;
4647
import org.opensearch.threadpool.ThreadPool;
4748
import org.opensearch.transport.SharedGroupFactory;
@@ -72,7 +73,7 @@ public class SecureNetty4Transport extends Netty4Transport {
7273

7374
private static final Logger logger = LogManager.getLogger(SecureNetty4Transport.class);
7475
private final SecureTransportSettingsProvider secureTransportSettingsProvider;
75-
private final SecureTransportSettingsProvider.ServerExceptionHandler exceptionHandler;
76+
private final TransportExceptionHandler exceptionHandler;
7677

7778
public SecureNetty4Transport(
7879
final Settings settings,
@@ -100,7 +101,7 @@ public SecureNetty4Transport(
100101

101102
this.secureTransportSettingsProvider = secureTransportSettingsProvider;
102103
this.exceptionHandler = secureTransportSettingsProvider.buildServerTransportExceptionHandler(settings, this)
103-
.orElse(SecureTransportSettingsProvider.ServerExceptionHandler.NOOP);
104+
.orElse(TransportExceptionHandler.NOOP);
104105
}
105106

106107
@Override

‎modules/transport-netty4/src/test/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransportTests.java

+12-34
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,8 @@
2929
import org.opensearch.http.HttpTransportSettings;
3030
import org.opensearch.http.NullDispatcher;
3131
import org.opensearch.http.netty4.Netty4HttpClient;
32-
import org.opensearch.plugins.SecureTransportSettingsProvider;
32+
import org.opensearch.plugins.SecureHttpTransportSettingsProvider;
33+
import org.opensearch.plugins.TransportExceptionHandler;
3334
import org.opensearch.rest.BytesRestResponse;
3435
import org.opensearch.rest.RestChannel;
3536
import org.opensearch.rest.RestRequest;
@@ -40,7 +41,6 @@
4041
import org.opensearch.threadpool.ThreadPool;
4142
import org.opensearch.transport.NettyAllocator;
4243
import org.opensearch.transport.SharedGroupFactory;
43-
import org.opensearch.transport.TcpTransport;
4444
import org.opensearch.transport.netty4.ssl.TrustAllManager;
4545
import org.junit.After;
4646
import org.junit.Before;
@@ -83,7 +83,6 @@
8383
import io.netty.handler.codec.http.HttpResponseStatus;
8484
import io.netty.handler.codec.http.HttpUtil;
8585
import io.netty.handler.codec.http.HttpVersion;
86-
import io.netty.handler.ssl.ClientAuth;
8786
import io.netty.handler.ssl.SslContextBuilder;
8887

8988
import static org.opensearch.core.rest.RestStatus.BAD_REQUEST;
@@ -104,7 +103,7 @@ public class SecureNetty4HttpServerTransportTests extends OpenSearchTestCase {
104103
private ThreadPool threadPool;
105104
private MockBigArrays bigArrays;
106105
private ClusterSettings clusterSettings;
107-
private SecureTransportSettingsProvider secureTransportSettingsProvider;
106+
private SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider;
108107

109108
@Before
110109
public void setup() throws Exception {
@@ -113,14 +112,9 @@ public void setup() throws Exception {
113112
bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService());
114113
clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
115114

116-
secureTransportSettingsProvider = new SecureTransportSettingsProvider() {
115+
secureHttpTransportSettingsProvider = new SecureHttpTransportSettingsProvider() {
117116
@Override
118-
public Optional<ServerExceptionHandler> buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) {
119-
return Optional.empty();
120-
}
121-
122-
@Override
123-
public Optional<ServerExceptionHandler> buildServerTransportExceptionHandler(Settings settings, TcpTransport transport) {
117+
public Optional<TransportExceptionHandler> buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) {
124118
return Optional.empty();
125119
}
126120

@@ -146,22 +140,6 @@ public Optional<SSLEngine> buildSecureHttpServerEngine(Settings settings, HttpSe
146140
throw new SSLException(ex);
147141
}
148142
}
149-
150-
@Override
151-
public Optional<SSLEngine> buildSecureServerTransportEngine(Settings settings, TcpTransport transport) throws SSLException {
152-
return Optional.empty();
153-
}
154-
155-
@Override
156-
public Optional<SSLEngine> buildSecureClientTransportEngine(Settings settings, String hostname, int port) throws SSLException {
157-
return Optional.of(
158-
SslContextBuilder.forClient()
159-
.clientAuth(ClientAuth.NONE)
160-
.trustManager(TrustAllManager.INSTANCE)
161-
.build()
162-
.newEngine(NettyAllocator.getAllocator())
163-
);
164-
}
165143
};
166144
}
167145

@@ -241,7 +219,7 @@ public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext,
241219
dispatcher,
242220
clusterSettings,
243221
new SharedGroupFactory(settings),
244-
secureTransportSettingsProvider,
222+
secureHttpTransportSettingsProvider,
245223
NoopTracer.INSTANCE
246224
)
247225
) {
@@ -292,7 +270,7 @@ public void testBindUnavailableAddress() {
292270
new NullDispatcher(),
293271
clusterSettings,
294272
new SharedGroupFactory(Settings.EMPTY),
295-
secureTransportSettingsProvider,
273+
secureHttpTransportSettingsProvider,
296274
NoopTracer.INSTANCE
297275
)
298276
) {
@@ -312,7 +290,7 @@ public void testBindUnavailableAddress() {
312290
new NullDispatcher(),
313291
clusterSettings,
314292
new SharedGroupFactory(settings),
315-
secureTransportSettingsProvider,
293+
secureHttpTransportSettingsProvider,
316294
NoopTracer.INSTANCE
317295
)
318296
) {
@@ -366,7 +344,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
366344
dispatcher,
367345
clusterSettings,
368346
new SharedGroupFactory(settings),
369-
secureTransportSettingsProvider,
347+
secureHttpTransportSettingsProvider,
370348
NoopTracer.INSTANCE
371349
)
372350
) {
@@ -430,7 +408,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
430408
dispatcher,
431409
clusterSettings,
432410
new SharedGroupFactory(Settings.EMPTY),
433-
secureTransportSettingsProvider,
411+
secureHttpTransportSettingsProvider,
434412
NoopTracer.INSTANCE
435413
)
436414
) {
@@ -487,7 +465,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
487465
dispatcher,
488466
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
489467
new SharedGroupFactory(settings),
490-
secureTransportSettingsProvider,
468+
secureHttpTransportSettingsProvider,
491469
NoopTracer.INSTANCE
492470
)
493471
) {
@@ -562,7 +540,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
562540
dispatcher,
563541
new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS),
564542
new SharedGroupFactory(settings),
565-
secureTransportSettingsProvider,
543+
secureHttpTransportSettingsProvider,
566544
NoopTracer.INSTANCE
567545
)
568546
) {

‎modules/transport-netty4/src/test/java/org/opensearch/transport/netty4/ssl/SimpleSecureNetty4TransportTests.java

+3-31
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@
2020
import org.opensearch.core.common.io.stream.NamedWriteableRegistry;
2121
import org.opensearch.core.common.transport.TransportAddress;
2222
import org.opensearch.core.indices.breaker.NoneCircuitBreakerService;
23-
import org.opensearch.http.HttpServerTransport;
2423
import org.opensearch.plugins.SecureTransportSettingsProvider;
24+
import org.opensearch.plugins.TransportExceptionHandler;
2525
import org.opensearch.telemetry.tracing.noop.NoopTracer;
2626
import org.opensearch.test.transport.MockTransportService;
2727
import org.opensearch.test.transport.StubbableTransport;
@@ -69,40 +69,12 @@ protected Transport build(Settings settings, final Version version, ClusterSetti
6969
NamedWriteableRegistry namedWriteableRegistry = new NamedWriteableRegistry(Collections.emptyList());
7070
final SecureTransportSettingsProvider secureTransportSettingsProvider = new SecureTransportSettingsProvider() {
7171
@Override
72-
public Optional<ServerExceptionHandler> buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) {
72+
public Optional<TransportExceptionHandler> buildServerTransportExceptionHandler(Settings settings, Transport transport) {
7373
return Optional.empty();
7474
}
7575

7676
@Override
77-
public Optional<ServerExceptionHandler> buildServerTransportExceptionHandler(Settings settings, TcpTransport transport) {
78-
return Optional.empty();
79-
}
80-
81-
@Override
82-
public Optional<SSLEngine> buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException {
83-
try {
84-
final KeyStore keyStore = KeyStore.getInstance("PKCS12");
85-
keyStore.load(
86-
SimpleSecureNetty4TransportTests.class.getResourceAsStream("/netty4-secure.jks"),
87-
"password".toCharArray()
88-
);
89-
90-
final KeyManagerFactory keyManagerFactory = KeyManagerFactory.getInstance("SunX509");
91-
keyManagerFactory.init(keyStore, "password".toCharArray());
92-
93-
SSLEngine engine = SslContextBuilder.forServer(keyManagerFactory)
94-
.trustManager(TrustAllManager.INSTANCE)
95-
.build()
96-
.newEngine(NettyAllocator.getAllocator());
97-
return Optional.of(engine);
98-
} catch (final IOException | NoSuchAlgorithmException | UnrecoverableKeyException | KeyStoreException
99-
| CertificateException ex) {
100-
throw new SSLException(ex);
101-
}
102-
}
103-
104-
@Override
105-
public Optional<SSLEngine> buildSecureServerTransportEngine(Settings settings, TcpTransport transport) throws SSLException {
77+
public Optional<SSLEngine> buildSecureServerTransportEngine(Settings settings, Transport transport) throws SSLException {
10678
try {
10779
final KeyStore keyStore = KeyStore.getInstance("PKCS12");
10880
keyStore.load(

0 commit comments

Comments
 (0)