From d0d88040af06614fffb4fddf79f4f40bfe55dbcc Mon Sep 17 00:00:00 2001 From: Andriy Redko Date: Fri, 26 Aug 2022 11:00:20 -0400 Subject: [PATCH] Addressing code review comments Signed-off-by: Andriy Redko --- .../security/ssl/DefaultSecurityKeyStore.java | 39 +++++++------------ .../SecuritySSLNettyHttpServerTransport.java | 2 +- 2 files changed, 16 insertions(+), 25 deletions(-) diff --git a/src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java b/src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java index de9854c788..54dfaf6306 100644 --- a/src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java +++ b/src/main/java/org/opensearch/security/ssl/DefaultSecurityKeyStore.java @@ -887,24 +887,8 @@ private SslContext buildSSLServerContext(final PrivateKey _key, final X509Certif final X509Certificate[] _trustedCerts, final Iterable ciphers, final SslProvider sslProvider, final ClientAuth authMode) throws SSLException { - final SslContextBuilder _sslContextBuilder = SslContextBuilder.forServer(_key, _cert) - .ciphers(Stream - .concat( - Http2SecurityUtil.CIPHERS.stream(), - StreamSupport.stream(ciphers.spliterator(), false)) - .collect(Collectors.toSet()), SupportedCipherSuiteFilter.INSTANCE) - .clientAuth(Objects.requireNonNull(authMode)) // https://github.com/netty/netty/issues/4722 - .sessionCacheSize(0).sessionTimeout(0).sslProvider(sslProvider) - .applicationProtocolConfig( - new ApplicationProtocolConfig( - Protocol.ALPN, - // NO_ADVERTISE is currently the only mode supported by both OpenSsl and JDK providers. - SelectorFailureBehavior.NO_ADVERTISE, - // ACCEPT is currently the only mode supported by both OpenSsl and JDK providers. - SelectedListenerFailureBehavior.ACCEPT, - ApplicationProtocolNames.HTTP_2, - ApplicationProtocolNames.HTTP_1_1 - )); + final SslContextBuilder _sslContextBuilder = configureSSLServerContextBuilder(SslContextBuilder.forServer(_key, _cert), + sslProvider, ciphers, authMode); if (_trustedCerts != null && _trustedCerts.length > 0) { _sslContextBuilder.trustManager(_trustedCerts); @@ -917,7 +901,19 @@ private SslContext buildSSLServerContext(final File _key, final File _cert, fina final String pwd, final Iterable ciphers, final SslProvider sslProvider, final ClientAuth authMode) throws SSLException { - final SslContextBuilder _sslContextBuilder = SslContextBuilder.forServer(_cert, _key, pwd) + final SslContextBuilder _sslContextBuilder = configureSSLServerContextBuilder(SslContextBuilder.forServer(_cert, _key, pwd), + sslProvider, ciphers, authMode); + + if (_trustedCerts != null) { + _sslContextBuilder.trustManager(_trustedCerts); + } + + return buildSSLContext0(_sslContextBuilder); + } + + private SslContextBuilder configureSSLServerContextBuilder(final SslContextBuilder builder, final SslProvider sslProvider, + final Iterable ciphers, final ClientAuth authMode) { + return builder .ciphers(Stream .concat( Http2SecurityUtil.CIPHERS.stream(), @@ -935,11 +931,6 @@ private SslContext buildSSLServerContext(final File _key, final File _cert, fina ApplicationProtocolNames.HTTP_2, ApplicationProtocolNames.HTTP_1_1 )); - if (_trustedCerts != null) { - _sslContextBuilder.trustManager(_trustedCerts); - } - - return buildSSLContext0(_sslContextBuilder); } private SslContext buildSSLClientContext(final PrivateKey _key, final X509Certificate[] _cert, diff --git a/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java b/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java index f56a26d17b..aa191201da 100644 --- a/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java +++ b/src/main/java/org/opensearch/security/ssl/http/netty/SecuritySSLNettyHttpServerTransport.java @@ -108,7 +108,7 @@ protected void initChannel(Channel ch) throws Exception { } @Override - protected void configureDefaultPipeline(Channel ch) { + protected void configurePipeline(Channel ch) { ch.pipeline().addLast(new Http2OrHttpHandler()); } }