Skip to content

gl signs requests to any GITLAWB_NODE with no scheme check, so a plaintext remote node sends the signature in cleartext #413

Description

@beardthelion

gl sends RFC 9421 signed requests to whatever GITLAWB_NODE names, with no
check on the scheme. Pointed at a remote node over http://, the signature and
the request body both go out in cleartext.

Verified by execution

Against origin/main at bfc44f9, with the built binary:

$ GITLAWB_NODE=http://10.0.0.36:7777 RUST_LOG=debug gl whoami
DEBUG reqwest::connect: starting new connection: http://10.0.0.36:7777/
DEBUG hyper_util::client::legacy::connect::http: connecting to 10.0.0.36:7777
Error: agent lookup failed: GET http://10.0.0.36:7777/api/v1/agents/did:key:z6Mks...

10.0.0.36 is this machine's LAN address, so the hop is off-loopback. It failed
only because nothing was listening. No warning, no refusal.

Why it is reachable

Every --node argument is declared with the same shared environment fallback,
for example crates/gl/src/agent.rs:20:

#[arg(long, default_value = "https://node.gitlawb.com", env = "GITLAWB_NODE")]

NodeClient::get_authed (crates/gl/src/http.rs:102) routes to get_signed
whenever a keypair is present, and get_signed builds its URL as
format!("{}{}", self.node_url, path) with no scheme inspection anywhere in
between.

The default is https://, so a stock install is not affected. What makes this
worth fixing is that GITLAWB_NODE is shared with git-remote-gitlawb, whose
own module docs and --help teach GITLAWB_NODE=http://my-node:7545 as the way
to point at a self-hosted node. An operator following that instruction downgrades
every gl command at the same time, and nothing says so.

Why cleartext matters here specifically

RFC 9421 authenticates a request; it does not encrypt one. On a plaintext hop the
body is readable and so is the Signature header. #253 records that a signed
request is replayable for roughly 600s and is not bound to a target host, which
turns a captured header into a usable credential for that window. Plaintext moves
that from a compromised-endpoint problem to a passive-observer one.

Fix direction

The chokepoint is NodeClient, not the ~40 call sites: every path already goes
through new / with_timeout. The obstacle is that new returns Self rather
than Result, so refusing there is a signature change across its callers.

Two shapes worth weighing before anyone writes code:

  1. Refuse a non-loopback http:// node, with an explicit opt-in environment
    variable for a trusted private network. Fails closed, and matches what the git
    transport should do. Breaking for anyone already pointing gl at a plaintext
    remote node.
  2. Warn once per invocation and proceed. No breakage, weaker guarantee.

Whichever is chosen, the loopback test should read the parsed address rather than
compare strings. is_loopback_url in crates/gl/src/doctor.rs:345 compares
against four literals (localhost, 127.0.0.1, [::1], ::1) and so misses
127.0.0.2, the IPv4-mapped [::ffff:127.0.0.1], and the numeric spellings that
Url normalises at parse time.

Related

#253 (replay window, no host binding) is the issue that sets the severity here.
#336 covers signed writes across a redirect, a neighbouring property on the same
surface. #367 and its PR #372 specify RFC 9421 for reimplementers and say nothing
about transport security at all, which is the same gap one level up.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    crate:glgl — the contributor CLIkind:securityVulnerability fix or hardeningsev:highMajor break or real security/trust risk, no easy workaroundsubsystem:apiNode REST API request/response surfacesubsystem:identityDID/UCAN, http-sig auth, push authorization

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions