server-util: Use http-auth crate to parse XMatrix

crates/ruma-server-util/CHANGELOG.md

@@ -7,9 +7,16 @@ Breaking changes:
 - The `sig` field in `XMatrix` has been changed from `String` to `Base64` to more accurately
   mirror its allowed values in the type system.
 
-Improvements:
+Bug fixes:
 
 - When encoding to a header value, `XMatrix` fields are now quoted and escaped correctly.
+- Use http-auth crate to parse `XMatrix`. Allows to parse the Authorization HTTP
+  header with full compatibility with RFC 7235
+
+Improvements:
+
+- Implement `Display`, `FromStr` and conversion to/from `http::HeaderValue` for
+  `XMatrix`
 
 # 0.3.0
 

crates/ruma-server-util/Cargo.toml

@@ -16,9 +16,11 @@ all-features = true
 
 [dependencies]
 headers = "0.4.0"
+http = { workspace = true }
+http-auth = { version = "0.1.9", default-features = false }
 ruma-common = { workspace = true }
+thiserror = { workspace = true }
 tracing = { workspace = true }
-yap = "0.12.0"
 
 [dev-dependencies]
 tracing-subscriber = "0.3.16"

crates/ruma-server-util/src/authorization.rs

@@ -1,26 +1,32 @@
 //! Common types for implementing federation authorization.
 
-use std::borrow::Cow;
+use std::{borrow::Cow, fmt, str::FromStr};
 
-use headers::{authorization::Credentials, HeaderValue};
+use headers::authorization::Credentials;
-use ruma_common::{serde::Base64, OwnedServerName, OwnedServerSigningKeyId};
+use http::HeaderValue;
+use http_auth::ChallengeParser;
+use ruma_common::{
+    serde::{Base64, Base64DecodeError},
+    IdParseError, OwnedServerName, OwnedServerSigningKeyId,
+};
+use thiserror::Error;
 use tracing::debug;
-use yap::{IntoTokens, TokenLocation, Tokens};
 
 /// Typed representation of an `Authorization` header of scheme `X-Matrix`, as defined in the
-/// [Matrix Server-Server API][spec]. Includes an implementation of
+/// [Matrix Server-Server API][spec].
-/// [`headers::authorization::Credentials`] for automatically handling the encoding and decoding
-/// when using a web framework that supports typed headers.
 ///
 /// [spec]: https://spec.matrix.org/latest/server-server-api/#request-authentication
+#[derive(Clone)]
 #[non_exhaustive]
 pub struct XMatrix {
     /// The server name of the sending server.
     pub origin: OwnedServerName,
-    /// The server name of the receiving sender. For compatibility with older servers, recipients
+    /// The server name of the receiving sender.
-    /// should accept requests without this parameter, but MUST always send it. If this property is
+    ///
-    /// included, but the value does not match the receiving server's name, the receiving server
+    /// For compatibility with older servers, recipients should accept requests without this
-    /// must deny the request with an HTTP status code 401 Unauthorized.
+    /// parameter, but MUST always send it. If this property is included, but the value does
+    /// not match the receiving server's name, the receiving server must deny the request with
+    /// an HTTP status code 401 Unauthorized.
     pub destination: Option<OwnedServerName>,
     /// The ID - including the algorithm name - of the sending server's key that was used to sign
     /// the request.
@@ -39,190 +45,94 @@ impl XMatrix {
     ) -> Self {
         Self { origin, destination: Some(destination), key, sig }
     }
+
+    /// Parse an X-Matrix Authorization header from the given string.
+    pub fn parse(s: impl AsRef<str>) -> Result<Self, XMatrixParseError> {
+        let parser = ChallengeParser::new(s.as_ref());
+        let mut xmatrix = None;
+
+        for challenge in parser {
+            let challenge = challenge?;
+
+            if challenge.scheme.eq_ignore_ascii_case(XMatrix::SCHEME) {
+                xmatrix = Some(challenge);
+                break;
+            }
         }
 
-fn parse_token<'a>(tokens: &mut impl Tokens<Item = &'a u8>) -> Option<Vec<u8>> {
+        let Some(xmatrix) = xmatrix else {
-    tokens.optional(|t| {
+            return Err(XMatrixParseError::NotFound);
-        let token: Vec<u8> = t.take_while(|c| is_tchar(**c)).as_iter().copied().collect();
+        };
-        if !token.is_empty() {
-            Some(token)
-        } else {
-            debug!("Returning early because of empty token at {}", t.location().offset());
-            None
-        }
-    })
-}
 
-// Matrix spec:
-// > For compatibility with older servers, the recipient should allow colons to be included in
-// > values without requiring the value to be enclosed in quotes.
-fn parse_token_with_colons<'a>(tokens: &mut impl Tokens<Item = &'a u8>) -> Option<Vec<u8>> {
-    tokens.optional(|t| {
-        let token: Vec<u8> =
-            t.take_while(|c| is_tchar(**c) || **c == b':').as_iter().copied().collect();
-        if !token.is_empty() {
-            Some(token)
-        } else {
-            debug!("Returning early because of empty token at {}", t.location().offset());
-            None
-        }
-    })
-}
-
-fn parse_quoted<'a>(tokens: &mut impl Tokens<Item = &'a u8>) -> Option<Vec<u8>> {
-    tokens.optional(|t| {
-        if !(t.token(&b'"')) {
-            return None;
-        }
-        let mut buffer = Vec::new();
-        loop {
-            match t.next()? {
-                // quoted pair
-                b'\\' => {
-                    let escaped = t.next().filter(|c| {
-                        if is_quoted_pair(**c) {
-                            true
-                        } else {
-                            debug!(
-                                "Encountered an illegal character {} at location {}",
-                                **c as char,
-                                t.location().offset()
-                            );
-                            false
-                        }
-                    })?;
-                    buffer.push(*escaped);
-                }
-                // end of quote
-                b'"' => break,
-                // regular character
-                c if is_qdtext(*c) => buffer.push(*c),
-                // Invalid character
-                c => {
-                    debug!(
-                        "Encountered an illegal character {} at location {}",
-                        *c as char,
-                        t.location().offset()
-                    );
-                    return None;
-                }
-            }
-        }
-        Some(buffer)
-    })
-}
-
-fn parse_xmatrix_field<'a>(tokens: &mut impl Tokens<Item = &'a u8>) -> Option<(String, Vec<u8>)> {
-    tokens.optional(|t| {
-        let name = parse_token(t).and_then(|name| {
-            let name = std::str::from_utf8(&name).ok()?.to_ascii_lowercase();
-            match name.as_str() {
-                "origin" | "destination" | "key" | "sig" => Some(name),
-                name => {
-                    debug!(
-                        "Encountered an invalid field name {} at location {}",
-                        name,
-                        t.location().offset()
-                    );
-                    None
-                }
-            }
-        })?;
-
-        if !t.token(&b'=') {
-            return None;
-        }
-
-        let value = parse_quoted(t).or_else(|| parse_token_with_colons(t))?;
-
-        Some((name, value))
-    })
-}
-
-fn parse_xmatrix<'a>(tokens: &mut impl Tokens<Item = &'a u8>) -> Option<XMatrix> {
-    tokens.optional(|t| {
-        if !t.tokens(b"X-Matrix ") {
-            debug!("Failed to parse X-Matrix credentials, didn't start with 'X-Matrix '");
-            return None;
-        }
         let mut origin = None;
         let mut destination = None;
         let mut key = None;
         let mut sig = None;
 
-        for (name, value) in t.sep_by(|t| parse_xmatrix_field(t), |t| t.token(&b',')).as_iter() {
+        for (name, value) in xmatrix.params {
-            match name.as_str() {
+            if name.eq_ignore_ascii_case("origin") {
-                "origin" => {
                 if origin.is_some() {
-                        debug!("Field origin duplicated in X-Matrix Authorization header");
+                    return Err(XMatrixParseError::DuplicateParameter("origin".to_owned()));
+                } else {
+                    origin = Some(OwnedServerName::try_from(value.to_unescaped())?);
                 }
-                    origin = Some(std::str::from_utf8(&value).ok()?.try_into().ok()?);
+            } else if name.eq_ignore_ascii_case("destination") {
-                }
-                "destination" => {
                 if destination.is_some() {
-                        debug!("Field destination duplicated in X-Matrix Authorization header");
+                    return Err(XMatrixParseError::DuplicateParameter("destination".to_owned()));
+                } else {
+                    destination = Some(OwnedServerName::try_from(value.to_unescaped())?);
                 }
-                    destination = Some(std::str::from_utf8(&value).ok()?.try_into().ok()?);
+            } else if name.eq_ignore_ascii_case("key") {
-                }
-                "key" => {
                 if key.is_some() {
-                        debug!("Field key duplicated in X-Matrix Authorization header");
+                    return Err(XMatrixParseError::DuplicateParameter("key".to_owned()));
+                } else {
+                    key = Some(OwnedServerSigningKeyId::try_from(value.to_unescaped())?);
                 }
-                    key = Some(std::str::from_utf8(&value).ok()?.try_into().ok()?);
+            } else if name.eq_ignore_ascii_case("sig") {
-                }
-                "sig" => {
                 if sig.is_some() {
-                        debug!("Field sig duplicated in X-Matrix Authorization header");
+                    return Err(XMatrixParseError::DuplicateParameter("sig".to_owned()));
-                    }
+                } else {
-                    sig = Some(Base64::parse(&value).ok()?);
+                    sig = Some(Base64::parse(value.to_unescaped())?);
-                }
-                name => {
-                    debug!("Unknown field {} found in X-Matrix Authorization header", name);
                 }
+            } else {
+                debug!("Unknown parameter {name} in X-Matrix Authorization header");
             }
         }
 
-        Some(XMatrix { origin: origin?, destination, key: key?, sig: sig? })
+        Ok(Self {
+            origin: origin
+                .ok_or_else(|| XMatrixParseError::MissingParameter("origin".to_owned()))?,
+            destination,
+            key: key.ok_or_else(|| XMatrixParseError::MissingParameter("key".to_owned()))?,
+            sig: sig.ok_or_else(|| XMatrixParseError::MissingParameter("sig".to_owned()))?,
         })
     }
-
-fn is_alpha(c: u8) -> bool {
-    (0x41..=0x5A).contains(&c) || (0x61..=0x7A).contains(&c)
 }
 
-fn is_digit(c: u8) -> bool {
+impl fmt::Debug for XMatrix {
-    (0x30..=0x39).contains(&c)
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("XMatrix")
+            .field("origin", &self.origin)
+            .field("destination", &self.destination)
+            .field("key", &self.key)
+            .finish_non_exhaustive()
+    }
 }
 
-fn is_tchar(c: u8) -> bool {
+/// Whether the given char is a [token char].
-    const TOKEN_CHARS: [u8; 15] =
+///
-        [b'!', b'#', b'$', b'%', b'&', b'\'', b'*', b'+', b'-', b'.', b'^', b'_', b'`', b'|', b'~'];
+/// [token char]: https://www.rfc-editor.org/rfc/rfc9110#section-5.6.2
-    is_alpha(c) || is_digit(c) || TOKEN_CHARS.contains(&c)
+fn is_tchar(c: char) -> bool {
+    const TOKEN_CHARS: [char; 15] =
+        ['!', '#', '$', '%', '&', '\'', '*', '+', '-', '.', '^', '_', '`', '|', '~'];
+    c.is_ascii_alphanumeric() || TOKEN_CHARS.contains(&c)
 }
 
-fn is_qdtext(c: u8) -> bool {
+/// If the field value does not contain only token chars, convert it to a [quoted string].
-    c == b'\t'
+///
-        || c == b' '
+/// [quoted string]: https://www.rfc-editor.org/rfc/rfc9110#section-5.6.4
-        || c == 0x21
+fn escape_field_value(value: &str) -> Cow<'_, str> {
-        || (0x23..=0x5B).contains(&c)
+    if !value.is_empty() && value.chars().all(is_tchar) {
-        || (0x5D..=0x7E).contains(&c)
-        || is_obs_text(c)
-}
-
-fn is_obs_text(c: u8) -> bool {
-    c >= 0x80 // The spec does contain an upper limit of 0xFF here, but that's enforced by the type
-}
-
-fn is_vchar(c: u8) -> bool {
-    (0x21..=0x7E).contains(&c)
-}
-
-fn is_quoted_pair(c: u8) -> bool {
-    c == b'\t' || c == b' ' || is_vchar(c) || is_obs_text(c)
-}
-
-fn escape_value(value: &str) -> Cow<'_, str> {
-    if !value.is_empty() && value.chars().all(|c| u8::try_from(c).is_ok_and(is_tchar)) {
         return Cow::Borrowed(value);
     }
 
@@ -230,28 +140,96 @@ fn escape_value(value: &str) -> Cow<'_, str> {
     Cow::Owned(format!("\"{value}\""))
 }
 
+impl fmt::Display for XMatrix {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let Self { origin, destination, key, sig } = self;
+
+        let origin = escape_field_value(origin.as_str());
+        let key = escape_field_value(key.as_str());
+        let sig = sig.encode();
+        let sig = escape_field_value(&sig);
+
+        write!(f, r#"{} "#, Self::SCHEME)?;
+
+        if let Some(destination) = destination {
+            let destination = escape_field_value(destination.as_str());
+            write!(f, r#"destination={destination},"#)?;
+        }
+
+        write!(f, "key={key},origin={origin},sig={sig}")
+    }
+}
+
+impl FromStr for XMatrix {
+    type Err = XMatrixParseError;
+
+    fn from_str(s: &str) -> Result<Self, Self::Err> {
+        Self::parse(s)
+    }
+}
+
+impl TryFrom<&HeaderValue> for XMatrix {
+    type Error = XMatrixParseError;
+
+    fn try_from(value: &HeaderValue) -> Result<Self, Self::Error> {
+        Self::parse(value.to_str()?)
+    }
+}
+
+impl From<&XMatrix> for HeaderValue {
+    fn from(value: &XMatrix) -> Self {
+        value.to_string().try_into().expect("header format is static")
+    }
+}
+
 impl Credentials for XMatrix {
     const SCHEME: &'static str = "X-Matrix";
 
     fn decode(value: &HeaderValue) -> Option<Self> {
-        let value: Vec<u8> = value.as_bytes().to_vec();
+        value.try_into().ok()
-        parse_xmatrix(&mut value.into_tokens())
     }
 
     fn encode(&self) -> HeaderValue {
-        let origin = escape_value(self.origin.as_str());
+        self.into()
-        let key = escape_value(self.key.as_str());
-        let sig = self.sig.encode();
-        let sig = escape_value(&sig);
-
-        if let Some(destination) = &self.destination {
-            let destination = escape_value(destination.as_str());
-            format!("X-Matrix origin={origin},destination={destination},key={key},sig={sig}")
-        } else {
-            format!("X-Matrix origin={origin},key={key},sig={sig}")
     }
-        .try_into()
+}
-        .expect("header format is static")
+
+/// An error when trying to parse an X-Matrix Authorization header.
+#[derive(Debug, Error)]
+#[non_exhaustive]
+pub enum XMatrixParseError {
+    /// The `HeaderValue` could not be converted to a `str`.
+    #[error(transparent)]
+    ToStr(#[from] http::header::ToStrError),
+
+    /// The string could not be parsed as a valid Authorization string.
+    #[error("{0}")]
+    ParseStr(String),
+
+    /// The credentials with the X-Matrix scheme were not found.
+    #[error("X-Matrix credentials not found")]
+    NotFound,
+
+    /// The parameter value could not be parsed as a Matrix ID.
+    #[error(transparent)]
+    ParseId(#[from] IdParseError),
+
+    /// The parameter value could not be parsed as base64.
+    #[error(transparent)]
+    ParseBase64(#[from] Base64DecodeError),
+
+    /// The parameter with the given name was not found.
+    #[error("missing parameter '{0}'")]
+    MissingParameter(String),
+
+    /// The parameter with the given name was found more than once.
+    #[error("duplicate parameter '{0}'")]
+    DuplicateParameter(String),
+}
+
+impl<'a> From<http_auth::parser::Error<'a>> for XMatrixParseError {
+    fn from(value: http_auth::parser::Error<'a>) -> Self {
+        Self::ParseStr(value.to_string())
     }
 }
 
@@ -270,7 +248,7 @@ mod tests {
         let origin = "origin.hs.example.com".try_into().unwrap();
         let key = "ed25519:key1".try_into().unwrap();
         let sig = Base64::new(b"test".to_vec());
-        let credentials: XMatrix = Credentials::decode(&header).unwrap();
+        let credentials = XMatrix::try_from(&header).unwrap();
         assert_eq!(credentials.origin, origin);
         assert_eq!(credentials.destination, None);
         assert_eq!(credentials.key, key);
@@ -280,7 +258,7 @@ mod tests {
 
         assert_eq!(
             credentials.encode(),
-            "X-Matrix origin=origin.hs.example.com,key=\"ed25519:key1\",sig=dGVzdA"
+            "X-Matrix key=\"ed25519:key1\",origin=origin.hs.example.com,sig=dGVzdA"
         );
     }
 
@@ -291,7 +269,7 @@ mod tests {
         let destination: OwnedServerName = "destination.hs.example.com".try_into().unwrap();
         let key = "ed25519:key1".try_into().unwrap();
         let sig = Base64::new(b"test".to_vec());
-        let credentials: XMatrix = Credentials::decode(&header).unwrap();
+        let credentials = XMatrix::try_from(&header).unwrap();
         assert_eq!(credentials.origin, origin);
         assert_eq!(credentials.destination, Some(destination.clone()));
         assert_eq!(credentials.key, key);
@@ -299,19 +277,19 @@ mod tests {
 
         let credentials = XMatrix::new(origin, destination, key, sig);
 
-        assert_eq!(credentials.encode(), "X-Matrix origin=origin.hs.example.com,destination=destination.hs.example.com,key=\"ed25519:key1\",sig=dGVzdA");
+        assert_eq!(credentials.encode(), "X-Matrix destination=destination.hs.example.com,key=\"ed25519:key1\",origin=origin.hs.example.com,sig=dGVzdA");
     }
 
     #[test]
     fn xmatrix_quoting() {
         let header = HeaderValue::from_static(
-            r#"X-Matrix origin=example.com:1234,key="abc\"def\\:ghi",sig=dGVzdA,"#,
+            r#"X-Matrix origin="example.com:1234",key="abc\"def\\:ghi",sig=dGVzdA,"#,
         );
 
         let origin: OwnedServerName = "example.com:1234".try_into().unwrap();
         let key = r#"abc"def\:ghi"#.try_into().unwrap();
         let sig = Base64::new(b"test".to_vec());
-        let credentials: XMatrix = Credentials::decode(&header).unwrap();
+        let credentials = XMatrix::try_from(&header).unwrap();
         assert_eq!(credentials.origin, origin);
         assert_eq!(credentials.destination, None);
         assert_eq!(credentials.key, key);
@@ -321,7 +299,19 @@ mod tests {
 
         assert_eq!(
             credentials.encode(),
-            r#"X-Matrix origin="example.com:1234",key="abc\"def\\:ghi",sig=dGVzdA"#
+            r#"X-Matrix key="abc\"def\\:ghi",origin="example.com:1234",sig=dGVzdA"#
         );
     }
+
+    #[test]
+    fn xmatrix_auth_1_3_with_extra_spaces() {
+        let header = HeaderValue::from_static("X-Matrix origin=\"origin.hs.example.com\"  ,     destination=\"destination.hs.example.com\",key=\"ed25519:key1\", sig=\"dGVzdA\"");
+        let credentials = XMatrix::try_from(&header).unwrap();
+        let sig = Base64::new(b"test".to_vec());
+
+        assert_eq!(credentials.origin, "origin.hs.example.com");
+        assert_eq!(credentials.destination.unwrap(), "destination.hs.example.com");
+        assert_eq!(credentials.key, "ed25519:key1");
+        assert_eq!(credentials.sig, sig);
+    }
 }