diff --git a/docs/SECURITY.md b/docs/SECURITY.md index fad5fda7..3271412a 100644 --- a/docs/SECURITY.md +++ b/docs/SECURITY.md @@ -54,6 +54,7 @@ Keep treats the Nostr relay as untrusted transport, not a trusted party. Coordin - **What a relay (or a TLS man-in-the-middle) can still do** is limited to metadata analysis (which keys talk, and when) and availability attacks (dropping, delaying, or censoring messages). It cannot read or forge coordination. - **For high assurance, run your own relay.** Self-hosting a relay (privkey's [`wisp`](https://github.com/privkeyio/wisp) is designed for this) and pointing your quorum at it removes the third-party relay from the trust and metadata surface entirely; binding it to a private, authenticated network (e.g. a WireGuard mesh) also removes reliance on relay TLS. This is the model the keep-node appliance uses. - **Certificate pinning is available as opt-in defense-in-depth** for the metadata/availability surface when coordinating over an external `wss://` relay (`verify_relay_certificate`, `PinningServerCertVerifier`). It hardens against a relay-TLS man-in-the-middle observing metadata or impersonating the relay endpoint; it is not required for confidentiality or integrity, which the two properties above already provide. +- **Strict pinning closes the first-use window.** By default a relay with no recorded pin is trusted on first connection (TOFU) and its SPKI hash is pinned for subsequent connections, so the very first connection is the exposed one. Enabling strict pinning (the "Strict pinning" toggle under Settings → TLS certificate pinning on desktop, `set_strict_cert_pinning` on mobile) rejects any `wss://` relay that has no recorded pin instead of trusting it. For high-assurance deployments, provision pins out-of-band before enabling strict mode: obtain each relay's SPKI SHA-256 (e.g. `openssl s_client -connect relay.example.com:443 -servername relay.example.com /dev/null | openssl x509 -pubkey -noout | openssl pkey -pubin -outform der | openssl dgst -sha256 -hex`) and place it in the pin store (`cert-pins.json` in the Keep data directory on desktop, mapping hostname to one or more lowercase hex SPKI hashes, e.g. `{"relay.example.com":"<64-hex>"}` or `{"relay.example.com":["",""]}`; `stage_certificate_pin` on mobile). The current check runs as a pre-flight TLS probe before the relay session is established: with strict mode on it rejects a relay that presents an un-pinned key, so a persistent man-in-the-middle presenting the same rogue certificate to every connection is refused. It does not yet re-verify the pin inside the live coordination handshake itself, so a selective attacker able to present one certificate to the probe and a different one to the data connection is not caught by the probe alone; installing the pin verifier on the live connection is tracked separately. For the strongest guarantee, run your own relay on an authenticated private network (above) rather than relying on relay TLS. ## Reporting Vulnerabilities diff --git a/keep-desktop/src/app/config.rs b/keep-desktop/src/app/config.rs index a03c9da0..1dd0ca35 100644 --- a/keep-desktop/src/app/config.rs +++ b/keep-desktop/src/app/config.rs @@ -22,6 +22,13 @@ pub struct Settings { pub bunker_auto_start: bool, #[serde(default)] pub local_signer_auto_start: bool, + /// Strict certificate pinning: when enabled, a `wss://` relay with no + /// pre-provisioned pin is rejected (fail-closed) instead of trusted on + /// first use, closing the first-connection MitM window plain TOFU leaves + /// open. Opt-in and off by default: enable only after every relay's pin + /// has been recorded, or new connections to un-pinned relays will fail. + #[serde(default)] + pub strict_cert_pinning: bool, } fn default_auto_lock_secs() -> u64 { @@ -42,6 +49,7 @@ impl Default for Settings { start_minimized: false, bunker_auto_start: false, local_signer_auto_start: false, + strict_cert_pinning: false, } } } @@ -188,3 +196,27 @@ pub(crate) fn migrate_json_config_to_vault(keep: &keep_core::Keep, keep_path: &s tracing::info!("Migrated relay/proxy config from JSON files to vault"); } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn settings_without_strict_pinning_field_default_to_off() { + // A settings.json written before the field existed must keep parsing + // and must not silently enable strict mode. + let legacy = r#"{"auto_lock_secs":300,"clipboard_clear_secs":30}"#; + let s: Settings = serde_json::from_str(legacy).unwrap(); + assert!(!s.strict_cert_pinning); + } + + #[test] + fn strict_pinning_round_trips_through_serde() { + let mut s = Settings::default(); + assert!(!s.strict_cert_pinning); + s.strict_cert_pinning = true; + let json = serde_json::to_string(&s).unwrap(); + let back: Settings = serde_json::from_str(&json).unwrap(); + assert!(back.strict_cert_pinning); + } +} diff --git a/keep-desktop/src/app/helpers.rs b/keep-desktop/src/app/helpers.rs index 491aa03f..9a7a809b 100644 --- a/keep-desktop/src/app/helpers.rs +++ b/keep-desktop/src/app/helpers.rs @@ -187,6 +187,7 @@ impl App { }, certificate_pins: self.certificate_pins.clone(), keep_path: self.keep_path.clone(), + require_pinned: self.settings.strict_cert_pinning, } } diff --git a/keep-desktop/src/app/mod.rs b/keep-desktop/src/app/mod.rs index 76a6ec18..e38af62a 100644 --- a/keep-desktop/src/app/mod.rs +++ b/keep-desktop/src/app/mod.rs @@ -950,6 +950,7 @@ mod tests { app.settings.start_minimized, false, Vec::new(), + app.settings.strict_cert_pinning, )); } @@ -985,6 +986,29 @@ mod tests { assert!(!app.settings.minimize_to_tray); } + #[test] + fn strict_pinning_toggle_updates_settings_and_screen_and_defaults_off() { + use crate::screen::settings; + let s = default_settings(); + assert!( + !s.strict_cert_pinning, + "strict pinning must be opt-in (default off)" + ); + let mut app = App::test_new(s, false); + set_settings_screen(&mut app); + + let _task = app.handle_settings_message_new(settings::Message::StrictPinningToggled(true)); + assert!(app.settings.strict_cert_pinning); + if let Screen::Settings(scr) = &app.screen { + assert!(scr.strict_cert_pinning); + } else { + panic!("expected settings screen"); + } + + let _task = app.handle_settings_message_new(settings::Message::StrictPinningToggled(false)); + assert!(!app.settings.strict_cert_pinning); + } + #[test] fn enable_minimize_to_tray_setting() { use crate::screen::settings; @@ -1116,6 +1140,7 @@ mod tests { start_minimized: true, bunker_auto_start: false, local_signer_auto_start: false, + strict_cert_pinning: false, }; let json = serde_json::to_string(&s).unwrap(); let parsed: Settings = serde_json::from_str(&json).unwrap(); diff --git a/keep-desktop/src/app/navigation.rs b/keep-desktop/src/app/navigation.rs index 2243fa16..6269390f 100644 --- a/keep-desktop/src/app/navigation.rs +++ b/keep-desktop/src/app/navigation.rs @@ -138,6 +138,7 @@ impl App { self.settings.start_minimized, self.has_tray, self.cert_pin_display_entries(), + self.settings.strict_cert_pinning, )); Task::none() } diff --git a/keep-desktop/src/app/settings.rs b/keep-desktop/src/app/settings.rs index 445f0464..e2ea47e2 100644 --- a/keep-desktop/src/app/settings.rs +++ b/keep-desktop/src/app/settings.rs @@ -124,6 +124,12 @@ impl App { s.start_minimized = v; } } + Event::StrictPinningToggled(v) => { + self.settings.strict_cert_pinning = v; + if let Screen::Settings(s) = &mut self.screen { + s.strict_cert_pinning = v; + } + } Event::KillSwitchActivate => { return self.handle_kill_switch_activate(); } diff --git a/keep-desktop/src/bunker_service.rs b/keep-desktop/src/bunker_service.rs index ecc9959a..3165281b 100644 --- a/keep-desktop/src/bunker_service.rs +++ b/keep-desktop/src/bunker_service.rs @@ -455,12 +455,18 @@ impl App { let kill_switch = self.kill_switch.clone(); let cert_pins = self.certificate_pins.clone(); let keep_path = self.keep_path.clone(); + let require_pinned = self.settings.strict_cert_pinning; Task::perform( async move { use crate::message::ConnectionError; - crate::frost::verify_relay_certificates(&relay_urls, &cert_pins, &keep_path) - .await?; + crate::frost::verify_relay_certificates( + &relay_urls, + &cert_pins, + &keep_path, + require_pinned, + ) + .await?; let keyring = tokio::task::spawn_blocking(move || extract_keyring(&keep_arc)) .await diff --git a/keep-desktop/src/frost.rs b/keep-desktop/src/frost.rs index 2a9e8040..c2e967d3 100644 --- a/keep-desktop/src/frost.rs +++ b/keep-desktop/src/frost.rs @@ -52,6 +52,7 @@ pub(crate) async fn verify_relay_certificates( relay_urls: &[String], certificate_pins: &Mutex, keep_path: &std::path::Path, + require_pinned: bool, ) -> Result<(), crate::message::ConnectionError> { use crate::message::ConnectionError; // Fail closed: refuse to connect when the on-disk pin store exists but is @@ -74,22 +75,23 @@ pub(crate) async fn verify_relay_certificates( .lock() .map_err(|_| ConnectionError::Other("Pin lock poisoned".to_string()))? .clone(); - // require_pinned=false: trust-on-first-use. A strict-mode config toggle - // that passes true is tracked as a follow-up. - let (_hash, new_pin) = keep_frost_net::verify_relay_certificate(url, &pins_snapshot, false) - .await - .map_err(|e| match e { - keep_frost_net::FrostNetError::CertificatePinMismatch { - hostname, - expected, - actual, - } => ConnectionError::PinMismatch(crate::message::PinMismatchInfo { - hostname, - expected, - actual, - }), - other => ConnectionError::Other(format!("{other}")), - })?; + // `require_pinned` comes from the strict-pinning setting: false is + // trust-on-first-use; true rejects any relay with no recorded pin. + let (_hash, new_pin) = + keep_frost_net::verify_relay_certificate(url, &pins_snapshot, require_pinned) + .await + .map_err(|e| match e { + keep_frost_net::FrostNetError::CertificatePinMismatch { + hostname, + expected, + actual, + } => ConnectionError::PinMismatch(crate::message::PinMismatchInfo { + hostname, + expected, + actual, + }), + other => ConnectionError::Other(format!("{other}")), + })?; if let Some((hostname, hash)) = new_pin { let mut guard = certificate_pins .lock() @@ -149,6 +151,9 @@ pub(crate) struct NetworkConfig { pub session_timeout: Option, pub certificate_pins: Arc>, pub keep_path: std::path::PathBuf, + /// Strict pinning policy: reject `wss://` relays with no recorded pin + /// instead of trusting on first use. + pub require_pinned: bool, } #[derive(Clone)] @@ -299,7 +304,13 @@ pub(crate) async fn setup_frost_node( let nonce_store_path = keep_path.join("frost-nonces"); keep_frost_net::install_default_crypto_provider(); - verify_relay_certificates(&relay_urls, &net.certificate_pins, &net.keep_path).await?; + verify_relay_certificates( + &relay_urls, + &net.certificate_pins, + &net.keep_path, + net.require_pinned, + ) + .await?; let nonce_store = keep_frost_net::FileNonceStore::new(&nonce_store_path) .map_err(|e| format!("Failed to create nonce store: {e}"))?; diff --git a/keep-desktop/src/screen/settings.rs b/keep-desktop/src/screen/settings.rs index 39e7b015..5ee4c6bb 100644 --- a/keep-desktop/src/screen/settings.rs +++ b/keep-desktop/src/screen/settings.rs @@ -20,6 +20,7 @@ pub enum Message { ProxyPortChanged(String), MinimizeToTrayToggled(bool), StartMinimizedToggled(bool), + StrictPinningToggled(bool), KillSwitchRequestConfirm, KillSwitchCancelConfirm, KillSwitchActivate, @@ -58,6 +59,9 @@ impl fmt::Debug for Message { Self::StartMinimizedToggled(v) => { f.debug_tuple("StartMinimizedToggled").field(v).finish() } + Self::StrictPinningToggled(v) => { + f.debug_tuple("StrictPinningToggled").field(v).finish() + } Self::KillSwitchRequestConfirm => f.write_str("KillSwitchRequestConfirm"), Self::KillSwitchCancelConfirm => f.write_str("KillSwitchCancelConfirm"), Self::KillSwitchActivate => f.write_str("KillSwitchActivate"), @@ -92,6 +96,7 @@ pub enum Event { ProxyPortChanged(u16), MinimizeToTrayToggled(bool), StartMinimizedToggled(bool), + StrictPinningToggled(bool), KillSwitchActivate, KillSwitchDeactivate(Zeroizing), CertPinClear(String), @@ -133,6 +138,7 @@ pub struct SettingsScreen { pub start_minimized: bool, pub has_tray: bool, pub certificate_pins: Vec<(String, String)>, + pub strict_cert_pinning: bool, clear_all_pins_confirm: bool, backup_active: bool, backup_passphrase: Zeroizing, @@ -164,6 +170,7 @@ impl SettingsScreen { start_minimized: bool, has_tray: bool, certificate_pins: Vec<(String, String)>, + strict_cert_pinning: bool, ) -> Self { Self { auto_lock_secs, @@ -181,6 +188,7 @@ impl SettingsScreen { start_minimized, has_tray, certificate_pins, + strict_cert_pinning, clear_all_pins_confirm: false, backup_active: false, backup_passphrase: Zeroizing::new(String::new()), @@ -214,6 +222,7 @@ impl SettingsScreen { } Message::MinimizeToTrayToggled(v) => Some(Event::MinimizeToTrayToggled(v)), Message::StartMinimizedToggled(v) => Some(Event::StartMinimizedToggled(v)), + Message::StrictPinningToggled(v) => Some(Event::StrictPinningToggled(v)), Message::KillSwitchRequestConfirm => { self.kill_switch_confirm = true; None @@ -726,9 +735,24 @@ impl SettingsScreen { } fn cert_pins_card(&self) -> Element<'_, Message> { + let strict_btn = toggle_button( + self.strict_cert_pinning, + Message::StrictPinningToggled(!self.strict_cert_pinning), + ); + let mode_caption = if self.strict_cert_pinning { + "Strict: relays with no recorded pin are rejected; provision pins first" + } else { + "Pins are set on first connection to each relay (TOFU)" + }; let mut col = column![ theme::label("TLS certificate pinning"), - theme::muted("Pins are set on first connection to each relay (TOFU)"), + theme::muted(mode_caption), + row![ + theme::muted("Strict pinning (reject relays with no recorded pin)"), + strict_btn + ] + .spacing(theme::space::SM) + .align_y(iced::Alignment::Center), ] .spacing(theme::space::SM); diff --git a/keep-frost-net/src/cert_pin.rs b/keep-frost-net/src/cert_pin.rs index 78c62c76..8e6db24f 100644 --- a/keep-frost-net/src/cert_pin.rs +++ b/keep-frost-net/src/cert_pin.rs @@ -298,11 +298,11 @@ pub(crate) fn evaluate_pin( if expected.is_empty() { if require_pinned { // Strict mode: refuse to trust an un-pre-provisioned host, closing - // the first-connection MitM window that plain TOFU leaves open. - return Err(FrostNetError::CertificatePinMismatch { + // the first-connection MitM window that plain TOFU leaves open. A + // distinct error (not a mismatch) lets callers tell "no pin yet, + // provision one" apart from "the pinned key changed". + return Err(FrostNetError::CertificatePinMissing { hostname: hostname.to_string(), - expected: "a pre-provisioned pin (strict cert pinning enabled)".into(), - actual: hex::encode(spki_hash), }); } // Trust-on-first-use: no pin yet, surface the observed hash to pin. @@ -358,7 +358,7 @@ mod tests { fn strict_mode_rejects_unpinned_host() { // Strict: an un-pre-provisioned host is rejected, not trusted-on-first-use. let err = evaluate_pin("relay.example.com", [7u8; 32], &[], true).unwrap_err(); - assert!(matches!(err, FrostNetError::CertificatePinMismatch { .. })); + assert!(matches!(err, FrostNetError::CertificatePinMissing { .. })); } #[test] diff --git a/keep-frost-net/src/error.rs b/keep-frost-net/src/error.rs index fb040287..51a39bed 100644 --- a/keep-frost-net/src/error.rs +++ b/keep-frost-net/src/error.rs @@ -36,6 +36,12 @@ pub enum FrostNetError { actual: String, }, + #[error( + "No certificate pin recorded for {hostname} and strict pinning is enabled; \ + provision the relay's pin or disable strict pinning" + )] + CertificatePinMissing { hostname: String }, + #[error("Timeout: {0}")] Timeout(String), diff --git a/keep-mobile/src/lib.rs b/keep-mobile/src/lib.rs index 081fbce9..7648a9d9 100644 --- a/keep-mobile/src/lib.rs +++ b/keep-mobile/src/lib.rs @@ -2750,7 +2750,12 @@ impl KeepMobile { } } } - Err(e @ keep_frost_net::FrostNetError::CertificatePinMismatch { .. }) => { + Err( + e @ (keep_frost_net::FrostNetError::CertificatePinMismatch { .. } + | keep_frost_net::FrostNetError::CertificatePinMissing { .. }), + ) => { + // Fail closed on both a changed pin and, under strict + // mode, an un-provisioned relay; do not silently skip. return Err(e.into()); } Err(e) => {