From 3f3d309a28f66f463bf7c6ade14e7908019b8aae Mon Sep 17 00:00:00 2001 From: Scott Mitchell Date: Thu, 1 Feb 2018 09:34:44 -0800 Subject: [PATCH] JdkSslContext supported cipher suites incorrect Motivation: JdkSslContext builds the list of supported cipher suites, but assumes that ciphers prefixed with SSL_ and TLS_ will be interchangeable. However this is not the case and only applies to a small subset of ciphers. This results in the JdkSslContext attempting to use unsupported ciphers. Modifications: - When building the list of ciphers in JdkSslContext we should first check if the engine supports the TLS_ prefix cipher. Result: Fixes https://github.com/netty/netty/issues/7673 --- .../io/netty/handler/ssl/JdkSslContext.java | 8 ++++- .../io/netty/handler/ssl/SslContextTest.java | 31 ++++++++++++++++++- 2 files changed, 37 insertions(+), 2 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 e027036146..41fd9c6f6a 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkSslContext.java @@ -105,7 +105,13 @@ public class JdkSslContext extends SslContext { //[3] https://www.ibm.com/developerworks/community/forums/html/topic?id=9b5a56a9-fa46-4031-b33b-df91e28d77c2 //[4] https://www.ibm.com/developerworks/rfe/execute?use_case=viewRfe&CR_ID=71770 if (supportedCipher.startsWith("SSL_")) { - SUPPORTED_CIPHERS.add("TLS_" + supportedCipher.substring("SSL_".length())); + final String tlsPrefixedCipherName = "TLS_" + supportedCipher.substring("SSL_".length()); + try { + engine.setEnabledCipherSuites(new String[]{tlsPrefixedCipherName}); + SUPPORTED_CIPHERS.add(tlsPrefixedCipherName); + } catch (IllegalArgumentException ignored) { + // The cipher is not supported ... move on to the next cipher. + } } } List ciphers = new ArrayList(); diff --git a/handler/src/test/java/io/netty/handler/ssl/SslContextTest.java b/handler/src/test/java/io/netty/handler/ssl/SslContextTest.java index 76526a5b25..1c0032885a 100644 --- a/handler/src/test/java/io/netty/handler/ssl/SslContextTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/SslContextTest.java @@ -18,12 +18,20 @@ package io.netty.handler.ssl; import org.junit.Assert; import org.junit.Test; -import javax.net.ssl.SSLException; import java.io.File; import java.io.IOException; +import java.security.KeyManagementException; +import java.security.NoSuchAlgorithmException; import java.security.PrivateKey; import java.security.spec.InvalidKeySpecException; +import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLEngine; +import javax.net.ssl.SSLException; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assume.assumeNotNull; + public abstract class SslContextTest { @Test(expected = IOException.class) @@ -85,5 +93,26 @@ public abstract class SslContextTest { newServerContext(crtFile, keyFile, ""); } + @Test + public void testSupportedCiphers() throws KeyManagementException, NoSuchAlgorithmException, SSLException { + SSLContext jdkSslContext = SSLContext.getInstance("TLS"); + jdkSslContext.init(null, null, null); + SSLEngine sslEngine = jdkSslContext.createSSLEngine(); + + String unsupportedCipher = "TLS_DH_anon_WITH_DES_CBC_SHA"; + IllegalArgumentException exception = null; + try { + sslEngine.setEnabledCipherSuites(new String[] {unsupportedCipher}); + } catch (IllegalArgumentException e) { + exception = e; + } + assumeNotNull(exception); + File keyFile = new File(getClass().getResource("test_unencrypted.pem").getFile()); + File crtFile = new File(getClass().getResource("test.crt").getFile()); + + SslContext sslContext = newServerContext(crtFile, keyFile, null); + assertFalse(sslContext.cipherSuites().contains(unsupportedCipher)); + } + protected abstract SslContext newServerContext(File crtFile, File keyFile, String pass) throws SSLException; }