From fbf4eac2dc79d647d4b91d113ae0c79426ae7ee8 Mon Sep 17 00:00:00 2001 From: timedout Date: Thu, 21 May 2026 20:03:18 +0100 Subject: [PATCH] fix: Ensure `event_id` is removed before policy-checking event --- .../rooms/event_handler/policy_server.rs | 17 ++++++++++++++--- src/service/rooms/timeline/create.rs | 9 +++++++-- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/service/rooms/event_handler/policy_server.rs b/src/service/rooms/event_handler/policy_server.rs index 0cec2d2bf..96f82a074 100644 --- a/src/service/rooms/event_handler/policy_server.rs +++ b/src/service/rooms/event_handler/policy_server.rs @@ -30,7 +30,16 @@ pub(super) fn verify_policy_signature( pdu_json: &CanonicalJsonObject, redaction_rules: &RedactionRules, ) -> bool { - trace!(data=?pdu_json, "Preparing to check policy server signature"); + #[cfg(debug_assertions)] + { + assert!( + !pdu_json.contains_key("event_id"), + "event_id should be removed from the JSON before verifying the policy server \ + signature" + ); + let pretty = serde_json::to_string(pdu_json).unwrap(); + trace!(data=%pretty, "Preparing to check policy server signature"); + }; let Some(canonical_json) = redact(pdu_json.clone(), redaction_rules, None) .ok() .and_then(|r| to_canonical_object(r).ok()) @@ -187,8 +196,10 @@ pub async fn policy_server_allows_event( if verify_policy_signature(&ps.via, ps_key, pdu_json, &room_version_rules.redaction) { Ok(()) } else { - Err!(BadServerResponse( - "Policy server signature was made with a different key to the one advertised" + Err(Error::Request( + ErrorKind::Unknown, + "Policy server signature was made with a different key to the one advertised".into(), + StatusCode::BAD_GATEWAY, )) } } diff --git a/src/service/rooms/timeline/create.rs b/src/service/rooms/timeline/create.rs index e8c555f3b..d86b9c442 100644 --- a/src/service/rooms/timeline/create.rs +++ b/src/service/rooms/timeline/create.rs @@ -302,8 +302,6 @@ pub async fn create_hash_and_sign_event( pdu.event_id = gen_event_id(&pdu_json, &room_version_rules)?; pdu_json.insert("event_id".into(), CanonicalJsonValue::String(pdu.event_id.clone().into())); // Verify that the *full* PDU isn't over 64KiB. - // Ruma only validates that it's under 64KiB before signing and hashing. - // Has to be cloned to prevent mutating pdu_json itself :( if !pdu_fits(&mut pdu_json.clone()) { // feckin huge PDU mate return Err!(Request(TooLarge("Message/PDU is too long (exceeds 65535 bytes)"))); @@ -315,6 +313,11 @@ pub async fn create_hash_and_sign_event( "Checking event in room {} with policy server", pdu.room_id.as_ref().map_or("None", |id| id.as_str()) ); + // We need to remove the event ID before getting a PS signature on the event. + // Note that we seemingly pointlessly add it above just to remove it here, but + // it's important to make sure the event ID isn't the field that makes the + // difference between an illegally-large event and one that is okay. + pdu_json.remove("event_id"); self.services .event_handler .policy_server_allows_event( @@ -325,6 +328,8 @@ pub async fn create_hash_and_sign_event( false, ) .await?; + pdu_json + .insert("event_id".into(), CanonicalJsonValue::String(pdu.event_id.clone().into())); } // Generate short event id