Skip to content

Commit dad027f

Browse files
committed
Ensure support of the transport-nio by security plugin (HTTP) (opensearch-project#16474)
* Ensure support of the transport-nio by security plugin (HTTP) Signed-off-by: Andriy Redko <[email protected]> * Add header verifier and decompressor support of secure NIO transport variant Signed-off-by: Andriy Redko <[email protected]> --------- Signed-off-by: Andriy Redko <[email protected]> (cherry picked from commit b25e10a)
1 parent 2fa8370 commit dad027f

File tree

19 files changed

+966
-35
lines changed

19 files changed

+966
-35
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
## [Unreleased 2.x]
77
### Added
88
- Add support for restoring from snapshot with search replicas ([#16111](https://github.com/opensearch-project/OpenSearch/pull/16111))
9+
- Ensure support of the transport-nio by security plugin ([#16474](https://github.com/opensearch-project/OpenSearch/pull/16474))
910
- Switch from `buildSrc/version.properties` to Gradle version catalog (`gradle/libs.versions.toml`) to enable dependabot to perform automated upgrades on common libs ([#16284](https://github.com/opensearch-project/OpenSearch/pull/16284))
1011
- Add dynamic setting allowing size > 0 requests to be cached in the request cache ([#16483](https://github.com/opensearch-project/OpenSearch/pull/16483/files))
1112

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,8 +63,8 @@
6363
* @see <a href="https://github.com/opensearch-project/security/blob/d526c9f6c2a438c14db8b413148204510b9fe2e2/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java">SecuritySSLNettyHttpServerTransport</a>
6464
*/
6565
public class SecureNetty4HttpServerTransport extends Netty4HttpServerTransport {
66-
public static final String REQUEST_HEADER_VERIFIER = "HeaderVerifier";
67-
public static final String REQUEST_DECOMPRESSOR = "RequestDecompressor";
66+
public static final String REQUEST_HEADER_VERIFIER = SecureHttpTransportSettingsProvider.REQUEST_HEADER_VERIFIER;
67+
public static final String REQUEST_DECOMPRESSOR = SecureHttpTransportSettingsProvider.REQUEST_DECOMPRESSOR;
6868

6969
private static final Logger logger = LogManager.getLogger(SecureNetty4HttpServerTransport.class);
7070
private final SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider;

plugins/transport-nio/build.gradle

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ dependencies {
5050
api "io.netty:netty-handler:${versions.netty}"
5151
api "io.netty:netty-resolver:${versions.netty}"
5252
api "io.netty:netty-transport:${versions.netty}"
53+
api "io.netty:netty-transport-native-unix-common:${versions.netty}"
5354
}
5455

5556
tasks.named("dependencyLicenses").configure {
@@ -151,10 +152,6 @@ thirdPartyAudit {
151152
'io.netty.internal.tcnative.SessionTicketKey',
152153
'io.netty.internal.tcnative.SniHostNameMatcher',
153154

154-
// from io.netty.channel.unix (netty)
155-
'io.netty.channel.unix.FileDescriptor',
156-
'io.netty.channel.unix.UnixChannel',
157-
158155
'reactor.blockhound.BlockHound$Builder',
159156
'reactor.blockhound.integration.BlockHoundIntegration'
160157
)
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
d1171bb99411f282068f49d780cedf8c9adeabfd

plugins/transport-nio/src/internalClusterTest/java/org/opensearch/http/nio/NioPipeliningIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,8 @@ public void testThatNioHttpServerSupportsPipelining() throws Exception {
6161
TransportAddress[] boundAddresses = httpServerTransport.boundAddress().boundAddresses();
6262
TransportAddress transportAddress = randomFrom(boundAddresses);
6363

64-
try (NioHttpClient nettyHttpClient = new NioHttpClient()) {
65-
Collection<FullHttpResponse> responses = nettyHttpClient.get(transportAddress.address(), requests);
64+
try (NioHttpClient client = NioHttpClient.http()) {
65+
Collection<FullHttpResponse> responses = client.get(transportAddress.address(), requests);
6666
assertThat(responses, hasSize(5));
6767

6868
Collection<String> opaqueIds = NioHttpClient.returnOpaqueIds(responses);

plugins/transport-nio/src/main/java/org/opensearch/http/nio/HttpReadWriteHandler.java

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
package org.opensearch.http.nio;
3434

35+
import org.opensearch.common.Nullable;
3536
import org.opensearch.common.unit.TimeValue;
3637
import org.opensearch.http.HttpHandlingSettings;
3738
import org.opensearch.http.HttpPipelinedRequest;
@@ -44,6 +45,8 @@
4445
import org.opensearch.nio.TaskScheduler;
4546
import org.opensearch.nio.WriteOperation;
4647

48+
import javax.net.ssl.SSLEngine;
49+
4750
import java.io.IOException;
4851
import java.util.ArrayList;
4952
import java.util.List;
@@ -58,6 +61,7 @@
5861
import io.netty.handler.codec.http.HttpObjectAggregator;
5962
import io.netty.handler.codec.http.HttpRequestDecoder;
6063
import io.netty.handler.codec.http.HttpResponseEncoder;
64+
import io.netty.handler.ssl.SslHandler;
6165

6266
public class HttpReadWriteHandler implements NioChannelHandler {
6367

@@ -77,6 +81,28 @@ public HttpReadWriteHandler(
7781
HttpHandlingSettings settings,
7882
TaskScheduler taskScheduler,
7983
LongSupplier nanoClock
84+
) {
85+
this(
86+
nioHttpChannel,
87+
transport,
88+
settings,
89+
taskScheduler,
90+
nanoClock,
91+
null, /* no header verifier */
92+
new HttpContentDecompressor(),
93+
null /* no SSL/TLS */
94+
);
95+
}
96+
97+
HttpReadWriteHandler(
98+
NioHttpChannel nioHttpChannel,
99+
NioHttpServerTransport transport,
100+
HttpHandlingSettings settings,
101+
TaskScheduler taskScheduler,
102+
LongSupplier nanoClock,
103+
@Nullable ChannelHandler headerVerifier,
104+
ChannelHandler decompressor,
105+
@Nullable SSLEngine sslEngine
80106
) {
81107
this.nioHttpChannel = nioHttpChannel;
82108
this.transport = transport;
@@ -85,14 +111,23 @@ public HttpReadWriteHandler(
85111
this.readTimeoutNanos = TimeUnit.MILLISECONDS.toNanos(settings.getReadTimeoutMillis());
86112

87113
List<ChannelHandler> handlers = new ArrayList<>(8);
114+
115+
SslHandler sslHandler = null;
116+
if (sslEngine != null) {
117+
sslHandler = new SslHandler(sslEngine);
118+
}
119+
88120
HttpRequestDecoder decoder = new HttpRequestDecoder(
89121
settings.getMaxInitialLineLength(),
90122
settings.getMaxHeaderSize(),
91123
settings.getMaxChunkSize()
92124
);
93125
decoder.setCumulator(ByteToMessageDecoder.COMPOSITE_CUMULATOR);
94126
handlers.add(decoder);
95-
handlers.add(new HttpContentDecompressor());
127+
if (headerVerifier != null) {
128+
handlers.add(headerVerifier);
129+
}
130+
handlers.add(decompressor);
96131
handlers.add(new HttpResponseEncoder());
97132
handlers.add(new HttpObjectAggregator(settings.getMaxContentLength()));
98133
if (settings.isCompression()) {
@@ -102,7 +137,7 @@ public HttpReadWriteHandler(
102137
handlers.add(new NioHttpResponseCreator());
103138
handlers.add(new NioHttpPipeliningHandler(transport.getLogger(), settings.getPipeliningMaxEvents()));
104139

105-
adaptor = new NettyAdaptor(handlers.toArray(new ChannelHandler[0]));
140+
adaptor = new NettyAdaptor(sslHandler, handlers.toArray(new ChannelHandler[0]));
106141
adaptor.addCloseListener((v, e) -> nioHttpChannel.close());
107142
}
108143

plugins/transport-nio/src/main/java/org/opensearch/http/nio/NettyAdaptor.java

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
package org.opensearch.http.nio;
3434

3535
import org.opensearch.ExceptionsHelper;
36+
import org.opensearch.common.Nullable;
3637
import org.opensearch.nio.FlushOperation;
3738
import org.opensearch.nio.Page;
3839
import org.opensearch.nio.WriteOperation;
@@ -49,16 +50,21 @@
4950
import io.netty.channel.ChannelOutboundHandlerAdapter;
5051
import io.netty.channel.ChannelPromise;
5152
import io.netty.channel.embedded.EmbeddedChannel;
53+
import io.netty.handler.ssl.SslHandler;
5254

5355
class NettyAdaptor {
5456

5557
private final EmbeddedChannel nettyChannel;
5658
private final LinkedList<FlushOperation> flushOperations = new LinkedList<>();
5759

5860
NettyAdaptor(ChannelHandler... handlers) {
59-
nettyChannel = new EmbeddedChannel();
60-
nettyChannel.pipeline().addLast("write_captor", new ChannelOutboundHandlerAdapter() {
61+
this(null, handlers);
62+
}
6163

64+
NettyAdaptor(@Nullable SslHandler sslHandler, ChannelHandler... handlers) {
65+
this.nettyChannel = new EmbeddedChannel();
66+
67+
nettyChannel.pipeline().addLast("write_captor", new ChannelOutboundHandlerAdapter() {
6268
@Override
6369
public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise) {
6470
// This is a little tricky. The embedded channel will complete the promise once it writes the message
@@ -75,12 +81,22 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
7581
}
7682
}
7783
});
84+
if (sslHandler != null) {
85+
nettyChannel.pipeline().addAfter("write_captor", "ssl_handler", sslHandler);
86+
}
7887
nettyChannel.pipeline().addLast(handlers);
7988
}
8089

8190
public void close() throws Exception {
8291
assert flushOperations.isEmpty() : "Should close outbound operations before calling close";
8392

93+
final SslHandler sslHandler = (SslHandler) nettyChannel.pipeline().get("ssl_handler");
94+
if (sslHandler != null) {
95+
// The nettyChannel.close() or sslHandler.closeOutbound() futures will block indefinitely,
96+
// removing the handler instead from the channel.
97+
nettyChannel.pipeline().remove(sslHandler);
98+
}
99+
84100
ChannelFuture closeFuture = nettyChannel.close();
85101
// This should be safe as we are not a real network channel
86102
closeFuture.await();

0 commit comments

Comments
 (0)