Skip to content

Commit

Permalink
Trying to use an invalid ticket should not mutate state (aws#4110)
Browse files Browse the repository at this point in the history
  • Loading branch information
@lrstewart
lrstewart committed Jul 25, 2023
1 parent bce2b1a commit 403d5e6
Show file tree
Hide file tree
Showing 4 changed files with 270 additions and 79 deletions.
3 changes: 3 additions & 0 deletions api/s2n.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2214,6 +2214,9 @@ S2N_API extern int s2n_session_ticket_get_lifetime(struct s2n_session_ticket *ti
/**
* De-serializes the session state and updates the connection accordingly.
*
* If this method fails, the connection should not be affected: calling s2n_negotiate
* with the connection should simply result in a full handshake.
*
* @param conn A pointer to the s2n_connection object
* @param session A pointer to a buffer of size `length`
* @param length The size of the `session` buffer
Expand Down
284 changes: 224 additions & 60 deletions tests/unit/s2n_resume_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,66 @@ int main(int argc, char **argv)
"18df06843d13a08bf2a449844c5f8a"
"478001bc4d4c627984d5a41da8d0402919");

uint8_t tls12_ticket[S2N_TLS12_STATE_SIZE_IN_BYTES_WITHOUT_EMS] = {
S2N_SERIALIZED_FORMAT_TLS12_V1,
S2N_TLS12,
TLS_RSA_WITH_AES_128_GCM_SHA256,
TICKET_ISSUE_TIME_BYTES,
};

uint8_t tls12_ticket_with_ems[S2N_TLS12_STATE_SIZE_IN_BYTES] = {
S2N_SERIALIZED_FORMAT_TLS12_V3,
S2N_TLS12,
TLS_RSA_WITH_AES_128_GCM_SHA256,
TICKET_ISSUE_TIME_BYTES,
};

uint8_t tls13_ticket[] = {
S2N_SERIALIZED_FORMAT_TLS13_V1,
S2N_TLS13,
TLS_AES_128_GCM_SHA256,
TICKET_ISSUE_TIME_BYTES,
TICKET_AGE_ADD_BYTES,
SECRET_LEN,
SECRET,
EMPTY_EARLY_DATA_SIZE,
};

uint8_t tls13_server_ticket[] = {
S2N_SERIALIZED_FORMAT_TLS13_V1,
S2N_TLS13,
TLS_AES_128_GCM_SHA256,
TICKET_ISSUE_TIME_BYTES,
TICKET_AGE_ADD_BYTES,
SECRET_LEN,
SECRET,
KEYING_MATERIAL_EXPIRATION_BYTES,
EMPTY_EARLY_DATA_SIZE,
};

uint8_t tls13_ticket_with_early_data[] = {
S2N_SERIALIZED_FORMAT_TLS13_V1,
S2N_TLS13,
TLS_AES_128_GCM_SHA256,
TICKET_ISSUE_TIME_BYTES,
TICKET_AGE_ADD_BYTES,
SECRET_LEN,
SECRET,
0x00,
0x00,
0x00,
NONEMPTY_EARLY_DATA_SIZE,
APP_PROTOCOL_LEN,
APP_PROTOCOL,
0x00,
EARLY_DATA_CONTEXT_LEN,
EARLY_DATA_CONTEXT,
};

uint8_t faulty_format_ticket[] = {
0xFF,
};

/* s2n_connection_get_session_state_size */
{
/* Safety */
Expand Down Expand Up @@ -642,66 +702,6 @@ int main(int argc, char **argv)

/* s2n_deserialize_resumption_state */
{
uint8_t tls12_ticket[S2N_TLS12_STATE_SIZE_IN_BYTES_WITHOUT_EMS] = {
S2N_SERIALIZED_FORMAT_TLS12_V1,
S2N_TLS12,
TLS_RSA_WITH_AES_128_GCM_SHA256,
TICKET_ISSUE_TIME_BYTES,
};

uint8_t tls12_ticket_with_ems[S2N_TLS12_STATE_SIZE_IN_BYTES] = {
S2N_SERIALIZED_FORMAT_TLS12_V3,
S2N_TLS12,
TLS_RSA_WITH_AES_128_GCM_SHA256,
TICKET_ISSUE_TIME_BYTES,
};

uint8_t tls13_ticket[] = {
S2N_SERIALIZED_FORMAT_TLS13_V1,
S2N_TLS13,
TLS_AES_128_GCM_SHA256,
TICKET_ISSUE_TIME_BYTES,
TICKET_AGE_ADD_BYTES,
SECRET_LEN,
SECRET,
EMPTY_EARLY_DATA_SIZE,
};

uint8_t tls13_server_ticket[] = {
S2N_SERIALIZED_FORMAT_TLS13_V1,
S2N_TLS13,
TLS_AES_128_GCM_SHA256,
TICKET_ISSUE_TIME_BYTES,
TICKET_AGE_ADD_BYTES,
SECRET_LEN,
SECRET,
KEYING_MATERIAL_EXPIRATION_BYTES,
EMPTY_EARLY_DATA_SIZE,
};

uint8_t tls13_ticket_with_early_data[] = {
S2N_SERIALIZED_FORMAT_TLS13_V1,
S2N_TLS13,
TLS_AES_128_GCM_SHA256,
TICKET_ISSUE_TIME_BYTES,
TICKET_AGE_ADD_BYTES,
SECRET_LEN,
SECRET,
0x00,
0x00,
0x00,
NONEMPTY_EARLY_DATA_SIZE,
APP_PROTOCOL_LEN,
APP_PROTOCOL,
0x00,
EARLY_DATA_CONTEXT_LEN,
EARLY_DATA_CONTEXT,
};

uint8_t faulty_format_ticket[] = {
0xFF,
};

/* Deserialize ticket with incorrect format errors */
{
struct s2n_blob ticket_blob = { 0 };
Expand Down Expand Up @@ -1675,5 +1675,169 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_config_free(config));
};

/* Test s2n_connection_set_session */
{
uint8_t server_state[] = "encrypted state";

DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(config);
EXPECT_SUCCESS(s2n_config_set_wall_clock(config, mock_time, NULL));
EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, "test_all"));

/* Invalid TLS1.2 tickets should not modify the connection.
*
* This basically tests that deserialization errors aren't fatal / unrecoverable.
*/
{
DEFER_CLEANUP(struct s2n_stuffer ticket_stuffer = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&ticket_stuffer, 0));
EXPECT_SUCCESS(s2n_stuffer_write_uint8(&ticket_stuffer, S2N_STATE_WITH_SESSION_TICKET));
EXPECT_SUCCESS(s2n_stuffer_write_uint16(&ticket_stuffer, sizeof(server_state)));
EXPECT_SUCCESS(s2n_stuffer_write_bytes(&ticket_stuffer, server_state, sizeof(server_state)));
EXPECT_SUCCESS(s2n_stuffer_write_bytes(&ticket_stuffer,
tls12_ticket_with_ems, sizeof(tls12_ticket_with_ems)));

size_t ticket_size = s2n_stuffer_data_available(&ticket_stuffer);
uint8_t *ticket_bytes = s2n_stuffer_raw_read(&ticket_stuffer, ticket_size);
EXPECT_NOT_NULL(ticket_bytes);

/* Test that deserialize modifies the connection in limited ways.
*
* No mechanism exists to do more than a shallow comparison of two connections.
* To prove that a shallow comparison is sufficient, we need to prove
* that s2n_deserialize_resumption_state does not modify the memory
* associated with pointers on the connection. To prove that, we can
* test that s2n_deserialize_resumption_state can successfully operate
* on an s2n_connection with a limited number of its pointers initialized.
*/
{
struct s2n_connection empty_conn = { 0 };
struct s2n_crypto_parameters crypto_params = { .cipher_suite = &s2n_null_cipher_suite };
empty_conn.secure = &crypto_params;
empty_conn.mode = S2N_CLIENT;
/* We can safely assume that a connection doesn't modify its config */
empty_conn.config = config;
EXPECT_SUCCESS(s2n_connection_set_session(&empty_conn, ticket_bytes, ticket_size));
EXPECT_SUCCESS(s2n_free(&empty_conn.client_ticket));
};

/* Test that deserialize does not modify the connection on parsing failure,
* given the constraints proven above.
*/
{
DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(conn);
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));

/* Trigger the deserialization failure as late as possible.
* The last byte is optional, so drop the last two bytes.
*/
size_t bad_ticket_size = ticket_size - 2;

/* Test the connection is not modified by a failed deserialize */
uint8_t saved_conn[sizeof(struct s2n_connection)] = { 0 };
EXPECT_MEMCPY_SUCCESS(saved_conn, conn, sizeof(struct s2n_connection));
uint8_t saved_secure[sizeof(struct s2n_crypto_parameters)] = { 0 };
EXPECT_MEMCPY_SUCCESS(saved_secure, conn->secure, sizeof(struct s2n_crypto_parameters));
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_set_session(conn, ticket_bytes, bad_ticket_size),
S2N_ERR_STUFFER_OUT_OF_DATA);
EXPECT_BYTEARRAY_EQUAL(saved_conn, conn, sizeof(struct s2n_connection));
EXPECT_BYTEARRAY_EQUAL(saved_secure, conn->secure, sizeof(struct s2n_crypto_parameters));

/* No valid ticket */
EXPECT_EQUAL(s2n_connection_get_session_length(conn), 0);
};

/* Test that deserialize does not modify the connection on a cipher selection failure,
* given the constraints proven above.
*/
{
DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(conn);
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));

/* Trigger the deserialization failure when checking the validity
* of the chosen cipher, not when parsing the ticket.
*/
EXPECT_SUCCESS(s2n_connection_set_cipher_preferences(conn, "null"));

/* Test the connection is not modified by a failed deserialize */
uint8_t saved_conn[sizeof(struct s2n_connection)] = { 0 };
EXPECT_MEMCPY_SUCCESS(saved_conn, conn, sizeof(struct s2n_connection));
uint8_t saved_secure[sizeof(struct s2n_crypto_parameters)] = { 0 };
EXPECT_MEMCPY_SUCCESS(saved_secure, conn->secure, sizeof(struct s2n_crypto_parameters));
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_set_session(conn, ticket_bytes, ticket_size),
S2N_ERR_CIPHER_NOT_SUPPORTED);
EXPECT_BYTEARRAY_EQUAL(saved_conn, conn, sizeof(struct s2n_connection));
EXPECT_BYTEARRAY_EQUAL(saved_secure, conn->secure, sizeof(struct s2n_crypto_parameters));

/* No valid ticket */
EXPECT_EQUAL(s2n_connection_get_session_length(conn), 0);
};
};

/* Invalid TLS1.3 tickets should not modify the connection.
*
* This basically tests that deserialization errors aren't fatal / unrecoverable.
*/
{
DEFER_CLEANUP(struct s2n_stuffer ticket_stuffer = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&ticket_stuffer, 0));
EXPECT_SUCCESS(s2n_stuffer_write_uint8(&ticket_stuffer, S2N_STATE_WITH_SESSION_TICKET));
EXPECT_SUCCESS(s2n_stuffer_write_uint16(&ticket_stuffer, sizeof(server_state)));
EXPECT_SUCCESS(s2n_stuffer_write_bytes(&ticket_stuffer, server_state, sizeof(server_state)));
EXPECT_SUCCESS(s2n_stuffer_write_bytes(&ticket_stuffer, tls13_ticket, sizeof(tls13_ticket)));

size_t ticket_size = s2n_stuffer_data_available(&ticket_stuffer);
uint8_t *ticket_bytes = s2n_stuffer_raw_read(&ticket_stuffer, ticket_size);
EXPECT_NOT_NULL(ticket_bytes);

/* Test that the connection is only shallowly modified by a successful deserialize.
*
* No mechanism exists to do more than a shallow comparison of two connections.
* To prove that a shallow comparison is sufficient, we need to prove
* that s2n_deserialize_resumption_state does not modify the memory
* associated with pointers on the connection. To prove that, we can
* test that s2n_deserialize_resumption_state can successfully operate
* on an s2n_connection with none of its pointers initialized.
*/
{
struct s2n_connection empty_conn = { 0 };
empty_conn.mode = S2N_CLIENT;
/* We can safely assume that a connection doesn't modify its config */
empty_conn.config = config;
EXPECT_SUCCESS(s2n_connection_set_session(&empty_conn, ticket_bytes, ticket_size));
EXPECT_OK(s2n_psk_parameters_wipe(&empty_conn.psk_params));
};

/* Test that deserialize does not modify the connection on failure,
* given the constraints proven above.
*/
{
DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(conn);
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));

/* Trigger the deserialization failure as late as possible in parsing.
* Drop the last byte we expect.
*/
size_t bad_ticket_size = ticket_size - 1;

/* Test the connection is not modified by a failed deserialize */
uint8_t saved[sizeof(struct s2n_connection)] = { 0 };
EXPECT_MEMCPY_SUCCESS(saved, conn, sizeof(struct s2n_connection));
EXPECT_FAILURE_WITH_ERRNO(s2n_connection_set_session(conn, ticket_bytes, bad_ticket_size),
S2N_ERR_STUFFER_OUT_OF_DATA);
EXPECT_BYTEARRAY_EQUAL(saved, conn, sizeof(struct s2n_connection));

/* No valid ticket */
EXPECT_EQUAL(s2n_connection_get_session_length(conn), 0);
};
};
};

END_TEST();
}
Loading

0 comments on commit 403d5e6

Please sign in to comment.