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.

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

Merged
merged 53 commits into from
Feb 6, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 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
1075f98
address PR comments:
Jan 22, 2025
cfc37af
address PR comments:
Jan 22, 2025
ba5cd74
address PR comments:
Jan 23, 2025
15410c5
Make TLS1.2 nst recv compliant with the RFC:
Jan 24, 2025
e4f723f
refactors to fix format
Jan 24, 2025
75fa5f2
fix cppcheck and remove specific sections to the RFC reference issue:
Jan 27, 2025
c50ccf4
assert key is not null
Jan 27, 2025
fc93562
remove changes for clients
Jan 27, 2025
dbf6562
improve the lifetime calculation logic
Jan 27, 2025
52dd19c
Manually set key_intro_time and current_time for lifetime test
Jan 28, 2025
5c6d9c2
fix clang-format issue
Jan 28, 2025
e421dbc
Simply logics and remove key query
Jan 28, 2025
56dca1b
Address PR comments:
Jan 29, 2025
e6458fb
Address PR comments:
Jan 29, 2025
96db831
aquire ticket key and handle TLS1.2 guard
Jan 30, 2025
cd67519
address clang format issue
Jan 31, 2025
769d548
fix grep simple mistake error
Jan 31, 2025
f01090a
code refactoring for s2n_server_nst_send
Jan 31, 2025
c289568
add unit test coverage for changes in generate lifetime ticket
Feb 3, 2025
c892c23
refactor: fix s2n_server_new_session_ticket_test
Feb 4, 2025
9048d74
address PR comments
Feb 4, 2025
dc22ffb
address pr comments:
Feb 4, 2025
4d1cce5
address PR comments:
Feb 5, 2025
07f8123
address PR comments
Feb 5, 2025
f3a2153
fix macro name
Feb 5, 2025
a659636
fix clang-format issue
Feb 5, 2025
dc9b225
Merge branch 'main' into session-ticket-age
boquan-fang Feb 5, 2025
4cea9a4
Address PR comments:
Feb 5, 2025
84d00b3
address pr comments for nits
Feb 5, 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
1 change: 1 addition & 0 deletions error/s2n_errno.c
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ static const char *no_such_error = "Internal s2n error";
ERR_ENTRY(S2N_ERR_DUPLICATE_PSK_IDENTITIES, "The list of pre-shared keys provided contains duplicate psk identities") \
ERR_ENTRY(S2N_ERR_OFFERED_PSKS_TOO_LONG, "The total pre-shared key data is too long to send over the wire") \
ERR_ENTRY(S2N_ERR_INVALID_SESSION_TICKET, "Session ticket data is not valid") \
ERR_ENTRY(S2N_ERR_ZERO_LIFETIME_TICKET, "Calculated session lifetime is zero") \
ERR_ENTRY(S2N_ERR_REENTRANCY, "Original execution must complete before method can be called again") \
ERR_ENTRY(S2N_ERR_INVALID_CERT_STATE, "Certificate validation entered an invalid state and is not able to continue") \
ERR_ENTRY(S2N_ERR_INVALID_EARLY_DATA_STATE, "Early data in invalid state") \
Expand Down
1 change: 1 addition & 0 deletions error/s2n_errno.h
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,7 @@ typedef enum {
S2N_ERR_BAD_HEX,
S2N_ERR_TEST_ASSERTION,
S2N_ERR_CONFIG_NULL_BEFORE_CH_CALLBACK,
S2N_ERR_ZERO_LIFETIME_TICKET,
S2N_ERR_T_INTERNAL_END,

/* S2N_ERR_T_USAGE */
Expand Down
6 changes: 5 additions & 1 deletion tests/unit/s2n_client_psk_extension_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,12 @@ static S2N_RESULT s2n_setup_encrypted_ticket(struct s2n_connection *conn, struct
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));

struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
RESULT_ENSURE(key != NULL, S2N_ERR_NO_TICKET_ENCRYPT_DECRYPT_KEY);

/* Create a valid resumption psk identity */
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, output));
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, key, output));

output->blob.size = s2n_stuffer_data_available(output);

return S2N_RESULT_OK;
Expand Down
15 changes: 12 additions & 3 deletions tests/unit/s2n_extended_master_secret_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,11 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_blob_init(&ticket_blob, ticket_data, S2N_TLS12_TICKET_SIZE_IN_BYTES));
EXPECT_SUCCESS(s2n_stuffer_init(&ticket, &ticket_blob));

struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
EXPECT_NOT_NULL(key);

/* Encrypt the ticket with EMS data */
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &ticket));
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, key, &ticket));

EXPECT_SUCCESS(s2n_connection_wipe(conn));
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));
Expand Down Expand Up @@ -88,8 +91,11 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_blob_init(&ticket_blob, ticket_data, S2N_TLS12_TICKET_SIZE_IN_BYTES));
EXPECT_SUCCESS(s2n_stuffer_init(&ticket, &ticket_blob));

struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
EXPECT_NOT_NULL(key);

/* Encrypt the ticket without EMS data */
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &ticket));
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, key, &ticket));

EXPECT_SUCCESS(s2n_connection_wipe(conn));
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));
Expand Down Expand Up @@ -125,8 +131,11 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_blob_init(&ticket_blob, ticket_data, S2N_TLS12_TICKET_SIZE_IN_BYTES));
EXPECT_SUCCESS(s2n_stuffer_init(&ticket, &ticket_blob));

struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
EXPECT_NOT_NULL(key);

/* Encrypt the ticket with EMS data */
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &ticket));
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, key, &ticket));

EXPECT_SUCCESS(s2n_connection_wipe(conn));
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));
Expand Down
5 changes: 4 additions & 1 deletion tests/unit/s2n_psk_offered_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,11 @@ static S2N_RESULT s2n_setup_encrypted_ticket(struct s2n_connection *conn, struct
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));

struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
RESULT_ENSURE(key != NULL, S2N_ERR_NO_TICKET_ENCRYPT_DECRYPT_KEY);

/* Create a valid resumption psk identity */
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, output));
RESULT_GUARD(s2n_resume_encrypt_session_ticket(conn, key, output));
output->blob.size = s2n_stuffer_data_available(output);

return S2N_RESULT_OK;
Expand Down
39 changes: 30 additions & 9 deletions tests/unit/s2n_resume_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -1298,7 +1298,7 @@ int main(int argc, char **argv)
DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(conn);
EXPECT_ERROR_WITH_ERRNO(s2n_resume_encrypt_session_ticket(conn, &conn->client_ticket_to_decrypt),
EXPECT_ERROR_WITH_ERRNO(s2n_resume_encrypt_session_ticket(conn, NULL, &conn->client_ticket_to_decrypt),
S2N_ERR_NO_TICKET_ENCRYPT_DECRYPT_KEY);
}

Expand All @@ -1314,8 +1314,11 @@ int main(int argc, char **argv)
EXPECT_OK(s2n_resumption_test_ticket_key_setup(config));
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));

struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
EXPECT_NOT_NULL(key);

struct s2n_stuffer output = { 0 };
EXPECT_ERROR_WITH_ERRNO(s2n_resume_encrypt_session_ticket(conn, &output), S2N_ERR_STUFFER_IS_FULL);
EXPECT_ERROR_WITH_ERRNO(s2n_resume_encrypt_session_ticket(conn, key, &output), S2N_ERR_STUFFER_IS_FULL);
}

/* Check encrypted data can be decrypted correctly for TLS12 */
Expand All @@ -1337,7 +1340,10 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_stuffer_write_bytes(&secret_stuffer, test_master_secret.data, S2N_TLS_SECRET_LEN));
conn->secure->cipher_suite = &s2n_ecdhe_ecdsa_with_aes_128_gcm_sha256;

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &conn->client_ticket_to_decrypt));
struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
EXPECT_NOT_NULL(key);

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, key, &conn->client_ticket_to_decrypt));
EXPECT_NOT_EQUAL(s2n_stuffer_data_available(&conn->client_ticket_to_decrypt), 0);

/* Wiping the master secret to prove that the decryption function actually writes the master secret */
Expand Down Expand Up @@ -1374,7 +1380,10 @@ int main(int argc, char **argv)
/* This secret is smaller than the maximum secret length */
EXPECT_TRUE(conn->tls13_ticket_fields.session_secret.size < S2N_TLS_SECRET_LEN);

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &output));
struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
EXPECT_NOT_NULL(key);

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

EXPECT_EQUAL(s2n_stuffer_data_available(&output), 0);
Expand Down Expand Up @@ -1410,7 +1419,10 @@ int main(int argc, char **argv)
/* This secret is equal to the maximum secret length */
EXPECT_EQUAL(conn->tls13_ticket_fields.session_secret.size, S2N_TLS_SECRET_LEN);

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &output));
struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
EXPECT_NOT_NULL(key);

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

EXPECT_EQUAL(s2n_stuffer_data_available(&output), 0);
Expand Down Expand Up @@ -1438,7 +1450,10 @@ int main(int argc, char **argv)
conn->actual_protocol_version = S2N_TLS12;
conn->handshake.handshake_type = NEGOTIATED;

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &conn->client_ticket_to_decrypt));
struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
EXPECT_NOT_NULL(key);

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, key, &conn->client_ticket_to_decrypt));
EXPECT_NOT_EQUAL(s2n_stuffer_data_available(&conn->client_ticket_to_decrypt), 0);

/* Modify the version number of the ticket */
Expand Down Expand Up @@ -1468,9 +1483,12 @@ int main(int argc, char **argv)
conn->actual_protocol_version = S2N_TLS12;
conn->handshake.handshake_type = NEGOTIATED;

struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
EXPECT_NOT_NULL(key);

DEFER_CLEANUP(struct s2n_stuffer valid_ticket = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&valid_ticket, 0));
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &valid_ticket));
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, key, &valid_ticket));
uint32_t ticket_size = s2n_stuffer_data_available(&valid_ticket);

/* Copy ticket so that we don't modify the original ticket */
Expand Down Expand Up @@ -1521,7 +1539,7 @@ int main(int argc, char **argv)
DEFER_CLEANUP(struct s2n_stuffer output = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&output, 0));

EXPECT_ERROR_WITH_ERRNO(s2n_resume_encrypt_session_ticket(conn, &output), S2N_ERR_KEY_CHECK);
EXPECT_ERROR_WITH_ERRNO(s2n_resume_encrypt_session_ticket(conn, key, &output), S2N_ERR_KEY_CHECK);
};

/* Check session ticket is correct when using early data with TLS1.3. */
Expand All @@ -1548,7 +1566,10 @@ int main(int argc, char **argv)
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));

EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &output));
struct s2n_ticket_key *key = s2n_get_ticket_encrypt_decrypt_key(conn->config);
EXPECT_NOT_NULL(key);

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

EXPECT_EQUAL(s2n_stuffer_data_available(&output), 0);
Expand Down
Loading
Loading