diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 42c5ee08b..74a04a7ef 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -10,18 +10,18 @@ repos: - repo: https://github.com/pre-commit/pre-commit-hooks rev: v6.0.0 hooks: - - id: fix-byte-order-marker - - id: check-case-conflict - - id: check-symlinks - - id: destroyed-symlinks - - id: check-yaml - - id: check-json - - id: check-toml - - id: end-of-file-fixer - - id: trailing-whitespace - - id: mixed-line-ending - - id: check-merge-conflict - - id: check-added-large-files + - id: fix-byte-order-marker + - id: check-case-conflict + - id: check-symlinks + - id: destroyed-symlinks + - id: check-yaml + - id: check-json + - id: check-toml + - id: end-of-file-fixer + - id: trailing-whitespace + - id: mixed-line-ending + - id: check-merge-conflict + - id: check-added-large-files - repo: https://github.com/crate-ci/typos rev: v1.50.2 @@ -29,12 +29,12 @@ repos: - id: typos - id: typos name: commit-msg-typos - stages: [commit-msg] + stages: [ commit-msg ] - repo: https://github.com/crate-ci/committed rev: v1.1.11 hooks: - - id: committed + - id: committed - repo: local hooks: @@ -42,10 +42,10 @@ repos: name: cargo fmt entry: cargo +nightly fmt -- language: system - types: [rust] + types: [ rust ] pass_filenames: false stages: - - pre-commit + - pre-commit - repo: local hooks: @@ -54,18 +54,6 @@ repos: entry: cargo clippy -- -D warnings language: system pass_filenames: false - types: [rust] + types: [ rust ] stages: - pre-push - - - repo: local - hooks: - - id: docs-build - name: docs build - entry: npm run docs:build - language: system - pass_filenames: false - files: docs/ - types: [mdx] - stages: - - pre-commit diff --git a/Cargo.toml b/Cargo.toml index 859640d35..45bf2d4b7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -975,6 +975,8 @@ let_underscore_future = { level = "allow", priority = 1 } # rust doesnt understand conduwuit's custom log macros literal_string_with_formatting_args = { level = "allow", priority = 1 } +# This lint is stupid, there's no point defining generics for simple trait params +impl-trait-in-params = { level = "allow", priority = 1 } needless_raw_string_hashes = "allow" diff --git a/conduwuit-example.toml b/conduwuit-example.toml index 9a478fa0c..24c5159ca 100644 --- a/conduwuit-example.toml +++ b/conduwuit-example.toml @@ -891,48 +891,88 @@ #proxy = "none" # Servers listed here will be used to gather public keys of other servers -# (notary trusted key servers). +# (trusted key notary servers), as well as serve as trusted servers for +# other operations like backfill and event fetching. # -# Currently, continuwuity doesn't support inbound batched key requests, so +# Currently, continuwuity doesn't support inbound key requests, so # this list should only contain other Synapse servers. # -# example: ["matrix.org", "tchncs.de"] +# example: ["matrix.org", "starstruck.systems"] +# +# It is possible to restrict which signing keys trusted servers are +# allowed to sign responses with. Without configuring this, all responses +# from trusted servers will be implicitly trusted, which is fine for most +# users. Strict verification is only necessary if TLS interception is part +# of your threat model. Even without strict verification, signatures from +# the originating server must be valid, regardless of whether the trusted +# server's is valid or not. +# +# To enable strict validation, you need to pass a TOML table containing +# the trusted server name, and any public keys (not key IDs!) that you +# expect to sign responses. You can fetch the current public keys a remote +# server uses with `!admin federation get-signing-keys `. +# Only add public keys that aren't expired, and make sure to remove any +# that expire at a later date! +# +# Example: +# ```ignore +# trusted_servers = [{server_name = "starstruck.systems", verify_keys=["Z+WZUaLLgkclXn1iQVzYIiJMjMEyltxg5LG8Dndu34A"]}] +# ``` +# +# You can also mix-and-match strict verification and lax verification +# trusted servers: +# +# ```ignore +# trusted_servers = [ +# {server_name = "starstruck.systems", verify_keys=["Z+WZUaLLgkclXn1iQVzYIiJMjMEyltxg5LG8Dndu34A"]}, +# "matrix.org" +# ] +# ``` +# +# Trusted servers are queried in parallel, so order does not matter. You +# should not have more than five entries. You can set this value to an +# empty list (`[]`) to operate without trusted server assistance, but this +# is discouraged for performance and reliability reasons. # #trusted_servers = ["matrix.org"] -# Whether to query the servers listed in trusted_servers first or query -# the origin server first. For best security, querying the origin server -# first is advised to minimize the exposure to a compromised trusted -# server. For maximum federation/join performance this can be set to true, -# however other options exist to query trusted servers first under -# specific high-load circumstances and should be evaluated before setting -# this to true. +# Whether to prioritise trusted server lookups over origin server lookups. +# +# By default, both notaries and origin servers are queried in parallel, +# and handled on a first-come-first-serve basis, meaning the server always +# uses the fastest response(s). +# +# If you wish to prioritise handling responses from trusted servers +# *before* handling responses from origin servers, set this to true. # #query_trusted_key_servers_first = false -# Whether to query the servers listed in trusted_servers first -# specifically on room joins. This option limits the exposure to a -# compromised trusted server to room joins only. The join operation -# requires gathering keys from many origin servers which can cause -# significant delays. Therefor this defaults to true to mitigate -# unexpected delays out-of-the-box. The security-paranoid or those willing -# to tolerate delays are advised to set this to false. Note that setting -# query_trusted_key_servers_first to true causes this option to be -# ignored. +# Any old signing keys that were used to sign events using this server +# name. # -#query_trusted_key_servers_first_on_join = true - -# Only query trusted servers for keys and never the origin server. This is -# intended for clusters or custom deployments using their trusted_servers -# as forwarding-agents to cache and deduplicate requests. Notary servers -# do not act as forwarding-agents by default, therefor do not enable this -# unless you know exactly what you are doing. +# You do not need to populate this unless you previously used a different +# server software with the same server_name configured, and then have +# migrated to continuwuity. # -#only_query_trusted_key_servers = false - -# Maximum number of keys to request in each trusted server batch query. +# The expiry timestamp should be as exact as you can get it, as making it +# too old may inadvertently invalidate historical events sent by your +# server, and potentially make it impossible for you to use some rooms as +# a result. If your key was compromised, and you set the expiry timestamp +# too late, some forged events may continue to be allowed. # -#trusted_server_batch_size = 1024 +# If not configured, continuwuity will discover its own historical keys +# through trusted servers. For improved security, you can set this to an +# empty map ({}) to disable this behaviour, if you know you have no old +# signing keys. +# +# Example: +# {"ed25519:1"={expired_ts=1790506890859,key="d29vZndvb2Z3b29md29vZg"}} +# +# The map key should be the key ID, expired_ts should be unix +# milliseconds, and the actual key value should be the unpadded base64 +# representation of the public key component of the expired signing key. +# +#old_verify_keys = # Max log level for continuwuity. Allows debug, info, warn, or error. # diff --git a/src/admin/debug/commands.rs b/src/admin/debug/commands.rs index 481990401..438afdef0 100644 --- a/src/admin/debug/commands.rs +++ b/src/admin/debug/commands.rs @@ -694,11 +694,11 @@ impl crate::Context<'_> { | Ok(value) => match self .services .server_keys - .verify_json(&value, &room_version_rules) + .verify_event_json(&value, &room_version_rules) .await { | Err(e) => return Err!("Signature verification failed: {e}"), - | Ok(()) => write!(self, "Signature correct"), + | Ok(_) => write!(self, "Signature correct"), }, } .await @@ -714,7 +714,7 @@ impl crate::Context<'_> { let msg = match self .services .server_keys - .verify_event(&event, &room_version_rules) + .verify_event_json(&event, &room_version_rules) .await { | Err(e) => return Err(e), @@ -849,7 +849,7 @@ impl crate::Context<'_> { for result in remote_state_response.pdus.iter().map(|pdu| { self.services .server_keys - .validate_and_add_event_id(pdu, &room_version_rules) + .verify_event_json_no_fetch_add_event_id(pdu, &room_version_rules) }) { let Ok((event_id, value)) = result.await else { continue; @@ -887,7 +887,7 @@ impl crate::Context<'_> { for result in remote_state_response.auth_chain.iter().map(|pdu| { self.services .server_keys - .validate_and_add_event_id(pdu, &room_version_rules) + .verify_event_json_no_fetch_add_event_id(pdu, &room_version_rules) }) { let Ok((event_id, value)) = result.await else { continue; @@ -943,60 +943,6 @@ impl crate::Context<'_> { .await } - pub(super) async fn get_signing_keys( - &self, - server_name: Option, - notary: Option, - query: bool, - ) -> Result { - let server_name = server_name.unwrap_or_else(|| self.services.server.name.clone()); - - if let Some(notary) = notary { - let signing_keys = self - .services - .server_keys - .notary_request(¬ary, &server_name) - .await?; - - let out = format!("```rs\n{signing_keys:#?}\n```"); - return self.write_str(&out).await; - } - - let signing_keys = if query { - self.services - .server_keys - .server_request(&server_name) - .await? - } else { - self.services - .server_keys - .signing_keys_for(&server_name) - .await? - }; - - let out = format!("```rs\n{signing_keys:#?}\n```"); - self.write_str(&out).await - } - - pub(super) async fn get_verify_keys(&self, server_name: Option) -> Result { - let server_name = server_name.unwrap_or_else(|| self.services.server.name.clone()); - - let keys = self - .services - .server_keys - .verify_keys_for(&server_name) - .await; - - let mut out = String::new(); - writeln!(out, "| Key ID | Public Key |")?; - writeln!(out, "| --- | --- |")?; - for (key_id, key) in keys { - writeln!(out, "| {key_id} | {key:?} |")?; - } - - self.write_str(&out).await - } - pub(super) async fn resolve_true_destination( &self, server_name: OwnedServerName, diff --git a/src/admin/debug/mod.rs b/src/admin/debug/mod.rs index c9d8d443d..f544637ab 100644 --- a/src/admin/debug/mod.rs +++ b/src/admin/debug/mod.rs @@ -103,22 +103,6 @@ pub enum DebugCommand { event_id: OwnedEventId, }, - /// Get and display signing keys from local cache or remote server. - GetSigningKeys { - server_name: Option, - - #[arg(long)] - notary: Option, - - #[arg(short, long)] - query: bool, - }, - - /// Get and display signing keys from local cache or remote server. - GetVerifyKeys { - server_name: Option, - }, - /// Sends a federation request to the remote server's /// `/_matrix/federation/v1/version` endpoint and measures the latency it /// took for the server to respond diff --git a/src/admin/federation/commands.rs b/src/admin/federation/commands.rs index 7f54a1d22..966c7bdcf 100644 --- a/src/admin/federation/commands.rs +++ b/src/admin/federation/commands.rs @@ -2,7 +2,7 @@ use std::fmt::Write; use conduwuit::{Err, Result, utils::response::LimitReadExt}; use futures::StreamExt; -use ruma::{OwnedRoomId, OwnedServerName, OwnedUserId}; +use ruma::{MilliSecondsSinceUnixEpoch, OwnedRoomId, OwnedServerName, OwnedUserId}; use crate::get_room_info; @@ -131,4 +131,30 @@ impl crate::Context<'_> { self.write_str(&format!("Rooms {user_id} shares with us ({num}):\n```\n{body}\n```")) .await } + + pub(super) async fn get_signing_keys(&self, server_name: OwnedServerName) -> Result { + let response = self + .services + .server_keys + .origin_request(server_name, MilliSecondsSinceUnixEpoch::now()) + .await?; + self.write_str("| Key ID | Public Key | Expired (unix milliseconds) |\n") + .await?; + self.write_str("| ------ | ---------- | --------------------------- |\n") + .await?; + for (key_id, key) in &response.verify_keys { + self.write_str(&format!("| {key_id} | {} | |\n", key.key)) + .await?; + } + for (key_id, old_key) in &response.old_verify_keys { + self.write_str(&format!( + "| {key_id} | {} | {:0<13} |\n", + old_key.key, + old_key.expired_ts.get().to_string() + )) + .await?; + } + + Ok(()) + } } diff --git a/src/admin/federation/mod.rs b/src/admin/federation/mod.rs index 4356ef11d..7a5d88c16 100644 --- a/src/admin/federation/mod.rs +++ b/src/admin/federation/mod.rs @@ -39,4 +39,9 @@ pub enum FederationCommand { RemoteUserInRooms { user_id: OwnedUserId, }, + + /// Queries the remote server for their current signing keys. + GetSigningKeys { + server_name: OwnedServerName, + }, } diff --git a/src/admin/processor.rs b/src/admin/processor.rs index 8881b820c..ba935fc7d 100644 --- a/src/admin/processor.rs +++ b/src/admin/processor.rs @@ -32,7 +32,6 @@ use crate::{admin, admin::AdminCommand, context::Context}; #[must_use] pub(super) fn complete(line: &str) -> String { complete_command(AdminCommand::command(), line) } -#[must_use] pub(super) fn dispatch(services: Arc, command: CommandInput) -> ProcessorFuture { Box::pin(handle_command(services, command)) } diff --git a/src/admin/query/globals.rs b/src/admin/query/globals.rs index c62627d48..b55a350ff 100644 --- a/src/admin/query/globals.rs +++ b/src/admin/query/globals.rs @@ -1,6 +1,5 @@ use clap::Subcommand; use conduwuit::Result; -use ruma::OwnedServerName; use crate::Context; @@ -12,12 +11,6 @@ pub enum GlobalsCommand { CurrentCount, LastCheckForAnnouncementsId, - - /// This returns an empty `Ok(BTreeMap<..>)` when there are no keys found - /// for the server. - SigningKeysFor { - origin: OwnedServerName, - }, } /// All the getters and iterators from src/database/key_value/globals.rs @@ -47,13 +40,6 @@ pub(super) async fn process(subcommand: GlobalsCommand, context: &Context<'_>) - .await; let query_time = timer.elapsed(); - write!(context, "Query completed in {query_time:?}:\n\n```rs\n{results:#?}\n```") - }, - | GlobalsCommand::SigningKeysFor { origin } => { - let timer = tokio::time::Instant::now(); - let results = services.server_keys.verify_keys_for(&origin).await; - let query_time = timer.elapsed(); - write!(context, "Query completed in {query_time:?}:\n\n```rs\n{results:#?}\n```") }, } diff --git a/src/admin/room/moderation.rs b/src/admin/room/moderation.rs index 7b6758c01..7168d7571 100644 --- a/src/admin/room/moderation.rs +++ b/src/admin/room/moderation.rs @@ -204,7 +204,7 @@ impl crate::Context<'_> { }, }; - room_ids.push(room_id.clone()); + room_ids.push(room_id); } if room_alias_or_id.is_room_alias_id() { diff --git a/src/api/router.rs b/src/api/router.rs index 9ef053a6d..a7f8c6145 100644 --- a/src/api/router.rs +++ b/src/api/router.rs @@ -200,10 +200,6 @@ pub fn build(router: Router, state: State) -> Router { router = router .ruma_route(&server::get_server_version_route) .route("/_matrix/key/v2/server", get(server::get_server_keys_route)) - .route( - "/_matrix/key/v2/server/{key_id}", - get(server::get_server_keys_deprecated_route), - ) .ruma_route(&server::get_public_rooms_route) .ruma_route(&server::get_public_rooms_filtered_route) .ruma_route(&server::send_transaction_message_route) diff --git a/src/api/router/auth.rs b/src/api/router/auth.rs index cbabf1b77..b29dc2e13 100644 --- a/src/api/router/auth.rs +++ b/src/api/router/auth.rs @@ -1,9 +1,13 @@ use std::any::{Any, TypeId}; -use conduwuit::{Err, Error, Result, err, utils::IterStream}; +use conduwuit::{ + Err, Error, Result, err, + utils::{IterStream, millis_since_unix_epoch}, +}; use http::StatusCode; use ruma::{ - DeviceId, OwnedDeviceId, OwnedServerName, OwnedUserId, UserId, + DeviceId, MilliSecondsSinceUnixEpoch, OwnedDeviceId, OwnedServerName, OwnedUserId, UInt, + UserId, api::{ IncomingRequest, auth_scheme::{ @@ -127,12 +131,18 @@ impl CheckAuth for ServerSignatures { return Err!(Request(Unauthorized("Destination mismatch."))); } + let min_valid_ts = + MilliSecondsSinceUnixEpoch(UInt::new_saturating(millis_since_unix_epoch())); let key = services .server_keys - .get_verify_key(&output.origin, &output.key) + .get_single_verify_key(&output.origin, &output.key, min_valid_ts) .await - .map_err(|e| { - err!(Request(Unauthorized(warn!("Failed to fetch signing keys: {e}")))) + .ok_or_else(|| { + err!(Request(Unauthorized(debug_warn!( + origin=%output.origin, + "Unable to acquire your signing key (ID: {})", + output.key + )))) })?; let keys: PubKeys = [(output.key.to_string(), key.key)].into(); @@ -160,8 +170,9 @@ impl CheckAuth for ServerSignatures { Ok(output.origin) }, - | Err(err) => - Err!(Request(Unauthorized(warn!("Failed to verify X-Matrix header: {err}")))), + | Err(err) => Err!(Request(Unauthorized(debug_warn!( + "Failed to verify X-Matrix header: {err}" + )))), } } } diff --git a/src/api/server/invite.rs b/src/api/server/invite.rs index 630277e2d..f9ad2dbb6 100644 --- a/src/api/server/invite.rs +++ b/src/api/server/invite.rs @@ -324,7 +324,7 @@ async fn validate_invite_state( services .server_keys - .verify_event(&state_event_json, room_version_rules) + .verify_event_json(&state_event_json, room_version_rules) .await .map_err(|e| { err!(Request(InvalidParam("Signature verification failed on invite event: {e}"))) diff --git a/src/api/server/key.rs b/src/api/server/key.rs index 4feb20e3e..b547ea15c 100644 --- a/src/api/server/key.rs +++ b/src/api/server/key.rs @@ -1,10 +1,8 @@ -use std::{ - mem::take, - time::{Duration, SystemTime}, -}; +use std::{collections::BTreeMap, mem::take, time::Duration}; use axum::{Json, extract::State, response::IntoResponse}; use conduwuit::{Result, utils::timepoint_from_now}; +use futures::StreamExt; use ruma::{ MilliSecondsSinceUnixEpoch, api::{ @@ -14,33 +12,62 @@ use ruma::{ assign, serde::Raw, }; +use service::{server_keys, server_keys::in_one_week}; /// # `GET /_matrix/key/v2/server` /// /// Gets the public signing keys of this server. -/// -/// - Matrix does not support invalidating public keys, so the key returned by -/// this will be valid forever. -// Response type for this endpoint is Json because we need to calculate a -// signature for the response pub(crate) async fn get_server_keys_route( State(services): State, ) -> Result { let server_name = services.globals.server_name(); - let active_key_id = services.server_keys.active_key_id(); - let mut all_keys = services.server_keys.verify_keys_for(server_name).await; + let verify_keys = BTreeMap::from([services.server_keys.active_verify_key()]); - let verify_keys = all_keys - .remove_entry(active_key_id) - .expect("active verify_key is missing"); - - let old_verify_keys = all_keys - .into_iter() - .map(|(id, key)| (id, OldVerifyKey::new(expires_ts(), key.key))) - .collect(); + let old_verify_keys = if let Some(k) = &services.config.old_verify_keys { + k.to_owned() + } else { + // We need to figure out any old signing keys we've acquired from + // notaries. + let mut keys = services.server_keys.signing_keys_for(server_name); + let mut old_keys = BTreeMap::new(); + while let Some(resp) = keys.next().await { + // We need to verify that this response is still trusted based on + // the current configuration. It's possible we got this response + // from a notary who we no longer trust. + if resp.valid_until_ts > in_one_week() { + continue; + } + let trusted = services.config.trusted_servers.iter().any(|notary| { + server_keys::Service::verify_server_keys_response( + &resp, + Some((notary.server_name(), notary.verify_keys())), + ) + .is_ok() + }); + if !trusted { + continue; + } + for (old_key_id, old_key) in resp.old_verify_keys { + if old_key.expired_ts > resp.valid_until_ts { + // This key expired in the future which is probably illegal + continue; + } + old_keys + .entry(old_key_id) + .and_modify(|current_old_key: &mut OldVerifyKey| { + // Use the lowest expiry timestamp + if current_old_key.expired_ts > old_key.expired_ts { + *current_old_key = old_key.clone(); + } + }) + .or_insert(old_key); + } + } + old_keys + }; let server_key = assign!(ServerSigningKeys::new(server_name.to_owned(), valid_until_ts()), { - verify_keys: [verify_keys].into(), + verify_keys, old_verify_keys, }); @@ -56,24 +83,7 @@ pub(crate) async fn get_server_keys_route( } fn valid_until_ts() -> MilliSecondsSinceUnixEpoch { - let dur = Duration::from_hours(168); + let dur = Duration::from_hours(12); let timepoint = timepoint_from_now(dur).expect("SystemTime should not overflow"); MilliSecondsSinceUnixEpoch::from_system_time(timepoint).expect("UInt should not overflow") } - -fn expires_ts() -> MilliSecondsSinceUnixEpoch { - let timepoint = SystemTime::now(); - MilliSecondsSinceUnixEpoch::from_system_time(timepoint).expect("UInt should not overflow") -} - -/// # `GET /_matrix/key/v2/server/{keyId}` -/// -/// Gets the public signing keys of this server. -/// -/// - Matrix does not support invalidating public keys, so the key returned by -/// this will be valid forever. -pub(crate) async fn get_server_keys_deprecated_route( - State(services): State, -) -> impl IntoResponse { - get_server_keys_route(State(services)).await -} diff --git a/src/api/server/utils.rs b/src/api/server/utils.rs index d9a67c2d3..a6803eb61 100644 --- a/src/api/server/utils.rs +++ b/src/api/server/utils.rs @@ -142,7 +142,7 @@ pub(crate) async fn validate_any_membership_event( services .server_keys - .verify_event(&pdu, room_version_rules) + .verify_event_json(&pdu, room_version_rules) .await .map_err(|e| { err!(Request(InvalidParam("Signature verification failed on membership event: {e}"))) diff --git a/src/core/config/check.rs b/src/core/config/check.rs index 866d330cf..6990abcff 100644 --- a/src/core/config/check.rs +++ b/src/core/config/check.rs @@ -1,8 +1,8 @@ -use std::env::consts::OS; +use std::{collections::HashSet, env::consts::OS}; use either::Either; use figment::Figment; -use ruma::events::room::server_acl::RoomServerAclEventContent; +use ruma::{OwnedServerName, events::room::server_acl::RoomServerAclEventContent}; use super::DEPRECATED_KEYS; use crate::{Config, Err, Result, Server, debug, debug_info, debug_warn, error, warn}; @@ -318,6 +318,33 @@ pub fn check(config: &Config) -> Result { ); } + if config + .trusted_servers + .iter() + .any(|t| t.server_name() == config.server_name) + && !config.federation_loopback + { + return Err!(Config( + "trusted_servers", + "Your own `server_name` cannot appear in `trusted_servers`" + )); + } + config + .trusted_servers + .iter() + .map(super::TrustedServer::server_name) + .try_fold(HashSet::::new(), |mut servers, server_name| { + if !servers.insert(server_name.to_owned()) { + Err!(Config( + "trusted_servers", + "Duplicate `server_name` in `trusted_servers`: {}", + server_name.as_str() + )) + } else { + Ok(servers) + } + })?; + Ok(()) } diff --git a/src/core/config/mod.rs b/src/core/config/mod.rs index 84193ba1c..3fc9566ca 100644 --- a/src/core/config/mod.rs +++ b/src/core/config/mod.rs @@ -20,8 +20,12 @@ use lettre::message::Mailbox; use openidconnect::{ClientId, ClientSecret, Scope}; use regex::RegexSet; use ruma::{ - OwnedRoomId, OwnedRoomOrAliasId, OwnedServerName, OwnedUserId, RoomVersionId, - api::client::{discovery::discover_support::ContactRole, rtc::RtcTransport}, + OwnedRoomId, OwnedRoomOrAliasId, OwnedServerName, OwnedServerSigningKeyId, OwnedUserId, + RoomVersionId, ServerName, + api::{ + client::{discovery::discover_support::ContactRole, rtc::RtcTransport}, + federation::discovery::OldVerifyKey, + }, profile::ProfileFieldName, serde::Base64, }; @@ -1085,52 +1089,92 @@ pub struct Config { pub proxy: ProxyConfig, /// Servers listed here will be used to gather public keys of other servers - /// (notary trusted key servers). + /// (trusted key notary servers), as well as serve as trusted servers for + /// other operations like backfill and event fetching. /// - /// Currently, continuwuity doesn't support inbound batched key requests, so + /// Currently, continuwuity doesn't support inbound key requests, so /// this list should only contain other Synapse servers. /// - /// example: ["matrix.org", "tchncs.de"] + /// example: ["matrix.org", "starstruck.systems"] + /// + /// It is possible to restrict which signing keys trusted servers are + /// allowed to sign responses with. Without configuring this, all responses + /// from trusted servers will be implicitly trusted, which is fine for most + /// users. Strict verification is only necessary if TLS interception is part + /// of your threat model. Even without strict verification, signatures from + /// the originating server must be valid, regardless of whether the trusted + /// server's is valid or not. + /// + /// To enable strict validation, you need to pass a TOML table containing + /// the trusted server name, and any public keys (not key IDs!) that you + /// expect to sign responses. You can fetch the current public keys a remote + /// server uses with `!admin federation get-signing-keys `. + /// Only add public keys that aren't expired, and make sure to remove any + /// that expire at a later date! + /// + /// Example: + /// ```ignore + /// trusted_servers = [{server_name = "starstruck.systems", verify_keys=["Z+WZUaLLgkclXn1iQVzYIiJMjMEyltxg5LG8Dndu34A"]}] + /// ``` + /// + /// You can also mix-and-match strict verification and lax verification + /// trusted servers: + /// + /// ```ignore + /// trusted_servers = [ + /// {server_name = "starstruck.systems", verify_keys=["Z+WZUaLLgkclXn1iQVzYIiJMjMEyltxg5LG8Dndu34A"]}, + /// "matrix.org" + /// ] + /// ``` + /// + /// Trusted servers are queried in parallel, so order does not matter. You + /// should not have more than five entries. You can set this value to an + /// empty list (`[]`) to operate without trusted server assistance, but this + /// is discouraged for performance and reliability reasons. /// /// default: ["matrix.org"] #[serde(default = "default_trusted_servers")] - pub trusted_servers: Vec, + pub trusted_servers: Vec, - /// Whether to query the servers listed in trusted_servers first or query - /// the origin server first. For best security, querying the origin server - /// first is advised to minimize the exposure to a compromised trusted - /// server. For maximum federation/join performance this can be set to true, - /// however other options exist to query trusted servers first under - /// specific high-load circumstances and should be evaluated before setting - /// this to true. + /// Whether to prioritise trusted server lookups over origin server lookups. + /// + /// By default, both notaries and origin servers are queried in parallel, + /// and handled on a first-come-first-serve basis, meaning the server always + /// uses the fastest response(s). + /// + /// If you wish to prioritise handling responses from trusted servers + /// *before* handling responses from origin servers, set this to true. #[serde(default)] pub query_trusted_key_servers_first: bool, - /// Whether to query the servers listed in trusted_servers first - /// specifically on room joins. This option limits the exposure to a - /// compromised trusted server to room joins only. The join operation - /// requires gathering keys from many origin servers which can cause - /// significant delays. Therefor this defaults to true to mitigate - /// unexpected delays out-of-the-box. The security-paranoid or those willing - /// to tolerate delays are advised to set this to false. Note that setting - /// query_trusted_key_servers_first to true causes this option to be - /// ignored. - #[serde(default = "true_fn")] - pub query_trusted_key_servers_first_on_join: bool, - - /// Only query trusted servers for keys and never the origin server. This is - /// intended for clusters or custom deployments using their trusted_servers - /// as forwarding-agents to cache and deduplicate requests. Notary servers - /// do not act as forwarding-agents by default, therefor do not enable this - /// unless you know exactly what you are doing. - #[serde(default)] - pub only_query_trusted_key_servers: bool, - - /// Maximum number of keys to request in each trusted server batch query. + /// Any old signing keys that were used to sign events using this server + /// name. /// - /// default: 1024 - #[serde(default = "default_trusted_server_batch_size")] - pub trusted_server_batch_size: usize, + /// You do not need to populate this unless you previously used a different + /// server software with the same server_name configured, and then have + /// migrated to continuwuity. + /// + /// The expiry timestamp should be as exact as you can get it, as making it + /// too old may inadvertently invalidate historical events sent by your + /// server, and potentially make it impossible for you to use some rooms as + /// a result. If your key was compromised, and you set the expiry timestamp + /// too late, some forged events may continue to be allowed. + /// + /// If not configured, continuwuity will discover its own historical keys + /// through trusted servers. For improved security, you can set this to an + /// empty map ({}) to disable this behaviour, if you know you have no old + /// signing keys. + /// + /// Example: + /// {"ed25519:1"={expired_ts=1790506890859,key="d29vZndvb2Z3b29md29vZg"}} + /// + /// The map key should be the key ID, expired_ts should be unix + /// milliseconds, and the actual key value should be the unpadded base64 + /// representation of the public key component of the expired signing key. + /// + /// default: + #[serde(default)] + pub old_verify_keys: Option>, /// Max log level for continuwuity. Allows debug, info, warn, or error. /// @@ -2838,6 +2882,45 @@ pub enum OidcProfileKeyImportMode { OnLogin, } +#[derive(Clone, Debug, Deserialize)] +#[serde(untagged)] +pub enum TrustedServer { + Config(TrustedServerConfig), + Name(OwnedServerName), +} + +impl TrustedServer { + #[must_use] + pub fn server_name(&self) -> &ServerName { + match self { + | Self::Name(server_name) => server_name, + | Self::Config(config) => &config.server_name, + } + } + + #[must_use] + pub fn verify_keys(&self) -> &[Base64] { + let Self::Config(config) = self else { + return &[]; + }; + config.verify_keys.as_slice() + } +} + +#[derive(Clone, Debug, Deserialize)] +pub struct TrustedServerConfig { + pub server_name: OwnedServerName, + #[serde(default)] + pub verify_keys: Vec, +} + +impl TrustedServerConfig { + #[must_use] + pub fn new(server_name: OwnedServerName) -> Self { + Self { server_name, verify_keys: Vec::new() } + } +} + const DEPRECATED_KEYS: &[&str] = &[ "cache_capacity", "conduit_cache_capacity_modifier", @@ -3092,8 +3175,17 @@ fn default_otlp_protocol() -> String { "http".to_owned() } fn default_tracing_flame_output_path() -> String { "./tracing.folded".to_owned() } -fn default_trusted_servers() -> Vec { - vec![OwnedServerName::try_from("matrix.org").unwrap()] +fn default_trusted_servers() -> Vec { + // TODO(nex): Once we can be a notary, add maintainer(?) homeservers here. + // Rationale: Users are already running our code, arguably that's a higher + // level of trust than is assigned to notaries in the first place. + // We should still remind everyone that notaries are evil and out to get you + // and to replace this list with servers they actually trust. + let trusted_servers = vec![ruma::owned_server_name!("matrix.org")]; + trusted_servers + .into_iter() + .map(TrustedServer::Name) + .collect() } /// do debug logging by default for debug builds @@ -3230,8 +3322,6 @@ fn parallelism_scaled_u32(val: u32) -> u32 { fn parallelism_scaled(val: usize) -> usize { val.saturating_mul(sys::available_parallelism()) } -fn default_trusted_server_batch_size() -> usize { 256 } - fn default_db_pool_workers() -> usize { sys::available_parallelism() .saturating_mul(4) diff --git a/src/core/error/err.rs b/src/core/error/err.rs index c1ae95654..70f1f82fb 100644 --- a/src/core/error/err.rs +++ b/src/core/error/err.rs @@ -100,7 +100,7 @@ macro_rules! err { }; ($variant:ident($($args:tt)+)) => { - $crate::error::Error::$variant($crate::format_maybe!($($args)+)) + $crate::error::Error::$variant($crate::format_maybe!($($args)+), /* Cow<'static, str> */) }; ($level:ident!($($args:tt)+)) => {{ diff --git a/src/database/maps.rs b/src/database/maps.rs index 063f88320..69e441372 100644 --- a/src/database/maps.rs +++ b/src/database/maps.rs @@ -275,7 +275,7 @@ pub(super) static MAPS: &[Descriptor] = &[ ..descriptor::RANDOM_SMALL }, Descriptor { - name: "server_signingkeys", + name: "servernamekeyid_response", ..descriptor::RANDOM }, Descriptor { diff --git a/src/service/admin/mod.rs b/src/service/admin/mod.rs index f02735e5c..15dc33d50 100644 --- a/src/service/admin/mod.rs +++ b/src/service/admin/mod.rs @@ -399,7 +399,7 @@ impl Service { let mut stream = admin_users; while let Some(user_id) = stream.next().await { - generated_admin_list.push(user_id.clone()); + generated_admin_list.push(user_id); } } } diff --git a/src/service/globals/mod.rs b/src/service/globals/mod.rs index fbbff0131..23c3f8db4 100644 --- a/src/service/globals/mod.rs +++ b/src/service/globals/mod.rs @@ -6,7 +6,7 @@ use async_trait::async_trait; use conduwuit::{Result, Server, SyncRwLock, error, utils::bytes::pretty}; use data::Data; use regex::RegexSet; -use ruma::{OwnedEventId, OwnedRoomAliasId, OwnedServerName, OwnedUserId, ServerName, UserId}; +use ruma::{OwnedEventId, OwnedRoomAliasId, OwnedUserId, ServerName, UserId}; use crate::service; @@ -106,8 +106,6 @@ impl Service { self.server.config.allow_announcements_check } - pub fn trusted_servers(&self) -> &[OwnedServerName] { &self.server.config.trusted_servers } - pub fn turn_password(&self) -> &String { &self.server.config.turn_password } pub fn turn_ttl(&self) -> u64 { self.server.config.turn_ttl } diff --git a/src/service/migrations.rs b/src/service/migrations.rs index 0a5881063..6d1050e58 100644 --- a/src/service/migrations.rs +++ b/src/service/migrations.rs @@ -82,6 +82,7 @@ async fn fresh(services: &Services) -> Result<()> { db["global"].insert(SPLIT_USERID_PASSWORD, []); db["global"].insert(DROP_ROOMSYNCTOKEN_SHORTSTATEHASH, []); db["global"].insert(UNEMBED_UNSIGNED_INFO, []); + db["global"].insert(DROP_OLD_SIGNING_KEYS_STORAGE, []); // Create the admin room and server user on first run info!("Creating admin room and server user"); @@ -272,6 +273,19 @@ async fn migrate(services: &Services) -> Result<()> { )); } + if db["global"] + .get(DROP_OLD_SIGNING_KEYS_STORAGE) + .await + .is_not_found() + { + services + .db + .db + .drop_column("server_signingkeys") + .inspect(|()| services.db["global"].insert(DROP_OLD_SIGNING_KEYS_STORAGE, [])) + .map_err(|e| err!("Failed to drop server_signingkeys: {e:?}"))?; + } + assert_eq!( services.globals.db.database_version().await, DATABASE_VERSION, @@ -987,3 +1001,5 @@ async fn unembed_unsigned_info( Ok(()) } + +const DROP_OLD_SIGNING_KEYS_STORAGE: &str = "drop_server_signingkeys"; diff --git a/src/service/rooms/event_handler/pdu_checks.rs b/src/service/rooms/event_handler/pdu_checks.rs index eeb3ecd64..458ff4f3b 100644 --- a/src/service/rooms/event_handler/pdu_checks.rs +++ b/src/service/rooms/event_handler/pdu_checks.rs @@ -123,7 +123,7 @@ impl super::Service { match self .services .server_keys - .verify_event(&pdu_json, room_version_rules) + .verify_event_json(&pdu_json, room_version_rules) .await { | Ok(ruma::signatures::Verified::All) => { diff --git a/src/service/rooms/membership/mod.rs b/src/service/rooms/membership/mod.rs index 8d014a650..31ba5d5fc 100644 --- a/src/service/rooms/membership/mod.rs +++ b/src/service/rooms/membership/mod.rs @@ -492,7 +492,7 @@ impl Service { let resp_auth = &resp_events.auth_chain; self.services .server_keys - .acquire_events_pubkeys(resp_auth.iter().chain(resp_state.iter())) + .get_events_keys(resp_auth.iter().chain(resp_state.iter()), &room_version_rules) .await; info!("Going through send_join response room_state"); @@ -505,7 +505,7 @@ impl Service { .then(|pdu| { self.services .server_keys - .validate_and_add_event_id_no_fetch(pdu, &room_version_rules) + .verify_event_json_no_fetch_add_event_id(pdu, &room_version_rules) .inspect_err(|e| { debug_warn!( "Could not validate send_join response room_state event: {e:?}" @@ -558,7 +558,7 @@ impl Service { .then(|pdu| { self.services .server_keys - .validate_and_add_event_id_no_fetch(pdu, &room_version_rules) + .verify_event_json_no_fetch_add_event_id(pdu, &room_version_rules) }) .ready_filter_map(Result::ok) .ready_for_each(|(event_id, value)| { diff --git a/src/service/rooms/timeline/backfill.rs b/src/service/rooms/timeline/backfill.rs index 626cb7667..ece996f6e 100644 --- a/src/service/rooms/timeline/backfill.rs +++ b/src/service/rooms/timeline/backfill.rs @@ -288,8 +288,14 @@ impl super::Service { } // Insert all trusted servers in the config - candidate_backfill_servers - .extend(self.services.server.config.trusted_servers.iter().cloned()); + candidate_backfill_servers.extend( + self.services + .server + .config + .trusted_servers + .iter() + .map(|n| n.server_name().to_owned()), + ); // Remove our own name, we can't request backfill from ourselves candidate_backfill_servers.remove(self.services.globals.server_name()); diff --git a/src/service/sending/sender.rs b/src/service/sending/sender.rs index b63e4a661..383891d87 100644 --- a/src/service/sending/sender.rs +++ b/src/service/sending/sender.rs @@ -317,7 +317,7 @@ impl Service { for (dest, events) in txns { if self.server.config.startup_netburst && !events.is_empty() { statuses.insert(dest.clone(), TransactionStatus::Running); - futures.push(self.send_events(dest.clone(), events)); + futures.push(self.send_events(dest, events)); } } } @@ -661,7 +661,7 @@ impl Service { .remove(&user_id) .expect("our read receipts always have the user here"); - let receipt_data = ReceiptData::new(receipt, vec![event_id.clone()]); + let receipt_data = ReceiptData::new(receipt, vec![event_id]); if read.insert(user_id, receipt_data).is_none() { *num = num.saturating_add(1); diff --git a/src/service/server_keys/acquire.rs b/src/service/server_keys/acquire.rs deleted file mode 100644 index 4451f738d..000000000 --- a/src/service/server_keys/acquire.rs +++ /dev/null @@ -1,248 +0,0 @@ -use std::{ - borrow::Borrow, - collections::{BTreeMap, BTreeSet}, - time::Duration, -}; - -use conduwuit::{debug, debug_error, debug_warn, error, info, result::FlatOk, trace, warn}; -use futures::{StreamExt, stream::FuturesUnordered}; -use ruma::{ - CanonicalJsonObject, OwnedServerName, OwnedServerSigningKeyId, ServerName, - ServerSigningKeyId, api::federation::discovery::ServerSigningKeys, serde::Raw, -}; -use serde_json::value::RawValue as RawJsonValue; -use tokio::time::{Instant, timeout_at}; - -use super::key_exists; - -type Batch = BTreeMap>; - -impl super::Service { - /// Returns all verify keys required to verify the incoming events. - pub async fn acquire_events_pubkeys<'a, I>(&self, events: I) - where - I: Iterator> + Send, - { - type Batch = BTreeMap>; - type Signatures = BTreeMap>; - - let mut batch = Batch::new(); - events - .cloned() - .map(Raw::::from_json) - .map(|event| event.get_field::("signatures")) - .filter_map(FlatOk::flat_ok) - .flat_map(IntoIterator::into_iter) - .for_each(|(server, sigs)| { - batch.entry(server).or_default().extend(sigs.into_keys()); - }); - - let batch = batch - .iter() - .map(|(server, keys)| (server.borrow(), keys.iter().map(Borrow::borrow))); - - self.acquire_pubkeys(batch).await; - } - - /// Acquires all the verify keys required in the provided batch, using - /// notaries and direct-fetch where applicable. - pub async fn acquire_pubkeys<'a, S, K>(&self, batch: S) - where - S: Iterator + Send + Clone, - K: Iterator + Send + Clone, - { - let notary_only = self.services.server.config.only_query_trusted_key_servers; - let notary_first_always = self.services.server.config.query_trusted_key_servers_first; - let notary_first_on_join = self - .services - .server - .config - .query_trusted_key_servers_first_on_join; - - let requested_servers = batch.clone().count(); - let requested_keys = batch.clone().flat_map(|(_, key_ids)| key_ids).count(); - - debug!("acquire {requested_keys} keys from {requested_servers}"); - - let mut missing = self.acquire_locals(batch).await; - let mut missing_keys = keys_count(&missing); - let mut missing_servers = missing.len(); - if missing_servers == 0 { - return; - } - - info!("{missing_keys} keys for {missing_servers} servers will be acquired"); - - if notary_first_always || notary_first_on_join { - missing = self.acquire_notary(missing.into_iter()).await; - missing_keys = keys_count(&missing); - missing_servers = missing.len(); - if missing_keys == 0 { - return; - } - - warn!( - "missing {missing_keys} keys for {missing_servers} servers from all notaries \ - first" - ); - } - - if !notary_only { - missing = self.acquire_origins(missing.into_iter()).await; - missing_keys = keys_count(&missing); - missing_servers = missing.len(); - if missing_keys == 0 { - return; - } - - debug_warn!("missing {missing_keys} keys for {missing_servers} servers unreachable"); - } - - if !notary_first_always && !notary_first_on_join { - missing = self.acquire_notary(missing.into_iter()).await; - missing_keys = keys_count(&missing); - missing_servers = missing.len(); - if missing_keys == 0 { - return; - } - - debug_warn!( - "still missing {missing_keys} keys for {missing_servers} servers from all \ - notaries." - ); - } - - if missing_keys > 0 { - warn!( - "did not obtain {missing_keys} keys for {missing_servers} servers out of \ - {requested_keys} total keys for {requested_servers} total servers." - ); - } - - for (server, key_ids) in missing { - debug_warn!(%server, ?key_ids, "missing"); - } - } - - /// Filters the batch to return only keys which we do not already have - /// locally. - async fn acquire_locals<'a, S, K>(&self, batch: S) -> Batch - where - S: Iterator + Send, - K: Iterator + Send, - { - let mut missing = Batch::new(); - for (server, key_ids) in batch { - for key_id in key_ids { - if !self.verify_key_exists(server, key_id).await { - missing - .entry(server.into()) - .or_default() - .push(key_id.into()); - } - } - } - - missing - } - - /// Fetches verification keys from the origin servers directly, returning - /// any keys that could not be fetched. - async fn acquire_origins(&self, batch: I) -> Batch - where - I: Iterator)> + Send, - { - let timeout = Instant::now() - .checked_add(Duration::from_secs(45)) - .expect("timeout overflows"); - - let mut requests: FuturesUnordered<_> = batch - .map(|(origin, key_ids)| self.acquire_origin(origin, key_ids, timeout)) - .collect(); - - let mut missing = Batch::new(); - while let Some((origin, key_ids)) = requests.next().await { - if !key_ids.is_empty() { - missing.insert(origin, key_ids); - } - } - - missing - } - - /// Fetches verification keys from an origin directly, returning any keys - /// that were fetched. - async fn acquire_origin( - &self, - origin: OwnedServerName, - mut key_ids: Vec, - timeout: Instant, - ) -> (OwnedServerName, Vec) { - match timeout_at(timeout, self.server_request(&origin)).await { - | Err(e) => debug_warn!(%origin, "timed out: {e}"), - | Ok(Err(e)) => debug_error!(%origin, "{e}"), - | Ok(Ok(server_keys)) => { - trace!( - %origin, - ?key_ids, - ?server_keys, - "received server_keys" - ); - - self.add_signing_keys(server_keys.clone()).await; - key_ids.retain(|key_id| !key_exists(&server_keys, key_id)); - }, - } - - (origin, key_ids) - } - - /// Fetches signing keys for the given batch from the configured notary - /// servers, returning any keys that could not be fetched. - async fn acquire_notary(&self, batch: I) -> Batch - where - I: Iterator)> + Send, - { - let mut missing: Batch = batch.collect(); - for notary in self.services.globals.trusted_servers() { - let missing_keys = keys_count(&missing); - let missing_servers = missing.len(); - debug!( - "Asking notary {notary} for {missing_keys} missing keys from {missing_servers} \ - servers" - ); - - let batch = missing - .iter() - .map(|(server, keys)| (server.borrow(), keys.iter().map(Borrow::borrow))); - - match self.batch_notary_request(notary, batch).await { - | Err(e) => error!("Failed to contact notary {notary:?}: {e}"), - | Ok(results) => - for server_keys in results { - self.acquire_notary_result(&mut missing, server_keys).await; - }, - } - } - - missing - } - - /// Contacts a notary and fetches signing keys for the given batch, removing - /// keys from the missing batch when they are found. - async fn acquire_notary_result(&self, missing: &mut Batch, server_keys: ServerSigningKeys) { - let server = &server_keys.server_name; - self.add_signing_keys(server_keys.clone()).await; - - if let Some(key_ids) = missing.get_mut(server) { - key_ids.retain(|key_id| !key_exists(&server_keys, key_id)); - if key_ids.is_empty() { - missing.remove(server); - } - } - } -} - -fn keys_count(batch: &Batch) -> usize { - batch.values().flat_map(|key_ids| key_ids.iter()).count() -} diff --git a/src/service/server_keys/get.rs b/src/service/server_keys/get.rs index a05340704..5011171c4 100644 --- a/src/service/server_keys/get.rs +++ b/src/service/server_keys/get.rs @@ -1,146 +1,526 @@ -use std::borrow::Borrow; +/// get.rs handles actually acquiring keys from remote servers. +use std::collections::BTreeMap; -use conduwuit::{Err, Result, trace}; +use assign::assign; +use conduwuit::{Result, debug, debug_error, debug_info, debug_warn, err, info, trace}; +use futures::{StreamExt, stream::FuturesUnordered}; use ruma::{ - CanonicalJsonObject, ServerName, ServerSigningKeyId, api::federation::discovery::VerifyKey, + CanonicalJsonObject, CanonicalJsonValue, MilliSecondsSinceUnixEpoch, ServerName, + ServerSigningKeyId, UInt, + api::federation::discovery::{ + ServerSigningKeys, VerifyKey, get_remote_server_keys_batch::v2::QueryCriteria, + }, room_version_rules::RoomVersionRules, + serde::Base64, + uint, }; +use serde_json::value::RawValue; +use tokio::select; -use super::{PubKeyMap, PubKeys, extract_key}; -use crate::server_keys::util::required_keys; +use super::{PubKeyMap, in_one_week}; +use crate::server_keys::{request::ServerKeysQuery, util::required_keys}; impl super::Service { + /// Extracts the minimum timestamp within which a signing key must be valid + /// for it to be able to verify this event. + pub(super) fn min_valid_ts_from_event( + object: &CanonicalJsonObject, + ) -> Result { + object + .get("origin_server_ts") + .and_then(CanonicalJsonValue::as_integer) + .map(i64::from) + .map(|ts| { + MilliSecondsSinceUnixEpoch(UInt::new_saturating( + u64::try_from(ts).unwrap_or_default(), + )) + }) + .ok_or_else(|| err!(Request(BadJson("Missing origin_server_ts in event")))) + } + /// Fetches the verify keys required to verify the incoming event. + /// + /// Only returns an error if the `min_valid_ts` cannot be inferred from the + /// object, or if calculating which keys are required fails (malformed + /// event). + /// + /// Keys which cannot be fetched are not included in the resulting map, but + /// will not cause an error. pub async fn get_event_keys( &self, object: &CanonicalJsonObject, room_version_rules: &RoomVersionRules, ) -> Result { - let required = match required_keys(object, &room_version_rules.signatures) { - | Ok(required) => required, - | Err(e) => { - return Err!(BadServerResponse( - "Failed to determine keys required to verify event: {e}" - )); - }, - }; + let min_valid_ts = Self::min_valid_ts_from_event(object)?; + + let required = required_keys(object, &room_version_rules.signatures).map_err(|e| { + err!(BadServerResponse("Failed to determine keys required to verify event: {e}")) + })?; let batch = required - .iter() - .map(|(s, ids)| (s.borrow(), ids.iter().map(Borrow::borrow))); + .into_iter() + .map(|(s, ids)| { + ( + s, + ids.into_iter() + .map(|key_id| (key_id, new_query_criteria(min_valid_ts))) + .collect::>(), + ) + }) + .collect::(); - Ok(self.get_pubkeys(batch).await) + Ok(self.fetch_verify_keys(batch).await) } - /// Fetches all verify keys for the given batch. Any keys which could not be - /// fetched are elided from the resulting map. - pub async fn get_pubkeys<'a, S, K>(&self, batch: S) -> PubKeyMap - where - S: Iterator + Send, - K: Iterator + Send, - { - let mut keys = PubKeyMap::new(); - for (server, key_ids) in batch { - let pubkeys = self.get_pubkeys_for(server, key_ids).await; - keys.insert(server.into(), pubkeys); - } - - keys - } - - /// Gets verify keys for the given server name and a set of key IDs. Any - /// keys that cannot be found are elided from the resulting set. - pub async fn get_pubkeys_for<'a, I>(&self, origin: &ServerName, key_ids: I) -> PubKeys - where - I: Iterator + Send, - { - let mut keys = PubKeys::new(); - for key_id in key_ids { - if let Ok(verify_key) = self.get_verify_key(origin, key_id).await { - keys.insert(key_id.into(), verify_key.key); - } - } - - keys - } - - /// Fetches a single verify key via origin and notaries. - pub async fn get_verify_key( + /// Fetches the verify keys required to verify all the incoming events. + pub async fn get_events_keys( &self, - origin: &ServerName, - key_id: &ServerSigningKeyId, - ) -> Result { - let notary_first = self.services.server.config.query_trusted_key_servers_first; - let notary_only = self.services.server.config.only_query_trusted_key_servers; - - if let Some(result) = self.verify_keys_for(origin).await.remove(key_id) { - trace!("Found key in cache"); - return Ok(result); - } - - if notary_first { - if let Ok(result) = self.get_verify_key_from_notaries(origin, key_id).await { - return Ok(result); - } - } - - if !notary_only { - if let Ok(result) = self.get_verify_key_from_origin(origin, key_id).await { - return Ok(result); - } - } - - if !notary_first { - if let Ok(result) = self.get_verify_key_from_notaries(origin, key_id).await { - return Ok(result); - } - } - - Err!(BadServerResponse(debug_error!( - %key_id, - %origin, - "Failed to fetch federation signing-key" - ))) - } - - /// Fetches verify keys from notary servers, adding them to the cache, and - /// returning the requested key if found. - async fn get_verify_key_from_notaries( - &self, - origin: &ServerName, - key_id: &ServerSigningKeyId, - ) -> Result { - for notary in self.services.globals.trusted_servers() { - if let Ok(server_keys) = self.notary_request(notary, origin).await { - for server_key in server_keys.clone() { - self.add_signing_keys(server_key).await; + raw_objects: impl Iterator>, + room_version_rules: &RoomVersionRules, + ) -> PubKeyMap { + let mut batch = ServerKeysQuery::new(); + for raw_object in raw_objects { + let object: CanonicalJsonObject = match serde_json::from_str(raw_object.get()) { + | Ok(object) => object, + | Err(e) => { + debug_error!(?raw_object, error=?e, "failed to parse incoming event"); + continue; + }, + }; + let min_valid_ts = match Self::min_valid_ts_from_event(&object) { + | Ok(ts) => ts, + | Err(e) => { + debug!(?object, error=?e, "incoming event has no `origin_server_ts` field, skipping"); + continue; + }, + }; + let required = match required_keys(&object, &room_version_rules.signatures) { + | Ok(required) => required, + | Err(e) => { + debug_warn!( + ?object, + error=?e, + "failed to determine required verify keys for incoming event, skipping" + ); + continue; + }, + }; + for (server_name, signing_key_ids) in required { + let batch_entry = batch.entry(server_name).or_default(); + for key_id in signing_key_ids { + batch_entry + .entry(key_id) + .and_modify(|criteria| { + // We want to use the lower of the two minimum valid + // timestamps if the key is already existing; + // otherwise, just insert this one. + criteria.minimum_valid_until_ts = Some( + criteria + .minimum_valid_until_ts + .expect("We never ask for no min valid ts") + .min(min_valid_ts), + ); + }) + .or_insert_with(|| new_query_criteria(min_valid_ts)); } + } + } - for server_key in server_keys { - if let Some(result) = extract_key(server_key, key_id) { - return Ok(result); + self.fetch_verify_keys(batch).await + } + + /// Fetches verify keys matching the required batch parameters. + /// + /// First attempts to fulfil any requirements from the local server cache, + /// falling back to asking origins and notaries. + /// + /// Origins and notaries are always asked in parallel, and will complement + /// each other if notaries are not prioritised, using a + /// first-come-first-serve stream consumption approach. + /// + /// If notaries are prioritised, this function will instead drain all notary + /// futures until all keys are found, or there are no more notary responses. + /// In that case, the origin future(s) will be consumed. + /// + /// If there are still unacquired keys, a debug info log will be emitted + /// containing further details, but an error will not be returned. + async fn fetch_verify_keys(&self, batch: ServerKeysQuery) -> PubKeyMap { + let mut still_missing = batch.clone(); + let mut result = PubKeyMap::new(); + trace!(?batch, "Preparing to fetch verify keys"); + + // First, filter out any keys we already have locally. + for (origin, keys) in &batch { + for (key_id, criteria) in keys { + let min_valid_ts = criteria + .minimum_valid_until_ts + .unwrap_or_else(|| MilliSecondsSinceUnixEpoch(uint!(0))); + + if let Some(server_keys) = self.get_signing_key(origin, key_id).await + && let Some(key) = + verify_key_from_response(key_id, &server_keys, min_valid_ts) + { + trace!(%origin, %key_id, ?min_valid_ts, "Verify key exists locally"); + Self::track_one_result( + &mut result, + &mut still_missing, + origin, + key_id, + key.key, + ); + continue; + } + trace!(%origin, %key_id, ?min_valid_ts, "Did not find verify key locally"); + } + } + if still_missing.is_empty() { + trace!("Not missing any more keys!"); + return result; + } + trace!(?still_missing, "Still missing some keys after local population"); + + // Now we need to fetch keys en masse. + // For each server in the set, we'll ping off a request to the origin to + // fetch their signing keys. Then, in parallel, we'll ask every notary + // we know for the full batch. If the server is configured to ask the + // origin first, we'll simply await the origin request first, failing + // back to the notary futures. Otherwise, we can work through each + // response as they come back until we either get all the keys we + // need, or we run out of servers to ask. + let mut origin_futures: FuturesUnordered<_> = FuturesUnordered::new(); + let mut notary_futures: FuturesUnordered<_> = FuturesUnordered::new(); + + for origin in still_missing.keys() { + let min_valid_ts_for_this_remote = batch + .get(origin) + .and_then(|keys| { + keys.values() + .filter_map(|criteria| criteria.minimum_valid_until_ts) + .max() + }) + .unwrap_or_else(|| MilliSecondsSinceUnixEpoch(uint!(0))); + trace!( + %origin, + ?min_valid_ts_for_this_remote, + "Spawning future to ask origin for their verify keys" + ); + origin_futures + .push(self.origin_request(origin.to_owned(), min_valid_ts_for_this_remote)); + } + + self.services + .server + .config + .trusted_servers + .iter() + .for_each(|notary| { + trace!(notary=%notary.server_name(), "Asking notary for still missing keys"); + notary_futures + .push(self.notary_request(notary.server_name(), still_missing.clone())); + }); + + let prioritise_notaries = !self.services.server.config.trusted_servers.is_empty() + && self.services.server.config.query_trusted_key_servers_first; + + // TODO: In the future, an option to establish a quorum between notaries + // may be desired. In such a case, the `still_missing { break }` + // blocks should be skipped in order to acquire all notary responses for + // later comparison. + + if !prioritise_notaries { + // If we aren't prioritising notaries, we can just select futures on + // a first-come-first-serve basis. + loop { + select! { + origin_response = origin_futures.next(), if !origin_futures.is_empty() => { + let origin_response = match origin_response { + Some(Ok(v)) => v, + None => continue, + Some(Err(e)) => { + debug_warn!(err=?e, "Got error response from origin"); + continue; + } + }; + trace!(?origin_response, "Got signing key response from origin"); + self.add_signing_keys(&origin_response); + Self::track_result(&mut result, &mut still_missing, &origin_response); + if still_missing.is_empty() { + break; + } + }, + notary_response = notary_futures.next(), if !notary_futures.is_empty() => { + let notary_response = match notary_response { + Some(Ok(v)) => v, + None => continue, + Some(Err(e)) => { + debug_warn!(err=?e, "Got error response from notary"); + continue; + } + }; + trace!( + chunks=%notary_response.len(), + "Got signing keys response from a notary" + ); + for server_keys in notary_response { + trace!(?server_keys, "Adding response from notary"); + self.add_signing_keys(&server_keys); + Self::track_result(&mut result, &mut still_missing, &server_keys); + if still_missing.is_empty() { + break; + } + } + } + else => { + trace!("Origin or notary future stream(s) finished."); + break + } + } + } + } else { + // Drain notary responses first, and then sift through origins. + trace!(count=%notary_futures.len(), "Waiting for notary responses..."); + while let Some(res) = notary_futures.next().await { + let Ok(res) = res else { + debug_warn!(err=?res.err(), "Got error response from notary"); + continue; + }; + for server_keys in res { + trace!(?server_keys, "Adding response from notary"); + self.add_signing_keys(&server_keys); + Self::track_result(&mut result, &mut still_missing, &server_keys); + } + if still_missing.is_empty() { + break; + } + } + if !still_missing.is_empty() { + trace!("Still missing some keys, waiting for origin responses..."); + while let Some(res) = origin_futures.next().await { + let Ok(res) = res else { + debug_warn!(err=?res.err(), "Got error response from origin"); + continue; + }; + trace!(response=?res, "Got signing key response from origin"); + self.add_signing_keys(&res); + Self::track_result(&mut result, &mut still_missing, &res); + if still_missing.is_empty() { + break; } } } } - Err!(Request(NotFound("Failed to fetch signing-key from notaries"))) + if !still_missing.is_empty() { + let missing_servers = still_missing.len(); + let missing_keys = still_missing + .values() + .map(BTreeMap::len) + .fold(0_usize, usize::saturating_add); + let fetched_servers = result.len(); + let fetched_keys = result + .values() + .map(BTreeMap::len) + .fold(0_usize, usize::saturating_add); + debug_info!( + %fetched_servers, + %fetched_keys, + %missing_servers, + %missing_keys, + "Could not acquire all requested signing keys" + ); + trace!(?still_missing, "Could not acquire all requested signing keys"); + } + result } - /// Fetches verify keys from the origin server, adding them to the cache, - /// and returning the requested key if found. - async fn get_verify_key_from_origin( + /// Tracks the result of a signing keys response by removing each of the + /// returned verify keys and old verify keys from the missing map, and + /// adding them to the results map. + fn track_result( + results: &mut PubKeyMap, + missing: &mut ServerKeysQuery, + batch: &ServerSigningKeys, + ) { + batch.verify_keys.iter().for_each(|(key_id, key)| { + Self::track_one_result(results, missing, &batch.server_name, key_id, key.key.clone()); + }); + batch.old_verify_keys.iter().for_each(|(key_id, key)| { + Self::track_one_result(results, missing, &batch.server_name, key_id, key.key.clone()); + }); + } + + /// Tracks a single signing key result by adding it to the results map and + /// removing it from the missing map. If all missing keys for an origin are + /// found (the missing list is empty), the origin is removed from the + /// missing map entirely. + fn track_one_result( + results: &mut PubKeyMap, + missing: &mut ServerKeysQuery, + origin: &ServerName, + key_id: &ServerSigningKeyId, + key: Base64, + ) { + results + .entry(origin.to_string()) + .or_default() + .insert(key_id.to_string(), key); + missing.entry(origin.to_owned()).and_modify(|keys| { + keys.remove(key_id); + }); + if missing.get(origin).is_some_and(BTreeMap::is_empty) { + missing.remove(origin); + } + } + + /// Fetches a single verify key. + /// + /// If a valid key cannot be found locally, the origin is queried. If it + /// fails to respond, or the desired verify key is not contained in the + /// response/expired before `min_valid_ts`, `None` is returned. + #[tracing::instrument(skip(self))] + pub async fn get_single_verify_key( &self, origin: &ServerName, key_id: &ServerSigningKeyId, - ) -> Result { - if let Ok(server_key) = self.server_request(origin).await { - self.add_signing_keys(server_key.clone()).await; - if let Some(result) = extract_key(server_key, key_id) { - return Ok(result); - } + min_valid_ts: MilliSecondsSinceUnixEpoch, + ) -> Option { + if let Some(vk) = self + .get_signing_key(origin, key_id) + .await + .and_then(|ssk| verify_key_from_response(key_id, &ssk, min_valid_ts)) + { + trace!("Found verify key locally"); + return Some(vk); } - Err!(Request(NotFound("Failed to fetch signing-key from origin"))) + trace!("Local miss for single key, asking origin"); + let response = self + .origin_request(origin.to_owned(), min_valid_ts) + .await + .inspect_err(|e| info!("Could not acquire signing key from origin server: {e:?}")) + .ok()?; + + verify_key_from_response(key_id, &response, min_valid_ts) + .inspect(|_| self.add_signing_keys(&response)) + } +} + +/// Generates a QueryCriteria with the minimum_valid_until_ts set to the target +/// epoch. +fn new_query_criteria(min_valid_ts: MilliSecondsSinceUnixEpoch) -> QueryCriteria { + assign!(QueryCriteria::new(), { minimum_valid_until_ts: Some(min_valid_ts) }) +} + +/// Extracts a verify key with the given ID from the response. +/// +/// If the provided `ServerSigningKeys` is dated to before the min valid TS, +/// this will throw an assertion panic in debug builds, or always return `None` +/// in release. If the key is found in `verify_keys`, it is returned as-is. If +/// the key is found in `old_verify_keys`, it is only returned if the expiry +/// timestamp is greater than or equal to the minimum valid timestamp. +fn verify_key_from_response( + key_id: &ServerSigningKeyId, + server_keys: &ServerSigningKeys, + min_valid_ts: MilliSecondsSinceUnixEpoch, +) -> Option { + if server_keys.valid_until_ts < min_valid_ts || server_keys.valid_until_ts > in_one_week() { + return None; + } + if let Some(key) = server_keys + .verify_keys + .iter() + .filter(|(k, _)| *k == key_id) + .map(|(_, v)| v) + .next() + { + return Some(key.to_owned()); + } + server_keys + .old_verify_keys + .iter() + .filter(|(k, v)| { + *k == key_id && v.expired_ts.min(server_keys.valid_until_ts) >= min_valid_ts + }) + .map(|(_, v)| v) + .next() + .map(|old_key| VerifyKey::new(old_key.key.clone())) +} + +#[cfg(test)] +mod test { + use ruma::Int; + use serde_json::json; + + use super::*; + use crate::server_keys::{Service, request::ServerKeysQuerySet}; + + #[test] + fn min_valid_ts_refuses_garbage() { + let garbage = vec![ + json!("string"), + json!("0"), // string ints aren't ints + CanonicalJsonValue::Null.into(), + json!([]), + json!({}), + json!(" "), + ]; + + for fixture in garbage { + let obj: CanonicalJsonObject = + serde_json::from_value(json!({"origin_server_ts": fixture})).unwrap(); + assert!( + Service::min_valid_ts_from_event(&obj).is_err(), + "garbage should always error instead of trying to guess" + ); + } + } + + #[test] + fn min_valid_ts_requires_field() { + // This just ensures we don't accidentally default to 0 or something + // dumb like that + assert!( + Service::min_valid_ts_from_event(&CanonicalJsonObject::new()).is_err(), + "origin_server_ts should be a required field and thus should error if not provided" + ); + } + + #[test] + fn min_valid_ts_accepts_negative_values() { + // Since we convert to u64 with saturating for MSSinceEpoch, this should + // give us a final timestamp of zero (since unsigned ints cant be + // negative). + let obj: CanonicalJsonObject = + serde_json::from_value(json!({"origin_server_ts": Int::MIN})).unwrap(); + assert_eq!( + Service::min_valid_ts_from_event(&obj) + .expect("Int::MIN should be a valid min timestamp") + .0, + uint!(0), + "Saturating Int::MIN to u64 should result in zero" + ); + } + + #[test] + fn tracking_server_result_removes_empty_server_from_missing_map() { + let server_name = ruma::owned_server_name!("example.com"); + let key_id = ServerSigningKeyId::parse("ed25519:1").unwrap(); + let mut missing = ServerKeysQuery::new(); + missing.insert(server_name.clone(), ServerKeysQuerySet::new()); + missing.entry(server_name.clone()).and_modify(|set| { + set.insert(key_id.clone(), QueryCriteria::new()); + }); + let mut results = PubKeyMap::new(); + + Service::track_one_result( + &mut results, + &mut missing, + &server_name, + &key_id, + Base64::new(Vec::new()), + ); + + assert!(missing.is_empty(), "missing should be empty after tracking the result"); + assert!(!results.is_empty(), "results should not be empty after tracking the result"); } } diff --git a/src/service/server_keys/keypair.rs b/src/service/server_keys/keypair.rs index b467da8ff..817d335cc 100644 --- a/src/service/server_keys/keypair.rs +++ b/src/service/server_keys/keypair.rs @@ -9,7 +9,8 @@ use super::VerifyKeys; pub(super) fn init(db: &Arc) -> Result<(Box, VerifyKeys)> { let keypair = load(db).inspect_err(|_e| { error!("Keypair invalid. Deleting..."); - remove(db); + let global = &db["global"]; + global.remove(b"keypair"); })?; let verify_key = VerifyKey::new(Base64::new(keypair.public_key().to_vec())); @@ -20,6 +21,7 @@ pub(super) fn init(db: &Arc) -> Result<(Box, VerifyKey Ok((keypair, verify_keys)) } +/// Loads the existing keypair from the database, otherwise creates a new one. fn load(db: &Arc) -> Result> { let (version, key) = db["global"] .get_blocking(b"keypair") @@ -44,6 +46,7 @@ fn load(db: &Arc) -> Result> { Ok(Box::new(key)) } +/// Creates a new keypair and writes it directly to the db. fn create(db: &Arc) -> Result<(String, Vec)> { let keypair = Ed25519KeyPair::generate(); @@ -55,9 +58,3 @@ fn create(db: &Arc) -> Result<(String, Vec)> { Ok(value) } - -#[inline] -fn remove(db: &Arc) { - let global = &db["global"]; - global.remove(b"keypair"); -} diff --git a/src/service/server_keys/mod.rs b/src/service/server_keys/mod.rs index b79b2b10d..ae1352d2b 100644 --- a/src/service/server_keys/mod.rs +++ b/src/service/server_keys/mod.rs @@ -1,4 +1,3 @@ -mod acquire; mod get; mod keypair; mod request; @@ -6,30 +5,28 @@ mod sign; mod util; mod verify; -use std::{collections::BTreeMap, sync::Arc, time::Duration}; +use std::{collections::BTreeMap, sync::Arc}; use conduwuit::{ Result, Server, - utils::{IterStream, timepoint_from_now}, + utils::{IterStream, ReadyExt, stream::TryIgnore}, }; -use database::{Deserialized, Json, Map}; -use futures::StreamExt; +use database::{Deserialized, Ignore, Interfix, Json, Map}; +use futures::{Stream, StreamExt}; +pub use request::in_one_week; use ruma::{ CanonicalJsonObject, MilliSecondsSinceUnixEpoch, OwnedServerSigningKeyId, ServerName, ServerSigningKeyId, api::federation::discovery::{ServerSigningKeys, VerifyKey}, room_version_rules::RoomVersionRules, - serde::Raw, signatures::{Ed25519KeyPair, PublicKeyMap, PublicKeySet}, }; -use serde_json::value::RawValue as RawJsonValue; use crate::{Dep, globals, sending, server_keys::util::required_keys}; pub struct Service { keypair: Box, verify_keys: VerifyKeys, - minimum_valid: Duration, services: Services, db: Data, } @@ -41,7 +38,7 @@ struct Services { } struct Data { - server_signingkeys: Arc, + servernamekeyid_response: Arc, } pub type VerifyKeys = BTreeMap; @@ -50,22 +47,19 @@ pub type PubKeys = PublicKeySet; impl crate::Service for Service { fn build(args: crate::Args<'_>) -> Result> { - let minimum_valid = Duration::from_hours(1); - let (keypair, verify_keys) = keypair::init(args.db)?; debug_assert!(verify_keys.len() == 1, "only one active verify_key supported"); Ok(Arc::new(Self { keypair, verify_keys, - minimum_valid, services: Services { globals: args.depend::("globals"), sending: args.depend::("sending"), server: args.server.clone(), }, db: Data { - server_signingkeys: args.db["server_signingkeys"].clone(), + servernamekeyid_response: args.db["servernamekeyid_response"].clone(), }, })) } @@ -77,40 +71,32 @@ impl Service { /// Fetches the server's active keypair. pub fn keypair(&self) -> &Ed25519KeyPair { &self.keypair } - /// Fetches the server's active signing key ID. - pub fn active_key_id(&self) -> &ServerSigningKeyId { self.active_verify_key().0 } - /// Fetches the server's active signing key. Panics if there's more than 1 /// active key. - pub fn active_verify_key(&self) -> (&ServerSigningKeyId, &VerifyKey) { + pub fn active_verify_key(&self) -> (OwnedServerSigningKeyId, VerifyKey) { debug_assert!(self.verify_keys.len() <= 1, "more than one active verify_key"); self.verify_keys .iter() .next() - .map(|(id, key)| (id.as_ref(), key)) + .map(|(id, key)| (id.to_owned(), key.to_owned())) .expect("missing active verify_key") } - /// Combines signing keys from the incoming response to the existing - /// database set. - async fn add_signing_keys(&self, new_keys: ServerSigningKeys) { - let origin = &new_keys.server_name; - - // (timo) Not atomic, but this is not critical - let mut keys: ServerSigningKeys = self - .db - .server_signingkeys - .get(origin) - .await - .deserialized() - .unwrap_or_else(|_| { - // Just insert "now", it doesn't matter - ServerSigningKeys::new(origin.to_owned(), MilliSecondsSinceUnixEpoch::now()) - }); - - keys.verify_keys.extend(new_keys.verify_keys); - keys.old_verify_keys.extend(new_keys.old_verify_keys); - self.db.server_signingkeys.raw_put(origin, Json(&keys)); + /// Adds a signing key response to the database. + fn add_signing_keys(&self, new_keys: &ServerSigningKeys) { + // TODO(nex): this results in massive amounts of data duplication. + // Perhaps a sliding approach would be better? where we just update the + // last known response if the only thing that changed was the + // valid_until_ts. This would require a more complex data structure + // though. For now, just storing the duplicated data is fine. + for key in new_keys + .verify_keys + .keys() + .chain(new_keys.old_verify_keys.keys()) + { + let db_key = (new_keys.server_name.as_str(), key.as_str()); + self.db.servernamekeyid_response.put(db_key, Json(new_keys)); + } } /// Checks if the keys required to verify an incoming PDU are already known @@ -123,11 +109,14 @@ impl Service { let Ok(required_keys) = required_keys(object, &room_version_rules.signatures) else { return false; }; + let Ok(min_valid_ts) = Self::min_valid_ts_from_event(object) else { + return false; + }; required_keys .iter() .flat_map(|(server, key_ids)| key_ids.iter().map(move |key_id| (server, key_id))) .stream() - .all(|(server, key_id)| self.verify_key_exists(server, key_id)) + .all(|(server, key_id)| self.verify_key_exists(server, key_id, min_valid_ts)) .await } @@ -137,40 +126,54 @@ impl Service { &self, origin: &ServerName, key_id: &ServerSigningKeyId, + minimum_valid_ts: MilliSecondsSinceUnixEpoch, ) -> bool { - type KeysMap = BTreeMap>; - - let Ok(keys) = self - .db - .server_signingkeys - .get(origin) - .await - .deserialized::>() - else { + let Some(keys) = self.get_signing_key(origin, key_id).await else { + // We have no keys for this server return false; }; - if let Ok(Some(verify_keys)) = keys.get_field::("verify_keys") { - if verify_keys.contains_key(key_id) { - return true; - } + if keys.valid_until_ts < minimum_valid_ts || keys.valid_until_ts > in_one_week() { + // This key response is stale and cannot be trusted. + return false; } - if let Ok(Some(old_verify_keys)) = keys.get_field::("old_verify_keys") { - if old_verify_keys.contains_key(key_id) { - return true; - } + if keys.verify_keys.contains_key(key_id) { + // An in-date, active key exists for this server. + return true; + } + + if let Some(expired_key) = keys.old_verify_keys.get(key_id) { + // This key can still be used to verify signatures provided it + // expired within the minimum valid time window. + return expired_key.expired_ts.min(keys.valid_until_ts) >= minimum_valid_ts; } false } - /// Returns all verify keys for the origin. + pub async fn get_signing_key( + &self, + origin: &ServerName, + key_id: &ServerSigningKeyId, + ) -> Option { + self.db + .servernamekeyid_response + .qry(&(origin.as_str(), key_id.as_str())) + .await + .deserialized::() + .ok() + } + + /// Returns all (active) verify keys for the origin. pub async fn verify_keys_for(&self, origin: &ServerName) -> VerifyKeys { let mut keys = self .signing_keys_for(origin) - .await - .map_or(BTreeMap::new(), |keys| merge_old_keys(keys).verify_keys); + .ready_fold(BTreeMap::new(), |mut base, keys| { + base.extend(keys.verify_keys); + base + }) + .await; if self.services.globals.server_is_ours(origin) { keys.extend(self.verify_keys.clone()); @@ -179,39 +182,15 @@ impl Service { keys } - /// Returns the stored server signing keys response for the origin. - pub async fn signing_keys_for(&self, origin: &ServerName) -> Result { - self.db.server_signingkeys.get(origin).await.deserialized() - } - - /// Returns the minimum_valid_ts for a key response from the current point - /// in time, plus the minimum valid time. - fn minimum_valid_ts(&self) -> MilliSecondsSinceUnixEpoch { - let timepoint = - timepoint_from_now(self.minimum_valid).expect("SystemTime should not overflow"); - MilliSecondsSinceUnixEpoch::from_system_time(timepoint).expect("UInt should not overflow") + /// Returns the stored server signing keys responses for the origin. + pub fn signing_keys_for<'a>( + &'a self, + origin: &'a ServerName, + ) -> impl Stream + Send + 'a { + self.db + .servernamekeyid_response + .stream_prefix(&(origin, Interfix)) + .ignore_err() + .map(|(_, v): (Ignore, ServerSigningKeys)| v) } } - -fn merge_old_keys(mut keys: ServerSigningKeys) -> ServerSigningKeys { - keys.verify_keys.extend( - keys.old_verify_keys - .clone() - .into_iter() - .map(|(key_id, old)| (key_id, VerifyKey::new(old.key))), - ); - - keys -} - -fn extract_key(mut keys: ServerSigningKeys, key_id: &ServerSigningKeyId) -> Option { - keys.verify_keys.remove(key_id).or_else(|| { - keys.old_verify_keys - .remove(key_id) - .map(|old| VerifyKey::new(old.key)) - }) -} - -fn key_exists(keys: &ServerSigningKeys, key_id: &ServerSigningKeyId) -> bool { - keys.verify_keys.contains_key(key_id) || keys.old_verify_keys.contains_key(key_id) -} diff --git a/src/service/server_keys/request.rs b/src/service/server_keys/request.rs index 043e894a9..5a4ce6a10 100644 --- a/src/service/server_keys/request.rs +++ b/src/service/server_keys/request.rs @@ -1,110 +1,39 @@ -use std::{collections::BTreeMap, fmt::Debug}; +use std::collections::BTreeMap; -use conduwuit::{Err, Result, debug}; -use get_remote_server_keys_batch::v2::Request; +use conduwuit::{Err, Result, trace, utils::millis_since_unix_epoch, warn}; use ruma::{ - OwnedServerName, OwnedServerSigningKeyId, ServerName, ServerSigningKeyId, + MilliSecondsSinceUnixEpoch, OwnedServerName, OwnedServerSigningKeyId, ServerName, UInt, api::federation::discovery::{ - ServerSigningKeys, get_remote_server_keys, - get_remote_server_keys_batch::{self, v2::QueryCriteria}, + ServerSigningKeys, + get_remote_server_keys_batch::{ + self, + v2::{QueryCriteria, Response as NotaryResponse}, + }, get_server_keys, }, + serde::Base64, + uint, }; -type RumaBatch = BTreeMap>; + +pub(crate) type ServerKeysQuery = BTreeMap; +pub(crate) type ServerKeysQuerySet = BTreeMap; impl super::Service { - /// Handles a batched notary request. - /// If the batch is empty, an empty vec is returned. - pub(super) async fn batch_notary_request<'a, S, K>( + /// Asks the origin server directly for its signing keys. + /// + /// An error is returned if the server responds with invalid keys; this is + /// in contrast with the notary request, which simply drops such responses. + pub async fn origin_request( &self, - notary: &ServerName, - batch: S, - ) -> Result> - where - S: Iterator + Send, - K: Iterator + Send, - { - let mut criteria = QueryCriteria::new(); - criteria.minimum_valid_until_ts = Some(self.minimum_valid_ts()); - - let mut server_keys = batch.fold(RumaBatch::new(), |mut batch, (server, key_ids)| { - batch - .entry(server.into()) - .or_default() - .extend(key_ids.map(|key_id| (key_id.into(), criteria.clone()))); - - batch - }); - - if server_keys.is_empty() { - return Ok(vec![]); - } - - let mut results = Vec::new(); - while let Some(batch) = server_keys - .keys() - .rev() - .take(self.services.server.config.trusted_server_batch_size) - .next_back() - .cloned() - { - let request = Request::new(server_keys.split_off(&batch)); - - debug!( - ?notary, - ?batch, - remaining = %server_keys.len(), - requesting = ?request.server_keys.keys(), - "notary request" - ); - - let response = self - .services - .sending - .send_unauthenticated_request(notary, request) - .await? - .server_keys - .into_iter() - .map(|key| key.deserialize()) - .filter_map(Result::ok); - - results.extend(response); - } - - Ok(results) - } - - /// Makes a request to a notary server to fetch signing keys. - pub async fn notary_request( - &self, - notary: &ServerName, - target: &ServerName, - ) -> Result + Clone + Debug + Send + use<>> { - use get_remote_server_keys::v2::Request; - - let request = Request::new(target.into(), self.minimum_valid_ts()); - - let response = self - .services - .sending - .send_unauthenticated_request(notary, request) - .await? - .server_keys - .into_iter() - .map(|key| key.deserialize()) - .filter_map(Result::ok); - - Ok(response) - } - - /// Makes a request to an origin server to fetch signing keys. - pub async fn server_request(&self, target: &ServerName) -> Result { + target: OwnedServerName, + min_valid_ts: MilliSecondsSinceUnixEpoch, + ) -> Result { use get_server_keys::v2::Request; let server_signing_key = self .services .sending - .send_unauthenticated_request(target, Request::new()) + .send_unauthenticated_request(&target, Request::new()) .await .map(|response| response.server_key) .and_then(|key| key.deserialize().map_err(Into::into))?; @@ -113,10 +42,172 @@ impl super::Service { return Err!(BadServerResponse(debug_warn!( requested = ?target, response = ?server_signing_key.server_name, - "Server responded with bogus server_name" + "Origin server responded with bogus server_name" + ))); + } + + if server_signing_key.valid_until_ts < min_valid_ts + || server_signing_key.valid_until_ts > in_one_week() + { + return Err!(BadServerResponse(debug_warn!( + ?target, + ?min_valid_ts, + ?server_signing_key.valid_until_ts, + "Origin server responded with server keys that are not in-date" + ))); + } + + if let Err(e) = Self::verify_server_keys_response(&server_signing_key, None) { + return Err!(BadServerResponse(debug_warn!( + ?target, + "Origin server failed to self-sign key response: {e}" ))); } Ok(server_signing_key) } + + /// Sends a request to a notary server, asking for the specified keys for + /// the given servers. + /// + /// The response is filtered before it is returned, dropping any chunks that + /// are invalid or do not match the requested keys. As a result, the request + /// may be successful, but the returned vec may be empty. + /// + /// + /// An error is returned if the notary server itself returns an error, + /// because of invalid data. + /// + /// Does not make a request if there are no keys specified in the batch. + pub async fn notary_request( + &self, + notary: &ServerName, + batch: ServerKeysQuery, + ) -> Result> { + use get_remote_server_keys_batch::v2::Request; + + let is_empty = batch.is_empty() || batch.values().all(BTreeMap::is_empty); + if is_empty { + return Ok(Vec::new()); + } + + let request = Request::new(batch.clone()); + let notary_keys = self + .services + .server + .config + .trusted_servers + .iter() + .find(|n| n.server_name() == notary) + .map(|nk| nk.verify_keys().to_owned()) + .unwrap_or_default(); + let keys = self + .services + .sending + .send_unauthenticated_request(notary, request) + .await + .map(|response| Self::parse_notary_response(&response))? + .into_iter() + .filter(|keys| { + Self::validate_notary_chunk(&batch, keys, notary, notary_keys.as_slice()) + }) + .collect(); + Ok(keys) + } + + /// Parses a notary response, returning a vec of server signing key + /// responses. Drops any chunks that cannot be deserialized. + fn parse_notary_response(raw: &NotaryResponse) -> Vec { + raw.server_keys + .iter() + .map(ruma::serde::Raw::deserialize) + .filter_map(Result::ok) + .collect() + } + + /// Validates a signing keys response received from a notary. Returns false + /// if the response should be dropped, true if it should be kept. + fn validate_notary_chunk( + batch: &ServerKeysQuery, + keys: &ServerSigningKeys, + notary_name: &ServerName, + notary_keys: &[Base64], + ) -> bool { + let Some(expected_keys) = batch.get(&keys.server_name) else { + trace!("Dropping signing keys chunk for unrequested server: {}", keys.server_name); + return false; + }; + let min_valid_ts = expected_keys + .values() + .map(|q| { + q.minimum_valid_until_ts + .unwrap_or_else(|| MilliSecondsSinceUnixEpoch(uint!(0))) + }) + .min() + .unwrap_or_else(|| MilliSecondsSinceUnixEpoch(uint!(0))); + + // If this notary response is older than any of the min_valid_ts we + // requested, then we should drop it. + if keys.valid_until_ts < min_valid_ts || keys.valid_until_ts > in_one_week() { + trace!( + ?min_valid_ts, + ?keys.valid_until_ts, + "Dropping signing keys chunk for server {} because it is older than the minimum \ + valid timestamp we requested", + keys.server_name + ); + return false; + } + + // Does this chunk even contain any keys we asked for? + // Note that this enforces min_valid_ts per-key, instead of using the + // lowest denominator. + let all_key_ids = keys + .verify_keys + .keys() + .chain(keys.old_verify_keys.keys()) + .map(ToOwned::to_owned) + .collect::>(); + let interesting = expected_keys.iter().any(|(k, q)| { + all_key_ids.contains(k) + && q.minimum_valid_until_ts + .is_none_or(|m| m <= keys.valid_until_ts) + }); + if !interesting { + trace!( + "Dropping signing keys chunk for server {} because it does not contain any keys \ + we requested", + keys.server_name + ); + return false; + } + + Self::verify_server_keys_response(keys, Some((notary_name, notary_keys))) + .inspect_err(|e| { + trace!( + ?e, + ?keys, + ?notary_name, + ?notary_keys, + "Dropping illegal server signing keys response chunk" + ); + warn!( + %notary_name, + server_name=%keys.server_name, + "Dropping illegal server signing keys response chunk: {e:?}" + ); + }) + .is_ok() + } } + +#[must_use] +pub fn in_one_week() -> MilliSecondsSinceUnixEpoch { + MilliSecondsSinceUnixEpoch( + UInt::new_saturating(millis_since_unix_epoch()).saturating_add(uint!(604_800_000)), + ) +} + +// TODO(nex): Unit tests for validate_notary_chunk would be really good, but are +// hard to do by nature of time advancing, signatures, and external notaries. +// We can rely on integration tests for now. diff --git a/src/service/server_keys/util.rs b/src/service/server_keys/util.rs index d73e338cf..eacf7d06d 100644 --- a/src/service/server_keys/util.rs +++ b/src/service/server_keys/util.rs @@ -7,7 +7,10 @@ use ruma::{ signatures::{VerificationError, required_server_signatures_to_verify_event}, }; -/// Extracts the server names and key ids to check signatures for given event. +/// Extracts the server names and key ids required to check signatures for the +/// given event. +/// +/// Spec: https://spec.matrix.org/v1.19/server-server-api/#validating-hashes-and-signatures-on-received-events pub(super) fn required_keys( object: &CanonicalJsonObject, rules: &SignaturesRules, diff --git a/src/service/server_keys/verify.rs b/src/service/server_keys/verify.rs index 8d28e10df..a13336605 100644 --- a/src/service/server_keys/verify.rs +++ b/src/service/server_keys/verify.rs @@ -1,44 +1,32 @@ -use conduwuit::{Err, Result, matrix::event::gen_event_id_canonical_json}; +use conduwuit::{ + Err, Result, matrix::event::gen_event_id_canonical_json, trace, utils::to_canonical_object, +}; use ruma::{ - CanonicalJsonObject, CanonicalJsonValue, OwnedEventId, room_version_rules::RoomVersionRules, + CanonicalJsonObject, CanonicalJsonValue, OwnedEventId, ServerName, SigningKeyAlgorithm, + api::federation::discovery::ServerSigningKeys, + room_version_rules::RoomVersionRules, + serde::{Base64, base64::Standard}, signatures::Verified, }; use serde_json::value::RawValue as RawJsonValue; impl super::Service { - /// Validates the incoming event, and then inserts the calculated event ID - /// into the event_id field. - pub async fn validate_and_add_event_id( - &self, - pdu: &RawJsonValue, - room_version_rules: &RoomVersionRules, - ) -> Result<(OwnedEventId, CanonicalJsonObject)> { - let (event_id, mut value) = gen_event_id_canonical_json(pdu, room_version_rules)?; - if let Err(e) = self.verify_event(&value, room_version_rules).await { - return Err!(BadServerResponse(debug_error!( - "Event {event_id} failed verification: {e:?}" - ))); - } - - value.insert("event_id".into(), CanonicalJsonValue::String(event_id.as_str().into())); - - Ok((event_id, value)) - } - /// Validates the incoming event only using locally known verification keys. /// Inserts the calculated event ID into the event_id field upon success. - pub async fn validate_and_add_event_id_no_fetch( + pub async fn verify_event_json_no_fetch_add_event_id( &self, pdu: &RawJsonValue, room_version_rules: &RoomVersionRules, ) -> Result<(OwnedEventId, CanonicalJsonObject)> { + // TODO(nex): this function is used in two places (same file) and is + // awfully named, can we just kill it? let (event_id, mut value) = gen_event_id_canonical_json(pdu, room_version_rules)?; if !self.required_keys_exist(&value, room_version_rules).await { return Err!(BadServerResponse(debug_warn!( - "Event {event_id} cannot be verified: missing keys." + "Event {event_id} cannot be verified: missing verify key(s) locally." ))); } - if let Err(e) = self.verify_event(&value, room_version_rules).await { + if let Err(e) = self.verify_event_json(&value, room_version_rules).await { return Err!(BadServerResponse(debug_error!( "Event {event_id} failed verification: {e:?}" ))); @@ -50,7 +38,7 @@ impl super::Service { } /// Verifies the incoming event after fetching the keys required to do so. - pub async fn verify_event( + pub async fn verify_event_json( &self, event: &CanonicalJsonObject, room_version_rules: &RoomVersionRules, @@ -59,15 +47,267 @@ impl super::Service { ruma::signatures::verify_event(&keys, event, room_version_rules).map_err(Into::into) } - /// Verifies an arbitrary JSON object after fetching the keys required to do - /// so. - pub async fn verify_json( - &self, - event: &CanonicalJsonObject, - room_version_rules: &RoomVersionRules, - ) -> Result { - // TODO: is this actually used anywhere? - let keys = self.get_event_keys(event, room_version_rules).await?; - ruma::signatures::verify_json(&keys, event).map_err(Into::into) + /// Verifies a server signing keys response, either as the + /// result of `GET /_matrix/key/v2/server`, or a chunk from + /// `POST /_matrix/key/v2/query`. + /// + /// Returns an error if the self-signature is invalid, or strict notary + /// validation is used (`notary` is non-empty) and the notary did not sign + /// the response. + /// + /// `notary` must either be empty or have ONE entry. Extraneous notaries + /// will be ignored. + pub fn verify_server_keys_response( + server_keys: &ServerSigningKeys, + notary: Option<(&ServerName, &[Base64])>, + ) -> Result<()> { + if let Some((notary_name, notary_keys)) = notary { + if !notary_keys.is_empty() { + Self::verify_notary_signature(server_keys, notary_name, notary_keys)?; + } + } + + // NOTE: old_verify_keys aren't considered here because old keys can't + // be used to sign key responses. + let mut set = ruma::signatures::PublicKeySet::new(); + server_keys.verify_keys.iter().for_each(|(key_id, key)| { + set.insert(key_id.to_string(), key.key.clone()); + }); + + let mut pubkey_map = ruma::signatures::PublicKeyMap::new(); + pubkey_map.insert(server_keys.server_name.to_string(), set); + + let mut canonical = to_canonical_object(server_keys)?; + Self::strip_extraneous_signatures(&mut canonical, &server_keys.server_name, &[]); + ruma::signatures::verify_json(&pubkey_map, &canonical).map_err(Into::into) + } + + /// Verifies that the notary has at least one valid signature present with + /// the provided signing keys. + fn verify_notary_signature( + server_keys: &ServerSigningKeys, + notary_name: &ServerName, + notary_keys: &[Base64], + ) -> Result<()> { + // This strictly verifies that the notary signed this response. + // This is used to mitigate exploits where a notary's TLS + // certificate has been compromised, which would otherwise allow + // them to produce attacker-controlled signing keys with valid + // self-signatures. + let Some(notary_signatures) = server_keys.signatures.get(notary_name) else { + return Err!(Request(Forbidden( + "Missing notary server signature from {notary_name}" + ))); + }; + let mut canonical_object = to_canonical_object(notary_signatures)?; + Self::strip_extraneous_signatures(&mut canonical_object, &server_keys.server_name, &[ + notary_name, + ]); + let for_verify = ruma::signatures::to_canonical_json_string_for_signing( + &to_canonical_object(server_keys).expect("server keys object must be canonical json"), + ) + .expect("canonical JSON must be stringable"); + + let raw_notary_keys = notary_keys.iter().map(Base64::as_bytes).collect::>(); + let raw_notary_signatures = notary_signatures + .values() + .flat_map(Base64::::parse); + + for signature in raw_notary_signatures { + let signature_bytes = signature.as_bytes(); + for notary_key in raw_notary_keys.iter().copied() { + let verified = ruma::signatures::verify_canonical_json_bytes( + &SigningKeyAlgorithm::Ed25519, + notary_key, + signature_bytes, + for_verify.as_bytes(), + ); + if verified.is_ok() { + trace!( + "Valid signature from {notary_name} present on signing keys response \ + for {}", + server_keys.server_name + ); + return Ok(()); + } + } + } + + Err!(Request(Forbidden( + "No valid signature from {notary_name} present on signing keys response" + ))) + } + + fn strip_extraneous_signatures( + canonical: &mut CanonicalJsonObject, + origin: &ServerName, + notaries: &[&ServerName], + ) { + canonical.entry("signatures".to_owned()).and_modify(|sigs| { + sigs.as_object_mut().map(|s| { + s.retain(|server_name, _| { + let Ok(server_name) = ServerName::parse(server_name) else { return false }; + origin == server_name || notaries.iter().any(|ns| *ns == server_name) + }); + Some(s) + }); + }); + } +} + +#[cfg(test)] +mod tests { + use ruma::{ + ServerSigningKeyId, + api::federation::discovery::get_server_keys::v2::Response as DirectKeysResponse, + owned_server_name, serde::Raw, server_name, + }; + + use super::*; + use crate::server_keys::Service; + + #[test] + fn verify_direct_server_keys_response() { + let response_data = serde_json::from_value(serde_json::json!( + { + "old_verify_keys": {}, + "server_name": "continuwuity.org", + "signatures": { + "continuwuity.org": { + "ed25519:PwHlNsFu": "YUA+lUu5tQcRhd44bxFcY+WRu8X9nLXT8FUpY44O48/oKF/9rWUhqJGw5uZeOuo2oBMAxtUmO6HT2kBI9WftBA" + } + }, + "valid_until_ts": 1787690539756_i64, + "verify_keys": { + "ed25519:PwHlNsFu": { + "key": "8eNx2s0zWW+heKAmOH5zKv/nCPkEpraDJfGHxDu6hFI" + } + } + } + )).map(Raw::from_json).unwrap(); + + Service::verify_server_keys_response(&response_data.deserialize().unwrap(), None) + .expect("Self-signature of real key response is valid"); + } + + #[test] + fn verify_direct_server_keys_response_ignores_extraneous_signatures() { + let response_data = serde_json::from_value(serde_json::json!( + { + "old_verify_keys": {}, + "server_name": "continuwuity.org", + "signatures": { + "continuwuity.org": { + "ed25519:PwHlNsFu": "YUA+lUu5tQcRhd44bxFcY+WRu8X9nLXT8FUpY44O48/oKF/9rWUhqJGw5uZeOuo2oBMAxtUmO6HT2kBI9WftBA" + }, + "example.org": { + "ed25519:1": "yadda yadda" + } + }, + "valid_until_ts": 1787690539756_i64, + "verify_keys": { + "ed25519:PwHlNsFu": { + "key": "8eNx2s0zWW+heKAmOH5zKv/nCPkEpraDJfGHxDu6hFI" + } + } + } + )).map(Raw::from_json).unwrap(); + + Service::verify_server_keys_response(&response_data.deserialize().unwrap(), None) + .expect("Self-signature of real key response is valid"); + } + + #[test] + fn verify_notary_response_checks_valid_notary_signature() { + let response_data = serde_json::from_value(serde_json::json!( + { + "old_verify_keys": {}, + "server_name": "continuwuity.org", + "signatures": { + "continuwuity.org": { + "ed25519:PwHlNsFu": "4YjHGqJ0HN93tvev4kx0qx/3w/h1nlEkFZni15h+5MZ91xUhQNa0BzqdraPU4/KJoXVcOQpmR+hsCVYb2/WlAQ" + }, + "starstruck.systems": { + "ed25519:a_Herg": "G4uXHfn9lizmixPFCBRXVnyf1HB1yyrxPVbtSOdViNHRFrFF66xmEpzDPs341L/Pjxtwfndi4Hw7zIsyTRqoCQ" + } + }, + "valid_until_ts": 1787691347175_i64, + "verify_keys": { + "ed25519:PwHlNsFu": { + "key": "8eNx2s0zWW+heKAmOH5zKv/nCPkEpraDJfGHxDu6hFI" + } + } + } + )).map(Raw::from_json).unwrap(); + let starstruck_pubkey = + Base64::::parse("Z4oHuEjdUbtC5W/z/gzc5B8raxNk+8gGwbNwgxHqW8Y").unwrap(); + Service::verify_server_keys_response( + &response_data.deserialize().unwrap(), + Some((server_name!("starstruck.systems"), &[starstruck_pubkey])), + ) + .expect("Server keys from notary are valid"); + } + + #[test] + fn ensure_verify_server_keys_response_rejects_invalid_self_signature() { + // even something as small as a mismatch in the timestamp should be + // enough to trigger this. + let response_data = serde_json::from_value(serde_json::json!( + { + "old_verify_keys": {}, + "server_name": "continuwuity.org", + "signatures": { + "continuwuity.org": { + "ed25519:PwHlNsFu": "YUA+lUu5tQcRhd44bxFcY+WRu8X9nLXT8FUpY44O48/oKF/9rWUhqJGw5uZeOuo2oBMAxtUmO6HT2kBI9WftBA" + } + }, + "valid_until_ts": 1787690539757_i64, + "verify_keys": { + "ed25519:PwHlNsFu": { + "key": "8eNx2s0zWW+heKAmOH5zKv/nCPkEpraDJfGHxDu6hFI" + } + } + } + )).map(Raw::from_json).unwrap(); + Service::verify_server_keys_response(&response_data.deserialize().unwrap(), None) + .expect_err("Tampered response should fail self-signature check"); + } + + #[test] + fn ensure_mismatched_notary_signature_fails() { + // The signature on a notary response is typically useless. However, + // when additional verification is configured, it can be used to prevent + // an attack where the notary's TLS is intercepted, and the attacker + // returns a valid signing key response that is signed by the attacker + // with the attacker controlled key. In this case, the self-signature + // would pass, and the attacker would be allowed to inject an + // adversarial signing key into our database, allowing them to + // completely forge any event they ever want to as the target. + // However, by basically doing key pinning, this attack can be mitigated + // by raising the attack difficulty to requiring both TLS interception, + // and the target notary's private key. + + // In this case, we'll use a valid self-signature, BUT incorrect + // notary signature. + // Without the additional notary validation, this would permit as TOFU. + let response_data = serde_json::from_value(serde_json::json!( + { + "old_verify_keys": {}, + "server_name": "continuwuity.org", + "signatures": { + "continuwuity.org": { + "ed25519:PwHlNsFu": "Ov3RCRYRUEgaMPfHP4YPUvjoBKE0yVTERNlgeHpxvh/+fknGeRnIZ4e6b0n1aI+gPU04Eb1DE4gaeTFigCudAg" + } + }, + "valid_until_ts": 1787690539756_i64, + "verify_keys": { + "ed25519:PwHlNsFu": { + "key": "8eNx2s0zWW+heKAmOH5zKv/nCPkEpraDJfGHxDu6hFI" + } + } + } + )).map(Raw::from_json).unwrap(); + Service::verify_server_keys_response(&response_data.deserialize().unwrap(), None) + .expect_err("Intercepted notary response should fail strict validation"); } }