Compare commits

...
Sign in to create a new pull request.

10 commits

Author SHA1 Message Date
nexy7574
6947b75f6e
Add suggested assertations to prevent potentially broken extremities 2025-06-19 13:46:52 +01:00
nexy7574
51185daca2
modify more log strings so they're more useful than not 2025-06-12 01:05:45 +01:00
nexy7574
6e438c8448
When in doubt, log all the things 2025-06-11 19:53:46 +01:00
nexy7574
e6aae8a994
log which room struggled to get mainline depth 2025-06-11 01:42:19 +01:00
nexy7574
cfff12190e
more logs 2025-06-11 01:27:25 +01:00
nexy7574
5ea42418f7
Unsafe, untested, and potentially overeager PDU sanity checks 2025-06-11 00:45:57 +01:00
nexy7574
3ebac17291
Fix room ID check 2025-06-10 23:00:09 +01:00
nexy7574
b44211c03e
Kick up a fuss when m.room.create is unfindable 2025-06-10 22:33:31 +01:00
nexy7574
24cd34ee98
Note about ruma#2064 in TODO 2025-06-07 00:55:03 +01:00
nexy7574
eda20ac4f5
fix an auth rule not applying correctly 2025-06-07 00:46:55 +01:00
6 changed files with 69 additions and 30 deletions

View file

@ -1243,6 +1243,7 @@ async fn join_room_by_id_helper_remote(
services.rooms.timeline.get_pdu(event_id).await.ok() services.rooms.timeline.get_pdu(event_id).await.ok()
}; };
debug!("running stateres check on send_join parsed PDU");
let auth_check = state_res::event_auth::auth_check( let auth_check = state_res::event_auth::auth_check(
&state_res::RoomVersion::new(&room_version_id)?, &state_res::RoomVersion::new(&room_version_id)?,
&parsed_join_pdu, &parsed_join_pdu,

View file

@ -13,6 +13,7 @@ use ruma::{
power_levels::RoomPowerLevelsEventContent, power_levels::RoomPowerLevelsEventContent,
third_party_invite::RoomThirdPartyInviteEventContent, third_party_invite::RoomThirdPartyInviteEventContent,
}, },
EventId,
int, int,
serde::{Base64, Raw}, serde::{Base64, Raw},
}; };
@ -21,7 +22,6 @@ use serde::{
de::{Error as _, IgnoredAny}, de::{Error as _, IgnoredAny},
}; };
use serde_json::{from_str as from_json_str, value::RawValue as RawJsonValue}; use serde_json::{from_str as from_json_str, value::RawValue as RawJsonValue};
use super::{ use super::{
Error, Event, Result, StateEventType, StateKey, TimelineEventType, Error, Event, Result, StateEventType, StateKey, TimelineEventType,
power_levels::{ power_levels::{
@ -217,8 +217,9 @@ where
} }
/* /*
// TODO: In the past this code caused problems federating with synapse, maybe this has been // TODO: In the past this code was commented as it caused problems with Synapse. This is no
// resolved already. Needs testing. // longer the case. This needs to be implemented.
// See also: https://github.com/ruma/ruma/pull/2064
// //
// 2. Reject if auth_events // 2. Reject if auth_events
// a. auth_events cannot have duplicate keys since it's a BTree // a. auth_events cannot have duplicate keys since it's a BTree
@ -250,11 +251,33 @@ where
let room_create_event = match room_create_event { let room_create_event = match room_create_event {
| None => { | None => {
warn!("no m.room.create event in auth chain"); error!(
create_event = room_create_event.as_ref().map(Event::event_id).unwrap_or(<&EventId>::try_from("$unknown").unwrap()).as_str(),
power_levels = power_levels_event.as_ref().map(Event::event_id).unwrap_or(<&EventId>::try_from("$unknown").unwrap()).as_str(),
member_event = sender_member_event.as_ref().map(Event::event_id).unwrap_or(<&EventId>::try_from("$unknown").unwrap()).as_str(),
"no m.room.create event found for {} ({})!",
incoming_event.event_id().as_str(),
incoming_event.room_id().as_str()
);
return Ok(false); return Ok(false);
}, },
| Some(e) => e, | Some(e) => e,
}; };
// just re-check 1.2 to work around a bug
let Some(room_id_server_name) = incoming_event.room_id().server_name() else {
warn!("room ID has no servername");
return Ok(false);
};
if room_id_server_name != room_create_event.sender().server_name() {
warn!(
"servername of room ID origin ({}) does not match servername of m.room.create \
sender ({})",
room_id_server_name,
room_create_event.sender().server_name()
);
return Ok(false);
}
// 3. If event does not have m.room.create in auth_events reject // 3. If event does not have m.room.create in auth_events reject
if !incoming_event if !incoming_event

View file

@ -609,7 +609,7 @@ where
let fetch_state = |ty: &StateEventType, key: &str| { let fetch_state = |ty: &StateEventType, key: &str| {
future::ready(auth_state.get(&ty.with_state_key(key))) future::ready(auth_state.get(&ty.with_state_key(key)))
}; };
debug!("running auth check on {:?}", event.event_id());
let auth_result = let auth_result =
auth_check(room_version, &event, current_third_party.as_ref(), fetch_state).await; auth_check(room_version, &event, current_third_party.as_ref(), fetch_state).await;
@ -726,8 +726,12 @@ where
Fut: Future<Output = Option<E>> + Send, Fut: Future<Output = Option<E>> + Send,
E: Event + Send + Sync, E: Event + Send + Sync,
{ {
let mut room_id = None;
while let Some(sort_ev) = event { while let Some(sort_ev) = event {
debug!(event_id = sort_ev.event_id().as_str(), "mainline"); trace!(event_id = sort_ev.event_id().as_str(), "mainline");
if room_id.is_none() {
room_id = Some(sort_ev.room_id().to_owned());
}
let id = sort_ev.event_id(); let id = sort_ev.event_id();
if let Some(depth) = mainline_map.get(id) { if let Some(depth) = mainline_map.get(id) {
@ -746,7 +750,7 @@ where
} }
} }
} }
// Did not find a power level event so we default to zero warn!("could not find a power event in the mainline map for {room_id:?}, defaulting to zero depth");
Ok(0) Ok(0)
} }

View file

@ -76,7 +76,7 @@ pub(super) async fn handle_outlier_pdu<'a>(
// 5. Reject "due to auth events" if can't get all the auth events or some of // 5. Reject "due to auth events" if can't get all the auth events or some of
// the auth events are also rejected "due to auth events" // the auth events are also rejected "due to auth events"
// NOTE: Step 5 is not applied anymore because it failed too often // NOTE: Step 5 is not applied anymore because it failed too often
debug!("Fetching auth events"); debug!("Fetching auth events for {}", incoming_pdu.event_id);
Box::pin(self.fetch_and_handle_outliers( Box::pin(self.fetch_and_handle_outliers(
origin, origin,
&incoming_pdu.auth_events, &incoming_pdu.auth_events,
@ -88,12 +88,12 @@ pub(super) async fn handle_outlier_pdu<'a>(
// 6. Reject "due to auth events" if the event doesn't pass auth based on the // 6. Reject "due to auth events" if the event doesn't pass auth based on the
// auth events // auth events
debug!("Checking based on auth events"); debug!("Checking {} based on auth events", incoming_pdu.event_id);
// Build map of auth events // Build map of auth events
let mut auth_events = HashMap::with_capacity(incoming_pdu.auth_events.len()); let mut auth_events = HashMap::with_capacity(incoming_pdu.auth_events.len());
for id in &incoming_pdu.auth_events { for id in &incoming_pdu.auth_events {
let Ok(auth_event) = self.services.timeline.get_pdu(id).await else { let Ok(auth_event) = self.services.timeline.get_pdu(id).await else {
warn!("Could not find auth event {id}"); warn!("Could not find auth event {id} for {}", incoming_pdu.event_id);
continue; continue;
}; };
@ -119,10 +119,7 @@ pub(super) async fn handle_outlier_pdu<'a>(
} }
// The original create event must be in the auth events // The original create event must be in the auth events
if !matches!( if !auth_events.contains_key(&(StateEventType::RoomCreate, String::new().into())) {
auth_events.get(&(StateEventType::RoomCreate, String::new().into())),
Some(_) | None
) {
return Err!(Request(InvalidParam("Incoming event refers to wrong create event."))); return Err!(Request(InvalidParam("Incoming event refers to wrong create event.")));
} }
@ -131,6 +128,7 @@ pub(super) async fn handle_outlier_pdu<'a>(
ready(auth_events.get(&key)) ready(auth_events.get(&key))
}; };
debug!("running auth check to handle outlier pdu {:?}", incoming_pdu.event_id);
let auth_check = state_res::event_auth::auth_check( let auth_check = state_res::event_auth::auth_check(
&to_room_version(&room_version_id), &to_room_version(&room_version_id),
&incoming_pdu, &incoming_pdu,

View file

@ -1,12 +1,6 @@
use std::{borrow::Borrow, collections::BTreeMap, iter::once, sync::Arc, time::Instant}; use std::{borrow::Borrow, collections::BTreeMap, iter::once, sync::Arc, time::Instant};
use conduwuit::{ use conduwuit::{Err, Result, debug, debug_info, err, implement, matrix::{EventTypeExt, PduEvent, StateKey, state_res}, trace, utils::stream::{BroadbandExt, ReadyExt}, warn, info};
Err, Result, debug, debug_info, err, implement,
matrix::{EventTypeExt, PduEvent, StateKey, state_res},
trace,
utils::stream::{BroadbandExt, ReadyExt},
warn,
};
use futures::{FutureExt, StreamExt, future::ready}; use futures::{FutureExt, StreamExt, future::ready};
use ruma::{CanonicalJsonValue, RoomId, ServerName, events::StateEventType}; use ruma::{CanonicalJsonValue, RoomId, ServerName, events::StateEventType};
@ -44,7 +38,7 @@ pub(super) async fn upgrade_outlier_to_timeline_pdu(
return Err!(Request(InvalidParam("Event has been soft failed"))); return Err!(Request(InvalidParam("Event has been soft failed")));
} }
debug!("Upgrading to timeline pdu"); debug!("Upgrading pdu {} from outlier to timeline pdu", incoming_pdu.event_id);
let timer = Instant::now(); let timer = Instant::now();
let room_version_id = get_room_version_id(create_event)?; let room_version_id = get_room_version_id(create_event)?;
@ -52,7 +46,7 @@ pub(super) async fn upgrade_outlier_to_timeline_pdu(
// backwards extremities doing all the checks in this list starting at 1. // backwards extremities doing all the checks in this list starting at 1.
// These are not timeline events. // These are not timeline events.
debug!("Resolving state at event"); debug!("Resolving state at event {}", incoming_pdu.event_id);
let mut state_at_incoming_event = if incoming_pdu.prev_events.len() == 1 { let mut state_at_incoming_event = if incoming_pdu.prev_events.len() == 1 {
self.state_at_incoming_degree_one(&incoming_pdu).await? self.state_at_incoming_degree_one(&incoming_pdu).await?
} else { } else {
@ -70,7 +64,7 @@ pub(super) async fn upgrade_outlier_to_timeline_pdu(
state_at_incoming_event.expect("we always set this to some above"); state_at_incoming_event.expect("we always set this to some above");
let room_version = to_room_version(&room_version_id); let room_version = to_room_version(&room_version_id);
debug!("Performing auth check"); debug!("Performing auth check to upgrade {}", incoming_pdu.event_id);
// 11. Check the auth of the event passes based on the state of the event // 11. Check the auth of the event passes based on the state of the event
let state_fetch_state = &state_at_incoming_event; let state_fetch_state = &state_at_incoming_event;
let state_fetch = |k: StateEventType, s: StateKey| async move { let state_fetch = |k: StateEventType, s: StateKey| async move {
@ -80,6 +74,7 @@ pub(super) async fn upgrade_outlier_to_timeline_pdu(
self.services.timeline.get_pdu(event_id).await.ok() self.services.timeline.get_pdu(event_id).await.ok()
}; };
debug!("running auth check on {}", incoming_pdu.event_id);
let auth_check = state_res::event_auth::auth_check( let auth_check = state_res::event_auth::auth_check(
&room_version, &room_version,
&incoming_pdu, &incoming_pdu,
@ -93,7 +88,7 @@ pub(super) async fn upgrade_outlier_to_timeline_pdu(
return Err!(Request(Forbidden("Event has failed auth check with state at the event."))); return Err!(Request(Forbidden("Event has failed auth check with state at the event.")));
} }
debug!("Gathering auth events"); debug!("Gathering auth events for {}", incoming_pdu.event_id);
let auth_events = self let auth_events = self
.services .services
.state .state
@ -111,6 +106,7 @@ pub(super) async fn upgrade_outlier_to_timeline_pdu(
ready(auth_events.get(&key).cloned()) ready(auth_events.get(&key).cloned())
}; };
debug!("running auth check on {} with claimed state auth", incoming_pdu.event_id);
let auth_check = state_res::event_auth::auth_check( let auth_check = state_res::event_auth::auth_check(
&room_version, &room_version,
&incoming_pdu, &incoming_pdu,
@ -121,7 +117,7 @@ pub(super) async fn upgrade_outlier_to_timeline_pdu(
.map_err(|e| err!(Request(Forbidden("Auth check failed: {e:?}"))))?; .map_err(|e| err!(Request(Forbidden("Auth check failed: {e:?}"))))?;
// Soft fail check before doing state res // Soft fail check before doing state res
debug!("Performing soft-fail check"); debug!("Performing soft-fail check on {}", incoming_pdu.event_id);
let soft_fail = match (auth_check, incoming_pdu.redacts_id(&room_version_id)) { let soft_fail = match (auth_check, incoming_pdu.redacts_id(&room_version_id)) {
| (false, _) => true, | (false, _) => true,
| (true, None) => false, | (true, None) => false,
@ -218,7 +214,8 @@ pub(super) async fn upgrade_outlier_to_timeline_pdu(
// 14. Check if the event passes auth based on the "current state" of the room, // 14. Check if the event passes auth based on the "current state" of the room,
// if not soft fail it // if not soft fail it
if soft_fail { if soft_fail {
debug!("Soft failing event"); info!("Soft failing event {}", incoming_pdu.event_id);
assert!(extremities.is_empty(), "soft_fail extremities empty");
let extremities = extremities.iter().map(Borrow::borrow); let extremities = extremities.iter().map(Borrow::borrow);
self.services self.services

View file

@ -698,6 +698,20 @@ impl Service {
.await .await
.saturating_add(uint!(1)); .saturating_add(uint!(1));
if state_key.is_none() {
if prev_events.is_empty() {
warn!("Timeline event had zero prev_events, something broke.");
return Err!(Request(Unknown("Timeline event had zero prev_events.")));
}
if depth.le(&uint!(2)) {
warn!(
"Had unsafe depth of {depth} in {room_id} when creating non-state event. \
Bad!"
);
return Err!(Request(Unknown("Unsafe depth for non-state event.")));
}
};
let mut unsigned = unsigned.unwrap_or_default(); let mut unsigned = unsigned.unwrap_or_default();
if let Some(state_key) = &state_key { if let Some(state_key) = &state_key {
@ -757,6 +771,7 @@ impl Service {
ready(auth_events.get(&key)) ready(auth_events.get(&key))
}; };
debug!("running auth check on new {} event by {} in {}", pdu.kind, pdu.sender, pdu.room_id);
let auth_check = state_res::auth_check( let auth_check = state_res::auth_check(
&room_version, &room_version,
&pdu, &pdu,
@ -961,8 +976,9 @@ impl Service {
state_lock: &'a RoomMutexGuard, state_lock: &'a RoomMutexGuard,
) -> Result<Option<RawPduId>> ) -> Result<Option<RawPduId>>
where where
Leaves: Iterator<Item = &'a EventId> + Send + 'a, Leaves: Iterator<Item = &'a EventId> + Send + Clone + 'a,
{ {
assert!(new_room_leaves.clone().count() > 0, "extremities are empty");
// We append to state before appending the pdu, so we don't have a moment in // We append to state before appending the pdu, so we don't have a moment in
// time with the pdu without it's state. This is okay because append_pdu can't // time with the pdu without it's state. This is okay because append_pdu can't
// fail. // fail.
@ -1142,7 +1158,7 @@ impl Service {
.boxed(); .boxed();
while let Some(ref backfill_server) = servers.next().await { while let Some(ref backfill_server) = servers.next().await {
info!("Asking {backfill_server} for backfill"); info!("Asking {backfill_server} for backfill in {:?}", room_id.to_owned());
let response = self let response = self
.services .services
.sending .sending
@ -1170,7 +1186,7 @@ impl Service {
} }
} }
info!("No servers could backfill, but backfill was needed in room {room_id}"); warn!("No servers could backfill, but backfill was needed in room {room_id}");
Ok(()) Ok(())
} }