diff --git a/README.md b/README.md index 3a092bf2..8c0e0c0c 100644 --- a/README.md +++ b/README.md @@ -188,7 +188,12 @@ export GITLAWB_NODE=http://localhost:7545 git clone gitlawb://did:key:z6Mk.../my-repo ``` -For public-network use, make sure `GITLAWB_NODE` points to the node you want. The helper defaults to localhost for local development. +For public-network use, make sure `GITLAWB_NODE` points to the node you want, over +`https://`. The helper defaults to localhost for local development, and plaintext +`http://` is allowed only to this machine: requests are signed but not encrypted, so +a cleartext hop to a remote node exposes the pack contents and the `Signature` +header. A remote `http://` node is refused, and `GITLAWB_ALLOW_INSECURE_HTTP=1` +overrides that for a trusted private network. ### Full lifecycle against an iCaptcha-enforcing node @@ -393,7 +398,7 @@ Important node settings: | `GITLAWB_P2P_BOOTSTRAP` | Comma-separated libp2p multiaddrs. | | `GITLAWB_BOOTSTRAP_DISABLE_SEEDS` | Disable embedded seed peers for isolated dev/test networks. | | `GITLAWB_REQUIRE_SIGNED_PEER_WRITES` | Require signed peer announce/sync writes. Defaults to `false` during the staged rollout below. | -| `GITLAWB_ENFORCE_OWNER_PUSH` | Require the authenticated pusher to be the repo owner on `git-receive-pack`. **Defaults to `true`.** A `did:key` signature is authentication, not authorization — anyone can mint a key and sign — so with this off every signed caller may push to every repository, private ones included. Delegated and CI keys count as non-owners: a UCAN `git/push` capability is verified but not yet honored for authorization, so they cannot push while this is on. Set `false` only for a rolling upgrade; see [`docs/RUN-A-NODE.md`](docs/RUN-A-NODE.md). | +| `GITLAWB_ENFORCE_OWNER_PUSH` | Require the authenticated pusher to be the repo owner on `git-receive-pack`. **Defaults to `true`.** A `did:key` signature is authentication, not authorization (anyone can mint a key and sign), so with this off every signed caller may push to every repository, private ones included. Delegated and CI keys count as non-owners: a UCAN `git/push` capability is verified but not yet honored for authorization, so they cannot push while this is on. Set `false` only for a rolling upgrade; see [`docs/RUN-A-NODE.md`](docs/RUN-A-NODE.md). | | `GITLAWB_AUTO_SYNC` | Enable automatic sync from known peers. | | `GITLAWB_MAX_PACK_BYTES` | Max git pack body size for smart-HTTP routes. | | `GITLAWB_GIT_SERVICE_TIMEOUT_SECS` | Max seconds a served git upload-pack, receive-pack, or `info/refs` advertisement may run before it is aborted (504). Default 600. Also bounds the withheld-blob classification walk (on both the upload-pack serve and receive-pack replication paths) and the push-side pin-candidate discovery (`rev-list` / `cat-file`), each reaped via process-group teardown at the deadline. On the path-scoped upload-pack path the classification walk and the pack serve share ONE deadline, so this value bounds their combined duration rather than granting each stage a full budget: a walk that consumes it leaves the serve nothing and the clone gets a 504. Serving large path-scoped repos may therefore need a higher value than they did when each stage was budgeted separately. Accepted range is 1 to 3153600000 (100 years), since the node derives deadlines from this value and a larger one cannot be represented. | diff --git a/crates/git-remote-gitlawb/src/main.rs b/crates/git-remote-gitlawb/src/main.rs index 738839c1..9843bdfa 100644 --- a/crates/git-remote-gitlawb/src/main.rs +++ b/crates/git-remote-gitlawb/src/main.rs @@ -66,6 +66,10 @@ fn main() -> Result<()> { // v0.1: default to localhost. Override with GITLAWB_NODE env var. let node_base = std::env::var("GITLAWB_NODE").unwrap_or_else(|_| "http://127.0.0.1:7545".to_string()); + check_transport_security( + &node_base, + std::env::var_os("GITLAWB_ALLOW_INSECURE_HTTP").is_some(), + )?; let repo_base = format!("{}/{}/{}", node_base, short_owner, repo_name); tracing::debug!("repo_base: {repo_base}"); @@ -78,6 +82,76 @@ fn main() -> Result<()> { run_helper(&repo_base, keypair.as_ref()) } +/// Whether `url` is plaintext http, regardless of where it points. +fn is_http(url: &str) -> bool { + reqwest::Url::parse(url.trim()) + .map(|u| u.scheme() == "http") + .unwrap_or(false) +} + +/// Whether `url` would send git data off this machine in cleartext. +/// +/// True only for `http://` to a non-loopback host. RFC 9421 signs the request +/// but does not encrypt it, so on a plaintext hop the pack contents and the +/// `Signature` header are both readable, and a captured signature is replayable +/// for its freshness window against any host. +/// +/// Loopback is decided from the parsed address rather than a string match, so +/// `127.0.0.2`, `[::1]` and an IPv4-mapped `[::ffff:127.0.0.1]` are all +/// recognised as this machine. A value that does not parse, or that is not +/// http(s), is not this guard's business and returns false: it fails later with +/// its own error, and naming it a TLS problem would misdirect the reader. +fn is_insecure_remote(url: &str) -> bool { + let Ok(parsed) = reqwest::Url::parse(url.trim()) else { + return false; + }; + if parsed.scheme() != "http" { + return false; + } + let Some(host) = parsed.host_str() else { + return false; + }; + let host = host.trim_end_matches('.').to_ascii_lowercase(); + if host == "localhost" { + return false; + } + // host_str() keeps the brackets on an IPv6 literal. + let bare = host.trim_start_matches('[').trim_end_matches(']'); + if let Ok(ip) = bare.parse::() { + if ip.is_loopback() { + return false; + } + // An IPv4-mapped IPv6 literal hides a v4 loopback from is_loopback(). + if let std::net::IpAddr::V6(v6) = ip { + if let Some(v4) = v6.to_ipv4_mapped() { + if v4.is_loopback() { + return false; + } + } + } + } + true +} + +/// Refuse a cleartext hop off this machine unless the operator has opted in. +/// +/// Fail closed: the alternative is signing a request and then handing it, and +/// the pack it carries, to anyone on the path. `GITLAWB_ALLOW_INSECURE_HTTP` +/// exists for a private LAN where the operator has decided that is acceptable. +fn check_transport_security(node_base: &str, allow_insecure: bool) -> Result<()> { + if allow_insecure || !is_insecure_remote(node_base) { + return Ok(()); + } + bail!( + "refusing to send git data to {node_base} over plaintext http.\n\ + Requests are signed but not encrypted, so the pack contents and the \ + Signature header are readable by anyone on the path, and a captured \ + signature can be replayed.\n\ + Use https://, or set GITLAWB_ALLOW_INSECURE_HTTP=1 to accept the risk \ + (for a trusted private network only)." + ) +} + // ── CLI argument handling ────────────────────────────────────────────────────── /// How the binary was invoked, derived from its CLI arguments. @@ -125,6 +199,7 @@ fn help_text() -> String { ENVIRONMENT:\n\ \x20 GITLAWB_NODE Node base URL (default: http://127.0.0.1:7545)\n\ \x20 GITLAWB_KEY Identity PEM path for signed fetch/push (default: ~/.gitlawb/identity.pem)\n\ + \x20 GITLAWB_ALLOW_INSECURE_HTTP Permit plaintext http:// to a non-loopback node (unset by default)\n\ \x20 GITLAWB_LOG Log filter (default: warn)\n\ \n\ FLAGS:\n\ @@ -199,7 +274,8 @@ fn handle_connect( other => bail!("unsupported git service: {other}"), } - let client = build_http_client()?; + // A loopback node must never be proxied; see build_http_client. + let client = build_http_client(!is_insecure_remote(repo_base) && is_http(repo_base))?; // ── Phase 1: ref advertisement (GET /info/refs?service=) ───────── // @@ -335,8 +411,22 @@ const HTTP_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(300); /// node named, and on a 307/308 the pack body went with them. Scope the follow to the /// origin that issued the redirect AND to an identical request-target, which is the /// same predicate `gl` uses. -fn build_http_client() -> Result { - Ok(reqwest::blocking::Client::builder() +/// `bypass_proxy` must be true whenever the node is on this machine. reqwest's +/// default client honours `HTTP_PROXY` / `ALL_PROXY` for a loopback URL too, so a +/// proxy variable pointing off-machine turns an allowed local request into a +/// cleartext hop carrying the `Signature` header, straight past +/// `check_transport_security`. Verified: with `HTTP_PROXY` set, reqwest logged +/// `proxy(...) intercepts 'http://127.0.0.1:7545/'` and dialled the proxy. A +/// local node is never legitimately reached through a proxy, so this refuses one +/// rather than trying to reimplement `NO_PROXY` parsing. +fn build_http_client(bypass_proxy: bool) -> Result { + let builder = reqwest::blocking::Client::builder(); + let builder = if bypass_proxy { + builder.no_proxy() + } else { + builder + }; + Ok(builder .timeout(HTTP_TIMEOUT) .redirect(reqwest::redirect::Policy::custom(same_origin_redirect)) .build()?) @@ -829,6 +919,148 @@ fn resolve_key_path() -> std::path::PathBuf { // ── Tests ───────────────────────────────────────────────────────────────────── +#[cfg(test)] +mod insecure_transport_tests { + use super::*; + + /// Loopback in every form the transport can legitimately be pointed at. + /// Plaintext to this machine is the documented local-alpha default and must + /// keep working, so a false positive here breaks every stock install. + #[test] + fn loopback_http_is_allowed() { + for url in [ + "http://127.0.0.1:7545", + "http://localhost:7545", + "http://[::1]:7545", + "http://127.0.0.2:7545", + "http://[::ffff:127.0.0.1]:7545", + "http://LocalHost:7545", + ] { + assert!( + !is_insecure_remote(url), + "{url} is this machine; plaintext to it must stay allowed" + ); + } + } + + /// The case the guard exists for: cleartext git data leaving the machine. + #[test] + fn remote_http_is_refused() { + for url in [ + "http://node.example.com:7545", + "http://10.0.0.36:7777", + "http://192.168.1.10", + "http://[2001:db8::1]:7545", + "http://8.8.8.8", + ] { + assert!( + is_insecure_remote(url), + "{url} sends git data off-machine in cleartext and must be refused" + ); + } + } + + /// TLS is always fine, loopback or not, so the guard keys on the scheme and + /// not merely on the host being remote. + #[test] + fn https_is_always_allowed() { + for url in [ + "https://node.gitlawb.com", + "https://10.0.0.36:7777", + "https://127.0.0.1:7545", + ] { + assert!(!is_insecure_remote(url), "{url} is TLS and must be allowed"); + } + } + + /// Spellings that defeat a naive loopback check. `Url` normalizes the scheme + /// and the host at parse time (lowercasing, and the WHATWG numeric forms), so + /// these must all still read as this machine. + #[test] + fn loopback_spellings_are_normalized_not_string_matched() { + for url in [ + "HTTP://127.0.0.1:7545", + "Http://LOCALHOST:7545", + "http://localhost.:7545", + "http://user:pw@127.0.0.1:7545", + "http://2130706433:7545", + "http://0x7f000001:7545", + "http://[::ffff:7f00:1]:7545", + ] { + assert!( + !is_insecure_remote(url), + "{url} resolves to this machine; plaintext to it must stay allowed" + ); + } + } + + /// The mirror of the above: an uppercase scheme must not become an escape + /// from the guard, and a remote host in any spelling is still remote. + #[test] + fn uppercase_scheme_does_not_escape_the_guard() { + for url in [ + "HTTP://node.example.com:7545", + "Http://8.8.8.8", + "http://NODE.EXAMPLE.COM", + "http://user:pw@node.example.com", + ] { + assert!( + is_insecure_remote(url), + "{url} is a remote cleartext hop and must be refused" + ); + } + } + + /// The gate itself, both directions, including the opt-in escape hatch. + #[test] + fn gate_refuses_remote_plaintext_unless_opted_in() { + let err = check_transport_security("http://node.example.com:7545", false) + .expect_err("remote plaintext must be refused by default"); + let msg = err.to_string(); + assert!( + msg.contains("plaintext http"), + "the refusal must say why: {msg}" + ); + assert!( + msg.contains("GITLAWB_ALLOW_INSECURE_HTTP"), + "the refusal must name the escape hatch: {msg}" + ); + + check_transport_security("http://node.example.com:7545", true) + .expect("the opt-in must permit the same URL"); + check_transport_security("http://127.0.0.1:7545", false) + .expect("the local default must keep working with no opt-in"); + check_transport_security("https://node.gitlawb.com", false) + .expect("TLS must need no opt-in"); + } + + /// The documented knob must appear in --help. A gate the operator cannot + /// discover reads as a broken transport rather than a deliberate refusal. + #[test] + fn help_documents_the_opt_in() { + assert!(help_text().contains("GITLAWB_ALLOW_INSECURE_HTTP")); + } + + /// An unparseable or non-http value is not classified as insecure here: it + /// fails later with its own error, and reporting it as a TLS problem would + /// send the reader after the wrong thing. + #[test] + fn unparseable_or_other_scheme_is_not_this_guards_problem() { + for url in [ + "", + " ", + "not a url", + "ftp://node.example.com", + "file:///tmp/x", + ] { + assert!( + !is_insecure_remote(url), + "{url:?} is not a cleartext-http-to-remote case" + ); + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -929,7 +1161,7 @@ mod tests { #[test] fn signed_requests_do_not_follow_a_redirect_off_the_node_origin() { let kp = Keypair::generate(); - let client = build_http_client().unwrap(); + let client = build_http_client(false).unwrap(); let mut elsewhere = mockito::Server::new(); let never = elsewhere @@ -1031,7 +1263,7 @@ mod tests { #[test] fn a_same_origin_path_changing_redirect_is_refused() { let kp = Keypair::generate(); - let client = build_http_client().unwrap(); + let client = build_http_client(false).unwrap(); let mut node = mockito::Server::new(); let bounce = node @@ -1082,7 +1314,7 @@ mod tests { #[test] fn an_identical_target_redirect_is_still_followed_up_to_the_chain_bound() { let kp = Keypair::generate(); - let client = build_http_client().unwrap(); + let client = build_http_client(false).unwrap(); let mut node = mockito::Server::new(); let loop_route = node @@ -1243,7 +1475,7 @@ mod tests { fn a_rewritten_target_never_receives_the_signature() { let kp = Keypair::generate(); let expected_did = kp.did().to_string(); - let client = build_http_client().unwrap(); + let client = build_http_client(false).unwrap(); let slot = std::sync::Arc::new(std::sync::Mutex::new(None::)); let mut node = mockito::Server::new(); @@ -1305,7 +1537,7 @@ mod tests { fn a_direct_signed_advertisement_verifies_under_the_node_verifier() { let kp = Keypair::generate(); let expected_did = kp.did().to_string(); - let client = build_http_client().unwrap(); + let client = build_http_client(false).unwrap(); let slot = std::sync::Arc::new(std::sync::Mutex::new(None::)); let mut node = mockito::Server::new(); diff --git a/crates/git-remote-gitlawb/tests/real_git_fetch.rs b/crates/git-remote-gitlawb/tests/real_git_fetch.rs index 72fdd5c2..a10fafa4 100644 --- a/crates/git-remote-gitlawb/tests/real_git_fetch.rs +++ b/crates/git-remote-gitlawb/tests/real_git_fetch.rs @@ -267,6 +267,15 @@ fn write_response(mut stream: TcpStream, status: &str, content_type: &str, body: /// Run `git fetch` in `clone` through the helper, with a hard timeout so a /// regression to the deadlock fails fast instead of hanging the suite. fn fetch_with_helper(clone: &Path, node_url: &str) -> (bool, std::process::Output) { + fetch_with_helper_env(clone, node_url, &[]) +} + +/// [`fetch_with_helper`] with extra environment for the helper process. +fn fetch_with_helper_env( + clone: &Path, + node_url: &str, + extra_env: &[(&str, &str)], +) -> (bool, std::process::Output) { let helper_bin = PathBuf::from(env!("CARGO_BIN_EXE_git-remote-gitlawb")); let helper_dir = helper_bin.parent().unwrap().to_path_buf(); let path_env = match std::env::var_os("PATH") { @@ -289,7 +298,14 @@ fn fetch_with_helper(clone: &Path, node_url: &str) -> (bool, std::process::Outpu // machine/CI with git-l10n installed and LANG set to a translated locale. .env("LC_ALL", "C") .env("GITLAWB_NODE", node_url) - .env("GITLAWB_KEY", "/nonexistent-key-for-anon-fetch"); + .env("GITLAWB_KEY", "/nonexistent-key-for-anon-fetch") + // The helper inherits this process's environment, so an operator or CI + // that exports the insecure-HTTP override would decide the transport + // policy for every test below. Clear it and let each test opt in. + .env_remove("GITLAWB_ALLOW_INSECURE_HTTP"); + for (k, v) in extra_env { + cmd.env(k, v); + } run_bounded(cmd, Duration::from_secs(30)) } @@ -1242,3 +1258,71 @@ fn run_bounded_bounds_join_when_leader_exits_leaving_a_pipe_holder() { held pipes do not stall the joins)" ); } + +/// The plaintext-transport gate, proven at its CALL SITE rather than in +/// isolation. `main.rs`'s unit tests cover `is_insecure_remote` and +/// `check_transport_security` as functions; neither notices if the call in +/// `main()` is deleted, which is the regression that would silently restore the +/// cleartext hop. Only driving the built binary binds the wiring. +/// +/// `192.0.2.1` is TEST-NET-1 (RFC 5737): non-loopback, reserved for +/// documentation, and never routable, so the gate is what stops this and no +/// connection is attempted even if it regressed. +#[test] +fn real_git_fetch_refuses_a_remote_plaintext_node() { + // A shallow fixture is enough: the gate fires before any negotiation. + let repos = build_divergent_repos(1); + let (ok, out) = fetch_with_helper(&repos.clone, "http://192.0.2.1:7545"); + let stderr = String::from_utf8_lossy(&out.stderr); + + assert!( + !ok || !out.status.success(), + "the fetch must not succeed: {stderr}" + ); + assert!( + stderr.contains("plaintext http"), + "the helper must refuse the cleartext hop and say so; stderr was: {stderr}" + ); + assert!( + stderr.contains("GITLAWB_ALLOW_INSECURE_HTTP"), + "the refusal must name the escape hatch; stderr was: {stderr}" + ); +} + +/// A proxy variable must not divert a LOOPBACK node off this machine. +/// +/// reqwest's default client honours `HTTP_PROXY` for a loopback URL, so before +/// the fix a proxy pointing off-machine carried the signed plaintext request to +/// it, past the transport guard, which only ever inspected the URL. Observed +/// directly: `proxy(http://...:9999/) intercepts 'http://127.0.0.1:7545/'`. +/// +/// The proxy here is a black hole on TEST-NET-1, so if the bypass regresses this +/// fetch cannot succeed: it either hangs to the harness timeout or fails to +/// connect. Success is only possible when the request went straight to the shim. +#[test] +fn real_git_fetch_ignores_a_proxy_for_a_loopback_node() { + let repos = build_divergent_repos(1); + let (server, clone) = (repos.server.clone(), repos.clone.clone()); + let shim = start_shim(server.clone(), ShimMode::Normal); + + let (completed, out) = fetch_with_helper_env( + &clone, + &shim.base_url, + &[ + ("HTTP_PROXY", "http://192.0.2.1:9999"), + ("http_proxy", "http://192.0.2.1:9999"), + ("ALL_PROXY", "http://192.0.2.1:9999"), + ], + ); + + assert!( + completed, + "the fetch did not finish; a proxied loopback request would stall here. stderr:\n{}", + String::from_utf8_lossy(&out.stderr) + ); + assert!( + out.status.success(), + "a loopback fetch must ignore the proxy variables. stderr:\n{}", + String::from_utf8_lossy(&out.stderr) + ); +}