Skip to content

Commit ae4f5fc

Browse files
committed
fix(node): address review findings on Arweave anchoring and cert chain (#26)
- Use configured gateway's data URL (/tx_id) instead of bundler API for verify - Bound untrusted response to 1 MiB on public verify route - Bind each anchor to its own ref update certificate (not repo-wide latest) - Make certificate storage append-only with unique (repo_id, seq) constraint - Add migration v13 to drop old unique index and add sequence uniqueness - Allocate chain sequence numbers atomically using per-repo advisory lock - Fail closed on missing predecessor cert during verification - Persist full RFC 9421 HTTP Signature context (signature-input, content-digest, path) - Verify pusher authorization proof in verify_anchor - Add legacy GITLAWB_IRYS_URL fallback for GITLAWB_BUNDLER_URL - Expose seq, prev, pusher_sig through certificate API list/get responses
1 parent 4bdd5a8 commit ae4f5fc

10 files changed

Lines changed: 365 additions & 139 deletions

File tree

‎Cargo.lock‎

Lines changed: 5 additions & 5 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/gitlawb-node/src/api/certs.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ pub async fn list_certs(
5252
"node_did": c.node_did,
5353
"signature": c.signature,
5454
"issued_at": c.issued_at,
55+
"seq": c.seq,
56+
"prev": c.prev,
57+
"pusher_sig": c.pusher_sig,
5558
})
5659
})
5760
.collect();
@@ -92,5 +95,8 @@ pub async fn get_cert(
9295
"node_did": cert.node_did,
9396
"signature": cert.signature,
9497
"issued_at": cert.issued_at,
98+
"seq": cert.seq,
99+
"prev": cert.prev,
100+
"pusher_sig": cert.pusher_sig,
95101
})))
96102
}

‎crates/gitlawb-node/src/api/events.rs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,9 @@ mod ref_updates_feed_tests {
440440
seq: 1,
441441
prev: "0".repeat(64),
442442
pusher_sig: None,
443+
signature_input: None,
444+
content_digest: None,
445+
request_path: None,
443446
}
444447
}
445448

‎crates/gitlawb-node/src/api/repos.rs‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use axum::Json;
55
use bytes::Bytes;
66
use std::sync::Arc;
77

8-
use crate::auth::{caller_authorized_to_push, AuthenticatedDid, PusherSignature};
8+
use crate::auth::{caller_authorized_to_push, AuthenticatedDid, PusherProof, PusherSignature};
99
use crate::db::RepoRecord;
1010
use chrono::{DateTime, Utc};
1111
use serde::{Deserialize, Serialize};
@@ -856,6 +856,7 @@ pub async fn git_receive_pack(
856856
Path((owner, repo)): Path<(String, String)>,
857857
Extension(auth): Extension<AuthenticatedDid>,
858858
Extension(pusher_sig): Extension<PusherSignature>,
859+
Extension(pusher_proof): Extension<PusherProof>,
859860
body: Bytes,
860861
) -> Result<Response> {
861862
let name = smart_http_repo_name(&repo)?;
@@ -972,6 +973,10 @@ pub async fn git_receive_pack(
972973
// The route is behind `require_signature`, so the verified pusher identity is
973974
// always present; use it directly rather than re-parsing the headers.
974975
let did = auth.0.as_str();
976+
// Collect certs keyed by ref_name so the anchoring loop below uses
977+
// the correct per-update certificate rather than a repo-wide latest.
978+
let mut ref_certs: std::collections::HashMap<String, crate::db::RefCertificate> =
979+
std::collections::HashMap::new();
975980
{
976981
// Use the first new commit hash we parsed, fall back to timestamp
977982
let commit_hash = ref_updates
@@ -999,11 +1004,15 @@ pub async fn git_receive_pack(
9991004
&update.new_sha,
10001005
did,
10011006
Some(pusher_sig.0.clone()),
1007+
Some(pusher_proof.signature_input.clone()),
1008+
Some(pusher_proof.content_digest.clone()),
1009+
Some(pusher_proof.request_path.clone()),
10021010
)
10031011
.await
10041012
{
10051013
Ok(c) => {
1006-
tracing::info!(cert_id = %c.id, repo = %record.name, ref_name = %update.ref_name, pusher = %did, "issued ref certificate")
1014+
tracing::info!(cert_id = %c.id, repo = %record.name, ref_name = %update.ref_name, pusher = %did, "issued ref certificate");
1015+
ref_certs.insert(update.ref_name.clone(), c);
10071016
}
10081017
Err(e) => {
10091018
tracing::warn!(err = %e, ref_name = %update.ref_name, "failed to issue ref certificate")
@@ -1210,7 +1219,6 @@ pub async fn git_receive_pack(
12101219
let db_clone = state.db.clone();
12111220
let http_client = Arc::clone(&state.http_client);
12121221
let node_did_str = state.node_did.to_string();
1213-
let repo_id_clone = record.id.clone();
12141222
let repo_slug = format!(
12151223
"{}/{}",
12161224
crate::db::normalize_owner_key(&record.owner_did),
@@ -1220,6 +1228,7 @@ pub async fn git_receive_pack(
12201228
.iter()
12211229
.map(|u| (u.ref_name.clone(), u.old_sha.clone(), u.new_sha.clone()))
12221230
.collect::<Vec<_>>();
1231+
let ref_certs_clone = ref_certs.clone();
12231232
let p2p_handle = state.p2p.clone();
12241233
let pusher_did_clone = did.to_string();
12251234
let db_for_peers = state.db.clone();
@@ -1311,11 +1320,10 @@ pub async fn git_receive_pack(
13111320
if announce && !bundler_url.is_empty() {
13121321
for (ref_name, old_sha, new_sha) in &ref_updates_clone {
13131322
let cid = cid_map.get(new_sha).cloned();
1314-
let cert = db_clone
1315-
.get_most_recent_cert(&repo_id_clone)
1316-
.await
1317-
.ok()
1318-
.flatten();
1323+
// Use the per-update certificate issued above, not a
1324+
// repo-wide latest, so each anchor embeds the exact
1325+
// certificate for its own ref transition.
1326+
let cert = ref_certs_clone.get(ref_name).cloned();
13191327
let anchor = crate::arweave::RefAnchor {
13201328
repo: repo_slug.clone(),
13211329
owner_did: owner_did_for_arweave.clone(),

‎crates/gitlawb-node/src/arweave.rs‎

Lines changed: 139 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use base64::Engine as _;
2222
use serde::Serialize;
2323
use serde_json::json;
2424
use sha2::Digest;
25+
use std::collections::HashMap;
2526
use std::str::FromStr;
2627

2728
/// Data describing a ref-update event to be anchored.
@@ -261,22 +262,32 @@ pub async fn verify_anchor(
261262
tx_id: &str,
262263
db: &crate::db::Db,
263264
) -> Result<VerifyResult> {
264-
// Fetch the data item from the bundler gateway.
265-
let url = format!("{}/v1/tx/{}", gateway_url.trim_end_matches('/'), tx_id);
265+
// Fetch the data item from the Arweave gateway's data path.
266+
// Gateways serve data at /{tx_id}, not /v1/tx/{id} (which is the bundler API).
267+
let url = format!("{}/{}", gateway_url.trim_end_matches('/'), tx_id);
266268
let resp = client
267269
.get(&url)
268270
.send()
269271
.await
270-
.map_err(|e| anyhow::anyhow!("failed to fetch data from bundler gateway: {e}"))?;
272+
.map_err(|e| anyhow::anyhow!("failed to fetch data from Arweave gateway: {e}"))?;
271273
if !resp.status().is_success() {
272274
return Ok(VerifyResult {
273275
valid: false,
274276
anchor: serde_json::Value::Null,
275277
certificate: None,
276-
errors: vec![format!("bundler gateway returned {}", resp.status())],
278+
errors: vec![format!("Arweave gateway returned {}", resp.status())],
277279
});
278280
}
281+
// Bound the untrusted response to 1 MiB to prevent memory exhaustion.
279282
let body_bytes = resp.bytes().await?;
283+
if body_bytes.len() > 1_048_576 {
284+
return Ok(VerifyResult {
285+
valid: false,
286+
anchor: serde_json::Value::Null,
287+
certificate: None,
288+
errors: vec!["response body exceeds 1 MiB limit".to_string()],
289+
});
290+
}
280291

281292
// Parse the payload — could be JSON or raw bytes depending on gateway
282293
let anchor: serde_json::Value = serde_json::from_slice(&body_bytes)?;
@@ -341,26 +352,132 @@ pub async fn verify_anchor(
341352
errors.push(format!("certificate signature verification failed: {e}"));
342353
}
343354

344-
// 2. Verify prev hash linkage against the predecessor at seq - 1
355+
// 2. Verify prev hash linkage against the predecessor at seq - 1.
356+
// Fail closed: a missing declared predecessor is treated as invalid.
345357
if c.seq > 1 {
346-
if let Ok(Some(pred)) = db.get_cert_by_seq(&c.repo_id, c.seq - 1).await {
347-
let prev_payload = serde_json::json!({
348-
"repo_id": pred.repo_id,
349-
"ref": pred.ref_name,
350-
"old": pred.old_sha,
351-
"new": pred.new_sha,
352-
"pusher": pred.pusher_did,
353-
"node": pred.node_did,
354-
"ts": pred.issued_at,
355-
});
356-
let prev_bytes = serde_json::to_vec(&prev_payload)?;
357-
let expected_prev = hex::encode(sha2::Sha256::digest(&prev_bytes));
358-
if c.prev != expected_prev {
358+
match db.get_cert_by_seq(&c.repo_id, c.seq - 1).await {
359+
Ok(Some(pred)) => {
360+
let prev_payload = serde_json::json!({
361+
"repo_id": pred.repo_id,
362+
"ref": pred.ref_name,
363+
"old": pred.old_sha,
364+
"new": pred.new_sha,
365+
"pusher": pred.pusher_did,
366+
"node": pred.node_did,
367+
"ts": pred.issued_at,
368+
});
369+
let prev_bytes = serde_json::to_vec(&prev_payload)?;
370+
let expected_prev = hex::encode(sha2::Sha256::digest(&prev_bytes));
371+
if c.prev != expected_prev {
372+
errors.push(format!(
373+
"prev hash mismatch: claimed {} expected {}",
374+
c.prev, expected_prev
375+
));
376+
}
377+
}
378+
Ok(None) => {
359379
errors.push(format!(
360-
"prev hash mismatch: claimed {} expected {}",
361-
c.prev, expected_prev
380+
"predecessor cert seq {} not found for repo {}",
381+
c.seq - 1,
382+
c.repo_id
362383
));
363384
}
385+
Err(e) => {
386+
errors.push(format!(
387+
"error looking up predecessor seq {}: {e}",
388+
c.seq - 1
389+
));
390+
}
391+
}
392+
}
393+
394+
// 3. Verify the pusher authorization proof (RFC 9421 HTTP Signature)
395+
// when all required context is available.
396+
if let (Some(pusher_sig), Some(sig_input), Some(content_digest), Some(request_path)) = (
397+
&c.pusher_sig,
398+
&c.signature_input,
399+
&c.content_digest,
400+
&c.request_path,
401+
) {
402+
match gitlawb_core::http_sig::HttpSignature::parse(
403+
sig_input,
404+
&format!("sig1=:{pusher_sig}:"),
405+
) {
406+
Ok(http_sig) => {
407+
let mut request_values: HashMap<String, String> = HashMap::new();
408+
request_values.insert("@method".to_string(), "POST".to_string());
409+
request_values.insert("@path".to_string(), request_path.clone());
410+
request_values.insert("content-digest".to_string(), content_digest.clone());
411+
412+
let sig_params_value = sig_input.strip_prefix("sig1=").unwrap_or(sig_input);
413+
let components_ref: Vec<&str> =
414+
http_sig.components.iter().map(String::as_str).collect();
415+
416+
match gitlawb_core::http_sig::build_signing_string(
417+
&components_ref,
418+
sig_params_value,
419+
&request_values,
420+
) {
421+
Ok(signing_string) => {
422+
let pusher_did = gitlawb_core::did::Did::from_str(&c.pusher_did);
423+
let pusher_vk = pusher_did.and_then(|d| d.to_verifying_key());
424+
match pusher_vk {
425+
Ok(vk) => {
426+
let sig_bytes: [u8; 64] =
427+
match base64::engine::general_purpose::STANDARD
428+
.decode(pusher_sig)
429+
{
430+
Ok(bytes) => match bytes.as_slice().try_into() {
431+
Ok(a) => a,
432+
Err(_) => {
433+
errors.push(
434+
"pusher signature is not 64 bytes"
435+
.to_string(),
436+
);
437+
return Ok(VerifyResult {
438+
valid: false,
439+
anchor,
440+
certificate: cert,
441+
errors,
442+
});
443+
}
444+
},
445+
Err(_) => {
446+
errors.push(
447+
"pusher signature is not valid base64"
448+
.to_string(),
449+
);
450+
return Ok(VerifyResult {
451+
valid: false,
452+
anchor,
453+
certificate: cert,
454+
errors,
455+
});
456+
}
457+
};
458+
if let Err(e) = gitlawb_core::identity::verify(
459+
&vk,
460+
signing_string.as_bytes(),
461+
&sig_bytes,
462+
) {
463+
errors.push(format!(
464+
"pusher signature verification failed: {e}"
465+
));
466+
}
467+
}
468+
Err(e) => {
469+
errors.push(format!("unresolvable pusher DID: {e}"));
470+
}
471+
}
472+
}
473+
Err(e) => {
474+
errors.push(format!("failed to build signing string: {e}"));
475+
}
476+
}
477+
}
478+
Err(e) => {
479+
errors.push(format!("failed to parse pusher Signature-Input: {e}"));
480+
}
364481
}
365482
}
366483
} else {
@@ -564,16 +681,14 @@ mod tests {
564681
#[tokio::test]
565682
async fn test_verify_anchor_uses_correct_gateway_url() {
566683
let mut server = mockito::Server::new_async().await;
684+
// Gateways serve data at /{tx_id}, not /v1/tx/{id}.
567685
let _mock = server
568-
.mock("GET", "/v1/tx/does-not-exist")
686+
.mock("GET", "/does-not-exist")
569687
.with_status(404)
570688
.create_async()
571689
.await;
572690

573691
let client = reqwest::Client::new();
574-
// verify_anchor needs a real PgPool; this test only exercises that
575-
// the function correctly formats the gateway URL and handles a 404.
576-
// It will error on the pool access which is expected without a test DB.
577692
let pool = sqlx::postgres::PgPoolOptions::new()
578693
.connect_lazy("postgres://localhost/gitlawb_test_placeholder")
579694
.expect("lazy pool creation should not fail");
@@ -582,13 +697,9 @@ mod tests {
582697

583698
match result {
584699
Ok(r) => {
585-
// With a lazy unconnected pool, get_most_recent_cert will fail,
586-
// but the function still returns Ok(VerifyResult) with errors.
587700
assert!(!r.valid);
588701
}
589702
Err(e) => {
590-
// On some systems the POSTGRES connection attempt may abort
591-
// rather than fail gracefully.
592703
let msg = e.to_string();
593704
assert!(
594705
msg.contains("pool") || msg.contains("error"),

0 commit comments

Comments
 (0)