fix(git-remote): refuse a plaintext node off this machine unless opted in

git-remote-gitlawb took GITLAWB_NODE verbatim and built every request URL
from it with no scheme check, so pointing it at a remote node over
http:// sent git data in cleartext. Verified against the built binary on
main: GITLAWB_NODE=http://10.0.0.36:7777 with a keypair loaded produced

    GET http://10.0.0.36:7777/z6Mkq.../probe-repo/info/refs?service=git-upload-pack
    hyper_util::client::legacy::connect::http: connecting to 10.0.0.36:7777

with no warning and no refusal.

RFC 9421 signs a request but does not encrypt it, so on a plaintext hop
the pack contents and the Signature header are both readable, and open
#253 records that a captured signature is replayable for around 600s and
is not bound to a target host. Plaintext is what turns that from a
compromised-endpoint problem into a passive-observer one.

The guard fails closed and keys on the parsed address, not a string
match. Url normalizes the scheme and host at parse time, so an uppercase
HTTP://, a trailing-dot FQDN, userinfo, and the WHATWG numeric spellings
(2130706433, 0x7f000001, [::ffff:7f00:1]) all still read as this machine.
Loopback plaintext is the documented local-alpha default, so a false
positive there would break every stock install.

GITLAWB_ALLOW_INSECURE_HTTP=1 is the escape hatch for a private LAN where
the operator has accepted the risk, and it is documented in --help. This
is a breaking change for anyone already running a remote node over
plaintext: they get an error naming the variable rather than a silent
downgrade.

Nine tests. Eight in main.rs cover the classifier over loopback, remote,
TLS, unparseable and adversarially-spelled inputs, the gate in both
directions including the opt-in, and that --help documents the variable.
The ninth drives a real git fetch through the built binary in
real_git_fetch.rs, because none of the other eight notice if the call in
main() is deleted, which is the regression that would silently restore
the cleartext hop. Three mutations verified load-bearing: neutering the
gate, breaking the localhost allowance, and deleting the call site.

Two things deliberately not in scope. gl reads the same GITLAWB_NODE and
signs against it, confirmed by execution, but its default is https and
the fix belongs in NodeClient::new across roughly forty call sites, so it
is its own change. And the check lives in the helper rather than
gitlawb-core because that crate is held to a dependency-purity job and
keeps `url` behind a feature; when #394 lands and moves resolution into a
shared resolve_transport_node, this folds into it.
This commit is contained in:
beardthelion committed 2026-09-07 22:24:58 -05:00
1 parent bfc44f926d
commit 12e68e5463
3 files changed
+247 -2

No files matched your search

+7 -2
View File
@@ -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. |
+210
View File
@@ -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,69 @@ fn main() -> Result<()> {
run_helper(&repo_base, keypair.as_ref())
}
/// 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::<std::net::IpAddr>() {
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 +192,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\
@@ -829,6 +897,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:[email protected]: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:[email protected]",
] {
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::*;
@@ -1242,3 +1242,33 @@ 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}"
);
}