From a5d70f73566044369231a43ae2f2be3bc5e9d715 Mon Sep 17 00:00:00 2001 From: strawberry Date: Thu, 2 Jan 2025 18:16:00 -0500 Subject: [PATCH] add some m.room.member checks on putting direct state events Signed-off-by: strawberry --- src/api/client/state.rs | 118 +++++++++++------- src/api/server/send_join.rs | 11 +- .../event_handler/handle_incoming_pdu.rs | 16 +-- .../rooms/event_handler/handle_outlier_pdu.rs | 2 +- src/service/rooms/timeline/mod.rs | 23 ++++ src/service/sending/sender.rs | 7 +- 6 files changed, 117 insertions(+), 60 deletions(-) diff --git a/src/api/client/state.rs b/src/api/client/state.rs index e5a7dd28..d00ee5e5 100644 --- a/src/api/client/state.rs +++ b/src/api/client/state.rs @@ -1,15 +1,13 @@ use axum::extract::State; -use conduwuit::{err, pdu::PduBuilder, utils::BoolExt, Err, Error, PduEvent, Result}; +use conduwuit::{err, pdu::PduBuilder, utils::BoolExt, Err, PduEvent, Result}; use ruma::{ - api::client::{ - error::ErrorKind, - state::{get_state_events, get_state_events_for_key, send_state_event}, - }, + api::client::state::{get_state_events, get_state_events_for_key, send_state_event}, events::{ room::{ canonical_alias::RoomCanonicalAliasEventContent, history_visibility::{HistoryVisibility, RoomHistoryVisibilityEventContent}, join_rules::{JoinRule, RoomJoinRulesEventContent}, + member::{MembershipState, RoomMemberEventContent}, }, AnyStateEventContent, StateEventType, }, @@ -23,11 +21,6 @@ use crate::{Ruma, RumaResponse}; /// # `PUT /_matrix/client/*/rooms/{roomId}/state/{eventType}/{stateKey}` /// /// Sends a state event into the room. -/// -/// - The only requirement for the content is that it has to be valid json -/// - Tries to send the event into the room, auth rules will determine if it is -/// allowed -/// - If event is new `canonical_alias`: Rejects if alias is incorrect pub(crate) async fn send_state_event_for_key_route( State(services): State, body: Ruma, @@ -41,7 +34,7 @@ pub(crate) async fn send_state_event_for_key_route( &body.room_id, &body.event_type, &body.body.body, - body.state_key.clone(), + &body.state_key, if body.appservice_info.is_some() { body.timestamp } else { @@ -55,11 +48,6 @@ pub(crate) async fn send_state_event_for_key_route( /// # `PUT /_matrix/client/*/rooms/{roomId}/state/{eventType}` /// /// Sends a state event into the room. -/// -/// - The only requirement for the content is that it has to be valid json -/// - Tries to send the event into the room, auth rules will determine if it is -/// allowed -/// - If event is new `canonical_alias`: Rejects if alias is incorrect pub(crate) async fn send_state_event_for_empty_key_route( State(services): State, body: Ruma, @@ -172,10 +160,10 @@ async fn send_state_event_for_key_helper( room_id: &RoomId, event_type: &StateEventType, json: &Raw, - state_key: String, + state_key: &str, timestamp: Option, ) -> Result { - allowed_to_send_state_event(services, room_id, event_type, json).await?; + allowed_to_send_state_event(services, room_id, event_type, state_key, json).await?; let state_lock = services.rooms.state.mutex.lock(room_id).await; let event_id = services .rooms @@ -184,7 +172,7 @@ async fn send_state_event_for_key_helper( PduBuilder { event_type: event_type.to_string().into(), content: serde_json::from_str(json.json().get())?, - state_key: Some(state_key), + state_key: Some(String::from(state_key)), timestamp, ..Default::default() }, @@ -201,6 +189,7 @@ async fn allowed_to_send_state_event( services: &Services, room_id: &RoomId, event_type: &StateEventType, + state_key: &str, json: &Raw, ) -> Result { match event_type { @@ -212,10 +201,7 @@ async fn allowed_to_send_state_event( // Forbid m.room.encryption if encryption is disabled | StateEventType::RoomEncryption => if !services.globals.allow_encryption() { - return Err(Error::BadRequest( - ErrorKind::forbidden(), - "Encryption has been disabled", - )); + return Err!(Request(Forbidden("Encryption is disabled on this homeserver."))); }, // admin room is a sensitive room, it should not ever be made public | StateEventType::RoomJoinRules => { @@ -225,10 +211,9 @@ async fn allowed_to_send_state_event( serde_json::from_str::(json.json().get()) { if join_rule.join_rule == JoinRule::Public { - return Err(Error::BadRequest( - ErrorKind::forbidden(), - "Admin room is not allowed to be public.", - )); + return Err!(Request(Forbidden( + "Admin room is a sensitive room, it cannot be made public" + ))); } } } @@ -236,26 +221,22 @@ async fn allowed_to_send_state_event( }, // admin room is a sensitive room, it should not ever be made world readable | StateEventType::RoomHistoryVisibility => { - if let Ok(admin_room_id) = services.admin.get_admin_room().await { - if admin_room_id == room_id { - if let Ok(visibility_content) = serde_json::from_str::< - RoomHistoryVisibilityEventContent, - >(json.json().get()) - { - if visibility_content.history_visibility + if let Ok(visibility_content) = + serde_json::from_str::(json.json().get()) + { + if let Ok(admin_room_id) = services.admin.get_admin_room().await { + if admin_room_id == room_id + && visibility_content.history_visibility == HistoryVisibility::WorldReadable - { - return Err(Error::BadRequest( - ErrorKind::forbidden(), - "Admin room is not allowed to be made world readable (public \ - room history).", - )); - } + { + return Err!(Request(Forbidden( + "Admin room is a sensitive room, it cannot be made world readable \ + (public room history)." + ))); } } } }, - // TODO: allow alias if it previously existed | StateEventType::RoomCanonicalAlias => { if let Ok(canonical_alias) = serde_json::from_str::(json.json().get()) @@ -289,6 +270,59 @@ async fn allowed_to_send_state_event( } } }, + | StateEventType::RoomMember => { + let Ok(membership_content) = + serde_json::from_str::(json.json().get()) + else { + return Err!(Request(BadJson( + "Membership content must have a valid JSON body with at least a valid \ + membership state." + ))); + }; + + let Ok(state_key) = UserId::parse(state_key) else { + return Err!(Request(BadJson( + "Membership event has invalid or non-existent state key" + ))); + }; + + if let Some(authorising_user) = membership_content.join_authorized_via_users_server { + if membership_content.membership != MembershipState::Join { + return Err!(Request(BadJson( + "join_authorised_via_users_server is only for member joins" + ))); + } + + if services + .rooms + .state_cache + .is_joined(state_key, room_id) + .await + { + return Err!(Request(InvalidParam( + "{state_key} is already joined, an authorising user is not required." + ))); + } + + if !services.globals.user_is_local(&authorising_user) { + return Err!(Request(InvalidParam( + "Authorising user {authorising_user} does not belong to this homeserver" + ))); + } + + if !services + .rooms + .state_cache + .is_joined(&authorising_user, room_id) + .await + { + return Err!(Request(InvalidParam( + "Authorising user {authorising_user} is not in the room, they cannot \ + authorise the join." + ))); + } + } + }, | _ => (), } diff --git a/src/api/server/send_join.rs b/src/api/server/send_join.rs index fe0277d1..6cbe5143 100644 --- a/src/api/server/send_join.rs +++ b/src/api/server/send_join.rs @@ -61,12 +61,11 @@ async fn create_join_event( }; let event_room_id: OwnedRoomId = serde_json::from_value( - serde_json::to_value( - value - .get("room_id") - .ok_or_else(|| err!(Request(BadJson("Event missing room_id property."))))?, - ) - .expect("CanonicalJson is valid json value"), + value + .get("room_id") + .ok_or_else(|| err!(Request(BadJson("Event missing room_id property."))))? + .clone() + .into(), ) .map_err(|e| err!(Request(BadJson(warn!("room_id field is not a valid room ID: {e}")))))?; diff --git a/src/service/rooms/event_handler/handle_incoming_pdu.rs b/src/service/rooms/event_handler/handle_incoming_pdu.rs index 0e0409b4..4c2fb2f7 100644 --- a/src/service/rooms/event_handler/handle_incoming_pdu.rs +++ b/src/service/rooms/event_handler/handle_incoming_pdu.rs @@ -4,12 +4,9 @@ use std::{ time::Instant, }; -use conduwuit::{debug, err, implement, warn, Error, Result}; +use conduwuit::{debug, err, implement, warn, Err, Result}; use futures::{FutureExt, TryFutureExt}; -use ruma::{ - api::client::error::ErrorKind, events::StateEventType, CanonicalJsonValue, EventId, RoomId, - ServerName, UserId, -}; +use ruma::{events::StateEventType, CanonicalJsonValue, EventId, RoomId, ServerName, UserId}; use super::{check_room_id, get_room_version_id}; use crate::rooms::timeline::RawPduId; @@ -58,15 +55,14 @@ pub async fn handle_incoming_pdu<'a>( // 1.1 Check the server is in the room if !self.services.metadata.exists(room_id).await { - return Err(Error::BadRequest(ErrorKind::NotFound, "Room is unknown to this server")); + return Err!(Request(NotFound("Room is unknown to this server"))); } // 1.2 Check if the room is disabled if self.services.metadata.is_disabled(room_id).await { - return Err(Error::BadRequest( - ErrorKind::forbidden(), - "Federation of this room is currently disabled on this server.", - )); + return Err!(Request(Forbidden( + "Federation of this room is currently disabled on this server." + ))); } // 1.3.1 Check room ACL on origin field/server diff --git a/src/service/rooms/event_handler/handle_outlier_pdu.rs b/src/service/rooms/event_handler/handle_outlier_pdu.rs index c3278329..3ad73295 100644 --- a/src/service/rooms/event_handler/handle_outlier_pdu.rs +++ b/src/service/rooms/event_handler/handle_outlier_pdu.rs @@ -68,7 +68,7 @@ pub(super) async fn handle_outlier_pdu<'a>( let incoming_pdu = serde_json::from_value::( serde_json::to_value(&val).expect("CanonicalJsonObj is a valid JsonValue"), ) - .map_err(|_| Error::bad_database("Event is not a valid PDU."))?; + .map_err(|e| err!(Request(BadJson(debug_warn!("Event is not a valid PDU: {e}")))))?; check_room_id(room_id, &incoming_pdu)?; diff --git a/src/service/rooms/timeline/mod.rs b/src/service/rooms/timeline/mod.rs index 81df7b35..2a272c38 100644 --- a/src/service/rooms/timeline/mod.rs +++ b/src/service/rooms/timeline/mod.rs @@ -901,6 +901,29 @@ impl Service { } }; + if pdu.kind == TimelineEventType::RoomMember { + let content: RoomMemberEventContent = pdu.get_content()?; + + if content.join_authorized_via_users_server.is_some() + && content.membership != MembershipState::Join + { + return Err!(Request(BadJson( + "join_authorised_via_users_server is only for member joins" + ))); + } + + if content + .join_authorized_via_users_server + .as_ref() + .is_some_and(|authorising_user| { + !self.services.globals.user_is_local(authorising_user) + }) { + return Err!(Request(InvalidParam( + "Authorising user does not belong to this homeserver" + ))); + } + } + // 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. diff --git a/src/service/sending/sender.rs b/src/service/sending/sender.rs index 482c31cf..a9abada4 100644 --- a/src/service/sending/sender.rs +++ b/src/service/sending/sender.rs @@ -739,7 +739,12 @@ impl Service { )); }; - let mut pdus = Vec::new(); + let mut pdus = Vec::with_capacity( + events + .iter() + .filter(|event| matches!(event, SendingEvent::Pdu(_))) + .count(), + ); for event in &events { match event { | SendingEvent::Pdu(pdu_id) => {