fix(client-ip): Fall back to peer IP instead of returning 500

changelog.d/+30d4369c.bugfix.md

@@ -0,0 +1 @@
+Fixed requests returning `500 Internal Server Error` when the header selected by `request_ip_source` is absent, duplicated, or malformed (for example Envoy omitting `X-Envoy-External-Address` on internal requests). The client IP now falls back to the connection peer address instead of failing the request. Contributed by @eleboucher

src/api/client/account/mod.rs

@@ -1,5 +1,4 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, err, info,
 	pdu::PartialPdu,
@@ -27,7 +26,7 @@ use ruma::{
 use service::{mailer::messages, uiaa::UiaaInitiator, users::HashedPassword};
 
 use super::DEVICE_ID_LENGTH;
-use crate::{Ruma, router::ClientIdentity};
+use crate::{Ruma, client_ip::ClientIp, router::ClientIdentity};
 
 pub(crate) mod register;
 pub(crate) mod threepid;

src/api/client/account/register.rs

@@ -1,7 +1,6 @@
 use std::collections::HashMap;
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, debug_info, info,
 	utils::{self},
@@ -26,7 +25,7 @@ use service::{
 };
 
 use super::DEVICE_ID_LENGTH;
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `POST /_matrix/client/v3/register`
 ///

src/api/client/dehydrated_device.rs

@@ -1,5 +1,4 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{Err, Result, at};
 use futures::StreamExt;
 use ruma::{
@@ -12,7 +11,7 @@ use ruma::{
 	assign,
 };
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 const MAX_BATCH_EVENTS: usize = 50;
 

src/api/client/device.rs

@@ -1,5 +1,4 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{Err, Result, debug, err, utils};
 use futures::StreamExt;
 use ruma::{
@@ -9,7 +8,7 @@ use ruma::{
 	},
 };
 
-use crate::{Ruma, client::DEVICE_ID_LENGTH};
+use crate::{Ruma, client::DEVICE_ID_LENGTH, client_ip::ClientIp};
 
 /// # `GET /_matrix/client/r0/devices`
 ///

src/api/client/directory.rs

@@ -1,5 +1,4 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, err, info,
 	utils::{
@@ -28,7 +27,7 @@ use ruma::{
 };
 use tokio::join;
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `POST /_matrix/client/v3/publicRooms`
 ///

src/api/client/media.rs

@@ -1,7 +1,6 @@
 use std::time::Duration;
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, err,
 	utils::{self, content_disposition::make_content_disposition, math::ruma_from_usize},
@@ -24,7 +23,7 @@ use ruma::{
 };
 use service::media::mxc::Mxc;
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `GET /_matrix/client/v1/media/config`
 pub(crate) async fn get_media_config_route(

src/api/client/media_legacy.rs

@@ -1,7 +1,6 @@
 #![allow(deprecated)]
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, err,
 	utils::{content_disposition::make_content_disposition, math::ruma_from_usize},
@@ -17,7 +16,7 @@ use ruma::{
 };
 use service::media::mxc::Mxc;
 
-use crate::{Ruma, RumaResponse, client::create_content_route};
+use crate::{Ruma, RumaResponse, client::create_content_route, client_ip::ClientIp};
 
 /// # `GET /_matrix/media/v3/config`
 ///

src/api/client/membership/invite.rs

@@ -1,5 +1,4 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, debug_error, err, info,
 	matrix::{event::gen_event_id_canonical_json, pdu::PartialPdu},
@@ -18,7 +17,7 @@ use ruminuwuity::invite_permission_config::FilterLevel;
 use service::Services;
 
 use super::banned_room_check;
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `POST /_matrix/client/r0/rooms/{roomId}/invite`
 ///

src/api/client/membership/join.rs

@@ -1,5 +1,4 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, debug,
 	result::FlatOk,
@@ -12,7 +11,7 @@ use ruma::{
 };
 
 use super::banned_room_check;
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `POST /_matrix/client/r0/rooms/{roomId}/join`
 ///

src/api/client/membership/knock.rs

@@ -1,7 +1,6 @@
 use std::{borrow::Borrow, collections::HashMap, iter::once, sync::Arc};
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, debug, debug_info, debug_warn, err, info,
 	matrix::{
@@ -40,7 +39,7 @@ use service::{
 };
 
 use super::banned_room_check;
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `POST /_matrix/client/*/knock/{roomIdOrAlias}`
 ///

src/api/client/message.rs

@@ -1,5 +1,4 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Error, Result, at, debug_warn,
 	matrix::{
@@ -38,7 +37,7 @@ use ruma::{
 };
 use ruminuwuity::invite_permission_config::FilterLevel;
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// list of safe and common non-state events to ignore if the user is ignored
 const IGNORED_MESSAGE_TYPES: &[TimelineEventType] = &[

src/api/client/read_marker.rs

@@ -1,7 +1,6 @@
 use std::collections::BTreeMap;
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{Err, PduCount, Result, err};
 use ruma::{
 	MilliSecondsSinceUnixEpoch,
@@ -13,7 +12,7 @@ use ruma::{
 	},
 };
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `POST /_matrix/client/r0/rooms/{roomId}/read_markers`
 ///

src/api/client/redact.rs

@@ -1,11 +1,10 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{Err, Result, matrix::pdu::PartialPdu};
 use ruma::{
 	api::client::redact::redact_event, assign, events::room::redaction::RoomRedactionEventContent,
 };
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `PUT /_matrix/client/r0/rooms/{roomId}/redact/{eventId}/{txnId}`
 ///

src/api/client/report.rs

@@ -1,7 +1,6 @@
 use std::{fmt::Write as _, time::Duration};
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{Err, Event, Result, debug_info, info, matrix::pdu::PduEvent, utils::ReadyExt};
 use conduwuit_service::Services;
 use ruma::{
@@ -14,7 +13,7 @@ use ruma::{
 };
 use tokio::time::sleep;
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 struct Report {
 	sender: OwnedUserId,

src/api/client/room/summary.rs

@@ -1,10 +1,9 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{Err, Result};
 use ruma::api::client::room::get_summary;
 use service::rooms::summary::Accessibility;
 
-use crate::{Ruma, router::ClientIdentity};
+use crate::{Ruma, client_ip::ClientIp, router::ClientIdentity};
 
 /// # `GET /_matrix/client/v1/room_summary/{roomIdOrAlias}`
 ///

src/api/client/send.rs

@@ -1,12 +1,11 @@
 use std::collections::BTreeMap;
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{Err, Result, err, matrix::pdu::PartialPdu, utils};
 use ruma::{api::client::message::send_message_event, events::MessageLikeEventType};
 use serde_json::from_str;
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `PUT /_matrix/client/v3/rooms/{roomId}/send/{eventType}/{txnId}`
 ///

src/api/client/session.rs

@@ -1,7 +1,6 @@
 use std::time::Duration;
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, debug, err, info,
 	utils::{self, ReadyExt, stream::BroadbandExt},
@@ -32,7 +31,7 @@ use ruma::{
 use service::users::DeviceToken;
 
 use super::DEVICE_ID_LENGTH;
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `GET /_matrix/client/v3/login`
 ///

src/api/client/state.rs

@@ -1,7 +1,6 @@
 #[cfg(test)]
 mod tests;
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, err,
 	matrix::{Event, pdu::PartialPdu},
@@ -28,7 +27,7 @@ use ruma::{
 };
 use serde_json::{json, value::to_raw_value};
 
-use crate::{Ruma, RumaResponse};
+use crate::{Ruma, RumaResponse, client_ip::ClientIp};
 
 /// # `PUT /_matrix/client/*/rooms/{roomId}/state/{eventType}/{stateKey}`
 ///

src/api/client/sync/v3/mod.rs

@@ -9,7 +9,6 @@ use std::{
 };
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Result, at, error, extract_variant,
 	utils::{
@@ -48,6 +47,7 @@ use crate::{
 		is_ignored_invite,
 		sync::v3::{joined::load_joined_room, left::load_left_room},
 	},
+	client_ip::ClientIp,
 };
 
 /// The default maximum number of events to return in the `timeline` key of

src/api/client/sync/v5.rs

@@ -6,7 +6,6 @@ use std::{
 };
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Error, Result, at, error, extract_variant, is_equal_to,
 	matrix::{Event, TypeStateKey, pdu::PduCount},
@@ -49,6 +48,7 @@ use crate::{
 	client::{
 		DEFAULT_BUMP_TYPES, TimelinePdus, ignored_filter, is_ignored_invite, sync::load_timeline,
 	},
+	client_ip::ClientIp,
 };
 
 type SyncInfo<'a> = (&'a UserId, &'a DeviceId, u64, &'a sync_events::v5::Request);

src/api/client/typing.rs

@@ -1,9 +1,8 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{Err, Result, utils, utils::math::Tried};
 use ruma::api::client::typing::create_typing_event::{self, v3::TypingInfo};
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `PUT /_matrix/client/r0/rooms/{roomId}/typing/{userId}`
 ///

src/api/client_ip.rs

@@ -0,0 +1,135 @@
+use std::{
+	convert::Infallible,
+	net::{IpAddr, Ipv4Addr, SocketAddr},
+};
+
+use axum::extract::{ConnectInfo, FromRequestParts};
+use axum_client_ip::ClientIp as SourcedClientIp;
+use conduwuit::debug_warn;
+use http::request::Parts;
+
+const UNKNOWN_IP: IpAddr = IpAddr::V4(Ipv4Addr::UNSPECIFIED);
+
+/// [`ClientIp`] extractor that falls back to the connection peer address
+/// instead of rejecting the request when `request_ip_source` can't be resolved.
+#[derive(Debug, Clone, Copy)]
+pub(crate) struct ClientIp(pub IpAddr);
+
+impl<S> FromRequestParts<S> for ClientIp
+where
+	S: Sync,
+{
+	type Rejection = Infallible;
+
+	async fn from_request_parts(parts: &mut Parts, state: &S) -> Result<Self, Self::Rejection> {
+		let ip = match SourcedClientIp::from_request_parts(parts, state).await {
+			| Ok(SourcedClientIp(ip)) => ip,
+			| Err(rejection) => {
+				debug_warn!(
+					%rejection,
+					"Could not resolve client IP from request_ip_source; using peer address"
+				);
+				parts
+					.extensions
+					.get::<ConnectInfo<SocketAddr>>()
+					.map_or(UNKNOWN_IP, |ConnectInfo(addr)| addr.ip())
+			},
+		};
+
+		Ok(Self(ip))
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use std::net::Ipv6Addr;
+
+	use axum_client_ip::ClientIpSource;
+
+	use super::*;
+
+	const PEER: SocketAddr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(198, 51, 100, 7)), 8448);
+	const PEER_V6: SocketAddr =
+		SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0x2001, 0xDB8, 0, 0, 0, 0, 0, 1)), 8448);
+
+	fn parts(
+		source: Option<ClientIpSource>,
+		peer: Option<SocketAddr>,
+		headers: &[(&str, &str)],
+	) -> Parts {
+		let mut builder = http::Request::builder();
+		for (name, value) in headers {
+			builder = builder.header(*name, *value);
+		}
+
+		let (mut parts, ()) = builder.body(()).unwrap().into_parts();
+		if let Some(source) = source {
+			parts.extensions.insert(source);
+		}
+		if let Some(peer) = peer {
+			parts.extensions.insert(ConnectInfo(peer));
+		}
+
+		parts
+	}
+
+	async fn extract(mut parts: Parts) -> IpAddr {
+		let ClientIp(ip) = ClientIp::from_request_parts(&mut parts, &()).await.unwrap();
+		ip
+	}
+
+	#[tokio::test]
+	async fn resolves_from_configured_source() {
+		let parts =
+			parts(Some(ClientIpSource::XRealIp), Some(PEER), &[("x-real-ip", "203.0.113.5")]);
+
+		assert_eq!(extract(parts).await, IpAddr::V4(Ipv4Addr::new(203, 0, 113, 5)));
+	}
+
+	#[tokio::test]
+	async fn resolves_ipv6_from_configured_source() {
+		let parts =
+			parts(Some(ClientIpSource::XRealIp), Some(PEER), &[("x-real-ip", "2001:db8::2")]);
+
+		assert_eq!(
+			extract(parts).await,
+			IpAddr::V6(Ipv6Addr::new(0x2001, 0xDB8, 0, 0, 0, 0, 0, 2))
+		);
+	}
+
+	#[tokio::test]
+	async fn falls_back_to_peer_when_header_missing() {
+		let parts = parts(Some(ClientIpSource::XRealIp), Some(PEER), &[]);
+
+		assert_eq!(extract(parts).await, PEER.ip());
+	}
+
+	#[tokio::test]
+	async fn falls_back_to_ipv6_peer_when_header_missing() {
+		let parts = parts(Some(ClientIpSource::XRealIp), Some(PEER_V6), &[]);
+
+		assert_eq!(extract(parts).await, PEER_V6.ip());
+	}
+
+	#[tokio::test]
+	async fn falls_back_to_peer_when_header_unparsable() {
+		let parts =
+			parts(Some(ClientIpSource::XRealIp), Some(PEER), &[("x-real-ip", "not-an-ip")]);
+
+		assert_eq!(extract(parts).await, PEER.ip());
+	}
+
+	#[tokio::test]
+	async fn falls_back_to_peer_when_source_unset() {
+		let parts = parts(None, Some(PEER), &[("x-real-ip", "203.0.113.5")]);
+
+		assert_eq!(extract(parts).await, PEER.ip());
+	}
+
+	#[tokio::test]
+	async fn falls_back_to_unspecified_without_peer() {
+		let parts = parts(Some(ClientIpSource::XRealIp), None, &[]);
+
+		assert_eq!(extract(parts).await, UNKNOWN_IP);
+	}
+}

src/api/mod.rs

@@ -11,6 +11,8 @@ pub mod client;
 pub mod router;
 pub mod server;
 
+pub(crate) mod client_ip;
+
 pub mod admin;
 
 pub(crate) use self::router::{Ruma, RumaResponse, State};

src/api/server/invite.rs

@@ -1,7 +1,6 @@
 use std::collections::{HashMap, hash_map::Entry};
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use base64::{Engine as _, engine::general_purpose};
 use conduwuit::{
 	Err, Error, EventTypeExt, PduEvent, Result, debug, err, error,
@@ -23,7 +22,7 @@ use ruma::{
 };
 use serde::Deserialize;
 
-use crate::{Ruma, server::utils::validate_any_membership_event};
+use crate::{Ruma, client_ip::ClientIp, server::utils::validate_any_membership_event};
 
 /// # `PUT /_matrix/federation/v2/invite/{roomId}/{eventId}`
 ///

src/api/server/media.rs

@@ -1,5 +1,4 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{Err, Result, utils::content_disposition::make_content_disposition};
 use conduwuit_service::media::{Dim, FileMeta};
 use ruma::api::federation::authenticated_media::{
@@ -7,7 +6,7 @@ use ruma::api::federation::authenticated_media::{
 };
 use service::media::mxc::Mxc;
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `GET /_matrix/federation/v1/media/download/{mediaId}`
 ///

src/api/server/publicrooms.rs

@@ -1,5 +1,4 @@
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{Err, Result, err};
 use ruma::{
 	api::federation::directory::{get_public_rooms, get_public_rooms_filtered},
@@ -7,7 +6,7 @@ use ruma::{
 	directory::Filter,
 };
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 /// # `POST /_matrix/federation/v1/publicRooms`
 ///

src/api/server/send.rs

@@ -5,7 +5,6 @@ use std::{
 };
 
 use axum::extract::State;
-use axum_client_ip::ClientIp;
 use conduwuit::{
 	Err, Error, Result, debug, debug_error, debug_warn, err, error,
 	result::LogErr,
@@ -47,7 +46,7 @@ use service::{
 use tokio::sync::watch::{Receiver, Sender};
 use tracing::instrument;
 
-use crate::Ruma;
+use crate::{Ruma, client_ip::ClientIp};
 
 type ResolvedMap = BTreeMap<OwnedEventId, Result>;
 type Pdu = (OwnedRoomId, OwnedEventId, CanonicalJsonObject);