Validate the ticket age for resumed sessions

If the ticket age calcualtions do not check out then we must not accept
early data (it could be a replay).

Reviewed-by: Rich Salz <rsalz@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/2737)

3 files changed, 50 insertions, 8 deletions

diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 2151d631e8..52515cb589 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -638,6 +638,14 @@ typedef enum {
  */
 # define EARLY_DATA_CIPHERTEXT_OVERHEAD ((6 * (EVP_GCM_TLS_TAG_LEN + 1)) + 2)
 
+/*
+ * The allowance we have between the client's calculated ticket age and our own.
+ * We allow for 10 seconds (units are in ms). If a ticket is presented and the
+ * client's age calculation is different by more than this than our own then we
+ * do not allow that ticket for early_data.
+ */
+# define TICKET_AGE_ALLOWANCE   (10 * 1000)
+
 #define MAX_COMPRESSIONS_SIZE   255
 
 struct ssl_comp_st {
@@ -1197,6 +1205,8 @@ struct ssl_st {
 
         /* Are we expecting to receive early data? */
         int early_data;
+        /* Is the session suitable for early data? */
+        int early_data_ok;
     } ext;
 
     /* Parsed form of the ClientHello, kept around across early_cb calls. */
diff --git a/ssl/statem/extensions_srvr.c b/ssl/statem/extensions_srvr.c
index 57f38738d8..99ce6ce80d 100644
--- a/ssl/statem/extensions_srvr.c
+++ b/ssl/statem/extensions_srvr.c
@@ -171,7 +171,8 @@ int tls_parse_ctos_early_data(SSL *s, PACKET *pkt, unsigned int context,
     }
 
     if (s->max_early_data == 0 || !s->hit || s->session->ext.tick_identity != 0
-            || s->early_data_state != SSL_EARLY_DATA_ACCEPTING) {
+            || s->early_data_state != SSL_EARLY_DATA_ACCEPTING
+            || !s->ext.early_data_ok) {
         s->ext.early_data = SSL_EARLY_DATA_REJECTED;
     } else {
         s->ext.early_data = SSL_EARLY_DATA_ACCEPTED;
@@ -693,6 +694,7 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     SSL_SESSION *sess = NULL;
     unsigned int id, i;
     const EVP_MD *md = NULL;
+    uint32_t ticket_age, now, agesec, agems;
 
     /*
      * If we have no PSK kex mode that we recognise then we can't resume so
@@ -709,16 +711,16 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
 
     for (id = 0; PACKET_remaining(&identities) != 0; id++) {
         PACKET identity;
-        unsigned long ticket_age;
+        unsigned long ticket_agel;
         int ret;
 
         if (!PACKET_get_length_prefixed_2(&identities, &identity)
-                || !PACKET_get_net_4(&identities, &ticket_age)) {
+                || !PACKET_get_net_4(&identities, &ticket_agel)) {
             *al = SSL_AD_DECODE_ERROR;
             return 0;
         }
 
-        /* TODO(TLS1.3): Should we validate the ticket age? */
+        ticket_age = (uint32_t)ticket_agel;
 
         ret = tls_decrypt_ticket(s, PACKET_data(&identity),
                                  PACKET_remaining(&identity), NULL, 0, &sess);
@@ -777,6 +779,32 @@ int tls_parse_ctos_psk(SSL *s, PACKET *pkt, unsigned int context, X509 *x,
     }
 
     sess->ext.tick_identity = id;
+
+    now = (uint32_t)time(NULL);
+    agesec = now - (uint32_t)sess->time;
+    agems = agesec * (uint32_t)1000;
+    ticket_age -= sess->ext.tick_age_add;
+
+
+    /*
+     * For simplicity we do our age calculations in seconds. If the client does
+     * it in ms then it could appear that their ticket age is longer than ours
+     * (our ticket age calculation should always be slightly longer than the
+     * client's due to the network latency). Therefore we add 1000ms to our age
+     * calculation to adjust for rounding errors.
+     */
+    if (sess->timeout >= agesec
+            && agems / (uint32_t)1000 == agesec
+            && ticket_age <= agems + 1000
+            && ticket_age + TICKET_AGE_ALLOWANCE >= agems + 1000) {
+        /*
+         * Ticket age is within tolerance and not expired. We allow it for early
+         * data
+         */
+        s->ext.early_data_ok = 1;
+    }
+
+
     SSL_SESSION_free(s->session);
     s->session = sess;
     return 1;
diff --git a/ssl/statem/statem_srvr.c b/ssl/statem/statem_srvr.c
index e6a84dfa7b..56db987ee4 100644
--- a/ssl/statem/statem_srvr.c
+++ b/ssl/statem/statem_srvr.c
@@ -3400,6 +3400,7 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
         if (RAND_bytes(age_add_u.age_add_c, sizeof(age_add_u)) <= 0)
             goto err;
         s->session->ext.tick_age_add = age_add_u.age_add;
+        s->session->time = (long)time(NULL);
     }
 
     /* get session encoding length */
@@ -3494,11 +3495,14 @@ int tls_construct_new_session_ticket(SSL *s, WPACKET *pkt)
     }
 
     /*
-     * Ticket lifetime hint (advisory only): We leave this unspecified
-     * for resumed session (for simplicity), and guess that tickets for
-     * new sessions will live as long as their sessions.
+     * Ticket lifetime hint: For TLSv1.2 this is advisory only and we leave this
+     * unspecified for resumed session (for simplicity).
+     * In TLSv1.3 we reset the "time" field above, and always specify the
+     * timeout.
      */
-    if (!WPACKET_put_bytes_u32(pkt, s->hit ? 0 : s->session->timeout)
+    if (!WPACKET_put_bytes_u32(pkt,
+                               (s->hit && !SSL_IS_TLS13(s))
+                               ? 0 : s->session->timeout)
             || (SSL_IS_TLS13(s)
                 && !WPACKET_put_bytes_u32(pkt, age_add_u.age_add))
                /* Now the actual ticket data */