factor incoming remote membership validation into a function

The whole dance with Extract on the join path previously was because I
didn't realize that we had the deserialized CanonicalJsonObject there
already.
This commit is contained in:
Olivia Lee 2025-12-26 17:17:36 -08:00 committed by Charles Hall
parent e6f6fb0861
commit f38f2d4fab

View file

@ -61,7 +61,6 @@ use ruma::{
OwnedServerSigningKeyId, OwnedSigningKeyId, OwnedUserId, RoomId, OwnedServerSigningKeyId, OwnedSigningKeyId, OwnedUserId, RoomId,
ServerName, Signatures, ServerName, Signatures,
}; };
use serde::Deserialize;
use serde_json::value::{to_raw_value, RawValue as RawJsonValue}; use serde_json::value::{to_raw_value, RawValue as RawJsonValue};
use tokio::sync::RwLock; use tokio::sync::RwLock;
use tracing::{debug, error, field, trace, trace_span, warn}; use tracing::{debug, error, field, trace, trace_span, warn};
@ -1583,13 +1582,6 @@ async fn create_join_event(
room_id: &RoomId, room_id: &RoomId,
pdu: &RawJsonValue, pdu: &RawJsonValue,
) -> Result<create_join_event::v2::RoomState> { ) -> Result<create_join_event::v2::RoomState> {
#[derive(Deserialize)]
struct ExtractPdu<'a> {
#[serde(rename = "type")]
event_type: &'a str,
content: RoomMemberEventContent,
}
if !services().rooms.metadata.exists(room_id)? { if !services().rooms.metadata.exists(room_id)? {
return Err(Error::BadRequest( return Err(Error::BadRequest(
ErrorKind::NotFound, ErrorKind::NotFound,
@ -1599,28 +1591,6 @@ async fn create_join_event(
services().rooms.event_handler.acl_check(sender_servername, room_id)?; services().rooms.event_handler.acl_check(sender_servername, room_id)?;
let extract: ExtractPdu<'_> =
serde_json::from_str(pdu.get()).map_err(|_| {
Error::BadRequest(
ErrorKind::InvalidParam,
"Event does not match expected schema",
)
})?;
if extract.event_type != RoomMemberEventContent::TYPE {
return Err(Error::BadRequest(
ErrorKind::InvalidParam,
"Event is not a membership event",
));
}
if extract.content.membership != MembershipState::Join {
return Err(Error::BadRequest(
ErrorKind::InvalidParam,
"Not allowed to send a non-join membership event to send_join \
endpoint.",
));
}
// TODO: Grapevine does not implement restricted join rules yet, we always // TODO: Grapevine does not implement restricted join rules yet, we always
// reject // reject
let join_rules_event = services().rooms.state_accessor.room_state_get( let join_rules_event = services().rooms.state_accessor.room_state_get(
@ -1681,6 +1651,8 @@ async fn create_join_event(
)); ));
}; };
validate_remote_member_event(&MembershipState::Join, &value)?;
let origin: OwnedServerName = serde_json::from_value( let origin: OwnedServerName = serde_json::from_value(
serde_json::to_value(value.get("origin").ok_or(Error::BadRequest( serde_json::to_value(value.get("origin").ok_or(Error::BadRequest(
ErrorKind::InvalidParam, ErrorKind::InvalidParam,
@ -1797,6 +1769,58 @@ pub(crate) async fn create_join_event_v2_route(
})) }))
} }
/// Validates that an event passed by a remote server to `/make_*` or `/invite`
/// actually is a membership event with the expected fields.
///
/// Without checking this, the remote server could use the remote membership
/// endpoints to trick our server into signing arbitrary malicious events.
fn validate_remote_member_event(
membership: &MembershipState,
event: &CanonicalJsonObject,
) -> Result<()> {
let event_type = event.get("type").ok_or_else(|| {
Error::BadRequest(
ErrorKind::InvalidParam,
"Event missing type property",
)
})?;
if event_type.as_str() != Some(RoomMemberEventContent::TYPE) {
return Err(Error::BadRequest(
ErrorKind::InvalidParam,
"Event is not a membership event",
));
}
let content: RoomMemberEventContent = serde_json::from_value(
event
.get("content")
.ok_or_else(|| {
Error::BadRequest(
ErrorKind::InvalidParam,
"Event missing content property",
)
})?
.clone()
.into(),
)
.map_err(|_| {
Error::BadRequest(
ErrorKind::InvalidParam,
"Event content is empty or invalid",
)
})?;
if &content.membership != membership {
return Err(Error::BadRequest(
ErrorKind::InvalidParam,
"Not allowed to send a non-invite membership event to invite \
endpoint.",
));
}
Ok(())
}
/// # `PUT /_matrix/federation/v2/invite/{roomId}/{eventId}` /// # `PUT /_matrix/federation/v2/invite/{roomId}/{eventId}`
/// ///
/// Invites a remote user to a room. /// Invites a remote user to a room.
@ -1837,45 +1861,7 @@ pub(crate) async fn create_invite_route(
) )
})?; })?;
let event_type = signed_event.get("type").ok_or_else(|| { validate_remote_member_event(&MembershipState::Invite, &signed_event)?;
Error::BadRequest(
ErrorKind::InvalidParam,
"Event missing type property",
)
})?;
if event_type.as_str() != Some(RoomMemberEventContent::TYPE) {
return Err(Error::BadRequest(
ErrorKind::InvalidParam,
"Event is not a membership event",
));
}
let content: RoomMemberEventContent = serde_json::from_value(
signed_event
.get("content")
.ok_or_else(|| {
Error::BadRequest(
ErrorKind::InvalidParam,
"Event missing content property",
)
})?
.clone()
.into(),
)
.map_err(|_| {
Error::BadRequest(
ErrorKind::InvalidParam,
"Event content is empty or invalid",
)
})?;
if content.membership != MembershipState::Invite {
return Err(Error::BadRequest(
ErrorKind::InvalidParam,
"Not allowed to send a non-invite membership event to invite \
endpoint.",
));
}
ruma::signatures::hash_and_sign_event( ruma::signatures::hash_and_sign_event(
services().globals.server_name().as_str(), services().globals.server_name().as_str(),