From 6b0655aecb7ff6e0c54d6d6f5097a2919b6c2355 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Wed, 13 May 2020 15:22:03 +0200
Subject: [PATCH] Allow to have the session tickets automatically managed by
 the native library

Motivation:

BoringSSL supports to automatically manage the session tickets to be used and so also rotate them etc. This is often prefered by users as it removed some complexity. We should support to make use of this.

Modifications:

- Allow to have setSessionTickets() called without an argument or an empty array
- Add tests

Result:

Easier usage of session tickets
---
 .../handler/ssl/OpenSslSessionContext.java    |  9 ++++--
 .../io/netty/handler/ssl/SslHandlerTest.java  | 32 +++++++++++++++----
 2 files changed, 32 insertions(+), 9 deletions(-)

diff --git a/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionContext.java b/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionContext.java
index e22ad9ec4e..8f46ad56db 100644
--- a/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionContext.java
+++ b/handler/src/main/java/io/netty/handler/ssl/OpenSslSessionContext.java
@@ -98,7 +98,10 @@ public abstract class OpenSslSessionContext implements SSLSessionContext {
     }
 
     /**
-     * Sets the SSL session ticket keys of this context.
+     * Sets the SSL session ticket keys of this context. Depending on the underlying native library you may omit the
+     * argument or pass an empty array and so let the native library handle the key generation and rotating for you.
+     * If this is supported by the underlying native library should be checked in this case. For example
+     * <a href="https://boringssl.googlesource.com/boringssl/">BoringSSL</a> is known to support this.
      */
     public void setTicketKeys(OpenSslSessionTicketKey... keys) {
         requireNonNull(keys, "keys");
@@ -110,7 +113,9 @@ public abstract class OpenSslSessionContext implements SSLSessionContext {
         writerLock.lock();
         try {
             SSLContext.clearOptions(context.ctx, SSL.SSL_OP_NO_TICKET);
-            SSLContext.setSessionTicketKeys(context.ctx, ticketKeys);
+            if (ticketKeys.length > 0) {
+                SSLContext.setSessionTicketKeys(context.ctx, ticketKeys);
+            }
         } finally {
             writerLock.unlock();
         }
diff --git a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java
index 63b2b8e374..b5baf27266 100644
--- a/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java
+++ b/handler/src/test/java/io/netty/handler/ssl/SslHandlerTest.java
@@ -1092,16 +1092,29 @@ public class SslHandlerTest {
 
     @Test(timeout = 5000L)
     public void testSessionTicketsWithTLSv12() throws Throwable {
-        testSessionTickets(SslUtils.PROTOCOL_TLS_V1_2);
+        testSessionTickets(SslUtils.PROTOCOL_TLS_V1_2, true);
     }
 
     @Test(timeout = 5000L)
     public void testSessionTicketsWithTLSv13() throws Throwable {
         assumeTrue(OpenSsl.isTlsv13Supported());
-        testSessionTickets(SslUtils.PROTOCOL_TLS_V1_3);
+        testSessionTickets(SslUtils.PROTOCOL_TLS_V1_3, true);
     }
 
-    private static void testSessionTickets(String protocol) throws Throwable {
+    @Test(timeout = 5000L)
+    public void testSessionTicketsWithTLSv12AndNoKey() throws Throwable {
+        assumeTrue(OpenSsl.isBoringSSL());
+        testSessionTickets(SslUtils.PROTOCOL_TLS_V1_2, false);
+    }
+
+    @Test(timeout = 5000L)
+    public void testSessionTicketsWithTLSv13AndNoKey() throws Throwable {
+        assumeTrue(OpenSsl.isTlsv13Supported());
+        assumeTrue(OpenSsl.isBoringSSL());
+        testSessionTickets(SslUtils.PROTOCOL_TLS_V1_3, false);
+    }
+
+    private static void testSessionTickets(String protocol, boolean withKey) throws Throwable {
         assumeTrue(OpenSsl.isAvailable());
         final SslContext sslClientCtx = SslContextBuilder.forClient()
                 .trustManager(InsecureTrustManagerFactory.INSTANCE)
@@ -1115,10 +1128,15 @@ public class SslHandlerTest {
                 .protocols(protocol)
                 .build();
 
-        OpenSslSessionTicketKey key = new OpenSslSessionTicketKey(new byte[OpenSslSessionTicketKey.NAME_SIZE],
-                new byte[OpenSslSessionTicketKey.HMAC_KEY_SIZE], new byte[OpenSslSessionTicketKey.AES_KEY_SIZE]);
-        ((OpenSslSessionContext) sslClientCtx.sessionContext()).setTicketKeys(key);
-        ((OpenSslSessionContext) sslServerCtx.sessionContext()).setTicketKeys(key);
+        if (withKey) {
+            OpenSslSessionTicketKey key = new OpenSslSessionTicketKey(new byte[OpenSslSessionTicketKey.NAME_SIZE],
+                    new byte[OpenSslSessionTicketKey.HMAC_KEY_SIZE], new byte[OpenSslSessionTicketKey.AES_KEY_SIZE]);
+            ((OpenSslSessionContext) sslClientCtx.sessionContext()).setTicketKeys(key);
+            ((OpenSslSessionContext) sslServerCtx.sessionContext()).setTicketKeys(key);
+        } else {
+            ((OpenSslSessionContext) sslClientCtx.sessionContext()).setTicketKeys();
+            ((OpenSslSessionContext) sslServerCtx.sessionContext()).setTicketKeys();
+        }
 
         EventLoopGroup group = new MultithreadEventLoopGroup(NioHandler.newFactory());
         Channel sc = null;