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: TLS 1.3 session resumption #6182

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Conversation

link2xt
Copy link
Collaborator

@link2xt link2xt commented Nov 6, 2024

This PR enables TLS 1.3 session resumption on the client for Rustls connections.
For native-tls there is no support for session resumption, but it is only used for connections without strict TLS: sfackler/rust-native-tls#308

Only TLS 1.3 session resumption is supported, TLS 1.2 is out of scope. It has worse security than TLS 1.3 mechanisms so not worth increasing attack surface as commented in the code.

Session storage is separated by port and ALPN because Rustls itself only separates by hostname: rustls/rustls#2196

Connection reestablishment can be tested for SMTP using deltachat-repl command send (send command now establishes a dedicated connection if disconnected) and for IMAP using deltachat-repl command fetch.
To see internal Rustls logs, run with RUST_LOG=rustls=debug cargo run -p deltachat-repl -- deltachat-db.
Configure an account with setqr dcaccount:... and configure.
Then instead of connecting select self-chat with chat 10 and send messages with send or download with fetch.

While testing this I discovered that Dovecot on chatmail servers does not support TLS session resumption: deltachat/chatmail#455
Postfix supports TLS session resumption but only issues one ticket when session is not resumed: deltachat/chatmail#456

Because of this, with chatmail server I only managed to test Postfix.
However, I did not see expected 1 RTT reduction that should happen in theory.

Even with artificial delay tc qdisc del dev <device> root netem delay 5s I did not see the diffierence between resumed and not resumed connection.

No idea why connection time is not reduced, maybe has to do with IMAP and SMTP starting using the server banner rather than contact request like in HTTP(S) or initial TCP window is too low.

@link2xt link2xt force-pushed the link2xt/tls-session-resumption branch from 643a3c6 to 5a315d3 Compare November 9, 2024 16:16
If scheduler is not connected,
use `send_msg_sync` to create a dedicated connection
and send a single message.

This allows easily testing SMTP connection establishment.
@link2xt link2xt force-pushed the link2xt/tls-session-resumption branch from 53f7fce to c0067f8 Compare November 10, 2024 05:11
@link2xt link2xt changed the title feat: TLS session resumption feat: TLS 1.3 session resumption Nov 10, 2024
@link2xt
Copy link
Collaborator Author

link2xt commented Nov 10, 2024

Actually looking at the TLS 1.3 handshake, I don't see how session resumption can reduce latency for IMAP and SMTP. The server is the first to talk in these protocols and it can always send the banner together with the Server Hello even if session resumption is not used. 0-RTT is useless for IMAP and SMTP unless we pretend that we already received the banner and send the first command in early data.

So we gain some reduction in data transferred because we no longer need to send server certificate if session is resumed, but that is only visible for bandwidth-limited connections. And cut some CPU load, but this is only interesting for server with a lot of client constantly reconnecting.

@link2xt link2xt marked this pull request as ready for review November 10, 2024 08:02
chat::send_text_msg(&context, sel_chat.as_ref().unwrap().get_id(), msg).await?;
if context.is_running().await {
chat::send_text_msg(&context, sel_chat.as_ref().unwrap().get_id(), msg).await?;
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm afraid this can break manual testing of some scenarios, maybe add a separate command?

.lock()
.entry((port, alpn.to_string()))
.or_insert_with(|| {
// This is the default as of Rustls version 0.23.16,
Copy link
Collaborator

Choose a reason for hiding this comment

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

"This" == not to share tickets across different ports/ALPNs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In this case I refer to session storage configuration, memory cache with 256 sessions max.

@link2xt link2xt marked this pull request as draft November 20, 2024 18:16
@link2xt
Copy link
Collaborator Author

link2xt commented Nov 20, 2024

I now think session storage should also be separate per-context. It is not a good idea to have multiple profiles that you want to keep separate inside a single Delta Chat application, but users might still do this. It is even possible to use Tor circuit isolation already by configuring separate usernames (IsolateSOCKSAuth) for SOCKS5 proxy and reusing TLS session across circuits makes it possible to link such accounts.

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

Successfully merging this pull request may close these issues.

2 participants