diff --git a/src/service/rooms/event_handler/fetch_auth.rs b/src/service/rooms/event_handler/fetch_auth.rs index 6e8bab3f3..8a905378f 100644 --- a/src/service/rooms/event_handler/fetch_auth.rs +++ b/src/service/rooms/event_handler/fetch_auth.rs @@ -127,6 +127,11 @@ async fn authorise_remote_auth_chain( continue; } + // IMPORTANT: We can't use the handy dandy `handle_outlier_pdu` function here + // because it may then try to fetch missing auth events, resulting in deep + // recursion. We will do the minimum required steps to validate the PDU here + // (steps 1 through 4). + let mut auth_events_by_key: HashMap<_, _> = HashMap::with_capacity(pdu.auth_events.len()); diff --git a/src/service/rooms/event_handler/fetch_prev.rs b/src/service/rooms/event_handler/fetch_prev.rs index 6e4f8559a..3f7e845f6 100644 --- a/src/service/rooms/event_handler/fetch_prev.rs +++ b/src/service/rooms/event_handler/fetch_prev.rs @@ -11,7 +11,7 @@ impl super::Service { /// Fetches any missing prev_events for this event and persists them before - /// returning. + /// returning. The caller is responsible for then handling the incoming PDU. pub(super) async fn fetch_prevs( &self, room_id: &RoomId, diff --git a/src/service/rooms/event_handler/handle_outlier_pdu.rs b/src/service/rooms/event_handler/handle_outlier_pdu.rs index 94f188dac..0104d4b6b 100644 --- a/src/service/rooms/event_handler/handle_outlier_pdu.rs +++ b/src/service/rooms/event_handler/handle_outlier_pdu.rs @@ -50,7 +50,7 @@ pub(super) async fn handle_outlier_pdu<'a, Pdu>( // 1. Check that the PDU follows the format for the room version // (in this case, just size check) - if !pdu_fits(&value) { + if !Self::pdu_format_check_1(&value) { warn!( "dropping incoming PDU {event_id} in room {room_id} from {origin} because it \ exceeds 65535 bytes or is otherwise too large." @@ -63,24 +63,9 @@ pub(super) async fn handle_outlier_pdu<'a, Pdu>( // 2. Check signatures, otherwise drop. // 3. Check content hash, redacting the event if it fails. - let mut incoming_pdu = match self - .services - .server_keys - .verify_event(&value, &room_version_rules) - .await - { - | Ok(ruma::signatures::Verified::All) => value, - | Ok(ruma::signatures::Verified::Signatures) => { - debug_info!("Content hash mismatch, redacting event and continuing"); - redact(value, &room_version_rules.redaction, None) - .map_err(|e| err!(Request(BadJson("Unable to redact {event_id}: {e}"))))? - }, - | Err(e) => { - return Err!(Request(Forbidden(debug_error!( - "Signature verification failed for {event_id}: {e}" - )))); - }, - }; + let mut incoming_pdu = self + .signature_hash_check_2_3(value, &room_version_rules) + .await?; // Now that we have checked the signature and hashes we can add the eventID and // convert to our PduEvent type diff --git a/src/service/rooms/event_handler/pdu_checks.rs b/src/service/rooms/event_handler/pdu_checks.rs index 3819ee309..da96c1295 100644 --- a/src/service/rooms/event_handler/pdu_checks.rs +++ b/src/service/rooms/event_handler/pdu_checks.rs @@ -1,16 +1,55 @@ use std::collections::HashMap; use conduwuit::{ - Err, Event, EventTypeExt, PduEvent, Result, debug, debug::DebugInspect, debug_error, err, - info, matrix::StateKey, state_res, trace, + Err, Event, EventTypeExt, PduEvent, Result, debug, debug::DebugInspect, debug_error, + debug_info, err, info, matrix::StateKey, state_res, trace, warn, }; use futures::future::ready; use ruma::{ - CanonicalJsonObject, OwnedEventId, ServerName, api::error::ErrorKind, events::StateEventType, - room_version_rules::RoomVersionRules, + CanonicalJsonObject, OwnedEventId, ServerName, api::error::ErrorKind, canonical_json::redact, + events::StateEventType, room_version_rules::RoomVersionRules, }; +use crate::rooms::timeline::pdu_fits; + impl super::Service { + /// Checks that the PDU conforms to the PDU format (check 1). This is + /// already mostly done during deserialisation, so this function just checks + /// that the PDU isn't a too large. + pub(super) fn pdu_format_check_1(pdu_json: &CanonicalJsonObject) -> bool { + // NOTE: if we do any more validation outside of deserialisation, it has to be + // done here. + pdu_fits(pdu_json) + } + + /// Checks that the PDU has a valid signature (check 2), and redacts it if + /// the content hash verification fails (check 3), returning the + /// potentially modified JSON. Returns an error if the PDU cannot be + /// redacted, or fails signature verification. + pub(super) async fn signature_hash_check_2_3( + &self, + pdu_json: CanonicalJsonObject, + room_version_rules: &RoomVersionRules, + ) -> Result { + match self + .services + .server_keys + .verify_event(&pdu_json, &room_version_rules) + .await + { + | Ok(ruma::signatures::Verified::All) => Ok(pdu_json), + | Ok(ruma::signatures::Verified::Signatures) => { + debug_info!("Content hash mismatch, redacting event and continuing"); + let redacted = redact(pdu_json, &room_version_rules.redaction, None) + .map_err(|e| err!(Request(BadJson("Unable to redact event: {e}"))))?; + Ok(redacted) + }, + | Err(e) => { + Err!(Request(Forbidden(debug_error!("Signature verification failed: {e}")))) + }, + } + } + /// Checks PDU check 4: Passes authorisation rules based on the event's auth /// events ([spec]). ///