From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Mon, 6 May 2019 12:34:18 +0000 (+0100)
Subject: TLS: increase resumption ticket lifetime to 2 hours
X-Git-Tag: exim-4.93-RC0~201
X-Git-Url: https://git.exim.org/exim.git/commitdiff_plain/dea4b5684c694c41105215bdb25f8e91b7c35c5d

TLS: increase resumption ticket lifetime to 2 hours
---

diff --git a/doc/doc-txt/experimental-spec.txt b/doc/doc-txt/experimental-spec.txt
index f304cf455..0f749c6cf 100644
--- a/doc/doc-txt/experimental-spec.txt
+++ b/doc/doc-txt/experimental-spec.txt
@@ -984,7 +984,10 @@ Security aspects:
  vulnarability surface.  An attacker able to decrypt it would have access
  all connections using the resumed session.
  The session ticket encryption key is not committed to storage by the server
- and is rotated regularly.  Tickets have limited lifetime.
+ and is rotated regularly (OpenSSL: 1hr, and one previous key is used for
+ overlap; GnuTLS 6hr but does not specify any overlap).
+ Tickets have limited lifetime (2hr, and new ones issued after 1hr under
+ OpenSSL.  GnuTLS 2hr, appears to not do overlap).
 
  There is a question-mark over the security of the Diffie-Helman parameters
  used for session negotiation. TBD.  q-value; cf bug 1895
diff --git a/src/src/tls-gnu.c b/src/src/tls-gnu.c
index 085f6b840..df07c536c 100644
--- a/src/src/tls-gnu.c
+++ b/src/src/tls-gnu.c
@@ -215,7 +215,7 @@ don't want to repeat this. */
 
 static gnutls_dh_params_t dh_server_params = NULL;
 
-static int ssl_session_timeout = 3600;	/* One hour */
+static int ssl_session_timeout = 7200;	/* Two hours */
 
 static const uschar * const exim_default_gnutls_priority = US"NORMAL";
 
@@ -2457,7 +2457,9 @@ if (verify_check_given_host(CUSS &ob->tls_resumption_hosts, host) == OK)
   tlsp->resumption |= RESUME_CLIENT_REQUESTED;
   if ((dbm_file = dbfn_open(US"tls", O_RDONLY, &dbblock, FALSE, FALSE)))
     {
-    /* key for the db is the IP */
+    /* Key for the db is the IP.  We'd like to filter the retrieved session
+    for ticket advisory expiry, but 3.6.1 seems to give no access to that */
+
     if ((dt = dbfn_read_with_length(dbm_file, host->address, &len)))
       if (!(rc = gnutls_session_set_data(session,
 		    CUS dt->session, (size_t)len - sizeof(dbdata_tls_session))))
diff --git a/src/src/tls-openssl.c b/src/src/tls-openssl.c
index df884355e..3092dce2e 100644
--- a/src/src/tls-openssl.c
+++ b/src/src/tls-openssl.c
@@ -315,7 +315,7 @@ static SSL_CTX *server_sni = NULL;
 
 static char ssl_errstring[256];
 
-static int  ssl_session_timeout = 3600;
+static int  ssl_session_timeout = 7200;		/* Two hours */
 static BOOL client_verify_optional = FALSE;
 static BOOL server_verify_optional = FALSE;
 
@@ -943,6 +943,12 @@ else
   EVP_DecryptInit_ex(ctx, key->aes_cipher, NULL, key->aes_key, iv);
 
   DEBUG(D_tls) debug_printf("ticket usable, STEK expire %ld\n", key->expire - now);
+
+  /* The ticket lifetime and renewal are the same as the STEK lifetime and
+  renewal, which is overenthusiastic.  A factor of, say, 3x longer STEK would
+  be better.  To do that we'd have to encode ticket lifetime in the name as
+  we don't yet see the restored session.  Could check posthandshake for TLS1.3
+  and trigger a new ticket then, but cannot do that for TLS1.2 */
   return key->renew < now ? 2 : 1;
   }
 }
diff --git a/test/scripts/5891-Resume-OpenSSL/5891 b/test/scripts/5891-Resume-OpenSSL/5891
index 116f5cfe9..58631f55e 100644
--- a/test/scripts/5891-Resume-OpenSSL/5891
+++ b/test/scripts/5891-Resume-OpenSSL/5891
@@ -26,7 +26,7 @@ Test message, not requesting resumption.
 ****
 killdaemon
 sleep 1
-sudo rm DIR/spool/db/tls
+sudo rm -f DIR/spool/db/tls
 #
 #
 ### TLS1.3