From f30dd0b31bec0114462164f8f39c13f418b8e674 Mon Sep 17 00:00:00 2001
From: Eric Anderson <ejona@google.com>
Date: Thu, 20 Jul 2017 16:38:40 -0700
Subject: [PATCH] Filter user-provided ciphers using RFC cipher names

Motivation:

Previously filterCipherSuites was being passed the OpenSSL-formatted
cipher names. Commit 43ae974 introduced a regression as it swapped to the
RFC/JDK format, except that user-provided ciphers were not converted and
remained in the OpenSSL format.

This mis-match would cause all user-provided to be thrown away, leading
to failure trying to set zero ciphers:
Exception in thread "main" javax.net.ssl.SSLException: failed to set cipher suite: []
	at io.netty.handler.ssl.ReferenceCountedOpenSslContext.<init>(ReferenceCountedOpenSslContext.java:299)
	at io.netty.handler.ssl.OpenSslContext.<init>(OpenSslContext.java:43)
	at io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:347)
	at io.netty.handler.ssl.OpenSslServerContext.<init>(OpenSslServerContext.java:335)
	at io.netty.handler.ssl.SslContext.newServerContextInternal(SslContext.java:421)
	at io.netty.handler.ssl.SslContextBuilder.build(SslContextBuilder.java:441)
Caused by: java.lang.Exception: Unable to configure permitted SSL ciphers (error:100000b1:SSL routines:OPENSSL_internal:NO_CIPHER_MATCH)
	at io.netty.internal.tcnative.SSLContext.setCipherSuite(Native Method)
	at io.netty.handler.ssl.ReferenceCountedOpenSslContext.<init>(ReferenceCountedOpenSslContext.java:295)
	... 7 more

Modifications:

Remove the reformatting of user-provided ciphers, as they are already in
the RFC/JDK format.

Result:

No regression, and the internals stay sane using the RFC/JDK format.
---
 .../ssl/ReferenceCountedOpenSslContext.java   | 19 +-----------
 .../io/netty/handler/ssl/SSLEngineTest.java   | 31 +++++++++++++++++++
 2 files changed, 32 insertions(+), 18 deletions(-)

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 60293c57e2..03e42ae04b 100644
--- a/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java
+++ b/handler/src/main/java/io/netty/handler/ssl/ReferenceCountedOpenSslContext.java
@@ -226,26 +226,9 @@ public abstract class ReferenceCountedOpenSslContext extends SslContext implemen
                     JDK_REJECT_CLIENT_INITIATED_RENEGOTIATION;
         }
         this.keyCertChain = keyCertChain == null ? null : keyCertChain.clone();
-        final List<String> convertedCiphers;
-        if (ciphers == null) {
-            convertedCiphers = null;
-        } else {
-            convertedCiphers = new ArrayList<String>();
-            for (String c : ciphers) {
-                if (c == null) {
-                    break;
-                }
-
-                String converted = CipherSuiteConverter.toOpenSsl(c);
-                if (converted != null) {
-                    c = converted;
-                }
-                convertedCiphers.add(c);
-            }
-        }
 
         unmodifiableCiphers = Arrays.asList(checkNotNull(cipherFilter, "cipherFilter").filterCipherSuites(
-                convertedCiphers, DEFAULT_CIPHERS, availableJavaCipherSuites()));
+                ciphers, DEFAULT_CIPHERS, availableJavaCipherSuites()));
 
         this.apn = checkNotNull(apn, "apn");
 
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 2babb9d66b..da8cc34b2e 100644
--- a/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java
+++ b/handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java
@@ -1709,6 +1709,37 @@ public abstract class SSLEngineTest {
         }
     }
 
+    @Test
+    public void testHandshakeCompletesWithoutFilteringSupportedCipher() throws Exception {
+        SelfSignedCertificate ssc = new SelfSignedCertificate();
+        // Select a mandatory cipher from the TLSv1.2 RFC https://www.ietf.org/rfc/rfc5246.txt so handshakes won't fail
+        // due to no shared/supported cipher.
+        final String sharedCipher = "TLS_RSA_WITH_AES_128_CBC_SHA";
+        clientSslCtx = SslContextBuilder.forClient()
+                .trustManager(InsecureTrustManagerFactory.INSTANCE)
+                .ciphers(Arrays.asList(sharedCipher), SupportedCipherSuiteFilter.INSTANCE)
+                .protocols(PROTOCOL_TLS_V1_2, PROTOCOL_TLS_V1)
+                .sslProvider(sslClientProvider())
+                .build();
+
+        serverSslCtx = SslContextBuilder.forServer(ssc.certificate(), ssc.privateKey())
+                .ciphers(Arrays.asList(sharedCipher), SupportedCipherSuiteFilter.INSTANCE)
+                .protocols(PROTOCOL_TLS_V1_2, PROTOCOL_TLS_V1)
+                .sslProvider(sslServerProvider())
+                .build();
+        SSLEngine clientEngine = null;
+        SSLEngine serverEngine = null;
+        try {
+            clientEngine = clientSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);
+            serverEngine = serverSslCtx.newEngine(UnpooledByteBufAllocator.DEFAULT);
+            handshake(clientEngine, serverEngine);
+        } finally {
+            cleanupClientSslEngine(clientEngine);
+            cleanupServerSslEngine(serverEngine);
+            ssc.delete();
+        }
+    }
+
     @Test
     public void testPacketBufferSizeLimit() throws Exception {
         SelfSignedCertificate cert = new SelfSignedCertificate();