Merge branch 'error-appservice-token-auth' into 'next'

fix: reject requests with authentication when not used

Closes #430

See merge request famedly/conduit!608

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"

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" ] }
 

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<RegistrationInfo>),
+    User((OwnedUserId, OwnedDeviceId)),
+    Invalid,
+    None,
+}
 
 #[async_trait]
 impl<T, S, B> FromRequest<S, B> for Ruma<T>
@@ -78,177 +85,194 @@ where
             None => query_params.access_token.as_deref(),
         };
 
-        let mut json_body = serde_json::from_slice::<CanonicalJsonValue>(&body).ok();
+        let token = if let Some(token) = token {
-
+            if let Some(reg_info) = services().appservice.find_from_token(token).await {
-        let appservice_registration = if let Some(token) = token {
+                Token::Appservice(Box::new(reg_info.clone()))
-            services().appservice.find_from_token(token).await
+            } 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::<CanonicalJsonValue>(&body).ok();
+
         let (sender_user, sender_device, sender_servername, from_appservice) =
-            if let Some(info) = appservice_registration {
+            match (metadata.authentication, token) {
-                match metadata.authentication {
+                (_, Token::Invalid) => {
-                    AuthScheme::AccessToken => {
+                    return Err(Error::BadRequest(
-                        let user_id = query_params.user_id.map_or_else(
+                        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::<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",
+                            };
+
+                            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
+                    let pub_key_map =
-                        (Some(user_id), None, None, true)
+                        BTreeMap::from_iter([(x_matrix.origin.as_str().to_owned(), keys)]);
-                    }
-                    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.",
-                                ))
-                            }
-                        };
 
-                        match services().users.find_from_token(token).unwrap() {
+                    match ruma::signatures::verify_json(&pub_key_map, &request_map) {
-                            None => {
+                        Ok(()) => (None, None, Some(x_matrix.origin), false),
-                                return Err(Error::BadRequest(
+                        Err(e) => {
-                                    ErrorKind::UnknownToken { soft_logout: false },
+                            warn!(
-                                    "Unknown access token.",
+                                "Failed to verify json request from {}: {}\n{:?}",
-                                ))
+                                x_matrix.origin, e, request_map
-                            }
+                            );
-                            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."));
-                        }
 
-                        let TypedHeader(Authorization(x_matrix)) = parts
+                            if parts.uri.to_string().contains('@') {
-                            .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",
-                                };
-
-                                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) => {
                                 warn!(
-                                    "Failed to verify json request from {}: {}\n{:?}",
+                                    "Request uri contained '@' character. Make sure your \
-                                    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.",
-                                ));
                             }
+
+                            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.",
+                    ));
                 }
             };