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
1 change: 1 addition & 0 deletions docs/SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 2>/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":["<hex1>","<hex2>"]}`; `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

Expand Down
32 changes: 32 additions & 0 deletions keep-desktop/src/app/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -42,6 +49,7 @@ impl Default for Settings {
start_minimized: false,
bunker_auto_start: false,
local_signer_auto_start: false,
strict_cert_pinning: false,
}
}
}
Expand Down Expand Up @@ -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);
}
}
1 change: 1 addition & 0 deletions keep-desktop/src/app/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}

Expand Down
25 changes: 25 additions & 0 deletions keep-desktop/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -950,6 +950,7 @@ mod tests {
app.settings.start_minimized,
false,
Vec::new(),
app.settings.strict_cert_pinning,
));
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Expand Down
1 change: 1 addition & 0 deletions keep-desktop/src/app/navigation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
6 changes: 6 additions & 0 deletions keep-desktop/src/app/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
10 changes: 8 additions & 2 deletions keep-desktop/src/bunker_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
45 changes: 28 additions & 17 deletions keep-desktop/src/frost.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub(crate) async fn verify_relay_certificates(
relay_urls: &[String],
certificate_pins: &Mutex<keep_frost_net::CertificatePinSet>,
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
Expand All @@ -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()
Expand Down Expand Up @@ -149,6 +151,9 @@ pub(crate) struct NetworkConfig {
pub session_timeout: Option<Duration>,
pub certificate_pins: Arc<Mutex<keep_frost_net::CertificatePinSet>>,
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)]
Expand Down Expand Up @@ -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}"))?;
Expand Down
26 changes: 25 additions & 1 deletion keep-desktop/src/screen/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ pub enum Message {
ProxyPortChanged(String),
MinimizeToTrayToggled(bool),
StartMinimizedToggled(bool),
StrictPinningToggled(bool),
KillSwitchRequestConfirm,
KillSwitchCancelConfirm,
KillSwitchActivate,
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -92,6 +96,7 @@ pub enum Event {
ProxyPortChanged(u16),
MinimizeToTrayToggled(bool),
StartMinimizedToggled(bool),
StrictPinningToggled(bool),
KillSwitchActivate,
KillSwitchDeactivate(Zeroizing<String>),
CertPinClear(String),
Expand Down Expand Up @@ -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<String>,
Expand Down Expand Up @@ -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,
Expand All @@ -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()),
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);

Expand Down
10 changes: 5 additions & 5 deletions keep-frost-net/src/cert_pin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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]
Expand Down
6 changes: 6 additions & 0 deletions keep-frost-net/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),

Expand Down
7 changes: 6 additions & 1 deletion keep-mobile/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down