Merge commit from fork

* Adds key check * PR feedback
aws · Jul 18, 2024 · ac5878b · ac5878b
1 parent 533b502
commit ac5878b
Show file tree

Hide file tree

Showing 2 changed files with 31 additions and 4 deletions.
diff --git a/tests/unit/s2n_resume_test.c b/tests/unit/s2n_resume_test.c
@@ -1412,6 +1412,32 @@ int main(int argc, char **argv)
             EXPECT_SUCCESS(s2n_config_free(config));
         };
 
+        /* Check session ticket can never be encrypted with a zero-filled ticket key */
+        {
+            DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER), s2n_connection_ptr_free);
+            EXPECT_NOT_NULL(conn);
+
+            DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
+            EXPECT_NOT_NULL(config);
+
+            /* Add a valid ticket key to the store */
+            EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(config, 1));
+            EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(config, ticket_key_name, strlen((char *) ticket_key_name),
+                    ticket_key.data, ticket_key.size, 0));
+            EXPECT_SUCCESS(s2n_connection_set_config(conn, config));
+
+            /* Manually zero out key bytes */
+            struct s2n_ticket_key *key = NULL;
+            EXPECT_OK(s2n_set_get(config->ticket_keys, 0, (void **) &key));
+            EXPECT_NOT_NULL(key);
+            POSIX_CHECKED_MEMSET((uint8_t *) key->aes_key, 0, S2N_AES256_KEY_LEN);
+
+            DEFER_CLEANUP(struct s2n_stuffer output = { 0 }, s2n_stuffer_free);
+            EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&output, 0));
+
+            EXPECT_FAILURE_WITH_ERRNO(s2n_encrypt_session_ticket(conn, &output), S2N_ERR_KEY_CHECK);
+        };
+
         /* Check session ticket is correct when using early data with TLS1.3. */
         {
             const uint8_t test_early_data_context[] = "context";

diff --git a/tls/s2n_resume.c b/tls/s2n_resume.c
@@ -757,8 +757,6 @@ struct s2n_ticket_key *s2n_find_ticket_key(struct s2n_config *config, const uint
             if (now >= ticket_key->intro_timestamp
                             + config->encrypt_decrypt_key_lifetime_in_nanos
                             + config->decrypt_key_lifetime_in_nanos) {
-                s2n_config_wipe_expired_ticket_crypto_keys(config, i);
-
                 return NULL;
             }
 
@@ -772,7 +770,6 @@ struct s2n_ticket_key *s2n_find_ticket_key(struct s2n_config *config, const uint
 int s2n_encrypt_session_ticket(struct s2n_connection *conn, struct s2n_stuffer *to)
 {
     struct s2n_ticket_key *key = NULL;
-    struct s2n_session_key aes_ticket_key = { 0 };
     struct s2n_blob aes_key_blob = { 0 };
 
     uint8_t iv_data[S2N_TLS_GCM_IV_LEN] = { 0 };
@@ -795,10 +792,15 @@ int s2n_encrypt_session_ticket(struct s2n_connection *conn, struct s2n_stuffer *
     POSIX_GUARD(s2n_stuffer_write(to, &iv));
 
     POSIX_GUARD(s2n_blob_init(&aes_key_blob, key->aes_key, S2N_AES256_KEY_LEN));
+    DEFER_CLEANUP(struct s2n_session_key aes_ticket_key = { 0 }, s2n_session_key_free);
     POSIX_GUARD(s2n_session_key_alloc(&aes_ticket_key));
     POSIX_GUARD_RESULT(s2n_aes256_gcm.init(&aes_ticket_key));
     POSIX_GUARD_RESULT(s2n_aes256_gcm.set_encryption_key(&aes_ticket_key, &aes_key_blob));
 
+    /* Ensure we never encrypt with a zero-filled key */
+    uint8_t zero_block[S2N_AES256_KEY_LEN] = { 0 };
+    POSIX_ENSURE(memcmp(key->aes_key, zero_block, S2N_AES256_KEY_LEN) != 0, S2N_ERR_KEY_CHECK);
+
     POSIX_GUARD(s2n_stuffer_init(&aad, &aad_blob));
     POSIX_GUARD(s2n_stuffer_write_bytes(&aad, key->implicit_aad, S2N_TICKET_AAD_IMPLICIT_LEN));
     POSIX_GUARD(s2n_stuffer_write_bytes(&aad, key->key_name, S2N_TICKET_KEY_NAME_LEN));
@@ -818,7 +820,6 @@ int s2n_encrypt_session_ticket(struct s2n_connection *conn, struct s2n_stuffer *
     POSIX_GUARD(s2n_aes256_gcm.io.aead.encrypt(&aes_ticket_key, &iv, &aad_blob, &state_blob, &state_blob));
 
     POSIX_GUARD_RESULT(s2n_aes256_gcm.destroy_key(&aes_ticket_key));
-    POSIX_GUARD(s2n_session_key_free(&aes_ticket_key));
 
     return S2N_SUCCESS;
 }