From 43ae9748d0d2e33813b2d5e13cca951ab04dd033 Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Wed, 12 Jul 2017 12:53:39 -0700 Subject: [PATCH] Unify default cipher suites betweek JDK and OpenSSL Motivation: Currently the default cipher suites are set independently between JDK and OpenSSL. We should use a common approach to setting the default ciphers. Also the OpenSsl default ciphers are expressed in terms of the OpenSSL cipher name conventions, which is not correct and may be exposed to the end user. OpenSSL should also use the RFC cipher names like the JDK defaults. Modifications: - Move the default cipher definition to a common location and use it in JDK and OpenSSL initialization - OpenSSL should not expose OpenSSL cipher names externally Result: Common initialization and OpenSSL doesn't expose custom cipher names. --- .../io/netty/handler/ssl/JdkSslContext.java | 57 +++++-------------- .../java/io/netty/handler/ssl/OpenSsl.java | 30 ++++++++-- .../ssl/ReferenceCountedOpenSslContext.java | 25 ++------ .../java/io/netty/handler/ssl/SslUtils.java | 46 +++++++++++++++ 4 files changed, 90 insertions(+), 68 deletions(-) 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 0ad6639e93..75d4b9da1e 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java @@ -20,11 +20,6 @@ import io.netty.buffer.ByteBufAllocator; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; -import javax.crypto.NoSuchPaddingException; -import javax.net.ssl.KeyManagerFactory; -import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLSessionContext; import java.io.File; import java.io.IOException; import java.security.InvalidAlgorithmParameterException; @@ -42,7 +37,16 @@ import java.util.HashSet; import java.util.List; import java.util.Set; -import static io.netty.util.internal.ObjectUtil.*; +import javax.crypto.NoSuchPaddingException; +import javax.net.ssl.KeyManagerFactory; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLSessionContext; + +import static io.netty.handler.ssl.SslUtils.DEFAULT_CIPHER_SUITES; +import static io.netty.handler.ssl.SslUtils.addIfSupported; +import static io.netty.handler.ssl.SslUtils.useFallbackCiphersIfDefaultIsEmpty; +import static io.netty.util.internal.ObjectUtil.checkNotNull; /** * An {@link SslContext} which uses JDK's SSL/TLS implementation. @@ -52,9 +56,9 @@ public class JdkSslContext extends SslContext { private static final InternalLogger logger = InternalLoggerFactory.getInstance(JdkSslContext.class); static final String PROTOCOL = "TLS"; - static final String[] DEFAULT_PROTOCOLS; - static final List DEFAULT_CIPHERS; - static final Set SUPPORTED_CIPHERS; + private static final String[] DEFAULT_PROTOCOLS; + private static final List DEFAULT_CIPHERS; + private static final Set SUPPORTED_CIPHERS; static { SSLContext context; @@ -105,31 +109,8 @@ public class JdkSslContext extends SslContext { } } List ciphers = new ArrayList(); - addIfSupported( - SUPPORTED_CIPHERS, ciphers, - // XXX: Make sure to sync this list with OpenSslEngineFactory. - // 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", - // AES256 requires JCE unlimited strength jurisdiction policy files. - "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", - // AES256 requires JCE unlimited strength jurisdiction policy files. - "TLS_RSA_WITH_AES_256_CBC_SHA"); - - if (ciphers.isEmpty()) { - // Use the default from JDK as fallback. - for (String cipher : engine.getEnabledCipherSuites()) { - if (cipher.contains("_RC4_")) { - continue; - } - ciphers.add(cipher); - } - } + addIfSupported(SUPPORTED_CIPHERS, ciphers, DEFAULT_CIPHER_SUITES); + useFallbackCiphersIfDefaultIsEmpty(ciphers, engine.getEnabledCipherSuites()); DEFAULT_CIPHERS = Collections.unmodifiableList(ciphers); if (logger.isDebugEnabled()) { @@ -138,14 +119,6 @@ public class JdkSslContext extends SslContext { } } - private static void addIfSupported(Set supported, List enabled, String... names) { - for (String n: names) { - if (supported.contains(n)) { - enabled.add(n); - } - } - } - private final String[] protocols; private final String[] cipherSuites; private final List unmodifiableCipherSuites; 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 d2f091ac72..68d12b3c41 100644 --- a/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java +++ b/handler/src/main/java/io/netty/handler/ssl/OpenSsl.java @@ -18,24 +18,31 @@ package io.netty.handler.ssl; import io.netty.buffer.ByteBuf; import io.netty.handler.ssl.util.SelfSignedCertificate; +import io.netty.internal.tcnative.Buffer; +import io.netty.internal.tcnative.Library; +import io.netty.internal.tcnative.SSL; +import io.netty.internal.tcnative.SSLContext; import io.netty.util.ReferenceCountUtil; import io.netty.util.ReferenceCounted; import io.netty.util.internal.NativeLibraryLoader; import io.netty.util.internal.SystemPropertyUtil; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; -import io.netty.internal.tcnative.Buffer; -import io.netty.internal.tcnative.Library; -import io.netty.internal.tcnative.SSL; -import io.netty.internal.tcnative.SSLContext; import java.security.AccessController; import java.security.PrivilegedAction; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; import java.util.LinkedHashSet; +import java.util.List; import java.util.Locale; import java.util.Set; +import static io.netty.handler.ssl.SslUtils.DEFAULT_CIPHER_SUITES; +import static io.netty.handler.ssl.SslUtils.addIfSupported; +import static io.netty.handler.ssl.SslUtils.useFallbackCiphersIfDefaultIsEmpty; + /** * Tells if {@code netty-tcnative} and its OpenSSL support * are available. @@ -47,6 +54,7 @@ public final class OpenSsl { private static final String UNKNOWN = "unknown"; private static final Throwable UNAVAILABILITY_CAUSE; + static final List DEFAULT_CIPHERS; static final Set AVAILABLE_CIPHER_SUITES; private static final Set AVAILABLE_OPENSSL_CIPHER_SUITES; private static final Set AVAILABLE_JAVA_CIPHER_SUITES; @@ -115,6 +123,7 @@ public final class OpenSsl { if (cause == null) { logger.debug("netty-tcnative using native library: {}", SSL.versionString()); + final List defaultCiphers = new ArrayList(); final Set availableOpenSslCipherSuites = new LinkedHashSet(128); boolean supportsKeyManagerFactory = false; boolean useKeyManagerFactory = false; @@ -134,6 +143,7 @@ public final class OpenSsl { } availableOpenSslCipherSuites.add(c); } + try { SSL.setHostNameValidation(ssl, 0, "netty.io"); supportsHostNameValidation = true; @@ -175,7 +185,6 @@ public final class OpenSsl { logger.warn("Failed to get the list of available OpenSSL cipher suites.", e); } AVAILABLE_OPENSSL_CIPHER_SUITES = Collections.unmodifiableSet(availableOpenSslCipherSuites); - final Set availableJavaCipherSuites = new LinkedHashSet( AVAILABLE_OPENSSL_CIPHER_SUITES.size() * 2); for (String cipher: AVAILABLE_OPENSSL_CIPHER_SUITES) { @@ -183,6 +192,11 @@ public final class OpenSsl { availableJavaCipherSuites.add(CipherSuiteConverter.toJava(cipher, "TLS")); availableJavaCipherSuites.add(CipherSuiteConverter.toJava(cipher, "SSL")); } + + useFallbackCiphersIfDefaultIsEmpty(defaultCiphers, availableJavaCipherSuites); + DEFAULT_CIPHERS = Collections.unmodifiableList(defaultCiphers); + addIfSupported(availableJavaCipherSuites, defaultCiphers, DEFAULT_CIPHER_SUITES); + AVAILABLE_JAVA_CIPHER_SUITES = Collections.unmodifiableSet(availableJavaCipherSuites); final Set availableCipherSuites = new LinkedHashSet( @@ -216,7 +230,13 @@ public final class OpenSsl { SUPPORTED_PROTOCOLS_SET = Collections.unmodifiableSet(protocols); SUPPORTS_OCSP = doesSupportOcsp(); + + if (logger.isDebugEnabled()) { + logger.debug("Supported protocols (OpenSSL): {} ", Arrays.asList(SUPPORTED_PROTOCOLS_SET)); + logger.debug("Default cipher suites (OpenSSL): {}", DEFAULT_CIPHERS); + } } else { + DEFAULT_CIPHERS = Collections.emptyList(); AVAILABLE_OPENSSL_CIPHER_SUITES = Collections.emptySet(); AVAILABLE_JAVA_CIPHER_SUITES = Collections.emptySet(); AVAILABLE_CIPHER_SUITES = Collections.emptySet(); 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 35839c9d06..4665e675c3 100644 --- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java @@ -45,10 +45,10 @@ import java.util.Arrays; import java.util.Collections; import java.util.List; import java.util.Map; - import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; + import javax.net.ssl.KeyManager; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; @@ -59,6 +59,8 @@ import javax.net.ssl.X509ExtendedTrustManager; import javax.net.ssl.X509KeyManager; import javax.net.ssl.X509TrustManager; +import static io.netty.handler.ssl.OpenSsl.DEFAULT_CIPHERS; +import static io.netty.handler.ssl.OpenSsl.availableJavaCipherSuites; import static io.netty.util.internal.ObjectUtil.checkNotNull; import static io.netty.util.internal.ObjectUtil.checkPositiveOrZero; @@ -100,7 +102,6 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen } }); - private static final List DEFAULT_CIPHERS; private static final Integer DH_KEY_LENGTH; private static final ResourceLeakDetector leakDetector = ResourceLeakDetectorFactory.instance().newResourceLeakDetector(ReferenceCountedOpenSslContext.class); @@ -176,24 +177,6 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen }; static { - List ciphers = new ArrayList(); - // XXX: Make sure to sync this list with JdkSslEngineFactory. - Collections.addAll( - ciphers, - "ECDHE-ECDSA-AES256-GCM-SHA384", - "ECDHE-ECDSA-AES128-GCM-SHA256", - "ECDHE-RSA-AES128-GCM-SHA256", - "ECDHE-RSA-AES128-SHA", - "ECDHE-RSA-AES256-SHA", - "AES128-GCM-SHA256", - "AES128-SHA", - "AES256-SHA"); - DEFAULT_CIPHERS = Collections.unmodifiableList(ciphers); - - if (logger.isDebugEnabled()) { - logger.debug("Default cipher suite (OpenSSL): " + ciphers); - } - Integer dhLen = null; try { @@ -271,7 +254,7 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen } unmodifiableCiphers = Arrays.asList(checkNotNull(cipherFilter, "cipherFilter").filterCipherSuites( - convertedCiphers, DEFAULT_CIPHERS, OpenSsl.availableOpenSslCipherSuites())); + convertedCiphers, DEFAULT_CIPHERS, availableJavaCipherSuites())); this.apn = checkNotNull(apn, "apn"); 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 bdf6567248..e4c6ac1d0d 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslUtils.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslUtils.java @@ -24,8 +24,13 @@ import io.netty.handler.codec.base64.Base64Dialect; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.util.List; +import java.util.Set; + import javax.net.ssl.SSLHandshakeException; +import static java.util.Arrays.asList; + /** * Constants for SSL packets. */ @@ -71,6 +76,47 @@ final class SslUtils { */ static final int NOT_ENCRYPTED = -2; + static final String[] DEFAULT_CIPHER_SUITES = new String[] { + // 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", + // AES256 requires JCE unlimited strength jurisdiction policy files. + "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", + // AES256 requires JCE unlimited strength jurisdiction policy files. + "TLS_RSA_WITH_AES_256_CBC_SHA" + }; + + /** + * Add elements from {@code names} into {@code enabled} if they are in {@code supported}. + */ + static void addIfSupported(Set supported, List enabled, String... names) { + for (String n: names) { + if (supported.contains(n)) { + enabled.add(n); + } + } + } + + static void useFallbackCiphersIfDefaultIsEmpty(List defaultCiphers, Iterable fallbackCiphers) { + if (defaultCiphers.isEmpty()) { + for (String cipher : fallbackCiphers) { + if (cipher.startsWith("SSL_") || cipher.contains("_RC4_")) { + continue; + } + defaultCiphers.add(cipher); + } + } + } + + static void useFallbackCiphersIfDefaultIsEmpty(List defaultCiphers, String... fallbackCiphers) { + useFallbackCiphersIfDefaultIsEmpty(defaultCiphers, asList(fallbackCiphers)); + } + /** * Converts the given exception to a {@link SSLHandshakeException}, if it isn't already. */