diff --git a/crates/gitlawb-core/src/http_sig.rs b/crates/gitlawb-core/src/http_sig.rs index 1089e34db..97cb1b63e 100644 --- a/crates/gitlawb-core/src/http_sig.rs +++ b/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 = 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 { + 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> { 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 = (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; diff --git a/crates/gitlawb-node/src/auth/mod.rs b/crates/gitlawb-node/src/auth/mod.rs index 720fb3ae1..f4abfd61e 100644 --- a/crates/gitlawb-node/src/auth/mod.rs +++ b/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 = 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 = 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" + ); + } }