git-ents.gitmain
⌘K
foforge
commit a13499b
feat: gate member pushes on a key validity window

A member key now carries an optional valid-after/valid-before window, rendered into the allowed_signers file git verifies pushes against, so an un-refreshed key stops authorizing new pushes once it lapses — staleness fails closed, while a previously-valid push stays verifiable via -Overify-time. Verified end-to-end against OpenSSH: an expired-window key is rejected, an in-window key accepted, a pre-window key rejected.

feat: add valid_after/valid_before to the member entry feat: render the validity window into allowed_signers options feat: add --valid-after/--valid-before to git ents members add refactor: drop the Row bound from the CLI Set presenter Assisted-by: Claude:claude-opus-4-8

Joseph D. Carpinelli · 1 month ago

Reviews

No reviews of this commit yet — record a verdict below.

Start a review

verdict

crates/git-ents-server/tests/pre_receive.rs @@ -66,8 +66,32 @@ } /// Create a bare server repo wired to the `pre-receive` verifier, listing the -/// public keys at `authorized` as signers. +/// public keys at `authorized` as signers with no validity window. fn server_repo(base: &Path, authorized: &[&Path]) -> PathBuf { + let members: Vec<Member> = authorized + .iter() + .map(|pubkey| Member { + pubkey, + valid_after: None, + valid_before: None, + }) + .collect(); + server_repo_with(base, &members) +} + +/// One authorized member written into the test `refs/meta/members` doc: a public +/// key and the validity window it is trusted within. +struct Member<'a> { + pubkey: &'a Path, + valid_after: Option<&'a str>, + valid_before: Option<&'a str>, +} + +/// Create a bare server repo wired to the `pre-receive` verifier whose +/// `refs/meta/members` lists `members` in the real on-disk layout — a +/// `members/<key-n>/` subtree per member holding a `key` blob and an +/// `valid_after`/`valid_before` `Option` subtree each. +fn server_repo_with(base: &Path, members: &[Member]) -> PathBuf { let repo = base.join("srv.git"); ok( base, @@ -88,12 +112,29 @@ std::fs::set_permissions(&hook, std::fs::Permissions::from_mode(0o755)).unwrap(); } - if !authorized.is_empty() { + if !members.is_empty() { + let option_tree = |bound: Option<&str>| match bound { + None => mktree(&repo, ""), + Some(value) => { + let blob = hash_object(&repo, value.as_bytes()); + mktree(&repo, &format!("100644 blob {blob}\tsome\n")) + } + }; let mut tree_entries = String::new(); - for (index, pubkey) in authorized.iter().enumerate() { - let key = std::fs::read_to_string(pubkey).unwrap(); - let oid = hash_object(&repo, key.as_bytes()); - tree_entries.push_str(&format!("100644 blob {oid}\tkey-{index}\n")); + for (index, member) in members.iter().enumerate() { + let key = std::fs::read_to_string(member.pubkey).unwrap(); + let key_blob = hash_object(&repo, key.as_bytes()); + let after_tree = option_tree(member.valid_after); + let before_tree = option_tree(member.valid_before); + let member_tree = mktree( + &repo, + &format!( + "100644 blob {key_blob}\tkey\n\ + 040000 tree {after_tree}\tvalid_after\n\ + 040000 tree {before_tree}\tvalid_before\n" + ), + ); + tree_entries.push_str(&format!("040000 tree {member_tree}\tkey-{index}\n")); } let members_tree = mktree(&repo, &tree_entries); let root_tree = mktree(&repo, &format!("040000 tree {members_tree}\tmembers\n")); @@ -201,6 +242,72 @@ std::fs::remove_dir_all(&base).ok(); } +#[test] +fn accepts_a_push_signed_by_an_in_window_key() { + let base = unique_dir("inwindow"); + let pubkey = keygen(&base, "id"); + let server = server_repo_with( + &base, + &[Member { + pubkey: &pubkey, + valid_after: Some("20200101"), + valid_before: Some("20990101"), + }], + ); + let work = work_repo(&base, Some(&pubkey)); + + assert!( + push(&work, &server, true), + "in-window signed push was rejected" + ); + std::fs::remove_dir_all(&base).ok(); +} + +#[test] +fn rejects_a_push_signed_by_an_expired_key() { + // The window lapsed before today, so the key no longer authorizes a new + // push — staleness fails closed. This is the Phase 1 security gate: if the + // verifier ignored `valid-before`, this push would be accepted. + let base = unique_dir("expired"); + let pubkey = keygen(&base, "id"); + let server = server_repo_with( + &base, + &[Member { + pubkey: &pubkey, + valid_after: None, + valid_before: Some("20200101"), + }], + ); + let work = work_repo(&base, Some(&pubkey)); + + assert!( + !push(&work, &server, true), + "push signed by an expired-window key was accepted" + ); + std::fs::remove_dir_all(&base).ok(); +} + +#[test] +fn rejects_a_push_signed_before_a_keys_window_opens() { + let base = unique_dir("future"); + let pubkey = keygen(&base, "id"); + let server = server_repo_with( + &base, + &[Member { + pubkey: &pubkey, + valid_after: Some("20990101"), + valid_before: None, + }], + ); + let work = work_repo(&base, Some(&pubkey)); + + assert!( + !push(&work, &server, true), + "push signed before the key's window opened was accepted" + ); + std::fs::remove_dir_all(&base).ok(); +} + #[test] fn accepts_any_push_before_signers_are_configured() { let base = unique_dir("bootstrap");
crates/git-ents/src/main.rs @@ -12,7 +12,6 @@ use clap::{Parser, Subcommand}; use git_ents::checks::{self, CHECKS_REF, Check}; use git_ents::signers::{self, MEMBERS_REF, Signer}; -use git_store::Row as _; #[derive(Parser)] #[command(name = "git-ents", about = "Helpful guardians of your git trees.")] @@ -61,6 +60,14 @@ /// Key to authorize; defaults to `user.signingkey`. #[arg(long)] key: Option<PathBuf>, + /// Trust the key only at or after this OpenSSH timestamp + /// (`YYYYMMDD[Z]` or `YYYYMMDDHHMM[SS][Z]`; append `Z` for UTC). + #[arg(long, value_name = "TIMESTAMP")] + valid_after: Option<String>, + /// Stop trusting the key after this OpenSSH timestamp; omit for trust + /// that never lapses on its own. + #[arg(long, value_name = "TIMESTAMP")] + valid_before: Option<String>, }, /// Remove a member from a remote's set and push the update. Remove { @@ -128,7 +135,12 @@ match action { Action::Setup { key, local } => setup(key.as_deref(), local), Action::List { remote } => list::<Signers>(&remote), - Action::Add { remote, key } => add(&remote, key.as_deref()), + Action::Add { + remote, + key, + valid_after, + valid_before, + } => add(&remote, key.as_deref(), valid_after, valid_before), Action::Remove { fingerprint, remote, @@ -150,27 +162,29 @@ } /// A `refs/meta/*` set the porcelain manages uniformly: a named ref synced from -/// and pushed to a remote, holding [`git_store::Row`] entries the CLI lists and -/// removes from. The two sets — authorized signers and configured checks — -/// share that flow and differ only in their row type and what the messages call -/// an entry; the thin [`load`](Set::load)/[`store`](Set::store) keep each on its -/// own typed module. +/// and pushed to a remote, holding entries the CLI lists and removes from. The +/// two sets — authorized signers and configured checks — share that flow and +/// differ only in their item type, what the messages call an entry, and how a +/// row presents; the thin [`load`](Set::load)/[`store`](Set::store) keep each on +/// its own typed module. trait Set { - /// The set's row type, a `(key, value)` pair under [`git_store::Row`]. - type Item: git_store::Row; + /// The set's item type. + type Item; /// The ref the set lives on. const REF: &'static str; /// The singular noun used in messages ("member", "check"). const NOUN: &'static str; - /// The set's rows. + /// The set's items. fn load(repo: &Path) -> Result<Vec<Self::Item>, String>; /// Replace the set with `items`. fn store(repo: &Path, items: &[Self::Item]) -> Result<(), String>; /// The line printed when the set is empty on `remote`. fn empty_listing(remote: &str) -> String; - /// The value column for a row, given its key and stored value. - fn row_value(key: &str, value: &str) -> String; + /// An item's key — its identity for removal and the left list column. + fn key(item: &Self::Item) -> String; + /// The right list column for an item. + fn value(item: &Self::Item) -> String; } /// The repository member set at `refs/meta/members`. @@ -193,8 +207,12 @@ format!("no members on {remote} (open bootstrap window)") } - fn row_value(_key: &str, value: &str) -> String { - key_comment(value) + fn key(item: &Signer) -> String { + item.fingerprint.clone() + } + + fn value(item: &Signer) -> String { + signer_label(item) } } @@ -218,9 +236,34 @@ format!("no checks configured on {remote}") } - fn row_value(_key: &str, value: &str) -> String { - value.to_owned() + fn key(item: &Check) -> String { + item.name.clone() } + + fn value(item: &Check) -> String { + item.command.clone() + } +} + +/// The list column for a member: its key comment and validity window, so an +/// expiry that has been set is visible at a glance rather than hidden in the +/// stored `allowed_signers` options. +fn signer_label(signer: &Signer) -> String { + let mut label = key_comment(&signer.key); + let mut window = Vec::new(); + if let Some(after) = &signer.valid_after { + window.push(format!("after {after}")); + } + if let Some(before) = &signer.valid_before { + window.push(format!("before {before}")); + } + if !window.is_empty() { + if !label.is_empty() { + label.push(' '); + } + label.push_str(&format!("({})", window.join(", "))); + } + label } /// Print each entry of the set `S` on `remote` as `<key> <value>`. @@ -233,8 +276,7 @@ return Ok(()); } for item in items { - let (key, value) = item.into_pair(); - println!("{key} {}", S::row_value(&key, &value)); + println!("{} {}", S::key(&item), S::value(&item)); } Ok(()) } @@ -243,15 +285,11 @@ fn remove<S: Set>(key: &str, remote: &str) -> Result<(), String> { let repo = repo()?; let expected = sync(remote, S::REF)?; - let before: Vec<(String, String)> = S::load(&repo)? - .into_iter() - .map(git_store::Row::into_pair) - .collect(); + let before = S::load(&repo)?; let count = before.len(); let after: Vec<S::Item> = before .into_iter() - .filter(|(k, _v)| k != key) - .map(|(k, v)| S::Item::from_pair(k, v)) + .filter(|item| S::key(item) != key) .collect(); if after.len() == count { return Err(format!("no {} named {key} on {remote}", S::NOUN)); @@ -425,8 +463,20 @@ Ok(Path::new(&home).join(".ssh").join("id_ed25519")) } -/// Authorize `key` on `remote` and push the updated set. -fn add(remote: &str, key: Option<&Path>) -> Result<(), String> { +/// Authorize `key` on `remote`, trusting it within the given validity window, +/// and push the updated set. +fn add( + remote: &str, + key: Option<&Path>, + valid_after: Option<String>, + valid_before: Option<String>, +) -> Result<(), String> { + if let Some(after) = &valid_after { + validate_timestamp(after)?; + } + if let Some(before) = &valid_before { + validate_timestamp(before)?; + } let repo = repo()?; let public_key = public_key(key)?; let fingerprint = fingerprint(&public_key)?; @@ -442,6 +492,8 @@ signers.push(Signer { fingerprint: fingerprint.clone(), key: public_key, + valid_after, + valid_before, }); signers::store(&repo, &signers).map_err(|error| error.to_string())?; push_signed(remote, MEMBERS_REF, expected.as_deref())?; @@ -449,6 +501,22 @@ Ok(()) } +/// Check that `value` is an OpenSSH `allowed_signers` timestamp: `YYYYMMDD`, +/// `YYYYMMDDHHMM`, or `YYYYMMDDHHMMSS`, each optionally suffixed `Z` for UTC. +/// Without `Z` the verifying server reads it in its own local time zone. +fn validate_timestamp(value: &str) -> Result<(), String> { + let digits = value.strip_suffix('Z').unwrap_or(value); + let well_formed = + matches!(digits.len(), 8 | 12 | 14) && digits.bytes().all(|b| b.is_ascii_digit()); + if well_formed { + Ok(()) + } else { + Err(format!( + "invalid timestamp {value:?}: expected YYYYMMDD[Z] or YYYYMMDDHHMM[SS][Z]" + )) + } +} + /// Report whether `key` is in `remote`'s set and how this client is configured. fn check(remote: &str, key: Option<&Path>) -> Result<(), String> { let repo = repo()?;
crates/git-ents/src/signers.rs @@ -1,10 +1,20 @@ //! The repository's members, sourced from the `refs/meta/members` ref. //! //! Push authentication trusts exactly one place: the `refs/meta/members` ref. -//! Its tree is a [`Members`] document mapping each fingerprint to its OpenSSH -//! public key. A member *is* one or more keys whose signed pushes are accepted. -//! The document is read and written through [`git_store`], so the trust list is -//! a typed value that lives in git — versioned, auditable, and itself pushable. +//! Its tree is a [`Members`] document mapping each fingerprint to the OpenSSH +//! public key held there and the validity window it is trusted within. A member +//! *is* one or more keys whose signed pushes are accepted while in window. The +//! document is read and written through [`git_store`], so the trust list is a +//! typed value that lives in git — versioned, auditable, and itself pushable. +//! +//! # Expiry +//! +//! Each key carries an optional `valid-after`/`valid-before` window rendered +//! into the `allowed_signers` file git verifies pushes against. The window is +//! the security primitive: an un-refreshed key stops authorizing *new* pushes +//! once it lapses, so stale trust fails closed. A previously-valid push stays +//! verifiable forever, since `ssh-keygen -Y verify -Overify-time` can pin the +//! check to the time the push was made. use std::collections::BTreeMap; use std::path::Path; @@ -15,42 +25,41 @@ pub const MEMBERS_REF: &str = "refs/meta/members"; /// The membership document stored at [`MEMBERS_REF`]: its `members/` subtree -/// maps each fingerprint to the OpenSSH public key held there. +/// maps each fingerprint to the [`Authorization`] held under it. #[derive(Debug, Clone, PartialEq, Eq, Facet)] struct Members { - members: BTreeMap<String, String>, + members: BTreeMap<String, Authorization>, } -impl git_store::MapDoc for Members { - fn from_entries(entries: BTreeMap<String, String>) -> Self { - Self { members: entries } - } - - fn into_entries(self) -> BTreeMap<String, String> { - self.members - } +/// One fingerprint's stored authorization: the OpenSSH public key it names and +/// the window that key is trusted within. The fingerprint is the map key, so it +/// is not repeated here. +#[derive(Debug, Clone, PartialEq, Eq, Facet)] +struct Authorization { + /// The OpenSSH public key the member signs with (`<type> <base64> [comment]`). + key: String, + /// The key is trusted at or after this OpenSSH timestamp; `None` is no lower + /// bound. + valid_after: Option<String>, + /// The key is trusted at or before this OpenSSH timestamp; `None` is no upper + /// bound — trust that never lapses on its own. + valid_before: Option<String>, } -/// One member's authorized signing key recorded in [`MEMBERS_REF`]. +/// One member's authorized signing key recorded in [`MEMBERS_REF`], with the +/// validity window it is trusted within. #[derive(Debug, Clone, PartialEq, Eq, Facet)] pub struct Signer { /// The key it is stored under — its fingerprint. pub fingerprint: String, /// The OpenSSH public key the blob holds (`<type> <base64> [comment]`). pub key: String, -} - -impl git_store::Row for Signer { - fn from_pair(fingerprint: String, key: String) -> Self { - Self { - fingerprint, - key: key.trim_end().to_owned(), - } - } - - fn into_pair(self) -> (String, String) { - (self.fingerprint, self.key) - } + /// The OpenSSH timestamp (`YYYYMMDD[Z]` or `YYYYMMDDHHMM[SS][Z]`) at or after + /// which the key is trusted, or `None` for no lower bound. + pub valid_after: Option<String>, + /// The OpenSSH timestamp at or before which the key is trusted, or `None` for + /// trust that never lapses on its own. + pub valid_before: Option<String>, } /// Load the members recorded at [`MEMBERS_REF`] in `repo`. @@ -59,17 +68,41 @@ /// not been pushed yet. A present but unreadable ref is an error so callers can /// fail closed rather than mistake corruption for "no members". pub fn load(repo: &Path) -> Result<Vec<Signer>, git_store::Error> { - git_store::Store::open(repo)?.load_rows::<Members, Signer>(MEMBERS_REF) + let Some(doc) = git_store::Store::open(repo)?.load::<Members>(MEMBERS_REF)? else { + return Ok(Vec::new()); + }; + Ok(doc + .members + .into_iter() + .map(|(fingerprint, held)| Signer { + fingerprint, + key: held.key, + valid_after: held.valid_after, + valid_before: held.valid_before, + }) + .collect()) } /// Write `signers` to [`MEMBERS_REF`], replacing any existing set, as a new /// commit. pub fn store(repo: &Path, signers: &[Signer]) -> Result<(), git_store::Error> { - git_store::Store::open(repo)?.store_rows::<Members, _>( - MEMBERS_REF, - signers.iter().cloned(), - "Update members", - ) + let members = Members { + members: signers + .iter() + .map(|signer| { + ( + signer.fingerprint.clone(), + Authorization { + key: signer.key.clone(), + valid_after: signer.valid_after.clone(), + valid_before: signer.valid_before.clone(), + }, + ) + }) + .collect(), + }; + git_store::Store::open(repo)?.store(MEMBERS_REF, &members, "Update members")?; + Ok(()) } /// Render `signers` as an OpenSSH `allowed_signers` file that authorizes any @@ -79,12 +112,27 @@ /// key set, not a binding between a key and a particular identity: `ssh-keygen /// -Y verify` accepts the push certificate as long as the signing key is one of /// these, whatever name the pusher signed under. +/// +/// Each key's `valid-after`/`valid-before` window is rendered as `allowed_signers` +/// options so git enforces expiry: out-of-window keys are not accepted. Options +/// are comma-joined, the syntax OpenSSH requires for more than one. #[must_use] pub fn allowed_signers(signers: &[Signer]) -> String { - signers - .iter() - .map(|signer| format!("* namespaces=\"git\" {}\n", signer.key)) - .collect() + signers.iter().map(allowed_signers_line).collect() +} + +/// One `allowed_signers` line for `signer`: the wildcard principal, its validity +/// window, the git namespace, and the key. +fn allowed_signers_line(signer: &Signer) -> String { + let mut options = Vec::new(); + if let Some(after) = &signer.valid_after { + options.push(format!("valid-after=\"{after}\"")); + } + if let Some(before) = &signer.valid_before { + options.push(format!("valid-before=\"{before}\"")); + } + options.push("namespaces=\"git\"".to_owned()); + format!("* {} {}\n", options.join(","), signer.key) } #[cfg(test)] @@ -96,7 +144,7 @@ )] use super::*; - use crate::testutil::{unique_repo as new_repo, write_meta_doc}; + use crate::testutil::{unique_repo as new_repo, write_members_doc}; const KEY_A: &str = "ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaA alice"; @@ -111,13 +159,18 @@ Signer { fingerprint: fingerprint.to_owned(), key: key.to_owned(), + valid_after: None, + valid_before: None, } } #[test] fn store_then_load_round_trips_the_signer_set() { let repo = unique_repo(); - let written = vec![signer("SHA256-aaa", KEY_A), signer("SHA256-bbb", KEY_B)]; + let mut bounded = signer("SHA256-bbb", KEY_B); + bounded.valid_after = Some("20260101".to_owned()); + bounded.valid_before = Some("20270101".to_owned()); + let written = vec![signer("SHA256-aaa", KEY_A), bounded]; store(&repo, &written).unwrap(); let mut loaded = load(&repo).unwrap(); @@ -144,22 +197,23 @@ #[test] fn loads_the_on_disk_members_format() { - // A fixture written as the real `members/<fingerprint>` blob layout must - // keep loading; this fails if the Members document's shape changes - // incompatibly with data already on a ref. + // A fixture written as the real `members/<fingerprint>/{key,valid_after, + // valid_before}` layout must keep loading; this fails if the Members + // document's shape changes incompatibly with data already on a ref. let repo = unique_repo(); - write_meta_doc( + write_members_doc( &repo, MEMBERS_REF, - "members", - &[("aa:bb:cc", KEY_A), ("dd:ee:ff", KEY_B)], + &[ + ("aa:bb:cc", KEY_A, None, None), + ("dd:ee:ff", KEY_B, None, Some("20270101")), + ], ); let mut loaded = load(&repo).unwrap(); loaded.sort_by(|a, b| a.fingerprint.cmp(&b.fingerprint)); - assert_eq!( - loaded, - vec![signer("aa:bb:cc", KEY_A), signer("dd:ee:ff", KEY_B)] - ); + let mut expected_b = signer("dd:ee:ff", KEY_B); + expected_b.valid_before = Some("20270101".to_owned()); + assert_eq!(loaded, vec![signer("aa:bb:cc", KEY_A), expected_b]); let _ = std::fs::remove_dir_all(&repo); } @@ -170,4 +224,17 @@ format!("* namespaces=\"git\" {KEY_A}\n") ); } + + #[test] + fn renders_the_validity_window_as_comma_joined_options() { + let mut bounded = signer("SHA256-aaa", KEY_A); + bounded.valid_after = Some("20260101".to_owned()); + bounded.valid_before = Some("20270101".to_owned()); + assert_eq!( + allowed_signers(&[bounded]), + format!( + "* valid-after=\"20260101\",valid-before=\"20270101\",namespaces=\"git\" {KEY_A}\n" + ) + ); + } }
crates/git-ents/src/testutil.rs @@ -70,6 +70,55 @@ assert!(status.success()); } +/// Lay a `Members` document out at `refname` as the real on-disk format: a +/// `members/<fingerprint>/` subtree per member holding a `key` blob and an +/// `valid_after`/`valid_before` `Option` subtree each (empty tree for `None`, a +/// single `some` blob for a bound). Asserts the loader still reads the format +/// independent of the writer. +pub(crate) fn write_members_doc( + repo: &Path, + refname: &str, + members: &[(&str, &str, Option<&str>, Option<&str>)], +) { + let option_tree = |bound: Option<&str>| match bound { + None => git_with_stdin(repo, &["mktree"], ""), + Some(value) => { + let blob = git_with_stdin(repo, &["hash-object", "-w", "--stdin"], value); + git_with_stdin(repo, &["mktree"], &format!("100644 blob {blob}\tsome\n")) + } + }; + let mut member_entries = String::new(); + for (fingerprint, key, valid_after, valid_before) in members { + let key_blob = git_with_stdin(repo, &["hash-object", "-w", "--stdin"], key); + let after_tree = option_tree(*valid_after); + let before_tree = option_tree(*valid_before); + let member_tree = git_with_stdin( + repo, + &["mktree"], + &format!( + "100644 blob {key_blob}\tkey\n\ + 040000 tree {after_tree}\tvalid_after\n\ + 040000 tree {before_tree}\tvalid_before\n" + ), + ); + member_entries.push_str(&format!("040000 tree {member_tree}\t{fingerprint}\n")); + } + let members_tree = git_with_stdin(repo, &["mktree"], &member_entries); + let root = git_with_stdin( + repo, + &["mktree"], + &format!("040000 tree {members_tree}\tmembers\n"), + ); + let commit = git_with_stdin(repo, &["commit-tree", &root, "-m", "fixture"], ""); + let status = Command::new("git") + .arg("-C") + .arg(repo) + .args(["update-ref", refname, &commit]) + .status() + .unwrap(); + assert!(status.success()); +} + /// Lay a `Config` document out at `refname` as the real on-disk format: a /// `description` blob, a `homepage` blob, and a `topics/` subtree of index-keyed /// (`0000`, `0001`, …) blobs, committed and pointed to by the ref. Asserts the