From 4d09c5ff98ebd4d2b0ff30f382cc655a463b52ef Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 29 Sep 2020 16:59:11 +0200 Subject: [PATCH] Respect the Provider when detecting if TLSv1.3 is used by default / supported (#10621) Motivation: We need to take the Provider into account as well when trying to detect if TLSv1.3 is used by default / supported Modifications: - Change utility method to respect provider as well - Change testcode Result: Less error-prone tests --- .../io/netty/handler/ssl/SslProvider.java | 27 ++++-- .../java/io/netty/handler/ssl/SslUtils.java | 91 ++++++++++++------- .../io/netty/handler/ssl/SSLEngineTest.java | 13 +-- 3 files changed, 79 insertions(+), 52 deletions(-) diff --git a/handler/src/main/java/io/netty/handler/ssl/SslProvider.java b/handler/src/main/java/io/netty/handler/ssl/SslProvider.java index 43f7b32df5..5f85bb0103 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslProvider.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslProvider.java @@ -19,6 +19,8 @@ package io.netty.handler.ssl; import io.netty.util.ReferenceCounted; import io.netty.util.internal.UnstableApi; +import java.security.Provider; + /** * An enumeration of SSL/TLS protocol providers. */ @@ -41,6 +43,7 @@ public enum SslProvider { * Returns {@code true} if the specified {@link SslProvider} supports * TLS ALPN Extension, {@code false} otherwise. */ + @SuppressWarnings("deprecation") public static boolean isAlpnSupported(final SslProvider provider) { switch (provider) { case JDK: @@ -57,15 +60,23 @@ public enum SslProvider { * Returns {@code true} if the specified {@link SslProvider} supports * TLS 1.3, {@code false} otherwise. */ - public static boolean isTlsv13Supported(final SslProvider provider) { - switch (provider) { + public static boolean isTlsv13Supported(final SslProvider sslProvider) { + return isTlsv13Supported(sslProvider, null); + } + + /** + * Returns {@code true} if the specified {@link SslProvider} supports + * TLS 1.3, {@code false} otherwise. + */ + public static boolean isTlsv13Supported(final SslProvider sslProvider, Provider provider) { + switch (sslProvider) { case JDK: - return SslUtils.isTLSv13SupportedByJDK(); + return SslUtils.isTLSv13SupportedByJDK(provider); case OPENSSL: case OPENSSL_REFCNT: return OpenSsl.isTlsv13Supported(); default: - throw new Error("Unknown SslProvider: " + provider); + throw new Error("Unknown SslProvider: " + sslProvider); } } @@ -73,15 +84,15 @@ public enum SslProvider { * Returns {@code true} if the specified {@link SslProvider} enables * TLS 1.3 by default, {@code false} otherwise. */ - static boolean isTlsv13EnabledByDefault(final SslProvider provider) { - switch (provider) { + static boolean isTlsv13EnabledByDefault(final SslProvider sslProvider, Provider provider) { + switch (sslProvider) { case JDK: - return SslUtils.isTLSv13EnabledByJDK(); + return SslUtils.isTLSv13EnabledByJDK(provider); case OPENSSL: case OPENSSL_REFCNT: return OpenSsl.isTlsv13Supported(); default: - throw new Error("Unknown SslProvider: " + provider); + throw new Error("Unknown SslProvider: " + sslProvider); } } } 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 ccc6ee7602..ba1585266d 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslUtils.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslUtils.java @@ -28,6 +28,9 @@ import io.netty.util.internal.logging.InternalLoggerFactory; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.security.KeyManagementException; +import java.security.NoSuchAlgorithmException; +import java.security.Provider; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashSet; @@ -110,35 +113,8 @@ final class SslUtils { private static final boolean TLSV1_3_JDK_DEFAULT_ENABLED; static { - boolean tlsv13Supported = false; - boolean tlsv13Enabled = false; - - Throwable cause = null; - try { - SSLContext context = SSLContext.getInstance("TLS"); - context.init(null, new TrustManager[0], null); - for (String supported: context.getSupportedSSLParameters().getProtocols()) { - if (PROTOCOL_TLS_V1_3.equals(supported)) { - tlsv13Supported = true; - break; - } - } - for (String enabled: context.getDefaultSSLParameters().getProtocols()) { - if (PROTOCOL_TLS_V1_3.equals(enabled)) { - tlsv13Enabled = true; - break; - } - } - } catch (Throwable error) { - cause = error; - } - if (cause == null) { - logger.debug("JDK SSLEngine supports TLSv1.3: {}", tlsv13Supported); - } else { - logger.debug("Unable to detect if JDK SSLEngine supports TLSv1.3, assuming no", cause); - } - TLSV1_3_JDK_SUPPORTED = tlsv13Supported; - TLSV1_3_JDK_DEFAULT_ENABLED = tlsv13Enabled; + TLSV1_3_JDK_SUPPORTED = isTLSv13SupportedByJDK0(null); + TLSV1_3_JDK_DEFAULT_ENABLED = isTLSv13EnabledByJDK0(null); if (TLSV1_3_JDK_SUPPORTED) { DEFAULT_TLSV13_CIPHER_SUITES = TLSV13_CIPHER_SUITES; } else { @@ -168,15 +144,64 @@ final class SslUtils { /** * Returns {@code true} if the JDK itself supports TLSv1.3, {@code false} otherwise. */ - static boolean isTLSv13SupportedByJDK() { - return TLSV1_3_JDK_SUPPORTED; + static boolean isTLSv13SupportedByJDK(Provider provider) { + if (provider == null) { + return TLSV1_3_JDK_SUPPORTED; + } + return isTLSv13SupportedByJDK0(provider); + } + + private static boolean isTLSv13SupportedByJDK0(Provider provider) { + try { + return arrayContains(newInitContext(provider) + .getSupportedSSLParameters().getProtocols(), PROTOCOL_TLS_V1_3); + } catch (Throwable cause) { + logger.debug("Unable to detect if JDK SSLEngine with provider {} supports TLSv1.3, assuming no", + provider, cause); + return false; + } } /** * Returns {@code true} if the JDK itself supports TLSv1.3 and enabled it by default, {@code false} otherwise. */ - static boolean isTLSv13EnabledByJDK() { - return TLSV1_3_JDK_DEFAULT_ENABLED; + static boolean isTLSv13EnabledByJDK(Provider provider) { + if (provider == null) { + return TLSV1_3_JDK_DEFAULT_ENABLED; + } + return isTLSv13EnabledByJDK0(provider); + } + + private static boolean isTLSv13EnabledByJDK0(Provider provider) { + try { + return arrayContains(newInitContext(provider) + .getDefaultSSLParameters().getProtocols(), PROTOCOL_TLS_V1_3); + } catch (Throwable cause) { + logger.debug("Unable to detect if JDK SSLEngine with provider {} enables TLSv1.3 by default," + + " assuming no", provider, cause); + return false; + } + } + + private static SSLContext newInitContext(Provider provider) + throws NoSuchAlgorithmException, KeyManagementException { + final SSLContext context; + if (provider == null) { + context = SSLContext.getInstance("TLS"); + } else { + context = SSLContext.getInstance("TLS", provider); + } + context.init(null, new TrustManager[0], null); + return context; + } + + static boolean arrayContains(String[] array, String value) { + for (String v: array) { + if (value.equals(v)) { + return true; + } + } + return false; } /** 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 7d62a85893..48d04eac98 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java @@ -3609,21 +3609,12 @@ public abstract class SSLEngineTest { ssc.delete(); } - assertEquals(SslProvider.isTlsv13EnabledByDefault(sslClientProvider()), + assertEquals(SslProvider.isTlsv13EnabledByDefault(sslClientProvider(), clientSslContextProvider()), arrayContains(clientProtocols, PROTOCOL_TLS_V1_3)); - assertEquals(SslProvider.isTlsv13EnabledByDefault(sslServerProvider()), + assertEquals(SslProvider.isTlsv13EnabledByDefault(sslServerProvider(), serverSslContextProvider()), arrayContains(serverProtocols, PROTOCOL_TLS_V1_3)); } - private static boolean arrayContains(String[] array, String value) { - for (String v: array) { - if (value.equals(v)) { - return true; - } - } - return false; - } - protected SSLEngine wrapEngine(SSLEngine engine) { return engine; }