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 3 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
46 changes: 46 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 @@ -13,6 +13,7 @@
* permissions and limitations under the License.
*/

#include "s2n.h"
#include "s2n_test.h"
#include "testlib/s2n_testlib.h"
#include "tls/extensions/s2n_psk_key_exchange_modes.h"
Expand Down Expand Up @@ -245,5 +246,50 @@ int main(int argc, char **argv)
EXPECT_SUCCESS(s2n_stuffer_free(&out));
};

/* Test should send */
{
jmayclin marked this conversation as resolved.
Show resolved Hide resolved
/* when neither resumption nor PSKs are enabled, the extension should not be sent */
jmayclin marked this conversation as resolved.
Show resolved Hide resolved
{
DEFER_CLEANUP(struct s2n_config *no_resumption_config = s2n_config_new(), s2n_config_ptr_free);
EXPECT_NOT_NULL(no_resumption_config);

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

jmayclin marked this conversation as resolved.
Show resolved Hide resolved
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_external_psk_new(), s2n_psk_free);
uint8_t identity[] = "alice";
uint8_t secret[] = "a secret";
EXPECT_SUCCESS(s2n_psk_set_identity(psk, identity, s2n_array_len(identity)));
EXPECT_SUCCESS(s2n_psk_set_secret(psk, secret, s2n_array_len(secret)));

EXPECT_SUCCESS(s2n_connection_append_psk(conn, psk));
jmayclin marked this conversation as resolved.
Show resolved Hide resolved

EXPECT_TRUE(s2n_psk_key_exchange_modes_extension.should_send(conn));
}
}
jmayclin marked this conversation as resolved.
Show resolved Hide resolved

END_TEST();
}
17 changes: 15 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,21 @@ 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.
jmayclin marked this conversation as resolved.
Show resolved Hide resolved
*/
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