From 7133497f08a8de361c7854c5e23709cac0346fd1 Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 31 Jul 2026 13:26:56 -0500 Subject: [PATCH 1/2] fix(core): reject cleartext Plain TLS to non-loopback hosts AccountConfig::validate silently accepted TlsMode::Plain for imap/smtp against any host, so a misconfig could send credentials in cleartext to a remote server. Plain is now rejected unless the host is loopback (127.0.0.1, ::1, or localhost); ImplicitTls/StartTls are unaffected. Refs #297 --- crates/nuncio-core/src/config.rs | 90 +++++++++++++++++++ .../tests/accounts_lifecycle_e2e_test.rs | 2 +- 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/crates/nuncio-core/src/config.rs b/crates/nuncio-core/src/config.rs index 5cbb012..acadd19 100644 --- a/crates/nuncio-core/src/config.rs +++ b/crates/nuncio-core/src/config.rs @@ -61,6 +61,17 @@ pub enum ConfigError { /// A DAV-protocol account's collection URL is not a valid http(s) URL. #[error("collection_url must be a valid http(s) URL")] InvalidCollectionUrl, + /// Cleartext `TlsMode::Plain` was requested against a non-loopback host, + /// which would send credentials over the network unencrypted. + #[error( + "{endpoint} host '{host}' is not loopback; refusing cleartext Plain TLS mode to a remote host" + )] + CleartextToRemoteHost { + /// Which endpoint the rejected setting applies to (e.g. "imap", "smtp"). + endpoint: &'static str, + /// The offending non-loopback host. + host: String, + }, } /// Complete configuration schema for a mail & calendar account. @@ -102,6 +113,19 @@ pub struct AccountConfig { pub collection_url: String, } +/// Whether `host` refers to the local loopback interface, either as a +/// literal IP address (`127.0.0.1`, `::1`, ...) or the conventional +/// `localhost` name. Used to allow cleartext `TlsMode::Plain` only against +/// local test servers, never against a remote host. +fn is_loopback_host(host: &str) -> bool { + let host = host.trim(); + if host.eq_ignore_ascii_case("localhost") { + return true; + } + host.parse::() + .is_ok_and(|ip| ip.is_loopback()) +} + impl AccountConfig { /// Minimum allowed background sync interval (10 seconds). pub const MIN_SYNC_INTERVAL_SECS: u64 = 10; @@ -155,6 +179,18 @@ impl AccountConfig { if self.smtp_port == 0 { return Err(ConfigError::InvalidPort); } + if self.imap_tls_mode == TlsMode::Plain && !is_loopback_host(&self.server_host) { + return Err(ConfigError::CleartextToRemoteHost { + endpoint: "imap", + host: self.server_host.clone(), + }); + } + if self.smtp_tls_mode == TlsMode::Plain && !is_loopback_host(&self.smtp_host) { + return Err(ConfigError::CleartextToRemoteHost { + endpoint: "smtp", + host: self.smtp_host.clone(), + }); + } } if self.keyring_secret_key.trim().is_empty() { return Err(ConfigError::EmptyField { @@ -358,6 +394,52 @@ mod tests { ); } + #[test] + fn plain_imap_to_remote_host_fails_validation() { + let mut config = valid_account(); + config.imap_tls_mode = TlsMode::Plain; + assert_eq!( + config.validate().unwrap_err(), + ConfigError::CleartextToRemoteHost { + endpoint: "imap", + host: "jmap.nuncio.mx".to_string(), + } + ); + } + + #[test] + fn plain_smtp_to_remote_host_fails_validation() { + let mut config = valid_account(); + config.smtp_tls_mode = TlsMode::Plain; + assert_eq!( + config.validate().unwrap_err(), + ConfigError::CleartextToRemoteHost { + endpoint: "smtp", + host: "smtp.nuncio.mx".to_string(), + } + ); + } + + #[test] + fn plain_imap_and_smtp_to_loopback_hosts_pass_validation() { + for host in ["127.0.0.1", "localhost", "::1", "LOCALHOST"] { + let mut config = valid_account(); + config.server_host = host.to_string(); + config.smtp_host = host.to_string(); + config.imap_tls_mode = TlsMode::Plain; + config.smtp_tls_mode = TlsMode::Plain; + assert!(config.validate().is_ok(), "host {host} should be allowed"); + } + } + + #[test] + fn implicit_tls_and_starttls_to_remote_host_pass_validation() { + let mut config = valid_account(); + config.imap_tls_mode = TlsMode::ImplicitTls; + config.smtp_tls_mode = TlsMode::StartTls; + assert!(config.validate().is_ok()); + } + #[test] fn account_protocol_serde_roundtrip() { let jmap_json = serde_json::to_string(&AccountProtocol::Jmap).unwrap(); @@ -415,5 +497,13 @@ mod tests { ConfigError::SyncIntervalTooShort.to_string(), "sync interval must be at least 10 seconds" ); + assert_eq!( + ConfigError::CleartextToRemoteHost { + endpoint: "imap", + host: "mail.example.com".to_string(), + } + .to_string(), + "imap host 'mail.example.com' is not loopback; refusing cleartext Plain TLS mode to a remote host" + ); } } diff --git a/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs b/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs index a6dd608..10c5b42 100644 --- a/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs +++ b/crates/nunciod/tests/accounts_lifecycle_e2e_test.rs @@ -87,7 +87,7 @@ fn sample_account_config() -> AccountConfig { smtp_tls_mode: TlsMode::Plain.into(), keyring_secret_key: KEYRING_KEY.to_string(), sync_interval_secs: 60, - smtp_host: "smtp.nuncio.mx".to_string(), + smtp_host: "127.0.0.1".to_string(), smtp_port: 25, collection_url: String::new(), } -- 2.54.0 From f2766b7d3bef066f3e8458089025d9ad69801295 Mon Sep 17 00:00:00 2001 From: James Maes Date: Fri, 31 Jul 2026 13:32:56 -0500 Subject: [PATCH 2/2] fix(core): reject cleartext CalDAV collection_url on remote hosts The CalDAV collection_url validation only checked the scheme prefix, so http:///... passed even though it sends the account's Basic-auth credentials in cleartext. Reuse the same loopback-only rule as the imap/smtp TlsMode::Plain check: plain http is now accepted only when the URL's host is loopback (127.0.0.0/8, ::1, or localhost); https to any host, and http to a loopback host, remain valid. Refs #297 --- crates/nuncio-core/src/config.rs | 57 ++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/crates/nuncio-core/src/config.rs b/crates/nuncio-core/src/config.rs index acadd19..df39c9c 100644 --- a/crates/nuncio-core/src/config.rs +++ b/crates/nuncio-core/src/config.rs @@ -126,6 +126,22 @@ fn is_loopback_host(host: &str) -> bool { .is_ok_and(|ip| ip.is_loopback()) } +/// Extract the host portion of an `http://`/`https://` URL, without pulling +/// in a full URL-parsing dependency for this one field. Handles a bracketed +/// IPv6 literal (`[::1]`) as well as a plain hostname/IPv4 literal, and +/// stops at the first `:` (port), `/` (path), `?`, or `#` that follows. +fn url_host(url: &str) -> Option<&str> { + let rest = url + .strip_prefix("https://") + .or_else(|| url.strip_prefix("http://"))?; + if let Some(bracketed) = rest.strip_prefix('[') { + let end = bracketed.find(']')?; + return Some(&bracketed[..end]); + } + let end = rest.find(['/', ':', '?', '#']).unwrap_or(rest.len()); + Some(&rest[..end]) +} + impl AccountConfig { /// Minimum allowed background sync interval (10 seconds). pub const MIN_SYNC_INTERVAL_SECS: u64 = 10; @@ -164,6 +180,18 @@ impl AccountConfig { if !(url.starts_with("http://") || url.starts_with("https://")) { return Err(ConfigError::InvalidCollectionUrl); } + // Plain `http://` is only tolerated against a loopback host (a + // local test server); any remote host must use `https://` to + // avoid sending the account's Basic-auth credentials in cleartext. + if url.starts_with("http://") { + let host = url_host(url).unwrap_or_default(); + if !is_loopback_host(host) { + return Err(ConfigError::CleartextToRemoteHost { + endpoint: "caldav", + host: host.to_string(), + }); + } + } } else { if self.server_host.trim().is_empty() { return Err(ConfigError::EmptyField { @@ -275,6 +303,35 @@ mod tests { ); } + #[test] + fn caldav_account_rejects_cleartext_http_to_remote_host() { + let mut config = valid_caldav_account(); + config.collection_url = "http://remote/dav/".to_string(); + assert_eq!( + config.validate().unwrap_err(), + ConfigError::CleartextToRemoteHost { + endpoint: "caldav", + host: "remote".to_string(), + } + ); + } + + #[test] + fn caldav_account_accepts_https_to_remote_host() { + let mut config = valid_caldav_account(); + config.collection_url = "https://remote/dav/".to_string(); + assert!(config.validate().is_ok()); + } + + #[test] + fn caldav_account_accepts_cleartext_http_to_loopback_host() { + for url in ["http://127.0.0.1/dav/", "http://localhost/dav/"] { + let mut config = valid_caldav_account(); + config.collection_url = url.to_string(); + assert!(config.validate().is_ok(), "url {url} should be allowed"); + } + } + #[test] fn account_protocol_is_dav_only_for_dav_protocols() { assert!(AccountProtocol::CalDav.is_dav()); -- 2.54.0