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
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
1 change: 0 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,6 @@ regex = { workspace = true }
rusqlite = { workspace = true, features = ["sqlcipher"] }
rust-hsluv = "0.1"
rustls-pki-types = "1.10.0"
rustls = { version = "0.23.14", default-features = false }
sanitize-filename = { workspace = true }
serde_json = { workspace = true }
serde_urlencoded = "0.7.1"
Expand Down
17 changes: 16 additions & 1 deletion deltachat-repl/src/cmdline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,22 @@ pub async fn cmdline(context: Context, line: &str, chat_id: &mut ChatId) -> Resu

let msg = format!("{arg1} {arg2}");

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?

// Send message over a dedicated SMTP connection
// and measure time.
//
// This can be used to benchmark SMTP connection establishment.
let time_start = std::time::SystemTime::now();

let mut msg = Message::new_text(msg);
chat::send_msg_sync(&context, sel_chat.as_ref().unwrap().get_id(), &mut msg)
.await?;

let time_needed = time_start.elapsed().unwrap_or_default();
println!("Sent message in {time_needed:?}.");
}
}
"sendempty" => {
ensure!(sel_chat.is_some(), "No chat selected.");
Expand Down
5 changes: 5 additions & 0 deletions src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,11 @@ impl Context {
self.scheduler.stop(self).await;
}

/// Returns true if IO scheduler is running.
pub async fn is_running(&self) -> bool {
self.scheduler.is_running().await
}

/// Restarts the IO scheduler if it was running before
/// when it is not running this is an no-op
pub async fn restart_io_if_running(&self) {
Expand Down
12 changes: 6 additions & 6 deletions src/imap/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ impl DerefMut for Client {
}

/// Converts port number to ALPN list.
fn alpn(port: u16) -> &'static [&'static str] {
fn alpn(port: u16) -> &'static str {
if port == 993 {
// Do not request ALPN on standard port.
&[]
""
} else {
&["imap"]
"imap"
}
}

Expand Down Expand Up @@ -242,7 +242,7 @@ impl Client {
let buffered_tcp_stream = client.into_inner();
let tcp_stream = buffered_tcp_stream.into_inner();

let tls_stream = wrap_tls(strict_tls, host, &[], tcp_stream)
let tls_stream = wrap_tls(strict_tls, host, addr.port(), "", tcp_stream)
.await
.context("STARTTLS upgrade failed")?;

Expand All @@ -262,7 +262,7 @@ impl Client {
let proxy_stream = proxy_config
.connect(context, domain, port, strict_tls)
.await?;
let tls_stream = wrap_tls(strict_tls, domain, alpn(port), proxy_stream).await?;
let tls_stream = wrap_tls(strict_tls, domain, port, alpn(port), proxy_stream).await?;
let buffered_stream = BufWriter::new(tls_stream);
let session_stream: Box<dyn SessionStream> = Box::new(buffered_stream);
let mut client = Client::new(session_stream);
Expand Down Expand Up @@ -315,7 +315,7 @@ impl Client {
let buffered_proxy_stream = client.into_inner();
let proxy_stream = buffered_proxy_stream.into_inner();

let tls_stream = wrap_tls(strict_tls, hostname, &[], proxy_stream)
let tls_stream = wrap_tls(strict_tls, hostname, port, "", proxy_stream)
.await
.context("STARTTLS upgrade failed")?;
let buffered_stream = BufWriter::new(tls_stream);
Expand Down
4 changes: 2 additions & 2 deletions src/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,10 @@ pub(crate) async fn connect_tls_inner(
addr: SocketAddr,
host: &str,
strict_tls: bool,
alpn: &[&str],
alpn: &str,
) -> Result<impl SessionStream> {
let tcp_stream = connect_tcp_inner(addr).await?;
let tls_stream = wrap_tls(strict_tls, host, alpn, tcp_stream).await?;
let tls_stream = wrap_tls(strict_tls, host, addr.port(), alpn, tcp_stream).await?;
Ok(tls_stream)
}

Expand Down
4 changes: 2 additions & 2 deletions src/net/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,11 @@ where
let proxy_stream = proxy_config
.connect(context, host, port, load_cache)
.await?;
let tls_stream = wrap_rustls(host, &[], proxy_stream).await?;
let tls_stream = wrap_rustls(host, port, "", proxy_stream).await?;
Box::new(tls_stream)
} else {
let tcp_stream = crate::net::connect_tcp(context, host, port, load_cache).await?;
let tls_stream = wrap_rustls(host, &[], tcp_stream).await?;
let tls_stream = wrap_rustls(host, port, "", tcp_stream).await?;
Box::new(tls_stream)
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/net/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,8 @@ impl ProxyConfig {
load_cache,
)
.await?;
let tls_stream = wrap_rustls(&https_config.host, &[], tcp_stream).await?;
let tls_stream =
wrap_rustls(&https_config.host, https_config.port, "", tcp_stream).await?;
let auth = if let Some((username, password)) = &https_config.user_password {
Some((username.as_str(), password.as_str()))
} else {
Expand Down
66 changes: 59 additions & 7 deletions src/net/tls.rs
Original file line number Diff line number Diff line change
@@ -1,27 +1,38 @@
//! TLS support.
use std::collections::HashMap;
use std::sync::Arc;

use anyhow::Result;
use once_cell::sync::Lazy;
use parking_lot::Mutex;

use crate::net::session::SessionStream;

use tokio_rustls::rustls::client::ClientSessionStore;

pub async fn wrap_tls(
strict_tls: bool,
hostname: &str,
alpn: &[&str],
port: u16,
alpn: &str,
stream: impl SessionStream + 'static,
) -> Result<impl SessionStream> {
if strict_tls {
let tls_stream = wrap_rustls(hostname, alpn, stream).await?;
let tls_stream = wrap_rustls(hostname, port, alpn, stream).await?;
let boxed_stream: Box<dyn SessionStream> = Box::new(tls_stream);
Ok(boxed_stream)
} else {
// We use native_tls because it accepts 1024-bit RSA keys.
// Rustls does not support them even if
// certificate checks are disabled: <https://github.com/rustls/rustls/issues/234>.
let alpns = if alpn.is_empty() {
Box::from([])
} else {
Box::from([alpn])
};
let tls = async_native_tls::TlsConnector::new()
.min_protocol_version(Some(async_native_tls::Protocol::Tlsv12))
.request_alpns(alpn)
.request_alpns(&alpns)
.danger_accept_invalid_hostnames(true)
.danger_accept_invalid_certs(true);
let tls_stream = tls.connect(hostname, stream).await?;
Expand All @@ -30,18 +41,59 @@ pub async fn wrap_tls(
}
}

type SessionMap = HashMap<(u16, String), Arc<dyn ClientSessionStore>>;

/// Map to store TLS session tickets.
///
/// Tickets are separated by port and ALPN
/// to avoid trying to use Postfix ticket for Dovecot and vice versa.
/// Doing so would not be a security issue,
/// but wastes the ticket and the opportunity to resume TLS session unnecessarily.
/// Rustls takes care of separating tickets that belong to different domain names.
static RESUMPTION_STORE: Lazy<Mutex<SessionMap>> = Lazy::new(Default::default);

pub async fn wrap_rustls(
hostname: &str,
alpn: &[&str],
port: u16,
alpn: &str,
stream: impl SessionStream,
) -> Result<impl SessionStream> {
let mut root_cert_store = rustls::RootCertStore::empty();
let mut root_cert_store = tokio_rustls::rustls::RootCertStore::empty();
root_cert_store.extend(webpki_roots::TLS_SERVER_ROOTS.iter().cloned());

let mut config = rustls::ClientConfig::builder()
let mut config = tokio_rustls::rustls::ClientConfig::builder()
.with_root_certificates(root_cert_store)
.with_no_client_auth();
config.alpn_protocols = alpn.iter().map(|s| s.as_bytes().to_vec()).collect();
config.alpn_protocols = if alpn.is_empty() {
vec![]
} else {
vec![alpn.as_bytes().to_vec()]
};

// Enable TLS 1.3 session resumption
// as defined in <https://www.rfc-editor.org/rfc/rfc8446#section-2.2>.
//
// Obsolete TLS 1.2 mechanisms defined in RFC 5246
// and RFC 5077 have worse security
// and are not worth increasing
// attack surface: <https://words.filippo.io/we-need-to-talk-about-session-tickets/>.
let resumption_store = Arc::clone(
RESUMPTION_STORE
.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.

// but we want to create multiple caches
// to separate them by port and ALPN.
Arc::new(tokio_rustls::rustls::client::ClientSessionMemoryCache::new(
256,
))
}),
);

let resumption = tokio_rustls::rustls::client::Resumption::store(resumption_store)
.tls12_resumption(tokio_rustls::rustls::client::Tls12Resumption::Disabled);
config.resumption = resumption;

let tls = tokio_rustls::TlsConnector::from(Arc::new(config));
let name = rustls_pki_types::ServerName::try_from(hostname)?.to_owned();
Expand Down
14 changes: 7 additions & 7 deletions src/smtp/connect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,13 @@ use crate::net::{
use crate::oauth2::get_oauth2_access_token;
use crate::tools::time;

/// Converts port number to ALPN list.
fn alpn(port: u16) -> &'static [&'static str] {
/// Converts port number to ALPN.
fn alpn(port: u16) -> &'static str {
if port == 465 {
// Do not request ALPN on standard port.
&[]
""
} else {
&["smtp"]
"smtp"
}
}

Expand Down Expand Up @@ -225,7 +225,7 @@ async fn connect_secure_proxy(
let proxy_stream = proxy_config
.connect(context, hostname, port, strict_tls)
.await?;
let tls_stream = wrap_tls(strict_tls, hostname, alpn(port), proxy_stream).await?;
let tls_stream = wrap_tls(strict_tls, hostname, port, alpn(port), proxy_stream).await?;
let mut buffered_stream = BufStream::new(tls_stream);
skip_smtp_greeting(&mut buffered_stream).await?;
let session_stream: Box<dyn SessionBufStream> = Box::new(buffered_stream);
Expand All @@ -248,7 +248,7 @@ async fn connect_starttls_proxy(
skip_smtp_greeting(&mut buffered_stream).await?;
let transport = new_smtp_transport(buffered_stream).await?;
let tcp_stream = transport.starttls().await?.into_inner();
let tls_stream = wrap_tls(strict_tls, hostname, &[], tcp_stream)
let tls_stream = wrap_tls(strict_tls, hostname, port, "", tcp_stream)
.await
.context("STARTTLS upgrade failed")?;
let buffered_stream = BufStream::new(tls_stream);
Expand Down Expand Up @@ -293,7 +293,7 @@ async fn connect_starttls(
skip_smtp_greeting(&mut buffered_stream).await?;
let transport = new_smtp_transport(buffered_stream).await?;
let tcp_stream = transport.starttls().await?.into_inner();
let tls_stream = wrap_tls(strict_tls, host, &[], tcp_stream)
let tls_stream = wrap_tls(strict_tls, host, addr.port(), "", tcp_stream)
.await
.context("STARTTLS upgrade failed")?;

Expand Down
Loading