fix(auth): check the scope and session token, canonicalize repeated headers, and time out stalled requests

Cargo.toml

@@ -59,8 +59,9 @@ futures = "0.3"
 # HTTP
 axum = { version = "0.8", features = ["macros"] }
 tower = { version = "0.5", features = ["util"] }
-tower-http = { version = "0.6", features = ["compression-gzip", "cors", "set-header"] }
+tower-http = { version = "0.6", features = ["compression-gzip", "cors", "set-header", "timeout"] }
 hyper = { version = "1" }
+hyper-util = { version = "0.1", features = ["tokio"] }
 # URI parsing for the backup store's endpoint check; the same major hyper and
 # the AWS SDK already use.
 http = { version = "1" }

crates/app/src/init_helpers.rs

@@ -247,9 +247,10 @@ backend = "{backend}"
 [server]
 bind_addr = "{bind_addr}"
 # port = 18443                   # HTTPS port
-# region = "us-east-1"           # AWS region for ARN generation
+# region = "us-east-1"           # AWS region for ARN generation; requests must be signed for this region
 # run_dir = "{run_dir}"          # Directory for PID file
 # throttling_enabled = false     # Enable provisioned throughput throttling
+# request_timeout_secs = 30      # Close a connection whose headers or body stall this long (1 to 86400)
 
 [server.tls]
 # TLS is mandatory. The server refuses to start with enabled = false.

crates/auth/src/lib.rs

@@ -104,18 +104,26 @@ pub trait CredentialStore: Send + Sync {
 
 /// `SigV4` auth provider with local credential store.
 ///
-/// Parses the `Authorization` header, looks up the access key, decrypts the
-/// secret, verifies the `SigV4` signature, and validates the request timestamp.
+/// Parses the `Authorization` header, validates the request timestamp, checks
+/// the credential scope, looks up the access key, decrypts the secret and
+/// verifies the `SigV4` signature, in that order (the service's order).
 /// Handles both long-lived (AKIA*) and temporary (ASIA* + X-Amz-Security-Token)
-/// credentials.
+/// credentials. The credential scope must name this server's region.
 pub struct BuiltinAuthProvider<C: CredentialStore> {
     credential_store: C,
+    region: String,
 }
 
 impl<C: CredentialStore> BuiltinAuthProvider<C> {
-    /// Create a new `BuiltinAuthProvider` with the given credential store.
-    pub fn new(credential_store: C) -> Self {
-        Self { credential_store }
+    /// Create a new `BuiltinAuthProvider` over `credential_store` for a server
+    /// deployed as `region`. A request whose credential scope names another
+    /// region is rejected, as the service rejects a scope for a region other
+    /// than the endpoint's.
+    pub fn new(credential_store: C, region: impl Into<String>) -> Self {
+        Self {
+            credential_store,
+            region: region.into(),
+        }
     }
 }
 
@@ -137,8 +145,22 @@ impl<C: CredentialStore + 'static> AuthProvider for BuiltinAuthProvider<C> {
         // Parse Authorization header
         let parsed = sigv4::parse::parse_authorization(auth_header)?;
 
-        // Look up credential before timestamp validation — real DynamoDB returns
-        // an invalid-key error even when the timestamp is also expired.
+        // The service checks the request timestamp, then the credential
+        // scope, then looks the key up (measured 2026-10-23: an unknown key
+        // with a stale date answers the timestamp error, an unknown key with
+        // a wrong region answers the scope message, and a stale date with a
+        // wrong region answers the timestamp error). Neither check reads the
+        // stored credential, so running them first leaves the S-5 timing
+        // argument below unchanged.
+        //
+        // Validate timestamp (±15 minute window).
+        sigv4::verify::validate_timestamp(headers)?;
+
+        // The credential scope must name this server's region and the
+        // dynamodb service; the service reports both complaints in one
+        // message when both are wrong.
+        sigv4::verify::check_scope(&parsed, &self.region)?;
+
         let credential = self
             .credential_store
             .lookup_credential(&parsed.access_key_id)
@@ -150,15 +172,14 @@ impl<C: CredentialStore + 'static> AuthProvider for BuiltinAuthProvider<C> {
             })?;
 
         // S-5: Track inactive status but do NOT return early. Continue through
-        // timestamp validation and signature verification to ensure constant-time
-        // failure paths — no timing difference between inactive, invalid, and
-        // absent keys.
+        // signature verification to ensure constant-time failure paths — no
+        // timing difference between inactive, invalid, and absent keys.
         let is_inactive = !credential.is_active;
 
-        // Validate timestamp (±15 minute window) after credential lookup.
-        sigv4::verify::validate_timestamp(headers)?;
-
-        // For session credentials, verify X-Amz-Security-Token matches the stored token.
+        // Session tokens. For a session credential the presented token must
+        // match the stored one. For a long-term key no token may be presented:
+        // the service validates any presented token and a long-term key has
+        // none, so the request fails as an invalid token.
         // CB-12: Session expiration is enforced at the credential store layer (fail-closed).
         // Expired sessions are never returned — the storage layer returns
         // ExpiredTokenException on the cache-miss path, and CachedCredentialStore
@@ -167,15 +188,19 @@ impl<C: CredentialStore + 'static> AuthProvider for BuiltinAuthProvider<C> {
         // S-5: Compare the session token in constant time and defer the
         // rejection until after signature verification, so neither the token's
         // byte contents nor its presence changes the failure-path timing.
+        // A header value that is not visible ASCII cannot be compared with
+        // the stored token, and it is still a token the client presented, so
+        // presence is read from the header map and the value from its text.
+        let token_header_present = headers.contains_key("x-amz-security-token");
+        let presented_token = headers
+            .get("x-amz-security-token")
+            .and_then(|v| v.to_str().ok());
         let session_token_ok = if credential.is_session {
-            let token = headers
-                .get("x-amz-security-token")
-                .and_then(|v| v.to_str().ok())
-                .unwrap_or("");
+            let token = presented_token.unwrap_or("");
             let expected = credential.session_token.as_deref().unwrap_or("");
             sigv4::verify::constant_time_eq(token.as_bytes(), expected.as_bytes())
         } else {
-            true
+            !token_header_present
         };
 
         // Verify SigV4 signature — always, even for inactive keys (S-5).
@@ -191,11 +216,19 @@ impl<C: CredentialStore + 'static> AuthProvider for BuiltinAuthProvider<C> {
 
         // S-5: Reject inactive keys or a mismatched session token only after
         // full signature verification, to prevent timing side-channels.
-        if is_inactive || !session_token_ok {
+        // The service's message for a presented token that does not resolve
+        // has no trailing period; its message for an unknown key or a missing
+        // token does (both measured 2026-09-16).
+        if is_inactive || (credential.is_session && presented_token.is_none()) {
             return Err(DynamoDbError::UnrecognizedClientException(
                 "The security token included in the request is invalid.".to_owned(),
             ));
         }
+        if !session_token_ok {
+            return Err(DynamoDbError::UnrecognizedClientException(
+                "The security token included in the request is invalid".to_owned(),
+            ));
+        }
 
         // Build identity from credential (clone fields because ZeroizeOnDrop
         // prevents moving out of the struct).
@@ -215,112 +248,4 @@ impl<C: CredentialStore + 'static> AuthProvider for BuiltinAuthProvider<C> {
 }
 
 #[cfg(test)]
-mod tests {
-    use super::*;
-    use axum::http::{HeaderMap, HeaderValue};
-
-    /// In-memory credential store for unit tests.
-    struct MockCredentialStore {
-        /// `Some(credential)` for found credentials, `None` for not found.
-        credential: Option<StoredCredential>,
-        /// If set, `lookup_credential` returns this error instead.
-        error: Option<&'static str>,
-    }
-
-    #[async_trait::async_trait]
-    impl CredentialStore for MockCredentialStore {
-        async fn lookup_credential(
-            &self,
-            _access_key_id: &str,
-        ) -> Result<Option<StoredCredential>, DynamoDbError> {
-            if let Some(msg) = self.error {
-                return Err(DynamoDbError::ExpiredTokenException(msg.to_owned()));
-            }
-            Ok(self.credential.clone())
-        }
-    }
-
-    fn make_headers_with_auth(access_key: &str, token: Option<&str>) -> HeaderMap {
-        let mut headers = HeaderMap::new();
-        // Use current time for the date to pass timestamp validation.
-        let now = time::OffsetDateTime::now_utc();
-        let date_str = format!(
-            "{:04}{:02}{:02}T{:02}{:02}{:02}Z",
-            now.year(),
-            now.month() as u8,
-            now.day(),
-            now.hour(),
-            now.minute(),
-            now.second(),
-        );
-        let date_short = &date_str[..8];
-        // Signature is fake — these tests exercise error paths (expired token,
-        // unknown key) that trigger before signature verification in
-        // authenticate().
-        let auth = format!(
-            "AWS4-HMAC-SHA256 Credential={}/{}/us-east-1/dynamodb/aws4_request, \
-             SignedHeaders=content-type;host;x-amz-date;x-amz-target, \
-             Signature=0000000000000000000000000000000000000000000000000000000000000000",
-            access_key, date_short
-        );
-        headers.insert("authorization", HeaderValue::from_str(&auth).unwrap());
-        headers.insert("x-amz-date", HeaderValue::from_str(&date_str).unwrap());
-        headers.insert("host", HeaderValue::from_static("localhost:18443"));
-        headers.insert(
-            "content-type",
-            HeaderValue::from_static("application/x-amz-json-1.0"),
-        );
-        headers.insert(
-            "x-amz-target",
-            HeaderValue::from_static("DynamoDB_20120810.ListTables"),
-        );
-        if let Some(t) = token {
-            headers.insert("x-amz-security-token", HeaderValue::from_str(t).unwrap());
-        }
-        headers
-    }
-
-    #[tokio::test]
-    async fn expired_session_returns_expired_token_exception() {
-        // CB-12: The credential store returns ExpiredTokenException directly
-        // for expired sessions (fail-closed). The auth layer propagates it.
-        let store = MockCredentialStore {
-            credential: None,
-            error: Some("The security token included in the request is expired"),
-        };
-        let provider = BuiltinAuthProvider::new(store);
-        let headers = make_headers_with_auth("ASIAEXTENDDB00000000", Some("test-token-value"));
-
-        let result = provider.authenticate(&headers, b"{}").await;
-        match result {
-            Err(DynamoDbError::ExpiredTokenException(msg)) => {
-                assert!(
-                    msg.contains("expired"),
-                    "Expected 'expired' in message: {msg}"
-                );
-            }
-            other => panic!("Expected ExpiredTokenException, got: {other:?}"),
-        }
-    }
-
-    #[tokio::test]
-    async fn nonexistent_key_returns_unrecognized_client() {
-        let store = MockCredentialStore {
-            credential: None,
-            error: None,
-        };
-        let provider = BuiltinAuthProvider::new(store);
-        let headers = make_headers_with_auth("AKIAXXXXXXXXXXXXXXXX", None);
-
-        let result = provider.authenticate(&headers, b"{}").await;
-        match result {
-            Err(DynamoDbError::UnrecognizedClientException(msg)) => {
-                assert!(
-                    msg.contains("invalid"),
-                    "Expected 'invalid' in message: {msg}"
-                );
-            }
-            other => panic!("Expected UnrecognizedClientException, got: {other:?}"),
-        }
-    }
-}
+mod tests;

crates/auth/src/sigv4/canonical.rs

@@ -39,8 +39,9 @@ pub fn canonical_request(
     // trusting the header would let the sender choose the hash that the
     // signature is checked against. When the client includes the header in
     // SignedHeaders it is covered as an ordinary header by
-    // build_canonical_headers. UNSIGNED-PAYLOAD is rejected by
-    // verify_signature() before this function is called.
+    // build_canonical_headers. A client that signed a literal such as
+    // UNSIGNED-PAYLOAD in place of the hash fails as a plain signature
+    // mismatch, which is what the service returns.
     let payload_hash = sha256_hex(body);
 
     format!(
@@ -65,10 +66,11 @@ pub fn string_to_sign(timestamp: &str, scope: &str, canonical_request: &str) ->
 
 /// Build canonical headers from the signed header list.
 ///
-/// Per the `SigV4` spec, header names are lowercased, values are trimmed,
-/// and headers are sorted alphabetically. Each line ends with `\n`.
-/// CB-7: Header names are explicitly lowercased to handle clients that
-/// send mixed-case `SignedHeaders` values.
+/// Per the `SigV4` spec, header names are lowercased, values are trimmed with
+/// internal whitespace collapsed, and a header that appears more than once
+/// contributes its values joined by commas in the order they were sent.
+/// Each line ends with `\n`. CB-7: Header names are explicitly lowercased to
+/// handle clients that send mixed-case `SignedHeaders` values.
 fn build_canonical_headers(headers: &HeaderMap, signed_headers: &str) -> String {
     let mut result = String::new();
     // signed_headers is already sorted and semicolon-delimited.
@@ -77,27 +79,32 @@ fn build_canonical_headers(headers: &HeaderMap, signed_headers: &str) -> String
     // does not require pre-lowercased input.
     for name in signed_headers.split(';') {
         let lower = name.to_ascii_lowercase();
-        let value = headers
-            .get(lower.as_str())
-            .and_then(|v| v.to_str().ok())
-            .unwrap_or("");
+        let mut value = headers
+            .get_all(lower.as_str())
+            .iter()
+            // A value that is not visible ASCII cannot have been signed as
+            // anything; it contributes an empty value rather than disappearing,
+            // so a request carrying such a header never canonicalizes the same
+            // as one without it.
+            .map(|v| v.to_str().unwrap_or(""))
+            // Trim leading/trailing whitespace, collapse internal whitespace
+            .map(|v| v.split_whitespace().collect::<Vec<_>>().join(" "))
+            .collect::<Vec<_>>()
+            .join(",");
         // HTTP/2 uses :authority instead of Host. If the host header is empty
         // or missing, fall back to the :authority pseudo-header value.
         // Defense-in-depth: handler.rs injects Host from URI authority for
         // HTTP/2 requests, so this fallback should rarely activate.
-        let value = if lower == "host" && value.is_empty() {
-            headers
+        if lower == "host" && value.is_empty() {
+            value = headers
                 .get(":authority")
                 .and_then(|v| v.to_str().ok())
-                .unwrap_or(value)
-        } else {
-            value
-        };
-        // Trim leading/trailing whitespace, collapse internal whitespace
-        let trimmed = value.split_whitespace().collect::<Vec<_>>().join(" ");
+                .map(|v| v.split_whitespace().collect::<Vec<_>>().join(" "))
+                .unwrap_or_default();
+        }
         result.push_str(&lower);
         result.push(':');
-        result.push_str(&trimmed);
+        result.push_str(&value);
         result.push('\n');
     }
     result
@@ -202,4 +209,46 @@ mod tests {
         assert_eq!(lines[8], sha256_hex(received_body));
         assert_ne!(lines[8], sha256_hex(signed_body));
     }
+
+    /// A header sent more than once contributes its values joined by commas,
+    /// which is how the service canonicalizes it.
+    #[test]
+    fn repeated_header_values_are_comma_joined_in_order() {
+        let mut headers = HeaderMap::new();
+        headers.insert("host", "localhost:18443".parse().unwrap());
+        headers.insert("x-amz-date", "20260415T120000Z".parse().unwrap());
+        headers.append("x-amz-meta-dup", " a ".parse().unwrap());
+        headers.append("x-amz-meta-dup", "b  c".parse().unwrap());
+
+        let creq = canonical_request(
+            "POST",
+            "/",
+            "",
+            &headers,
+            "host;x-amz-date;x-amz-meta-dup",
+            b"{}",
+        );
+
+        let lines: Vec<&str> = creq.split('\n').collect();
+        assert_eq!(lines[5], "x-amz-meta-dup:a,b c");
+    }
+
+    /// A duplicate value that is not visible ASCII cannot be signed as anything.
+    /// It must still change the canonical form, so a request carrying it does not
+    /// verify under a signature made without it.
+    #[test]
+    fn unsignable_duplicate_value_changes_the_canonical_form() {
+        let mut plain = HeaderMap::new();
+        plain.insert("host", "localhost:18443".parse().unwrap());
+        plain.insert("x-amz-meta-dup", "a".parse().unwrap());
+        let mut with_extra = plain.clone();
+        with_extra.append(
+            "x-amz-meta-dup",
+            axum::http::HeaderValue::from_bytes(b"\xff\xfe").unwrap(),
+        );
+        let a = canonical_request("POST", "/", "", &plain, "host;x-amz-meta-dup", b"{}");
+        let b = canonical_request("POST", "/", "", &with_extra, "host;x-amz-meta-dup", b"{}");
+        assert_ne!(a, b);
+        assert!(b.contains("x-amz-meta-dup:a,\n"), "{b}");
+    }
 }