Skip to content

fix: copy ticket key before use to avoid concurrent rotation UAF #6043

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jouho wants to merge 1 commit into
base: main
Choose a base branch
from
+36 −15
Open

fix: copy ticket key before use to avoid concurrent rotation UAF #6043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions tls/s2n_resume.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -494,8 +494,14 @@ S2N_RESULT s2n_store_to_cache(struct s2n_connection *conn)

RESULT_GUARD_POSIX(s2n_stuffer_init(&to, &entry));

struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, key, &to));
/* The lookup returns a pointer into the shared, unsynchronized config->ticket_keys array.
* A concurrent call to s2n_config_add_ticket_crypto_key (key rotation) can reallocate,
* shift, or overwrite that array, invalidating the pointer. Copy the key to the stack
* immediately so later operations never dereference memory another thread might change. */
struct s2n_ticket_key *key_ptr = s2n_get_ticket_encrypt_decrypt_key(conn->config);
RESULT_ENSURE(key_ptr != NULL, S2N_ERR_NO_TICKET_ENCRYPT_DECRYPT_KEY);
struct s2n_ticket_key key = *key_ptr;
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, &key, &to));

/* Store to the cache */
conn->config->cache_store(conn, conn->config->cache_store_data, S2N_TLS_SESSION_CACHE_TTL, conn->session_id, conn->session_id_len, entry.data, entry.size);
Expand Down Expand Up @@ -905,12 +911,17 @@ S2N_RESULT s2n_resume_decrypt_session(struct s2n_connection *conn, struct s2n_st
uint8_t key_name[S2N_TICKET_KEY_NAME_LEN] = { 0 };
RESULT_GUARD_POSIX(s2n_stuffer_read_bytes(from, key_name, sizeof(key_name)));

struct s2n_ticket_key *key = s2n_find_ticket_key(conn->config, key_name);
/* The lookup returns a pointer into the shared, unsynchronized config->ticket_keys array.
* A concurrent call to s2n_config_add_ticket_crypto_key (key rotation) can reallocate,
* shift, or overwrite that array, invalidating the pointer. Copy the key to the stack
* immediately so later operations never dereference memory another thread might change. */
struct s2n_ticket_key *key_ptr = s2n_find_ticket_key(conn->config, key_name);
/* Key has expired; do full handshake */
RESULT_ENSURE(key != NULL, S2N_ERR_KEY_USED_IN_SESSION_TICKET_NOT_FOUND);
RESULT_ENSURE(key_ptr != NULL, S2N_ERR_KEY_USED_IN_SESSION_TICKET_NOT_FOUND);
struct s2n_ticket_key key = *key_ptr;

struct s2n_unique_ticket_key ticket_key = { 0 };
RESULT_GUARD_POSIX(s2n_blob_init(&ticket_key.initial_key, key->aes_key, sizeof(key->aes_key)));
RESULT_GUARD_POSIX(s2n_blob_init(&ticket_key.initial_key, key.aes_key, sizeof(key.aes_key)));
RESULT_GUARD_POSIX(s2n_stuffer_read_bytes(from, ticket_key.info, sizeof(ticket_key.info)));
RESULT_GUARD(s2n_resume_generate_unique_ticket_key(&ticket_key));

Expand All @@ -934,8 +945,8 @@ S2N_RESULT s2n_resume_decrypt_session(struct s2n_connection *conn, struct s2n_st
RESULT_GUARD_POSIX(s2n_blob_init(&aad_blob, aad_data, sizeof(aad_data)));
struct s2n_stuffer aad = { 0 };
RESULT_GUARD_POSIX(s2n_stuffer_init(&aad, &aad_blob));
RESULT_GUARD_POSIX(s2n_stuffer_write_bytes(&aad, key->implicit_aad, sizeof(key->implicit_aad)));
RESULT_GUARD_POSIX(s2n_stuffer_write_bytes(&aad, key->key_name, sizeof(key->key_name)));
RESULT_GUARD_POSIX(s2n_stuffer_write_bytes(&aad, key.implicit_aad, sizeof(key.implicit_aad)));
RESULT_GUARD_POSIX(s2n_stuffer_write_bytes(&aad, key.key_name, sizeof(key.key_name)));

/* Initialize blob to be decrypted */
struct s2n_blob en_blob = { 0 };
Expand Down
26 changes: 18 additions & 8 deletions tls/s2n_server_new_session_ticket.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,11 +162,16 @@ S2N_RESULT s2n_server_nst_write(struct s2n_connection *conn, uint32_t *lifetime_
struct s2n_stuffer output = { 0 };
RESULT_GUARD_POSIX(s2n_stuffer_init(&output, session_ticket));

struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
RESULT_ENSURE(key != NULL, S2N_ERR_NO_TICKET_ENCRYPT_DECRYPT_KEY);
/* The lookup returns a pointer into the shared, unsynchronized config->ticket_keys array.
* A concurrent call to s2n_config_add_ticket_crypto_key (key rotation) can reallocate,
* shift, or overwrite that array, invalidating the pointer. Copy the key to the stack
* immediately so later operations never dereference memory another thread might change. */
struct s2n_ticket_key *key_ptr = s2n_get_ticket_encrypt_decrypt_key(conn->config);
RESULT_ENSURE(key_ptr != NULL, S2N_ERR_NO_TICKET_ENCRYPT_DECRYPT_KEY);
struct s2n_ticket_key key = *key_ptr;

RESULT_GUARD(s2n_generate_ticket_lifetime(conn, key->intro_timestamp, lifetime_hint_in_secs));
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, key, &output));
RESULT_GUARD(s2n_generate_ticket_lifetime(conn, key.intro_timestamp, lifetime_hint_in_secs));
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, &key, &output));

return S2N_RESULT_OK;
}
Expand Down Expand Up @@ -302,8 +307,13 @@ S2N_RESULT s2n_tls13_server_nst_write(struct s2n_connection *conn, struct s2n_st
RESULT_ENSURE_REF(conn);
RESULT_ENSURE_REF(output);

struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
RESULT_ENSURE(key != NULL, S2N_ERR_NO_TICKET_ENCRYPT_DECRYPT_KEY);
/* The lookup returns a pointer into the shared, unsynchronized config->ticket_keys array.
* A concurrent call to s2n_config_add_ticket_crypto_key (key rotation) can reallocate,
* shift, or overwrite that array, invalidating the pointer. Copy the key to the stack
* immediately so later operations never dereference memory another thread might change. */
struct s2n_ticket_key *key_ptr = s2n_get_ticket_encrypt_decrypt_key(conn->config);
RESULT_ENSURE(key_ptr != NULL, S2N_ERR_NO_TICKET_ENCRYPT_DECRYPT_KEY);
struct s2n_ticket_key key = *key_ptr;

struct s2n_ticket_fields *ticket_fields = &conn->tls13_ticket_fields;

Expand All @@ -314,7 +324,7 @@ S2N_RESULT s2n_tls13_server_nst_write(struct s2n_connection *conn, struct s2n_st
RESULT_GUARD_POSIX(s2n_stuffer_reserve_uint24(output, &message_size));

uint32_t ticket_lifetime_in_secs = 0;
RESULT_GUARD(s2n_generate_ticket_lifetime(conn, key->intro_timestamp, &ticket_lifetime_in_secs));
RESULT_GUARD(s2n_generate_ticket_lifetime(conn, key.intro_timestamp, &ticket_lifetime_in_secs));

RESULT_ENSURE(ticket_lifetime_in_secs > 0, S2N_ERR_ZERO_LIFETIME_TICKET);
RESULT_GUARD_POSIX(s2n_stuffer_write_uint32(output, ticket_lifetime_in_secs));
Expand Down Expand Up @@ -346,7 +356,7 @@ S2N_RESULT s2n_tls13_server_nst_write(struct s2n_connection *conn, struct s2n_st
/* Write ticket */
struct s2n_stuffer_reservation ticket_size = { 0 };
RESULT_GUARD_POSIX(s2n_stuffer_reserve_uint16(output, &ticket_size));
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, key, output));
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, &key, output));
RESULT_GUARD_POSIX(s2n_stuffer_write_vector_size(&ticket_size));

RESULT_GUARD_POSIX(s2n_extension_list_send(S2N_EXTENSION_LIST_NST, conn, output));
Expand Down
Loading