Skip to content

Commit b75aa2f

Browse files
authored
Merge pull request #236 from Gitlawb/fix/gl-acceptance-findings
Fix/gl acceptance findings
2 parents e14c2d3 + c681af7 commit b75aa2f

7 files changed

Lines changed: 520 additions & 50 deletions

File tree

‎.github/workflows/release.yml‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,6 +591,15 @@ jobs:
591591
bin.install "git-remote-gitlawb"
592592
end
593593
594+
def caveats
595+
<<~CAVEATS
596+
oh-my-zsh's git plugin aliases gl='git pull', which shadows this
597+
binary in interactive shells. If \`gl\` prints "fatal: not a git
598+
repository", run:
599+
echo 'unalias gl 2>/dev/null' >> ~/.zshrc && source ~/.zshrc
600+
CAVEATS
601+
end
602+
594603
test do
595604
assert_match version.to_s, shell_output("#{bin}/gl --version")
596605
end

‎crates/gl/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ path = "src/main.rs"
1313
[dependencies]
1414
gitlawb-core = { path = "../gitlawb-core" }
1515
icaptcha-client = { path = "../icaptcha-client" }
16+
base64 = { workspace = true }
1617
tokio = { workspace = true }
1718
serde = { workspace = true }
1819
serde_json = { workspace = true }

‎crates/gl/src/cert.rs‎

Lines changed: 215 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,16 @@ pub enum CertCmd {
4141
node: String,
4242
#[arg(long)]
4343
dir: Option<PathBuf>,
44+
/// Exit non-zero unless the Ed25519 signature verifies AND the
45+
/// issuing node matches the queried node (or --expect-node)
46+
#[arg(long)]
47+
verify: bool,
48+
/// Expected issuing node DID for --verify. A valid signature alone
49+
/// only proves the cert is internally consistent — signed by whatever
50+
/// key it names — so --verify also anchors the issuer to a DID you
51+
/// trust: this value when given, else the queried node's DID.
52+
#[arg(long, requires = "verify")]
53+
expect_node: Option<String>,
4454
},
4555
}
4656

@@ -52,7 +62,9 @@ pub async fn run(args: CertArgs) -> Result<()> {
5262
id,
5363
node,
5464
dir,
55-
} => cmd_show(repo, id, node, dir).await,
65+
verify,
66+
expect_node,
67+
} => cmd_show(repo, id, node, dir, verify, expect_node).await,
5668
}
5769
}
5870

@@ -114,7 +126,14 @@ async fn cmd_list(repo: String, node: String, dir: Option<PathBuf>) -> Result<()
114126
Ok(())
115127
}
116128

117-
async fn cmd_show(repo: String, id: String, node: String, dir: Option<PathBuf>) -> Result<()> {
129+
async fn cmd_show(
130+
repo: String,
131+
id: String,
132+
node: String,
133+
dir: Option<PathBuf>,
134+
require_valid: bool,
135+
expect_node: Option<String>,
136+
) -> Result<()> {
118137
let (owner, name) = resolve_repo(&repo, &node, dir.as_deref()).await?;
119138

120139
let client = signed_client(&node, dir.as_deref());
@@ -148,39 +167,124 @@ async fn cmd_show(repo: String, id: String, node: String, dir: Option<PathBuf>)
148167
println!(" Signature: {signature}");
149168
println!();
150169

151-
// Reconstruct the signing payload and verify
152-
// Fetch the node's current public key to verify
153-
let info: Value = client
154-
.get("/")
155-
.await?
156-
.json()
157-
.await
158-
.context("failed to fetch node info")?;
159-
let current_node_did = info["did"].as_str().unwrap_or("");
170+
// Verify the Ed25519 signature: rebuild the exact canonical payload the
171+
// node signed (see gitlawb-node/src/cert.rs::issue_ref_certificate) and
172+
// check it against the public key embedded in the certificate's node DID.
173+
// This proves the cert is internally authentic — signed by the key it
174+
// names; the node-DID comparison below covers *which* node that is.
175+
let repo_id = cert["repo_id"].as_str().unwrap_or("");
176+
let verdict = verify_signature(
177+
repo_id, ref_name, old_sha, new_sha, pusher, node_did, issued_at, signature,
178+
);
160179

161180
println!("Signature verification:");
162-
println!(" Signing payload would be:");
163-
println!(" {{\"repo_id\": ..., \"ref\": \"{ref_name}\", \"old\": \"{old_sha}\",");
164-
println!(" \"new\": \"{new_sha}\", \"pusher\": \"{pusher}\",");
165-
println!(" \"node\": \"{node_did}\", \"ts\": \"{issued_at}\"}}");
166-
println!();
181+
match &verdict {
182+
Ok(()) => {
183+
println!(
184+
" VALID — Ed25519 signature verified against the key the certificate names ({node_did})"
185+
);
186+
}
187+
Err(reason) => {
188+
println!(" INVALID — {reason}");
189+
}
190+
}
167191

168-
if current_node_did == node_did {
169-
println!(" Node DID matches current node. Signature is an Ed25519/base64url value.");
170-
println!(" To verify offline, use the node's Ed25519 public key derived from:");
171-
println!(" did:key → {node_did}");
172-
} else {
173-
println!(" WARNING: Certificate node DID ({node_did}) does not match");
174-
println!(" current node DID ({current_node_did}).");
175-
println!(" This certificate was issued by a different node.");
192+
// Contextual only — the verdict above stands on its own, so a node-info
193+
// hiccup here must not turn a successfully displayed certificate into an
194+
// error exit.
195+
let current_node_did = match client.get("/").await {
196+
Ok(resp) => resp
197+
.json::<Value>()
198+
.await
199+
.ok()
200+
.and_then(|info| info["did"].as_str().map(str::to_string)),
201+
Err(_) => None,
202+
};
203+
match current_node_did.as_deref() {
204+
Some(current) if current == node_did => {
205+
println!(" Issuing node DID matches the node being queried.");
206+
}
207+
Some(current) => {
208+
println!(" WARNING: Certificate node DID ({node_did}) does not match");
209+
println!(" current node DID ({current}).");
210+
println!(" This certificate was issued by a different node.");
211+
}
212+
None => {
213+
println!(" NOTE: could not fetch current node info — skipping node-DID comparison.");
214+
}
176215
}
177216

178-
println!();
179-
println!(" Signature (base64url): {signature}");
217+
if require_valid {
218+
if let Err(reason) = verdict {
219+
anyhow::bail!("certificate signature did not verify: {reason}");
220+
}
221+
// A valid signature proves internal consistency only: the payload was
222+
// signed by whatever key the certificate itself names. A hostile
223+
// source can mint a keypair, put its DID in node_did, and self-sign.
224+
// --verify therefore also anchors the issuer to a trusted DID:
225+
// --expect-node when given, else the DID of the node being queried.
226+
let expected = expect_node.as_deref().or(current_node_did.as_deref());
227+
match expected {
228+
Some(expected) if expected == node_did => {}
229+
Some(expected) => anyhow::bail!(
230+
"certificate is signed by {node_did}, but the expected issuer is {expected} — \
231+
a valid signature alone proves internal consistency, not a trusted issuer"
232+
),
233+
None => anyhow::bail!(
234+
"cannot anchor the issuer: node info is unreachable and no --expect-node was given"
235+
),
236+
}
237+
}
180238

181239
Ok(())
182240
}
183241

242+
/// Rebuild the node's canonical signing payload (field order must match
243+
/// gitlawb-node/src/cert.rs::issue_ref_certificate exactly) and verify the
244+
/// certificate's Ed25519 signature against the key embedded in `node_did`.
245+
#[allow(clippy::too_many_arguments)]
246+
fn verify_signature(
247+
repo_id: &str,
248+
ref_name: &str,
249+
old_sha: &str,
250+
new_sha: &str,
251+
pusher: &str,
252+
node_did: &str,
253+
issued_at: &str,
254+
signature_b64: &str,
255+
) -> std::result::Result<(), String> {
256+
use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine};
257+
use std::str::FromStr;
258+
259+
let payload = serde_json::json!({
260+
"repo_id": repo_id,
261+
"ref": ref_name,
262+
"old": old_sha,
263+
"new": new_sha,
264+
"pusher": pusher,
265+
"node": node_did,
266+
"ts": issued_at,
267+
});
268+
let payload_bytes =
269+
serde_json::to_vec(&payload).map_err(|e| format!("could not serialize payload: {e}"))?;
270+
271+
let did =
272+
gitlawb_core::did::Did::from_str(node_did).map_err(|e| format!("bad node DID: {e}"))?;
273+
let verifying_key = did
274+
.to_verifying_key()
275+
.map_err(|e| format!("cannot derive a public key from {node_did}: {e}"))?;
276+
277+
let sig_vec = URL_SAFE_NO_PAD
278+
.decode(signature_b64)
279+
.map_err(|e| format!("signature is not valid base64url: {e}"))?;
280+
let sig_bytes: [u8; 64] = sig_vec
281+
.try_into()
282+
.map_err(|_| "signature is not 64 bytes".to_string())?;
283+
284+
gitlawb_core::identity::verify(&verifying_key, &payload_bytes, &sig_bytes)
285+
.map_err(|_| "Ed25519 signature does not match the signed payload".to_string())
286+
}
287+
184288
async fn resolve_cert_id(client: &NodeClient, owner: &str, name: &str, id: &str) -> Result<String> {
185289
if id.len() >= 36 {
186290
return Ok(id.to_string());
@@ -209,3 +313,88 @@ async fn resolve_cert_id(client: &NodeClient, owner: &str, name: &str, id: &str)
209313
_ => anyhow::bail!("certificate prefix {id} matches multiple certificates"),
210314
}
211315
}
316+
317+
#[cfg(test)]
318+
mod tests {
319+
use super::*;
320+
321+
/// Pins gl's payload reconstruction to the frozen canonical byte form the
322+
/// node signs (default serde_json maps = alphabetically ordered keys). If
323+
/// serialization drifts — a field added, or a preserve_order feature
324+
/// landing anywhere in the workspace (feature unification flips every
325+
/// crate at once) — this literal stops matching and the test fails,
326+
/// instead of every real certificate silently rendering INVALID.
327+
#[test]
328+
fn payload_serialization_matches_frozen_canonical_form() {
329+
let payload = serde_json::json!({
330+
"repo_id": "repo-1",
331+
"ref": "refs/heads/main",
332+
"old": "oldsha",
333+
"new": "newsha",
334+
"pusher": "did:key:z6MkPusher",
335+
"node": "did:key:z6MkNode",
336+
"ts": "2026-07-22T00:00:00+00:00",
337+
});
338+
let frozen = concat!(
339+
r#"{"new":"newsha","node":"did:key:z6MkNode","old":"oldsha","#,
340+
r#""pusher":"did:key:z6MkPusher","ref":"refs/heads/main","#,
341+
r#""repo_id":"repo-1","ts":"2026-07-22T00:00:00+00:00"}"#,
342+
);
343+
assert_eq!(serde_json::to_string(&payload).unwrap(), frozen);
344+
}
345+
346+
/// Signing exactly as the node does must round-trip through
347+
/// verify_signature; any field tampering must fail it.
348+
#[test]
349+
fn verify_signature_round_trip_and_tamper() {
350+
let kp = gitlawb_core::identity::Keypair::generate();
351+
let node_did = kp.did().as_str().to_string();
352+
353+
let payload = serde_json::json!({
354+
"repo_id": "repo-1",
355+
"ref": "refs/heads/main",
356+
"old": "0".repeat(40),
357+
"new": "a".repeat(40),
358+
"pusher": "did:key:z6MkPusher",
359+
"node": node_did,
360+
"ts": "2026-07-22T00:00:00+00:00",
361+
});
362+
let sig = kp.sign_b64(&serde_json::to_vec(&payload).unwrap());
363+
364+
let ok = verify_signature(
365+
"repo-1",
366+
"refs/heads/main",
367+
&"0".repeat(40),
368+
&"a".repeat(40),
369+
"did:key:z6MkPusher",
370+
&node_did,
371+
"2026-07-22T00:00:00+00:00",
372+
&sig,
373+
);
374+
assert!(ok.is_ok(), "expected valid signature, got: {ok:?}");
375+
376+
let tampered = verify_signature(
377+
"repo-1",
378+
"refs/heads/main",
379+
&"0".repeat(40),
380+
&"b".repeat(40), // new_sha changed after signing
381+
"did:key:z6MkPusher",
382+
&node_did,
383+
"2026-07-22T00:00:00+00:00",
384+
&sig,
385+
);
386+
assert!(tampered.is_err(), "tampered payload must not verify");
387+
388+
let garbage = verify_signature(
389+
"repo-1",
390+
"refs/heads/main",
391+
&"0".repeat(40),
392+
&"a".repeat(40),
393+
"did:key:z6MkPusher",
394+
&node_did,
395+
"2026-07-22T00:00:00+00:00",
396+
"not-base64url!!!",
397+
);
398+
assert!(garbage.is_err(), "malformed signature must not verify");
399+
}
400+
}

0 commit comments

Comments
 (0)