git-ents.gitmain
⌘K
foforge
commit 5acebd1
feat: sign in by proving a member key and attribute web edits to the human

Reworks browser editing to GitHub’s shape without GitHub’s central trust. Signing in no longer surrenders a private key: the server issues a one-time challenge, the member signs it locally (ssh-keygen -Y sign), and pastes back the signature and public key, which the server verifies — the key never leaves their machine, and the session holds only the public key. An edit is then signed with the server’s own member key through the same pre-receive gate, but the commit is authored by the signed-in member and committed by the git-ents identity, so history reads "$USERNAME via the web". Because no member secret is ever held, encrypted-at-rest persistence is moot and was not added.

feat: add git_store::Store::store_authored to attribute a commit’s author feat: add git_ents::config::store_to_ref_authored feat: sign in via a one-time challenge proving control of a member key feat: sign browser edits with the server’s own --web-signing-key, authored by the member deprecates: the paste-the-private-key web sign-in 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/src/http.rs @@ -562,6 +562,8 @@ hooks_dir: hooks_dir.map(PathBuf::from), checks_queue: PathBuf::from("/data/checks-queue"), sessions: crate::web::new_sessions(), + challenges: crate::web::new_challenges(), + web_signing_key: None, } }
crates/git-ents-server/src/main.rs @@ -47,6 +47,12 @@ #[arg(long, env = "GIT_ENTS_HOOKS_DIR")] hooks_dir: Option<PathBuf>, + /// The server's own SSH private key, used to sign browser-made edits. Its + /// public half must be a member of any repo edited through the web. Editing + /// is disabled unless this is set. + #[arg(long, env = "GIT_ENTS_WEB_SIGNING_KEY")] + web_signing_key: Option<PathBuf>, + /// Directory where the `post-receive` hook queues pushes for the check /// worker to run asynchronously. #[arg( @@ -83,9 +89,14 @@ /// Directory the `post-receive` hook queues pushes into and the check /// worker drains; passed down to the hook via [`checks::QUEUE_ENV`]. pub(crate) checks_queue: PathBuf, - /// In-memory web sessions: a browser's signed-in web key, held for the life - /// of the process and never persisted. + /// In-memory web sessions: a browser's signed-in public key, held for the + /// life of the process and never persisted. pub(crate) sessions: web::Sessions, + /// Outstanding one-time sign-in challenges awaiting a signature. + pub(crate) challenges: web::Challenges, + /// The server's own signing key for browser-made edits; `None` disables + /// editing. + pub(crate) web_signing_key: Option<PathBuf>, } fn main() -> ExitCode { @@ -138,6 +149,8 @@ hooks_dir: args.hooks_dir, checks_queue: args.checks_queue, sessions: web::new_sessions(), + challenges: web::new_challenges(), + web_signing_key: args.web_signing_key, }; // Drain queued pushes and run their checks for the life of the server.
crates/git-ents-server/tests/web_edit.rs @@ -6,9 +6,11 @@ reason = "integration test binary" )] -//! End-to-end coverage for authenticated browser edits: signing in with a web -//! key, then saving a settings change that must travel through the real -//! `pre-receive` gate as a signed push before it lands on `refs/meta/config`. +//! End-to-end coverage for authenticated browser edits: proving control of a +//! member key by signing a one-time challenge (the key never leaves the client), +//! then saving a settings change that travels through the real `pre-receive` +//! gate — signed with the server's own key, authored by the member — before it +//! lands on `refs/meta/config`. use std::io::{Read, Write}; use std::net::{TcpListener, TcpStream}; @@ -16,19 +18,17 @@ use std::process::{Child, Command, Stdio}; const BIN: &str = env!("CARGO_BIN_EXE_git-ents-server"); +const LOGIN_NAMESPACE: &str = "git-ents-login"; #[test] fn a_member_edits_settings_through_the_browser() { let env = Server::start(); let bare = env.create_repo("repo.git"); - let key = keygen(env.scratch(), "web"); - env.add_member(&bare, "alice", &pubkey(&key)); + env.add_server_member(&bare); + let alice = keygen(env.scratch(), "alice"); + env.add_member(&bare, "alice", &pubkey(&alice)); - // Sign in with the web key; the server derives its public half and opens a - // session whose cookie we carry from here on. - let signin = env.post("/login", "", &form(&[("private_key", &read(&key))])); - assert_eq!(signin.status, 303, "sign-in should redirect"); - let cookie = signin.session_cookie().unwrap(); + let cookie = env.sign_in(&alice); // The settings page now offers an edit form; read its CSRF token. let page = env.get("/repo.git/settings", &cookie); @@ -36,9 +36,8 @@ page.body.contains("name=\"csrf\""), "edit form should render" ); - let csrf = page.csrf().unwrap(); + let csrf = page.field("csrf").unwrap(); - // Save a change to every General field. let edit = env.post( "/repo.git/settings", &cookie, @@ -68,14 +67,17 @@ ); assert!(after.body.contains("forge"), "topics did not land"); - // And the gate really moved the ref: a fresh signed commit now sits on it. - assert!( - git( - &bare, - &["rev-parse", "--verify", "--quiet", "refs/meta/config"] - ) - .is_some(), - "refs/meta/config should exist after the edit" + // "$USERNAME via Web": the commit is authored by the member and committed by + // the server identity. + assert_eq!( + git(&bare, &["log", "-1", "--format=%an", "refs/meta/config"]).as_deref(), + Some("alice"), + "the edit should be authored by the member" + ); + assert_eq!( + git(&bare, &["log", "-1", "--format=%cn", "refs/meta/config"]).as_deref(), + Some("git-ents"), + "the committer should be the server identity" ); } @@ -83,14 +85,11 @@ fn an_edit_without_a_valid_csrf_token_is_refused() { let env = Server::start(); let bare = env.create_repo("repo.git"); - let key = keygen(env.scratch(), "web"); - env.add_member(&bare, "alice", &pubkey(&key)); - - let cookie = env - .post("/login", "", &form(&[("private_key", &read(&key))])) - .session_cookie() - .unwrap(); + env.add_server_member(&bare); + let alice = keygen(env.scratch(), "alice"); + env.add_member(&bare, "alice", &pubkey(&alice)); + let cookie = env.sign_in(&alice); let edit = env.post( "/repo.git/settings", &cookie, @@ -98,10 +97,9 @@ ); assert_eq!(edit.status, 200, "a bad-CSRF edit should not redirect"); assert!( - env.get("/repo.git/settings", &cookie) + !env.get("/repo.git/settings", &cookie) .body - .contains("name=\"csrf\"") - && !page_description_is(&env, &cookie, "sneaky"), + .contains("sneaky"), "the description must be unchanged" ); } @@ -110,15 +108,14 @@ fn a_non_member_is_not_offered_an_edit_form() { let env = Server::start(); let bare = env.create_repo("repo.git"); + env.add_server_member(&bare); let member = keygen(env.scratch(), "member"); env.add_member(&bare, "alice", &pubkey(&member)); - // Sign in with a key that is *not* a member of this repo. + // A real key that is simply not a member of this repo signs in fine — a + // session proves key control, not authority. let intruder = keygen(env.scratch(), "intruder"); - let cookie = env - .post("/login", "", &form(&[("private_key", &read(&intruder))])) - .session_cookie() - .unwrap(); + let cookie = env.sign_in(&intruder); let page = env.get("/repo.git/settings", &cookie); assert!( @@ -131,23 +128,45 @@ ); } -/// Whether the rendered settings page shows `value` as the description. -fn page_description_is(env: &Server, cookie: &str, value: &str) -> bool { - env.get("/repo.git/settings", cookie).body.contains(value) +#[test] +fn a_signature_that_does_not_match_the_public_key_is_refused() { + let env = Server::start(); + let alice = keygen(env.scratch(), "alice"); + let mallory = keygen(env.scratch(), "mallory"); + + // Sign the challenge with mallory's key but claim alice's public key. + let nonce = env.get("/login", "").field("nonce").unwrap(); + let signature = sign_nonce(&mallory, &nonce); + let attempt = env.post( + "/login", + "", + &form(&[ + ("nonce", &nonce), + ("public_key", &pubkey(&alice)), + ("signature", &signature), + ]), + ); + assert_eq!( + attempt.status, 200, + "a mismatched signature must not open a session" + ); + assert!( + attempt.session_cookie().is_none(), + "no session cookie should be set" + ); } -/// A running server with its data and hooks directories. +/// A running server enforcing the signed-push gate and holding a web signing key. struct Server { child: Child, port: u16, data: tempfile::TempDir, scratch: tempfile::TempDir, + server_key: PathBuf, _hooks: tempfile::TempDir, } impl Server { - /// Start a server enforcing the signed-push gate: a nonce seed plus a hooks - /// directory whose `pre-receive` is the compiled verifier. fn start() -> Self { let data = tempfile::tempdir().unwrap(); let scratch = tempfile::tempdir().unwrap(); @@ -159,17 +178,18 @@ use std::os::unix::fs::PermissionsExt; std::fs::set_permissions(&hook, std::fs::Permissions::from_mode(0o755)).unwrap(); } + let server_key = keygen(scratch.path(), "web-server"); let port = free_port(); let child = Command::new(BIN) - .arg("--port") - .arg(port.to_string()) + .args(["--port", &port.to_string()]) .arg("--data-dir") .arg(data.path()) - .arg("--cert-nonce-seed") - .arg("test-seed") + .args(["--cert-nonce-seed", "test-seed"]) .arg("--hooks-dir") .arg(hooks.path()) + .arg("--web-signing-key") + .arg(&server_key) .spawn() .unwrap(); wait_for_port(port); @@ -178,6 +198,7 @@ port, data, scratch, + server_key, _hooks: hooks, } } @@ -186,8 +207,7 @@ self.scratch.path() } - /// Create a bare repo by pushing an initial commit to it (auto-init), and - /// return its on-disk path. + /// Create a bare repo by pushing an initial commit to it (auto-init). fn create_repo(&self, name: &str) -> PathBuf { let work = self.scratch.path().join(format!("work-{name}")); std::fs::create_dir_all(&work).unwrap(); @@ -204,9 +224,34 @@ self.data.path().join(name) } + /// Add the server's own key as a member, so it may sign web edits. + fn add_server_member(&self, bare: &Path) { + self.add_member(bare, "web-server", &pubkey(&self.server_key)); + } + + /// Sign in with `key` via the challenge flow, returning the session cookie. + fn sign_in(&self, key: &Path) -> String { + let nonce = self.get("/login", "").field("nonce").unwrap(); + let signature = sign_nonce(key, &nonce); + let response = self.post( + "/login", + "", + &form(&[ + ("nonce", &nonce), + ("public_key", &pubkey(key)), + ("signature", &signature), + ]), + ); + assert_eq!( + response.status, 303, + "sign-in should redirect: {}", + response.body + ); + response.session_cookie().unwrap() + } + /// Write a member ref `refs/meta/member/<username>` into `bare` directly, in - /// the on-disk layout the loader reads: a `principal` blob, empty - /// `valid_after`/`valid_before` subtrees, and a `trust/Keys/key` blob. + /// the on-disk layout the loader reads. fn add_member(&self, bare: &Path, username: &str, public_key: &str) { let principal = hash_object(bare, username.as_bytes()); let key_blob = hash_object(bare, public_key.as_bytes()); @@ -235,7 +280,12 @@ } fn get(&self, path: &str, cookie: &str) -> Http { - self.request("GET", path, &[("Cookie", cookie)], "") + let headers: Vec<(&str, &str)> = if cookie.is_empty() { + vec![] + } else { + vec![("Cookie", cookie)] + }; + self.request("GET", path, &headers, "") } fn post(&self, path: &str, cookie: &str, body: &str) -> Http { @@ -246,7 +296,6 @@ self.request("POST", path, &headers, body) } - /// Send one HTTP/1.0 request and parse the response. fn request(&self, method: &str, path: &str, headers: &[(&str, &str)], body: &str) -> Http { let mut request = format!("{method} {path} HTTP/1.0\r\nHost: 127.0.0.1\r\n"); for (name, value) in headers { @@ -297,8 +346,6 @@ } } - /// The `ents_session=<token>` pair from a `Set-Cookie` header, ready to send - /// back as a `Cookie` value. fn session_cookie(&self) -> Option<String> { self.headers .iter() @@ -308,10 +355,10 @@ .map(str::to_owned) } - /// The CSRF token rendered in the page's hidden field. - fn csrf(&self) -> Option<String> { - let marker = "name=\"csrf\" value=\""; - let start = self.body.find(marker)? + marker.len(); + /// The value of a hidden form field rendered as `name="<field>" value="…"`. + fn field(&self, field: &str) -> Option<String> { + let marker = format!("name=\"{field}\" value=\""); + let start = self.body.find(&marker)? + marker.len(); let rest = self.body.get(start..)?; let end = rest.find('"')?; rest.get(..end).map(str::to_owned) @@ -350,12 +397,36 @@ key } -fn pubkey(private: &Path) -> String { - read(&private.with_extension("pub")).trim().to_owned() +/// Sign `nonce` under the login namespace with `key`, returning the SSHSIG. +fn sign_nonce(key: &Path, nonce: &str) -> String { + let mut child = Command::new("ssh-keygen") + .args(["-Y", "sign", "-n", LOGIN_NAMESPACE, "-f"]) + .arg(key) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .unwrap(); + child + .stdin + .take() + .unwrap() + .write_all(nonce.as_bytes()) + .unwrap(); + let output = child.wait_with_output().unwrap(); + assert!( + output.status.success(), + "ssh-keygen -Y sign failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout).into_owned() } -fn read(path: &Path) -> String { - std::fs::read_to_string(path).unwrap() +fn pubkey(private: &Path) -> String { + std::fs::read_to_string(private.with_extension("pub")) + .unwrap() + .trim() + .to_owned() } /// Encode form fields as `application/x-www-form-urlencoded`. @@ -367,7 +438,7 @@ .join("&") } -/// Percent-encode one form value. +/// Percent-encode one form value, leaving only the unreserved set unescaped. fn encode(value: &str) -> String { value .bytes() @@ -413,7 +484,6 @@ .status .success() .then(|| String::from_utf8_lossy(&output.stdout).trim().to_owned()) - .filter(|out| !out.is_empty() || args.first() == Some(&"update-ref")) } fn hash_object(bare: &Path, bytes: &[u8]) -> String {
crates/git-ents/src/config.rs @@ -56,6 +56,25 @@ Ok(()) } +/// Like [`store_to_ref`], but recording `author` (a `(name, email)` pair) as the +/// commit's author while the committer stays the git-ents system identity. The +/// web write path uses this so an edit landed by the server still names the human +/// who made it. +pub fn store_to_ref_authored( + repo: &Path, + refname: &str, + config: &Config, + author: (&str, &str), +) -> Result<(), git_store::Error> { + git_store::Store::open(repo)?.store_authored( + refname, + config, + "Update configuration", + author, + )?; + Ok(()) +} + #[cfg(test)] mod tests { #![allow(
crates/git-store/src/lib.rs @@ -117,7 +117,24 @@ ) -> Result<(), Error> { let tree = facet_git_tree::serialize_into(value, &self.odb)?; let parents = self.ref_commit(refname)?.into_iter().collect(); - let commit = self.write_commit(tree, parents, message)?; + let commit = self.write_commit(tree, parents, message, None)?; + self.set_ref(refname, commit) + } + + /// Like [`store`](Self::store), but attributing authorship to `author` + /// (a `(name, email)` pair) while the committer stays the git-ents system + /// identity — the way a web edit records the human who made the change while + /// the server is the committer. + pub fn store_authored<T: for<'a> Facet<'a>>( + &self, + refname: &str, + value: &T, + message: &str, + author: (&str, &str), + ) -> Result<(), Error> { + let tree = facet_git_tree::serialize_into(value, &self.odb)?; + let parents = self.ref_commit(refname)?.into_iter().collect(); + let commit = self.write_commit(tree, parents, message, Some(author))?; self.set_ref(refname, commit) } @@ -136,7 +153,7 @@ Some(tip) => self.read_commit(&tip)?.parents, None => Vec::new(), }; - let commit = self.write_commit(tree, parents, message)?; + let commit = self.write_commit(tree, parents, message, None)?; self.set_ref(refname, commit) } @@ -257,22 +274,34 @@ } /// Wrap `tree` in a commit over `parents` and write it to the durable store. + /// The committer is always the git-ents system identity; `author` overrides + /// the authorship when set, otherwise it too is the system identity. fn write_commit( &self, tree: ObjectId, parents: Vec<ObjectId>, message: &str, + author: Option<(&str, &str)>, ) -> Result<ObjectId, Error> { - let signature = gix::actor::Signature { + let time = gix::date::Time::now_utc(); + let committer = gix::actor::Signature { name: IDENTITY_NAME.into(), email: IDENTITY_EMAIL.into(), - time: gix::date::Time::now_utc(), + time, + }; + let author = match author { + Some((name, email)) => gix::actor::Signature { + name: name.into(), + email: email.into(), + time, + }, + None => committer.clone(), }; let commit = Commit { tree, parents: parents.into(), - author: signature.clone(), - committer: signature, + author, + committer, encoding: None, message: message.into(), extra_headers: Vec::new(),
crates/git-ents-server/src/web/mod.rs @@ -26,7 +26,7 @@ use crate::AppState; use crate::http::{MAX_REPO_DEPTH, is_bare_repo, valid_segment}; -pub(crate) use self::write::{Sessions, new_sessions}; +pub(crate) use self::write::{Challenges, Sessions, new_challenges, new_sessions}; /// Who is signed in for the current request, resolved per repository: a member's /// web key authorizes edits only on a repo whose member list contains it. @@ -59,7 +59,11 @@ return index(state, session.as_ref()).into_response(); } if segments == ["login"] { - return login_page(session.as_ref(), None).into_response(); + let challenge = match session { + Some(_) => None, + None => write::issue_challenge(&state.challenges).ok(), + }; + return login_page(session.as_ref(), challenge.as_deref(), None).into_response(); } if let Some((repo, rel, rest)) = resolve_repo(&state.data_dir, &segments) { @@ -111,9 +115,12 @@ let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); if segments == ["login"] { - return match write::login(&state.sessions, &body) { + return match write::login(&state.sessions, &state.challenges, &body) { Ok(token) => redirect("/login", Some(session_cookie(&token, secure))), - Err(error) => login_page(None, Some(&error)).into_response(), + Err(error) => { + let challenge = write::issue_challenge(&state.challenges).ok(); + login_page(None, challenge.as_deref(), Some(&error)).into_response() + } }; } if segments == ["logout"] { @@ -158,10 +165,14 @@ cookie: Option<&str>, body: Bytes, ) -> Response { - let (Some(seed), Some(hooks)) = (state.cert_nonce_seed.clone(), state.hooks_dir.clone()) else { + let (Some(seed), Some(hooks), Some(signing_key)) = ( + state.cert_nonce_seed.clone(), + state.hooks_dir.clone(), + state.web_signing_key.clone(), + ) else { return edit_error( rel, - "Editing is disabled: this server is not enforcing the signed-push gate.", + "Editing is disabled: this server has no web signing key or signed-push gate.", ) .into_response(); }; @@ -189,7 +200,15 @@ let cookie = cookie.map(str::to_owned); let repo = repo.to_owned(); let result = tokio::task::spawn_blocking(move || { - write::edit_config(&sessions, cookie.as_deref(), &repo, &edit, &seed, &hooks) + write::edit_config( + &sessions, + cookie.as_deref(), + &repo, + &edit, + &seed, + &hooks, + &signing_key, + ) }) .await; @@ -501,9 +520,14 @@ } } -/// The sign-in page: paste a web key to open a session. `error` shows a failed -/// attempt's reason. -fn login_page(session: Option<&write::SessionSnapshot>, error: Option<&str>) -> Markup { +/// The sign-in page: prove control of a member key by signing a one-time +/// challenge locally, without ever surrendering the key. `error` shows a failed +/// attempt's reason; `challenge` is the nonce to sign. +fn login_page( + session: Option<&write::SessionSnapshot>, + challenge: Option<&str>, + error: Option<&str>, +) -> Markup { page( "Sign in", html! { @@ -511,23 +535,33 @@ div.page-header { h1.page-title { "Sign in" } } @if let Some(s) = session { p { "Signed in as " strong { (s.label) } "." } - p.muted { "Your web key signs edits made in the browser." } + p.muted { "Edits you make in the browser are attributed to your member key." } } @else { p.shell-note { - "Paste the " strong { "private" } " half of a web key whose public half you " - "have added to your member ref. It is held in memory for this session only " - "and is never written to disk." + "Prove control of a web key whose public half is a member of the repository. " + "Sign the one-time challenge below on your own machine — the key never leaves it." } @if let Some(error) = error { div.card-row.muted { "Could not sign in: " (error) } } - form.edit-form method="post" action="/login" { - label { "Key name (optional)" } - input type="text" name="label" placeholder="laptop web key"; - label { "Private key" } - textarea name="private_key" rows="8" spellcheck="false" - placeholder="-----BEGIN OPENSSH PRIVATE KEY-----" {} - button.btn type="submit" { "Sign in" } + @if let Some(nonce) = challenge { + p.shell-note { "Run this, then paste the output and your public key:" } + pre.signin-cmd { + "printf %s '" (nonce) "' | ssh-keygen -Y sign -n " + (write::LOGIN_NAMESPACE) " -f ~/.ssh/your_web_key" + } + form.edit-form method="post" action="/login" { + input type="hidden" name="nonce" value=(nonce); + label { "Public key" } + input type="text" name="public_key" spellcheck="false" + placeholder="ssh-ed25519 AAAA… you@host"; + label { "Signature" } + textarea name="signature" rows="8" spellcheck="false" + placeholder="-----BEGIN SSH SIGNATURE-----" {} + button.btn type="submit" { "Sign in" } + } + } @else { + div.card-row.muted { "Could not start a sign-in challenge; reload to retry." } } } },
crates/git-ents-server/src/web/style.css @@ -365,3 +365,4 @@ .inline-edit { display: flex; gap: .5rem; flex: 1; align-items: center; } .inline-edit input { flex: 1; min-width: 0; } .inline-edit .btn { margin-top: 0; padding: .35rem .85rem; } +.signin-cmd { font-family: var(--font-mono); font-size: .82rem; color: var(--color-text); background: var(--color-code-bg); border: 1px solid var(--color-border); border-radius: var(--radius-sm); padding: .7rem .9rem; margin: 0 0 1rem; white-space: pre-wrap; word-break: break-all; }
crates/git-ents-server/src/web/write.rs @@ -1,43 +1,55 @@ //! Authenticated browser writes. //! -//! A web session holds one member's *web key* — a key whose public half they -//! have already added to their member ref through a normal signed push. An edit -//! made in the browser is performed as a real `git push --signed` into the repo, -//! so it travels through the very same `pre-receive` gate a command-line push -//! does. Nothing here is a second trust path: the gate alone decides whether a -//! change lands; this module only stages the change and produces a signed push -//! for it to judge. +//! Signing in never surrenders a private key. The server issues a one-time +//! challenge; the member signs it locally with their web key and pastes back the +//! signature and their public key. The server verifies that signature against +//! the pasted key, which proves the browser controls it — the same proof a CLI +//! push gives, without the key ever leaving the member's machine. //! -//! The web key lives in memory for the life of the process and is never written -//! to disk. A server restart drops every session. +//! An edit is then landed as a real `git push --signed` onto `refs/meta/config`, +//! signed with the *server's own* member key, so it passes the very same +//! `pre-receive` gate a CLI push does. The commit's author is the signed-in +//! human (resolved from their key's membership); the committer is the server. +//! Nothing secret to the member is ever held or persisted: a session keeps only +//! their public key. use std::collections::HashMap; -use std::io::Read as _; +use std::io::{Read as _, Write as _}; use std::path::Path; use std::process::{Command, Stdio}; use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; /// The cookie that carries a session token. pub(super) const COOKIE: &str = "ents_session"; +/// The SSHSIG namespace a sign-in signature is made under — distinct from git's +/// own `git` namespace, so a login signature can never double as a push and vice +/// versa. +pub(super) const LOGIN_NAMESPACE: &str = "git-ents-login"; + +/// How long an issued sign-in challenge stays valid. +const CHALLENGE_TTL: Duration = Duration::from_secs(600); + /// In-memory session table, shared by every handler. pub(crate) type Sessions = Arc<Mutex<HashMap<String, Session>>>; -/// One browser session: the web key it signs edits with and a display label. +/// Outstanding sign-in challenges and when each was issued; consumed once. +pub(crate) type Challenges = Arc<Mutex<HashMap<String, Instant>>>; + +/// One browser session. It holds only the member's *public* key — enough to +/// authorize per repository — plus a display label and a CSRF token. pub(crate) struct Session { - /// The PEM private key the session signs pushes with. In memory only. - private_key: String, - /// The derived public key line (`type base64`), matched against members. + /// The member's public key line (`type base64`), matched against members. public_key: String, - /// A human label for the key — its given name, or its type. + /// A human label for the key — its comment, or its type. label: String, /// A per-session token that state-changing form posts must echo back, so a /// cross-site request (which cannot read it) cannot act as the user. csrf: String, } -/// A cheap, cloneable view of a session for rendering and authorization, without -/// the private key. +/// A cheap, cloneable view of a session for rendering and authorization. #[derive(Clone)] pub(super) struct SessionSnapshot { pub(super) label: String, @@ -57,6 +69,34 @@ Arc::new(Mutex::new(HashMap::new())) } +/// Create an empty challenge table. +pub(crate) fn new_challenges() -> Challenges { + Arc::new(Mutex::new(HashMap::new())) +} + +/// Issue a fresh one-time sign-in challenge, returning the nonce to sign. +pub(super) fn issue_challenge(challenges: &Challenges) -> Result<String, String> { + let nonce = random_token()?; + let mut table = challenges + .lock() + .map_err(|_poisoned| "challenge store unavailable".to_owned())?; + let now = Instant::now(); + table.retain(|_nonce, issued| now.duration_since(*issued) < CHALLENGE_TTL); + table.insert(nonce.clone(), now); + Ok(nonce) +} + +/// Consume `nonce`, returning whether it was a live, unexpired challenge. +fn take_challenge(challenges: &Challenges, nonce: &str) -> bool { + let Ok(mut table) = challenges.lock() else { + return false; + }; + match table.remove(nonce) { + Some(issued) => Instant::now().duration_since(issued) < CHALLENGE_TTL, + None => false, + } +} + /// The session a `Cookie` header points at, as a snapshot, if any. pub(super) fn snapshot(sessions: &Sessions, cookie: Option<&str>) -> Option<SessionSnapshot> { let token = token(cookie?)?; @@ -69,29 +109,30 @@ }) } -/// Open a session for the web key in `body` (a `private_key` form field), set its -/// cookie, and return the token. The key is accepted as long as it parses; an -/// edit is authorized per-repository against the live member list, so holding a -/// session grants nothing on its own. -pub(super) fn login(sessions: &Sessions, body: &[u8]) -> Result<String, String> { +/// Complete a sign-in: verify the pasted `signature` over the issued `nonce` +/// against the pasted `public_key`, and on success open a session and return its +/// token. Holding a session grants nothing on its own — an edit is authorized +/// per repository against the live member list. +pub(super) fn login( + sessions: &Sessions, + challenges: &Challenges, + body: &[u8], +) -> Result<String, String> { let fields = form(body); - let private_key = fields - .get("private_key") - .map(String::as_str) - .unwrap_or_default() - .trim() - .to_owned(); - if private_key.is_empty() { - return Err("paste a private key to sign in".to_owned()); + let public_key = trimmed(&fields, "public_key"); + let signature = trimmed(&fields, "signature"); + let nonce = trimmed(&fields, "nonce"); + if public_key.is_empty() || signature.is_empty() { + return Err("paste your public key and the signature".to_owned()); + } + if !take_challenge(challenges, &nonce) { + return Err("your sign-in challenge expired; reload and try again".to_owned()); + } + if !verify_login_signature(&public_key, &nonce, &signature)? { + return Err("the signature did not match that public key".to_owned()); } - let public_key = derive_public_key(&private_key)?; - let label = fields - .get("label") - .map(|l| l.trim()) - .filter(|l| !l.is_empty()) - .map(str::to_owned) - .unwrap_or_else(|| key_type(&public_key)); + let label = key_comment(&public_key).unwrap_or_else(|| key_type(&public_key)); let token = random_token()?; let csrf = random_token()?; let mut table = sessions @@ -100,8 +141,7 @@ table.insert( token.clone(), Session { - private_key, - public_key, + public_key: normalize_key(&public_key), label, csrf, }, @@ -131,13 +171,14 @@ } } -/// Land a configuration change by staging it on a throwaway ref and pushing it, -/// signed with the session's web key, onto `refs/meta/config` — through the -/// `pre-receive` gate. Returns `Ok` only when the gate accepts the push. +/// Land a configuration change: stage it on a throwaway ref authored by the +/// signed-in member, then push it onto `refs/meta/config` signed with the +/// server's key, through the `pre-receive` gate. Returns `Ok` only when the gate +/// accepts the push. /// -/// `seed` and `hooks` are the server's signed-push nonce seed and hooks -/// directory; both are required, so a web edit is never a way around a server -/// that is not enforcing the gate. +/// `seed`, `hooks`, and `signing_key` are the server's signed-push nonce seed, +/// hooks directory, and own member key; all are required, so a web edit is never +/// a way around a server that is not enforcing the gate. pub(super) fn edit_config( sessions: &Sessions, cookie: Option<&str>, @@ -145,18 +186,20 @@ edit: &ConfigEdit, seed: &str, hooks: &Path, + signing_key: &Path, ) -> Result<(), String> { let token = cookie .and_then(token) .ok_or_else(|| "sign in to edit settings".to_owned())?; - let (private_key, public_key) = { + let public_key = { let table = sessions .lock() .map_err(|_poisoned| "session store unavailable".to_owned())?; - let session = table + table .get(&token) - .ok_or_else(|| "sign in to edit settings".to_owned())?; - (session.private_key.clone(), session.public_key.clone()) + .ok_or_else(|| "sign in to edit settings".to_owned())? + .public_key + .clone() }; let username = member_for_public_key(repo, &public_key) @@ -169,28 +212,21 @@ config.topics = edit.topics.clone(); let staging = format!("refs/web-staging/{}", random_token()?); - let result = stage_and_push( - repo, - &staging, - &config, - &private_key, - &username, - seed, - hooks, - ); + let result = stage_and_push(repo, &staging, &config, &username, signing_key, seed, hooks); // Clean up the staging ref whether or not the push was accepted. let _cleanup = git(repo, &["update-ref", "-d", &staging]); result } -/// Point `staging` at the current config tip, build the new config commit on it, -/// then push it signed onto `refs/meta/config`. +/// Point `staging` at the current config tip, build the new config commit on it +/// authored by `username`, then push it signed with the server's key onto +/// `refs/meta/config`. fn stage_and_push( repo: &Path, staging: &str, config: &git_ents::config::Config, - private_key: &str, username: &str, + signing_key: &Path, seed: &str, hooks: &Path, ) -> Result<(), String> { @@ -198,13 +234,13 @@ git(repo, &["update-ref", staging, &tip]) .map_err(|e| format!("could not stage the edit: {e}"))?; } - git_ents::config::store_to_ref(repo, staging, config) + let email = format!("{username}@web"); + git_ents::config::store_to_ref_authored(repo, staging, config, (username, &email)) .map_err(|e| format!("could not build the edit: {e}"))?; - let keydir = tempfile::tempdir().map_err(|e| format!("could not create temp dir: {e}"))?; - let keyfile = keydir.path().join("web-key"); - write_private_key(&keyfile, private_key)?; - + let signer = signing_key + .to_str() + .ok_or_else(|| "signing key path is not UTF-8".to_owned())?; let hooks = hooks .to_str() .ok_or_else(|| "hooks path is not UTF-8".to_owned())?; @@ -219,11 +255,13 @@ .arg(repo) .args(["-c", "gpg.format=ssh"]) .arg("-c") - .arg(format!("user.signingkey={}", keyfile.display())) - .arg("-c") - .arg(format!("user.name={username}")) - .arg("-c") - .arg(format!("user.email={username}@web")) + .arg(format!("user.signingkey={signer}")) + .args([ + "-c", + "user.name=git-ents-web", + "-c", + "user.email=web@git-ents", + ]) .arg("push") .arg("--signed") .arg(format!("--receive-pack={receive_pack}")) @@ -239,6 +277,44 @@ } } +/// Verify an SSHSIG `signature` over `nonce` was made by `public_key` under the +/// login namespace, using `ssh-keygen -Y verify` against a one-key allowed +/// signers file. +fn verify_login_signature(public_key: &str, nonce: &str, signature: &str) -> Result<bool, String> { + let dir = tempfile::tempdir().map_err(|e| format!("could not create temp dir: {e}"))?; + let allowed = dir.path().join("allowed_signers"); + let sig = dir.path().join("nonce.sig"); + write_file( + &allowed, + format!( + "* namespaces=\"{LOGIN_NAMESPACE}\" {}\n", + normalize_key(public_key) + ) + .as_bytes(), + )?; + write_file(&sig, signature.as_bytes())?; + + let mut child = Command::new("ssh-keygen") + .args(["-Y", "verify", "-n", LOGIN_NAMESPACE, "-I", "web", "-f"]) + .arg(&allowed) + .arg("-s") + .arg(&sig) + .stdin(Stdio::piped()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .spawn() + .map_err(|e| format!("could not run ssh-keygen: {e}"))?; + if let Some(mut stdin) = child.stdin.take() { + stdin + .write_all(nonce.as_bytes()) + .map_err(|e| format!("could not hand the challenge to ssh-keygen: {e}"))?; + } + Ok(child + .wait() + .map_err(|e| format!("ssh-keygen did not complete: {e}"))? + .success()) +} + /// The username of the member whose web key matches `public_key`, if any. The /// match is on the key type and body, ignoring any trailing comment. pub(super) fn member_for_public_key(repo: &Path, public_key: &str) -> Option<String> { @@ -271,40 +347,17 @@ .to_owned() } -/// Derive the public key line from a PEM private key, validating it parses. -fn derive_public_key(private_key: &str) -> Result<String, String> { - let dir = tempfile::tempdir().map_err(|e| format!("could not create temp dir: {e}"))?; - let keyfile = dir.path().join("web-key"); - write_private_key(&keyfile, private_key)?; - let output = Command::new("ssh-keygen") - .arg("-y") - .arg("-f") - .arg(&keyfile) - .output() - .map_err(|e| format!("could not run ssh-keygen: {e}"))?; - if !output.status.success() { - return Err("that does not look like a usable private key".to_owned()); - } - let line = String::from_utf8_lossy(&output.stdout).trim().to_owned(); - if line.is_empty() { - return Err("could not derive a public key".to_owned()); - } - Ok(line) +/// The key's trailing comment, if it carries one. +fn key_comment(public_key: &str) -> Option<String> { + public_key + .split_whitespace() + .nth(2) + .map(str::to_owned) + .filter(|comment| !comment.is_empty()) } -/// Write a private key to `path` with `0600` permissions and a trailing newline, -/// alongside no public key — ssh derives the public half when signing. -fn write_private_key(path: &Path, private_key: &str) -> Result<(), String> { - let mut contents = private_key.trim_end().to_owned(); - contents.push('\n'); - std::fs::write(path, &contents).map_err(|e| format!("could not write key: {e}"))?; - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt as _; - std::fs::set_permissions(path, std::fs::Permissions::from_mode(0o600)) - .map_err(|e| format!("could not secure key file: {e}"))?; - } - Ok(()) +fn write_file(path: &Path, bytes: &[u8]) -> Result<(), String> { + std::fs::write(path, bytes).map_err(|e| format!("could not write {}: {e}", path.display())) } /// The pre-receive rejection reason from git's stderr, or a generic message. @@ -358,7 +411,7 @@ }) } -/// A fresh, unguessable session token: 32 random bytes from the OS, hex-encoded. +/// A fresh, unguessable token: 32 random bytes from the OS, hex-encoded. fn random_token() -> Result<String, String> { let mut bytes = [0u8; 32]; std::fs::File::open("/dev/urandom") @@ -372,6 +425,15 @@ form(body).remove(name) } +/// A form field's trimmed value, or the empty string. +fn trimmed(fields: &HashMap<String, String>, name: &str) -> String { + fields + .get(name) + .map(|v| v.trim()) + .unwrap_or_default() + .to_owned() +} + /// Parse an `application/x-www-form-urlencoded` body into its fields. fn form(body: &[u8]) -> HashMap<String, String> { let text = String::from_utf8_lossy(body); @@ -427,15 +489,24 @@ ); } + #[test] + fn reads_a_keys_comment_as_its_label() { + assert_eq!( + key_comment("ssh-ed25519 AAAA laptop").as_deref(), + Some("laptop") + ); + assert_eq!(key_comment("ssh-ed25519 AAAA"), None); + } + #[test] fn decodes_form_fields() { assert_eq!( - field(b"label=my+web+key&private_key=line1%0Aline2", "label").as_deref(), - Some("my web key"), + field(b"public_key=ssh-ed25519+AAAA&signature=a%0Ab", "public_key").as_deref(), + Some("ssh-ed25519 AAAA"), ); assert_eq!( - field(b"label=my+web+key&private_key=line1%0Aline2", "private_key").as_deref(), - Some("line1\nline2"), + field(b"public_key=ssh-ed25519+AAAA&signature=a%0Ab", "signature").as_deref(), + Some("a\nb"), ); } @@ -449,10 +520,13 @@ } #[test] - fn random_tokens_are_long_and_distinct() { - let a = random_token().unwrap(); - let b = random_token().unwrap(); - assert_eq!(a.len(), 64); - assert_ne!(a, b); + fn a_consumed_challenge_does_not_verify_twice() { + let challenges = new_challenges(); + let nonce = issue_challenge(&challenges).unwrap(); + assert!(take_challenge(&challenges, &nonce), "first use should pass"); + assert!( + !take_challenge(&challenges, &nonce), + "a challenge is one-time" + ); } }