From 8566fd10191f86487433ece7e4c3ba70383dcf08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Victor=20No=C3=ABl?= Date: Tue, 26 Apr 2016 09:25:30 +0200 Subject: [PATCH] Add startTls parameter to SslContextBuilder Motivation: There is an incoherence in terms of API when one wants to use startTls: without startTls one can use the SslContextBuilder's method newHandler, but with startTls, the developper is forced to call directly the SslHandler constructor. Modifications: Introduce startTls as a SslContextBuilder parameter as well as a member in SslContext (and thus Jdk and OpenSsl implementations!). Always use this information to call the SslHandler constructor. Use false by default, in particular in deprecated constructors of the SSL implementations. The client Context use false by default Results: Fixes #5170 and more generally homogenise the API so that everything can be done via SslContextBuilder. --- .../handler/ssl/JdkSslClientContext.java | 4 +-- .../io/netty/handler/ssl/JdkSslContext.java | 7 +++-- .../handler/ssl/JdkSslServerContext.java | 6 ++-- .../handler/ssl/OpenSslClientContext.java | 2 +- .../io/netty/handler/ssl/OpenSslContext.java | 9 +++--- .../handler/ssl/OpenSslServerContext.java | 10 +++--- .../ReferenceCountedOpenSslClientContext.java | 2 +- .../ssl/ReferenceCountedOpenSslContext.java | 10 +++--- .../ReferenceCountedOpenSslServerContext.java | 8 ++--- .../java/io/netty/handler/ssl/SniHandler.java | 12 ++++--- .../java/io/netty/handler/ssl/SslContext.java | 31 ++++++++++++------- .../netty/handler/ssl/SslContextBuilder.java | 11 ++++++- 12 files changed, 67 insertions(+), 45 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkSslClientContext.java b/handler/src/main/java/io/netty/handler/ssl/JdkSslClientContext.java index 90e6914439..4a4f04506a 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkSslClientContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkSslClientContext.java @@ -251,7 +251,7 @@ public final class JdkSslClientContext extends JdkSslContext { trustCertCollectionFile), trustManagerFactory, toX509CertificatesInternal(keyCertChainFile), toPrivateKeyInternal(keyFile, keyPassword), keyPassword, keyManagerFactory, sessionCacheSize, sessionTimeout), true, - ciphers, cipherFilter, apn, ClientAuth.NONE); + ciphers, cipherFilter, apn, ClientAuth.NONE, false); } JdkSslClientContext(X509Certificate[] trustCertCollection, TrustManagerFactory trustManagerFactory, @@ -260,7 +260,7 @@ public final class JdkSslClientContext extends JdkSslContext { ApplicationProtocolConfig apn, long sessionCacheSize, long sessionTimeout) throws SSLException { super(newSSLContext(trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory, sessionCacheSize, sessionTimeout), true, - ciphers, cipherFilter, toNegotiator(apn, false), ClientAuth.NONE); + ciphers, cipherFilter, toNegotiator(apn, false), ClientAuth.NONE, false); } private static SSLContext newSSLContext(X509Certificate[] trustCertCollection, diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java b/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java index 2156bacef7..7e64675ff2 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java @@ -153,7 +153,7 @@ public class JdkSslContext extends SslContext { public JdkSslContext(SSLContext sslContext, boolean isClient, ClientAuth clientAuth) { this(sslContext, isClient, null, IdentityCipherSuiteFilter.INSTANCE, - JdkDefaultApplicationProtocolNegotiator.INSTANCE, clientAuth); + JdkDefaultApplicationProtocolNegotiator.INSTANCE, clientAuth, false); } /** @@ -169,11 +169,12 @@ public class JdkSslContext extends SslContext { public JdkSslContext(SSLContext sslContext, boolean isClient, Iterable ciphers, CipherSuiteFilter cipherFilter, ApplicationProtocolConfig apn, ClientAuth clientAuth) { - this(sslContext, isClient, ciphers, cipherFilter, toNegotiator(apn, !isClient), clientAuth); + this(sslContext, isClient, ciphers, cipherFilter, toNegotiator(apn, !isClient), clientAuth, false); } JdkSslContext(SSLContext sslContext, boolean isClient, Iterable ciphers, CipherSuiteFilter cipherFilter, - JdkApplicationProtocolNegotiator apn, ClientAuth clientAuth) { + JdkApplicationProtocolNegotiator apn, ClientAuth clientAuth, boolean startTls) { + super(startTls); this.apn = checkNotNull(apn, "apn"); this.clientAuth = checkNotNull(clientAuth, "clientAuth"); cipherSuites = checkNotNull(cipherFilter, "cipherFilter").filterCipherSuites( diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkSslServerContext.java b/handler/src/main/java/io/netty/handler/ssl/JdkSslServerContext.java index 4434c3a41c..e75f07553b 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkSslServerContext.java @@ -215,17 +215,17 @@ public final class JdkSslServerContext extends JdkSslContext { super(newSSLContext(toX509CertificatesInternal(trustCertCollectionFile), trustManagerFactory, toX509CertificatesInternal(keyCertChainFile), toPrivateKeyInternal(keyFile, keyPassword), keyPassword, keyManagerFactory, sessionCacheSize, sessionTimeout), false, - ciphers, cipherFilter, apn, ClientAuth.NONE); + ciphers, cipherFilter, apn, ClientAuth.NONE, false); } JdkSslServerContext(X509Certificate[] trustCertCollection, TrustManagerFactory trustManagerFactory, X509Certificate[] keyCertChain, PrivateKey key, String keyPassword, KeyManagerFactory keyManagerFactory, Iterable ciphers, CipherSuiteFilter cipherFilter, ApplicationProtocolConfig apn, long sessionCacheSize, long sessionTimeout, - ClientAuth clientAuth) throws SSLException { + ClientAuth clientAuth, boolean startTls) throws SSLException { super(newSSLContext(trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory, sessionCacheSize, sessionTimeout), false, - ciphers, cipherFilter, toNegotiator(apn, true), clientAuth); + ciphers, cipherFilter, toNegotiator(apn, true), clientAuth, startTls); } private static SSLContext newSSLContext(X509Certificate[] trustCertCollection, diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java index bd7a28dd12..bbbbbee512 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslClientContext.java @@ -185,7 +185,7 @@ public final class OpenSslClientContext extends OpenSslContext { long sessionCacheSize, long sessionTimeout) throws SSLException { super(ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, SSL.SSL_MODE_CLIENT, keyCertChain, - ClientAuth.NONE); + ClientAuth.NONE, false); boolean success = false; try { sessionContext = newSessionContext(this, ctx, engineMap, trustCertCollection, trustManagerFactory, diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java index 4f159ce2c2..4cafbfb8f9 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslContext.java @@ -29,17 +29,18 @@ import javax.net.ssl.SSLException; public abstract class OpenSslContext extends ReferenceCountedOpenSslContext { OpenSslContext(Iterable ciphers, CipherSuiteFilter cipherFilter, ApplicationProtocolConfig apnCfg, long sessionCacheSize, long sessionTimeout, int mode, Certificate[] keyCertChain, - ClientAuth clientAuth) + ClientAuth clientAuth, boolean startTls) throws SSLException { super(ciphers, cipherFilter, apnCfg, sessionCacheSize, sessionTimeout, mode, keyCertChain, - clientAuth, false); + clientAuth, startTls, false); } OpenSslContext(Iterable ciphers, CipherSuiteFilter cipherFilter, OpenSslApplicationProtocolNegotiator apn, long sessionCacheSize, long sessionTimeout, int mode, Certificate[] keyCertChain, - ClientAuth clientAuth) throws SSLException { - super(ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, mode, keyCertChain, clientAuth, false); + ClientAuth clientAuth, boolean startTls) throws SSLException { + super(ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, mode, keyCertChain, clientAuth, startTls, + false); } @Override diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java index a71d5d35dd..a65e07dc2a 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslServerContext.java @@ -323,16 +323,16 @@ public final class OpenSslServerContext extends OpenSslContext { this(toX509CertificatesInternal(trustCertCollectionFile), trustManagerFactory, toX509CertificatesInternal(keyCertChainFile), toPrivateKeyInternal(keyFile, keyPassword), keyPassword, keyManagerFactory, ciphers, cipherFilter, - apn, sessionCacheSize, sessionTimeout, ClientAuth.NONE); + apn, sessionCacheSize, sessionTimeout, ClientAuth.NONE, false); } OpenSslServerContext( X509Certificate[] trustCertCollection, TrustManagerFactory trustManagerFactory, X509Certificate[] keyCertChain, PrivateKey key, String keyPassword, KeyManagerFactory keyManagerFactory, Iterable ciphers, CipherSuiteFilter cipherFilter, ApplicationProtocolConfig apn, - long sessionCacheSize, long sessionTimeout, ClientAuth clientAuth) throws SSLException { + long sessionCacheSize, long sessionTimeout, ClientAuth clientAuth, boolean startTls) throws SSLException { this(trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory, ciphers, - cipherFilter, toNegotiator(apn), sessionCacheSize, sessionTimeout, clientAuth); + cipherFilter, toNegotiator(apn), sessionCacheSize, sessionTimeout, clientAuth, startTls); } @SuppressWarnings("deprecation") @@ -340,9 +340,9 @@ public final class OpenSslServerContext extends OpenSslContext { X509Certificate[] trustCertCollection, TrustManagerFactory trustManagerFactory, X509Certificate[] keyCertChain, PrivateKey key, String keyPassword, KeyManagerFactory keyManagerFactory, Iterable ciphers, CipherSuiteFilter cipherFilter, OpenSslApplicationProtocolNegotiator apn, - long sessionCacheSize, long sessionTimeout, ClientAuth clientAuth) throws SSLException { + long sessionCacheSize, long sessionTimeout, ClientAuth clientAuth, boolean startTls) throws SSLException { super(ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, SSL.SSL_MODE_SERVER, keyCertChain, - clientAuth); + clientAuth, startTls); // Create a new SSL_CTX and configure it. boolean success = false; try { diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java index 7ea0ded1ba..97cd3e3956 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java @@ -57,7 +57,7 @@ public final class ReferenceCountedOpenSslClientContext extends ReferenceCounted long sessionCacheSize, long sessionTimeout) throws SSLException { super(ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, SSL.SSL_MODE_CLIENT, keyCertChain, - ClientAuth.NONE, true); + ClientAuth.NONE, false, true); boolean success = false; try { sessionContext = newSessionContext(this, ctx, engineMap, trustCertCollection, trustManagerFactory, diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java index 7eb56268f8..eaa0ed26fd 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java @@ -198,16 +198,18 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen ReferenceCountedOpenSslContext(Iterable ciphers, CipherSuiteFilter cipherFilter, ApplicationProtocolConfig apnCfg, long sessionCacheSize, long sessionTimeout, - int mode, Certificate[] keyCertChain, ClientAuth clientAuth, boolean leakDetection) - throws SSLException { + int mode, Certificate[] keyCertChain, ClientAuth clientAuth, boolean startTls, + boolean leakDetection) throws SSLException { this(ciphers, cipherFilter, toNegotiator(apnCfg), sessionCacheSize, sessionTimeout, mode, keyCertChain, - clientAuth, leakDetection); + clientAuth, startTls, leakDetection); } ReferenceCountedOpenSslContext(Iterable ciphers, CipherSuiteFilter cipherFilter, OpenSslApplicationProtocolNegotiator apn, long sessionCacheSize, long sessionTimeout, int mode, Certificate[] keyCertChain, - ClientAuth clientAuth, boolean leakDetection) throws SSLException { + ClientAuth clientAuth, boolean startTls, boolean leakDetection) throws SSLException { + super(startTls); + OpenSsl.ensureAvailability(); if (mode != SSL.SSL_MODE_SERVER && mode != SSL.SSL_MODE_CLIENT) { diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java index 49349a61c5..ace2153ecc 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslServerContext.java @@ -49,18 +49,18 @@ public final class ReferenceCountedOpenSslServerContext extends ReferenceCounted X509Certificate[] trustCertCollection, TrustManagerFactory trustManagerFactory, X509Certificate[] keyCertChain, PrivateKey key, String keyPassword, KeyManagerFactory keyManagerFactory, Iterable ciphers, CipherSuiteFilter cipherFilter, ApplicationProtocolConfig apn, - long sessionCacheSize, long sessionTimeout, ClientAuth clientAuth) throws SSLException { + long sessionCacheSize, long sessionTimeout, ClientAuth clientAuth, boolean startTls) throws SSLException { this(trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory, ciphers, - cipherFilter, toNegotiator(apn), sessionCacheSize, sessionTimeout, clientAuth); + cipherFilter, toNegotiator(apn), sessionCacheSize, sessionTimeout, clientAuth, startTls); } private ReferenceCountedOpenSslServerContext( X509Certificate[] trustCertCollection, TrustManagerFactory trustManagerFactory, X509Certificate[] keyCertChain, PrivateKey key, String keyPassword, KeyManagerFactory keyManagerFactory, Iterable ciphers, CipherSuiteFilter cipherFilter, OpenSslApplicationProtocolNegotiator apn, - long sessionCacheSize, long sessionTimeout, ClientAuth clientAuth) throws SSLException { + long sessionCacheSize, long sessionTimeout, ClientAuth clientAuth, boolean startTls) throws SSLException { super(ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, SSL.SSL_MODE_SERVER, keyCertChain, - clientAuth, true); + clientAuth, startTls, true); // Create a new SSL_CTX and configure it. boolean success = false; try { diff --git a/handler/src/main/java/io/netty/handler/ssl/SniHandler.java b/handler/src/main/java/io/netty/handler/ssl/SniHandler.java index 56c98ef183..c53716e6cb 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SniHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SniHandler.java @@ -340,16 +340,18 @@ public class SniHandler extends ByteToMessageDecoder implements ChannelOutboundH * It's also possible for the hostname argument to be {@code null}. */ protected void replaceHandler(ChannelHandlerContext ctx, String hostname, SslContext sslContext) throws Exception { - SSLEngine sslEngine = null; + SslHandler sslHandler = null; try { - sslEngine = sslContext.newEngine(ctx.alloc()); - ctx.pipeline().replace(this, SslHandler.class.getName(), SslContext.newHandler(sslEngine)); - sslEngine = null; + sslHandler = sslContext.newHandler(ctx.alloc()); + ctx.pipeline().replace(this, SslHandler.class.getName(), sslHandler); + sslHandler = null; } finally { // Since the SslHandler was not inserted into the pipeline the ownership of the SSLEngine was not // transferred to the SslHandler. // See https://github.com/netty/netty/issues/5678 - ReferenceCountUtil.safeRelease(sslEngine); + if (sslHandler != null) { + ReferenceCountUtil.safeRelease(sslHandler.engine()); + } } } diff --git a/handler/src/main/java/io/netty/handler/ssl/SslContext.java b/handler/src/main/java/io/netty/handler/ssl/SslContext.java index 7ae00dba62..9372ba87b8 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslContext.java @@ -93,6 +93,8 @@ public abstract class SslContext { } } + private final boolean startTls; + /** * Returns the default server-side implementation provider currently in use. * @@ -382,7 +384,7 @@ public abstract class SslContext { toX509Certificates(keyCertChainFile), toPrivateKey(keyFile, keyPassword), keyPassword, keyManagerFactory, ciphers, cipherFilter, apn, - sessionCacheSize, sessionTimeout, ClientAuth.NONE); + sessionCacheSize, sessionTimeout, ClientAuth.NONE, false); } catch (Exception e) { if (e instanceof SSLException) { throw (SSLException) e; @@ -396,7 +398,7 @@ public abstract class SslContext { X509Certificate[] trustCertCollection, TrustManagerFactory trustManagerFactory, X509Certificate[] keyCertChain, PrivateKey key, String keyPassword, KeyManagerFactory keyManagerFactory, Iterable ciphers, CipherSuiteFilter cipherFilter, ApplicationProtocolConfig apn, - long sessionCacheSize, long sessionTimeout, ClientAuth clientAuth) throws SSLException { + long sessionCacheSize, long sessionTimeout, ClientAuth clientAuth, boolean startTls) throws SSLException { if (provider == null) { provider = defaultServerProvider(); @@ -407,17 +409,17 @@ public abstract class SslContext { return new JdkSslServerContext( trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory, ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, - clientAuth); + clientAuth, startTls); case OPENSSL: return new OpenSslServerContext( trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory, ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, - clientAuth); + clientAuth, startTls); case OPENSSL_REFCNT: return new ReferenceCountedOpenSslServerContext( trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory, ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, - clientAuth); + clientAuth, startTls); default: throw new Error(provider.toString()); } @@ -774,10 +776,19 @@ public abstract class SslContext { return apn; } + /** + * Creates a new instance (startTls set to false). + */ + protected SslContext() { + this(false); + } + /** * Creates a new instance. */ - protected SslContext() { } + protected SslContext(boolean startTls) { + this.startTls = startTls; + } /** * Returns {@code true} if and only if this context is for server-side. @@ -852,7 +863,7 @@ public abstract class SslContext { * @return a new {@link SslHandler} */ public final SslHandler newHandler(ByteBufAllocator alloc) { - return newHandler(newEngine(alloc)); + return new SslHandler(newEngine(alloc), startTls); } /** @@ -866,11 +877,7 @@ public abstract class SslContext { * @return a new {@link SslHandler} */ public final SslHandler newHandler(ByteBufAllocator alloc, String peerHost, int peerPort) { - return newHandler(newEngine(alloc, peerHost, peerPort)); - } - - static SslHandler newHandler(SSLEngine engine) { - return new SslHandler(engine); + return new SslHandler(newEngine(alloc, peerHost, peerPort), startTls); } /** diff --git a/handler/src/main/java/io/netty/handler/ssl/SslContextBuilder.java b/handler/src/main/java/io/netty/handler/ssl/SslContextBuilder.java index 9f70b07682..40d89adc27 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslContextBuilder.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslContextBuilder.java @@ -137,6 +137,7 @@ public final class SslContextBuilder { private long sessionCacheSize; private long sessionTimeout; private ClientAuth clientAuth = ClientAuth.NONE; + private boolean startTls; private SslContextBuilder(boolean forServer) { this.forServer = forServer; @@ -383,6 +384,14 @@ public final class SslContextBuilder { return this; } + /** + * {@code true} if the first write request shouldn't be encrypted. + */ + public SslContextBuilder startTls(boolean startTls) { + this.startTls = startTls; + return this; + } + /** * Create new {@code SslContext} instance with configured settings. *

If {@link #sslProvider(SslProvider)} is set to {@link SslProvider#OPENSSL_REFCNT} then the caller is @@ -392,7 +401,7 @@ public final class SslContextBuilder { if (forServer) { return SslContext.newServerContextInternal(provider, trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory, - ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, clientAuth); + ciphers, cipherFilter, apn, sessionCacheSize, sessionTimeout, clientAuth, startTls); } else { return SslContext.newClientContextInternal(provider, trustCertCollection, trustManagerFactory, keyCertChain, key, keyPassword, keyManagerFactory,