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()
};
debug!("running stateres check on send_join parsed PDU");
let auth_check = state_res::event_auth::auth_check(
&state_res::RoomVersion::new(&room_version_id)?,
&parsed_join_pdu,

View file

@ -13,6 +13,7 @@ use ruma::{
power_levels::RoomPowerLevelsEventContent,
third_party_invite::RoomThirdPartyInviteEventContent,
},
EventId,
int,
serde::{Base64, Raw},
};
@ -21,7 +22,6 @@ use serde::{
de::{Error as _, IgnoredAny},
};
use serde_json::{from_str as from_json_str, value::RawValue as RawJsonValue};
use super::{
Error, Event, Result, StateEventType, StateKey, TimelineEventType,
power_levels::{
@ -217,8 +217,9 @@ where
}
/*
// TODO: In the past this code caused problems federating with synapse, maybe this has been
// resolved already. Needs testing.
// TODO: In the past this code was commented as it caused problems with Synapse. This is no
// longer the case. This needs to be implemented.
// See also: https://github.com/ruma/ruma/pull/2064
//
// 2. Reject if auth_events
// 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 {
| 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);
},
| 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
if !incoming_event

View file

@ -609,7 +609,7 @@ where
let fetch_state = |ty: &StateEventType, key: &str| {
future::ready(auth_state.get(&ty.with_state_key(key)))
};
debug!("running auth check on {:?}", event.event_id());
let auth_result =
auth_check(room_version, &event, current_third_party.as_ref(), fetch_state).await;
@ -726,8 +726,12 @@ where
Fut: Future<Output = Option<E>> + Send,
E: Event + Send + Sync,
{
let mut room_id = None;
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();
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)
}

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
// the auth events are also rejected "due to auth events"
// 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(
origin,
&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
// auth events
debug!("Checking based on auth events");
debug!("Checking {} based on auth events", incoming_pdu.event_id);
// Build map of auth events
let mut auth_events = HashMap::with_capacity(incoming_pdu.auth_events.len());
for id in &incoming_pdu.auth_events {
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;
};
@ -119,10 +119,7 @@ pub(super) async fn handle_outlier_pdu<'a>(
}
// The original create event must be in the auth events
if !matches!(
auth_events.get(&(StateEventType::RoomCreate, String::new().into())),
Some(_) | None
) {
if !auth_events.contains_key(&(StateEventType::RoomCreate, String::new().into())) {
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))
};
debug!("running auth check to handle outlier pdu {:?}", incoming_pdu.event_id);
let auth_check = state_res::event_auth::auth_check(
&to_room_version(&room_version_id),
&incoming_pdu,

View file

@ -1,12 +1,6 @@
use std::{borrow::Borrow, collections::BTreeMap, iter::once, sync::Arc, time::Instant};
use conduwuit::{
Err, Result, debug, debug_info, err, implement,
matrix::{EventTypeExt, PduEvent, StateKey, state_res},
trace,
utils::stream::{BroadbandExt, ReadyExt},
warn,
};
use conduwuit::{Err, Result, debug, debug_info, err, implement, matrix::{EventTypeExt, PduEvent, StateKey, state_res}, trace, utils::stream::{BroadbandExt, ReadyExt}, warn, info};
use futures::{FutureExt, StreamExt, future::ready};
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")));
}
debug!("Upgrading to timeline pdu");
debug!("Upgrading pdu {} from outlier to timeline pdu", incoming_pdu.event_id);
let timer = Instant::now();
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.
// 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 {
self.state_at_incoming_degree_one(&incoming_pdu).await?
} 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");
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
let state_fetch_state = &state_at_incoming_event;
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()
};
debug!("running auth check on {}", incoming_pdu.event_id);
let auth_check = state_res::event_auth::auth_check(
&room_version,
&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.")));
}
debug!("Gathering auth events");
debug!("Gathering auth events for {}", incoming_pdu.event_id);
let auth_events = self
.services
.state
@ -111,6 +106,7 @@ pub(super) async fn upgrade_outlier_to_timeline_pdu(
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(
&room_version,
&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:?}"))))?;
// 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)) {
| (false, _) => true,
| (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,
// if not soft fail it
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);
self.services

View file

@ -698,6 +698,20 @@ impl Service {
.await
.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();
if let Some(state_key) = &state_key {
@ -757,6 +771,7 @@ impl Service {
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(
&room_version,
&pdu,
@ -961,8 +976,9 @@ impl Service {
state_lock: &'a RoomMutexGuard,
) -> Result<Option<RawPduId>>
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
// time with the pdu without it's state. This is okay because append_pdu can't
// fail.
@ -1142,7 +1158,7 @@ impl Service {
.boxed();
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
.services
.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(())
}