Skip to content

Commit

Permalink
[FEATURE] Improve built-in secure transports support
Browse files Browse the repository at this point in the history
Signed-off-by: Andriy Redko <[email protected]>
  • Loading branch information
reta committed Mar 26, 2024
1 parent 10fc755 commit cac3d99
Show file tree
Hide file tree
Showing 15 changed files with 312 additions and 155 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,19 +37,25 @@
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.Optional;

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;
Expand All @@ -59,9 +65,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 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,
Expand All @@ -72,7 +83,7 @@ public SecureNetty4HttpServerTransport(
final Dispatcher dispatcher,
final ClusterSettings clusterSettings,
final SharedGroupFactory sharedGroupFactory,
final SecureTransportSettingsProvider secureTransportSettingsProvider,
final SecureHttpTransportSettingsProvider secureHttpTransportSettingsProvider,
final Tracer tracer
) {
super(
Expand All @@ -86,9 +97,33 @@ 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);

this.headerVerifier = secureHttpTransportSettingsProvider.getHttpTransportAdapterProviders(settings)
.stream()
.filter(p -> HEADER_VERIFIER.equalsIgnoreCase(p.name()))
.findFirst()
.flatMap(p -> p.create(settings, this, ChannelInboundHandlerAdapter.class))
.orElse(null);

this.decompressorProvider = secureHttpTransportSettingsProvider.getHttpTransportAdapterProviders(settings)
.stream()
.filter(p -> REQUEST_DECOMPRESSOR.equalsIgnoreCase(p.name()))
.findFirst()
.orElseGet(() -> new TransportAdapterProvider<HttpServerTransport>() {
@Override
public String name() {
return REQUEST_DECOMPRESSOR;

Check warning on line 119 in modules/transport-netty4/src/main/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransport.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L119 was not covered by tests
}

@Override
public <C> Optional<C> create(Settings settings, HttpServerTransport transport, Class<C> adapterClass) {
return Optional.empty();
}
});
}

@Override
Expand Down Expand Up @@ -152,7 +187,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);
Expand All @@ -166,4 +201,17 @@ protected void configurePipeline(Channel ch) {
ch.pipeline().addLast(new Http2OrHttpHandler());
}
}

protected ChannelInboundHandlerAdapter createHeaderVerifier() {
if (headerVerifier != null) {
return headerVerifier;

Check warning on line 207 in modules/transport-netty4/src/main/java/org/opensearch/http/netty4/ssl/SecureNetty4HttpServerTransport.java

View check run for this annotation

Codecov / codecov/patch

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

Added line #L207 was not covered by tests
} else {
return super.createHeaderVerifier();
}
}

@Override
protected ChannelInboundHandlerAdapter createDecompressor() {
return decompressorProvider.create(settings, this, ChannelInboundHandlerAdapter.class).orElseGet(super::createDecompressor);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -174,7 +175,7 @@ public Map<String, Supplier<HttpServerTransport>> getSecureHttpTransports(
dispatcher,
clusterSettings,
getSharedGroupFactory(settings),
secureTransportSettingsProvider,
secureHttpTransportSettingsProvider,
tracer
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -100,7 +101,7 @@ public SecureNetty4Transport(

this.secureTransportSettingsProvider = secureTransportSettingsProvider;
this.exceptionHandler = secureTransportSettingsProvider.buildServerTransportExceptionHandler(settings, this)
.orElse(SecureTransportSettingsProvider.ServerExceptionHandler.NOOP);
.orElse(TransportExceptionHandler.NOOP);
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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();
}

Expand All @@ -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())
);
}
};
}

Expand Down Expand Up @@ -241,7 +219,7 @@ public void dispatchBadRequest(RestChannel channel, ThreadContext threadContext,
dispatcher,
clusterSettings,
new SharedGroupFactory(settings),
secureTransportSettingsProvider,
secureHttpTransportSettingsProvider,
NoopTracer.INSTANCE
)
) {
Expand Down Expand Up @@ -292,7 +270,7 @@ public void testBindUnavailableAddress() {
new NullDispatcher(),
clusterSettings,
new SharedGroupFactory(Settings.EMPTY),
secureTransportSettingsProvider,
secureHttpTransportSettingsProvider,
NoopTracer.INSTANCE
)
) {
Expand All @@ -312,7 +290,7 @@ public void testBindUnavailableAddress() {
new NullDispatcher(),
clusterSettings,
new SharedGroupFactory(settings),
secureTransportSettingsProvider,
secureHttpTransportSettingsProvider,
NoopTracer.INSTANCE
)
) {
Expand Down Expand Up @@ -366,7 +344,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
dispatcher,
clusterSettings,
new SharedGroupFactory(settings),
secureTransportSettingsProvider,
secureHttpTransportSettingsProvider,
NoopTracer.INSTANCE
)
) {
Expand Down Expand Up @@ -430,7 +408,7 @@ public void dispatchBadRequest(final RestChannel channel, final ThreadContext th
dispatcher,
clusterSettings,
new SharedGroupFactory(Settings.EMPTY),
secureTransportSettingsProvider,
secureHttpTransportSettingsProvider,
NoopTracer.INSTANCE
)
) {
Expand Down Expand Up @@ -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
)
) {
Expand Down Expand Up @@ -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
)
) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
Loading

0 comments on commit cac3d99

Please sign in to comment.