Skip to content
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

fix: calculation of session ticket age #5001

Open
wants to merge 24 commits into
base: main
Choose a base branch
from
+232 −96
Open

fix: calculation of session ticket age #5001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
54631eb
fix: improper session ticket age calculation
Jan 3, 2025
a5e1181
fix: s2n_generate_ticket_lifetime function and tests
Jan 6, 2025
5d6cd41
Fix formats and add checks for keys in s2n_resume_test
Jan 7, 2025
91e6379
Roll back changes for s2n_resume_encrypt_session_ticket
Jan 7, 2025
5906150
refactor changes for format
Jan 7, 2025
0f47df4
Add config cleanup to fix memory issue
Jan 7, 2025
b13143e
add RESULT_ENSURE_REF check for input key
Jan 7, 2025
97f2ec1
fix the MIN comparison for min lifetime
Jan 7, 2025
bd9f4b2
address PR comments:
Jan 8, 2025
f59b6bf
refactor: record current time and retrieve key intro time for TLS1.3
Jan 10, 2025
0eb5525
make s2n_get_ticket_encrypt_decrypt_key not visible
Jan 11, 2025
b31f110
refactor: change the tls13_ticket_fields varaible names to ticket_fields
Jan 13, 2025
938eb66
fix: calculation of session ticket age for TLS1.2
Jan 13, 2025
1acad6b
refactor: prevent sending zero lifetime new session ticket for TLS1.2
Jan 9, 2025
6f2387a
refactor: prevent sending zero lifetime new session ticket for TLS1.3
Jan 13, 2025
ab28660
refactor: remove errno changes
Jan 13, 2025
7332abd
refactor: use s2n_find_ticket_key to get session ticket key
Jan 14, 2025
e80719b
fix: clang-format issue
Jan 14, 2025
15f08b2
fix: grep simple mistake error
Jan 14, 2025
edc611c
refactor: add key_intro_time to the ticket field
Jan 14, 2025
c6b7d78
refactor: improve generate lifetime ticket lifetime logic
Jan 14, 2025
e940156
address PR comments:
Jan 15, 2025
3e356d2
address PR comments:
Jan 21, 2025
1320b95
fix clang-format issue
Jan 21, 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
2 changes: 2 additions & 0 deletions stuffer/s2n_stuffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
#define SIZEOF_IN_BITS(t) (sizeof(t) * CHAR_BIT)

#define SIZEOF_UINT24 3
#define SIZEOF_UINT32 4

struct s2n_stuffer {
/* The data for the s2n_stuffer */
Expand Down Expand Up @@ -143,6 +144,7 @@ S2N_RESULT s2n_stuffer_reservation_validate(const struct s2n_stuffer_reservation
int S2N_RESULT_MUST_USE s2n_stuffer_reserve_uint8(struct s2n_stuffer *stuffer, struct s2n_stuffer_reservation *reservation);
int S2N_RESULT_MUST_USE s2n_stuffer_reserve_uint16(struct s2n_stuffer *stuffer, struct s2n_stuffer_reservation *reservation);
int S2N_RESULT_MUST_USE s2n_stuffer_reserve_uint24(struct s2n_stuffer *stuffer, struct s2n_stuffer_reservation *reservation);
int S2N_RESULT_MUST_USE s2n_stuffer_reserve_uint32(struct s2n_stuffer *stuffer, struct s2n_stuffer_reservation *reservation);
int S2N_RESULT_MUST_USE s2n_stuffer_write_reservation(struct s2n_stuffer_reservation *reservation, const uint32_t value);
/* Reservations are primarily intended to handle the variable-length vector type
* defined in the RFC: https://tools.ietf.org/html/rfc8446#section-3.4
Expand Down
5 changes: 5 additions & 0 deletions stuffer/s2n_stuffer_network_order.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ int s2n_stuffer_reserve_uint24(struct s2n_stuffer *stuffer, struct s2n_stuffer_r
return s2n_stuffer_reserve(stuffer, reservation, SIZEOF_UINT24);
}

int s2n_stuffer_reserve_uint32(struct s2n_stuffer *stuffer, struct s2n_stuffer_reservation *reservation)
{
return s2n_stuffer_reserve(stuffer, reservation, SIZEOF_UINT32);
}

int s2n_stuffer_read_uint32(struct s2n_stuffer *stuffer, uint32_t *u)
{
POSIX_ENSURE_REF(u);
Expand Down
2 changes: 1 addition & 1 deletion tests/cbmc/sources/make_common_datastructures.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ void cbmc_populate_s2n_connection(struct s2n_connection *s2n_connection)
s2n_connection->verify_host_fn = malloc(sizeof(*(s2n_connection->verify_host_fn))); /* Function pointer. */
s2n_connection->data_for_verify_host = malloc(sizeof(*(s2n_connection->data_for_verify_host)));
cbmc_populate_s2n_blob(&(s2n_connection->client_ticket));
cbmc_populate_s2n_ticket_fields(&(s2n_connection->tls13_ticket_fields));
cbmc_populate_s2n_ticket_fields(&(s2n_connection->ticket_fields));
cbmc_populate_s2n_stuffer(&(s2n_connection->client_ticket_to_decrypt));
cbmc_populate_s2n_blob(&(s2n_connection->application_protocols_overridden));
cbmc_populate_s2n_blob(&(s2n_connection->cookie));
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/s2n_client_psk_extension_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ static int mock_time(void *data, uint64_t *nanoseconds)

static S2N_RESULT s2n_setup_encrypted_ticket(struct s2n_connection *conn, struct s2n_stuffer *output)
{
conn->tls13_ticket_fields = (struct s2n_ticket_fields){ 0 };
conn->ticket_fields = (struct s2n_ticket_fields){ 0 };
uint8_t test_secret_data[] = "test secret";
RESULT_GUARD_POSIX(s2n_alloc(&conn->tls13_ticket_fields.session_secret, sizeof(test_secret_data)));
RESULT_CHECKED_MEMCPY(conn->tls13_ticket_fields.session_secret.data, test_secret_data, sizeof(test_secret_data));
RESULT_GUARD_POSIX(s2n_alloc(&conn->ticket_fields.session_secret, sizeof(test_secret_data)));
RESULT_CHECKED_MEMCPY(conn->ticket_fields.session_secret.data, test_secret_data, sizeof(test_secret_data));

/* Create a valid resumption psk identity */
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, output));
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/s2n_psk_offered_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@

static S2N_RESULT s2n_setup_encrypted_ticket(struct s2n_connection *conn, struct s2n_stuffer *output)
{
conn->tls13_ticket_fields = (struct s2n_ticket_fields){ 0 };
conn->ticket_fields = (struct s2n_ticket_fields){ 0 };
uint8_t test_secret_data[] = "test secret";
RESULT_GUARD_POSIX(s2n_alloc(&conn->tls13_ticket_fields.session_secret, sizeof(test_secret_data)));
RESULT_CHECKED_MEMCPY(conn->tls13_ticket_fields.session_secret.data, test_secret_data, sizeof(test_secret_data));
RESULT_GUARD_POSIX(s2n_alloc(&conn->ticket_fields.session_secret, sizeof(test_secret_data)));
RESULT_CHECKED_MEMCPY(conn->ticket_fields.session_secret.data, test_secret_data, sizeof(test_secret_data));

/* Create a valid resumption psk identity */
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, output));
Expand Down
56 changes: 28 additions & 28 deletions tests/unit/s2n_resume_test.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ int main(int argc, char **argv)
/* Set non-zero length secret */
uint8_t secret_size = 0;
EXPECT_SUCCESS(s2n_hmac_digest_size(conn->secure->cipher_suite->prf_alg, &secret_size));
EXPECT_SUCCESS(s2n_realloc(&conn->tls13_ticket_fields.session_secret, secret_size));
EXPECT_SUCCESS(s2n_realloc(&conn->ticket_fields.session_secret, secret_size));

/* Result matches constant */
size_t actual_size = 0;
Expand Down Expand Up @@ -206,7 +206,7 @@ int main(int argc, char **argv)
/* Set non-zero length secret */
uint8_t secret_size = 0;
EXPECT_SUCCESS(s2n_hmac_digest_size(conn->secure->cipher_suite->prf_alg, &secret_size));
EXPECT_SUCCESS(s2n_realloc(&conn->tls13_ticket_fields.session_secret, secret_size));
EXPECT_SUCCESS(s2n_realloc(&conn->ticket_fields.session_secret, secret_size));

/* Result matches constant */
size_t actual_size = 0;
Expand All @@ -232,7 +232,7 @@ int main(int argc, char **argv)
/* Set non-zero length secret */
uint8_t secret_size = 0;
EXPECT_SUCCESS(s2n_hmac_digest_size(conn->secure->cipher_suite->prf_alg, &secret_size));
EXPECT_SUCCESS(s2n_alloc(&conn->tls13_ticket_fields.session_secret, secret_size));
EXPECT_SUCCESS(s2n_alloc(&conn->ticket_fields.session_secret, secret_size));

/* Result matches constants */
size_t actual_size = 0;
Expand Down Expand Up @@ -260,7 +260,7 @@ int main(int argc, char **argv)
/* Set non-zero length secret */
uint8_t secret_size = 0;
EXPECT_SUCCESS(s2n_hmac_digest_size(conn->secure->cipher_suite->prf_alg, &secret_size));
EXPECT_SUCCESS(s2n_alloc(&conn->tls13_ticket_fields.session_secret, secret_size));
EXPECT_SUCCESS(s2n_alloc(&conn->ticket_fields.session_secret, secret_size));

/* Set early data fields */
const uint8_t data[] = "test data";
Expand Down Expand Up @@ -324,7 +324,7 @@ int main(int argc, char **argv)
/* Set non-zero length secret */
uint8_t secret_size = 0;
EXPECT_SUCCESS(s2n_hmac_digest_size(conn->secure->cipher_suite->prf_alg, &secret_size));
EXPECT_SUCCESS(s2n_alloc(&conn->tls13_ticket_fields.session_secret, secret_size));
EXPECT_SUCCESS(s2n_alloc(&conn->ticket_fields.session_secret, secret_size));
int session_length = s2n_connection_get_session_length(conn);
EXPECT_NOT_EQUAL(session_length, 0);

Expand Down Expand Up @@ -473,8 +473,8 @@ int main(int argc, char **argv)
DEFER_CLEANUP(struct s2n_stuffer output = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&output, 0));

conn->tls13_ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->tls13_ticket_fields.session_secret));
conn->ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->ticket_fields.session_secret));

EXPECT_OK(s2n_tls13_serialize_resumption_state(conn, &output));

Expand All @@ -496,11 +496,11 @@ int main(int argc, char **argv)

uint32_t ticket_age_add = 0;
EXPECT_SUCCESS(s2n_stuffer_read_uint32(&output, &ticket_age_add));
EXPECT_EQUAL(ticket_age_add, conn->tls13_ticket_fields.ticket_age_add);
EXPECT_EQUAL(ticket_age_add, conn->ticket_fields.ticket_age_add);

uint8_t secret_len = 0;
EXPECT_SUCCESS(s2n_stuffer_read_uint8(&output, &secret_len));
EXPECT_EQUAL(secret_len, conn->tls13_ticket_fields.session_secret.size);
EXPECT_EQUAL(secret_len, conn->ticket_fields.session_secret.size);

uint8_t session_secret[S2N_TLS_SECRET_LEN] = { 0 };
EXPECT_SUCCESS(s2n_stuffer_read_bytes(&output, session_secret, secret_len));
Expand Down Expand Up @@ -531,8 +531,8 @@ int main(int argc, char **argv)
DEFER_CLEANUP(struct s2n_stuffer output = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&output, 0));

conn->tls13_ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->tls13_ticket_fields.session_secret));
conn->ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->ticket_fields.session_secret));

/* New expiration time */
{
Expand Down Expand Up @@ -612,8 +612,8 @@ int main(int argc, char **argv)
DEFER_CLEANUP(struct s2n_stuffer output = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&output, 0));

conn->tls13_ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->tls13_ticket_fields.session_secret));
conn->ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->ticket_fields.session_secret));

/* Write ticket without early data. Save size for comparison. */
EXPECT_SUCCESS(s2n_connection_set_server_max_early_data_size(conn, 0));
Expand Down Expand Up @@ -687,10 +687,10 @@ int main(int argc, char **argv)
DEFER_CLEANUP(struct s2n_stuffer output = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&output, 0));

conn->tls13_ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->tls13_ticket_fields.session_secret));
conn->ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->ticket_fields.session_secret));

EXPECT_SUCCESS(s2n_realloc(&conn->tls13_ticket_fields.session_secret, test_cases[i].secret_size));
EXPECT_SUCCESS(s2n_realloc(&conn->ticket_fields.session_secret, test_cases[i].secret_size));
if (test_cases[i].success) {
EXPECT_OK(s2n_tls13_serialize_resumption_state(conn, &output));
} else {
Expand Down Expand Up @@ -1137,8 +1137,8 @@ int main(int argc, char **argv)
DEFER_CLEANUP(struct s2n_stuffer stuffer = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&stuffer, 0));

conn->tls13_ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = TICKET_AGE_ADD };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->tls13_ticket_fields.session_secret));
conn->ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = TICKET_AGE_ADD };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->ticket_fields.session_secret));

/* Initialize client ticket */
uint8_t client_ticket[] = { CLIENT_TICKET };
Expand Down Expand Up @@ -1194,8 +1194,8 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&stuffer, 0));

/* Initialize client ticket */
conn->tls13_ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = TICKET_AGE_ADD };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->tls13_ticket_fields.session_secret));
conn->ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = TICKET_AGE_ADD };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->ticket_fields.session_secret));
uint8_t client_ticket[] = { CLIENT_TICKET };
EXPECT_SUCCESS(s2n_realloc(&conn->client_ticket, sizeof(client_ticket)));
EXPECT_MEMCPY_SUCCESS(conn->client_ticket.data, client_ticket, sizeof(client_ticket));
Expand Down Expand Up @@ -1368,11 +1368,11 @@ int main(int argc, char **argv)

DEFER_CLEANUP(struct s2n_stuffer output = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&output, 0));
conn->tls13_ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->tls13_ticket_fields.session_secret));
conn->ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_session_secret, &conn->ticket_fields.session_secret));

/* This secret is smaller than the maximum secret length */
EXPECT_TRUE(conn->tls13_ticket_fields.session_secret.size < S2N_TLS_SECRET_LEN);
EXPECT_TRUE(conn->ticket_fields.session_secret.size < S2N_TLS_SECRET_LEN);

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &output));
Expand Down Expand Up @@ -1404,11 +1404,11 @@ int main(int argc, char **argv)

DEFER_CLEANUP(struct s2n_stuffer output = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&output, 0));
conn->tls13_ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_master_secret, &conn->tls13_ticket_fields.session_secret));
conn->ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_master_secret, &conn->ticket_fields.session_secret));

/* This secret is equal to the maximum secret length */
EXPECT_EQUAL(conn->tls13_ticket_fields.session_secret.size, S2N_TLS_SECRET_LEN);
EXPECT_EQUAL(conn->ticket_fields.session_secret.size, S2N_TLS_SECRET_LEN);

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &output));
Expand Down Expand Up @@ -1545,8 +1545,8 @@ int main(int argc, char **argv)

DEFER_CLEANUP(struct s2n_stuffer output = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&output, 0));
conn->tls13_ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_master_secret, &conn->tls13_ticket_fields.session_secret));
conn->ticket_fields = (struct s2n_ticket_fields){ .ticket_age_add = 1 };
EXPECT_SUCCESS(s2n_dup(&test_master_secret, &conn->ticket_fields.session_secret));

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &output));
Expand Down
Loading
Loading