fix(node): require Content-Digest on a signed request

crates/gitlawb-core/src/http_sig.rs

@@ -14,7 +14,7 @@
 use base64::{engine::general_purpose::STANDARD, Engine};
 use chrono::Utc;
 use sha2::{Digest, Sha256};
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 
 use crate::did::Did;
 use crate::identity::Keypair;
@@ -67,11 +67,20 @@ impl HttpSignature {
         let components_str = &rest[open + 1..close];
         let params_str = &rest[close + 1..]; // starts with ';'
 
-        // "\"@method\" \"@path\" \"content-digest\"" → ["@method", "@path", "content-digest"]
-        let components: Vec<String> = components_str
-            .split_whitespace()
-            .map(|s| s.trim_matches('"').to_string())
-            .collect();
+        // RFC 9421 §2.1: a component identifier appears at most once. Gitlawb
+        // only supports bare names; parameters are rejected so reordered
+        // equivalent forms cannot bypass duplicate detection as distinct strings.
+        let mut components = Vec::new();
+        let mut seen = HashSet::new();
+        for token in components_str.split_whitespace() {
+            let name = parse_covered_component(token)?;
+            if !seen.insert(name.clone()) {
+                return Err(Error::HttpSignature(format!(
+                    "duplicate covered component '{name}' in Signature-Input"
+                )));
+            }
+            components.push(name);
+        }
 
         let params = parse_params(params_str)?;
 
@@ -203,6 +212,33 @@ pub fn compute_content_digest(body: &[u8]) -> String {
     format!("sha-256=:{}:", STANDARD.encode(hasher.finalize()))
 }
 
+/// Parse one covered-component token from Signature-Input's parenthesized list.
+///
+/// Gitlawb only supports bare component names (`"@method"`, `"@path"`,
+/// `"content-digest"`). RFC 9421 permits parameters on identifiers; they are
+/// rejected rather than accepted in a form `missing_components()` and
+/// `build_signing_string()` cannot handle consistently.
+fn parse_covered_component(token: &str) -> Result<String> {
+    let token = token.trim();
+    let open = token
+        .strip_prefix('"')
+        .ok_or_else(|| Error::HttpSignature("component must be a quoted string".into()))?;
+    let close = open
+        .find('"')
+        .ok_or_else(|| Error::HttpSignature("unterminated component string".into()))?;
+    let name = &open[..close];
+    if name.is_empty() {
+        return Err(Error::HttpSignature("empty component name".into()));
+    }
+    let trailing = open[close + 1..].trim();
+    if !trailing.is_empty() {
+        return Err(Error::HttpSignature(format!(
+            "component parameters are not supported on '{name}'"
+        )));
+    }
+    Ok(name.to_string())
+}
+
 /// Parse `;key="value";key2=value` parameter string into a map.
 fn parse_params(s: &str) -> Result<HashMap<String, String>> {
     let mut map = HashMap::new();
@@ -272,6 +308,91 @@ mod tests {
         assert!(missing.contains(&"content-digest"));
     }
 
+    /// RFC 9421 §2.1: a component identifier must not appear twice in the
+    /// covered list. Repeating one is not a way to say anything, but it does
+    /// repeat a line in the signing string, so the size of what a verifier
+    /// builds (and what a node persists alongside a claim) is set by how many
+    /// times the caller chose to write the same name.
+    #[test]
+    fn parse_rejects_component_parameters() {
+        let kp = Keypair::generate();
+        let did = kp.did();
+        let sig_input = format!(
+            r#"sig1=("@method" "@path" "content-digest";sf);keyid="{did}";alg="ed25519";created=1000"#
+        );
+        let err = HttpSignature::parse(
+            &sig_input,
+            "sig1=:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA:",
+        )
+        .expect_err("parameterized components must be refused");
+        assert!(
+            err.to_string().contains("component parameters"),
+            "got: {err}"
+        );
+    }
+
+    #[test]
+    fn parse_rejects_reordered_equivalent_component_parameters() {
+        let kp = Keypair::generate();
+        let did = kp.did();
+        let sig_input = format!(
+            r#"sig1=("content-digest";sf;tr "content-digest";tr;sf "@method" "@path");keyid="{did}";alg="ed25519";created=1000"#
+        );
+        let err = HttpSignature::parse(
+            &sig_input,
+            "sig1=:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA:",
+        )
+        .expect_err("reordered equivalent parameters must not bypass as distinct components");
+        assert!(
+            err.to_string().contains("component parameters"),
+            "got: {err}"
+        );
+    }
+
+    #[test]
+    fn parse_many_distinct_components_completes_quickly() {
+        use std::time::{Duration, Instant};
+
+        let kp = Keypair::generate();
+        let did = kp.did();
+        let names: Vec<String> = (0..20_000).map(|i| format!("\"c{i}\"")).collect();
+        let inner = names.join(" ");
+        let sig_input = format!(r#"sig1=({inner});keyid="{did}";alg="ed25519";created=1000"#);
+        let start = Instant::now();
+        let result = HttpSignature::parse(
+            &sig_input,
+            "sig1=:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA:",
+        );
+        let elapsed = start.elapsed();
+        assert!(
+            elapsed < Duration::from_secs(2),
+            "duplicate tracking must stay linear, took {elapsed:?}"
+        );
+        assert!(result.is_ok(), "distinct names should parse: {:?}", result);
+    }
+
+    #[test]
+    fn parse_rejects_duplicate_components() {
+        let kp = Keypair::generate();
+        let did = kp.did();
+        let sig_input = format!(
+            r#"sig1=("@method" "@path" "@path" "content-digest");keyid="{did}";alg="ed25519";created=1000"#
+        );
+        let err = HttpSignature::parse(&sig_input, "sig1=:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA:")
+            .expect_err("a repeated component must be refused");
+        assert!(
+            err.to_string().contains("duplicate"),
+            "the error must name the duplication, got: {err}"
+        );
+
+        // The control: the same list without the repeat still parses.
+        let ok = format!(
+            r#"sig1=("@method" "@path" "content-digest");keyid="{did}";alg="ed25519";created=1000"#
+        );
+        HttpSignature::parse(&ok, "sig1=:AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA:")
+            .expect("a distinct component list must still parse");
+    }
+
     #[test]
     fn verify_signature_end_to_end() {
         use crate::identity::verify;

crates/gitlawb-node/src/auth/mod.rs

@@ -162,17 +162,34 @@ pub async fn require_signature(request: Request, next: Next) -> Response {
         .unwrap_or("/")
         .to_string();
 
-    let content_digest = parts
+    // Required, never defaulted. The signing string is rebuilt from the request,
+    // so an absent header would make the covered `content-digest` empty on both
+    // sides: verification would pass and the digest-versus-body comparison below
+    // would have nothing to compare, leaving a signed request free to carry a
+    // body its signature never covered. `sign_request` always emits the header,
+    // so refusing here costs no conforming client anything.
+    let content_digest = match parts
         .headers
         .get("content-digest")
         .and_then(|v| v.to_str().ok())
-        .unwrap_or("")
-        .to_string();
+    {
+        Some(v) => v.to_string(),
+        None => {
+            return (
+                StatusCode::BAD_REQUEST,
+                Json(json!({
+                    "error": "missing_content_digest",
+                    "message": "Content-Digest header is required on a signed request",
+                })),
+            )
+                .into_response()
+        }
+    };
 
     let mut request_values: HashMap<String, String> = HashMap::new();
     request_values.insert("@method".to_string(), method);
     request_values.insert("@path".to_string(), path_and_query);
-    request_values.insert("content-digest".to_string(), content_digest);
+    request_values.insert("content-digest".to_string(), content_digest.clone());
 
     // The @signature-params value is the part of Signature-Input after "sig1="
     let sig_params_value = sig_input.strip_prefix("sig1=").unwrap_or(&sig_input);
@@ -217,23 +234,18 @@ pub async fn require_signature(request: Request, next: Next) -> Response {
             .into_response();
     }
 
-    // Verify Content-Digest matches the actual request body
-    if let Some(claimed) = parts
-        .headers
-        .get("content-digest")
-        .and_then(|v| v.to_str().ok())
-    {
-        let actual = compute_content_digest(&body_bytes);
-        if claimed != actual {
-            return (
-                StatusCode::BAD_REQUEST,
-                Json(json!({
-                    "error": "content_digest_mismatch",
-                    "message": "Content-Digest does not match request body",
-                })),
-            )
-                .into_response();
-        }
+    // Verify Content-Digest matches the actual request body. Unconditional: the
+    // header's presence was established above, so there is no branch here that
+    // skips the comparison.
+    if content_digest != compute_content_digest(&body_bytes) {
+        return (
+            StatusCode::BAD_REQUEST,
+            Json(json!({
+                "error": "content_digest_mismatch",
+                "message": "Content-Digest does not match request body",
+            })),
+        )
+            .into_response();
     }
 
     tracing::info!(did = %sig.key_id, "✓ authenticated request");
@@ -610,4 +622,75 @@ mod tests {
         let body_json: serde_json::Value = serde_json::from_slice(&body_bytes).unwrap();
         assert_eq!(body_json["error"], "invalid_ucan");
     }
+
+    /// A request that signs `content-digest` as the EMPTY string and then sends
+    /// no Content-Digest header at all must be refused before any handler runs.
+    ///
+    /// This is the whole point of requiring the header rather than defaulting it:
+    /// the middleware rebuilds the signing string from the request, so an absent
+    /// header makes the covered digest empty on both sides. The Ed25519 check
+    /// passes, the body-versus-digest comparison is skipped for want of a header,
+    /// and a signed request carries a body the signature never covered.
+    #[tokio::test]
+    async fn signed_request_without_content_digest_is_rejected() {
+        use base64::{engine::general_purpose::STANDARD, Engine};
+        use std::sync::atomic::{AtomicBool, Ordering};
+
+        static REACHED: AtomicBool = AtomicBool::new(false);
+
+        let kp = Keypair::generate();
+        let did = kp.did();
+        let created = chrono::Utc::now().timestamp();
+        let signature_input = format!(
+            r#"sig1=("@method" "@path" "content-digest");keyid="{did}";alg="ed25519";created={created}"#
+        );
+        let sig_params_value = &signature_input["sig1=".len()..];
+
+        let mut values: HashMap<String, String> = HashMap::new();
+        values.insert("@method".to_string(), "POST".to_string());
+        values.insert("@path".to_string(), "/x".to_string());
+        // Exactly what the middleware derives when the header is absent.
+        values.insert("content-digest".to_string(), String::new());
+        let signing_string =
+            build_signing_string(COVERED_COMPONENTS, sig_params_value, &values).unwrap();
+        let signature = format!(
+            "sig1=:{}:",
+            STANDARD.encode(kp.sign(signing_string.as_bytes()).to_bytes())
+        );
+
+        let app = Router::new()
+            .route(
+                "/x",
+                axum::routing::post(|| async {
+                    REACHED.store(true, Ordering::SeqCst);
+                    StatusCode::OK
+                }),
+            )
+            .layer(middleware::from_fn(require_signature));
+
+        let req = Request::builder()
+            .method("POST")
+            .uri("/x")
+            .header("signature-input", signature_input)
+            .header("signature", signature)
+            .body(Body::from("a body no signature covers"))
+            .unwrap();
+
+        let resp = app.oneshot(req).await.unwrap();
+        assert_eq!(
+            resp.status(),
+            StatusCode::BAD_REQUEST,
+            "a signed request with no Content-Digest header must be refused"
+        );
+        let bytes = axum::body::to_bytes(resp.into_body(), 2048).await.unwrap();
+        let body: serde_json::Value = serde_json::from_slice(&bytes).unwrap();
+        assert_eq!(
+            body["error"], "missing_content_digest",
+            "the refusal must name the absent Content-Digest header"
+        );
+        assert!(
+            !REACHED.load(Ordering::SeqCst),
+            "the handler must never see a request whose body no signature covers"
+        );
+    }
 }