Skip to content

Commit

Permalink
fix: Refactor some s2n_resume functions (#4648)
Browse files Browse the repository at this point in the history
  • Loading branch information
maddeleine authored Jul 23, 2024
1 parent 2a09c57 commit c3a5680
Show file tree
Hide file tree
Showing 12 changed files with 168 additions and 171 deletions.
6 changes: 3 additions & 3 deletions tests/saw/spec/handshake/handshake.saw
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_decrypt_session_ticket";
s2n_decrypt_session_ticket <- crucible_llvm_unsafe_assume_spec llvm "s2n_decrypt_session_ticket" s2n_decrypt_session_ticket_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;
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_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_ticket];
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
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_decrypt_session_ticket_spec. This is essentially
// Specification for s2n_resume_decrypt_session_ticket_spec. This is essentially
// a noop function that returns 0 from the perspective of our current proof
let s2n_decrypt_session_ticket_spec = do {
let s2n_resume_decrypt_session_ticket_spec = do {
pconn <- crucible_alloc_readonly (llvm_struct "struct.s2n_connection");
from <- crucible_alloc_readonly (llvm_struct "struct.s2n_stuffer");

Expand Down
2 changes: 1 addition & 1 deletion tests/unit/s2n_client_psk_extension_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ static S2N_RESULT s2n_setup_encrypted_ticket(struct s2n_connection *conn, struct
RESULT_CHECKED_MEMCPY(conn->tls13_ticket_fields.session_secret.data, test_secret_data, sizeof(test_secret_data));

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

return S2N_RESULT_OK;
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/s2n_extended_master_secret_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_stuffer_init(&ticket, &ticket_blob));

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

EXPECT_SUCCESS(s2n_connection_wipe(conn));
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));
Expand Down Expand Up @@ -89,7 +89,7 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_stuffer_init(&ticket, &ticket_blob));

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

EXPECT_SUCCESS(s2n_connection_wipe(conn));
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));
Expand Down Expand Up @@ -126,7 +126,7 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_stuffer_init(&ticket, &ticket_blob));

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

EXPECT_SUCCESS(s2n_connection_wipe(conn));
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/s2n_psk_offered_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ static S2N_RESULT s2n_setup_encrypted_ticket(struct s2n_connection *conn, struct
RESULT_CHECKED_MEMCPY(conn->tls13_ticket_fields.session_secret.data, test_secret_data, sizeof(test_secret_data));

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

return S2N_RESULT_OK;
Expand Down
50 changes: 40 additions & 10 deletions tests/unit/s2n_resume_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -1281,13 +1281,43 @@ int main(int argc, char **argv)
};
};

/* s2n_encrypt_session_ticket */
/* s2n_resume_encrypt_session_ticket */
{
/* Session ticket keys. Taken from test vectors in https://tools.ietf.org/html/rfc5869 */
uint8_t ticket_key_name[16] = "2016.07.26.15\0";
S2N_BLOB_FROM_HEX(ticket_key,
"077709362c2e32df0ddc3f0dc47bba6390b6c73bb50f9c3122ec844ad7c2b3e5");

/* Check error is thrown when no ticket key is available */
{
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),
S2N_ERR_NO_TICKET_ENCRYPT_DECRYPT_KEY);
}

/* Check error is thrown when stuffer is out of memory for the ticket */
{
DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(config);

/* Adds a valid ticket encryption key */
EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(config, 1));
uint64_t current_time = 0;
EXPECT_SUCCESS(config->wall_clock(config->sys_clock_ctx, &current_time));
EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(config, ticket_key_name, strlen((char *) ticket_key_name),
ticket_key.data, ticket_key.size, current_time / ONE_SEC_IN_NANOS));

DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(conn);
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));

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

/* Check encrypted data can be decrypted correctly for TLS12 */
{
struct s2n_connection *conn = NULL;
Expand All @@ -1312,13 +1342,13 @@ 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_SUCCESS(s2n_encrypt_session_ticket(conn, &conn->client_ticket_to_decrypt));
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &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 */
memset(conn->secrets.version.tls12.master_secret, 0, test_master_secret.size);

EXPECT_SUCCESS(s2n_decrypt_session_ticket(conn, &conn->client_ticket_to_decrypt));
EXPECT_OK(s2n_resume_decrypt_session_ticket(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 @@ -1355,8 +1385,8 @@ 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_SUCCESS(s2n_encrypt_session_ticket(conn, &output));
EXPECT_SUCCESS(s2n_decrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &output));

EXPECT_EQUAL(s2n_stuffer_data_available(&output), 0);

Expand Down Expand Up @@ -1397,8 +1427,8 @@ 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_SUCCESS(s2n_encrypt_session_ticket(conn, &output));
EXPECT_SUCCESS(s2n_decrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &output));

EXPECT_EQUAL(s2n_stuffer_data_available(&output), 0);

Expand Down Expand Up @@ -1435,7 +1465,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_FAILURE_WITH_ERRNO(s2n_encrypt_session_ticket(conn, &output), S2N_ERR_KEY_CHECK);
EXPECT_ERROR_WITH_ERRNO(s2n_resume_encrypt_session_ticket(conn, &output), S2N_ERR_KEY_CHECK);
};

/* Check session ticket is correct when using early data with TLS1.3. */
Expand Down Expand Up @@ -1466,8 +1496,8 @@ 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_SUCCESS(s2n_encrypt_session_ticket(conn, &output));
EXPECT_SUCCESS(s2n_decrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_encrypt_session_ticket(conn, &output));
EXPECT_OK(s2n_resume_decrypt_session_ticket(conn, &output));

EXPECT_EQUAL(s2n_stuffer_data_available(&output), 0);

Expand Down
8 changes: 4 additions & 4 deletions tests/unit/s2n_session_ticket_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -1087,7 +1087,7 @@ 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 */
/* s2n_resume_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());
Expand All @@ -1107,13 +1107,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_FAILURE_WITH_ERRNO(s2n_decrypt_session_ticket(server_conn, &server_conn->client_ticket_to_decrypt), S2N_ERR_DECRYPT);
EXPECT_ERROR_WITH_ERRNO(s2n_resume_decrypt_session_ticket(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_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_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());
Expand All @@ -1133,7 +1133,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_FAILURE_WITH_ERRNO(s2n_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_ticket(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
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_decrypt_session_ticket(conn, &conn->client_ticket_to_decrypt) == S2N_SUCCESS) {
if (s2n_result_is_ok(s2n_resume_decrypt_session_ticket(conn, &conn->client_ticket_to_decrypt))) {
return S2N_SUCCESS;
}

Expand Down
4 changes: 2 additions & 2 deletions tls/s2n_psk.c
Original file line number Diff line number Diff line change
Expand Up @@ -326,8 +326,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_decrypt_session_ticket appends a new PSK with the decrypted values. */
POSIX_GUARD(s2n_decrypt_session_ticket(psk_list->conn, &ticket_stuffer));
/* 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));
}

struct s2n_psk *chosen_psk = NULL;
Expand Down
Loading

0 comments on commit c3a5680

Please sign in to comment.