Skip to content

Commit

Permalink
Add HRR compliance comments and tests for TLS RFC section 4.2.8 (#3362)
Browse files Browse the repository at this point in the history
  • Loading branch information
@goatgoose
goatgoose authored Jul 6, 2022
1 parent aa8f7fb commit dea5dba
Show file tree
Hide file tree
Showing 8 changed files with 241 additions and 49 deletions.
3 changes: 3 additions & 0 deletions tests/unit/s2n_client_early_data_indication_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -440,6 +440,9 @@ int main(int argc, char **argv)
server_conn->handshake.message_number = 2;
client_conn->handshake.message_number = 2;

/* Update the selected_group to ensure the HRR is valid */
client_conn->kex_params.client_ecc_evp_params.negotiated_curve = &s2n_ecc_curve_secp521r1;

EXPECT_SUCCESS(s2n_server_hello_retry_send(server_conn));
EXPECT_SUCCESS(s2n_stuffer_copy(&server_conn->handshake.io, &client_conn->handshake.io,
s2n_stuffer_data_available(&server_conn->handshake.io)));
Expand Down
140 changes: 140 additions & 0 deletions tests/unit/s2n_client_hello_retry_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1415,6 +1415,146 @@ int main(int argc, char **argv)
S2N_ERR_BAD_MESSAGE);
}

/**
*= https://tools.ietf.org/rfc/rfc8446#4.2.8
*= type=test
*# Upon receipt of this extension in a HelloRetryRequest, the client
*# MUST verify that (1) the selected_group field corresponds to a group
*# which was provided in the "supported_groups" extension in the
*# original ClientHello
**/
{
/* Create a custom security policy without secp521r1 */
const struct s2n_ecc_named_curve *const test_ecc_pref_list_for_retry[] = {
&s2n_ecc_curve_secp256r1,
&s2n_ecc_curve_secp384r1,
};
const struct s2n_ecc_preferences test_ecc_preferences_for_retry = {
.count = s2n_array_len(test_ecc_pref_list_for_retry),
.ecc_curves = test_ecc_pref_list_for_retry,
};
struct s2n_security_policy security_policy_test_tls13_retry_temp = {
.minimum_protocol_version = S2N_TLS10,
.cipher_preferences = &cipher_preferences_20190801,
.kem_preferences = &kem_preferences_null,
.signature_preferences = &s2n_signature_preferences_20200207,
.certificate_signature_preferences = &s2n_certificate_signature_preferences_20201110,
.ecc_preferences = &test_ecc_preferences_for_retry,
};

DEFER_CLEANUP(struct s2n_cert_chain_and_key *chain_and_key,
s2n_cert_chain_and_key_ptr_free);
EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&chain_and_key,
S2N_DEFAULT_ECDSA_TEST_CERT_CHAIN,
S2N_DEFAULT_ECDSA_TEST_PRIVATE_KEY));

DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(),
s2n_config_ptr_free);
EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, chain_and_key));
EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, "default_tls13"));
EXPECT_SUCCESS(s2n_config_disable_x509_verification(config));

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

DEFER_CLEANUP(struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(client_conn);
EXPECT_SUCCESS(s2n_connection_set_config(client_conn, config));

struct s2n_test_io_pair io_pair = { 0 };
EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair));
EXPECT_SUCCESS(s2n_connections_set_io_pair(client_conn, server_conn, &io_pair));

/* Force the HRR path */
client_conn->security_policy_override = &security_policy_test_tls13_retry_temp;

/* ClientHello 1 */
EXPECT_SUCCESS(s2n_client_hello_send(client_conn));
EXPECT_SUCCESS(s2n_stuffer_copy(&client_conn->handshake.io, &server_conn->handshake.io,
s2n_stuffer_data_available(&client_conn->handshake.io)));

/* Server receives ClientHello 1 */
EXPECT_SUCCESS(s2n_client_hello_recv(server_conn));
EXPECT_SUCCESS(s2n_set_connection_hello_retry_flags(server_conn));

/* Server sends HelloRetryRequest */
EXPECT_SUCCESS(s2n_server_hello_retry_send(server_conn));

EXPECT_SUCCESS(s2n_stuffer_wipe(&client_conn->handshake.io));
EXPECT_SUCCESS(s2n_stuffer_copy(&server_conn->handshake.io, &client_conn->handshake.io,
s2n_stuffer_data_available(&server_conn->handshake.io)));
client_conn->handshake.message_number = HELLO_RETRY_MSG_NO;

/* Set the curve to secp521r1, which was not provided in supported_groups */
client_conn->kex_params.server_ecc_evp_params.negotiated_curve = &s2n_ecc_curve_secp521r1;

/* Client receives HelloRetryRequest */
EXPECT_FAILURE_WITH_ERRNO(s2n_server_hello_recv(client_conn),
S2N_ERR_INVALID_HELLO_RETRY);
}

/**
*= https://tools.ietf.org/rfc/rfc8446#4.2.8
*= type=test
*# If using (EC)DHE key establishment and a HelloRetryRequest containing a
*# "key_share" extension was received by the client, the client MUST
*# verify that the selected NamedGroup in the ServerHello is the same as
*# that in the HelloRetryRequest. If this check fails, the client MUST
*# abort the handshake with an "illegal_parameter" alert.
**/
{
DEFER_CLEANUP(struct s2n_cert_chain_and_key *chain_and_key,
s2n_cert_chain_and_key_ptr_free);
EXPECT_SUCCESS(s2n_test_cert_chain_and_key_new(&chain_and_key,
S2N_DEFAULT_ECDSA_TEST_CERT_CHAIN,
S2N_DEFAULT_ECDSA_TEST_PRIVATE_KEY));

DEFER_CLEANUP(struct s2n_config *config = s2n_config_new(),
s2n_config_ptr_free);
EXPECT_SUCCESS(s2n_config_add_cert_chain_and_key_to_store(config, chain_and_key));
EXPECT_SUCCESS(s2n_config_set_cipher_preferences(config, "default_tls13"));
EXPECT_SUCCESS(s2n_config_disable_x509_verification(config));

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

DEFER_CLEANUP(struct s2n_connection *client_conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(client_conn);
EXPECT_SUCCESS(s2n_connection_set_config(client_conn, config));
EXPECT_SUCCESS(s2n_connection_set_blinding(client_conn, S2N_SELF_SERVICE_BLINDING));

struct s2n_test_io_pair io_pair = { 0 };
EXPECT_SUCCESS(s2n_io_pair_init_non_blocking(&io_pair));
EXPECT_SUCCESS(s2n_connections_set_io_pair(client_conn, server_conn, &io_pair));

/* Force the HRR path */
client_conn->security_policy_override = &security_policy_test_tls13_retry;

/* Skip to before client receives ServerHello 2 */
s2n_blocked_status blocked = 0;
EXPECT_OK(s2n_negotiate_until_message(client_conn, &blocked, SERVER_HELLO));
EXPECT_OK(s2n_negotiate_until_message(server_conn, &blocked, HELLO_RETRY_MSG));
EXPECT_OK(s2n_negotiate_until_message(server_conn, &blocked, CLIENT_HELLO));
EXPECT_OK(s2n_negotiate_until_message(client_conn, &blocked, CLIENT_HELLO));
EXPECT_OK(s2n_negotiate_until_message(client_conn, &blocked, SERVER_HELLO));
EXPECT_OK(s2n_negotiate_until_message(server_conn, &blocked, SERVER_HELLO));
EXPECT_OK(s2n_negotiate_until_message(server_conn, &blocked, ENCRYPTED_EXTENSIONS));

/* Set the negotiated curve to something other than what was sent in the HRR */
client_conn->kex_params.server_ecc_evp_params.negotiated_curve = &s2n_ecc_curve_secp521r1;

/* Client receives ServerHello 2 */
EXPECT_ERROR_WITH_ERRNO(s2n_negotiate_until_message(client_conn, &blocked, ENCRYPTED_EXTENSIONS),
S2N_ERR_BAD_MESSAGE);
}

EXPECT_SUCCESS(s2n_disable_tls13_in_test());

END_TEST();
Expand Down
24 changes: 14 additions & 10 deletions tests/unit/s2n_client_key_share_extension_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,17 @@ int main(int argc, char **argv)

/* Test s2n_client_key_share_extension.send with HelloRetryRequest */
{
/* For HelloRetryRequests when a keyshare does not match, test that s2n_client_key_share_extension.send replaces the list of keyshares,
* with a list containing a single KeyShareEntry for the server selected group. */
/**
* For HelloRetryRequests when a keyshare does not match, test that s2n_client_key_share_extension.send replaces
* the list of keyshares with a list containing a single KeyShareEntry for the server selected group.
*
*= https://tools.ietf.org/rfc/rfc8446#4.2.8
*= type=test
*# Otherwise, when sending the new ClientHello, the client MUST
*# replace the original "key_share" extension with one containing only a
*# new KeyShareEntry for the group indicated in the selected_group field
*# of the triggering HelloRetryRequest.
**/
if (s2n_is_evp_apis_supported()) {
struct s2n_connection *conn;
struct s2n_config *config;
Expand Down Expand Up @@ -306,7 +315,7 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_connection_free(conn));
}

/* For HelloRetryRequests, verify that we can resend an existing share to reject early data. */
/* For HelloRetryRequests, verify that we cannot resend an existing share. */
{
struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT);
EXPECT_NOT_NULL(conn);
Expand All @@ -332,13 +341,8 @@ int main(int argc, char **argv)
conn->early_data_state = S2N_EARLY_DATA_REJECTED;
conn->kex_params.server_ecc_evp_params.negotiated_curve = curve;

EXPECT_SUCCESS(s2n_client_key_share_extension.send(conn, &second_extension));
EXPECT_EQUAL(conn->kex_params.client_ecc_evp_params.negotiated_curve, curve);
EXPECT_NOT_NULL(conn->kex_params.client_ecc_evp_params.evp_pkey);

/* Same shares (same bytes) are written both times */
EXPECT_EQUAL(first_extension.write_cursor, second_extension.write_cursor);
EXPECT_BYTEARRAY_EQUAL(first_extension.blob.data, second_extension.blob.data, first_extension.write_cursor);
EXPECT_FAILURE_WITH_ERRNO(s2n_client_key_share_extension.send(conn, &second_extension),
S2N_ERR_BAD_KEY_SHARE);

EXPECT_SUCCESS(s2n_stuffer_free(&first_extension));
EXPECT_SUCCESS(s2n_stuffer_free(&second_extension));
Expand Down
3 changes: 3 additions & 0 deletions tests/unit/s2n_server_early_data_indication_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,9 @@ int main(int argc, char **argv)
server_conn->handshake.message_number = 2;
client_conn->handshake.message_number = 2;

/* Update the selected_group to ensure the HRR is valid */
client_conn->kex_params.client_ecc_evp_params.negotiated_curve = &s2n_ecc_curve_secp521r1;

EXPECT_SUCCESS(s2n_server_hello_retry_send(server_conn));
EXPECT_SUCCESS(s2n_stuffer_copy(&server_conn->handshake.io, &client_conn->handshake.io,
s2n_stuffer_data_available(&server_conn->handshake.io)));
Expand Down
35 changes: 9 additions & 26 deletions tests/unit/s2n_server_hello_retry_test.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,15 @@ int main(int argc, char **argv)
EXPECT_NOT_NULL(security_policy);
const struct s2n_ecc_named_curve *test_curve = security_policy->ecc_preferences->ecc_curves[0];

/* Retry for key share is valid */
/**
* Retry for key share is valid
*
*= https://tools.ietf.org/rfc/rfc8446#4.2.8
*= type=test
*# and (2) the selected_group field does not
*# correspond to a group which was provided in the "key_share" extension
*# in the original ClientHello.
**/
{
struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT);
EXPECT_NOT_NULL(conn);
Expand All @@ -608,31 +616,6 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_connection_free(conn));
}

/* Retry for early data is valid */
{
struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT);
EXPECT_NOT_NULL(conn);
EXPECT_SUCCESS(s2n_connection_set_cipher_preferences(conn, "20201021"));
conn->actual_protocol_version = S2N_TLS13;
conn->secure.cipher_suite = &s2n_tls13_aes_256_gcm_sha384;

conn->kex_params.server_ecc_evp_params.negotiated_curve = test_curve;
conn->kex_params.client_ecc_evp_params.negotiated_curve = test_curve;
conn->kex_params.client_ecc_evp_params.evp_pkey = EVP_PKEY_new();
EXPECT_NOT_NULL(conn->kex_params.client_ecc_evp_params.evp_pkey);

/* Early data rejected: allow retry */
conn->early_data_state = S2N_EARLY_DATA_REJECTED;
EXPECT_SUCCESS(s2n_server_hello_retry_recv(conn));

/* Early data not rejected: do NOT allow retry */
conn->early_data_state = S2N_EARLY_DATA_NOT_REQUESTED;
EXPECT_FAILURE_WITH_ERRNO(s2n_server_hello_retry_recv(conn),
S2N_ERR_INVALID_HELLO_RETRY);

EXPECT_SUCCESS(s2n_connection_free(conn));
}

/* Retry for multiple reasons is valid */
{
struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT);
Expand Down
24 changes: 24 additions & 0 deletions tls/extensions/s2n_client_key_share.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,13 @@ static int s2n_generate_default_ecc_key_share(struct s2n_connection *conn, struc
POSIX_GUARD(s2n_ecc_evp_params_free(client_params));
}

/**
*= https://tools.ietf.org/rfc/rfc8446#4.2.8
*# Otherwise, when sending the new ClientHello, the client MUST
*# replace the original "key_share" extension with one containing only a
*# new KeyShareEntry for the group indicated in the selected_group field
*# of the triggering HelloRetryRequest.
**/
client_params->negotiated_curve = server_curve;
} else {
client_params->negotiated_curve = ecc_pref->ecc_curves[0];
Expand Down Expand Up @@ -164,6 +171,13 @@ static int s2n_generate_default_pq_hybrid_key_share(struct s2n_connection *conn,
POSIX_GUARD(s2n_kem_group_free(client_params));
}

/**
*= https://tools.ietf.org/rfc/rfc8446#4.2.8
*# Otherwise, when sending the new ClientHello, the client MUST
*# replace the original "key_share" extension with one containing only a
*# new KeyShareEntry for the group indicated in the selected_group field
*# of the triggering HelloRetryRequest.
**/
client_params->kem_group = server_group;
} else {
client_params->kem_group = kem_pref->tls13_kem_groups[0];
Expand All @@ -175,6 +189,16 @@ static int s2n_generate_default_pq_hybrid_key_share(struct s2n_connection *conn,

static int s2n_client_key_share_send(struct s2n_connection *conn, struct s2n_stuffer *out)
{
if (s2n_is_hello_retry_handshake(conn)) {
const struct s2n_ecc_named_curve *server_curve = conn->kex_params.server_ecc_evp_params.negotiated_curve;
const struct s2n_ecc_named_curve *client_curve = conn->kex_params.client_ecc_evp_params.negotiated_curve;
const struct s2n_kem_group *server_group = conn->kex_params.server_kem_group_params.kem_group;
const struct s2n_kem_group *client_group = conn->kex_params.client_kem_group_params.kem_group;

/* Ensure a new key share will be sent after a hello retry request */
POSIX_ENSURE(server_curve != client_curve || server_group != client_group, S2N_ERR_BAD_KEY_SHARE);
}

struct s2n_stuffer_reservation shares_size = {0};
POSIX_GUARD(s2n_stuffer_reserve_uint16(out, &shares_size));
POSIX_GUARD(s2n_generate_default_pq_hybrid_key_share(conn, out));
Expand Down
19 changes: 18 additions & 1 deletion tls/extensions/s2n_server_key_share.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,24 @@ static int s2n_server_key_share_recv_ecc(struct s2n_connection *conn, uint16_t n
}

struct s2n_ecc_evp_params *server_ecc_evp_params = &conn->kex_params.server_ecc_evp_params;
server_ecc_evp_params->negotiated_curve = ecc_pref->ecc_curves[supported_curve_index];
const struct s2n_ecc_named_curve *negotiated_curve = ecc_pref->ecc_curves[supported_curve_index];

/**
*= https://tools.ietf.org/rfc/rfc8446#4.2.8
*# If using (EC)DHE key establishment and a HelloRetryRequest containing a
*# "key_share" extension was received by the client, the client MUST
*# verify that the selected NamedGroup in the ServerHello is the same as
*# that in the HelloRetryRequest. If this check fails, the client MUST
*# abort the handshake with an "illegal_parameter" alert.
**/
if (s2n_is_hello_retry_handshake(conn) && !s2n_is_hello_retry_message(conn)) {
POSIX_ENSURE_REF(server_ecc_evp_params->negotiated_curve);
const struct s2n_ecc_named_curve *previous_negotiated_curve = server_ecc_evp_params->negotiated_curve;
POSIX_ENSURE(negotiated_curve == previous_negotiated_curve,
S2N_ERR_BAD_MESSAGE);
}

server_ecc_evp_params->negotiated_curve = negotiated_curve;

/* If this is a HelloRetryRequest, we won't have a key share. We just have the selected group.
* Set the server negotiated curve and exit early so a proper keyshare can be generated. */
Expand Down
Loading

0 comments on commit dea5dba

Please sign in to comment.