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: Send zero-length NST when session key is expired #4532

Merged
merged 36 commits into from
May 15, 2024
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
2e48de1
send zero-length NST when session key is expired
jouho Apr 30, 2024
becb306
add another to test the impact on tls 1.3 path
jouho Apr 30, 2024
e023d46
check if tls1.3 is fully supported
jouho Apr 30, 2024
edff88a
use shorter variable names
jouho May 1, 2024
f22a5b9
Merge branch 'main' into zero-length-tls12-NST-message
jouho May 1, 2024
8352bce
Merge branch 'main' into zero-length-tls12-NST-message
jouho May 1, 2024
c4a5474
modify test setups
jouho May 1, 2024
9beba2c
address comment
jouho May 1, 2024
8ead510
add compliance comments
jouho May 1, 2024
8838bdf
Merge branch 'main' into zero-length-tls12-NST-message
jouho May 1, 2024
ab18c07
add unit test for s2n_server_send
jouho May 6, 2024
c1948e8
Merge branch 'main' into zero-length-tls12-NST-message
jouho May 6, 2024
e86b5b3
address clang format issues
jouho May 6, 2024
ae343b8
edit rfc comment format
jouho May 6, 2024
9302a3a
fix rfc number
jouho May 6, 2024
112574c
remove unnecessary expect_false()
jouho May 6, 2024
82709ee
add test case for sending non-zero nst
jouho May 6, 2024
8118b83
use suggested comment
jouho May 7, 2024
aad3a35
remove unnecessary test case
jouho May 7, 2024
13ec6d3
check all data is read
jouho May 7, 2024
1c8b71d
give better test description
jouho May 7, 2024
0a4c8ee
add compliance comment to the test
jouho May 7, 2024
9ff9261
fix comment format
jouho May 7, 2024
474f40f
Merge branch 'main' into zero-length-tls12-NST-message
jouho May 7, 2024
4fc4de4
Merge branch 'main' into zero-length-tls12-NST-message
jouho May 8, 2024
3554753
verify ticket length and and lifetime hint is 0
jouho May 8, 2024
ca92f58
fix input orders
jouho May 8, 2024
14d56a3
address PR feedbacks
jouho May 8, 2024
6b3ace8
send zero-length nst upon failing to retrieve encrypt key
jouho May 13, 2024
9b09e91
address linter issue
jouho May 13, 2024
8aeedd3
Merge branch 'main' into zero-length-tls12-NST-message
jouho May 13, 2024
907da2b
address redundant code
jouho May 13, 2024
ab7c57d
remove unnecessary test setup
jouho May 15, 2024
1bf7d88
change return value to S2N_SUCCESS
jouho May 15, 2024
8ee8b0f
Merge branch 'main' into zero-length-tls12-NST-message
jouho May 15, 2024
b2e650d
remove unnecessary #define
jouho May 15, 2024
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
93 changes: 93 additions & 0 deletions tests/unit/s2n_server_new_session_ticket_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#define MAX_TEST_SESSION_SIZE 300

#define EXPECT_TICKETS_SENT(conn, count) EXPECT_OK(s2n_assert_tickets_sent(conn, count))
#define S2N_CLOCK_SYS CLOCK_REALTIME

jouho marked this conversation as resolved.
Show resolved Hide resolved
static S2N_RESULT s2n_assert_tickets_sent(struct s2n_connection *conn, uint16_t expected_tickets_sent)
{
Expand Down Expand Up @@ -80,6 +81,32 @@ static int s2n_setup_test_resumption_secret(struct s2n_connection *conn)
return S2N_SUCCESS;
}

/**
* This function is used to "skip" time in unit tests. It will mock the system
* time to be current_time (ns) + data (ns). The "data" parameter is a uint64_t
* passed in as a void*.
*/
int mock_nanoseconds_since_epoch(void *data, uint64_t *nanoseconds)
{
struct timespec current_time;

clock_gettime(S2N_CLOCK_SYS, &current_time);

/**
* current_time fields are represented as time_t, and time_t has a platform
* dependent size. On 32 bit platforms, attempting to convert the current
* system time to nanoseconds will overflow, causing odd failures in unit
* tests. We upcast current_time fields to uint64_t before multiplying to
* avoid this.
*/
*nanoseconds = 0;
*nanoseconds += (uint64_t) current_time.tv_sec * ONE_SEC_IN_NANOS;
*nanoseconds += (uint64_t) current_time.tv_nsec;
*nanoseconds += *(uint64_t *) data;

return 0;
}

int main(int argc, char **argv)
{
BEGIN_TEST();
Expand Down Expand Up @@ -801,6 +828,72 @@ int main(int argc, char **argv)
};
};

/* s2n_server_nst_send */
{
/* TLS 1.2 server sends a new session ticket key */
jouho marked this conversation as resolved.
Show resolved Hide resolved
{
DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(config);
DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(conn);
EXPECT_OK(s2n_resumption_test_ticket_key_setup(config));
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));

EXPECT_NOT_EQUAL(s2n_stuffer_space_remaining(&conn->handshake.io), 0);
jouho marked this conversation as resolved.
Show resolved Hide resolved

conn->config->use_tickets = 1;
jouho marked this conversation as resolved.
Show resolved Hide resolved
conn->session_ticket_status = S2N_NEW_TICKET;

EXPECT_SUCCESS(s2n_server_nst_send(conn));
EXPECT_TICKETS_SENT(conn, 1);

uint32_t lifetime = 0;
EXPECT_SUCCESS(s2n_stuffer_read_uint32(&conn->handshake.io, &lifetime));
EXPECT_TRUE(lifetime > 0);

uint16_t ticket_len = 0;
EXPECT_SUCCESS(s2n_stuffer_read_uint16(&conn->handshake.io, &ticket_len));
EXPECT_TRUE(ticket_len > 0);
};
jouho marked this conversation as resolved.
Show resolved Hide resolved

/* TLS 1.2 server sends zero ticket lifetime and zero-length ticket if key expires */
{
jouho marked this conversation as resolved.
Show resolved Hide resolved
DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(config);
DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(conn);
EXPECT_OK(s2n_resumption_test_ticket_key_setup(config));
EXPECT_SUCCESS(s2n_connection_set_config(conn, config));

EXPECT_NOT_EQUAL(s2n_stuffer_space_remaining(&conn->handshake.io), 0);
jouho marked this conversation as resolved.
Show resolved Hide resolved

conn->config->use_tickets = 1;
jouho marked this conversation as resolved.
Show resolved Hide resolved
conn->session_ticket_status = S2N_NEW_TICKET;

/* Expire current session ticket key so that server no longer holds a valid key */
uint64_t mock_delay = config->encrypt_decrypt_key_lifetime_in_nanos;
EXPECT_SUCCESS(s2n_config_set_wall_clock(config, mock_nanoseconds_since_epoch,
&mock_delay));
jouho marked this conversation as resolved.
Show resolved Hide resolved

/* Send a new session ticket with lifetime hint and session ticket length as zero.
jouho marked this conversation as resolved.
Show resolved Hide resolved
* tickets_sent does not get incremented in this codepath and therefore we
* technically expect 0 tickets_sent in connection object.
*/
EXPECT_SUCCESS(s2n_server_nst_send(conn));
EXPECT_TICKETS_SENT(conn, 0);

uint32_t lifetime = 0;
EXPECT_SUCCESS(s2n_stuffer_read_uint32(&conn->handshake.io, &lifetime));
EXPECT_TRUE(lifetime == 0);

uint16_t ticket_len = 0;
EXPECT_SUCCESS(s2n_stuffer_read_uint16(&conn->handshake.io, &ticket_len));
EXPECT_TRUE(ticket_len == 0);
};
}

/* s2n_tls13_server_nst_send */
{
/* Mode is not server */
Expand Down
108 changes: 108 additions & 0 deletions tests/unit/s2n_session_ticket_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -1416,6 +1416,114 @@ int main(int argc, char **argv)
EXPECT_TRUE(IS_RESUMPTION_HANDSHAKE(server));
}

/* Test TLS 1.2 Server sends a zero-length ticket in the NewSessionTicket handshake
* if the ticket key was expired after SERVER_HELLO
*/
{
DEFER_CLEANUP(struct s2n_config *client_configuration = s2n_config_new(),
s2n_config_ptr_free);
EXPECT_NOT_NULL(client_configuration);
EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(client_configuration, 1));
EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(client_configuration));

DEFER_CLEANUP(struct s2n_config *server_configuration = s2n_config_new(),
s2n_config_ptr_free);
EXPECT_NOT_NULL(server_configuration);
EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(server_configuration, 1));
EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(server_configuration));
jouho marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(server_configuration,
chain_and_key));

EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_configuration, ticket_key_name1,
s2n_array_len(ticket_key_name1), ticket_key1, s2n_array_len(ticket_key1), 0));

DEFER_CLEANUP(struct s2n_connection *client = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(client);
EXPECT_SUCCESS(s2n_connection_set_config(client, client_configuration));

DEFER_CLEANUP(struct s2n_connection *server = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(server);
EXPECT_SUCCESS(s2n_connection_set_config(server, server_configuration));

DEFER_CLEANUP(struct s2n_test_io_stuffer_pair test_io = { 0 },
s2n_io_stuffer_pair_free);
EXPECT_OK(s2n_io_stuffer_pair_init(&test_io));
EXPECT_OK(s2n_connections_set_io_stuffer_pair(client, server, &test_io));

/* Perform initial part of handshake to verify that a valid key exists */
jouho marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_OK(s2n_negotiate_test_server_and_client_until_message(server, client, CLIENT_FINISHED));

/* Expire current session ticket key so that server no longer holds a valid key */
uint64_t mock_delay = server_configuration->encrypt_decrypt_key_lifetime_in_nanos;
EXPECT_SUCCESS(s2n_config_set_wall_clock(server_configuration, mock_nanoseconds_since_epoch,
&mock_delay));

/* Attempt to send a NewSessionTicket. This should send a zero-length NST message */
EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(server, client));

/* Verify that TLS1.2 was negotiated */
EXPECT_EQUAL(client->actual_protocol_version, S2N_TLS12);
EXPECT_EQUAL(server->actual_protocol_version, S2N_TLS12);

/* Verify that the server did issue a new session ticket */
EXPECT_TRUE(IS_ISSUING_NEW_SESSION_TICKET(server));
}
jouho marked this conversation as resolved.
Show resolved Hide resolved

/* Test TLS 1.3 Server does not send a zero-length ticket in the NewSessionTicket handshake
jouho marked this conversation as resolved.
Show resolved Hide resolved
* if the ticket key was expired after SERVER_HELLO
*/
if (s2n_is_tls13_fully_supported()) {
DEFER_CLEANUP(struct s2n_config *client_configuration = s2n_config_new(),
s2n_config_ptr_free);
EXPECT_NOT_NULL(client_configuration);
EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(client_configuration, 1));
EXPECT_SUCCESS(s2n_config_set_cipher_preferences(client_configuration, "default_tls13"));
EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(client_configuration));

DEFER_CLEANUP(struct s2n_config *server_configuration = s2n_config_new(),
s2n_config_ptr_free);
EXPECT_NOT_NULL(server_configuration);
EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(server_configuration, 1));
EXPECT_SUCCESS(s2n_config_set_cipher_preferences(server_configuration, "default_tls13"));
EXPECT_SUCCESS(s2n_config_set_unsafe_for_testing(server_configuration));
EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(server_configuration,
chain_and_key));

EXPECT_SUCCESS(s2n_config_add_ticket_crypto_key(server_configuration, ticket_key_name1,
s2n_array_len(ticket_key_name1), ticket_key1, s2n_array_len(ticket_key1), 0));

DEFER_CLEANUP(struct s2n_connection *client = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(client);
EXPECT_SUCCESS(s2n_connection_set_config(client, client_configuration));

DEFER_CLEANUP(struct s2n_connection *server = s2n_connection_new(S2N_SERVER),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(server);
EXPECT_SUCCESS(s2n_connection_set_config(server, server_configuration));

DEFER_CLEANUP(struct s2n_test_io_stuffer_pair test_io = { 0 }, s2n_io_stuffer_pair_free);
EXPECT_OK(s2n_io_stuffer_pair_init(&test_io));
EXPECT_OK(s2n_connections_set_io_stuffer_pair(client, server, &test_io));

/* Negotiate until session ticket is encrypted with session ticket key */
EXPECT_OK(s2n_negotiate_test_server_and_client_until_message(server, client, CLIENT_FINISHED));

/* After session ticket is encrypted, expire current session ticket key */
uint64_t mock_delay = server_configuration->encrypt_decrypt_key_lifetime_in_nanos;
EXPECT_SUCCESS(s2n_config_set_wall_clock(server_configuration, mock_nanoseconds_since_epoch,
&mock_delay));

/* Attempt to send a NewSessionTicket. This should not send a zero-length NST message */
EXPECT_SUCCESS(s2n_negotiate_test_server_and_client(server, client));

/* Verify that TLS1.3 was negotiated */
EXPECT_EQUAL(client->actual_protocol_version, S2N_TLS13);
EXPECT_EQUAL(server->actual_protocol_version, S2N_TLS13);
}

EXPECT_SUCCESS(s2n_io_pair_close(&io_pair));
EXPECT_SUCCESS(s2n_cert_chain_and_key_free(chain_and_key));
EXPECT_SUCCESS(s2n_cert_chain_and_key_free(ecdsa_chain_and_key));
Expand Down
24 changes: 21 additions & 3 deletions tls/s2n_server_new_session_ticket.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,12 +82,30 @@ int s2n_server_nst_send(struct s2n_connection *conn)
uint32_t lifetime_hint_in_secs =
(conn->config->encrypt_decrypt_key_lifetime_in_nanos + conn->config->decrypt_key_lifetime_in_nanos) / ONE_SEC_IN_NANOS;

/* When server changes it's mind mid handshake send lifetime hint and session ticket length as zero */
if (!conn->config->use_tickets) {
/* When server changes it's mind mid handshake, or if session key used to encrypt session ticket
maddeleine marked this conversation as resolved.
Show resolved Hide resolved
jouho marked this conversation as resolved.
Show resolved Hide resolved
* is expired, send lifetime hint and session ticket length as zero
*
*= https://www.rfc-editor.org/rfc/rfc5077#section-3.3
*# This message MUST be sent if the server included
*# a SessionTicket extension in the ServerHello.
jouho marked this conversation as resolved.
Show resolved Hide resolved
*
*= https://www.rfc-editor.org/rfc/rfc5077#section-3.3
*# If the server determines that it does not want to include a
*# ticket after it has included the SessionTicket extension in the
*# ServerHello, then it sends a zero-length ticket in the
*# NewSessionTicket handshake message.
*
*= https://www.rfc-editor.org/rfc/rfc5077#section-3.3
*# struct {
*# uint32 ticket_lifetime_hint;
*# opaque ticket<0..2^16-1>;
*# } NewSessionTicket;
jouho marked this conversation as resolved.
Show resolved Hide resolved
**/
if (!conn->config->use_tickets || s2n_result_is_error(s2n_config_is_encrypt_key_available(conn->config))) {
jouho marked this conversation as resolved.
Show resolved Hide resolved
POSIX_GUARD(s2n_stuffer_write_uint32(&conn->handshake.io, 0));
POSIX_GUARD(s2n_stuffer_write_uint16(&conn->handshake.io, 0));

return 0;
return S2N_SUCCESS;
}

if (!s2n_server_sending_nst(conn)) {
Expand Down
Loading