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

s2n client unable to resume connections with rustls server #4124

Closed
jmayclin opened this issue Jul 29, 2023 · 4 comments
Closed

s2n client unable to resume connections with rustls server #4124

jmayclin opened this issue Jul 29, 2023 · 4 comments

Comments

@jmayclin
Copy link
Contributor

Problem:

s2n-tls clients are unable to use TLS 1.3 session resumption with rustls servers.

Turning on rustls logging, we see the following messages

[2023-07-28T23:55:38Z DEBUG rustls::server::hs] decided upon suite TLS13_AES_128_GCM_SHA256
[2023-07-28T23:55:38Z DEBUG rustls::server::tls13::client_hello] Client unwilling to resume, DHE_KE not offered

s2n-tls does not send the key_exchange_mode extension unless there is a PSK already available.

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

However, rustls won't send a session ticket unless the key_exchange_mode is included in the first flight of messages.

The RFC is unfortunately under-specified about this scenario.

In order to use PSKs, clients MUST also send a "psk_key_exchange_modes" extension.
-- https://www.rfc-editor.org/rfc/rfc8446#section-4.2.9

It is very clear that the extension must be sent, but is not clear about when it must be sent. However, this section from the RFC does imply that the key_exchange_mode extension should be sent before the server responds with a session ticket

The semantics of this extension are that the client only supports the use of PSKs with these modes, which restricts both the use of PSKs offered in this ClientHello and those which the server might supply via NewSessionTicket.
-- https://www.rfc-editor.org/rfc/rfc8446#section-4.2.9

Openssl and Rustls both include the key_exchange_mode extension in the first flight of messages.

s2n-tls and Openssl both return session tickets even if the key_exchange_mode isn't sent.

Solution:

Send a key_exchange_modes extension whenever the client wants to use resumption. The proposed change is listed below.

diff --git a/tls/extensions/s2n_psk_key_exchange_modes.c b/tls/extensions/s2n_psk_key_exchange_modes.c
index 2062ffa5..3a2ddde4 100644
--- a/tls/extensions/s2n_psk_key_exchange_modes.c
+++ b/tls/extensions/s2n_psk_key_exchange_modes.c
@@ -38,8 +38,8 @@ 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;
+    /* send a psk_key_exchange_modes extension whenever we are trying to use psks */
+    return conn->config->use_tickets;
 }
 
 static int s2n_psk_key_exchange_modes_send(struct s2n_connection *conn, struct s2n_stuffer *out)

Requirements / Acceptance Criteria:

Ideally we would merge in testing with this change. However the tests that I wrote for this are using #4114 , so I'll need to wait for that to be merged in before I can add my tests.

@jmayclin jmayclin changed the title s2n client unable to resumption connections with rustls server s2n client unable to resume connections with rustls server Jul 29, 2023
@jmayclin jmayclin self-assigned this Jul 29, 2023
@ctz
Copy link

ctz commented Aug 1, 2023

However, rustls won't send a session ticket unless the key_exchange_mode is included in the first flight of messages.

I think that behaviour is justified by:

Servers SHOULD NOT send NewSessionTicket with tickets that are not compatible with the advertised modes; however, if a server does so, the impact will just be that the client's attempts at resumption fail.

As a server, it's impossible to make this decision without the client including key_exchange_mode in the full handshake.

The RFC is unfortunately under-specified about this scenario.

Yeah. Could be something that could be fixed in 8446bis?

@jmayclin
Copy link
Contributor Author

jmayclin commented Aug 7, 2023

I talked with some teammates about this, and an interesting point was raised.

As a server, it's impossible to make this decision without the client including key_exchange_mode in the full handshake.

That is only the case if the session ticket format depends on the Key Exchange mode. For s2n-tls tickets are agnostic to the key exchange mode (even though we only support psk_dhe_ke) so we are actually complying with

Servers SHOULD NOT send NewSessionTicket with tickets that are compliant with

even when a client hasn't sent their supported key exchange modes.

We're still planning on switching to a behavior compatible with rustls, but it adds some interesting context to the "RFC ambiguity" discussion.

@lrstewart
Copy link
Contributor

Hmmm although thinking about it more, since s2n-tls only supports psk_dhe_ke, you could argue that s2n-tls tickets actually are dependent on key exchange mode. As in, if the client doesn't support psk_dhe_ke, they don't support session resumption with s2n-tls at all, so no tickets will ever be compatible.

@jmayclin
Copy link
Contributor Author

jmayclin commented Oct 6, 2023

Addressed in #4177

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants