mirror of
https://relay.ngit.dev/npub15qydau2hjma6ngxkl2cyar74wzyjshvl65za5k5rl69264ar2exs5cyejr/ngit-grasp.git
synced 2026-10-05 15:08:24 +00:00
DanConwayDev
6a3b723eaf
After deploying a8964bb to gitnostr.com, production logs showed
event-directed proactive sync dialling ws://localhost:3334,
ws://127.0.0.1:7334, and ws://100.125.184.46:7334 (CGNAT). Repository
announcements, state events, and PR events are untrusted - anyone can
publish them - yet their relays/clone tags reached the outbound
WebSocket and git-fetch sinks after syntax-only checks, letting a
crafted event point a public relay at loopback, private, link-local,
or local-name infrastructure (SSRF).
Add one fail-closed outbound target policy (src/outbound.rs) applied
immediately before every event-directed sink so no call path can
bypass it:
- RelayConnection::connect re-authorizes (with DNS vetting) before
every dial and reconnect. SyncManager::register_relay additionally
refuses to register forbidden targets so they never enter the
reconnect lifecycle, and memoizes rejections so stored events cannot
spam logs or starve the bounded purgatory sync tick.
- RealSyncContext::fetch_oids authorizes clone URLs from announcements
and purgatory PR events immediately before spawning git fetch, then
pins the vetted DNS answers via http.curloptResolve and confines the
subprocess with GIT_ALLOW_PROTOCOL=http:https,
http.followRedirects=false, cleared proxy config/environment, and an
empty credential helper, so redirects, proxies, or alternate
protocols cannot escape the authorized target.
The policy enforces per-sink scheme allowlists (ws/wss for relays,
http/https for git), rejects embedded credentials and local hostnames
(localhost, single-label names, IANA special-use suffixes), and
requires IP literals and every DNS answer to be globally reachable.
Service admission (lists_service) and the don't-fetch-from-ourselves
filter now compare parsed host and port instead of substrings, so
gitnostr.com.attacker.example or a path containing the domain no
longer satisfies a check for gitnostr.com.
The operator-configured bootstrap relay stays usable even when local:
trust is carried by RelayTargetSource::OperatorConfigured at
construction, never by comparing event URLs against the configured
value, so event URLs that merely resemble the bootstrap relay are
still rejected. The new NGIT_SYNC_ALLOW_NON_GLOBAL_TARGETS option
(default false; documented in configuration.md, module.nix, and
.env.example) relaxes only the reachability checks for integration
tests and closed development networks; the TestRelay fixture sets it
because the test infrastructure lives on loopback, while the new
regression tests opt back into production behaviour.
Known limitation: nostr-sdk's connect API takes a URL, not a
pre-resolved address, so relay DNS is re-validated before every dial
but re-resolved by the SDK during connection, leaving a narrow
DNS-rebinding window (documented in defensive-measures.md). Git
fetches do not share this window because their DNS answers are pinned.
Closing it requires upstream connector support rather than a custom
connector here.
Validation: tests/outbound_policy.rs adds integration scenarios
against the real relay binary proving that loopback relay URLs and
loopback git clone URLs produce no outbound connection (counting TCP
listeners stand in for attacker infrastructure), that private,
link-local, CGNAT, unspecified, and multicast literals plus localhost
and credential URLs are rejected, that the local bootstrap relay still
connects while a resembling event URL is rejected, and that
substring-embedded domains are no longer admitted. src/outbound.rs
unit tests cover the reachability matrix (including 100.125.184.46)
and exact service matching. cargo fmt, cargo clippy (workspace, zero
warnings), the full cargo test suite, and cargo test -p grasp-audit
--lib all pass in the nix dev shell.