From 82924952fb8898aab0748f28e24bb4129965c241 Mon Sep 17 00:00:00 2001 From: link2xt Date: Sun, 29 Mar 2026 17:47:12 +0200 Subject: [PATCH] feat: allow TLS connections with invalid certificate if the key is unchanged This change weakens TLS checks. Every time we make a successful TLS connection, we remember public key hash from the certificate in relation to the hostname. If later we connect to the same hostname and the public key does not change, we skip checking certificate chain. This way we will still connect successfully even if certificate expires or becomes invalid for another reason, but keeps the key. We always check that certificate corresponds to the hostname. We also do this for certificates starting with _ where we allow self-signed certificates, so self-signed certificates with mismatching domains are not allowed. Previously we did not check this for domains starting with _. --- src/context.rs | 10 +++- src/imap/client.rs | 8 +++ src/net.rs | 6 +- src/net/http.rs | 4 ++ src/net/proxy.rs | 2 + src/net/tls.rs | 61 ++++++++++++++------ src/net/tls/danger.rs | 89 ++++++++++++++++++++++++---- src/net/tls/spki.rs | 131 ++++++++++++++++++++++++++++++++++++++++++ src/smtp/connect.rs | 35 ++++++++++- src/sql.rs | 8 +++ src/sql/migrations.rs | 16 ++++++ 11 files changed, 338 insertions(+), 32 deletions(-) create mode 100644 src/net/tls/spki.rs diff --git a/src/context.rs b/src/context.rs index 92c1a36ce..64fb08ec2 100644 --- a/src/context.rs +++ b/src/context.rs @@ -25,7 +25,7 @@ use crate::key::self_fingerprint; use crate::log::warn; use crate::logged_debug_assert; use crate::message::{self, MessageState, MsgId}; -use crate::net::tls::TlsSessionStore; +use crate::net::tls::{SpkiHashStore, TlsSessionStore}; use crate::peer_channels::Iroh; use crate::push::PushSubscriber; use crate::quota::QuotaInfo; @@ -308,6 +308,13 @@ pub struct InnerContext { /// TLS session resumption cache. pub(crate) tls_session_store: TlsSessionStore, + /// Store for TLS SPKI hashes. + /// + /// Used to remember public keys + /// of TLS certificates to accept them + /// even after they expire. + pub(crate) spki_hash_store: SpkiHashStore, + /// Iroh for realtime peer channels. pub(crate) iroh: Arc>>, @@ -511,6 +518,7 @@ impl Context { push_subscriber, push_subscribed: AtomicBool::new(false), tls_session_store: TlsSessionStore::new(), + spki_hash_store: SpkiHashStore::new(), iroh: Arc::new(RwLock::new(None)), self_fingerprint: OnceLock::new(), self_public_key: Mutex::new(None), diff --git a/src/imap/client.rs b/src/imap/client.rs index 335557fce..adf6e8db1 100644 --- a/src/imap/client.rs +++ b/src/imap/client.rs @@ -220,6 +220,8 @@ impl Client { alpn(addr.port()), logging_stream, &context.tls_session_store, + &context.spki_hash_store, + &context.sql, ) .await?; let buffered_stream = BufWriter::new(tls_stream); @@ -282,6 +284,8 @@ impl Client { "", tcp_stream, &context.tls_session_store, + &context.spki_hash_store, + &context.sql, ) .await .context("STARTTLS upgrade failed")?; @@ -310,6 +314,8 @@ impl Client { alpn(port), proxy_stream, &context.tls_session_store, + &context.spki_hash_store, + &context.sql, ) .await?; let buffered_stream = BufWriter::new(tls_stream); @@ -373,6 +379,8 @@ impl Client { "", proxy_stream, &context.tls_session_store, + &context.spki_hash_store, + &context.sql, ) .await .context("STARTTLS upgrade failed")?; diff --git a/src/net.rs b/src/net.rs index 3ff181efd..c46f26017 100644 --- a/src/net.rs +++ b/src/net.rs @@ -12,7 +12,7 @@ use tokio_io_timeout::TimeoutStream; use crate::context::Context; use crate::net::session::SessionStream; -use crate::net::tls::TlsSessionStore; +use crate::net::tls::{SpkiHashStore, TlsSessionStore}; use crate::sql::Sql; use crate::tools::time; @@ -130,6 +130,8 @@ pub(crate) async fn connect_tls_inner( strict_tls: bool, alpn: &str, tls_session_store: &TlsSessionStore, + spki_hash_store: &SpkiHashStore, + sql: &Sql, ) -> Result { let use_sni = true; let tcp_stream = connect_tcp_inner(addr).await?; @@ -141,6 +143,8 @@ pub(crate) async fn connect_tls_inner( alpn, tcp_stream, tls_session_store, + spki_hash_store, + sql, ) .await?; Ok(tls_stream) diff --git a/src/net/http.rs b/src/net/http.rs index 2ab1a297c..062245f39 100644 --- a/src/net/http.rs +++ b/src/net/http.rs @@ -87,6 +87,8 @@ where "", proxy_stream, &context.tls_session_store, + &context.spki_hash_store, + &context.sql, ) .await?; Box::new(tls_stream) @@ -99,6 +101,8 @@ where "", tcp_stream, &context.tls_session_store, + &context.spki_hash_store, + &context.sql, ) .await?; Box::new(tls_stream) diff --git a/src/net/proxy.rs b/src/net/proxy.rs index c2e5a9429..021f15109 100644 --- a/src/net/proxy.rs +++ b/src/net/proxy.rs @@ -434,6 +434,8 @@ impl ProxyConfig { "", tcp_stream, &context.tls_session_store, + &context.spki_hash_store, + &context.sql, ) .await?; let auth = if let Some((username, password)) = &https_config.user_password { diff --git a/src/net/tls.rs b/src/net/tls.rs index 0013f23be..cd8629e9f 100644 --- a/src/net/tls.rs +++ b/src/net/tls.rs @@ -6,13 +6,20 @@ use std::sync::Arc; use anyhow::Result; use crate::net::session::SessionStream; +use crate::sql::Sql; +use crate::tools::time; use tokio_rustls::rustls; use tokio_rustls::rustls::client::ClientSessionStore; +use tokio_rustls::rustls::server::ParsedCertificate; mod danger; -use danger::NoCertificateVerification; +use danger::CustomCertificateVerifier; +mod spki; +pub use spki::SpkiHashStore; + +#[expect(clippy::too_many_arguments)] pub async fn wrap_tls<'a>( strict_tls: bool, hostname: &str, @@ -21,10 +28,21 @@ pub async fn wrap_tls<'a>( alpn: &str, stream: impl SessionStream + 'static, tls_session_store: &TlsSessionStore, + spki_hash_store: &SpkiHashStore, + sql: &Sql, ) -> Result { if strict_tls { - let tls_stream = - wrap_rustls(hostname, port, use_sni, alpn, stream, tls_session_store).await?; + let tls_stream = wrap_rustls( + hostname, + port, + use_sni, + alpn, + stream, + tls_session_store, + spki_hash_store, + sql, + ) + .await?; let boxed_stream: Box = Box::new(tls_stream); Ok(boxed_stream) } else { @@ -94,6 +112,7 @@ impl TlsSessionStore { } } +#[expect(clippy::too_many_arguments)] pub async fn wrap_rustls<'a>( hostname: &str, port: u16, @@ -101,9 +120,11 @@ pub async fn wrap_rustls<'a>( alpn: &str, stream: impl SessionStream + 'a, tls_session_store: &TlsSessionStore, + spki_hash_store: &SpkiHashStore, + sql: &Sql, ) -> Result { - let mut root_cert_store = rustls::RootCertStore::empty(); - root_cert_store.extend(webpki_roots::TLS_SERVER_ROOTS.iter().cloned()); + let root_cert_store = + rustls::RootCertStore::from_iter(webpki_roots::TLS_SERVER_ROOTS.iter().cloned()); let mut config = rustls::ClientConfig::builder() .with_root_certificates(root_cert_store) @@ -127,20 +148,28 @@ pub async fn wrap_rustls<'a>( config.resumption = resumption; config.enable_sni = use_sni; - // Do not verify certificates for hostnames starting with `_`. - // They are used for servers with self-signed certificates, e.g. for local testing. - // Hostnames starting with `_` can have only self-signed TLS certificates or wildcard certificates. - // It is not possible to get valid non-wildcard TLS certificates because CA/Browser Forum requirements - // explicitly state that domains should start with a letter, digit or hyphen: - // https://github.com/cabforum/servercert/blob/24f38fd4765e019db8bb1a8c56bf63c7115ce0b0/docs/BR.md - if hostname.starts_with("_") { - config - .dangerous() - .set_certificate_verifier(Arc::new(NoCertificateVerification::new())); - } + config + .dangerous() + .set_certificate_verifier(Arc::new(CustomCertificateVerifier::new( + spki_hash_store.get_spki_hash(hostname, sql).await?, + ))); let tls = tokio_rustls::TlsConnector::from(Arc::new(config)); let name = tokio_rustls::rustls::pki_types::ServerName::try_from(hostname)?.to_owned(); let tls_stream = tls.connect(name, stream).await?; + + // Successfully connected. + // Remember SPKI hash to accept it later if certificate expires. + let (_io, client_connection) = tls_stream.get_ref(); + if let Some(end_entity) = client_connection + .peer_certificates() + .and_then(|certs| certs.first()) + { + let now = time(); + let parsed_certificate = ParsedCertificate::try_from(end_entity)?; + let spki = parsed_certificate.subject_public_key_info(); + spki_hash_store.save_spki(hostname, &spki, sql, now).await?; + } + Ok(tls_stream) } diff --git a/src/net/tls/danger.rs b/src/net/tls/danger.rs index bd0267ddc..e757680be 100644 --- a/src/net/tls/danger.rs +++ b/src/net/tls/danger.rs @@ -1,26 +1,93 @@ -//! Dangerous TLS implementation of accepting invalid certificates for Rustls. +//! Custom TLS verification. +//! +//! We want to accept expired certificates. +use rustls::RootCertStore; +use rustls::client::{verify_server_cert_signed_by_trust_anchor, verify_server_name}; use rustls::pki_types::{CertificateDer, ServerName, UnixTime}; +use rustls::server::ParsedCertificate; use tokio_rustls::rustls; -#[derive(Debug)] -pub(super) struct NoCertificateVerification(); +use crate::net::tls::spki::spki_hash; -impl NoCertificateVerification { - pub(super) fn new() -> Self { - Self() +#[derive(Debug)] +pub(super) struct CustomCertificateVerifier { + /// Root certificates. + root_cert_store: RootCertStore, + + /// Expected SPKI hash as a base64 of SHA-256. + spki_hash: Option, +} + +impl CustomCertificateVerifier { + pub(super) fn new(spki_hash: Option) -> Self { + let root_cert_store = + RootCertStore::from_iter(webpki_roots::TLS_SERVER_ROOTS.iter().cloned()); + Self { + root_cert_store, + spki_hash, + } } } -impl rustls::client::danger::ServerCertVerifier for NoCertificateVerification { +impl rustls::client::danger::ServerCertVerifier for CustomCertificateVerifier { fn verify_server_cert( &self, - _end_entity: &CertificateDer<'_>, - _intermediates: &[CertificateDer<'_>], - _server_name: &ServerName<'_>, + end_entity: &CertificateDer<'_>, + intermediates: &[CertificateDer<'_>], + server_name: &ServerName<'_>, + // OCSP is a certificate revocation mechanism that is intentionally ignored. + // It is practically not used and is essentially deprecated + // in favor of Certificate Revocation Lists. + // Let's Encrypt has disabled OCSP responders in 2025: + // . + // Theoretically checking of stapled OCSP responses could be implemented, + // but it is not interesting to implement it because it is not used + // by the servers: . _ocsp_response: &[u8], - _now: UnixTime, + now: UnixTime, ) -> Result { + let parsed_certificate = ParsedCertificate::try_from(end_entity)?; + + let spki = parsed_certificate.subject_public_key_info(); + + let provider = rustls::crypto::ring::default_provider(); + + if let ServerName::DnsName(dns_name) = server_name + && dns_name.as_ref().starts_with("_") + { + // Do not verify certificates for hostnames starting with `_`. + // They are used for servers with self-signed certificates, e.g. for local testing. + // Hostnames starting with `_` can have only self-signed TLS certificates or wildcard certificates. + // It is not possible to get valid non-wildcard TLS certificates because CA/Browser Forum requirements + // explicitly state that domains should start with a letter, digit or hyphen: + // https://github.com/cabforum/servercert/blob/24f38fd4765e019db8bb1a8c56bf63c7115ce0b0/docs/BR.md + } else if let Some(hash) = &self.spki_hash + && spki_hash(&spki) == *hash + { + // Last time we successfully connected to this hostname with TLS checks, + // SPKI had this hash. + // It does not matter if certificate has now expired. + } else { + // verify_server_cert_signed_by_trust_anchor does no revocation checking: + // + // We don't do it either. + verify_server_cert_signed_by_trust_anchor( + &parsed_certificate, + &self.root_cert_store, + intermediates, + now, + provider.signature_verification_algorithms.all, + )?; + } + + // Verify server name unconditionally. + // + // We do this even for self-signed certificates when hostname starts with `_` + // so we don't try to connect to captive portals + // and fail on MITM certificates if they are generated once + // and reused for all hostnames. + verify_server_name(&parsed_certificate, server_name)?; Ok(rustls::client::danger::ServerCertVerified::assertion()) } diff --git a/src/net/tls/spki.rs b/src/net/tls/spki.rs new file mode 100644 index 000000000..b76fb0964 --- /dev/null +++ b/src/net/tls/spki.rs @@ -0,0 +1,131 @@ +//! SPKI hash storage. +//! +//! We store hashes of Subject Public Key Info from TLS certificates +//! after successful connection to allow connecting when +//! server certificate expires as long as the key is not changed. + +use std::collections::BTreeMap; + +use anyhow::Context as _; +use anyhow::Result; +use base64::Engine as _; +use parking_lot::RwLock; +use sha2::{Digest, Sha256}; +use tokio_rustls::rustls::pki_types::SubjectPublicKeyInfoDer; + +use crate::sql::Sql; +use crate::tools::time; + +/// Calculates Subject Public Key Info SHA-256 hash and returns it as base64. +/// +/// This is the same format as used in . +/// You can calculate the same hash for any remote host with +/// ```sh +/// openssl s_client -connect "$HOST:993" -servername "$HOST" /dev/null | +/// openssl x509 -pubkey -noout | +/// openssl pkey -pubin -outform der | +/// openssl dgst -sha256 -binary | +/// openssl enc -base64 +/// ``` +pub fn spki_hash(spki: &SubjectPublicKeyInfoDer) -> String { + let spki_hash = Sha256::digest(spki); + base64::engine::general_purpose::STANDARD.encode(spki_hash) +} + +/// Write-through cache for SPKI hashes. +#[derive(Debug)] +pub struct SpkiHashStore { + /// Map from hostnames to base64 of SHA-256 hashes. + pub hash_store: RwLock>, +} + +impl SpkiHashStore { + pub fn new() -> Self { + Self { + hash_store: RwLock::new(BTreeMap::new()), + } + } + + /// Returns base64 of SPKI hash if we have previously successfully connected to given hostname. + pub async fn get_spki_hash(&self, hostname: &str, sql: &Sql) -> Result> { + if let Some(hash) = self.hash_store.read().get(hostname).cloned() { + return Ok(Some(hash)); + } + + match sql + .query_row_optional( + "SELECT spki_hash FROM tls_spki WHERE host=?", + (hostname,), + |row| { + let spki_hash: String = row.get(0)?; + Ok(spki_hash) + }, + ) + .await? + { + Some(hash) => { + self.hash_store + .write() + .insert(hostname.to_string(), hash.clone()); + Ok(Some(hash)) + } + None => Ok(None), + } + } + + /// Saves SPKI hash after successful connection. + pub async fn save_spki( + &self, + hostname: &str, + spki: &SubjectPublicKeyInfoDer<'_>, + sql: &Sql, + timestamp: i64, + ) -> Result<()> { + let hash = spki_hash(spki); + self.hash_store + .write() + .insert(hostname.to_string(), hash.clone()); + sql.execute( + "INSERT OR REPLACE INTO tls_spki (host, spki_hash, timestamp) VALUES (?, ?, ?)", + (hostname, hash, timestamp), + ) + .await?; + Ok(()) + } + + /// Removes stale entries from SPKI storage. + pub async fn cleanup(&self, sql: &Sql) -> Result<()> { + let now = time(); + let removed_hosts = sql + .query_map_vec( + "DELETE FROM tls_spki WHERE ? > timestamp + ? RETURNING host", + (now, 30 * 24 * 60 * 60), + |row| { + let host: String = row.get(0)?; + Ok(host) + }, + ) + .await + .context("DELETE FROM tls_spki")?; + + // Fix timestamps that happen to be in the future + // if we had clock set incorrectly when the timestamp was stored. + // Otherwise entry may take more than 30 days to expire. + sql.execute( + "UPDATE tls_spki SET timestamp = ?1 WHERE timestamp > ?1", + (now,), + ) + .await?; + + let mut lock = self.hash_store.write(); + for host in removed_hosts { + // We may accidentally remove a host that was added + // to the cache after SQL query but before we got + // the write lock on `hash_store`. + // It is unlikely and will only result + // in additional SQL query next time. + lock.remove(&host); + } + Ok(()) + } +} diff --git a/src/smtp/connect.rs b/src/smtp/connect.rs index e535b77c9..28215a4ca 100644 --- a/src/smtp/connect.rs +++ b/src/smtp/connect.rs @@ -11,11 +11,12 @@ use crate::log::warn; use crate::net::dns::{lookup_host_with_cache, update_connect_timestamp}; use crate::net::proxy::ProxyConfig; use crate::net::session::SessionBufStream; -use crate::net::tls::{TlsSessionStore, wrap_tls}; +use crate::net::tls::{SpkiHashStore, TlsSessionStore, wrap_tls}; use crate::net::{ connect_tcp_inner, connect_tls_inner, run_connection_attempts, update_connection_history, }; use crate::oauth2::get_oauth2_access_token; +use crate::sql::Sql; use crate::tools::time; use crate::transport::ConnectionCandidate; use crate::transport::ConnectionSecurity; @@ -111,10 +112,26 @@ async fn connection_attempt( ); let res = match security { ConnectionSecurity::Tls => { - connect_secure(resolved_addr, host, strict_tls, &context.tls_session_store).await + connect_secure( + resolved_addr, + host, + strict_tls, + &context.tls_session_store, + &context.spki_hash_store, + &context.sql, + ) + .await } ConnectionSecurity::Starttls => { - connect_starttls(resolved_addr, host, strict_tls, &context.tls_session_store).await + connect_starttls( + resolved_addr, + host, + strict_tls, + &context.tls_session_store, + &context.spki_hash_store, + &context.sql, + ) + .await } ConnectionSecurity::Plain => connect_insecure(resolved_addr).await, }; @@ -240,6 +257,8 @@ async fn connect_secure_proxy( alpn(port), proxy_stream, &context.tls_session_store, + &context.spki_hash_store, + &context.sql, ) .await?; let mut buffered_stream = BufStream::new(tls_stream); @@ -273,6 +292,8 @@ async fn connect_starttls_proxy( "", tcp_stream, &context.tls_session_store, + &context.spki_hash_store, + &context.sql, ) .await .context("STARTTLS upgrade failed")?; @@ -299,6 +320,8 @@ async fn connect_secure( hostname: &str, strict_tls: bool, tls_session_store: &TlsSessionStore, + spki_hash_store: &SpkiHashStore, + sql: &Sql, ) -> Result> { let tls_stream = connect_tls_inner( addr, @@ -306,6 +329,8 @@ async fn connect_secure( strict_tls, alpn(addr.port()), tls_session_store, + spki_hash_store, + sql, ) .await?; let mut buffered_stream = BufStream::new(tls_stream); @@ -319,6 +344,8 @@ async fn connect_starttls( host: &str, strict_tls: bool, tls_session_store: &TlsSessionStore, + spki_hash_store: &SpkiHashStore, + sql: &Sql, ) -> Result> { let use_sni = false; let tcp_stream = connect_tcp_inner(addr).await?; @@ -336,6 +363,8 @@ async fn connect_starttls( "", tcp_stream, tls_session_store, + spki_hash_store, + sql, ) .await .context("STARTTLS upgrade failed")?; diff --git a/src/sql.rs b/src/sql.rs index a59d56550..78c6df3d8 100644 --- a/src/sql.rs +++ b/src/sql.rs @@ -874,6 +874,14 @@ pub async fn housekeeping(context: &Context) -> Result<()> { .log_err(context) .ok(); + context + .spki_hash_store + .cleanup(&context.sql) + .await + .context("Failed to prune SPKI store") + .log_err(context) + .ok(); + // Cleanup `imap` and `imap_sync` entries for deleted transports. // // Transports may be deleted directly or via sync messages, diff --git a/src/sql/migrations.rs b/src/sql/migrations.rs index 23cc6be11..249697dd4 100644 --- a/src/sql/migrations.rs +++ b/src/sql/migrations.rs @@ -2360,6 +2360,22 @@ ALTER TABLE contacts ADD COLUMN name_normalized TEXT; .await?; } + inc_and_check(&mut migration_version, 151)?; + if dbversion < migration_version { + sql.execute_migration( + "CREATE TABLE tls_spki ( + host TEXT NOT NULL UNIQUE, + spki_hash TEXT NOT NULL, -- base64 of SPKI SHA-256 hash + timestamp INTEGER NOT NULL -- timestamp of the last time we have seen this key + ) STRICT; + -- Index on host column is created implicitly because of UNIQUE constraint. + CREATE INDEX tls_spki_index_timestamp ON tls_spki (timestamp); + ", + migration_version, + ) + .await?; + } + let new_version = sql .get_raw_config_int(VERSION_CFG) .await?