From a0b65eda1ed72cae2d75782ad4248f3ba875d4b3 Mon Sep 17 00:00:00 2001
From: Matthias Ahouansou <matthias@ahouansou.cz>
Date: Fri, 12 Apr 2024 20:25:20 -0400
Subject: [PATCH] merge the huge authentication MR mess (reject requests with
authentication when not used)
and (fix: allow invalid auth when no auth is required)
Signed-off-by: strawberry <strawberry@puppygock.gay>
---
src/api/ruma_wrapper/axum.rs | 357 +++++++++++++++--------------------
1 file changed, 149 insertions(+), 208 deletions(-)
diff --git a/src/api/ruma_wrapper/axum.rs b/src/api/ruma_wrapper/axum.rs
index f1ea1b856..902b6168e 100644
--- a/src/api/ruma_wrapper/axum.rs
+++ b/src/api/ruma_wrapper/axum.rs
@@ -15,13 +15,20 @@
use http::{Request, StatusCode};
use ruma::{
api::{client::error::ErrorKind, AuthScheme, IncomingRequest, OutgoingResponse},
- CanonicalJsonValue, OwnedDeviceId, OwnedServerName, UserId,
+ CanonicalJsonValue, OwnedDeviceId, OwnedServerName, OwnedUserId, UserId,
};
use serde::Deserialize;
use tracing::{debug, error, trace, warn};
use super::{Ruma, RumaResponse};
-use crate::{services, Error, Result};
+use crate::{service::appservice::RegistrationInfo, services, Error, Result};
+
+enum Token {
+ Appservice(Box<RegistrationInfo>),
+ User((OwnedUserId, OwnedDeviceId)),
+ Invalid,
+ None,
+}
#[derive(Deserialize)]
struct QueryParams {
@@ -66,7 +73,7 @@ async fn from_request(req: Request<B>, _state: &S) -> Result<Self, Self::Rejecti
let query_params: QueryParams = match serde_html_form::from_str(query) {
Ok(params) => params,
Err(e) => {
- error!(%query, "Failed to deserialize query parameters: {}", e);
+ error!(%query, "Failed to deserialize query parameters: {e}");
return Err(Error::BadRequest(ErrorKind::Unknown, "Failed to read query parameters"));
},
};
@@ -76,228 +83,162 @@ async fn from_request(req: Request<B>, _state: &S) -> Result<Self, Self::Rejecti
None => query_params.access_token.as_deref(),
};
- let mut json_body = serde_json::from_slice::<CanonicalJsonValue>(&body).ok();
-
- let appservice_registration = if let Some(token) = token {
- services().appservice.find_from_token(token).await
+ let token = if let Some(token) = token {
+ if let Some(reg_info) = services().appservice.find_from_token(token).await {
+ Token::Appservice(Box::new(reg_info))
+ } else if let Some((user_id, device_id)) = services().users.find_from_token(token)? {
+ Token::User((user_id, OwnedDeviceId::from(device_id)))
+ } else {
+ Token::Invalid
+ }
} else {
- None
+ Token::None
};
- let (sender_user, sender_device, sender_servername, from_appservice) =
- if let Some(info) = appservice_registration {
- match metadata.authentication {
- AuthScheme::AccessToken => {
- let user_id = query_params.user_id.map_or_else(
- || {
- UserId::parse_with_server_name(
- info.registration.sender_localpart.as_str(),
- services().globals.server_name(),
- )
- .unwrap()
- },
- |s| UserId::parse(s).unwrap(),
- );
-
- debug!("User ID: {:?}", user_id);
-
- if !services().users.exists(&user_id)? {
- return Err(Error::BadRequest(ErrorKind::forbidden(), "User does not exist."));
- }
+ let mut json_body = serde_json::from_slice::<CanonicalJsonValue>(&body).ok();
- // TODO: Check if appservice is allowed to be that user
- (Some(user_id), None, None, true)
- },
- AuthScheme::AccessTokenOptional | AuthScheme::AppserviceToken => {
- let user_id = query_params.user_id.map_or_else(
- || {
- UserId::parse_with_server_name(
- info.registration.sender_localpart.as_str(),
- services().globals.server_name(),
- )
- .unwrap()
- },
- |s| UserId::parse(s).unwrap(),
- );
-
- debug!("User ID: {:?}", user_id);
-
- if !services().users.exists(&user_id)? {
- (None, None, None, true)
- } else {
- // TODO: Check if appservice is allowed to be that user
- (Some(user_id), None, None, true)
- }
+ let (sender_user, sender_device, sender_servername, from_appservice) = match (metadata.authentication, token) {
+ (AuthScheme::None, Token::Invalid) => (None, None, None, false),
+ (_, Token::Invalid) => {
+ return Err(Error::BadRequest(
+ ErrorKind::UnknownToken {
+ soft_logout: false,
},
- AuthScheme::ServerSignatures | AuthScheme::None => (None, None, None, true),
+ "Unknown access token.",
+ ))
+ },
+ (
+ AuthScheme::AccessToken
+ | AuthScheme::AppserviceToken
+ | AuthScheme::AccessTokenOptional
+ | AuthScheme::None,
+ Token::Appservice(info),
+ ) => {
+ let user_id = query_params
+ .user_id
+ .map_or_else(
+ || {
+ UserId::parse_with_server_name(
+ info.registration.sender_localpart.as_str(),
+ services().globals.server_name(),
+ )
+ },
+ UserId::parse,
+ )
+ .map_err(|_| Error::BadRequest(ErrorKind::InvalidUsername, "Username is invalid."))?;
+ if !services().users.exists(&user_id)? {
+ return Err(Error::BadRequest(ErrorKind::forbidden(), "User does not exist."));
}
- } else {
- match metadata.authentication {
- AuthScheme::AccessToken => {
- let Some(token) = token else {
- return Err(Error::BadRequest(ErrorKind::MissingToken, "Missing access token."));
- };
- match services().users.find_from_token(token)? {
- None => {
- return Err(Error::BadRequest(
- ErrorKind::UnknownToken {
- soft_logout: false,
- },
- "Unknown access token.",
- ))
- },
- Some((user_id, device_id)) => {
- (Some(user_id), Some(OwnedDeviceId::from(device_id)), None, false)
- },
- }
- },
- AuthScheme::AccessTokenOptional => {
- let token = token.unwrap_or("");
-
- if token.is_empty() {
- (None, None, None, false)
- } else {
- match services().users.find_from_token(token)? {
- None => {
- return Err(Error::BadRequest(
- ErrorKind::UnknownToken {
- soft_logout: false,
- },
- "Unknown access token.",
- ))
- },
- Some((user_id, device_id)) => {
- (Some(user_id), Some(OwnedDeviceId::from(device_id)), None, false)
- },
- }
- }
- },
- // treat non-appservice registrations as None authentication
- AuthScheme::AppserviceToken => (None, None, None, false),
- AuthScheme::ServerSignatures => {
- if !services().globals.allow_federation() {
- return Err(Error::bad_config("Federation is disabled."));
- }
-
- let TypedHeader(Authorization(x_matrix)) = parts
- .extract::<TypedHeader<Authorization<XMatrix>>>()
- .await
- .map_err(|e| {
- warn!("Missing or invalid Authorization header: {}", e);
-
- let msg = match e.reason() {
- TypedHeaderRejectionReason::Missing => "Missing Authorization header.",
- TypedHeaderRejectionReason::Error(_) => "Invalid X-Matrix signatures.",
- _ => "Unknown header-related error",
- };
+ // TODO: Check if appservice is allowed to be that user
+ (Some(user_id), None, None, true)
+ },
+ (AuthScheme::AccessToken, Token::None) => {
+ return Err(Error::BadRequest(ErrorKind::MissingToken, "Missing access token."));
+ },
+ (
+ AuthScheme::AccessToken | AuthScheme::AccessTokenOptional | AuthScheme::None,
+ Token::User((user_id, device_id)),
+ ) => (Some(user_id), Some(device_id), None, false),
+ (AuthScheme::ServerSignatures, Token::None) => {
+ if !services().globals.allow_federation() {
+ return Err(Error::bad_config("Federation is disabled."));
+ }
- Error::BadRequest(ErrorKind::forbidden(), msg)
- })?;
+ let TypedHeader(Authorization(x_matrix)) = parts
+ .extract::<TypedHeader<Authorization<XMatrix>>>()
+ .await
+ .map_err(|e| {
+ warn!("Missing or invalid Authorization header: {e}");
- let origin_signatures =
- BTreeMap::from_iter([(x_matrix.key.clone(), CanonicalJsonValue::String(x_matrix.sig))]);
+ let msg = match e.reason() {
+ TypedHeaderRejectionReason::Missing => "Missing Authorization header.",
+ TypedHeaderRejectionReason::Error(_) => "Invalid X-Matrix signatures.",
+ _ => "Unknown header-related error",
+ };
- let signatures = BTreeMap::from_iter([(
- x_matrix.origin.as_str().to_owned(),
- CanonicalJsonValue::Object(origin_signatures),
- )]);
+ Error::BadRequest(ErrorKind::forbidden(), msg)
+ })?;
- let server_destination = services().globals.server_name().as_str().to_owned();
+ let origin_signatures =
+ BTreeMap::from_iter([(x_matrix.key.clone(), CanonicalJsonValue::String(x_matrix.sig))]);
- if let Some(destination) = x_matrix.destination.as_ref() {
- if destination != &server_destination {
- return Err(Error::BadRequest(ErrorKind::forbidden(), "Invalid authorization."));
- }
- }
+ let signatures = BTreeMap::from_iter([(
+ x_matrix.origin.as_str().to_owned(),
+ CanonicalJsonValue::Object(origin_signatures),
+ )]);
- let mut request_map = BTreeMap::from_iter([
- ("method".to_owned(), CanonicalJsonValue::String(parts.method.to_string())),
- ("uri".to_owned(), CanonicalJsonValue::String(parts.uri.to_string())),
- (
- "origin".to_owned(),
- CanonicalJsonValue::String(x_matrix.origin.as_str().to_owned()),
- ),
- ("destination".to_owned(), CanonicalJsonValue::String(server_destination)),
- ("signatures".to_owned(), CanonicalJsonValue::Object(signatures)),
- ]);
-
- if let Some(json_body) = &json_body {
- request_map.insert("content".to_owned(), json_body.clone());
- };
+ let server_destination = services().globals.server_name().as_str().to_owned();
- let keys_result = services()
- .rooms
- .event_handler
- .fetch_signing_keys_for_server(&x_matrix.origin, vec![x_matrix.key.clone()])
- .await;
-
- let keys = match keys_result {
- Ok(b) => b,
- Err(e) => {
- warn!("Failed to fetch signing keys: {}", e);
- return Err(Error::BadRequest(ErrorKind::forbidden(), "Failed to fetch signing keys."));
- },
- };
+ if let Some(destination) = x_matrix.destination.as_ref() {
+ if destination != &server_destination {
+ return Err(Error::BadRequest(ErrorKind::forbidden(), "Invalid authorization."));
+ }
+ }
- let pub_key_map = BTreeMap::from_iter([(x_matrix.origin.as_str().to_owned(), keys)]);
-
- match ruma::signatures::verify_json(&pub_key_map, &request_map) {
- Ok(()) => (None, None, Some(x_matrix.origin), false),
- Err(e) => {
- warn!(
- "Failed to verify json request from {}: {}\n{:?}",
- x_matrix.origin, e, request_map
- );
-
- if parts.uri.to_string().contains('@') {
- warn!(
- "Request uri contained '@' character. Make sure your reverse proxy gives \
- Conduit the raw uri (apache: use nocanon)"
- );
- }
-
- return Err(Error::BadRequest(
- ErrorKind::forbidden(),
- "Failed to verify X-Matrix signatures.",
- ));
- },
+ let mut request_map = BTreeMap::from_iter([
+ ("method".to_owned(), CanonicalJsonValue::String(parts.method.to_string())),
+ ("uri".to_owned(), CanonicalJsonValue::String(parts.uri.to_string())),
+ (
+ "origin".to_owned(),
+ CanonicalJsonValue::String(x_matrix.origin.as_str().to_owned()),
+ ),
+ ("destination".to_owned(), CanonicalJsonValue::String(server_destination)),
+ ("signatures".to_owned(), CanonicalJsonValue::Object(signatures)),
+ ]);
+
+ if let Some(json_body) = &json_body {
+ request_map.insert("content".to_owned(), json_body.clone());
+ };
+
+ let keys_result = services()
+ .rooms
+ .event_handler
+ .fetch_signing_keys_for_server(&x_matrix.origin, vec![x_matrix.key.clone()])
+ .await;
+
+ let keys = keys_result.map_err(|e| {
+ warn!("Failed to fetch signing keys: {e}");
+ Error::BadRequest(ErrorKind::forbidden(), "Failed to fetch signing keys.")
+ })?;
+
+ let pub_key_map = BTreeMap::from_iter([(x_matrix.origin.as_str().to_owned(), keys)]);
+
+ match ruma::signatures::verify_json(&pub_key_map, &request_map) {
+ Ok(()) => (None, None, Some(x_matrix.origin), false),
+ Err(e) => {
+ warn!("Failed to verify json request from {}: {e}\n{request_map:?}", x_matrix.origin);
+
+ if parts.uri.to_string().contains('@') {
+ warn!(
+ "Request uri contained '@' character. Make sure your reverse proxy gives Conduit the \
+ raw uri (apache: use nocanon)"
+ );
}
- },
- AuthScheme::None => match parts.uri.path() {
- // TOOD: can we do a better check here?
- // allow_public_room_directory_without_auth
- "/_matrix/client/v3/publicRooms" | "/_matrix/client/r0/publicRooms" => {
- if !services()
- .globals
- .config
- .allow_public_room_directory_without_auth
- {
- let Some(token) = token else {
- return Err(Error::BadRequest(ErrorKind::MissingToken, "Missing access token."));
- };
-
- match services().users.find_from_token(token)? {
- None => {
- return Err(Error::BadRequest(
- ErrorKind::UnknownToken {
- soft_logout: false,
- },
- "Unknown access token.",
- ))
- },
- Some((user_id, device_id)) => {
- (Some(user_id), Some(OwnedDeviceId::from(device_id)), None, false)
- },
- }
- } else {
- (None, None, None, false)
- }
- },
- _ => (None, None, None, false),
+
+ return Err(Error::BadRequest(
+ ErrorKind::forbidden(),
+ "Failed to verify X-Matrix signatures.",
+ ));
},
}
- };
+ },
+ (AuthScheme::None | AuthScheme::AppserviceToken | AuthScheme::AccessTokenOptional, Token::None) => {
+ (None, None, None, false)
+ },
+ (AuthScheme::ServerSignatures, Token::Appservice(_) | Token::User(_)) => {
+ return Err(Error::BadRequest(
+ ErrorKind::Unauthorized,
+ "Only server signatures should be used on this endpoint.",
+ ));
+ },
+ (AuthScheme::AppserviceToken, Token::User(_)) => {
+ return Err(Error::BadRequest(
+ ErrorKind::Unauthorized,
+ "Only appservice access tokens should be used on this endpoint.",
+ ));
+ },
+ };
let mut http_request = Request::builder().uri(parts.uri).method(parts.method);
*http_request.headers_mut().unwrap() = parts.headers;
@@ -398,7 +339,7 @@ fn decode(value: &http::HeaderValue) -> Option<Self> {
"key" => key = Some(value.to_owned()),
"sig" => sig = Some(value.to_owned()),
"destination" => destination = Some(value.to_owned()),
- _ => debug!("Unexpected field `{}` in X-Matrix Authorization header", name),
+ _ => debug!("Unexpected field `{name}` in X-Matrix Authorization header"),
}
}
--
GitLab