Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
beardthelion wants to merge 1 commit into
base: main
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions crates/gitlawb-core/src/http_sig.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,17 @@ impl HttpSignature {
.map(|s| s.trim_matches('"').to_string())
.collect();

// RFC 9421 §2.1: a component identifier appears at most once. A repeat
// says nothing extra and only lengthens the signing string a verifier
// has to build, so it is refused rather than folded.
for (i, c) in components.iter().enumerate() {
if components[i + 1..].contains(c) {
return Err(Error::HttpSignature(format!(
"duplicate covered component '{c}' in Signature-Input"
)));
}
}

Comment on lines +76 to +86

@coderabbitai coderabbitai Bot Aug 8, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

rg -n -C 10 \
  'split_whitespace|trim_matches|build_signing_string|components_ref|parse_rejects_duplicate_components' \
  crates/gitlawb-core/src/http_sig.rs \
  crates/gitlawb-node/src/auth/mod.rs

# Expect: parameterized identifiers are parsed structurally, duplicate equality
# ignores parameter order, and tests cover reordered equivalent parameters.

Repository: Gitlawb/node

Length of output: 19485

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

sed -n '1,180p' crates/gitlawb-core/src/http_sig.rs
printf '\n--- RFC references in repository ---\n'
rg -n -i 'RFC 9421|component identifier|signature-input|sf;|structured field' \
  crates README.md .github 2>/dev/null | head -200

Repository: Gitlawb/node

Length of output: 17409

🌐 Web query:

RFC 9421 component identifier parameters equality order duplicate covered component

💡 Result:

In RFC 9421 (HTTP Message Signatures), the rules regarding component identifier equality and duplicates in a covered components list are as follows [1][2]: Duplicate Rules Within a single list of covered components, each unique component identifier must occur only once [1][2]. Two component identifiers are considered distinct if their component names differ or if any of their associated parameters differ [1][2]. Conversely, multiple component identifiers that share the same component name may be included if they have distinct parameters (e.g., "foo";bar and "foo";baz are permitted together) [1][2]. Equality and Parameter Order When comparing two component identifiers for equality, the order of parameters is not significant [1][2]. For example, "foo";bar;baz and "foo";baz;bar are considered equivalent, and therefore, both cannot be included in the same list of covered components [1][2]. Processing Requirement Although parameter order is not significant for equality checks, it must be preserved when processing a component identifier (such as during parsing for verification) [1][2]. A system is not allowed to transform one equivalent form into another (e.g., reordering parameters) during processing [1][2]. Once an order for parameters is chosen for a signature, it cannot be changed [1][2].

Citations:

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

sed -n '55,125p' crates/gitlawb-core/src/http_sig.rs
sed -n '45,225p' crates/gitlawb-node/src/auth/mod.rs
sed -n '270,320p' crates/gitlawb-core/src/http_sig.rs

python3 - <<'PY'
from collections import Counter

covered = ['`@method`', '`@path`', 'content-digest']
request_values = {
    '`@method`': 'POST',
    '`@path`': '/x',
    'content-digest': 'sha-256=:abc:',
}
components_str = '"example-dict";sf;tr "example-dict";tr;sf'

components = [part.strip('"') for part in components_str.split()]
duplicates = [name for name, count in Counter(components).items() if count > 1]
missing = [name for name in covered if name not in components]
lookups = [(name, request_values.get(name)) for name in components]

print('components:', components)
print('raw duplicate check:', duplicates)
print('required components missing:', missing)
print('request-value lookups:', lookups)
PY

Repository: Gitlawb/node

Length of output: 11671

Handle parameterized component identifiers end to end.

HttpSignature::parse stores "content-digest";sf as raw text. Reordered parameters can bypass this duplicate check because RFC 9421 ignores parameter order. The node later rejects these inputs because missing_components() and build_signing_string() only support bare names. Either reject component parameters during parsing or parse and serialize them consistently. Add a test for reordered equivalent parameters.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/gitlawb-core/src/http_sig.rs` around lines 76 - 86, The
HttpSignature::parse component validation must handle parameterized component
identifiers consistently instead of comparing raw text while later methods only
support bare names. Either reject any component parameters during parsing, or
normalize and preserve them through missing_components() and
build_signing_string(); also add coverage proving reordered equivalent
parameters cannot bypass duplicate detection.

let params = parse_params(params_str)?;

let key_id: Did = params
Expand Down Expand Up @@ -272,6 +283,33 @@ 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_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;
Expand Down
125 changes: 104 additions & 21 deletions crates/gitlawb-node/src/auth/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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"
);
}
}
Loading