diff --git a/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java b/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java index 1fac36c1bc..deeeb0cfeb 100644 --- a/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java +++ b/handler/src/main/java/io/netty/handler/ssl/CipherSuiteConverter.java @@ -122,33 +122,6 @@ final class CipherSuiteConverter { } } - /** - * Converts the specified Java cipher suites to the colon-separated OpenSSL cipher suite specification. - */ - static String toOpenSsl(Iterable javaCipherSuites) { - final StringBuilder buf = new StringBuilder(); - for (String c: javaCipherSuites) { - if (c == null) { - break; - } - - String converted = toOpenSsl(c); - if (converted != null) { - c = converted; - } - - buf.append(c); - buf.append(':'); - } - - if (buf.length() > 0) { - buf.setLength(buf.length() - 1); - return buf.toString(); - } else { - return ""; - } - } - /** * Converts the specified Java cipher suite to its corresponding OpenSSL cipher suite name. * @@ -423,5 +396,47 @@ final class CipherSuiteConverter { return hmacAlgo; } + /** + * Convert the given ciphers if needed to OpenSSL format and append them to the correct {@link StringBuilder} + * depending on if its a TLSv1.3 cipher or not. If this methods returns without throwing an exception its + * guaranteed that at least one of the {@link StringBuilder}s contain some ciphers that can be used to configure + * OpenSSL. + */ + static void convertToCipherStrings( + Iterable cipherSuites, StringBuilder cipherBuilder, StringBuilder cipherTLSv13Builder) { + for (String c: cipherSuites) { + if (c == null) { + break; + } + + String converted = toOpenSsl(c); + if (converted == null) { + converted = c; + } + + if (!OpenSsl.isCipherSuiteAvailable(converted)) { + throw new IllegalArgumentException("unsupported cipher suite: " + c + '(' + converted + ')'); + } + + if (SslUtils.isTLSv13Cipher(converted)) { + cipherTLSv13Builder.append(converted); + cipherTLSv13Builder.append(':'); + } else { + cipherBuilder.append(converted); + cipherBuilder.append(':'); + } + } + + if (cipherBuilder.length() == 0 && cipherTLSv13Builder.length() == 0) { + throw new IllegalArgumentException("empty cipher suites"); + } + if (cipherBuilder.length() > 0) { + cipherBuilder.setLength(cipherBuilder.length() - 1); + } + if (cipherTLSv13Builder.length() > 0) { + cipherTLSv13Builder.setLength(cipherTLSv13Builder.length() - 1); + } + } + private CipherSuiteConverter() { } } 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 6aef52a246..d74bbdec98 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java @@ -17,6 +17,7 @@ package io.netty.handler.ssl; import io.netty.buffer.ByteBufAllocator; +import io.netty.util.ReferenceCountUtil; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -26,6 +27,7 @@ import java.security.InvalidAlgorithmParameterException; import java.security.KeyException; import java.security.KeyStoreException; import java.security.NoSuchAlgorithmException; +import java.security.Provider; import java.security.Security; import java.security.UnrecoverableKeyException; import java.security.cert.CertificateException; @@ -34,6 +36,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; @@ -58,11 +61,13 @@ public class JdkSslContext extends SslContext { static final String PROTOCOL = "TLS"; private static final String[] DEFAULT_PROTOCOLS; private static final List DEFAULT_CIPHERS; + private static final List DEFAULT_CIPHERS_NON_TLSV13; private static final Set SUPPORTED_CIPHERS; + private static final Set SUPPORTED_CIPHERS_NON_TLSV13; + private static final Provider DEFAULT_PROVIDER; static { SSLContext context; - int i; try { context = SSLContext.getInstance(PROTOCOL); context.init(null, null, null); @@ -70,31 +75,54 @@ public class JdkSslContext extends SslContext { throw new Error("failed to initialize the default SSL context", e); } - SSLEngine engine = context.createSSLEngine(); + DEFAULT_PROVIDER = context.getProvider(); + SSLEngine engine = context.createSSLEngine(); + DEFAULT_PROTOCOLS = defaultProtocols(engine); + + SUPPORTED_CIPHERS = Collections.unmodifiableSet(supportedCiphers(engine)); + DEFAULT_CIPHERS = Collections.unmodifiableList(defaultCiphers(engine, SUPPORTED_CIPHERS)); + + List ciphersNonTLSv13 = new ArrayList(DEFAULT_CIPHERS); + ciphersNonTLSv13.removeAll(Arrays.asList(SslUtils.DEFAULT_TLSV13_CIPHER_SUITES)); + DEFAULT_CIPHERS_NON_TLSV13 = Collections.unmodifiableList(ciphersNonTLSv13); + + Set suppertedCiphersNonTLSv13 = new LinkedHashSet(SUPPORTED_CIPHERS); + suppertedCiphersNonTLSv13.removeAll(Arrays.asList(SslUtils.DEFAULT_TLSV13_CIPHER_SUITES)); + SUPPORTED_CIPHERS_NON_TLSV13 = Collections.unmodifiableSet(suppertedCiphersNonTLSv13); + + if (logger.isDebugEnabled()) { + logger.debug("Default protocols (JDK): {} ", Arrays.asList(DEFAULT_PROTOCOLS)); + logger.debug("Default cipher suites (JDK): {}", DEFAULT_CIPHERS); + } + } + + private static String[] defaultProtocols(SSLEngine engine) { // Choose the sensible default list of protocols. final String[] supportedProtocols = engine.getSupportedProtocols(); Set supportedProtocolsSet = new HashSet(supportedProtocols.length); - for (i = 0; i < supportedProtocols.length; ++i) { + for (int i = 0; i < supportedProtocols.length; ++i) { supportedProtocolsSet.add(supportedProtocols[i]); } List protocols = new ArrayList(); addIfSupported( supportedProtocolsSet, protocols, - "TLSv1.2", "TLSv1.1", "TLSv1"); + // Do not include TLSv1.3 for now by default. + SslUtils.PROTOCOL_TLS_V1_2, SslUtils.PROTOCOL_TLS_V1_1, SslUtils.PROTOCOL_TLS_V1); if (!protocols.isEmpty()) { - DEFAULT_PROTOCOLS = protocols.toArray(new String[0]); - } else { - DEFAULT_PROTOCOLS = engine.getEnabledProtocols(); + return protocols.toArray(new String[0]); } + return engine.getEnabledProtocols(); + } + private static Set supportedCiphers(SSLEngine engine) { // Choose the sensible default list of cipher suites. final String[] supportedCiphers = engine.getSupportedCipherSuites(); - SUPPORTED_CIPHERS = new HashSet(supportedCiphers.length); - for (i = 0; i < supportedCiphers.length; ++i) { + Set supportedCiphersSet = new LinkedHashSet(supportedCiphers.length); + for (int i = 0; i < supportedCiphers.length; ++i) { String supportedCipher = supportedCiphers[i]; - SUPPORTED_CIPHERS.add(supportedCipher); + supportedCiphersSet.add(supportedCipher); // IBM's J9 JVM utilizes a custom naming scheme for ciphers and only returns ciphers with the "SSL_" // prefix instead of the "TLS_" prefix (as defined in the JSSE cipher suite names [1]). According to IBM's // documentation [2] the "SSL_" prefix is "interchangeable" with the "TLS_" prefix. @@ -108,21 +136,29 @@ public class JdkSslContext extends SslContext { final String tlsPrefixedCipherName = "TLS_" + supportedCipher.substring("SSL_".length()); try { engine.setEnabledCipherSuites(new String[]{tlsPrefixedCipherName}); - SUPPORTED_CIPHERS.add(tlsPrefixedCipherName); + supportedCiphersSet.add(tlsPrefixedCipherName); } catch (IllegalArgumentException ignored) { // The cipher is not supported ... move on to the next cipher. } } } - List ciphers = new ArrayList(); - addIfSupported(SUPPORTED_CIPHERS, ciphers, DEFAULT_CIPHER_SUITES); - useFallbackCiphersIfDefaultIsEmpty(ciphers, engine.getEnabledCipherSuites()); - DEFAULT_CIPHERS = Collections.unmodifiableList(ciphers); + return supportedCiphersSet; + } - if (logger.isDebugEnabled()) { - logger.debug("Default protocols (JDK): {} ", Arrays.asList(DEFAULT_PROTOCOLS)); - logger.debug("Default cipher suites (JDK): {}", DEFAULT_CIPHERS); + private static List defaultCiphers(SSLEngine engine, Set supportedCiphers) { + List ciphers = new ArrayList(); + addIfSupported(supportedCiphers, ciphers, DEFAULT_CIPHER_SUITES); + useFallbackCiphersIfDefaultIsEmpty(ciphers, engine.getEnabledCipherSuites()); + return ciphers; + } + + private static boolean isTlsV13Supported(String[] protocols) { + for (String protocol: protocols) { + if (SslUtils.PROTOCOL_TLS_V1_3.equals(protocol)) { + return true; + } } + return false; } private final String[] protocols; @@ -205,11 +241,49 @@ public class JdkSslContext extends SslContext { super(startTls); this.apn = checkNotNull(apn, "apn"); this.clientAuth = checkNotNull(clientAuth, "clientAuth"); - cipherSuites = checkNotNull(cipherFilter, "cipherFilter").filterCipherSuites( - ciphers, DEFAULT_CIPHERS, SUPPORTED_CIPHERS); - this.protocols = protocols == null ? DEFAULT_PROTOCOLS : protocols; - unmodifiableCipherSuites = Collections.unmodifiableList(Arrays.asList(cipherSuites)); this.sslContext = checkNotNull(sslContext, "sslContext"); + + final List defaultCiphers; + final Set supportedCiphers; + if (DEFAULT_PROVIDER.equals(sslContext.getProvider())) { + this.protocols = protocols == null? DEFAULT_PROTOCOLS : protocols; + if (isTlsV13Supported(this.protocols)) { + supportedCiphers = SUPPORTED_CIPHERS; + defaultCiphers = DEFAULT_CIPHERS; + } else { + // TLSv1.3 is not supported, ensure we do not include any TLSv1.3 ciphersuite. + supportedCiphers = SUPPORTED_CIPHERS_NON_TLSV13; + defaultCiphers = DEFAULT_CIPHERS_NON_TLSV13; + } + } else { + // This is a different Provider then the one used by the JDK by default so we can not just assume + // the same protocols and ciphers are supported. For example even if Java11+ is used Conscrypt will + // not support TLSv1.3 and the TLSv1.3 ciphersuites. + SSLEngine engine = sslContext.createSSLEngine(); + try { + if (protocols == null) { + this.protocols = defaultProtocols(engine); + } else { + this.protocols = protocols; + } + supportedCiphers = supportedCiphers(engine); + defaultCiphers = defaultCiphers(engine, supportedCiphers); + if (!isTlsV13Supported(this.protocols)) { + // TLSv1.3 is not supported, ensure we do not include any TLSv1.3 ciphersuite. + for (String cipher: SslUtils.DEFAULT_TLSV13_CIPHER_SUITES) { + supportedCiphers.remove(cipher); + defaultCiphers.remove(cipher); + } + } + } finally { + ReferenceCountUtil.release(engine); + } + } + + cipherSuites = checkNotNull(cipherFilter, "cipherFilter").filterCipherSuites( + ciphers, defaultCiphers, supportedCiphers); + + unmodifiableCipherSuites = Collections.unmodifiableList(Arrays.asList(cipherSuites)); this.isClient = isClient; } diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java b/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java index e614fccdcd..955404b25e 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java @@ -31,6 +31,7 @@ import io.netty.util.internal.SystemPropertyUtil; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; +import javax.net.ssl.SSLException; import java.security.AccessController; import java.security.PrivilegedAction; import java.util.ArrayList; @@ -48,6 +49,7 @@ import static io.netty.handler.ssl.SslUtils.PROTOCOL_SSL_V3; import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1; import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_1; import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_2; +import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_3; /** * Tells if {@code netty-tcnative} and its OpenSSL support @@ -66,6 +68,12 @@ public final class OpenSsl { private static final boolean SUPPORTS_HOSTNAME_VALIDATION; private static final boolean USE_KEYMANAGER_FACTORY; private static final boolean SUPPORTS_OCSP; + private static final String TLSV13_CIPHERS = "TLS_AES_256_GCM_SHA384" + ':' + + "TLS_CHACHA20_POLY1305_SHA256" + ':' + + "TLS_AES_128_GCM_SHA256" + ':' + + "TLS_AES_128_CCM_8_SHA256" + ':' + + "TLS_AES_128_CCM_SHA256"; + private static final boolean TLSV13_SUPPORTED; static final Set SUPPORTED_PROTOCOLS_SET; @@ -139,17 +147,30 @@ public final class OpenSsl { boolean supportsKeyManagerFactory = false; boolean useKeyManagerFactory = false; boolean supportsHostNameValidation = false; + boolean tlsv13Supported = false; + try { final long sslCtx = SSLContext.make(SSL.SSL_PROTOCOL_ALL, SSL.SSL_MODE_SERVER); long certBio = 0; SelfSignedCertificate cert = null; try { - SSLContext.setCipherSuite(sslCtx, "ALL"); + if (PlatformDependent.javaVersion() >= 11) { + try { + SSLContext.setCipherSuite(sslCtx, TLSV13_CIPHERS, true); + tlsv13Supported = true; + } catch (Exception ignore) { + tlsv13Supported = false; + } + } + SSLContext.setCipherSuite(sslCtx, "ALL", false); + final long ssl = SSL.newSSL(sslCtx, true); try { for (String c: SSL.getCiphers(ssl)) { // Filter out bad input. - if (c == null || c.isEmpty() || availableOpenSslCipherSuites.contains(c)) { + if (c == null || c.isEmpty() || availableOpenSslCipherSuites.contains(c) || + // Filter out TLSv1.3 ciphers if not supported. + !tlsv13Supported && SslUtils.isTLSv13Cipher(c)) { continue; } availableOpenSslCipherSuites.add(c); @@ -200,8 +221,13 @@ public final class OpenSsl { AVAILABLE_OPENSSL_CIPHER_SUITES.size() * 2); for (String cipher: AVAILABLE_OPENSSL_CIPHER_SUITES) { // Included converted but also openssl cipher name - availableJavaCipherSuites.add(CipherSuiteConverter.toJava(cipher, "TLS")); - availableJavaCipherSuites.add(CipherSuiteConverter.toJava(cipher, "SSL")); + if (!SslUtils.isTLSv13Cipher(cipher)) { + availableJavaCipherSuites.add(CipherSuiteConverter.toJava(cipher, "TLS")); + availableJavaCipherSuites.add(CipherSuiteConverter.toJava(cipher, "SSL")); + } else { + // TLSv1.3 ciphers have the correct format. + availableJavaCipherSuites.add(cipher); + } } addIfSupported(availableJavaCipherSuites, defaultCiphers, DEFAULT_CIPHER_SUITES); @@ -239,6 +265,18 @@ public final class OpenSsl { protocols.add(PROTOCOL_TLS_V1_2); } + // This is only supported by java11 and later. + if (tlsv13Supported && doesSupportProtocol(SSL.SSL_PROTOCOL_TLSV1_3, SSL.SSL_OP_NO_TLSv1_3) + && PlatformDependent.javaVersion() >= 11) { + // We can only support TLS1.3 when using Java 11 or higher as otherwise it will fail to create the + // internal instance of an sun.security.ssl.ProtocolVersion as can not parse the version string :/ + // See http://mail.openjdk.java.net/pipermail/security-dev/2018-September/018242.html + protocols.add(PROTOCOL_TLS_V1_3); + TLSV13_SUPPORTED = true; + } else { + TLSV13_SUPPORTED = false; + } + SUPPORTED_PROTOCOLS_SET = Collections.unmodifiableSet(protocols); SUPPORTS_OCSP = doesSupportOcsp(); @@ -256,6 +294,7 @@ public final class OpenSsl { USE_KEYMANAGER_FACTORY = false; SUPPORTED_PROTOCOLS_SET = Collections.emptySet(); SUPPORTS_OCSP = false; + TLSV13_SUPPORTED = false; } } @@ -450,4 +489,8 @@ public final class OpenSsl { ReferenceCountUtil.safeRelease(counted); } } + + static boolean isTlsv13Supported() { + return TLSV13_SUPPORTED; + } } 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 9e524a7a00..4972905baa 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslClientContext.java @@ -24,8 +24,11 @@ import io.netty.internal.tcnative.SSLContext; import java.security.KeyStore; import java.security.PrivateKey; import java.security.cert.X509Certificate; + +import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Set; import javax.net.ssl.KeyManagerFactory; @@ -47,6 +50,12 @@ import javax.security.auth.x500.X500Principal; public final class ReferenceCountedOpenSslClientContext extends ReferenceCountedOpenSslContext { private static final InternalLogger logger = InternalLoggerFactory.getInstance(ReferenceCountedOpenSslClientContext.class); + private static final Set SUPPORTED_KEY_TYPES = Collections.unmodifiableSet(new LinkedHashSet( + Arrays.asList(OpenSslKeyMaterialManager.KEY_TYPE_RSA, + OpenSslKeyMaterialManager.KEY_TYPE_DH_RSA, + OpenSslKeyMaterialManager.KEY_TYPE_EC, + OpenSslKeyMaterialManager.KEY_TYPE_EC_RSA, + OpenSslKeyMaterialManager.KEY_TYPE_EC_EC))); private final OpenSslSessionContext sessionContext; ReferenceCountedOpenSslClientContext(X509Certificate[] trustCertCollection, TrustManagerFactory trustManagerFactory, @@ -277,7 +286,8 @@ public final class ReferenceCountedOpenSslClientContext extends ReferenceCounted */ private static Set supportedClientKeyTypes(byte[] clientCertificateTypes) { if (clientCertificateTypes == null) { - return Collections.emptySet(); + // Try all of the supported key types. + return SUPPORTED_KEY_TYPES; } Set result = new HashSet(clientCertificateTypes.length); for (byte keyTypeCode : clientCertificateTypes) { 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 714f473f7a..6f471b361a 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java @@ -225,26 +225,71 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen boolean success = false; try { try { - int opts = SSL.SSL_PROTOCOL_SSLV3 | SSL.SSL_PROTOCOL_TLSV1 | - SSL.SSL_PROTOCOL_TLSV1_1 | SSL.SSL_PROTOCOL_TLSV1_2; - ctx = SSLContext.make(opts, mode); + int protocolOpts = SSL.SSL_PROTOCOL_SSLV3 | SSL.SSL_PROTOCOL_TLSV1 | + SSL.SSL_PROTOCOL_TLSV1_1 | SSL.SSL_PROTOCOL_TLSV1_2; + if (OpenSsl.isTlsv13Supported()) { + protocolOpts |= SSL.SSL_PROTOCOL_TLSV1_3; + } + ctx = SSLContext.make(protocolOpts, mode); } catch (Exception e) { throw new SSLException("failed to create an SSL_CTX", e); } - SSLContext.setOptions(ctx, SSLContext.getOptions(ctx) | - SSL.SSL_OP_NO_SSLv2 | - SSL.SSL_OP_NO_SSLv3 | - SSL.SSL_OP_CIPHER_SERVER_PREFERENCE | + boolean tlsv13Supported = OpenSsl.isTlsv13Supported(); + StringBuilder cipherBuilder = new StringBuilder(); + StringBuilder cipherTLSv13Builder = new StringBuilder(); - // We do not support compression at the moment so we should explicitly disable it. - SSL.SSL_OP_NO_COMPRESSION | + /* List the ciphers that are permitted to negotiate. */ + try { + if (unmodifiableCiphers.isEmpty()) { + // Set non TLSv1.3 ciphers. + SSLContext.setCipherSuite(ctx, StringUtil.EMPTY_STRING, false); + if (tlsv13Supported) { + // Set TLSv1.3 ciphers. + SSLContext.setCipherSuite(ctx, StringUtil.EMPTY_STRING, true); + } + } else { + CipherSuiteConverter.convertToCipherStrings( + unmodifiableCiphers, cipherBuilder, cipherTLSv13Builder); - // Disable ticket support by default to be more inline with SSLEngineImpl of the JDK. - // This also let SSLSession.getId() work the same way for the JDK implementation and the - // OpenSSLEngine. If tickets are supported SSLSession.getId() will only return an ID on the - // server-side if it could make use of tickets. - SSL.SSL_OP_NO_TICKET); + // Set non TLSv1.3 ciphers. + SSLContext.setCipherSuite(ctx, cipherBuilder.toString(), false); + if (tlsv13Supported) { + // Set TLSv1.3 ciphers. + SSLContext.setCipherSuite(ctx, cipherTLSv13Builder.toString(), true); + } + } + } catch (SSLException e) { + throw e; + } catch (Exception e) { + throw new SSLException("failed to set cipher suite: " + unmodifiableCiphers, e); + } + + int options = SSLContext.getOptions(ctx) | + SSL.SSL_OP_NO_SSLv2 | + SSL.SSL_OP_NO_SSLv3 | + // Disable TLSv1.3 by default for now. Even if TLSv1.3 is not supported this will + // work fine as in this case SSL_OP_NO_TLSv1_3 will be 0. + SSL.SSL_OP_NO_TLSv1_3 | + + SSL.SSL_OP_CIPHER_SERVER_PREFERENCE | + + // We do not support compression at the moment so we should explicitly disable it. + SSL.SSL_OP_NO_COMPRESSION | + + // Disable ticket support by default to be more inline with SSLEngineImpl of the JDK. + // This also let SSLSession.getId() work the same way for the JDK implementation and the + // OpenSSLEngine. If tickets are supported SSLSession.getId() will only return an ID on the + // server-side if it could make use of tickets. + SSL.SSL_OP_NO_TICKET; + + if (cipherBuilder.length() == 0) { + // No ciphers that are compatible with SSLv2 / SSLv3 / TLSv1 / TLSv1.1 / TLSv1.2 + options |= SSL.SSL_OP_NO_SSLv2 | SSL.SSL_OP_NO_SSLv3 | SSL.SSL_OP_NO_TLSv1 + | SSL.SSL_OP_NO_TLSv1_1 | SSL.SSL_OP_NO_TLSv1_2; + } + + SSLContext.setOptions(ctx, options); // We need to enable SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER as the memory address may change between // calling OpenSSLEngine.wrap(...). @@ -255,15 +300,6 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen SSLContext.setTmpDHLength(ctx, DH_KEY_LENGTH); } - /* List the ciphers that are permitted to negotiate. */ - try { - SSLContext.setCipherSuite(ctx, CipherSuiteConverter.toOpenSsl(unmodifiableCiphers)); - } catch (SSLException e) { - throw e; - } catch (Exception e) { - throw new SSLException("failed to set cipher suite: " + unmodifiableCiphers, e); - } - List nextProtoList = apn.protocols(); /* Set next protocols for next protocol negotiation extension, if specified */ if (!nextProtoList.isEmpty()) { diff --git a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java index 49851c93b5..4ddcc2aefa 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngine.java @@ -66,9 +66,8 @@ import static io.netty.handler.ssl.SslUtils.PROTOCOL_SSL_V3; import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1; import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_1; import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_2; +import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_3; import static io.netty.handler.ssl.SslUtils.SSL_RECORD_HEADER_LENGTH; -import static io.netty.internal.tcnative.SSL.SSL_MAX_PLAINTEXT_LENGTH; -import static io.netty.internal.tcnative.SSL.SSL_MAX_RECORD_LENGTH; import static io.netty.util.internal.EmptyArrays.EMPTY_CERTIFICATES; import static io.netty.util.internal.EmptyArrays.EMPTY_JAVAX_X509_CERTIFICATES; import static io.netty.util.internal.ObjectUtil.checkNotNull; @@ -109,12 +108,14 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc private static final int OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1 = 2; private static final int OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_1 = 3; private static final int OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_2 = 4; + private static final int OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_3 = 5; private static final int[] OPENSSL_OP_NO_PROTOCOLS = { SSL.SSL_OP_NO_SSLv2, SSL.SSL_OP_NO_SSLv3, SSL.SSL_OP_NO_TLSv1, SSL.SSL_OP_NO_TLSv1_1, - SSL.SSL_OP_NO_TLSv1_2 + SSL.SSL_OP_NO_TLSv1_2, + SSL.SSL_OP_NO_TLSv1_3 }; /** * The flags argument is usually 0. @@ -124,11 +125,11 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc /** * Depends upon tcnative ... only use if tcnative is available! */ - static final int MAX_PLAINTEXT_LENGTH = SSL_MAX_PLAINTEXT_LENGTH; + static final int MAX_PLAINTEXT_LENGTH = SSL.SSL_MAX_PLAINTEXT_LENGTH; /** * Depends upon tcnative ... only use if tcnative is available! */ - private static final int MAX_RECORD_SIZE = SSL_MAX_RECORD_LENGTH; + private static final int MAX_RECORD_SIZE = SSL.SSL_MAX_RECORD_LENGTH; private static final AtomicIntegerFieldUpdater DESTROYED_UPDATER = AtomicIntegerFieldUpdater.newUpdater(ReferenceCountedOpenSslEngine.class, "destroyed"); @@ -1206,7 +1207,10 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc // As rejectRemoteInitiatedRenegotiation() is called in a finally block we also need to check if we shutdown // the engine before as otherwise SSL.getHandshakeCount(ssl) will throw an NPE if the passed in ssl is 0. // See https://github.com/netty/netty/issues/7353 - if (!isDestroyed() && SSL.getHandshakeCount(ssl) > 1) { + if (!isDestroyed() && SSL.getHandshakeCount(ssl) > 1 && + // As we may count multiple handshakes when TLSv1.3 is used we should just ignore this here as + // renegotiation is not supported in TLSv1.3 as per spec. + !SslUtils.PROTOCOL_TLS_V1_3.equals(session.getProtocol()) && handshakeState == HandshakeState.FINISHED) { // TODO: In future versions me may also want to send a fatal_alert to the client and so notify it // that the renegotiation failed. shutdown(); @@ -1379,15 +1383,18 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc if (enabled == null) { return EmptyArrays.EMPTY_STRINGS; } else { + List enabledList = new ArrayList(); synchronized (this) { for (int i = 0; i < enabled.length; i++) { String mapped = toJavaCipherSuite(enabled[i]); - if (mapped != null) { - enabled[i] = mapped; + final String cipher = mapped == null ? enabled[i] : mapped; + if (!OpenSsl.isTlsv13Supported() && SslUtils.isTLSv13Cipher(cipher)) { + continue; } + enabledList.add(cipher); } } - return enabled; + return enabledList.toArray(new String[0]); } } @@ -1396,35 +1403,28 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc checkNotNull(cipherSuites, "cipherSuites"); final StringBuilder buf = new StringBuilder(); - for (String c: cipherSuites) { - if (c == null) { - break; - } - - String converted = CipherSuiteConverter.toOpenSsl(c); - if (converted == null) { - converted = c; - } - - if (!OpenSsl.isCipherSuiteAvailable(converted)) { - throw new IllegalArgumentException("unsupported cipher suite: " + c + '(' + converted + ')'); - } - - buf.append(converted); - buf.append(':'); - } - - if (buf.length() == 0) { - throw new IllegalArgumentException("empty cipher suites"); - } - buf.setLength(buf.length() - 1); + final StringBuilder bufTLSv13 = new StringBuilder(); + CipherSuiteConverter.convertToCipherStrings(Arrays.asList(cipherSuites), buf, bufTLSv13); final String cipherSuiteSpec = buf.toString(); + final String cipherSuiteSpecTLSv13 = bufTLSv13.toString(); + if (!OpenSsl.isTlsv13Supported() && !cipherSuiteSpecTLSv13.isEmpty()) { + throw new IllegalArgumentException("TLSv1.3 is not supported by this java version."); + } synchronized (this) { if (!isDestroyed()) { + // TODO: Should we also adjust the protocols based on if there are any ciphers left that can be used + // for TLSv1.3 or for previor SSL/TLS versions ? try { - SSL.setCipherSuites(ssl, cipherSuiteSpec); + // Set non TLSv1.3 ciphers. + SSL.setCipherSuites(ssl, cipherSuiteSpec, false); + + if (OpenSsl.isTlsv13Supported()) { + // Set TLSv1.3 ciphers. + SSL.setCipherSuites(ssl, cipherSuiteSpecTLSv13, true); + } + } catch (Exception e) { throw new IllegalStateException("failed to enable cipher suites: " + cipherSuiteSpec, e); } @@ -1462,6 +1462,9 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc if (isProtocolEnabled(opts, SSL.SSL_OP_NO_TLSv1_2, PROTOCOL_TLS_V1_2)) { enabled.add(PROTOCOL_TLS_V1_2); } + if (isProtocolEnabled(opts, SSL.SSL_OP_NO_TLSv1_3, PROTOCOL_TLS_V1_3)) { + enabled.add(PROTOCOL_TLS_V1_3); + } if (isProtocolEnabled(opts, SSL.SSL_OP_NO_SSLv2, PROTOCOL_SSL_V2)) { enabled.add(PROTOCOL_SSL_V2); } @@ -1533,13 +1536,20 @@ public class ReferenceCountedOpenSslEngine extends SSLEngine implements Referenc if (maxProtocolIndex < OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_2) { maxProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_2; } + } else if (p.equals(PROTOCOL_TLS_V1_3)) { + if (minProtocolIndex > OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_3) { + minProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_3; + } + if (maxProtocolIndex < OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_3) { + maxProtocolIndex = OPENSSL_OP_NO_PROTOCOL_INDEX_TLSv1_3; + } } } synchronized (this) { if (!isDestroyed()) { // Clear out options which disable protocols SSL.clearOptions(ssl, SSL.SSL_OP_NO_SSLv2 | SSL.SSL_OP_NO_SSLv3 | SSL.SSL_OP_NO_TLSv1 | - SSL.SSL_OP_NO_TLSv1_1 | SSL.SSL_OP_NO_TLSv1_2); + SSL.SSL_OP_NO_TLSv1_1 | SSL.SSL_OP_NO_TLSv1_2 | SSL.SSL_OP_NO_TLSv1_3); int opts = 0; for (int i = 0; i < minProtocolIndex; ++i) { diff --git a/handler/src/main/java/io/netty/handler/ssl/SslUtils.java b/handler/src/main/java/io/netty/handler/ssl/SslUtils.java index 414c9d1d18..f1f000fa8f 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslUtils.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslUtils.java @@ -22,9 +22,14 @@ import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.base64.Base64; import io.netty.handler.codec.base64.Base64Dialect; import io.netty.util.NetUtil; +import io.netty.util.internal.EmptyArrays; +import io.netty.util.internal.PlatformDependent; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Set; @@ -36,7 +41,11 @@ import static java.util.Arrays.asList; * Constants for SSL packets. */ final class SslUtils { - + // See https://tools.ietf.org/html/rfc8446#appendix-B.4 + private static final Set TLSV13_CIPHERS = Collections.unmodifiableSet(new HashSet( + asList("TLS_AES_256_GCM_SHA384", "TLS_CHACHA20_POLY1305_SHA256", + "TLS_AES_128_GCM_SHA256", "TLS_AES_128_CCM_8_SHA256", + "TLS_AES_128_CCM_SHA256"))); // Protocols static final String PROTOCOL_SSL_V2_HELLO = "SSLv2Hello"; static final String PROTOCOL_SSL_V2 = "SSLv2"; @@ -44,6 +53,7 @@ final class SslUtils { static final String PROTOCOL_TLS_V1 = "TLSv1"; static final String PROTOCOL_TLS_V1_1 = "TLSv1.1"; static final String PROTOCOL_TLS_V1_2 = "TLSv1.2"; + static final String PROTOCOL_TLS_V1_3 = "TLSv1.3"; /** * change cipher spec @@ -85,20 +95,36 @@ final class SslUtils { */ static final int NOT_ENCRYPTED = -2; - static final String[] DEFAULT_CIPHER_SUITES = { + static final String[] DEFAULT_CIPHER_SUITES; + static final String[] DEFAULT_TLSV13_CIPHER_SUITES; + + static { + if (PlatformDependent.javaVersion() >= 11) { + DEFAULT_TLSV13_CIPHER_SUITES = new String[] { "TLS_AES_128_GCM_SHA256", "TLS_AES_256_GCM_SHA384" }; + } else { + DEFAULT_TLSV13_CIPHER_SUITES = EmptyArrays.EMPTY_STRINGS; + } + + List defaultCiphers = new ArrayList(); // GCM (Galois/Counter Mode) requires JDK 8. - "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", - "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", - "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", + defaultCiphers.add("TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384"); + defaultCiphers.add("TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"); + defaultCiphers.add("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"); + defaultCiphers.add("TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA"); // AES256 requires JCE unlimited strength jurisdiction policy files. - "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", + defaultCiphers.add("TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA"); // GCM (Galois/Counter Mode) requires JDK 8. - "TLS_RSA_WITH_AES_128_GCM_SHA256", - "TLS_RSA_WITH_AES_128_CBC_SHA", + defaultCiphers.add("TLS_RSA_WITH_AES_128_GCM_SHA256"); + defaultCiphers.add("TLS_RSA_WITH_AES_128_CBC_SHA"); // AES256 requires JCE unlimited strength jurisdiction policy files. - "TLS_RSA_WITH_AES_256_CBC_SHA" - }; + defaultCiphers.add("TLS_RSA_WITH_AES_256_CBC_SHA"); + + for (String tlsv13Cipher: DEFAULT_TLSV13_CIPHER_SUITES) { + defaultCiphers.add(tlsv13Cipher); + } + + DEFAULT_CIPHER_SUITES = defaultCiphers.toArray(new String[0]); + } /** * Add elements from {@code names} into {@code enabled} if they are in {@code supported}. @@ -361,6 +387,14 @@ final class SslUtils { !NetUtil.isValidIpV6Address(hostname); } + /** + * Returns {@code true} if the the given cipher (in openssl format) is for TLSv1.3, {@code false} otherwise. + */ + static boolean isTLSv13Cipher(String cipher) { + // See https://tools.ietf.org/html/rfc8446#appendix-B.4 + return TLSV13_CIPHERS.contains(cipher); + } + private SslUtils() { } } diff --git a/handler/src/test/java/io/netty/handler/ssl/CipherSuiteCanaryTest.java b/handler/src/test/java/io/netty/handler/ssl/CipherSuiteCanaryTest.java index 22e5f43bf8..9c394ccf63 100644 --- a/handler/src/test/java/io/netty/handler/ssl/CipherSuiteCanaryTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/CipherSuiteCanaryTest.java @@ -125,12 +125,16 @@ public class CipherSuiteCanaryTest { final SslContext sslServerContext = SslContextBuilder.forServer(CERT.certificate(), CERT.privateKey()) .sslProvider(serverSslProvider) .ciphers(ciphers) + // As this is not a TLSv1.3 cipher we should ensure we talk something else. + .protocols(SslUtils.PROTOCOL_TLS_V1_2) .build(); try { final SslContext sslClientContext = SslContextBuilder.forClient() .sslProvider(clientSslProvider) .ciphers(ciphers) + // As this is not a TLSv1.3 cipher we should ensure we talk something else. + .protocols(SslUtils.PROTOCOL_TLS_V1_2) .trustManager(InsecureTrustManagerFactory.INSTANCE) .build(); diff --git a/handler/src/test/java/io/netty/handler/ssl/CipherSuiteConverterTest.java b/handler/src/test/java/io/netty/handler/ssl/CipherSuiteConverterTest.java index ffe53d2ba8..f70da234c7 100644 --- a/handler/src/test/java/io/netty/handler/ssl/CipherSuiteConverterTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/CipherSuiteConverterTest.java @@ -22,8 +22,7 @@ import org.junit.Test; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.sameInstance; -import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertThat; +import static org.junit.Assert.*; public class CipherSuiteConverterTest { diff --git a/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java b/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java index 870f71ef36..d666535b19 100644 --- a/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java @@ -31,17 +31,17 @@ import static org.junit.Assume.assumeTrue; @RunWith(Parameterized.class) public class ConscryptJdkSslEngineInteropTest extends SSLEngineTest { - @Parameterized.Parameters(name = "{index}: bufferType = {0}") - public static Collection data() { - List params = new ArrayList(); + @Parameterized.Parameters(name = "{index}: bufferType = {0}, combo = {1}") + public static Collection data() { + List params = new ArrayList(); for (BufferType type: BufferType.values()) { - params.add(type); + params.add(new Object[] { type, ProtocolCipherCombo.tlsv12()}); } return params; } - public ConscryptJdkSslEngineInteropTest(BufferType type) { - super(type); + public ConscryptJdkSslEngineInteropTest(BufferType type, ProtocolCipherCombo combo) { + super(type, combo); } @BeforeClass diff --git a/handler/src/test/java/io/netty/handler/ssl/ConscryptSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/ConscryptSslEngineTest.java index e57fd58be0..8c6121b6fb 100644 --- a/handler/src/test/java/io/netty/handler/ssl/ConscryptSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/ConscryptSslEngineTest.java @@ -30,17 +30,17 @@ import static org.junit.Assume.assumeTrue; @RunWith(Parameterized.class) public class ConscryptSslEngineTest extends SSLEngineTest { - @Parameterized.Parameters(name = "{index}: bufferType = {0}") - public static Collection data() { - List params = new ArrayList(); + @Parameterized.Parameters(name = "{index}: bufferType = {0}, combo = {1}") + public static Collection data() { + List params = new ArrayList(); for (BufferType type: BufferType.values()) { - params.add(type); + params.add(new Object[] { type, ProtocolCipherCombo.tlsv12()}); } return params; } - public ConscryptSslEngineTest(BufferType type) { - super(type); + public ConscryptSslEngineTest(BufferType type, ProtocolCipherCombo combo) { + super(type, combo); } @BeforeClass diff --git a/handler/src/test/java/io/netty/handler/ssl/JdkConscryptSslEngineInteropTest.java b/handler/src/test/java/io/netty/handler/ssl/JdkConscryptSslEngineInteropTest.java index 6d8862a0f2..309490af59 100644 --- a/handler/src/test/java/io/netty/handler/ssl/JdkConscryptSslEngineInteropTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/JdkConscryptSslEngineInteropTest.java @@ -16,6 +16,7 @@ package io.netty.handler.ssl; import java.security.Provider; + import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; @@ -31,17 +32,17 @@ import static org.junit.Assume.assumeTrue; @RunWith(Parameterized.class) public class JdkConscryptSslEngineInteropTest extends SSLEngineTest { - @Parameterized.Parameters(name = "{index}: bufferType = {0}") - public static Collection data() { - List params = new ArrayList(); + @Parameterized.Parameters(name = "{index}: bufferType = {0}, combo = {1}") + public static Collection data() { + List params = new ArrayList(); for (BufferType type: BufferType.values()) { - params.add(type); + params.add(new Object[] { type, ProtocolCipherCombo.tlsv12()}); } return params; } - public JdkConscryptSslEngineInteropTest(BufferType type) { - super(type); + public JdkConscryptSslEngineInteropTest(BufferType type, ProtocolCipherCombo combo) { + super(type, combo); } @BeforeClass diff --git a/handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java b/handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java index 0eed0b3087..a85b665ff2 100644 --- a/handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/JdkOpenSslEngineInteroptTest.java @@ -15,6 +15,7 @@ */ package io.netty.handler.ssl; +import io.netty.util.internal.PlatformDependent; import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; @@ -32,17 +33,21 @@ import static org.junit.Assume.assumeTrue; @RunWith(Parameterized.class) public class JdkOpenSslEngineInteroptTest extends SSLEngineTest { - @Parameterized.Parameters(name = "{index}: bufferType = {0}") - public static Collection data() { - List params = new ArrayList(); + @Parameterized.Parameters(name = "{index}: bufferType = {0}, combo = {1}") + public static Collection data() { + List params = new ArrayList(); for (BufferType type: BufferType.values()) { - params.add(type); + params.add(new Object[] { type, ProtocolCipherCombo.tlsv12()}); + + if (PlatformDependent.javaVersion() >= 11 && OpenSsl.isTlsv13Supported()) { + params.add(new Object[] { type, ProtocolCipherCombo.tlsv13() }); + } } return params; } - public JdkOpenSslEngineInteroptTest(BufferType type) { - super(type); + public JdkOpenSslEngineInteroptTest(BufferType type, ProtocolCipherCombo protocolCipherCombo) { + super(type, protocolCipherCombo); } @BeforeClass diff --git a/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java index f37a6aff25..74f000fd01 100644 --- a/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java @@ -26,6 +26,7 @@ import java.security.Provider; import java.util.ArrayList; import java.util.Collection; +import io.netty.util.internal.EmptyArrays; import io.netty.util.internal.PlatformDependent; import org.junit.Ignore; import org.junit.Test; @@ -141,12 +142,15 @@ public class JdkSslEngineTest extends SSLEngineTest { private static final String FALLBACK_APPLICATION_LEVEL_PROTOCOL = "my-protocol-http1_1"; private static final String APPLICATION_LEVEL_PROTOCOL_NOT_COMPATIBLE = "my-protocol-FOO"; - @Parameterized.Parameters(name = "{index}: providerType = {0}, bufferType = {1}") + @Parameterized.Parameters(name = "{index}: providerType = {0}, bufferType = {1}, combo = {2}") public static Collection data() { List params = new ArrayList(); for (ProviderType providerType : ProviderType.values()) { for (BufferType bufferType : BufferType.values()) { - params.add(new Object[]{providerType, bufferType}); + params.add(new Object[]{ providerType, bufferType, ProtocolCipherCombo.tlsv12()}); + if (PlatformDependent.javaVersion() >= 11) { + params.add(new Object[] { providerType, bufferType, ProtocolCipherCombo.tlsv13() }); + } } } return params; @@ -156,8 +160,8 @@ public class JdkSslEngineTest extends SSLEngineTest { private Provider provider; - public JdkSslEngineTest(ProviderType providerType, BufferType bufferType) { - super(bufferType); + public JdkSslEngineTest(ProviderType providerType, BufferType bufferType, ProtocolCipherCombo protocolCipherCombo) { + super(bufferType, protocolCipherCombo); this.providerType = providerType; } @@ -235,9 +239,11 @@ public class JdkSslEngineTest extends SSLEngineTest { InsecureTrustManagerFactory.INSTANCE, null, IdentityCipherSuiteFilter.INSTANCE, clientApn, 0, 0); - setupHandlers(serverSslCtx, clientSslCtx); + setupHandlers(new TestDelegatingSslContext(serverSslCtx), new TestDelegatingSslContext(clientSslCtx)); assertTrue(clientLatch.await(2, TimeUnit.SECONDS)); - assertTrue(clientException instanceof SSLHandshakeException); + // When using TLSv1.3 the handshake is NOT sent in an extra round trip which means there will be + // no exception reported in this case but just the channel will be closed. + assertTrue(clientException instanceof SSLHandshakeException || clientException == null); } } catch (SkipTestException e) { // ALPN availability is dependent on the java version. If ALPN is not available because of @@ -358,4 +364,16 @@ public class JdkSslEngineTest extends SSLEngineTest { super(message); } } + + private final class TestDelegatingSslContext extends DelegatingSslContext { + TestDelegatingSslContext(SslContext ctx) { + super(ctx); + } + + @Override + protected void initEngine(SSLEngine engine) { + engine.setEnabledProtocols(protocols()); + engine.setEnabledCipherSuites(ciphers().toArray(EmptyArrays.EMPTY_STRINGS)); + } + } } diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java index 99fb8fa82e..630de226d6 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslEngineTest.java @@ -67,17 +67,21 @@ public class OpenSslEngineTest extends SSLEngineTest { private static final String PREFERRED_APPLICATION_LEVEL_PROTOCOL = "my-protocol-http2"; private static final String FALLBACK_APPLICATION_LEVEL_PROTOCOL = "my-protocol-http1_1"; - @Parameterized.Parameters(name = "{index}: bufferType = {0}") - public static Collection data() { - List params = new ArrayList(); + @Parameterized.Parameters(name = "{index}: bufferType = {0}, combo = {1}") + public static Collection data() { + List params = new ArrayList(); for (BufferType type: BufferType.values()) { - params.add(type); + params.add(new Object[] { type, ProtocolCipherCombo.tlsv12()}); + + if (PlatformDependent.javaVersion() >= 11 && OpenSsl.isTlsv13Supported()) { + params.add(new Object[] { type, ProtocolCipherCombo.tlsv13() }); + } } return params; } - public OpenSslEngineTest(BufferType type) { - super(type); + public OpenSslEngineTest(BufferType type, ProtocolCipherCombo cipherCombo) { + super(type, cipherCombo); } @BeforeClass @@ -206,13 +210,17 @@ public class OpenSslEngineTest extends SSLEngineTest { @Test public void testWrapBuffersNoWritePendingError() throws Exception { clientSslCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(sslClientProvider()) - .build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .build(); + .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SSLEngine clientEngine = null; SSLEngine serverEngine = null; try { @@ -240,13 +248,17 @@ public class OpenSslEngineTest extends SSLEngineTest { @Test public void testOnlySmallBufferNeededForWrap() throws Exception { clientSslCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(sslClientProvider()) - .build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .build(); + .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SSLEngine clientEngine = null; SSLEngine serverEngine = null; try { @@ -291,13 +303,17 @@ public class OpenSslEngineTest extends SSLEngineTest { @Test public void testNeededDstCapacityIsCorrectlyCalculated() throws Exception { clientSslCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(sslClientProvider()) - .build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .build(); + .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SSLEngine clientEngine = null; SSLEngine serverEngine = null; try { @@ -327,13 +343,17 @@ public class OpenSslEngineTest extends SSLEngineTest { @Test public void testSrcsLenOverFlowCorrectlyHandled() throws Exception { clientSslCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(sslClientProvider()) - .build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .build(); + .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SSLEngine clientEngine = null; SSLEngine serverEngine = null; try { @@ -374,9 +394,11 @@ public class OpenSslEngineTest extends SSLEngineTest { @Test public void testCalculateOutNetBufSizeOverflow() throws SSLException { clientSslCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(sslClientProvider()) - .build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SSLEngine clientEngine = null; try { clientEngine = clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT); @@ -390,9 +412,11 @@ public class OpenSslEngineTest extends SSLEngineTest { @Test public void testCalculateOutNetBufSize0() throws SSLException { clientSslCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(sslClientProvider()) - .build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SSLEngine clientEngine = null; try { clientEngine = clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT); @@ -415,13 +439,17 @@ public class OpenSslEngineTest extends SSLEngineTest { private void testCorrectlyCalculateSpaceForAlert(boolean jdkCompatabilityMode) throws Exception { SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .build(); + .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); clientSslCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(sslClientProvider()) - .build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SSLEngine clientEngine = null; SSLEngine serverEngine = null; try { @@ -473,13 +501,13 @@ public class OpenSslEngineTest extends SSLEngineTest { @Test public void testWrapWithDifferentSizesTLSv1() throws Exception { clientSslCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(sslClientProvider()) - .build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .build(); SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .build(); + .sslProvider(sslServerProvider()) + .build(); testWrapWithDifferentSizes(PROTOCOL_TLS_V1, "AES128-SHA"); testWrapWithDifferentSizes(PROTOCOL_TLS_V1, "ECDHE-RSA-AES128-SHA"); @@ -504,13 +532,13 @@ public class OpenSslEngineTest extends SSLEngineTest { @Test public void testWrapWithDifferentSizesTLSv1_1() throws Exception { clientSslCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(sslClientProvider()) - .build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .build(); SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .build(); + .sslProvider(sslServerProvider()) + .build(); testWrapWithDifferentSizes(PROTOCOL_TLS_V1_1, "ECDHE-RSA-AES256-SHA"); testWrapWithDifferentSizes(PROTOCOL_TLS_V1_1, "AES256-SHA"); @@ -613,12 +641,16 @@ public class OpenSslEngineTest extends SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newHandler(UnpooledByteBufAllocator.DEFAULT).engine()); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newHandler(UnpooledByteBufAllocator.DEFAULT).engine()); @@ -690,12 +722,16 @@ public class OpenSslEngineTest extends SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newHandler(UnpooledByteBufAllocator.DEFAULT).engine()); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newHandler(UnpooledByteBufAllocator.DEFAULT).engine()); @@ -774,12 +810,16 @@ public class OpenSslEngineTest extends SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newHandler(UnpooledByteBufAllocator.DEFAULT).engine()); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newHandler(UnpooledByteBufAllocator.DEFAULT).engine()); @@ -849,12 +889,16 @@ public class OpenSslEngineTest extends SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newHandler(UnpooledByteBufAllocator.DEFAULT).engine()); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newHandler(UnpooledByteBufAllocator.DEFAULT).engine()); @@ -982,8 +1026,10 @@ public class OpenSslEngineTest extends SSLEngineTest { assumeTrue(PlatformDependent.javaVersion() >= 8); SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .build(); + .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SSLEngine engine = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); try { @@ -1002,8 +1048,10 @@ public class OpenSslEngineTest extends SSLEngineTest { byte[] name = "rb8hx3pww30y3tvw0mwy.v1_1".getBytes(CharsetUtil.UTF_8); SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .build(); + .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SSLEngine engine = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); try { @@ -1022,8 +1070,10 @@ public class OpenSslEngineTest extends SSLEngineTest { public void testAlgorithmConstraintsThrows() throws Exception { SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .build(); + .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); SSLEngine engine = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); try { diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslJdkSslEngineInteroptTest.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslJdkSslEngineInteroptTest.java index d2a00c5432..bc1106e7a3 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslJdkSslEngineInteroptTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslJdkSslEngineInteroptTest.java @@ -15,6 +15,7 @@ */ package io.netty.handler.ssl; +import io.netty.util.internal.PlatformDependent; import org.junit.BeforeClass; import org.junit.Ignore; import org.junit.Test; @@ -34,17 +35,21 @@ import static org.junit.Assume.assumeTrue; @RunWith(Parameterized.class) public class OpenSslJdkSslEngineInteroptTest extends SSLEngineTest { - @Parameterized.Parameters(name = "{index}: bufferType = {0}") - public static Collection data() { - List params = new ArrayList(); + @Parameterized.Parameters(name = "{index}: bufferType = {0}, combo = {1}") + public static Collection data() { + List params = new ArrayList(); for (BufferType type: BufferType.values()) { - params.add(type); + params.add(new Object[] { type, ProtocolCipherCombo.tlsv12()}); + + if (PlatformDependent.javaVersion() >= 11 && OpenSsl.isTlsv13Supported()) { + params.add(new Object[] { type, ProtocolCipherCombo.tlsv13() }); + } } return params; } - public OpenSslJdkSslEngineInteroptTest(BufferType type) { - super(type); + public OpenSslJdkSslEngineInteroptTest(BufferType type, ProtocolCipherCombo combo) { + super(type, combo); } @BeforeClass diff --git a/handler/src/test/java/io/netty/handler/ssl/OpenSslTestUtils.java b/handler/src/test/java/io/netty/handler/ssl/OpenSslTestUtils.java index e8c46ed7a0..0a3ff97f32 100644 --- a/handler/src/test/java/io/netty/handler/ssl/OpenSslTestUtils.java +++ b/handler/src/test/java/io/netty/handler/ssl/OpenSslTestUtils.java @@ -15,6 +15,12 @@ */ package io.netty.handler.ssl; +import io.netty.util.internal.PlatformDependent; + +import java.util.Arrays; +import java.util.Collections; + +import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_2; import static org.junit.Assume.assumeTrue; final class OpenSslTestUtils { @@ -28,4 +34,17 @@ final class OpenSslTestUtils { static boolean isBoringSSL() { return "BoringSSL".equals(OpenSsl.versionString()); } + + static SslContextBuilder configureProtocolForMutualAuth( + SslContextBuilder ctx, SslProvider sslClientProvider, SslProvider sslServerProvider) { + if (PlatformDependent.javaVersion() >= 11 + && sslClientProvider == SslProvider.JDK && sslServerProvider != SslProvider.JDK) { + // Make sure we do not use TLSv1.3 as there seems to be a bug currently in the JDK TLSv1.3 implementation. + // See: + // - http://mail.openjdk.java.net/pipermail/security-dev/2018-September/018191.html + // - https://bugs.openjdk.java.net/projects/JDK/issues/JDK-8210846 + ctx.protocols(PROTOCOL_TLS_V1_2).ciphers(Collections.singleton("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256")); + } + return ctx; + } } diff --git a/handler/src/test/java/io/netty/handler/ssl/ParameterizedSslHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/ParameterizedSslHandlerTest.java index 2abc33e8a3..780ddf670e 100644 --- a/handler/src/test/java/io/netty/handler/ssl/ParameterizedSslHandlerTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/ParameterizedSslHandlerTest.java @@ -381,12 +381,21 @@ public class ParameterizedSslHandlerTest { SelfSignedCertificate ssc = new SelfSignedCertificate(); final SslContext sslServerCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(serverProvider) - .build(); + .sslProvider(serverProvider) + // Use TLSv1.2 as we depend on the fact that the handshake + // is done in an extra round trip in the test which + // is not true in TLSv1.3 + .protocols(SslUtils.PROTOCOL_TLS_V1_2) + .build(); final SslContext sslClientCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(clientProvider).build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(clientProvider) + // Use TLSv1.2 as we depend on the fact that the handshake + // is done in an extra round trip in the test which + // is not true in TLSv1.3 + .protocols(SslUtils.PROTOCOL_TLS_V1_2) + .build(); EventLoopGroup group = new NioEventLoopGroup(); Channel sc = null; diff --git a/handler/src/test/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngineTest.java index ddbd0b16de..588619d3a7 100644 --- a/handler/src/test/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/ReferenceCountedOpenSslEngineTest.java @@ -23,8 +23,8 @@ import javax.net.ssl.SSLEngine; public class ReferenceCountedOpenSslEngineTest extends OpenSslEngineTest { - public ReferenceCountedOpenSslEngineTest(BufferType type) { - super(type); + public ReferenceCountedOpenSslEngineTest(BufferType type, ProtocolCipherCombo combo) { + super(type, combo); } @Override @@ -60,9 +60,11 @@ public class ReferenceCountedOpenSslEngineTest extends OpenSslEngineTest { @Test(expected = NullPointerException.class) public void testNotLeakOnException() throws Exception { clientSslCtx = SslContextBuilder.forClient() - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .sslProvider(sslClientProvider()) - .build(); + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); clientSslCtx.newEngine(null); } diff --git a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java index 4248e1fca4..0094c92d3e 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java @@ -33,6 +33,7 @@ import io.netty.channel.nio.NioEventLoopGroup; import io.netty.channel.socket.SocketChannel; import io.netty.channel.socket.nio.NioServerSocketChannel; import io.netty.channel.socket.nio.NioSocketChannel; +import io.netty.handler.ssl.ApplicationProtocolConfig.Protocol; import io.netty.handler.ssl.util.InsecureTrustManagerFactory; import io.netty.handler.ssl.util.SelfSignedCertificate; import io.netty.handler.ssl.util.SimpleTrustManagerFactory; @@ -67,6 +68,7 @@ import java.security.Provider; import java.security.cert.Certificate; import java.security.cert.CertificateException; import java.util.Arrays; +import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -79,6 +81,7 @@ import javax.net.ssl.ManagerFactoryParameters; import javax.net.ssl.SNIHostName; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; +import javax.net.ssl.SSLEngineResult.Status; import javax.net.ssl.SSLException; import javax.net.ssl.SSLHandshakeException; import javax.net.ssl.SSLParameters; @@ -89,14 +92,7 @@ import javax.net.ssl.TrustManagerFactorySpi; import javax.net.ssl.X509TrustManager; import javax.security.cert.X509Certificate; -import static io.netty.handler.ssl.SslUtils.PROTOCOL_SSL_V2; -import static io.netty.handler.ssl.SslUtils.PROTOCOL_SSL_V2_HELLO; -import static io.netty.handler.ssl.SslUtils.PROTOCOL_SSL_V3; -import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1; -import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_1; -import static io.netty.handler.ssl.SslUtils.PROTOCOL_TLS_V1_2; -import static io.netty.handler.ssl.SslUtils.SSL_RECORD_HEADER_LENGTH; - +import static io.netty.handler.ssl.SslUtils.*; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; @@ -220,10 +216,42 @@ public abstract class SSLEngineTest { Mixed } - private final BufferType type; + static final class ProtocolCipherCombo { + private static final ProtocolCipherCombo TLSV12 = new ProtocolCipherCombo( + PROTOCOL_TLS_V1_2, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256"); + private static final ProtocolCipherCombo TLSV13 = new ProtocolCipherCombo( + PROTOCOL_TLS_V1_3, "TLS_AES_128_GCM_SHA256"); + final String protocol; + final String cipher; - protected SSLEngineTest(BufferType type) { + private ProtocolCipherCombo(String protocol, String cipher) { + this.protocol = protocol; + this.cipher = cipher; + } + + static ProtocolCipherCombo tlsv12() { + return TLSV12; + } + + static ProtocolCipherCombo tlsv13() { + return TLSV13; + } + + @Override + public String toString() { + return "ProtocolCipherCombo{" + + "protocol='" + protocol + '\'' + + ", cipher='" + cipher + '\'' + + '}'; + } + } + + private final BufferType type; + private final ProtocolCipherCombo protocolCipherCombo; + + protected SSLEngineTest(BufferType type, ProtocolCipherCombo protocolCipherCombo) { this.type = type; + this.protocolCipherCombo = protocolCipherCombo; } protected ByteBuffer allocateBuffer(int len) { @@ -620,36 +648,46 @@ public abstract class SSLEngineTest { } protected boolean mySetupMutualAuthServerIsValidException(Throwable cause) { - return cause instanceof SSLHandshakeException || cause instanceof ClosedChannelException; + // As in TLSv1.3 the handshake is sent without an extra roundtrip an SSLException is valid as well. + return cause instanceof SSLException || cause instanceof ClosedChannelException; } protected void mySetupMutualAuthServerInitSslHandler(SslHandler handler) { } + private SslContextBuilder configureProtocolForMutualAuth(SslContextBuilder ctx) { + return OpenSslTestUtils.configureProtocolForMutualAuth(ctx, sslClientProvider(), sslServerProvider()); + } + private void mySetupMutualAuth(KeyManagerFactory serverKMF, final File serverTrustManager, KeyManagerFactory clientKMF, File clientTrustManager, ClientAuth clientAuth, final boolean failureExpected, final boolean serverInitEngine) throws SSLException, InterruptedException { - serverSslCtx = SslContextBuilder.forServer(serverKMF) - .sslProvider(sslServerProvider()) - .sslContextProvider(serverSslContextProvider()) - .trustManager(serverTrustManager) - .clientAuth(clientAuth) - .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) - .sessionCacheSize(0) - .sessionTimeout(0) - .build(); + serverSslCtx = configureProtocolForMutualAuth( + SslContextBuilder.forServer(serverKMF) + .protocols(protocols()) + .ciphers(ciphers()) + .sslProvider(sslServerProvider()) + .sslContextProvider(serverSslContextProvider()) + .trustManager(serverTrustManager) + .clientAuth(clientAuth) + .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) + .sessionCacheSize(0) + .sessionTimeout(0)).build(); + + clientSslCtx = configureProtocolForMutualAuth( + SslContextBuilder.forClient() + .protocols(protocols()) + .ciphers(ciphers()) + .sslProvider(sslClientProvider()) + .sslContextProvider(clientSslContextProvider()) + .trustManager(clientTrustManager) + .keyManager(clientKMF) + .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) + .sessionCacheSize(0) + .sessionTimeout(0)).build(); - clientSslCtx = SslContextBuilder.forClient() - .sslProvider(sslClientProvider()) - .sslContextProvider(clientSslContextProvider()) - .trustManager(clientTrustManager) - .keyManager(clientKMF) - .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) - .sessionCacheSize(0) - .sessionTimeout(0) - .build(); serverConnectedChannel = null; sb = new ServerBootstrap(); cb = new Bootstrap(); @@ -711,10 +749,11 @@ public abstract class SSLEngineTest { @Override public void userEventTriggered(ChannelHandlerContext ctx, Object evt) throws Exception { if (evt == SslHandshakeCompletionEvent.SUCCESS) { - if (failureExpected) { - clientException = new IllegalStateException("handshake complete. expected failure"); + // With TLS1.3 a mutal auth error will not be propagated as a handshake error most of the + // time as the handshake needs NO extra roundtrip. + if (!failureExpected) { + clientLatch.countDown(); } - clientLatch.countDown(); } else if (evt instanceof SslHandshakeCompletionEvent) { clientException = ((SslHandshakeCompletionEvent) evt).cause(); clientLatch.countDown(); @@ -724,7 +763,7 @@ public abstract class SSLEngineTest { @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { - if (cause.getCause() instanceof SSLHandshakeException) { + if (cause.getCause() instanceof SSLException) { clientException = cause.getCause(); clientLatch.countDown(); } else { @@ -735,7 +774,7 @@ public abstract class SSLEngineTest { } }); - serverChannel = sb.bind(new InetSocketAddress(0)).sync().channel(); + serverChannel = sb.bind(new InetSocketAddress(8443)).sync().channel(); int port = ((InetSocketAddress) serverChannel.localAddress()).getPort(); ChannelFuture ccf = cb.connect(new InetSocketAddress(NetUtil.LOCALHOST, port)); @@ -776,6 +815,8 @@ public abstract class SSLEngineTest { final String expectedHost = "localhost"; serverSslCtx = SslContextBuilder.forServer(serverCrtFile, serverKeyFile, null) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .sslContextProvider(serverSslContextProvider()) .trustManager(InsecureTrustManagerFactory.INSTANCE) .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) @@ -785,12 +826,15 @@ public abstract class SSLEngineTest { clientSslCtx = SslContextBuilder.forClient() .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .sslContextProvider(clientSslContextProvider()) .trustManager(clientTrustCrtFile) .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) .sessionCacheSize(0) .sessionTimeout(0) .build(); + serverConnectedChannel = null; sb = new ServerBootstrap(); cb = new Bootstrap(); @@ -897,24 +941,28 @@ public abstract class SSLEngineTest { File servertTrustCrtFile, File serverKeyFile, final File serverCrtFile, String serverKeyPassword, File clientTrustCrtFile, File clientKeyFile, File clientCrtFile, String clientKeyPassword) throws InterruptedException, SSLException { - serverSslCtx = SslContextBuilder.forServer(serverCrtFile, serverKeyFile, serverKeyPassword) - .sslProvider(sslServerProvider()) - .sslContextProvider(serverSslContextProvider()) - .trustManager(servertTrustCrtFile) - .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) - .sessionCacheSize(0) - .sessionTimeout(0) - .build(); + serverSslCtx = configureProtocolForMutualAuth( + SslContextBuilder.forServer(serverCrtFile, serverKeyFile, serverKeyPassword) + .sslProvider(sslServerProvider()) + .sslContextProvider(serverSslContextProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .trustManager(servertTrustCrtFile) + .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) + .sessionCacheSize(0) + .sessionTimeout(0)).build(); + clientSslCtx = configureProtocolForMutualAuth( + SslContextBuilder.forClient() + .sslProvider(sslClientProvider()) + .sslContextProvider(clientSslContextProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .trustManager(clientTrustCrtFile) + .keyManager(clientCrtFile, clientKeyFile, clientKeyPassword) + .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) + .sessionCacheSize(0) + .sessionTimeout(0)).build(); - clientSslCtx = SslContextBuilder.forClient() - .sslProvider(sslClientProvider()) - .sslContextProvider(clientSslContextProvider()) - .trustManager(clientTrustCrtFile) - .keyManager(clientCrtFile, clientKeyFile, clientKeyPassword) - .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) - .sessionCacheSize(0) - .sessionTimeout(0) - .build(); serverConnectedChannel = null; sb = new ServerBootstrap(); cb = new Bootstrap(); @@ -930,6 +978,7 @@ public abstract class SSLEngineTest { SSLEngine engine = serverSslCtx.newEngine(ch.alloc()); engine.setUseClientMode(false); engine.setNeedClientAuth(true); + p.addLast(new SslHandler(engine)); p.addLast(new MessageDelegatorChannelHandler(serverReceiver, serverLatch)); p.addLast(new ChannelInboundHandlerAdapter() { @@ -986,13 +1035,14 @@ public abstract class SSLEngineTest { protected void initChannel(Channel ch) throws Exception { ch.config().setAllocator(new TestByteBufAllocator(ch.config().getAllocator(), type)); + SslHandler handler = clientSslCtx.newHandler(ch.alloc()); + handler.engine().setNeedClientAuth(true); ChannelPipeline p = ch.pipeline(); - p.addLast(clientSslCtx.newHandler(ch.alloc())); + p.addLast(handler); p.addLast(new MessageDelegatorChannelHandler(clientReceiver, clientLatch)); p.addLast(new ChannelInboundHandlerAdapter() { @Override public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) throws Exception { - cause.printStackTrace(); if (cause.getCause() instanceof SSLHandshakeException) { clientException = cause.getCause(); clientLatch.countDown(); @@ -1081,11 +1131,15 @@ public abstract class SSLEngineTest { .trustManager(InsecureTrustManagerFactory.INSTANCE) .sslProvider(sslClientProvider()) .sslContextProvider(clientSslContextProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) .sslProvider(sslServerProvider()) .sslContextProvider(serverSslContextProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine clientEngine = null; SSLEngine serverEngine = null; @@ -1110,11 +1164,15 @@ public abstract class SSLEngineTest { clientSslCtx = SslContextBuilder.forClient() .trustManager(InsecureTrustManagerFactory.INSTANCE) .sslProvider(sslClientProvider()) + // This test only works for non TLSv1.3 for now + .protocols(PROTOCOL_TLS_V1_2) .sslContextProvider(clientSslContextProvider()) .build(); SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) .sslProvider(sslServerProvider()) + // This test only works for non TLSv1.3 for now + .protocols(PROTOCOL_TLS_V1_2) .sslContextProvider(serverSslContextProvider()) .build(); SSLEngine clientEngine = null; @@ -1145,8 +1203,11 @@ public abstract class SSLEngineTest { throws CertificateException, SSLException, InterruptedException, ExecutionException { final SelfSignedCertificate ssc = new SelfSignedCertificate(); serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(sslServerProvider()) - .sslContextProvider(serverSslContextProvider()).build(); + .sslProvider(sslServerProvider()) + .sslContextProvider(serverSslContextProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); sb = new ServerBootstrap() .group(new NioEventLoopGroup(1)) .channel(NioServerSocketChannel.class) @@ -1196,8 +1257,12 @@ public abstract class SSLEngineTest { serverChannel = sb.bind(new InetSocketAddress(0)).syncUninterruptibly().channel(); clientSslCtx = SslContextBuilder.forClient() - .sslProvider(SslProvider.JDK) // OpenSslEngine doesn't support renegotiation on client side - .trustManager(InsecureTrustManagerFactory.INSTANCE).build(); + // OpenSslEngine doesn't support renegotiation on client side + .sslProvider(SslProvider.JDK) + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); cb = new Bootstrap(); cb.group(new NioEventLoopGroup(1)) @@ -1257,9 +1322,11 @@ public abstract class SSLEngineTest { File serverKeyFile = new File(getClass().getResource("test_unencrypted.pem").getFile()); File serverCrtFile = new File(getClass().getResource("test.crt").getFile()); serverSslCtx = SslContextBuilder.forServer(serverCrtFile, serverKeyFile) - .sslProvider(sslServerProvider()) - .sslContextProvider(serverSslContextProvider()) - .build(); + .sslProvider(sslServerProvider()) + .sslContextProvider(serverSslContextProvider()) + .protocols(protocols()) + .ciphers(ciphers()) + .build(); sslEngine = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -1338,7 +1405,10 @@ public abstract class SSLEngineTest { cTOsPos = cTOs.position(); sTOcPos = sTOc.position(); - if (!clientHandshakeFinished) { + if (!clientHandshakeFinished || + // After the handshake completes it is possible we have more data that was send by the server as + // the server will send session updates after the handshake. In this case continue to unwrap. + SslUtils.PROTOCOL_TLS_V1_3.equals(clientEngine.getSession().getProtocol())) { int clientAppReadBufferPos = clientAppReadBuffer.position(); clientResult = clientEngine.unwrap(sTOc, clientAppReadBuffer); @@ -1350,7 +1420,7 @@ public abstract class SSLEngineTest { clientHandshakeFinished = true; } } else { - assertFalse(sTOc.hasRemaining()); + assertEquals(0, sTOc.remaining()); } if (!serverHandshakeFinished) { @@ -1433,24 +1503,35 @@ public abstract class SSLEngineTest { SelfSignedCertificate ssc = new SelfSignedCertificate(); try { - setupHandlers(SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey(), null) - .sslProvider(sslServerProvider()) - .sslContextProvider(serverSslContextProvider()) - .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) - .applicationProtocolConfig(serverApn) - .sessionCacheSize(0) - .sessionTimeout(0) - .build(), + SslContextBuilder serverCtxBuilder = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey(), null) + .sslProvider(sslServerProvider()) + .sslContextProvider(serverSslContextProvider()) + .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) + .applicationProtocolConfig(serverApn) + .sessionCacheSize(0) + .sessionTimeout(0); + if (serverApn.protocol() == Protocol.NPN || serverApn.protocol() == Protocol.NPN_AND_ALPN) { + // NPN is not really well supported with TLSv1.3 so force to use TLSv1.2 + // See https://github.com/openssl/openssl/issues/3665 + serverCtxBuilder.protocols(PROTOCOL_TLS_V1_2); + } - SslContextBuilder.forClient() - .sslProvider(sslClientProvider()) - .sslContextProvider(clientSslContextProvider()) - .applicationProtocolConfig(clientApn) - .trustManager(InsecureTrustManagerFactory.INSTANCE) - .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) - .sessionCacheSize(0) - .sessionTimeout(0) - .build()); + SslContextBuilder clientCtxBuilder = SslContextBuilder.forClient() + .sslProvider(sslClientProvider()) + .sslContextProvider(clientSslContextProvider()) + .applicationProtocolConfig(clientApn) + .trustManager(InsecureTrustManagerFactory.INSTANCE) + .ciphers(null, IdentityCipherSuiteFilter.INSTANCE) + .sessionCacheSize(0) + .sessionTimeout(0); + + if (clientApn.protocol() == Protocol.NPN || clientApn.protocol() == Protocol.NPN_AND_ALPN) { + // NPN is not really well supported with TLSv1.3 so force to use TLSv1.2 + // See https://github.com/openssl/openssl/issues/3665 + clientCtxBuilder.protocols(PROTOCOL_TLS_V1_2); + } + + setupHandlers(serverCtxBuilder.build(), clientCtxBuilder.build()); } finally { ssc.delete(); } @@ -1511,6 +1592,11 @@ public abstract class SSLEngineTest { ctx.fireExceptionCaught(cause); } } + + @Override + public void channelInactive(ChannelHandlerContext ctx) throws Exception { + clientLatch.countDown(); + } }); } }); @@ -1524,12 +1610,15 @@ public abstract class SSLEngineTest { @Test(timeout = 30000) public void testMutualAuthSameCertChain() throws Exception { - serverSslCtx = SslContextBuilder.forServer( - new ByteArrayInputStream(X509_CERT_PEM.getBytes(CharsetUtil.UTF_8)), - new ByteArrayInputStream(PRIVATE_KEY_PEM.getBytes(CharsetUtil.UTF_8))) - .trustManager(new ByteArrayInputStream(X509_CERT_PEM.getBytes(CharsetUtil.UTF_8))) - .clientAuth(ClientAuth.REQUIRE).sslProvider(sslServerProvider()) - .sslContextProvider(serverSslContextProvider()).build(); + serverSslCtx = configureProtocolForMutualAuth( + SslContextBuilder.forServer( + new ByteArrayInputStream(X509_CERT_PEM.getBytes(CharsetUtil.UTF_8)), + new ByteArrayInputStream(PRIVATE_KEY_PEM.getBytes(CharsetUtil.UTF_8))) + .trustManager(new ByteArrayInputStream(X509_CERT_PEM.getBytes(CharsetUtil.UTF_8))) + .clientAuth(ClientAuth.REQUIRE).sslProvider(sslServerProvider()) + .sslContextProvider(serverSslContextProvider()) + .protocols(protocols()) + .ciphers(ciphers())).build(); sb = new ServerBootstrap(); sb.group(new NioEventLoopGroup(), new NioEventLoopGroup()); @@ -1580,13 +1669,14 @@ public abstract class SSLEngineTest { } }).bind(new InetSocketAddress(0)).syncUninterruptibly().channel(); - clientSslCtx = SslContextBuilder.forClient() - .keyManager( + clientSslCtx = configureProtocolForMutualAuth( + SslContextBuilder.forClient().keyManager( new ByteArrayInputStream(CLIENT_X509_CERT_CHAIN_PEM.getBytes(CharsetUtil.UTF_8)), new ByteArrayInputStream(CLIENT_PRIVATE_KEY_PEM.getBytes(CharsetUtil.UTF_8))) .trustManager(new ByteArrayInputStream(X509_CERT_PEM.getBytes(CharsetUtil.UTF_8))) .sslProvider(sslClientProvider()) - .sslContextProvider(clientSslContextProvider()).build(); + .sslContextProvider(clientSslContextProvider()) + .protocols(protocols()).ciphers(ciphers())).build(); cb = new Bootstrap(); cb.group(new NioEventLoopGroup()); cb.channel(NioSocketChannel.class); @@ -1610,12 +1700,16 @@ public abstract class SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -1790,12 +1884,16 @@ public abstract class SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -1829,6 +1927,8 @@ public abstract class SSLEngineTest { clientSslCtx = SslContextBuilder .forClient() .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -1857,6 +1957,8 @@ public abstract class SSLEngineTest { clientSslCtx = SslContextBuilder .forClient() .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -1881,12 +1983,16 @@ public abstract class SSLEngineTest { clientSslCtx = SslContextBuilder .forClient() .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -1928,12 +2034,16 @@ public abstract class SSLEngineTest { clientSslCtx = SslContextBuilder .forClient() .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -1965,12 +2075,16 @@ public abstract class SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + // This test only works for non TLSv1.3 for now + .protocols(PROTOCOL_TLS_V1_2) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + // This test only works for non TLSv1.3 for now + .protocols(PROTOCOL_TLS_V1_2) .build(); SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -2032,6 +2146,7 @@ public abstract class SSLEngineTest { result = server.wrap(empty, encryptedServerToClient); encryptedServerToClient.flip(); + assertEquals(SSLEngineResult.Status.CLOSED, result.getStatus()); // UNWRAP/WRAP are not expected after this point assertEquals(SSLEngineResult.HandshakeStatus.NOT_HANDSHAKING, result.getHandshakeStatus()); @@ -2046,6 +2161,7 @@ public abstract class SSLEngineTest { assertTrue(server.isInboundDone()); result = client.unwrap(encryptedServerToClient, plainClientOut); + plainClientOut.flip(); assertEquals(SSLEngineResult.Status.CLOSED, result.getStatus()); // UNWRAP/WRAP are not expected after this point @@ -2106,12 +2222,16 @@ public abstract class SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -2145,12 +2265,16 @@ public abstract class SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -2220,12 +2344,16 @@ public abstract class SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -2240,21 +2368,35 @@ public abstract class SSLEngineTest { int srcLen = plainClientOut.remaining(); SSLEngineResult result; - while (encClientToServer.position() <= server.getSession().getPacketBufferSize()) { + int count = 0; + do { + int plainClientOutPosition = plainClientOut.position(); + int encClientToServerPosition = encClientToServer.position(); result = client.wrap(plainClientOut, encClientToServer); + if (result.getStatus() == Status.BUFFER_OVERFLOW) { + // We did not have enough room to wrap + assertEquals(plainClientOutPosition, plainClientOut.position()); + assertEquals(encClientToServerPosition, encClientToServer.position()); + break; + } assertEquals(SSLEngineResult.Status.OK, result.getStatus()); assertEquals(srcLen, result.bytesConsumed()); assertTrue(result.bytesProduced() > 0); plainClientOut.clear(); - } + ++count; + } while (encClientToServer.position() < server.getSession().getPacketBufferSize()); + + // Check that we were able to wrap multiple times. + assertTrue(count >= 2); encClientToServer.flip(); result = server.unwrap(encClientToServer, plainServerOut); assertEquals(SSLEngineResult.Status.OK, result.getStatus()); assertTrue(result.bytesConsumed() > 0); assertTrue(result.bytesProduced() > 0); + assertTrue(encClientToServer.hasRemaining()); } finally { cert.delete(); cleanupClientSslEngine(client); @@ -2270,12 +2412,16 @@ public abstract class SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -2341,12 +2487,16 @@ public abstract class SSLEngineTest { .forClient() .trustManager(cert.cert()) .sslProvider(sslClientProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine client = wrapEngine(clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); serverSslCtx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -2393,6 +2543,8 @@ public abstract class SSLEngineTest { SslContext ctx = SslContextBuilder .forServer(cert.certificate(), cert.privateKey()) .sslProvider(sslServerProvider()) + .protocols(protocols()) + .ciphers(ciphers()) .build(); SSLEngine server = wrapEngine(ctx.newEngine(UnpooledByteBufAllocator.DEFAULT)); @@ -2495,4 +2647,12 @@ public abstract class SSLEngineTest { protected SSLEngine wrapEngine(SSLEngine engine) { return engine; } + + protected List ciphers() { + return Collections.singletonList(protocolCipherCombo.cipher); + } + + protected String[] protocols() { + return new String[] { protocolCipherCombo.protocol }; + } } diff --git a/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java b/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java index 0a9429e9c1..20f2ccbb14 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslContextBuilderTest.java @@ -15,18 +15,19 @@ */ package io.netty.handler.ssl; -import static org.junit.Assert.assertFalse; -import static org.junit.Assert.assertTrue; - import io.netty.buffer.UnpooledByteBufAllocator; import io.netty.handler.ssl.util.SelfSignedCertificate; import org.junit.Assume; +import org.junit.Ignore; +import org.junit.Rule; import org.junit.Test; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; import java.util.Collections; +import static org.junit.Assert.*; + public class SslContextBuilderTest { @Test @@ -79,10 +80,19 @@ public class SslContextBuilderTest { testInvalidCipher(SslProvider.JDK); } - @Test(expected = SSLException.class) + @Test public void testInvalidCipherOpenSSL() throws Exception { Assume.assumeTrue(OpenSsl.isAvailable()); - testInvalidCipher(SslProvider.OPENSSL); + try { + // This may fail or not depending on the OpenSSL version used + // See https://github.com/openssl/openssl/issues/7196 + testInvalidCipher(SslProvider.OPENSSL); + if (!OpenSsl.versionString().contains("1.1.1")) { + fail(); + } + } catch (SSLException expected) { + // ok + } } private static void testInvalidCipher(SslProvider provider) throws Exception { diff --git a/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java b/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java index d935cdf683..ab7de93787 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java @@ -124,9 +124,10 @@ public class SslErrorTest { Assume.assumeTrue(OpenSsl.isAvailable()); SelfSignedCertificate ssc = new SelfSignedCertificate(); - final SslContext sslServerCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) - .sslProvider(serverProvider) - .trustManager(new SimpleTrustManagerFactory() { + final SslContext sslServerCtx = OpenSslTestUtils.configureProtocolForMutualAuth( + SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey()) + .sslProvider(serverProvider) + .trustManager(new SimpleTrustManagerFactory() { @Override protected void engineInit(KeyStore keyStore) { } @Override @@ -154,13 +155,13 @@ public class SslErrorTest { } } }; } - }).clientAuth(ClientAuth.REQUIRE).build(); + }).clientAuth(ClientAuth.REQUIRE), clientProvider, serverProvider).build(); - final SslContext sslClientCtx = SslContextBuilder.forClient() + final SslContext sslClientCtx = OpenSslTestUtils.configureProtocolForMutualAuth(SslContextBuilder.forClient() .trustManager(InsecureTrustManagerFactory.INSTANCE) .keyManager(new File(getClass().getResource("test.crt").getFile()), new File(getClass().getResource("test_unencrypted.pem").getFile())) - .sslProvider(clientProvider).build(); + .sslProvider(clientProvider), clientProvider, serverProvider).build(); Channel serverChannel = null; Channel clientChannel = null; diff --git a/handler/src/test/java/io/netty/handler/ssl/SslUtilsTest.java b/handler/src/test/java/io/netty/handler/ssl/SslUtilsTest.java index c1de33dd6e..ce9a22d717 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslUtilsTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslUtilsTest.java @@ -28,6 +28,7 @@ import java.security.NoSuchAlgorithmException; import static io.netty.handler.ssl.SslUtils.getEncryptedPacketLength; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; public class SslUtilsTest { @@ -63,4 +64,15 @@ public class SslUtilsTest { engine.beginHandshake(); return engine; } + + @Test + public void testIsTLSv13Cipher() { + assertTrue(SslUtils.isTLSv13Cipher("TLS_AES_128_GCM_SHA256")); + assertTrue(SslUtils.isTLSv13Cipher("TLS_AES_256_GCM_SHA384")); + assertTrue(SslUtils.isTLSv13Cipher("TLS_CHACHA20_POLY1305_SHA256")); + assertTrue(SslUtils.isTLSv13Cipher("TLS_AES_128_CCM_SHA256")); + assertTrue(SslUtils.isTLSv13Cipher("TLS_AES_128_CCM_8_SHA256")); + assertFalse(SslUtils.isTLSv13Cipher("TLS_DHE_RSA_WITH_AES_128_GCM_SHA256")); + } + } diff --git a/pom.xml b/pom.xml index 6c42c7977b..9f7a80b759 100644 --- a/pom.xml +++ b/pom.xml @@ -241,7 +241,7 @@ fedora netty-tcnative - 2.0.17.Final + 2.0.18.Final ${os.detected.classifier} org.conscrypt conscrypt-openjdk-uber diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslClientRenegotiateTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslClientRenegotiateTest.java index 1a49bde78b..8036f081f4 100644 --- a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslClientRenegotiateTest.java +++ b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslClientRenegotiateTest.java @@ -138,7 +138,8 @@ public class SocketSslClientRenegotiateTest extends AbstractSocketTest { public void initChannel(Channel sch) throws Exception { serverChannel = sch; serverSslHandler = serverCtx.newHandler(sch.alloc()); - + // As we test renegotiation we should use a protocol that support it. + serverSslHandler.engine().setEnabledProtocols(new String[] { "TLSv1.2" }); sch.pipeline().addLast("ssl", serverSslHandler); sch.pipeline().addLast("handler", serverHandler); } @@ -150,7 +151,8 @@ public class SocketSslClientRenegotiateTest extends AbstractSocketTest { public void initChannel(Channel sch) throws Exception { clientChannel = sch; clientSslHandler = clientCtx.newHandler(sch.alloc()); - + // As we test renegotiation we should use a protocol that support it. + clientSslHandler.engine().setEnabledProtocols(new String[] { "TLSv1.2" }); sch.pipeline().addLast("ssl", clientSslHandler); sch.pipeline().addLast("handler", clientHandler); } diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslEchoTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslEchoTest.java index 7c94b5a671..4cdeae98be 100644 --- a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslEchoTest.java +++ b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslEchoTest.java @@ -123,17 +123,33 @@ public class SocketSslEchoTest extends AbstractSocketTest { "autoRead = {5}, useChunkedWriteHandler = {6}, useCompositeByteBuf = {7}") public static Collection data() throws Exception { List serverContexts = new ArrayList(); - serverContexts.add(SslContextBuilder.forServer(CERT_FILE, KEY_FILE).sslProvider(SslProvider.JDK).build()); + serverContexts.add(SslContextBuilder.forServer(CERT_FILE, KEY_FILE) + .sslProvider(SslProvider.JDK) + // As we test renegotiation we should use a protocol that support it. + .protocols("TLSv1.2") + .build()); List clientContexts = new ArrayList(); - clientContexts.add(SslContextBuilder.forClient().sslProvider(SslProvider.JDK).trustManager(CERT_FILE).build()); + clientContexts.add(SslContextBuilder.forClient() + .sslProvider(SslProvider.JDK) + .trustManager(CERT_FILE) + // As we test renegotiation we should use a protocol that support it. + .protocols("TLSv1.2") + .build()); boolean hasOpenSsl = OpenSsl.isAvailable(); if (hasOpenSsl) { serverContexts.add(SslContextBuilder.forServer(CERT_FILE, KEY_FILE) - .sslProvider(SslProvider.OPENSSL).build()); - clientContexts.add(SslContextBuilder.forClient().sslProvider(SslProvider.OPENSSL) - .trustManager(CERT_FILE).build()); + .sslProvider(SslProvider.OPENSSL) + // As we test renegotiation we should use a protocol that support it. + .protocols("TLSv1.2") + .build()); + clientContexts.add(SslContextBuilder.forClient() + .sslProvider(SslProvider.OPENSSL) + .trustManager(CERT_FILE) + // As we test renegotiation we should use a protocol that support it. + .protocols("TLSv1.2") + .build()); } else { logger.warn("OpenSSL is unavailable and thus will not be tested.", OpenSsl.unavailabilityCause()); } diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslSessionReuseTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslSessionReuseTest.java index 4071c84610..5d0fd0a5e4 100644 --- a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslSessionReuseTest.java +++ b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketSslSessionReuseTest.java @@ -98,7 +98,7 @@ public class SocketSslSessionReuseTest extends AbstractSocketTest { public void testSslSessionReuse(ServerBootstrap sb, Bootstrap cb) throws Throwable { final ReadAndDiscardHandler sh = new ReadAndDiscardHandler(true, true); final ReadAndDiscardHandler ch = new ReadAndDiscardHandler(false, true); - final String[] protocols = new String[]{ "TLSv1", "TLSv1.1", "TLSv1.2" }; + final String[] protocols = { "TLSv1", "TLSv1.1", "TLSv1.2" }; sb.childHandler(new ChannelInitializer() { @Override