feat: Strip extraneous signatures to avoid leaking our own notaries

src/api/router.rs

@@ -200,6 +200,7 @@ pub fn build(router: Router<State>, state: State) -> Router<State> {
 		router = router
 			.ruma_route(&server::get_server_version_route)
 			.route("/_matrix/key/v2/server", get(server::get_server_keys_route))
+			.ruma_route(&server::get_remote_server_keys_batch_route)
 			.ruma_route(&server::get_remote_server_keys_route)
 			.ruma_route(&server::get_public_rooms_route)
 			.ruma_route(&server::get_public_rooms_filtered_route)

src/api/server/key.rs

@@ -11,7 +11,8 @@
 	api::{
 		OutgoingResponseExt,
 		federation::discovery::{
-			OldVerifyKey, ServerSigningKeys, get_remote_server_keys_batch, get_server_keys,
+			OldVerifyKey, ServerSigningKeys, get_remote_server_keys,
+			get_remote_server_keys_batch, get_server_keys,
 		},
 	},
 	assign,
@@ -96,7 +97,9 @@ fn valid_until_ts() -> MilliSecondsSinceUnixEpoch {
 	MilliSecondsSinceUnixEpoch::from_system_time(timepoint).expect("UInt should not overflow")
 }
 
-pub(crate) async fn get_remote_server_keys_route(
+const MAX_KEYS_PER_QUERY: usize = 16 * 1024;
+
+pub(crate) async fn get_remote_server_keys_batch_route(
 	State(services): State<crate::State>,
 	body: Ruma<get_remote_server_keys_batch::v2::Request>,
 ) -> Result<get_remote_server_keys_batch::v2::Response> {
@@ -105,8 +108,10 @@ pub(crate) async fn get_remote_server_keys_route(
 		.iter()
 		.fold(0_usize, |acc, _| acc.saturating_add(1));
 
-	if total_queried_keys > 16384 {
-		return Err!(Request(Forbidden("Too many keys requested")));
+	if total_queried_keys > MAX_KEYS_PER_QUERY {
+		return Err!(Request(Forbidden(
+			"Too many keys requested ({total_queried_keys} > {MAX_KEYS_PER_QUERY})"
+		)));
 	} else if total_queried_keys == 0 {
 		return Ok(get_remote_server_keys_batch::v2::Response::new(Vec::new()));
 	}
@@ -122,6 +127,9 @@ pub(crate) async fn get_remote_server_keys_route(
 					.broad_filter_map(|ssk| async move {
 						let mut canonical = to_canonical_object(&ssk).ok()?;
 						services.server_keys.sign_json(&mut canonical).ok()?;
+						server_keys::strip_extraneous_signatures(&mut canonical, server_name, &[
+							services.globals.server_name(),
+						]);
 						to_raw_value(&canonical)
 							.map(Raw::<ServerSigningKeys>::from_json)
 							.ok()
@@ -148,9 +156,36 @@ pub(crate) async fn get_remote_server_keys_route(
 			}
 			let mut canonical = to_canonical_object(&ssk)?;
 			services.server_keys.sign_json(&mut canonical)?;
+			server_keys::strip_extraneous_signatures(&mut canonical, server_name, &[services
+				.globals
+				.server_name()]);
 			response.push(to_raw_value(&canonical).map(Raw::<ServerSigningKeys>::from_json)?);
 		}
 	}
 
 	Ok(get_remote_server_keys_batch::v2::Response::new(response))
 }
+
+pub(crate) async fn get_remote_server_keys_route(
+	State(services): State<crate::State>,
+	body: Ruma<get_remote_server_keys::v2::Request>,
+) -> Result<get_remote_server_keys::v2::Response> {
+	let min_valid_ts = body.minimum_valid_until_ts;
+	let response = services
+		.server_keys
+		.signing_keys_for(&body.server_name)
+		.broad_filter_map(|ssk| async move {
+			if ssk.valid_until_ts > in_one_week() || ssk.valid_until_ts < min_valid_ts {
+				return None;
+			}
+
+			let mut canonical = to_canonical_object(&ssk).ok()?;
+			services.server_keys.sign_json(&mut canonical).ok()?;
+			to_raw_value(&canonical)
+				.map(Raw::<ServerSigningKeys>::from_json)
+				.ok()
+		})
+		.collect::<Vec<_>>()
+		.await;
+	Ok(get_remote_server_keys::v2::Response::new(response))
+}

src/service/server_keys/mod.rs

@@ -21,6 +21,7 @@
 	room_version_rules::RoomVersionRules,
 	signatures::{Ed25519KeyPair, PublicKeyMap, PublicKeySet},
 };
+pub use verify::strip_extraneous_signatures;
 
 use crate::{Dep, globals, sending, server_keys::util::required_keys};
 

src/service/server_keys/verify.rs

@@ -78,7 +78,7 @@ pub fn verify_server_keys_response(
 		pubkey_map.insert(server_keys.server_name.to_string(), set);
 
 		let mut canonical = to_canonical_object(server_keys)?;
-		Self::strip_extraneous_signatures(&mut canonical, &server_keys.server_name, &[]);
+		strip_extraneous_signatures(&mut canonical, &server_keys.server_name, &[]);
 		ruma::signatures::verify_json(&pubkey_map, &canonical).map_err(Into::into)
 	}
 
@@ -100,7 +100,7 @@ fn verify_notary_signature(
 			)));
 		};
 		let mut canonical_object = to_canonical_object(notary_signatures)?;
-		Self::strip_extraneous_signatures(&mut canonical_object, &server_keys.server_name, &[
+		strip_extraneous_signatures(&mut canonical_object, &server_keys.server_name, &[
 			notary_name,
 		]);
 		let for_verify = ruma::signatures::to_canonical_json_string_for_signing(
@@ -137,22 +137,22 @@ fn verify_notary_signature(
 			"No valid signature from {notary_name} present on signing keys response"
 		)))
 	}
+}
 
-	fn strip_extraneous_signatures(
-		canonical: &mut CanonicalJsonObject,
-		origin: &ServerName,
-		notaries: &[&ServerName],
-	) {
-		canonical.entry("signatures".to_owned()).and_modify(|sigs| {
-			sigs.as_object_mut().map(|s| {
-				s.retain(|server_name, _| {
-					let Ok(server_name) = ServerName::parse(server_name) else { return false };
-					origin == server_name || notaries.iter().any(|ns| *ns == server_name)
-				});
-				Some(s)
+pub fn strip_extraneous_signatures(
+	canonical: &mut CanonicalJsonObject,
+	origin: &ServerName,
+	notaries: &[&ServerName],
+) {
+	canonical.entry("signatures".to_owned()).and_modify(|sigs| {
+		sigs.as_object_mut().map(|s| {
+			s.retain(|server_name, _| {
+				let Ok(server_name) = ServerName::parse(server_name) else { return false };
+				origin == server_name || notaries.iter().any(|ns| *ns == server_name)
 			});
+			Some(s)
 		});
-	}
+	});
 }
 
 #[cfg(test)]