From c453d45598f1043b432149b714e2565f12f32360 Mon Sep 17 00:00:00 2001 From: Matthias Ahouansou Date: Wed, 12 Jun 2024 19:22:19 +0200 Subject: [PATCH] fix(keys): only use keys valid at the time of PDU or transaction, and actually refresh keys Previously, we only fetched keys once, only requesting them again if we have any missing, allowing for ancient keys to be used to sign PDUs and transactions Now we refresh keys that either have or are about to expire, preventing attacks that make use of leaked private keys of a homeserver We also ensure that when validating PDUs or transactions, that they are valid at the origin_server_ts or time of us receiving the transaction respectfully As to not break event authorization for old rooms, we need to keep old keys around We move verify_keys which we no longer see in direct requests to the origin to old_verify_keys We keep old_verify_keys indefinitely as mentioned above, as to not break event authorization (at least until a future MSC addresses this) --- src/api/client_server/membership.rs | 43 ++- src/api/ruma_wrapper/axum.rs | 19 +- src/database/key_value/globals.rs | 131 +++++---- src/service/admin/mod.rs | 44 ++- src/service/globals/data.rs | 92 +++++- src/service/globals/mod.rs | 107 +++++-- src/service/rooms/event_handler/mod.rs | 376 ++++++++++++++++--------- src/service/rooms/timeline/mod.rs | 8 +- 8 files changed, 584 insertions(+), 236 deletions(-) diff --git a/src/api/client_server/membership.rs b/src/api/client_server/membership.rs index 25a9061a..1ca711e2 100644 --- a/src/api/client_server/membership.rs +++ b/src/api/client_server/membership.rs @@ -18,9 +18,8 @@ use ruma::{ }, StateEventType, TimelineEventType, }, - serde::Base64, - state_res, CanonicalJsonObject, CanonicalJsonValue, EventId, OwnedEventId, OwnedRoomId, - OwnedServerName, OwnedUserId, RoomId, RoomVersionId, UserId, + state_res, CanonicalJsonObject, CanonicalJsonValue, EventId, MilliSecondsSinceUnixEpoch, + OwnedEventId, OwnedRoomId, OwnedServerName, OwnedUserId, RoomId, RoomVersionId, UserId, }; use serde_json::value::{to_raw_value, RawValue as RawJsonValue}; use std::{ @@ -32,7 +31,10 @@ use tokio::sync::RwLock; use tracing::{debug, error, info, warn}; use crate::{ - service::pdu::{gen_event_id_canonical_json, PduBuilder}, + service::{ + globals::SigningKeys, + pdu::{gen_event_id_canonical_json, PduBuilder}, + }, services, utils, Error, PduEvent, Result, Ruma, }; @@ -1130,7 +1132,7 @@ async fn make_join_request( async fn validate_and_add_event_id( pdu: &RawJsonValue, room_version: &RoomVersionId, - pub_key_map: &RwLock>>, + pub_key_map: &RwLock>, ) -> Result<(OwnedEventId, CanonicalJsonObject)> { let mut value: CanonicalJsonObject = serde_json::from_str(pdu.get()).map_err(|e| { error!("Invalid PDU in server response: {:?}: {:?}", pdu, e); @@ -1177,8 +1179,35 @@ async fn validate_and_add_event_id( } } - if let Err(e) = ruma::signatures::verify_event(&*pub_key_map.read().await, &value, room_version) - { + let origin_server_ts = value.get("origin_server_ts").ok_or_else(|| { + error!("Invalid PDU, no origin_server_ts field"); + Error::BadRequest( + ErrorKind::MissingParam, + "Invalid PDU, no origin_server_ts field", + ) + })?; + + let origin_server_ts: MilliSecondsSinceUnixEpoch = { + let ts = origin_server_ts.as_integer().ok_or_else(|| { + Error::BadRequest( + ErrorKind::InvalidParam, + "origin_server_ts must be an integer", + ) + })?; + + MilliSecondsSinceUnixEpoch(i64::from(ts).try_into().map_err(|_| { + Error::BadRequest(ErrorKind::InvalidParam, "Time must be after the unix epoch") + })?) + }; + + let unfiltered_keys = (*pub_key_map.read().await).clone(); + + let keys = + services() + .globals + .filter_keys_server_map(unfiltered_keys, origin_server_ts, room_version); + + if let Err(e) = ruma::signatures::verify_event(&keys, &value, room_version) { warn!("Event {} failed verification {:?} {}", event_id, pdu, e); back_off(event_id).await; return Err(Error::BadServerResponse("Event failed verification.")); diff --git a/src/api/ruma_wrapper/axum.rs b/src/api/ruma_wrapper/axum.rs index 9411c533..047f7dcf 100644 --- a/src/api/ruma_wrapper/axum.rs +++ b/src/api/ruma_wrapper/axum.rs @@ -14,7 +14,7 @@ use http::{Request, StatusCode}; use ruma::{ api::{client::error::ErrorKind, AuthScheme, IncomingRequest, OutgoingResponse}, server_util::authorization::XMatrix, - CanonicalJsonValue, OwnedDeviceId, OwnedUserId, UserId, + CanonicalJsonValue, MilliSecondsSinceUnixEpoch, OwnedDeviceId, OwnedUserId, UserId, }; use serde::Deserialize; use tracing::{debug, error, warn}; @@ -231,7 +231,7 @@ where let keys_result = services() .rooms .event_handler - .fetch_signing_keys(&x_matrix.origin, vec![x_matrix.key.to_string()]) + .fetch_signing_keys(&x_matrix.origin, vec![x_matrix.key.to_string()], false) .await; let keys = match keys_result { @@ -245,8 +245,19 @@ where } }; - let pub_key_map = - BTreeMap::from_iter([(x_matrix.origin.as_str().to_owned(), keys)]); + // Only verify_keys that are currently valid should be used for validating requests + // as per MSC4029 + let pub_key_map = BTreeMap::from_iter([( + x_matrix.origin.as_str().to_owned(), + if keys.valid_until_ts > MilliSecondsSinceUnixEpoch::now() { + keys.verify_keys + .into_iter() + .map(|(id, key)| (id, key.key)) + .collect() + } else { + BTreeMap::new() + }, + )]); match ruma::signatures::verify_json(&pub_key_map, &request_map) { Ok(()) => (None, None, Some(x_matrix.origin), None), diff --git a/src/database/key_value/globals.rs b/src/database/key_value/globals.rs index 2851ce53..bd47cb42 100644 --- a/src/database/key_value/globals.rs +++ b/src/database/key_value/globals.rs @@ -1,15 +1,19 @@ -use std::collections::{BTreeMap, HashMap}; +use std::collections::HashMap; use async_trait::async_trait; use futures_util::{stream::FuturesUnordered, StreamExt}; use lru_cache::LruCache; use ruma::{ - api::federation::discovery::{ServerSigningKeys, VerifyKey}, + api::federation::discovery::{OldVerifyKey, ServerSigningKeys}, signatures::Ed25519KeyPair, - DeviceId, MilliSecondsSinceUnixEpoch, OwnedServerSigningKeyId, ServerName, UserId, + DeviceId, ServerName, UserId, }; -use crate::{database::KeyValueDatabase, service, services, utils, Error, Result}; +use crate::{ + database::KeyValueDatabase, + service::{self, globals::SigningKeys}, + services, utils, Error, Result, +}; pub const COUNTER: &[u8] = b"c"; pub const LAST_CHECK_FOR_UPDATES_COUNT: &[u8] = b"u"; @@ -237,64 +241,97 @@ lasttimelinecount_cache: {lasttimelinecount_cache}\n" self.global.remove(b"keypair") } - fn add_signing_key( + fn add_signing_key_from_trusted_server( &self, origin: &ServerName, new_keys: ServerSigningKeys, - ) -> Result> { - // Not atomic, but this is not critical - let signingkeys = self.server_signingkeys.get(origin.as_bytes())?; + ) -> Result { + let prev_keys = self.server_signingkeys.get(origin.as_bytes())?; - let mut keys = signingkeys - .and_then(|keys| serde_json::from_slice(&keys).ok()) - .unwrap_or_else(|| { - // Just insert "now", it doesn't matter - ServerSigningKeys::new(origin.to_owned(), MilliSecondsSinceUnixEpoch::now()) - }); + Ok( + if let Some(mut prev_keys) = + prev_keys.and_then(|keys| serde_json::from_slice::(&keys).ok()) + { + let ServerSigningKeys { + verify_keys, + old_verify_keys, + .. + } = new_keys; - let ServerSigningKeys { - verify_keys, - old_verify_keys, - .. - } = new_keys; + prev_keys.verify_keys.extend(verify_keys); + prev_keys.old_verify_keys.extend(old_verify_keys); + prev_keys.valid_until_ts = new_keys.valid_until_ts; - keys.verify_keys.extend(verify_keys); - keys.old_verify_keys.extend(old_verify_keys); + self.server_signingkeys.insert( + origin.as_bytes(), + &serde_json::to_vec(&prev_keys).expect("serversigningkeys can be serialized"), + )?; - self.server_signingkeys.insert( - origin.as_bytes(), - &serde_json::to_vec(&keys).expect("serversigningkeys can be serialized"), - )?; + prev_keys.into() + } else { + self.server_signingkeys.insert( + origin.as_bytes(), + &serde_json::to_vec(&new_keys).expect("serversigningkeys can be serialized"), + )?; - let mut tree = keys.verify_keys; - tree.extend( - keys.old_verify_keys - .into_iter() - .map(|old| (old.0, VerifyKey::new(old.1.key))), - ); + new_keys.into() + }, + ) + } - Ok(tree) + fn add_signing_key_from_origin( + &self, + origin: &ServerName, + new_keys: ServerSigningKeys, + ) -> Result { + let prev_keys = self.server_signingkeys.get(origin.as_bytes())?; + + Ok( + if let Some(mut prev_keys) = + prev_keys.and_then(|keys| serde_json::from_slice::(&keys).ok()) + { + let ServerSigningKeys { + verify_keys, + old_verify_keys, + .. + } = new_keys; + + // Moving `verify_keys` no longer present to `old_verify_keys` + for (key_id, key) in prev_keys.verify_keys { + if !verify_keys.contains_key(&key_id) { + prev_keys + .old_verify_keys + .insert(key_id, OldVerifyKey::new(prev_keys.valid_until_ts, key.key)); + } + } + + prev_keys.verify_keys = verify_keys; + prev_keys.old_verify_keys.extend(old_verify_keys); + prev_keys.valid_until_ts = new_keys.valid_until_ts; + + self.server_signingkeys.insert( + origin.as_bytes(), + &serde_json::to_vec(&prev_keys).expect("serversigningkeys can be serialized"), + )?; + + prev_keys.into() + } else { + self.server_signingkeys.insert( + origin.as_bytes(), + &serde_json::to_vec(&new_keys).expect("serversigningkeys can be serialized"), + )?; + + new_keys.into() + }, + ) } /// This returns an empty `Ok(BTreeMap<..>)` when there are no keys found for the server. - fn signing_keys_for( - &self, - origin: &ServerName, - ) -> Result> { + fn signing_keys_for(&self, origin: &ServerName) -> Result> { let signingkeys = self .server_signingkeys .get(origin.as_bytes())? - .and_then(|bytes| serde_json::from_slice(&bytes).ok()) - .map(|keys: ServerSigningKeys| { - let mut tree = keys.verify_keys; - tree.extend( - keys.old_verify_keys - .into_iter() - .map(|old| (old.0, VerifyKey::new(old.1.key))), - ); - tree - }) - .unwrap_or_else(BTreeMap::new); + .and_then(|bytes| serde_json::from_slice::(&bytes).ok()); Ok(signingkeys) } diff --git a/src/service/admin/mod.rs b/src/service/admin/mod.rs index b3b7a74e..70c63381 100644 --- a/src/service/admin/mod.rs +++ b/src/service/admin/mod.rs @@ -19,7 +19,8 @@ use ruma::{ }, TimelineEventType, }, - EventId, OwnedRoomAliasId, OwnedRoomId, RoomAliasId, RoomId, RoomVersionId, ServerName, UserId, + EventId, MilliSecondsSinceUnixEpoch, OwnedRoomAliasId, OwnedRoomId, RoomAliasId, RoomId, + RoomVersionId, ServerName, UserId, }; use serde_json::value::to_raw_value; use tokio::sync::{mpsc, Mutex, RwLock}; @@ -858,15 +859,46 @@ impl Service { services() .rooms .event_handler + // Generally we shouldn't be checking against expired keys unless required, so in the admin + // room it might be best to not allow expired keys .fetch_required_signing_keys(&value, &pub_key_map) .await?; - let pub_key_map = pub_key_map.read().await; - match ruma::signatures::verify_json(&pub_key_map, &value) { - Ok(_) => RoomMessageEventContent::text_plain("Signature correct"), - Err(e) => RoomMessageEventContent::text_plain(format!( + let mut expired_key_map = BTreeMap::new(); + let mut valid_key_map = BTreeMap::new(); + + for (server, keys) in pub_key_map.into_inner().into_iter() { + if keys.valid_until_ts > MilliSecondsSinceUnixEpoch::now() { + valid_key_map.insert( + server, + keys.verify_keys + .into_iter() + .map(|(id, key)| (id, key.key)) + .collect(), + ); + } else { + expired_key_map.insert( + server, + keys.verify_keys + .into_iter() + .map(|(id, key)| (id, key.key)) + .collect(), + ); + } + } + + if ruma::signatures::verify_json(&valid_key_map, &value).is_ok() { + RoomMessageEventContent::text_plain("Signature correct") + } else if let Err(e) = + ruma::signatures::verify_json(&expired_key_map, &value) + { + RoomMessageEventContent::text_plain(format!( "Signature verification failed: {e}" - )), + )) + } else { + RoomMessageEventContent::text_plain( + "Signature correct (with expired keys)", + ) } } Err(e) => RoomMessageEventContent::text_plain(format!("Invalid json: {e}")), diff --git a/src/service/globals/data.rs b/src/service/globals/data.rs index 8a66751b..167e823c 100644 --- a/src/service/globals/data.rs +++ b/src/service/globals/data.rs @@ -1,13 +1,71 @@ -use std::collections::BTreeMap; - -use async_trait::async_trait; -use ruma::{ - api::federation::discovery::{ServerSigningKeys, VerifyKey}, - signatures::Ed25519KeyPair, - DeviceId, OwnedServerSigningKeyId, ServerName, UserId, +use std::{ + collections::BTreeMap, + time::{Duration, SystemTime}, }; -use crate::Result; +use crate::{services, Result}; +use async_trait::async_trait; +use ruma::{ + api::federation::discovery::{OldVerifyKey, ServerSigningKeys, VerifyKey}, + serde::Base64, + signatures::Ed25519KeyPair, + DeviceId, MilliSecondsSinceUnixEpoch, ServerName, UserId, +}; +use serde::Deserialize; + +/// Similar to ServerSigningKeys, but drops a few unnecessary fields we don't require post-validation +#[derive(Deserialize, Debug, Clone)] +pub struct SigningKeys { + pub verify_keys: BTreeMap, + pub old_verify_keys: BTreeMap, + pub valid_until_ts: MilliSecondsSinceUnixEpoch, +} + +impl SigningKeys { + /// Creates the SigningKeys struct, using the keys of the current server + pub fn load_own_keys() -> Self { + let mut keys = Self { + verify_keys: BTreeMap::new(), + old_verify_keys: BTreeMap::new(), + valid_until_ts: MilliSecondsSinceUnixEpoch::from_system_time( + SystemTime::now() + Duration::from_secs(7 * 86400), + ) + .expect("Should be valid until year 500,000,000"), + }; + + keys.verify_keys.insert( + format!("ed25519:{}", services().globals.keypair().version()), + VerifyKey { + key: Base64::new(services().globals.keypair.public_key().to_vec()), + }, + ); + + keys + } +} + +impl From for SigningKeys { + fn from(value: ServerSigningKeys) -> Self { + let ServerSigningKeys { + verify_keys, + old_verify_keys, + valid_until_ts, + .. + } = value; + + Self { + verify_keys: verify_keys + .into_iter() + .map(|(id, key)| (id.to_string(), key)) + .collect(), + old_verify_keys: old_verify_keys + .into_iter() + .map(|(id, key)| (id.to_string(), key)) + .collect(), + valid_until_ts, + } + } +} #[async_trait] pub trait Data: Send + Sync { @@ -21,17 +79,23 @@ pub trait Data: Send + Sync { fn clear_caches(&self, amount: u32); fn load_keypair(&self) -> Result; fn remove_keypair(&self) -> Result<()>; - fn add_signing_key( + /// Only extends the cached keys, not moving any verify_keys to old_verify_keys, as if we suddenly + /// recieve requests from the origin server, we want to be able to accept requests from them + fn add_signing_key_from_trusted_server( &self, origin: &ServerName, new_keys: ServerSigningKeys, - ) -> Result>; - - /// This returns an empty `Ok(BTreeMap<..>)` when there are no keys found for the server. - fn signing_keys_for( + ) -> Result; + /// Extends cached keys, as well as moving verify_keys that are not present in these new keys to + /// old_verify_keys, so that potnetially comprimised keys cannot be used to make requests + fn add_signing_key_from_origin( &self, origin: &ServerName, - ) -> Result>; + new_keys: ServerSigningKeys, + ) -> Result; + + /// This returns an empty `Ok(BTreeMap<..>)` when there are no keys found for the server. + fn signing_keys_for(&self, origin: &ServerName) -> Result>; fn database_version(&self) -> Result; fn bump_database_version(&self, new_version: u64) -> Result<()>; } diff --git a/src/service/globals/mod.rs b/src/service/globals/mod.rs index caf2b3a3..fc695f86 100644 --- a/src/service/globals/mod.rs +++ b/src/service/globals/mod.rs @@ -1,9 +1,8 @@ mod data; pub use data::Data; -use ruma::{ - serde::Base64, OwnedDeviceId, OwnedEventId, OwnedRoomId, OwnedServerName, - OwnedServerSigningKeyId, OwnedUserId, -}; +pub use data::SigningKeys; +use ruma::MilliSecondsSinceUnixEpoch; +use ruma::{serde::Base64, OwnedDeviceId, OwnedEventId, OwnedRoomId, OwnedServerName, OwnedUserId}; use ruma::{OwnedRoomAliasId, RoomAliasId}; use crate::api::server_server::FedDest; @@ -14,10 +13,7 @@ use hickory_resolver::TokioAsyncResolver; use hyper_util::client::legacy::connect::dns::{GaiResolver, Name as HyperName}; use reqwest::dns::{Addrs, Name, Resolve, Resolving}; use ruma::{ - api::{ - client::sync::sync_events, - federation::discovery::{ServerSigningKeys, VerifyKey}, - }, + api::{client::sync::sync_events, federation::discovery::ServerSigningKeys}, DeviceId, RoomVersionId, ServerName, UserId, }; use std::str::FromStr; @@ -393,36 +389,89 @@ impl Service { room_versions } - /// TODO: the key valid until timestamp is only honored in room version > 4 - /// Remove the outdated keys and insert the new ones. - /// /// This doesn't actually check that the keys provided are newer than the old set. - pub fn add_signing_key( + pub fn add_signing_key_from_trusted_server( &self, origin: &ServerName, new_keys: ServerSigningKeys, - ) -> Result> { - self.db.add_signing_key(origin, new_keys) + ) -> Result { + self.db + .add_signing_key_from_trusted_server(origin, new_keys) } - /// This returns an empty `Ok(BTreeMap<..>)` when there are no keys found for the server. - pub fn signing_keys_for( + /// Same as from_trusted_server, except it will move active keys not present in `new_keys` to old_signing_keys + pub fn add_signing_key_from_origin( &self, origin: &ServerName, - ) -> Result> { - let mut keys = self.db.signing_keys_for(origin)?; - if origin == self.server_name() { - keys.insert( - format!("ed25519:{}", services().globals.keypair().version()) - .try_into() - .expect("found invalid server signing keys in DB"), - VerifyKey { - key: Base64::new(self.keypair.public_key().to_vec()), - }, - ); - } + new_keys: ServerSigningKeys, + ) -> Result { + self.db.add_signing_key_from_origin(origin, new_keys) + } - Ok(keys) + /// This returns Ok(None) when there are no keys found for the server. + pub fn signing_keys_for(&self, origin: &ServerName) -> Result> { + Ok(self.db.signing_keys_for(origin)?.or_else(|| { + if origin == self.server_name() { + Some(SigningKeys::load_own_keys()) + } else { + None + } + })) + } + + /// Filters the key map of multiple servers down to keys that should be accepted given the expiry time, + /// room version, and timestamp of the paramters + pub fn filter_keys_server_map( + &self, + keys: BTreeMap, + timestamp: MilliSecondsSinceUnixEpoch, + room_version_id: &RoomVersionId, + ) -> BTreeMap> { + keys.into_iter() + .filter_map(|(server, keys)| { + self.filter_keys_single_server(keys, timestamp, room_version_id) + .map(|keys| (server, keys)) + }) + .collect() + } + + /// Filters the keys of a single server down to keys that should be accepted given the expiry time, + /// room version, and timestamp of the paramters + pub fn filter_keys_single_server( + &self, + keys: SigningKeys, + timestamp: MilliSecondsSinceUnixEpoch, + room_version_id: &RoomVersionId, + ) -> Option> { + if keys.valid_until_ts > timestamp + // valid_until_ts MUST be ignored in room versions 1, 2, 3, and 4. + // https://spec.matrix.org/v1.10/server-server-api/#get_matrixkeyv2server + || matches!(room_version_id, RoomVersionId::V1 + | RoomVersionId::V2 + | RoomVersionId::V4 + | RoomVersionId::V3) + { + // Given that either the room version allows stale keys, or the valid_until_ts is + // in the future, all verify_keys are valid + let mut map: BTreeMap<_, _> = keys + .verify_keys + .into_iter() + .map(|(id, key)| (id, key.key)) + .collect(); + + map.extend(keys.old_verify_keys.into_iter().filter_map(|(id, key)| { + // Even on old room versions, we don't allow old keys if they are expired + if key.expired_ts > timestamp { + Some((id, key.key)) + } else { + None + } + })); + + Some(map) + } else { + None + } } pub fn database_version(&self) -> Result { diff --git a/src/service/rooms/event_handler/mod.rs b/src/service/rooms/event_handler/mod.rs index 13d855dc..0bdfd4ae 100644 --- a/src/service/rooms/event_handler/mod.rs +++ b/src/service/rooms/event_handler/mod.rs @@ -9,6 +9,7 @@ use std::{ }; use futures_util::{stream::FuturesUnordered, Future, StreamExt}; +use globals::SigningKeys; use ruma::{ api::{ client::error::ErrorKind, @@ -30,7 +31,6 @@ use ruma::{ StateEventType, TimelineEventType, }, int, - serde::Base64, state_res::{self, RoomVersion, StateMap}, uint, CanonicalJsonObject, CanonicalJsonValue, EventId, MilliSecondsSinceUnixEpoch, OwnedServerName, OwnedServerSigningKeyId, RoomId, RoomVersionId, ServerName, @@ -78,7 +78,7 @@ impl Service { room_id: &'a RoomId, value: BTreeMap, is_timeline_event: bool, - pub_key_map: &'a RwLock>>, + pub_key_map: &'a RwLock>, ) -> Result>> { // 0. Check the server is in the room if !services().rooms.metadata.exists(room_id)? { @@ -304,19 +304,12 @@ impl Service { room_id: &'a RoomId, mut value: BTreeMap, auth_events_known: bool, - pub_key_map: &'a RwLock>>, + pub_key_map: &'a RwLock>, ) -> AsyncRecursiveType<'a, Result<(Arc, BTreeMap)>> { Box::pin(async move { // 1.1. Remove unsigned field value.remove("unsigned"); - // TODO: For RoomVersion6 we must check that Raw<..> is canonical do we anywhere?: https://matrix.org/docs/spec/rooms/v6#canonical-json - - // We go through all the signatures we see on the value and fetch the corresponding signing - // keys - self.fetch_required_signing_keys(&value, pub_key_map) - .await?; - // 2. Check signatures, otherwise drop // 3. check content hash, redact if doesn't match let create_event_content: RoomCreateEventContent = @@ -329,41 +322,80 @@ impl Service { let room_version = RoomVersion::new(room_version_id).expect("room version is supported"); - let guard = pub_key_map.read().await; - let mut val = match ruma::signatures::verify_event(&guard, &value, room_version_id) { - Err(e) => { - // Drop - warn!("Dropping bad event {}: {}", event_id, e,); - return Err(Error::BadRequest( - ErrorKind::InvalidParam, - "Signature verification failed", - )); - } - Ok(ruma::signatures::Verified::Signatures) => { - // Redact - warn!("Calculated hash does not match: {}", event_id); - let obj = match ruma::canonical_json::redact(value, room_version_id, None) { - Ok(obj) => obj, - Err(_) => { - return Err(Error::BadRequest( - ErrorKind::InvalidParam, - "Redaction failed", - )) - } - }; + // TODO: For RoomVersion6 we must check that Raw<..> is canonical do we anywhere?: https://matrix.org/docs/spec/rooms/v6#canonical-json - // Skip the PDU if it is redacted and we already have it as an outlier event - if services().rooms.timeline.get_pdu_json(event_id)?.is_some() { + // We go through all the signatures we see on the value and fetch the corresponding signing + // keys + self.fetch_required_signing_keys(&value, pub_key_map) + .await?; + + let origin_server_ts = value.get("origin_server_ts").ok_or_else(|| { + error!("Invalid PDU, no origin_server_ts field"); + Error::BadRequest( + ErrorKind::MissingParam, + "Invalid PDU, no origin_server_ts field", + ) + })?; + + let origin_server_ts: MilliSecondsSinceUnixEpoch = { + let ts = origin_server_ts.as_integer().ok_or_else(|| { + Error::BadRequest( + ErrorKind::InvalidParam, + "origin_server_ts must be an integer", + ) + })?; + + MilliSecondsSinceUnixEpoch(i64::from(ts).try_into().map_err(|_| { + Error::BadRequest(ErrorKind::InvalidParam, "Time must be after the unix epoch") + })?) + }; + + let guard = pub_key_map.read().await; + + let pkey_map = (*guard).clone(); + + // Removing all the expired keys, unless the room version allows stale keys + let filtered_keys = services().globals.filter_keys_server_map( + pkey_map, + origin_server_ts, + room_version_id, + ); + + let mut val = + match ruma::signatures::verify_event(&filtered_keys, &value, room_version_id) { + Err(e) => { + // Drop + warn!("Dropping bad event {}: {}", event_id, e,); return Err(Error::BadRequest( ErrorKind::InvalidParam, - "Event was redacted and we already knew about it", + "Signature verification failed", )); } + Ok(ruma::signatures::Verified::Signatures) => { + // Redact + warn!("Calculated hash does not match: {}", event_id); + let obj = match ruma::canonical_json::redact(value, room_version_id, None) { + Ok(obj) => obj, + Err(_) => { + return Err(Error::BadRequest( + ErrorKind::InvalidParam, + "Redaction failed", + )) + } + }; - obj - } - Ok(ruma::signatures::Verified::All) => value, - }; + // Skip the PDU if it is redacted and we already have it as an outlier event + if services().rooms.timeline.get_pdu_json(event_id)?.is_some() { + return Err(Error::BadRequest( + ErrorKind::InvalidParam, + "Event was redacted and we already knew about it", + )); + } + + obj + } + Ok(ruma::signatures::Verified::All) => value, + }; drop(guard); @@ -487,7 +519,7 @@ impl Service { create_event: &PduEvent, origin: &ServerName, room_id: &RoomId, - pub_key_map: &RwLock>>, + pub_key_map: &RwLock>, ) -> Result>> { // Skip the PDU if we already have it as a timeline event if let Ok(Some(pduid)) = services().rooms.timeline.get_pdu_id(&incoming_pdu.event_id) { @@ -1097,7 +1129,7 @@ impl Service { create_event: &'a PduEvent, room_id: &'a RoomId, room_version_id: &'a RoomVersionId, - pub_key_map: &'a RwLock>>, + pub_key_map: &'a RwLock>, ) -> AsyncRecursiveType<'a, Vec<(Arc, Option>)>> { Box::pin(async move { @@ -1280,7 +1312,7 @@ impl Service { create_event: &PduEvent, room_id: &RoomId, room_version_id: &RoomVersionId, - pub_key_map: &RwLock>>, + pub_key_map: &RwLock>, initial_set: Vec>, ) -> Result<( Vec>, @@ -1378,7 +1410,7 @@ impl Service { pub(crate) async fn fetch_required_signing_keys( &self, event: &BTreeMap, - pub_key_map: &RwLock>>, + pub_key_map: &RwLock>, ) -> Result<()> { let signatures = event .get("signatures") @@ -1407,6 +1439,7 @@ impl Service { ) })?, signature_ids, + true, ) .await; @@ -1434,7 +1467,7 @@ impl Service { pdu: &RawJsonValue, servers: &mut BTreeMap>, room_version: &RoomVersionId, - pub_key_map: &mut RwLockWriteGuard<'_, BTreeMap>>, + pub_key_map: &mut RwLockWriteGuard<'_, BTreeMap>, ) -> Result<()> { let value: CanonicalJsonObject = serde_json::from_str(pdu.get()).map_err(|e| { error!("Invalid PDU in server response: {:?}: {:?}", pdu, e); @@ -1485,8 +1518,18 @@ impl Service { let signature_ids = signature_object.keys().cloned().collect::>(); - let contains_all_ids = |keys: &BTreeMap| { - signature_ids.iter().all(|id| keys.contains_key(id)) + let contains_all_ids = |keys: &SigningKeys| { + signature_ids.iter().all(|id| { + keys.verify_keys + .keys() + .map(ToString::to_string) + .any(|key_id| id == &key_id) + || keys + .old_verify_keys + .keys() + .map(ToString::to_string) + .any(|key_id| id == &key_id) + }) }; let origin = <&ServerName>::try_from(signature_server.as_str()).map_err(|_| { @@ -1499,19 +1542,14 @@ impl Service { trace!("Loading signing keys for {}", origin); - let result: BTreeMap<_, _> = services() - .globals - .signing_keys_for(origin)? - .into_iter() - .map(|(k, v)| (k.to_string(), v.key)) - .collect(); + if let Some(result) = services().globals.signing_keys_for(origin)? { + if !contains_all_ids(&result) { + trace!("Signing key not loaded for {}", origin); + servers.insert(origin.to_owned(), BTreeMap::new()); + } - if !contains_all_ids(&result) { - trace!("Signing key not loaded for {}", origin); - servers.insert(origin.to_owned(), BTreeMap::new()); + pub_key_map.insert(origin.to_string(), result); } - - pub_key_map.insert(origin.to_string(), result); } Ok(()) @@ -1521,7 +1559,7 @@ impl Service { &self, event: &create_join_event::v2::Response, room_version: &RoomVersionId, - pub_key_map: &RwLock>>, + pub_key_map: &RwLock>, ) -> Result<()> { let mut servers: BTreeMap< OwnedServerName, @@ -1584,10 +1622,7 @@ impl Service { let result = services() .globals - .add_signing_key(&k.server_name, k.clone())? - .into_iter() - .map(|(k, v)| (k.to_string(), v.key)) - .collect::>(); + .add_signing_key_from_trusted_server(&k.server_name, k.clone())?; pkm.insert(k.server_name.to_string(), result); } @@ -1618,12 +1653,9 @@ impl Service { if let (Ok(get_keys_response), origin) = result { info!("Result is from {origin}"); if let Ok(key) = get_keys_response.server_key.deserialize() { - let result: BTreeMap<_, _> = services() + let result = services() .globals - .add_signing_key(&origin, key)? - .into_iter() - .map(|(k, v)| (k.to_string(), v.key)) - .collect(); + .add_signing_key_from_origin(&origin, key)?; pub_key_map.write().await.insert(origin.to_string(), result); } } @@ -1681,9 +1713,23 @@ impl Service { &self, origin: &ServerName, signature_ids: Vec, - ) -> Result> { - let contains_all_ids = - |keys: &BTreeMap| signature_ids.iter().all(|id| keys.contains_key(id)); + // Whether to ask for keys from trusted servers. Should be false when getting + // keys for validating requests, as per MSC4029 + query_via_trusted_servers: bool, + ) -> Result { + let contains_all_ids = |keys: &SigningKeys| { + signature_ids.iter().all(|id| { + keys.verify_keys + .keys() + .map(ToString::to_string) + .any(|key_id| id == &key_id) + || keys + .old_verify_keys + .keys() + .map(ToString::to_string) + .any(|key_id| id == &key_id) + }) + }; let permit = services() .globals @@ -1744,94 +1790,172 @@ impl Service { trace!("Loading signing keys for {}", origin); - let mut result: BTreeMap<_, _> = services() - .globals - .signing_keys_for(origin)? - .into_iter() - .map(|(k, v)| (k.to_string(), v.key)) - .collect(); + let result = services().globals.signing_keys_for(origin)?; - if contains_all_ids(&result) { - return Ok(result); + let mut expires_soon_or_has_expired = false; + + if let Some(result) = result.clone() { + let ts_threshold = MilliSecondsSinceUnixEpoch::from_system_time( + SystemTime::now() + Duration::from_secs(30 * 60), + ) + .expect("Should be valid until year 500,000,000"); + + debug!( + "The treshhold is {:?}, found time is {:?} for server {}", + ts_threshold, result.valid_until_ts, origin + ); + + if contains_all_ids(&result) { + // We want to ensure that the keys remain valid by the time the other functions that handle signatures reach them + if result.valid_until_ts > ts_threshold { + debug!( + "Keys for {} are deemed as valid, as they expire at {:?}", + &origin, &result.valid_until_ts + ); + return Ok(result); + } + + expires_soon_or_has_expired = true; + } } + let mut keys = result.unwrap_or_else(|| SigningKeys { + verify_keys: BTreeMap::new(), + old_verify_keys: BTreeMap::new(), + valid_until_ts: MilliSecondsSinceUnixEpoch::now(), + }); + + // We want to set this to the max, and then lower it whenever we see older keys + keys.valid_until_ts = MilliSecondsSinceUnixEpoch::from_system_time( + SystemTime::now() + Duration::from_secs(7 * 86400), + ) + .expect("Should be valid until year 500,000,000"); + debug!("Fetching signing keys for {} over federation", origin); - if let Some(server_key) = services() + if let Some(mut server_key) = services() .sending .send_federation_request(origin, get_server_keys::v2::Request::new()) .await .ok() .and_then(|resp| resp.server_key.deserialize().ok()) { + // Keys should only be valid for a maximum of seven days + server_key.valid_until_ts = server_key.valid_until_ts.min( + MilliSecondsSinceUnixEpoch::from_system_time( + SystemTime::now() + Duration::from_secs(7 * 86400), + ) + .expect("Should be valid until year 500,000,000"), + ); + services() .globals - .add_signing_key(origin, server_key.clone())?; + .add_signing_key_from_origin(origin, server_key.clone())?; - result.extend( + if keys.valid_until_ts > server_key.valid_until_ts { + keys.valid_until_ts = server_key.valid_until_ts; + } + + keys.verify_keys.extend( server_key .verify_keys .into_iter() - .map(|(k, v)| (k.to_string(), v.key)), + .map(|(id, key)| (id.to_string(), key)), ); - result.extend( + keys.old_verify_keys.extend( server_key .old_verify_keys .into_iter() - .map(|(k, v)| (k.to_string(), v.key)), + .map(|(id, key)| (id.to_string(), key)), ); - if contains_all_ids(&result) { - return Ok(result); + if contains_all_ids(&keys) { + return Ok(keys); } } - for server in services().globals.trusted_servers() { - debug!("Asking {} for {}'s signing key", server, origin); - if let Some(server_keys) = services() - .sending - .send_federation_request( - server, - get_remote_server_keys::v2::Request::new( - origin.to_owned(), - MilliSecondsSinceUnixEpoch::from_system_time( - SystemTime::now() - .checked_add(Duration::from_secs(3600)) - .expect("SystemTime to large"), - ) - .expect("time is valid"), - ), - ) - .await - .ok() - .map(|resp| { - resp.server_keys - .into_iter() - .filter_map(|e| e.deserialize().ok()) - .collect::>() - }) - { - trace!("Got signing keys: {:?}", server_keys); - for k in server_keys { - services().globals.add_signing_key(origin, k.clone())?; - result.extend( - k.verify_keys + if query_via_trusted_servers { + for server in services().globals.trusted_servers() { + debug!("Asking {} for {}'s signing key", server, origin); + if let Some(server_keys) = services() + .sending + .send_federation_request( + server, + get_remote_server_keys::v2::Request::new( + origin.to_owned(), + MilliSecondsSinceUnixEpoch::from_system_time( + SystemTime::now() + .checked_add(Duration::from_secs(3600)) + .expect("SystemTime to large"), + ) + .expect("time is valid"), + ), + ) + .await + .ok() + .map(|resp| { + resp.server_keys .into_iter() - .map(|(k, v)| (k.to_string(), v.key)), - ); - result.extend( - k.old_verify_keys - .into_iter() - .map(|(k, v)| (k.to_string(), v.key)), - ); - } + .filter_map(|e| e.deserialize().ok()) + .collect::>() + }) + { + trace!("Got signing keys: {:?}", server_keys); + for mut k in server_keys { + if k.valid_until_ts + // Half an hour should give plenty of time for the server to respond with keys that are still + // valid, given we requested keys which are valid at least an hour from now + < MilliSecondsSinceUnixEpoch::from_system_time( + SystemTime::now() + Duration::from_secs(30 * 60), + ) + .expect("Should be valid until year 500,000,000") + { + // Keys should only be valid for a maximum of seven days + k.valid_until_ts = k.valid_until_ts.min( + MilliSecondsSinceUnixEpoch::from_system_time( + SystemTime::now() + Duration::from_secs(7 * 86400), + ) + .expect("Should be valid until year 500,000,000"), + ); - if contains_all_ids(&result) { - return Ok(result); + if keys.valid_until_ts > k.valid_until_ts { + keys.valid_until_ts = k.valid_until_ts; + } + + services() + .globals + .add_signing_key_from_trusted_server(origin, k.clone())?; + keys.verify_keys.extend( + k.verify_keys + .into_iter() + .map(|(id, key)| (id.to_string(), key)), + ); + keys.old_verify_keys.extend( + k.old_verify_keys + .into_iter() + .map(|(id, key)| (id.to_string(), key)), + ); + } else { + warn!( + "Server {} gave us keys older than we requested, valid until: {:?}", + origin, k.valid_until_ts + ); + } + + if contains_all_ids(&keys) { + return Ok(keys); + } + } } } } + // We should return these keys if fresher keys were not found + if expires_soon_or_has_expired { + info!("Returning stale keys for {}", origin); + return Ok(keys); + } + drop(permit); back_off(signature_ids).await; diff --git a/src/service/rooms/timeline/mod.rs b/src/service/rooms/timeline/mod.rs index 5908a2ea..29d8339d 100644 --- a/src/service/rooms/timeline/mod.rs +++ b/src/service/rooms/timeline/mod.rs @@ -21,7 +21,6 @@ use ruma::{ GlobalAccountDataEventType, StateEventType, TimelineEventType, }, push::{Action, Ruleset, Tweak}, - serde::Base64, state_res::{self, Event, RoomVersion}, uint, user_id, CanonicalJsonObject, CanonicalJsonValue, EventId, OwnedEventId, OwnedRoomId, OwnedServerName, RoomId, RoomVersionId, ServerName, UserId, @@ -33,7 +32,10 @@ use tracing::{error, info, warn}; use crate::{ api::server_server, - service::pdu::{EventHash, PduBuilder}, + service::{ + globals::SigningKeys, + pdu::{EventHash, PduBuilder}, + }, services, utils, Error, PduEvent, Result, }; @@ -1214,7 +1216,7 @@ impl Service { &self, origin: &ServerName, pdu: Box, - pub_key_map: &RwLock>>, + pub_key_map: &RwLock>, ) -> Result<()> { let (event_id, value, room_id) = server_server::parse_incoming_pdu(&pdu)?;