fix: Don't try to sign events that don't originate from us

src/api/server/make_join.rs

@@ -1,7 +1,9 @@
 use std::borrow::ToOwned;
 
 use axum::extract::State;
-use conduwuit::{Err, Error, Result, debug, debug_info, info, matrix::pdu::PduBuilder, warn};
+use conduwuit::{
+	Err, Error, Result, debug, debug_info, err, info, matrix::pdu::PduBuilder, utils, warn,
+};
 use conduwuit_service::Services;
 use futures::StreamExt;
 use ruma::{
@@ -40,6 +42,7 @@ pub(crate) async fn create_join_event_template_route(
 	{
 		info!(
 			origin = body.origin().as_str(),
+			room_id = %body.room_id,
 			"Refusing to serve make_join for room we aren't participating in"
 		);
 		return Err!(Request(NotFound("This server is not participating in that room.")));
@@ -133,10 +136,10 @@ pub(crate) async fn create_join_event_template_route(
 		}
 	}
 
-	let (_pdu, mut pdu_json) = services
+	let (pdu, _) = services
 		.rooms
 		.timeline
-		.create_hash_and_sign_event(
+		.create_event(
 			PduBuilder::state(body.user_id.to_string(), &RoomMemberEventContent {
 				join_authorized_via_users_server,
 				..RoomMemberEventContent::new(MembershipState::Join)
@@ -147,6 +150,8 @@ pub(crate) async fn create_join_event_template_route(
 		)
 		.await?;
 	drop(state_lock);
+	let mut pdu_json = utils::to_canonical_object(&pdu)
+		.expect("Barebones PDU should be convertible to canonical JSON");
 	pdu_json.remove("event_id");
 
 	Ok(prepare_join_event::v1::Response {

src/api/server/make_knock.rs

@@ -1,6 +1,6 @@
 use RoomVersionId::*;
 use axum::extract::State;
-use conduwuit::{Err, Error, Result, debug_warn, info, matrix::pdu::PduBuilder, warn};
+use conduwuit::{Err, Error, Result, debug_warn, info, matrix::pdu::PduBuilder, utils, warn};
 use ruma::{
 	RoomVersionId,
 	api::{client::error::ErrorKind, federation::knock::create_knock_event_template},
@@ -28,6 +28,7 @@ pub(crate) async fn create_knock_event_template_route(
 	{
 		info!(
 			origin = body.origin().as_str(),
+			room_id = %body.room_id,
 			"Refusing to serve make_knock for room we aren't participating in"
 		);
 		return Err!(Request(NotFound("This server is not participating in that room.")));
@@ -98,10 +99,10 @@ pub(crate) async fn create_knock_event_template_route(
 		}
 	}
 
-	let (_pdu, mut pdu_json) = services
+	let (pdu, _) = services
 		.rooms
 		.timeline
-		.create_hash_and_sign_event(
+		.create_event(
 			PduBuilder::state(
 				body.user_id.to_string(),
 				&RoomMemberEventContent::new(MembershipState::Knock),
@@ -113,9 +114,9 @@ pub(crate) async fn create_knock_event_template_route(
 		.await?;
 
 	drop(state_lock);
-
+	let mut pdu_json = utils::to_canonical_object(&pdu)
-	// room v3 and above removed the "event_id" field from remote PDU format
+		.expect("Barebones PDU should be convertible to canonical JSON");
-	super::maybe_strip_event_id(&mut pdu_json, &room_version_id)?;
+	pdu_json.remove("event_id");
 
 	Ok(create_knock_event_template::v1::Response {
 		room_version: room_version_id,

src/api/server/make_leave.rs

@@ -1,5 +1,5 @@
 use axum::extract::State;
-use conduwuit::{Err, Result, info, matrix::pdu::PduBuilder};
+use conduwuit::{Err, Result, info, matrix::pdu::PduBuilder, utils};
 use ruma::{
 	api::federation::membership::prepare_leave_event,
 	events::room::member::{MembershipState, RoomMemberEventContent},
@@ -49,10 +49,10 @@ pub(crate) async fn create_leave_event_template_route(
 	let room_version_id = services.rooms.state.get_room_version(&body.room_id).await?;
 	let state_lock = services.rooms.state.mutex.lock(&body.room_id).await;
 
-	let (_pdu, mut pdu_json) = services
+	let (pdu, _) = services
 		.rooms
 		.timeline
-		.create_hash_and_sign_event(
+		.create_event(
 			PduBuilder::state(
 				body.user_id.to_string(),
 				&RoomMemberEventContent::new(MembershipState::Leave),
@@ -64,9 +64,9 @@ pub(crate) async fn create_leave_event_template_route(
 		.await?;
 
 	drop(state_lock);
-
+	let mut pdu_json = utils::to_canonical_object(&pdu)
-	// room v3 and above removed the "event_id" field from remote PDU format
+		.expect("Barebones PDU should be convertible to canonical JSON");
-	maybe_strip_event_id(&mut pdu_json, &room_version_id)?;
+	pdu_json.remove("event_id");
 
 	Ok(prepare_leave_event::v1::Response {
 		room_version: Some(room_version_id),

src/service/rooms/timeline/create.rs

@@ -56,36 +56,32 @@ pub fn pdu_fits(owned_obj: &mut CanonicalJsonObject) -> bool {
 	}
 }
 
+/// Pulls the room version ID out of the given (create) event.
+fn room_version_from_event(
+	room_id: OwnedRoomId,
+	event_type: &TimelineEventType,
+	content: &RawValue,
+) -> Result<RoomVersionId> {
+	if event_type == &TimelineEventType::RoomCreate {
+		let content: RoomCreateEventContent = serde_json::from_str(content.get())?;
+		Ok(content.room_version)
+	} else {
+		Err(Error::InconsistentRoomState(
+			"non-create event for room of unknown version",
+			room_id,
+		))
+	}
+}
+
+// Creates an event, but does not hash or sign it.
 #[implement(super::Service)]
-pub async fn create_hash_and_sign_event(
+pub async fn create_event(
 	&self,
 	pdu_builder: PduBuilder,
 	sender: &UserId,
 	room_id: Option<&RoomId>,
-	_mutex_lock: &RoomMutexGuard, /* Take mutex guard to make sure users get the room
+	_mutex_lock: &RoomMutexGuard,
-	                               * state mutex */
+) -> Result<(PduEvent, RoomVersionId)> {
-) -> Result<(PduEvent, CanonicalJsonObject)> {
-	#[allow(clippy::boxed_local)]
-	fn from_evt(
-		room_id: OwnedRoomId,
-		event_type: &TimelineEventType,
-		content: &RawValue,
-	) -> Result<RoomVersionId> {
-		if event_type == &TimelineEventType::RoomCreate {
-			let content: RoomCreateEventContent = serde_json::from_str(content.get())?;
-			Ok(content.room_version)
-		} else {
-			Err(Error::InconsistentRoomState(
-				"non-create event for room of unknown version",
-				room_id,
-			))
-		}
-	}
-
-	if !self.services.globals.user_is_local(sender) {
-		return Err!(Request(Forbidden("Sender must be a local user")));
-	}
-
 	let PduBuilder {
 		event_type,
 		content,
@@ -108,12 +104,16 @@ pub async fn create_hash_and_sign_event(
 				.get_room_version(room_id)
 				.await
 				.or_else(|_| {
-					from_evt(room_id.to_owned(), &event_type.clone(), &content.clone())
+					room_version_from_event(
+						room_id.to_owned(),
+						&event_type.clone(),
+						&content.clone(),
+					)
 				})?
 		},
 		| None => {
 			trace!("No room ID, assuming room creation");
-			from_evt(
+			room_version_from_event(
 				RoomId::new(self.services.globals.server_name()),
 				&event_type.clone(),
 				&content.clone(),
@@ -186,7 +186,7 @@ pub async fn create_hash_and_sign_event(
 		}
 	}
 
-	let mut pdu = PduEvent {
+	let pdu = PduEvent {
 		event_id: ruma::event_id!("$thiswillbefilledinlater").into(),
 		room_id: room_id.map(ToOwned::to_owned),
 		sender: sender.to_owned(),
@@ -259,19 +259,29 @@ pub async fn create_hash_and_sign_event(
 		pdu.event_id,
 		pdu.room_id.as_ref().map_or("None", |id| id.as_str())
 	);
+	Ok((pdu, room_version_id))
+}
 
+#[implement(super::Service)]
+pub async fn create_hash_and_sign_event(
+	&self,
+	pdu_builder: PduBuilder,
+	sender: &UserId,
+	room_id: Option<&RoomId>,
+	mutex_lock: &RoomMutexGuard, /* Take mutex guard to make sure users get the room
+	                              * state mutex */
+) -> Result<(PduEvent, CanonicalJsonObject)> {
+	if !self.services.globals.user_is_local(sender) {
+		return Err!(Request(Forbidden("Sender must be a local user")));
+	}
+	let (mut pdu, room_version_id) = self
+		.create_event(pdu_builder, sender, room_id, mutex_lock)
+		.await?;
 	// Hash and sign
 	let mut pdu_json = utils::to_canonical_object(&pdu).map_err(|e| {
 		err!(Request(BadJson(warn!("Failed to convert PDU to canonical JSON: {e}"))))
 	})?;
-
+	pdu_json.remove("event_id");
-	// room v3 and above removed the "event_id" field from remote PDU format
-	match room_version_id {
-		| RoomVersionId::V1 | RoomVersionId::V2 => {},
-		| _ => {
-			pdu_json.remove("event_id");
-		},
-	}
 
 	trace!("hashing and signing event {}", pdu.event_id);
 	if let Err(e) = self