Skip to content

Commit 6cd5aef

Browse files
retareta
committed
[BUG] Cannot communicate with http2 when reactor-netty is enabled
Signed-off-by: Andriy Redko <[email protected]>
1 parent 6bf1a6d commit 6cd5aef

File tree

4 files changed

+84
-70
lines changed

4 files changed

+84
-70
lines changed

‎plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ReactorNetty4HttpServerTransport.java

Lines changed: 33 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
package org.opensearch.http.reactor.netty4;
1010

11+
import org.opensearch.OpenSearchException;
1112
import org.opensearch.common.Nullable;
1213
import org.opensearch.common.network.NetworkService;
1314
import org.opensearch.common.settings.ClusterSettings;
@@ -27,35 +28,34 @@
2728
import org.opensearch.http.HttpServerChannel;
2829
import org.opensearch.http.reactor.netty4.ssl.SslUtils;
2930
import org.opensearch.plugins.SecureHttpTransportSettingsProvider;
31+
import org.opensearch.plugins.SecureHttpTransportSettingsProvider.SecureHttpTransportParameters;
3032
import org.opensearch.rest.RestHandler;
3133
import org.opensearch.rest.RestRequest.Method;
3234
import org.opensearch.telemetry.tracing.Tracer;
3335
import org.opensearch.threadpool.ThreadPool;
3436
import org.opensearch.transport.reactor.SharedGroupFactory;
3537
import org.opensearch.transport.reactor.netty4.Netty4Utils;
3638

37-
import javax.net.ssl.SSLEngine;
38-
import javax.net.ssl.SSLException;
39-
import javax.net.ssl.SSLSessionContext;
39+
import javax.net.ssl.KeyManagerFactory;
4040

4141
import java.net.InetSocketAddress;
4242
import java.net.SocketOption;
4343
import java.time.Duration;
4444
import java.util.Arrays;
45-
import java.util.List;
4645
import java.util.Optional;
4746

4847
import io.netty.buffer.ByteBuf;
49-
import io.netty.buffer.ByteBufAllocator;
5048
import io.netty.channel.ChannelOption;
5149
import io.netty.channel.socket.nio.NioChannelOption;
5250
import io.netty.handler.codec.http.DefaultLastHttpContent;
5351
import io.netty.handler.codec.http.FullHttpResponse;
5452
import io.netty.handler.codec.http.HttpContent;
55-
import io.netty.handler.ssl.ApplicationProtocolNegotiator;
53+
import io.netty.handler.ssl.ApplicationProtocolConfig;
54+
import io.netty.handler.ssl.ApplicationProtocolNames;
5655
import io.netty.handler.ssl.SslContext;
56+
import io.netty.handler.ssl.SslContextBuilder;
57+
import io.netty.handler.ssl.SupportedCipherSuiteFilter;
5758
import io.netty.handler.timeout.ReadTimeoutException;
58-
import io.netty.util.ReferenceCountUtil;
5959
import org.reactivestreams.Publisher;
6060
import reactor.core.publisher.Mono;
6161
import reactor.core.scheduler.Scheduler;
@@ -306,59 +306,33 @@ private HttpServer configure(final HttpServer server) throws Exception {
306306

307307
// Configure SSL context if available
308308
if (secureHttpTransportSettingsProvider != null) {
309-
final SSLEngine engine = secureHttpTransportSettingsProvider.buildSecureHttpServerEngine(settings, this)
310-
.orElseGet(SslUtils::createDefaultServerSSLEngine);
311-
312-
try {
313-
final List<String> cipherSuites = Arrays.asList(engine.getEnabledCipherSuites());
314-
final List<String> applicationProtocols = Arrays.asList(engine.getSSLParameters().getApplicationProtocols());
315-
316-
configured = configured.secure(spec -> spec.sslContext(new SslContext() {
317-
@Override
318-
public SSLSessionContext sessionContext() {
319-
throw new UnsupportedOperationException(); /* server only, should never be called */
320-
}
321-
322-
@Override
323-
public SSLEngine newEngine(ByteBufAllocator alloc, String peerHost, int peerPort) {
324-
throw new UnsupportedOperationException(); /* server only, should never be called */
325-
}
326-
327-
@Override
328-
public SSLEngine newEngine(ByteBufAllocator alloc) {
329-
try {
330-
return secureHttpTransportSettingsProvider.buildSecureHttpServerEngine(
331-
settings,
332-
ReactorNetty4HttpServerTransport.this
333-
).orElseGet(SslUtils::createDefaultServerSSLEngine);
334-
} catch (final SSLException ex) {
335-
throw new UnsupportedOperationException("Unable to create SSLEngine", ex);
336-
}
337-
}
338-
339-
@Override
340-
public boolean isClient() {
341-
return false; /* server only */
342-
}
343-
344-
@Override
345-
public List<String> cipherSuites() {
346-
return cipherSuites;
347-
}
309+
final Optional<SecureHttpTransportParameters> parameters = secureHttpTransportSettingsProvider.parameters(settings);
310+
311+
final KeyManagerFactory keyManagerFactory = parameters.flatMap(SecureHttpTransportParameters::keyManagerFactory)
312+
.orElseThrow(() -> new OpenSearchException("The KeyManagerFactory instance is not provided"));
313+
314+
final SslContextBuilder sslContextBuilder = SslContextBuilder.forServer(keyManagerFactory);
315+
parameters.flatMap(SecureHttpTransportParameters::trustManagerFactory).ifPresent(sslContextBuilder::trustManager);
316+
parameters.map(SecureHttpTransportParameters::cipherSuites)
317+
.ifPresent(ciphers -> sslContextBuilder.ciphers(ciphers, SupportedCipherSuiteFilter.INSTANCE));
318+
319+
final SslContext sslContext = sslContextBuilder.protocols(
320+
parameters.map(SecureHttpTransportParameters::protocols).orElseGet(() -> Arrays.asList(SslUtils.DEFAULT_SSL_PROTOCOLS))
321+
)
322+
.applicationProtocolConfig(
323+
new ApplicationProtocolConfig(
324+
ApplicationProtocolConfig.Protocol.ALPN,
325+
// NO_ADVERTISE is currently the only mode supported by both OpenSsl and JDK providers.
326+
ApplicationProtocolConfig.SelectorFailureBehavior.NO_ADVERTISE,
327+
// ACCEPT is currently the only mode supported by both OpenSsl and JDK providers.
328+
ApplicationProtocolConfig.SelectedListenerFailureBehavior.ACCEPT,
329+
ApplicationProtocolNames.HTTP_2,
330+
ApplicationProtocolNames.HTTP_1_1
331+
)
332+
)
333+
.build();
348334

349-
@Override
350-
public ApplicationProtocolNegotiator applicationProtocolNegotiator() {
351-
return new ApplicationProtocolNegotiator() {
352-
@Override
353-
public List<String> protocols() {
354-
return applicationProtocols;
355-
}
356-
};
357-
}
358-
}).build()).protocol(HttpProtocol.HTTP11, HttpProtocol.H2);
359-
} finally {
360-
ReferenceCountUtil.release(engine);
361-
}
335+
configured = configured.secure(spec -> spec.sslContext(sslContext)).protocol(HttpProtocol.HTTP11, HttpProtocol.H2);
362336
} else {
363337
configured = configured.protocol(HttpProtocol.HTTP11, HttpProtocol.H2C);
364338
}

‎plugins/transport-reactor-netty4/src/main/java/org/opensearch/http/reactor/netty4/ssl/SslUtils.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@
2121
* Helper class for creating default SSL engines
2222
*/
2323
public class SslUtils {
24-
private static final String[] DEFAULT_SSL_PROTOCOLS = { "TLSv1.3", "TLSv1.2", "TLSv1.1" };
24+
/**
25+
* Default support TLS protocols
26+
*/
27+
public static final String[] DEFAULT_SSL_PROTOCOLS = { "TLSv1.3", "TLSv1.2", "TLSv1.1" };
2528

2629
private SslUtils() {}
2730

‎plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ReactorHttpClient.java

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import org.opensearch.core.xcontent.ToXContent;
1919
import org.opensearch.core.xcontent.XContentBuilder;
2020
import org.opensearch.tasks.Task;
21-
import org.opensearch.test.OpenSearchTestCase;
2221

2322
import java.io.Closeable;
2423
import java.io.IOException;
@@ -52,8 +51,8 @@
5251
import reactor.core.publisher.Flux;
5352
import reactor.core.publisher.Mono;
5453
import reactor.core.publisher.ParallelFlux;
55-
import reactor.netty.http.Http11SslContextSpec;
5654
import reactor.netty.http.Http2SslContextSpec;
55+
import reactor.netty.http.HttpProtocol;
5756
import reactor.netty.http.client.HttpClient;
5857

5958
import static io.netty.handler.codec.http.HttpHeaderNames.HOST;
@@ -265,16 +264,14 @@ private HttpClient createClient(final InetSocketAddress remoteAddress, final Nio
265264
.runOn(eventLoopGroup)
266265
.host(remoteAddress.getHostString())
267266
.port(remoteAddress.getPort())
268-
.compress(compression);
267+
.compress(compression)
268+
.protocol(HttpProtocol.H2, HttpProtocol.HTTP11);
269269

270270
if (secure) {
271271
return client.secure(
272272
spec -> spec.sslContext(
273-
OpenSearchTestCase.randomBoolean()
274-
/* switch between HTTP 1.1/HTTP 2 randomly, both are supported */ ? Http11SslContextSpec.forClient()
275-
.configure(s -> s.clientAuth(ClientAuth.NONE).trustManager(InsecureTrustManagerFactory.INSTANCE))
276-
: Http2SslContextSpec.forClient()
277-
.configure(s -> s.clientAuth(ClientAuth.NONE).trustManager(InsecureTrustManagerFactory.INSTANCE))
273+
Http2SslContextSpec.forClient()
274+
.configure(s -> s.clientAuth(ClientAuth.NONE).trustManager(InsecureTrustManagerFactory.INSTANCE))
278275
)
279276
);
280277
}

‎plugins/transport-reactor-netty4/src/test/java/org/opensearch/http/reactor/netty4/ssl/SecureReactorNetty4HttpServerTransportTests.java

Lines changed: 42 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,11 @@
4848
import javax.net.ssl.KeyManagerFactory;
4949
import javax.net.ssl.SSLEngine;
5050
import javax.net.ssl.SSLException;
51+
import javax.net.ssl.TrustManagerFactory;
5152

5253
import java.nio.charset.StandardCharsets;
54+
import java.util.Arrays;
55+
import java.util.Collection;
5356
import java.util.Collections;
5457
import java.util.Optional;
5558
import java.util.concurrent.CountDownLatch;
@@ -80,6 +83,7 @@
8083
import io.netty.handler.codec.http.HttpResponseStatus;
8184
import io.netty.handler.codec.http.HttpUtil;
8285
import io.netty.handler.codec.http.HttpVersion;
86+
import io.netty.handler.codec.http2.Http2SecurityUtil;
8387
import io.netty.handler.ssl.SslContextBuilder;
8488
import io.netty.handler.ssl.util.InsecureTrustManagerFactory;
8589

@@ -108,7 +112,45 @@ public void setup() throws Exception {
108112
bigArrays = new MockBigArrays(new MockPageCacheRecycler(Settings.EMPTY), new NoneCircuitBreakerService());
109113
clusterSettings = new ClusterSettings(Settings.EMPTY, ClusterSettings.BUILT_IN_CLUSTER_SETTINGS);
110114

115+
var keyManagerFactory = KeyManagerFactory.getInstance("PKIX");
116+
keyManagerFactory.init(KeyStoreUtils.createServerKeyStore(), KEYSTORE_PASSWORD);
117+
111118
secureHttpTransportSettingsProvider = new SecureHttpTransportSettingsProvider() {
119+
@Override
120+
public Optional<SecureHttpTransportParameters> parameters(Settings settings) {
121+
return Optional.of(new SecureHttpTransportParameters() {
122+
@Override
123+
public Optional<KeyManagerFactory> keyManagerFactory() {
124+
return Optional.of(keyManagerFactory);
125+
}
126+
127+
@Override
128+
public Optional<String> sslProvider() {
129+
return Optional.empty();
130+
}
131+
132+
@Override
133+
public Optional<String> clientAuth() {
134+
return Optional.empty();
135+
}
136+
137+
@Override
138+
public Collection<String> protocols() {
139+
return Arrays.asList(SslUtils.DEFAULT_SSL_PROTOCOLS);
140+
}
141+
142+
@Override
143+
public Collection<String> cipherSuites() {
144+
return Http2SecurityUtil.CIPHERS;
145+
}
146+
147+
@Override
148+
public Optional<TrustManagerFactory> trustManagerFactory() {
149+
return Optional.of(InsecureTrustManagerFactory.INSTANCE);
150+
}
151+
});
152+
}
153+
112154
@Override
113155
public Optional<TransportExceptionHandler> buildHttpServerExceptionHandler(Settings settings, HttpServerTransport transport) {
114156
return Optional.empty();
@@ -117,8 +159,6 @@ public Optional<TransportExceptionHandler> buildHttpServerExceptionHandler(Setti
117159
@Override
118160
public Optional<SSLEngine> buildSecureHttpServerEngine(Settings settings, HttpServerTransport transport) throws SSLException {
119161
try {
120-
var keyManagerFactory = KeyManagerFactory.getInstance("PKIX");
121-
keyManagerFactory.init(KeyStoreUtils.createServerKeyStore(), KEYSTORE_PASSWORD);
122162
SSLEngine engine = SslContextBuilder.forServer(keyManagerFactory)
123163
.trustManager(InsecureTrustManagerFactory.INSTANCE)
124164
.build()

0 commit comments

Comments
 (0)