Skip to content

fix: tighten session ticket lifetime #5217

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

Merged
merged 4 commits into from
Mar 28, 2025
Merged

fix: tighten session ticket lifetime #5217

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e84498f
tighten session ticket lifetime
CarolYeh910 Mar 24, 2025
03d785a
add encrypt key assertion
CarolYeh910 Mar 26, 2025
ef19118
address PR feedback
CarolYeh910 Mar 27, 2025
b44c32c
Merge branch 'main' into tighten-ticket-lifetime
CarolYeh910 Mar 28, 2025
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
6 changes: 3 additions & 3 deletions tests/saw/spec/handshake/handshake.saw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ let prove_handshake_io_lowlevel = do {
s2n_generate_new_client_session_id <- crucible_llvm_unsafe_assume_spec llvm "s2n_generate_new_client_session_id" s2n_generate_new_client_session_id_spec;
print "s2n_allowed_to_cache_connection";
s2n_allowed_to_cache_connection <- crucible_llvm_unsafe_assume_spec llvm "s2n_allowed_to_cache_connection" s2n_allowed_to_cache_connection_spec;
print "s2n_resume_decrypt_session_ticket";
s2n_resume_decrypt_session_ticket <- crucible_llvm_unsafe_assume_spec llvm "s2n_resume_decrypt_session_ticket" s2n_resume_decrypt_session_ticket_spec;
print "s2n_resume_decrypt_session";
s2n_resume_decrypt_session <- crucible_llvm_unsafe_assume_spec llvm "s2n_resume_decrypt_session" s2n_resume_decrypt_session_spec;
let dependencies = [s2n_socket_write_uncork, s2n_socket_write_cork, s2n_socket_was_corked, s2n_connection_is_managed_corked, s2n_socket_quickack];

print "Proving correctness of get_auth_type";
Expand All @@ -85,7 +85,7 @@ let prove_handshake_io_lowlevel = do {
// depends on whether chosen_psk is NULL.
//
// Issue #3052 is about removing the need to invoke the specification twice.
let s2n_conn_set_handshake_type_ovs = [s2n_allowed_to_cache_connection, auth_type_proof, s2n_generate_new_client_session_id, s2n_resume_decrypt_session_ticket];
let s2n_conn_set_handshake_type_ovs = [s2n_allowed_to_cache_connection, auth_type_proof, s2n_generate_new_client_session_id, s2n_resume_decrypt_session];
print "Proving correctness of s2n_conn_set_handshake_type (NULL chosen_psk)";
s2n_conn_set_handshake_type_chosen_psk_null_proof <- crucible_llvm_verify llvm "s2n_conn_set_handshake_type" s2n_conn_set_handshake_type_ovs false (s2n_conn_set_handshake_type_spec true) (w4_unint_yices []);
print "Proving correctness of s2n_conn_set_handshake_type (non-NULL chosen_psk)";
Expand Down
4 changes: 2 additions & 2 deletions tests/saw/spec/handshake/handshake_io_lowlevel.saw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -459,9 +459,9 @@ let s2n_generate_new_client_session_id_spec = do {
crucible_return (crucible_term {{ 0 : [32] }});
};

// Specification for s2n_resume_decrypt_session_ticket_spec. This is essentially
// Specification for s2n_resume_decrypt_session_spec. This is essentially
// a noop function that returns 0 from the perspective of our current proof
let s2n_resume_decrypt_session_ticket_spec = do {
let s2n_resume_decrypt_session_spec = do {
pconn <- crucible_alloc_readonly (llvm_struct "struct.s2n_connection");
from <- crucible_alloc_readonly (llvm_struct "struct.s2n_stuffer");

Expand Down
16 changes: 8 additions & 8 deletions tests/unit/s2n_resume_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1349,7 +1349,7 @@ int main(int argc, char **argv)
/* Wiping the master secret to prove that the decryption function actually writes the master secret */
memset(conn->secrets.version.tls12.master_secret, 0, test_master_secret.size);

EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &conn->client_ticket_to_decrypt));
EXPECT_OK(s2n_resume_decrypt_session(conn, &conn->client_ticket_to_decrypt));
EXPECT_EQUAL(s2n_stuffer_data_available(&conn->client_ticket_to_decrypt), 0);

/* Check decryption was successful by comparing master key */
Expand Down Expand Up @@ -1384,7 +1384,7 @@ int main(int argc, char **argv)
EXPECT_NOT_NULL(key);

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, key, &output));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_decrypt_session(conn, &output));

EXPECT_EQUAL(s2n_stuffer_data_available(&output), 0);

Expand Down Expand Up @@ -1423,7 +1423,7 @@ int main(int argc, char **argv)
EXPECT_NOT_NULL(key);

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, key, &output));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_decrypt_session(conn, &output));

EXPECT_EQUAL(s2n_stuffer_data_available(&output), 0);

Expand Down Expand Up @@ -1461,13 +1461,13 @@ int main(int argc, char **argv)
EXPECT_EQUAL(*version_num, S2N_PRE_ENCRYPTED_STATE_V1);
*version_num = S2N_PRE_ENCRYPTED_STATE_V1 + 100;

EXPECT_ERROR_WITH_ERRNO(s2n_resume_decrypt_session_ticket(conn, &conn->client_ticket_to_decrypt),
EXPECT_ERROR_WITH_ERRNO(s2n_resume_decrypt_session(conn, &conn->client_ticket_to_decrypt),
S2N_ERR_SAFETY);

/* The correct version number should succeed */
*version_num = S2N_PRE_ENCRYPTED_STATE_V1;
EXPECT_SUCCESS(s2n_stuffer_reread(&conn->client_ticket_to_decrypt));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &conn->client_ticket_to_decrypt));
EXPECT_OK(s2n_resume_decrypt_session(conn, &conn->client_ticket_to_decrypt));
}

/* Check error is thrown when info bytes used to generate the ticket key are incorrect */
Expand Down Expand Up @@ -1511,12 +1511,12 @@ int main(int argc, char **argv)
memset(info_ptr, 0, S2N_TICKET_INFO_SIZE);
EXPECT_SUCCESS(s2n_stuffer_reread(&conn->client_ticket_to_decrypt));

EXPECT_ERROR_WITH_ERRNO(s2n_resume_decrypt_session_ticket(conn, &conn->client_ticket_to_decrypt),
EXPECT_ERROR_WITH_ERRNO(s2n_resume_decrypt_session(conn, &conn->client_ticket_to_decrypt),
S2N_ERR_DECRYPT);

/* The correct info bytes should succeed */
EXPECT_SUCCESS(s2n_stuffer_reread(&valid_ticket));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &valid_ticket));
EXPECT_OK(s2n_resume_decrypt_session(conn, &valid_ticket));
}

/* Check session ticket can never be encrypted with a zero-filled ticket key */
Expand Down Expand Up @@ -1570,7 +1570,7 @@ int main(int argc, char **argv)
EXPECT_NOT_NULL(key);

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, key, &output));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_decrypt_session(conn, &output));

EXPECT_EQUAL(s2n_stuffer_data_available(&output), 0);

Expand Down
49 changes: 24 additions & 25 deletions tests/unit/s2n_session_ticket_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ int main(int argc, char **argv)
* 1) Client sends empty ST extension. Server issues NST.
* 2) Client sends empty ST extension. Server does a full handshake, but is unable to issue NST due to absence of an encrypt-decrypt key.
* 3) Client sends non-empty ST extension. Server does an abbreviated handshake without issuing NST.
* 4) Client sends non-empty ST extension. Server does an abbreviated handshake and issues a NST because the key is in decrypt-only state.
* 4) Client sends non-empty ST extension. Server does an abbreviated handshake without issuing NST even though the key is in decrypt-only state.
* 5) Client sends non-empty ST extension. Server does an abbreviated handshake, but does not issue a NST even though the key is in
* decrypt-only state due to absence of encrypt-decrypt key.
* 6) Client sends non-empty ST extension. Server does a full handshake and issues a NST because the key is not found.
Expand Down Expand Up @@ -218,8 +218,8 @@ int main(int argc, char **argv)
uint64_t mock_current_time = 0;
EXPECT_SUCCESS(s2n_config_set_wall_clock(server_config, mock_time, &mock_current_time));

EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_config, ticket_key_name1, s2n_array_len(ticket_key_name1),
ticket_key1, s2n_array_len(ticket_key1), 0));
EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_config, ticket_key_name2, s2n_array_len(ticket_key_name2),
ticket_key2, s2n_array_len(ticket_key2), 0));

EXPECT_SUCCESS(s2n_connection_set_config(server_conn, server_config));

Expand All @@ -236,7 +236,7 @@ int main(int argc, char **argv)
serialized_session_state_length = s2n_connection_get_session_length(client_conn);
EXPECT_EQUAL(s2n_connection_get_session(client_conn, serialized_session_state, serialized_session_state_length), serialized_session_state_length);
EXPECT_BYTEARRAY_EQUAL(serialized_session_state + S2N_TICKET_KEY_NAME_LOCATION,
ticket_key_name1, s2n_array_len(ticket_key_name1));
ticket_key_name2, s2n_array_len(ticket_key_name2));

/* Verify the lifetime hint from the server */
EXPECT_EQUAL(s2n_connection_get_session_ticket_lifetime_hint(client_conn), S2N_SESSION_STATE_CONFIGURABLE_LIFETIME_IN_SECS);
Expand Down Expand Up @@ -312,8 +312,8 @@ int main(int argc, char **argv)
uint64_t mock_current_time = 0;
EXPECT_SUCCESS(s2n_config_set_wall_clock(server_config, mock_time, &mock_current_time));

EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_config, ticket_key_name1, s2n_array_len(ticket_key_name1),
ticket_key1, s2n_array_len(ticket_key1), 0));
EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_config, ticket_key_name2, s2n_array_len(ticket_key_name2),
ticket_key2, s2n_array_len(ticket_key2), 0));

EXPECT_SUCCESS(s2n_connection_set_config(server_conn, server_config));

Expand Down Expand Up @@ -343,8 +343,8 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_config_free(client_config));
};

/* Client sends non-empty ST extension. Server does an abbreviated handshake and issues a NST
* because the key is in decrypt-only state.
/* Client sends non-empty ST extension. Server does an abbreviated handshake without issuing a NST
* even though the key is in decrypt-only state.
*/
{
EXPECT_NOT_NULL(client_conn = s2n_connection_new(S2N_CLIENT));
Expand Down Expand Up @@ -372,36 +372,35 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_config_set_wall_clock(server_config, mock_time, &mock_current_time));

/* Add one ST key */
EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_config, ticket_key_name1, s2n_array_len(ticket_key_name1), ticket_key1, s2n_array_len(ticket_key1), 0));
EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_config, ticket_key_name2, s2n_array_len(ticket_key_name2), ticket_key2, s2n_array_len(ticket_key2), 0));

/* Add a mock delay such that key 1 moves to decrypt-only state */
mock_current_time += server_config->encrypt_decrypt_key_lifetime_in_nanos;

uint32_t key_intro_time = mock_current_time / ONE_SEC_IN_NANOS;
EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_config, ticket_key_name2, s2n_array_len(ticket_key_name2),
ticket_key2, s2n_array_len(ticket_key2), key_intro_time));
EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_config, ticket_key_name1, s2n_array_len(ticket_key_name1),
ticket_key1, s2n_array_len(ticket_key1), key_intro_time));

/* Verify there is an encrypt key available */
EXPECT_OK(s2n_config_is_encrypt_key_available(server_config));

EXPECT_SUCCESS(s2n_connection_set_config(server_conn, server_config));

EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(server_conn, client_conn));

/* Verify that the server did an abbreviated handshake and issued NST */
/* Verify that the server did an abbreviated handshake without issuing NST */
EXPECT_TRUE(IS_RESUMPTION_HANDSHAKE(server_conn));
EXPECT_TRUE(IS_ISSUING_NEW_SESSION_TICKET(server_conn));
EXPECT_FALSE(IS_ISSUING_NEW_SESSION_TICKET(server_conn));

/* Verify that client_ticket is not same as before because server issued a NST */
/* Verify that client_ticket is the same as before because server didn't issue a NST */
uint8_t old_session_ticket[S2N_PARTIAL_SESSION_STATE_INFO_IN_BYTES + S2N_TLS12_TICKET_SIZE_IN_BYTES];
EXPECT_MEMCPY_SUCCESS(old_session_ticket, serialized_session_state, S2N_PARTIAL_SESSION_STATE_INFO_IN_BYTES + S2N_TLS12_TICKET_SIZE_IN_BYTES);

s2n_connection_get_session(client_conn, serialized_session_state, serialized_session_state_length);
EXPECT_TRUE(memcmp(old_session_ticket, serialized_session_state, S2N_PARTIAL_SESSION_STATE_INFO_IN_BYTES + S2N_TLS12_TICKET_SIZE_IN_BYTES));
EXPECT_EQUAL(s2n_connection_get_session(client_conn, serialized_session_state, serialized_session_state_length), serialized_session_state_length);
EXPECT_BYTEARRAY_EQUAL(old_session_ticket, serialized_session_state, S2N_PARTIAL_SESSION_STATE_INFO_IN_BYTES + S2N_TLS12_TICKET_SIZE_IN_BYTES);

/* Verify the lifetime hint from the server */
EXPECT_EQUAL(s2n_connection_get_session_ticket_lifetime_hint(client_conn), S2N_SESSION_STATE_CONFIGURABLE_LIFETIME_IN_SECS);

/* Verify that the new NST is encrypted using second ST */
EXPECT_BYTEARRAY_EQUAL(serialized_session_state + S2N_TICKET_KEY_NAME_LOCATION,
ticket_key_name2, s2n_array_len(ticket_key_name2));
EXPECT_EQUAL(s2n_connection_get_session_ticket_lifetime_hint(client_conn), 0);

EXPECT_SUCCESS(s2n_shutdown_test_server_and_client(server_conn, client_conn));

Expand Down Expand Up @@ -1127,7 +1126,7 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_config_free(client_config));
};

/* s2n_resume_decrypt_session_ticket fails to decrypt when presented with a valid ticket_key, valid iv and invalid encrypted blob */
/* s2n_resume_decrypt_session 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());
Expand All @@ -1151,13 +1150,13 @@ int main(int argc, char **argv)
POSIX_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_ERROR_WITH_ERRNO(s2n_resume_decrypt_session_ticket(server_conn, &server_conn->client_ticket_to_decrypt), S2N_ERR_DECRYPT);
EXPECT_ERROR_WITH_ERRNO(s2n_resume_decrypt_session(server_conn, &server_conn->client_ticket_to_decrypt), S2N_ERR_DECRYPT);

EXPECT_SUCCESS(s2n_connection_free(server_conn));
EXPECT_SUCCESS(s2n_config_free(server_config));
};

/* s2n_resume_decrypt_session_ticket fails with a key not found error when presented with an invalid ticket_key, valid iv and invalid encrypted blob */
/* s2n_resume_decrypt_session 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());
Expand All @@ -1181,7 +1180,7 @@ int main(int argc, char **argv)
POSIX_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_ERROR_WITH_ERRNO(s2n_resume_decrypt_session_ticket(server_conn, &server_conn->client_ticket_to_decrypt), S2N_ERR_KEY_USED_IN_SESSION_TICKET_NOT_FOUND);
EXPECT_ERROR_WITH_ERRNO(s2n_resume_decrypt_session(server_conn, &server_conn->client_ticket_to_decrypt), S2N_ERR_KEY_USED_IN_SESSION_TICKET_NOT_FOUND);

EXPECT_SUCCESS(s2n_connection_free(server_conn));
EXPECT_SUCCESS(s2n_config_free(server_config));
Expand Down
2 changes: 1 addition & 1 deletion tls/s2n_handshake_io.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1046,7 +1046,7 @@ int s2n_conn_set_handshake_type(struct s2n_connection *conn)
/* We reuse the session if a valid TLS12 ticket is provided.
* Otherwise, we will perform a full handshake and then generate
* a new session ticket. */
if (s2n_result_is_ok(s2n_resume_decrypt_session_ticket(conn, &conn->client_ticket_to_decrypt))) {
if (s2n_result_is_ok(s2n_resume_decrypt_session(conn, &conn->client_ticket_to_decrypt))) {
return S2N_SUCCESS;
}

Expand Down
4 changes: 2 additions & 2 deletions tls/s2n_psk.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,8 +333,8 @@ int s2n_offered_psk_list_choose_psk(struct s2n_offered_psk_list *psk_list, struc
POSIX_GUARD(s2n_stuffer_init(&ticket_stuffer, &psk->identity));
POSIX_GUARD(s2n_stuffer_skip_write(&ticket_stuffer, psk->identity.size));

/* s2n_resume_decrypt_session_ticket appends a new PSK with the decrypted values. */
POSIX_GUARD_RESULT(s2n_resume_decrypt_session_ticket(psk_list->conn, &ticket_stuffer));
/* s2n_resume_decrypt_session appends a new PSK with the decrypted values. */
POSIX_GUARD_RESULT(s2n_resume_decrypt_session(psk_list->conn, &ticket_stuffer));
}

struct s2n_psk *chosen_psk = NULL;
Expand Down
42 changes: 3 additions & 39 deletions tls/s2n_resume.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,7 @@ int s2n_resume_from_cache(struct s2n_connection *conn)
struct s2n_stuffer from = { 0 };
POSIX_GUARD(s2n_stuffer_init(&from, &entry));
POSIX_GUARD(s2n_stuffer_write(&from, &entry));
POSIX_GUARD_RESULT(s2n_resume_decrypt_session_cache(conn, &from));
POSIX_GUARD_RESULT(s2n_resume_decrypt_session(conn, &from));

return 0;
}
Expand Down Expand Up @@ -752,7 +752,7 @@ struct s2n_ticket_key *s2n_get_ticket_encrypt_decrypt_key(struct s2n_config *con
return ticket_key;
}

/* This function is used in s2n_resume_decrypt_session_ticket in order for s2n to
/* This function is used in s2n_resume_decrypt_session in order for s2n to
* find the matching key that was used for encryption.
*/
struct s2n_ticket_key *s2n_find_ticket_key(struct s2n_config *config, const uint8_t name[S2N_TICKET_KEY_NAME_LEN])
Expand Down Expand Up @@ -889,13 +889,11 @@ S2N_RESULT s2n_resume_encrypt_session_ticket(struct s2n_connection *conn,
return S2N_RESULT_OK;
}

static S2N_RESULT s2n_resume_decrypt_session(struct s2n_connection *conn, struct s2n_stuffer *from,
uint64_t *key_intro_time)
S2N_RESULT s2n_resume_decrypt_session(struct s2n_connection *conn, struct s2n_stuffer *from)
{
RESULT_ENSURE_REF(conn);
RESULT_ENSURE_REF(from);
RESULT_ENSURE_REF(conn->config);
RESULT_ENSURE_REF(key_intro_time);

/* Read version number */
uint8_t version = 0;
Expand Down Expand Up @@ -956,40 +954,6 @@ static S2N_RESULT s2n_resume_decrypt_session(struct s2n_connection *conn, struct
RESULT_GUARD_POSIX(s2n_stuffer_skip_write(&state_stuffer, state_blob_size));
RESULT_GUARD(s2n_deserialize_resumption_state(conn, &from->blob, &state_stuffer));

/* Store this key timestamp for session ticket logic */
*key_intro_time = key->intro_timestamp;

return S2N_RESULT_OK;
}

S2N_RESULT s2n_resume_decrypt_session_ticket(struct s2n_connection *conn, struct s2n_stuffer *from)
{
RESULT_ENSURE_REF(conn);
RESULT_ENSURE_REF(conn->config);

uint64_t key_intro_time = 0;
RESULT_GUARD(s2n_resume_decrypt_session(conn, from, &key_intro_time));

if (s2n_connection_get_protocol_version(conn) >= S2N_TLS13) {
return S2N_RESULT_OK;
}

/* A new key is assigned for the ticket if the key used to encrypt current ticket is expired */
uint64_t now = 0;
RESULT_GUARD(s2n_config_wall_clock(conn->config, &now));
if (now >= key_intro_time + conn->config->encrypt_decrypt_key_lifetime_in_nanos) {
if (s2n_result_is_ok(s2n_config_is_encrypt_key_available(conn->config))) {
conn->session_ticket_status = S2N_NEW_TICKET;
RESULT_GUARD(s2n_handshake_type_set_tls12_flag(conn, WITH_SESSION_TICKET));
}
}
return S2N_RESULT_OK;
}

S2N_RESULT s2n_resume_decrypt_session_cache(struct s2n_connection *conn, struct s2n_stuffer *from)
{
uint64_t key_intro_time = 0;
RESULT_GUARD(s2n_resume_decrypt_session(conn, from, &key_intro_time));
return S2N_RESULT_OK;
}

Expand Down
Loading
Loading