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

feat: send psk_ke_modes ext in first flight #4177

Merged
merged 9 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions bindings/rust/bench/src/harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -506,8 +506,7 @@ mod tests {
.try_init()
.unwrap();
session_resumption::<S2NConnection, S2NConnection>();
// https://github.com/aws/s2n-tls/issues/4124
// session_resumption::<S2NConnection, RustlsConnection>();
session_resumption::<S2NConnection, RustlsConnection>();
session_resumption::<S2NConnection, OpenSslConnection>();

session_resumption::<RustlsConnection, RustlsConnection>();
Expand Down
42 changes: 42 additions & 0 deletions tests/unit/s2n_psk_key_exchange_modes_extension_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,48 @@ int main(int argc, char **argv)
};
};

/* Test: s2n_psk_key_exchange_modes_should_send */
{
/* When neither resumption nor PSKs are enabled, the extension should not be sent. */
{
DEFER_CLEANUP(struct s2n_config *no_resumption_config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(no_resumption_config);
EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(no_resumption_config, false));

Comment on lines +230 to +231
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I like this as a solution :)

DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT), s2n_connection_ptr_free);
EXPECT_SUCCESS(s2n_connection_set_config(conn, no_resumption_config));

EXPECT_FALSE(s2n_psk_key_exchange_modes_extension.should_send(conn));
};

/* When session resumption is enabled, the extension should be sent. */
{
DEFER_CLEANUP(struct s2n_config *resumption_config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(resumption_config);
EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(resumption_config, true));

DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT), s2n_connection_ptr_free);
EXPECT_SUCCESS(s2n_connection_set_config(conn, resumption_config));

EXPECT_TRUE(s2n_psk_key_exchange_modes_extension.should_send(conn));
};

/* When a client is using out-of-band PSKs, the extension should be sent. */
{
DEFER_CLEANUP(struct s2n_config *psk_config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(psk_config);
EXPECT_SUCCESS(s2n_config_set_session_tickets_onoff(psk_config, false));

DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT), s2n_connection_ptr_free);
EXPECT_SUCCESS(s2n_connection_set_config(conn, psk_config));

DEFER_CLEANUP(struct s2n_psk *psk = s2n_test_psk_new(conn), s2n_psk_free);
EXPECT_SUCCESS(s2n_connection_append_psk(conn, psk));

EXPECT_TRUE(s2n_psk_key_exchange_modes_extension.should_send(conn));
};
};

/* Functional test */
{
struct s2n_stuffer out = { 0 };
Expand Down
16 changes: 14 additions & 2 deletions tls/extensions/s2n_psk_key_exchange_modes.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,20 @@ const s2n_extension_type s2n_psk_key_exchange_modes_extension = {

static bool s2n_psk_key_exchange_modes_should_send(struct s2n_connection *conn)
{
/* Only send a psk_key_exchange_modes extension if psks available */
return conn->psk_params.psk_list.len > 0;
/**
*= https://tools.ietf.org/rfc/rfc8446#section-4.2.9
*# Servers MUST NOT select a key exchange mode that is not listed by the
*# client. This extension also restricts the modes for use with PSK
*# resumption.
*
* The RFC is ambiguous about whether the psk_kx_modes extension should be
* sent in the first flight of messages, but we choose to do so for
* interoperability with other TLS implementations.
* https://github.com/aws/s2n-tls/issues/4124
* The final check for psk_list.len > 0 is necessary because external
* PSKs are used without setting `use_tickets` to true.
*/
return conn->config->use_tickets || conn->psk_params.psk_list.len > 0;
}

static int s2n_psk_key_exchange_modes_send(struct s2n_connection *conn, struct s2n_stuffer *out)
Expand Down