From 3a74ce6151c0eb2fefb35f67c22aad9c93c020e7 Mon Sep 17 00:00:00 2001 From: Erwan Leboucher Date: Thu, 16 Jul 2026 13:38:37 +0200 Subject: [PATCH] fix(client-ip): Fall back to peer IP instead of returning 500 --- changelog.d/+30d4369c.bugfix.md | 1 + src/api/client/account/mod.rs | 3 +- src/api/client/account/register.rs | 3 +- src/api/client/dehydrated_device.rs | 3 +- src/api/client/device.rs | 3 +- src/api/client/directory.rs | 3 +- src/api/client/media.rs | 3 +- src/api/client/media_legacy.rs | 3 +- src/api/client/membership/invite.rs | 3 +- src/api/client/membership/join.rs | 3 +- src/api/client/membership/knock.rs | 3 +- src/api/client/message.rs | 3 +- src/api/client/read_marker.rs | 3 +- src/api/client/redact.rs | 3 +- src/api/client/report.rs | 3 +- src/api/client/room/summary.rs | 3 +- src/api/client/send.rs | 3 +- src/api/client/session.rs | 3 +- src/api/client/state.rs | 3 +- src/api/client/sync/v3/mod.rs | 2 +- src/api/client/sync/v5.rs | 2 +- src/api/client/typing.rs | 3 +- src/api/client_ip.rs | 135 ++++++++++++++++++++++++++++ src/api/mod.rs | 2 + src/api/server/invite.rs | 3 +- src/api/server/media.rs | 3 +- src/api/server/publicrooms.rs | 3 +- src/api/server/send.rs | 3 +- 28 files changed, 163 insertions(+), 48 deletions(-) create mode 100644 changelog.d/+30d4369c.bugfix.md create mode 100644 src/api/client_ip.rs diff --git a/changelog.d/+30d4369c.bugfix.md b/changelog.d/+30d4369c.bugfix.md new file mode 100644 index 000000000..aad3fcd0d --- /dev/null +++ b/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 diff --git a/src/api/client/account/mod.rs b/src/api/client/account/mod.rs index 62248a7ca..51060ec7d 100644 --- a/src/api/client/account/mod.rs +++ b/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; diff --git a/src/api/client/account/register.rs b/src/api/client/account/register.rs index 39a02e666..3317cbc08 100644 --- a/src/api/client/account/register.rs +++ b/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` /// diff --git a/src/api/client/dehydrated_device.rs b/src/api/client/dehydrated_device.rs index 6f0698999..ec6648bc5 100644 --- a/src/api/client/dehydrated_device.rs +++ b/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; diff --git a/src/api/client/device.rs b/src/api/client/device.rs index f81934900..bf9111576 100644 --- a/src/api/client/device.rs +++ b/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` /// diff --git a/src/api/client/directory.rs b/src/api/client/directory.rs index 6b209b4eb..789e067be 100644 --- a/src/api/client/directory.rs +++ b/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` /// diff --git a/src/api/client/media.rs b/src/api/client/media.rs index 05b63358d..6b6e46949 100644 --- a/src/api/client/media.rs +++ b/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( diff --git a/src/api/client/media_legacy.rs b/src/api/client/media_legacy.rs index f6ceb3a54..73a8ded37 100644 --- a/src/api/client/media_legacy.rs +++ b/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` /// diff --git a/src/api/client/membership/invite.rs b/src/api/client/membership/invite.rs index ac233675d..dd4bf62e6 100644 --- a/src/api/client/membership/invite.rs +++ b/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` /// diff --git a/src/api/client/membership/join.rs b/src/api/client/membership/join.rs index ba7f0eaa0..3b578b57c 100644 --- a/src/api/client/membership/join.rs +++ b/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` /// diff --git a/src/api/client/membership/knock.rs b/src/api/client/membership/knock.rs index 988767046..6bde8cbf2 100644 --- a/src/api/client/membership/knock.rs +++ b/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}` /// diff --git a/src/api/client/message.rs b/src/api/client/message.rs index 2c3ccc782..30956b7d0 100644 --- a/src/api/client/message.rs +++ b/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] = &[ diff --git a/src/api/client/read_marker.rs b/src/api/client/read_marker.rs index a476f6790..155892e8b 100644 --- a/src/api/client/read_marker.rs +++ b/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` /// diff --git a/src/api/client/redact.rs b/src/api/client/redact.rs index 377b14f4c..ab976ef27 100644 --- a/src/api/client/redact.rs +++ b/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}` /// diff --git a/src/api/client/report.rs b/src/api/client/report.rs index a7acd9790..10748b7b9 100644 --- a/src/api/client/report.rs +++ b/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, diff --git a/src/api/client/room/summary.rs b/src/api/client/room/summary.rs index 80c2e2f70..3463037d7 100644 --- a/src/api/client/room/summary.rs +++ b/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}` /// diff --git a/src/api/client/send.rs b/src/api/client/send.rs index 7a34c496d..339157d70 100644 --- a/src/api/client/send.rs +++ b/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}` /// diff --git a/src/api/client/session.rs b/src/api/client/session.rs index a7e86b6ac..6f93e6996 100644 --- a/src/api/client/session.rs +++ b/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` /// diff --git a/src/api/client/state.rs b/src/api/client/state.rs index 1067501d3..79354e1f3 100644 --- a/src/api/client/state.rs +++ b/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}` /// diff --git a/src/api/client/sync/v3/mod.rs b/src/api/client/sync/v3/mod.rs index 90b8ca83b..fd90d4df8 100644 --- a/src/api/client/sync/v3/mod.rs +++ b/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 diff --git a/src/api/client/sync/v5.rs b/src/api/client/sync/v5.rs index 3602cc45b..5887e6aee 100644 --- a/src/api/client/sync/v5.rs +++ b/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); diff --git a/src/api/client/typing.rs b/src/api/client/typing.rs index 4cc0dc6a0..582d8d650 100644 --- a/src/api/client/typing.rs +++ b/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}` /// diff --git a/src/api/client_ip.rs b/src/api/client_ip.rs new file mode 100644 index 000000000..a739b654d --- /dev/null +++ b/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 FromRequestParts for ClientIp +where + S: Sync, +{ + type Rejection = Infallible; + + async fn from_request_parts(parts: &mut Parts, state: &S) -> Result { + 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::>() + .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, + peer: Option, + 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); + } +} diff --git a/src/api/mod.rs b/src/api/mod.rs index 7cbd3d209..da9b1e715 100644 --- a/src/api/mod.rs +++ b/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}; diff --git a/src/api/server/invite.rs b/src/api/server/invite.rs index 392bfa7a9..89fb4bf7c 100644 --- a/src/api/server/invite.rs +++ b/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}` /// diff --git a/src/api/server/media.rs b/src/api/server/media.rs index 56f791889..49df25366 100644 --- a/src/api/server/media.rs +++ b/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}` /// diff --git a/src/api/server/publicrooms.rs b/src/api/server/publicrooms.rs index 9cf691603..edb72fee4 100644 --- a/src/api/server/publicrooms.rs +++ b/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` /// diff --git a/src/api/server/send.rs b/src/api/server/send.rs index bc1dbf32a..c9cd5b3de 100644 --- a/src/api/server/send.rs +++ b/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; type Pdu = (OwnedRoomId, OwnedEventId, CanonicalJsonObject);