Skip to content
Merged
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
10 changes: 9 additions & 1 deletion src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<RwLock<Option<Iroh>>>,

Expand Down Expand Up @@ -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),
Expand Down
8 changes: 8 additions & 0 deletions src/imap/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -282,6 +284,8 @@ impl Client {
"",
tcp_stream,
&context.tls_session_store,
&context.spki_hash_store,
&context.sql,
)
.await
.context("STARTTLS upgrade failed")?;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -373,6 +379,8 @@ impl Client {
"",
proxy_stream,
&context.tls_session_store,
&context.spki_hash_store,
&context.sql,
)
.await
.context("STARTTLS upgrade failed")?;
Expand Down
6 changes: 5 additions & 1 deletion src/net.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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<impl SessionStream + 'static> {
let use_sni = true;
let tcp_stream = connect_tcp_inner(addr).await?;
Expand All @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions src/net/http.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ where
"",
proxy_stream,
&context.tls_session_store,
&context.spki_hash_store,
&context.sql,
)
.await?;
Box::new(tls_stream)
Expand All @@ -99,6 +101,8 @@ where
"",
tcp_stream,
&context.tls_session_store,
&context.spki_hash_store,
&context.sql,
)
.await?;
Box::new(tls_stream)
Expand Down
2 changes: 2 additions & 0 deletions src/net/proxy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,6 +436,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 {
Expand Down
61 changes: 45 additions & 16 deletions src/net/tls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)]
Comment thread
j-g00da marked this conversation as resolved.
pub async fn wrap_tls<'a>(
strict_tls: bool,
hostname: &str,
Expand All @@ -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<impl SessionStream + 'a> {
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<dyn SessionStream> = Box::new(tls_stream);
Ok(boxed_stream)
} else {
Expand Down Expand Up @@ -94,16 +112,19 @@ impl TlsSessionStore {
}
}

#[expect(clippy::too_many_arguments)]
pub async fn wrap_rustls<'a>(
hostname: &str,
port: u16,
use_sni: bool,
alpn: &str,
stream: impl SessionStream + 'a,
tls_session_store: &TlsSessionStore,
spki_hash_store: &SpkiHashStore,
sql: &Sql,
) -> Result<impl SessionStream + 'a> {
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)
Expand All @@ -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)
}
87 changes: 77 additions & 10 deletions src/net/tls/danger.rs
Original file line number Diff line number Diff line change
@@ -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;

use crate::net::tls::spki::spki_hash;

#[derive(Debug)]
pub(super) struct NoCertificateVerification();
pub(super) struct CustomCertificateVerifier {
/// Root certificates.
root_cert_store: RootCertStore,

/// Expected SPKI hash as a base64 of SHA-256.
spki_hash: Option<String>,
}

impl NoCertificateVerification {
pub(super) fn new() -> Self {
Self()
impl CustomCertificateVerifier {
pub(super) fn new(spki_hash: Option<String>) -> 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:
// <https://letsencrypt.org/2025/08/06/ocsp-service-has-reached-end-of-life>.
// 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: <https://github.com/rustls/webpki/issues/217>.
_ocsp_response: &[u8],
_now: UnixTime,
now: UnixTime,
) -> Result<rustls::client::danger::ServerCertVerified, rustls::Error> {
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:
Comment thread
link2xt marked this conversation as resolved.
// <https://docs.rs/rustls/0.23.37/rustls/client/fn.verify_server_cert_signed_by_trust_anchor.html>
// 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())
}

Expand Down
Loading