From 5c634ceb6b755eaddeb1b854542d9e4a16d763f8 Mon Sep 17 00:00:00 2001 From: Matthias Ahouansou Date: Sat, 30 Mar 2024 11:02:39 +0000 Subject: [PATCH] fix: reject requests with authentication when not used --- Cargo.lock | 36 ++-- Cargo.toml | 2 +- src/api/ruma_wrapper/axum.rs | 322 +++++++++++++++++++---------------- 3 files changed, 198 insertions(+), 162 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4d294c08..bd83f2d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1992,7 +1992,7 @@ dependencies = [ [[package]] name = "ruma" version = "0.9.4" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "assign", "js_int", @@ -2011,7 +2011,7 @@ dependencies = [ [[package]] name = "ruma-appservice-api" version = "0.9.0" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "js_int", "ruma-common", @@ -2023,7 +2023,7 @@ dependencies = [ [[package]] name = "ruma-client-api" version = "0.17.4" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "as_variant", "assign", @@ -2042,7 +2042,7 @@ dependencies = [ [[package]] name = "ruma-common" version = "0.12.1" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "as_variant", "base64", @@ -2061,16 +2061,18 @@ dependencies = [ "serde_html_form", "serde_json", "thiserror", + "time", "tracing", "url", "uuid", + "web-time", "wildmatch", ] [[package]] name = "ruma-events" version = "0.27.11" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "as_variant", "indexmap 2.2.5", @@ -2092,7 +2094,7 @@ dependencies = [ [[package]] name = "ruma-federation-api" version = "0.8.0" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "js_int", "ruma-common", @@ -2104,7 +2106,7 @@ dependencies = [ [[package]] name = "ruma-identifiers-validation" version = "0.9.3" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "js_int", "thiserror", @@ -2113,7 +2115,7 @@ dependencies = [ [[package]] name = "ruma-identity-service-api" version = "0.8.0" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "js_int", "ruma-common", @@ -2123,7 +2125,7 @@ dependencies = [ [[package]] name = "ruma-macros" version = "0.12.0" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "once_cell", "proc-macro-crate", @@ -2138,7 +2140,7 @@ dependencies = [ [[package]] name = "ruma-push-gateway-api" version = "0.8.0" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "js_int", "ruma-common", @@ -2150,7 +2152,7 @@ dependencies = [ [[package]] name = "ruma-signatures" version = "0.14.0" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "base64", "ed25519-dalek", @@ -2166,7 +2168,7 @@ dependencies = [ [[package]] name = "ruma-state-res" version = "0.10.0" -source = "git+https://github.com/ruma/ruma?rev=1a1c61ee1e8f0936e956a3b69c931ce12ee28475#1a1c61ee1e8f0936e956a3b69c931ce12ee28475" +source = "git+https://github.com/ruma/ruma?rev=5495b85aa311c2805302edb0a7de40399e22b397#5495b85aa311c2805302edb0a7de40399e22b397" dependencies = [ "itertools 0.11.0", "js_int", @@ -3276,6 +3278,16 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "web-time" +version = "1.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5a6580f308b1fad9207618087a65c04e7a10bc77e02c8e84e9b00dd4b12fa0bb" +dependencies = [ + "js-sys", + "wasm-bindgen", +] + [[package]] name = "weezl" version = "0.1.8" diff --git a/Cargo.toml b/Cargo.toml index ac334728..d2b828ae 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,7 +37,7 @@ tower-http = { version = "0.4.1", features = ["add-extension", "cors", "sensitiv # Used for matrix spec type definitions and helpers #ruma = { version = "0.4.0", features = ["compat", "rand", "appservice-api-c", "client-api", "federation-api", "push-gateway-api-c", "state-res", "unstable-pre-spec", "unstable-exhaustive-types"] } -ruma = { git = "https://github.com/ruma/ruma", rev = "1a1c61ee1e8f0936e956a3b69c931ce12ee28475", features = ["compat", "rand", "appservice-api-c", "client-api", "federation-api", "push-gateway-api-c", "state-res", "unstable-msc2448", "unstable-msc3575", "unstable-exhaustive-types", "ring-compat", "unstable-unspecified" ] } +ruma = { git = "https://github.com/ruma/ruma", rev = "5495b85aa311c2805302edb0a7de40399e22b397", features = ["compat", "rand", "appservice-api-c", "client-api", "federation-api", "push-gateway-api-c", "state-res", "unstable-msc2448", "unstable-msc3575", "unstable-exhaustive-types", "ring-compat", "unstable-unspecified" ] } #ruma = { git = "https://github.com/timokoesters/ruma", rev = "4ec9c69bb7e09391add2382b3ebac97b6e8f4c64", features = ["compat", "rand", "appservice-api-c", "client-api", "federation-api", "push-gateway-api-c", "state-res", "unstable-msc2448", "unstable-msc3575", "unstable-exhaustive-types", "ring-compat", "unstable-unspecified" ] } #ruma = { path = "../ruma/crates/ruma", features = ["compat", "rand", "appservice-api-c", "client-api", "federation-api", "push-gateway-api-c", "state-res", "unstable-msc2448", "unstable-msc3575", "unstable-exhaustive-types", "ring-compat", "unstable-unspecified" ] } diff --git a/src/api/ruma_wrapper/axum.rs b/src/api/ruma_wrapper/axum.rs index a5ef7f7e..43e73c66 100644 --- a/src/api/ruma_wrapper/axum.rs +++ b/src/api/ruma_wrapper/axum.rs @@ -15,13 +15,20 @@ use bytes::{Buf, BufMut, Bytes, BytesMut}; 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, warn}; use super::{Ruma, RumaResponse}; -use crate::{services, Error, Result}; +use crate::{service::appservice::RegistrationInfo, services, Error, Result}; + +enum Token { + Appservice(Box), + User((OwnedUserId, OwnedDeviceId)), + Invalid, + None, +} #[async_trait] impl FromRequest for Ruma @@ -78,177 +85,194 @@ where None => query_params.access_token.as_deref(), }; - let mut json_body = serde_json::from_slice::(&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.clone())) + } 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 mut json_body = serde_json::from_slice::(&body).ok(); + 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( + match (metadata.authentication, token) { + (_, Token::Invalid) => { + return Err(Error::BadRequest( + ErrorKind::UnknownToken { soft_logout: false }, + "Unknown access token.", + )) + } + ( + AuthScheme::AccessToken + | AuthScheme::AppserviceToken + | AuthScheme::AccessTokenOptional, + 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(), ) - .unwrap() }, - |s| UserId::parse(s).unwrap(), - ); + 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.", + )); + } - if !services().users.exists(&user_id).unwrap() { + // 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, + 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.")); + } + + let TypedHeader(Authorization(x_matrix)) = parts + .extract::>>() + .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", + }; + + Error::BadRequest(ErrorKind::Forbidden, msg) + })?; + + let origin_signatures = BTreeMap::from_iter([( + x_matrix.key.clone(), + CanonicalJsonValue::String(x_matrix.sig), + )]); + + 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( + services().globals.server_name().as_str().to_owned(), + ), + ), + ( + "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(&x_matrix.origin, vec![x_matrix.key.to_owned()]) + .await; + + let keys = match keys_result { + Ok(b) => b, + Err(e) => { + warn!("Failed to fetch signing keys: {}", e); return Err(Error::BadRequest( ErrorKind::Forbidden, - "User does not exist.", + "Failed to fetch signing keys.", )); } + }; - // TODO: Check if appservice is allowed to be that user - (Some(user_id), None, None, true) - } - AuthScheme::ServerSignatures => (None, None, None, true), - AuthScheme::None => (None, None, None, true), - } - } else { - match metadata.authentication { - AuthScheme::AccessToken => { - let token = match token { - Some(token) => token, - _ => { - return Err(Error::BadRequest( - ErrorKind::MissingToken, - "Missing access token.", - )) - } - }; + let pub_key_map = + BTreeMap::from_iter([(x_matrix.origin.as_str().to_owned(), keys)]); - match services().users.find_from_token(token).unwrap() { - 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::ServerSignatures => { - if !services().globals.allow_federation() { - return Err(Error::bad_config("Federation is disabled.")); - } + 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 + ); - let TypedHeader(Authorization(x_matrix)) = parts - .extract::>>() - .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", - }; - - Error::BadRequest(ErrorKind::Forbidden, msg) - })?; - - let origin_signatures = BTreeMap::from_iter([( - x_matrix.key.clone(), - CanonicalJsonValue::String(x_matrix.sig), - )]); - - 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( - services().globals.server_name().as_str().to_owned(), - ), - ), - ( - "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(&x_matrix.origin, vec![x_matrix.key.to_owned()]) - .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.", - )); - } - }; - - 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) => { + if parts.uri.to_string().contains('@') { 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 \ + "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.", - )); + ); } + + return Err(Error::BadRequest( + ErrorKind::Forbidden, + "Failed to verify X-Matrix signatures.", + )); } } - AuthScheme::None => (None, None, None, false), + } + ( + AuthScheme::None + | AuthScheme::AppserviceToken + | AuthScheme::AccessTokenOptional, + Token::None, + ) => (None, None, None, false), + ( + AuthScheme::ServerSignatures | AuthScheme::None, + Token::Appservice(_) | Token::User(_), + ) => { + return Err(Error::BadRequest( + ErrorKind::Unauthorized, + "Access tokens should not 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.", + )); } };