fix: allow for clock skew in resumption (#4650)

aws · Jul 25, 2024 · 390d796 · 390d796
1 parent c3a5680
commit 390d796
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 6 deletions.
diff --git a/tests/unit/s2n_resume_test.c b/tests/unit/s2n_resume_test.c
@@ -1253,13 +1253,23 @@ int main(int argc, char **argv)
 
     /* s2n_validate_ticket_age */
     {
-        /* Ticket issue time is in the future */
+        /* Ticket issue time is beyond MAX_ALLOWED_CLOCK_SKEW */
         {
             uint64_t current_time = SECONDS_TO_NANOS(0);
-            uint64_t issue_time = 10;
+            uint64_t issue_time = SECONDS_TO_NANOS(MAX_ALLOWED_CLOCK_SKEW_SEC + 1);
             EXPECT_ERROR_WITH_ERRNO(s2n_validate_ticket_age(current_time, issue_time), S2N_ERR_INVALID_SESSION_TICKET);
         };
 
+        /* Ticket issue time is within MAX_ALLOWED_CLOCK_SKEW
+         * Distributed deployments with clock skew may see a ticket issue time in
+         * the future.
+         */
+        {
+            uint64_t current_time = SECONDS_TO_NANOS(0);
+            EXPECT_OK(s2n_validate_ticket_age(current_time, SECONDS_TO_NANOS(MAX_ALLOWED_CLOCK_SKEW_SEC - 1)));
+            EXPECT_OK(s2n_validate_ticket_age(current_time, SECONDS_TO_NANOS(MAX_ALLOWED_CLOCK_SKEW_SEC)));
+        };
+
         /** Ticket age is longer than a week
          *= https://www.rfc-editor.org/rfc/rfc8446#section-4.6.1
          *= type=test

diff --git a/tls/s2n_resume.c b/tls/s2n_resume.c
@@ -278,12 +278,27 @@ static S2N_RESULT s2n_tls12_client_deserialize_session_state(struct s2n_connecti
     return S2N_RESULT_OK;
 }
 
+/* `s2n_validate_ticket_age` is a best effort check that the session ticket is
+ * less than one week old.
+ *
+ * Clock skew between hosts or the possibility of a clock jump prevent this from
+ * being a precise check.
+ */
 static S2N_RESULT s2n_validate_ticket_age(uint64_t current_time, uint64_t ticket_issue_time)
 {
-    RESULT_ENSURE(current_time >= ticket_issue_time, S2N_ERR_INVALID_SESSION_TICKET);
-    uint64_t ticket_age_in_nanos = current_time - ticket_issue_time;
-    uint64_t ticket_age_in_sec = ticket_age_in_nanos / ONE_SEC_IN_NANOS;
-    RESULT_ENSURE(ticket_age_in_sec <= ONE_WEEK_IN_SEC, S2N_ERR_INVALID_SESSION_TICKET);
+    /* If the `ticket_issue_time` is in the future, then we are observing clock skew.
+     * We shouldn't fully reject the ticket, but we assert that the clock skew is
+     * less than some MAX_ALLOWED_CLOCK_SKEW_SEC
+     */
+    if (current_time < ticket_issue_time) {
+        uint64_t clock_skew_in_nanos = ticket_issue_time - current_time;
+        uint64_t clock_skew_in_seconds = clock_skew_in_nanos / ONE_SEC_IN_NANOS;
+        RESULT_ENSURE(clock_skew_in_seconds <= MAX_ALLOWED_CLOCK_SKEW_SEC, S2N_ERR_INVALID_SESSION_TICKET);
+    } else {
+        uint64_t ticket_age_in_nanos = current_time - ticket_issue_time;
+        uint64_t ticket_age_in_sec = ticket_age_in_nanos / ONE_SEC_IN_NANOS;
+        RESULT_ENSURE(ticket_age_in_sec <= ONE_WEEK_IN_SEC, S2N_ERR_INVALID_SESSION_TICKET);
+    }
     return S2N_RESULT_OK;
 }
 

diff --git a/tls/s2n_resume.h b/tls/s2n_resume.h
@@ -23,6 +23,10 @@
 #define S2N_TLS13_FIXED_STATE_SIZE            21
 #define S2N_TLS13_FIXED_EARLY_DATA_STATE_SIZE 3
 
+/* This is used in session ticket validation. This controls how far in the future
+ * the session ticket issue time can be while still being accepted.
+ */
+#define MAX_ALLOWED_CLOCK_SKEW_SEC     3600
 #define S2N_TLS_SESSION_CACHE_TTL      (6 * 60 * 60)
 #define S2N_TICKET_KEY_NAME_LEN        16
 #define S2N_TICKET_AAD_IMPLICIT_LEN    12