Add support for boringssl and TLSv1.3 (#8412)

Motivation:

0ddc62cec0 added support for TLSv1.3 when using openssl 1.1.1. Now that BoringSSL chromium-stable branch supports it as well we can also support it with netty-tcnative-boringssl-static.
During this some unit tests failed with BoringSSL which was caused by not correctly handling flush() while the handshake is still in progress.

Modification:

- Upgrade netty-tcnative version which also supports TLSv1.3 when using BoringSSL
- Correctly handle flush() when done while the handshake is still in progress in all cases.

Result:

Easier for people to enable TLSv1.3 when using native SSL impl.
Ensure flush() while handshake is in progress will always be honored.

handler/src/main/java/io/netty/handler/ssl/OpenSsl.java

@@ -66,7 +66,7 @@ public final class OpenSsl {
                                                  "TLS_AES_128_CCM_8_SHA256" + ':' +
                                                  "TLS_AES_128_CCM_SHA256";
     private static final boolean TLSV13_SUPPORTED;
-
+    private static final boolean IS_BORINGSSL;
     static final Set<String> SUPPORTED_PROTOCOLS_SET;
 
     static {
@@ -141,6 +141,8 @@ public final class OpenSsl {
             boolean supportsHostNameValidation = false;
             boolean tlsv13Supported = false;
 
+            IS_BORINGSSL = "BoringSSL".equals(versionString());
+
             try {
                 final long sslCtx = SSLContext.make(SSL.SSL_PROTOCOL_ALL, SSL.SSL_MODE_SERVER);
                 long certBio = 0;
@@ -160,12 +162,19 @@ public final class OpenSsl {
                             // Filter out bad input.
                             if (c == null || c.isEmpty() || availableOpenSslCipherSuites.contains(c) ||
                                 // Filter out TLSv1.3 ciphers if not supported.
-                                !tlsv13Supported && SslUtils.isTLSv13Cipher(c)) {
+                                !tlsv13Supported && isTLSv13Cipher(c)) {
                                 continue;
                             }
                             availableOpenSslCipherSuites.add(c);
                         }
-
+                        if (IS_BORINGSSL) {
+                            // Currently BoringSSL does not include these when calling SSL.getCiphers() even when these
+                            // are supported.
+                            Collections.addAll(availableOpenSslCipherSuites,
+                                               "TLS_AES_128_GCM_SHA256",
+                                               "TLS_AES_256_GCM_SHA384" ,
+                                               "TLS_CHACHA20_POLY1305_SHA256");
+                        }
                         try {
                             SSL.setHostNameValidation(ssl, 0, "netty.io");
                             supportsHostNameValidation = true;
@@ -240,7 +249,7 @@ public final class OpenSsl {
                     AVAILABLE_OPENSSL_CIPHER_SUITES.size() * 2);
             for (String cipher: AVAILABLE_OPENSSL_CIPHER_SUITES) {
                 // Included converted but also openssl cipher name
-                if (!SslUtils.isTLSv13Cipher(cipher)) {
+                if (!isTLSv13Cipher(cipher)) {
                     availableJavaCipherSuites.add(CipherSuiteConverter.toJava(cipher, "TLS"));
                     availableJavaCipherSuites.add(CipherSuiteConverter.toJava(cipher, "SSL"));
                 } else {
@@ -312,6 +321,7 @@ public final class OpenSsl {
             SUPPORTED_PROTOCOLS_SET = Collections.emptySet();
             SUPPORTS_OCSP = false;
             TLSV13_SUPPORTED = false;
+            IS_BORINGSSL = false;
         }
     }
 
@@ -510,4 +520,8 @@ public final class OpenSsl {
     static boolean isTlsv13Supported() {
         return TLSV13_SUPPORTED;
     }
+
+    static boolean isBoringSSL() {
+        return IS_BORINGSSL;
+    }
 }

handler/src/main/java/io/netty/handler/ssl/SslHandler.java

@@ -1385,13 +1385,7 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                             wrapLater = true;
                             continue;
                         }
-                        if (flushedBeforeHandshake) {
+
-                            // We need to call wrap(...) in case there was a flush done before the handshake completed.
-                            //
-                            // See https://github.com/netty/netty/pull/2437
-                            flushedBeforeHandshake = false;
-                            wrapLater = true;
-                        }
                         // If we are not handshaking and there is no more data to unwrap then the next call to unwrap
                         // will not produce any data. We can avoid the potentially costly unwrap operation and break
                         // out of the loop.
@@ -1414,6 +1408,15 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH
                 }
             }
 
+            if (flushedBeforeHandshake && handshakePromise.isDone()) {
+                // We need to call wrap(...) in case there was a flush done before the handshake completed to ensure
+                // we do not stale.
+                //
+                // See https://github.com/netty/netty/pull/2437
+                flushedBeforeHandshake = false;
+                wrapLater = true;
+            }
+
             if (wrapLater) {
                 wrap(ctx, true);
             }

handler/src/test/java/io/netty/handler/ssl/OpenSslTestUtils.java

@@ -24,8 +24,4 @@ final class OpenSslTestUtils {
     static void checkShouldUseKeyManagerFactory() {
         assumeTrue(OpenSsl.supportsKeyManagerFactory() && OpenSsl.useKeyManagerFactory());
     }
-
-    static boolean isBoringSSL() {
-        return "BoringSSL".equals(OpenSsl.versionString());
-    }
 }

handler/src/test/java/io/netty/handler/ssl/SSLEngineTest.java

@@ -1087,7 +1087,7 @@ public abstract class SSLEngineTest {
                                                MessageReceiver receiver) throws Exception {
         List<ByteBuf> dataCapture = null;
         try {
-            sendChannel.writeAndFlush(message);
+            assertTrue(sendChannel.writeAndFlush(message).await(5, TimeUnit.SECONDS));
             receiverLatch.await(5, TimeUnit.SECONDS);
             message.resetReaderIndex();
             ArgumentCaptor<ByteBuf> captor = ArgumentCaptor.forClass(ByteBuf.class);

handler/src/test/java/io/netty/handler/ssl/SniClientJava8TestUtil.java

@@ -182,7 +182,7 @@ final class SniClientJava8TestUtil {
             if (clientSide) {
                 Assert.assertEquals(0, extendedSSLSession.getPeerSupportedSignatureAlgorithms().length);
             } else {
-                if (session instanceof OpenSslSession && OpenSslTestUtils.isBoringSSL()) {
+                if (session instanceof OpenSslSession && OpenSsl.isBoringSSL()) {
                     // BoringSSL does not support SSL_get_sigalgs(...)
                     // https://boringssl.googlesource.com/boringssl/+/ba16a1e405c617f4179bd780ad15522fb25b0a65%5E%21/
                     Assert.assertEquals(0, extendedSSLSession.getPeerSupportedSignatureAlgorithms().length);

handler/src/test/java/io/netty/handler/ssl/SslErrorTest.java

@@ -205,7 +205,7 @@ public class SslErrorTest {
                                                 verifyException(unwrappedCause, "expired", promise);
                                             } else if (reason == CertPathValidatorException.BasicReason.NOT_YET_VALID) {
                                                 // BoringSSL uses "expired" in this case while others use "bad"
-                                                if (OpenSslTestUtils.isBoringSSL()) {
+                                                if (OpenSsl.isBoringSSL()) {
                                                     verifyException(unwrappedCause, "expired", promise);
                                                 } else {
                                                     verifyException(unwrappedCause, "bad", promise);
@@ -217,7 +217,7 @@ public class SslErrorTest {
                                             verifyException(unwrappedCause, "expired", promise);
                                         } else if (exception instanceof CertificateNotYetValidException) {
                                             // BoringSSL uses "expired" in this case while others use "bad"
-                                            if (OpenSslTestUtils.isBoringSSL()) {
+                                            if (OpenSsl.isBoringSSL()) {
                                                 verifyException(unwrappedCause, "expired", promise);
                                             } else {
                                                 verifyException(unwrappedCause, "bad", promise);

pom.xml

@@ -241,7 +241,7 @@
     <!-- Fedora-"like" systems. This is currently only used for the netty-tcnative dependency -->
     <os.detection.classifierWithLikes>fedora</os.detection.classifierWithLikes>
     <tcnative.artifactId>netty-tcnative</tcnative.artifactId>
-    <tcnative.version>2.0.18.Final</tcnative.version>
+    <tcnative.version>2.0.19.Final</tcnative.version>
     <tcnative.classifier>${os.detected.classifier}</tcnative.classifier>
     <conscrypt.groupId>org.conscrypt</conscrypt.groupId>
     <conscrypt.artifactId>conscrypt-openjdk-uber</conscrypt.artifactId>