Fix memory allocation when session ticket is used (#2470)

Author: lrstewart

crypto/s2n_cipher.c

@@ -41,13 +41,15 @@ int s2n_session_key_alloc(struct s2n_session_key *key)
 
 int s2n_session_key_free(struct s2n_session_key *key)
 {
-    notnull_check(key->evp_cipher_ctx);
-    EVP_CIPHER_CTX_free(key->evp_cipher_ctx);
-    key->evp_cipher_ctx = NULL;
+    if (key->evp_cipher_ctx != NULL) {
+        EVP_CIPHER_CTX_free(key->evp_cipher_ctx);
+        key->evp_cipher_ctx = NULL;
+    }
 #if defined(S2N_CIPHER_AEAD_API_AVAILABLE)
-    notnull_check(key->evp_aead_ctx);
-    EVP_AEAD_CTX_free(key->evp_aead_ctx);
-    key->evp_aead_ctx = NULL;
+    if (key->evp_aead_ctx != NULL) {
+        EVP_AEAD_CTX_free(key->evp_aead_ctx);
+        key->evp_aead_ctx = NULL;
+    }
 #endif
 
     return 0;

tests/unit/s2n_session_ticket_test.c

@@ -964,6 +964,58 @@ int main(int argc, char **argv)
         EXPECT_SUCCESS(s2n_config_free(client_config));
     }
 
+    /* s2n_decrypt_session_ticket fails to decrypt when presented with a valid ticket_key, valid iv and invalid encrypted blob */
+    {
+        EXPECT_NOT_NULL(server_conn = s2n_connection_new(S2N_SERVER));
+        EXPECT_NOT_NULL(server_config = s2n_config_new());
+
+        /* Add Session Ticket key on the server config */
+        EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(server_config, 1));
+        EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_config, ticket_key_name1, strlen((char *)ticket_key_name1), ticket_key1, sizeof(ticket_key1), 0));
+        EXPECT_SUCCESS(s2n_connection_set_config(server_conn, server_config));
+
+        /* Setup stuffers value containing the valid key name, valid iv and invalid encrypted blob */
+        GUARD(s2n_stuffer_write_bytes(&server_conn->client_ticket_to_decrypt, ticket_key_name1, sizeof(ticket_key_name1)));
+
+        uint8_t valid_iv[S2N_TLS_GCM_IV_LEN] = {0};
+        GUARD(s2n_stuffer_write_bytes(&server_conn->client_ticket_to_decrypt, valid_iv, sizeof(valid_iv)));
+
+        uint8_t invalid_en_data[S2N_STATE_SIZE_IN_BYTES + S2N_TLS_GCM_TAG_LEN] = {0};
+        GUARD(s2n_stuffer_write_bytes(&server_conn->client_ticket_to_decrypt, invalid_en_data, sizeof(invalid_en_data)));
+
+        server_conn->session_ticket_status = S2N_DECRYPT_TICKET;
+        EXPECT_FAILURE_WITH_ERRNO(s2n_decrypt_session_ticket(server_conn), S2N_ERR_DECRYPT);
+
+        EXPECT_SUCCESS(s2n_connection_free(server_conn));
+        EXPECT_SUCCESS(s2n_config_free(server_config));
+    }
+
+    /* s2n_decrypt_session_ticket fails with a key not found error when presented with an invalid ticket_key, valid iv and invalid encrypted blob */
+    {
+        EXPECT_NOT_NULL(server_conn = s2n_connection_new(S2N_SERVER));
+        EXPECT_NOT_NULL(server_config = s2n_config_new());
+
+        /* Add Session Ticket key on the server config */
+        EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(server_config, 1));
+        EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_config, ticket_key_name1, strlen((char *)ticket_key_name1), ticket_key1, sizeof(ticket_key1), 0));
+        EXPECT_SUCCESS(s2n_connection_set_config(server_conn, server_config));
+
+        /* Setup stuffers value containing the invalid key name, valid iv and invalid encrypted blob */
+        GUARD(s2n_stuffer_write_bytes(&server_conn->client_ticket_to_decrypt, ticket_key_name2, sizeof(ticket_key_name2)));
+
+        uint8_t valid_iv[S2N_TLS_GCM_IV_LEN] = {0};
+        GUARD(s2n_stuffer_write_bytes(&server_conn->client_ticket_to_decrypt, valid_iv, sizeof(valid_iv)));
+
+        uint8_t invalid_en_data[S2N_STATE_SIZE_IN_BYTES + S2N_TLS_GCM_TAG_LEN] = {0};
+        GUARD(s2n_stuffer_write_bytes(&server_conn->client_ticket_to_decrypt, invalid_en_data, sizeof(invalid_en_data)));
+
+        server_conn->session_ticket_status = S2N_DECRYPT_TICKET;
+        EXPECT_FAILURE_WITH_ERRNO(s2n_decrypt_session_ticket(server_conn), S2N_ERR_KEY_USED_IN_SESSION_TICKET_NOT_FOUND);
+
+        EXPECT_SUCCESS(s2n_connection_free(server_conn));
+        EXPECT_SUCCESS(s2n_config_free(server_config));
+    }
+
     EXPECT_SUCCESS(s2n_io_pair_close(&io_pair));
     EXPECT_SUCCESS(s2n_cert_chain_and_key_free(chain_and_key));
     free(cert_chain);

tls/s2n_resume.c

@@ -529,7 +529,7 @@ int s2n_encrypt_session_ticket(struct s2n_connection *conn, struct s2n_stuffer *
 int s2n_decrypt_session_ticket(struct s2n_connection *conn)
 {
     struct s2n_ticket_key *key;
-    struct s2n_session_key aes_ticket_key = {0};
+    DEFER_CLEANUP(struct s2n_session_key aes_ticket_key = {0}, s2n_session_key_free);
     struct s2n_blob aes_key_blob = {0};
     struct s2n_stuffer *from;
 
@@ -581,9 +581,6 @@ int s2n_decrypt_session_ticket(struct s2n_connection *conn)
 
     GUARD(s2n_deserialize_resumption_state(conn, &state));
 
-    GUARD(s2n_aes256_gcm.destroy_key(&aes_ticket_key));
-    GUARD(s2n_session_key_free(&aes_ticket_key));
-
     uint64_t now;
     GUARD(conn->config->wall_clock(conn->config->sys_clock_ctx, &now));