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 all 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
63 changes: 62 additions & 1 deletion tests/unit/s2n_server_new_session_ticket_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
#define MAX_TEST_SESSION_SIZE 300

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

static S2N_RESULT s2n_assert_tickets_sent(struct s2n_connection *conn, uint16_t expected_tickets_sent)
{
uint16_t tickets_sent = 0;
Expand Down Expand Up @@ -801,6 +800,68 @@ int main(int argc, char **argv)
};
};

/* s2n_server_nst_send */
{
/* s2n_server_nst_send writes a non-zero ticket when a valid encryption key exists */
{
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));

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);

EXPECT_EQUAL(s2n_stuffer_data_available(&conn->handshake.io),
S2N_TLS12_TICKET_SIZE_IN_BYTES);
};
jouho marked this conversation as resolved.
Show resolved Hide resolved

/* s2n_server_nst_send writes a zero-length ticket when no valid encryption key exists
*
*= https://www.rfc-editor.org/rfc/rfc5077#section-3.3
*= type=test
*# 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.
**/
{
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_SUCCESS(s2n_connection_set_config(conn, config));

conn->session_ticket_status = S2N_NEW_TICKET;

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);
EXPECT_EQUAL(s2n_stuffer_data_available(&conn->handshake.io), 0);
};
}

/* s2n_tls13_server_nst_send */
{
/* Mode is not server */
Expand Down
61 changes: 61 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,67 @@ 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_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));

/* Stop the handshake after the peers have established that a ticket
* will be sent in this handshake.
*/
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 issued zero-length session ticket */
EXPECT_TRUE(IS_ISSUING_NEW_SESSION_TICKET(server));

/* Client does not have a session ticket since it received zero-length NST message */
EXPECT_EQUAL(client->client_ticket.size, 0);
jouho marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_EQUAL(client->ticket_lifetime_hint, 0);
}

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
2 changes: 1 addition & 1 deletion tls/s2n_resume.c
Original file line number Diff line number Diff line change
Expand Up @@ -817,7 +817,7 @@ int s2n_encrypt_session_ticket(struct s2n_connection *conn, struct s2n_stuffer *
POSIX_GUARD(s2n_aes256_gcm.destroy_key(&aes_ticket_key));
POSIX_GUARD(s2n_session_key_free(&aes_ticket_key));

return 0;
return S2N_SUCCESS;
}

int s2n_decrypt_session_ticket(struct s2n_connection *conn, struct s2n_stuffer *from)
Expand Down
17 changes: 12 additions & 5 deletions tls/s2n_server_new_session_ticket.c
Original file line number Diff line number Diff line change
Expand Up @@ -82,23 +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) {
/* Send a zero-length ticket in the NewSessionTicket message if the server changes
* its mind mid-handshake or if there are no valid encrypt keys currently available.
*
*= 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.
**/
POSIX_GUARD(s2n_stuffer_init(&to, &entry));
if (!conn->config->use_tickets || s2n_encrypt_session_ticket(conn, &to) != S2N_SUCCESS) {
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)) {
POSIX_BAIL(S2N_ERR_SENDING_NST);
}

POSIX_GUARD(s2n_stuffer_init(&to, &entry));
POSIX_GUARD(s2n_stuffer_write_uint32(&conn->handshake.io, lifetime_hint_in_secs));
POSIX_GUARD(s2n_stuffer_write_uint16(&conn->handshake.io, session_ticket_len));

POSIX_GUARD(s2n_encrypt_session_ticket(conn, &to));
POSIX_GUARD(s2n_stuffer_write(&conn->handshake.io, &to.blob));

/* For parity with TLS1.3, track the single ticket sent.
Expand Down
Loading