Skip to content

fix(security): activation check must not reach internal addresses - #430

Draft
mptfire wants to merge 1 commit into
LNVPS:masterfrom
mptfire:audit/activation-ssrf-guard
Draft

mptfire wants to merge 1 commit into
LNVPS:masterfrom
mptfire:audit/activation-ssrf-guard

Conversation

@mptfire

@mptfire mptfire commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Found by an independent security audit of the tree at e5d95b0 (2026-11-01).

Problem

Path-based domain activation (check_path_activation, lnvps_api/src/worker.rs) fetches http://<domain>/.well-known/nostr.json?name=<hash> where <domain> is customer-supplied (validated as a syntactic public domain at registration, but its DNS is fully customer-controlled). The fetch used the worker's shared redirect-following client with no address validation, so activation doubled as an SSRF primitive:

  • a registered domain whose A/AAAA record resolves into loopback/private/link-local space lets a customer probe internal services from the worker host (status/timing oracle via the activation result);
  • a 30x from the customer's server redirected the worker anywhere (reqwest default policy follows up to 10 hops);
  • classic DNS rebinding also applied (resolve-then-fetch with no IP pinning).

Bare-IP domains are already rejected at registration (TLD must be alphabetic, dns_name.rs), so this needs a customer-controlled hostname — still fully within a customer's power.

Fix

check_path_activation now:

  1. resolves the domain itself and refuses any address classified non-public (new is_non_public in lnvps_api_common::network — loopback/private/link-local/unspecified/broadcast/documentation, ULA, v6 link-local; the same classes geoip already treats as non-geolocatable);
  2. pins the connection to the resolved IP with the domain as Host header, which kills DNS rebinding for this fetch;
  3. uses a dedicated no-redirect client (redirect(Policy::none())), so a customer server cannot bounce the worker to an address the guard never saw.

Validation

  • New unit tests: the is_non_public classifier (12 negative + 2 positive cases) and path_activation_never_fetches_a_non_public_address (a localhost-resolving domain returns false before any request).
  • cargo build -p lnvps_api and clippy green; no behaviour change for public-resolving domains beyond the (correct) refusal of redirects.
  • Not covered locally: an HTTP-level test of the redirect refusal (needs a live server in the e2e harness) — recommended follow-up.

Audit reference: LNVM-008 (Medium). The same guard should eventually back every outbound fetch of user-supplied hosts (NWC relay dialling is noted separately in the audit).


Update: redirects are now followed, one validated hop at a time

The original version of this PR used a no-redirect client. That was too blunt: the activation URL is hardcoded http://, so any customer server answering with the common http → https redirect previously activated fine and would have failed. The branch now follows redirects manually, at most 3 hops, validating every hop:

  1. each hop resolves its host and refuses non-public addresses (is_non_public);
  2. the request connects to the validated address while TLS/Host still name the real hostname (reqwest per-client resolve) — DNS rebinding cannot swap in an internal address, and the http→https upgrade keeps working;
  3. Location targets are resolved by a unit-tested helper (redirect_target): relative locations stay on the current validated hop, absolute http(s) targets become the next validated hop, and non-http(s) schemes are refused.

Behavior summary: same-host http→https upgrades work; cross-host redirects work only when the target resolves public; loopback/private/link-local and non-http(s) targets fail activation.

Also added the API_CHANGELOG.md entry.

Path-based domain activation fetched http://<customer domain>/.well-known/
nostr.json?name=<hash> with a redirect-following client and no address
validation. The domain is customer-supplied, so activation doubled as an
SSRF primitive: a name resolving into loopback/private/link-local space, a
redirect into one, or plain DNS rebinding let a customer probe or reach
internal services from the worker host.

The check now resolves each hop itself and refuses non-public addresses
(new is_non_public in lnvps_api_common::network), connects to the
validated address while TLS/Host still name the real hostname (DNS
rebinding cannot swap it), and follows redirects manually with a 3-hop
bound — so the common http->https upgrade on the customer's own host
keeps working while a hostile hop cannot bounce the worker into internal
space. redirect_target unit-tests Location resolution.

Found by the 2026-11-01 security audit.
@mptfire
mptfire force-pushed the audit/activation-ssrf-guard branch from 6221ca3 to b7d2382 Compare November 1, 2026 22:50

@v0l v0l left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pinning and the per-hop validation are the right shape. Blocking on these:

  1. is_non_public misses ranges that matter for SSRF. ::ffff:127.0.0.1 / ::ffff:10.0.0.1 (IPv4-mapped v6) pass as public, as do 100.64.0.0/10 (CGNAT, which is where a lot of our internal routing lives), 0.0.0.0/8, multicast, 198.18.0.0/15, 240.0.0.0/4, and NAT64 64:ff9b::/96. Map v4-mapped/compatible v6 back to v4 with to_ipv4_mapped() before classifying, and add the rest. Add each to private_and_special_ranges_are_rejected.
  2. Scheme is derived from the port. let scheme = if port == 443 { "https" } else { "http" } drops next_scheme on the floor, so https://host:8443/ is fetched over plain http and http://host:443/ over TLS. Carry the scheme through the loop instead of let _ = next_scheme;.
  3. The redirect path has no test. The interesting behaviour of this PR is "follows http->https on the customer's host, refuses a hop into private space". That is testable without the e2e harness: bind a TcpListener on a public-looking hostname is not possible, but redirect_target + pinned_activation_client can be exercised against a wiremock server once is_non_public is injectable (e.g. a fn(&IpAddr) -> bool parameter, defaulting to is_non_public at the call site). Please add one test that follows a redirect and one that refuses it.
  4. let _ = hop; and let _ = next_scheme; are leftovers, remove them.
  5. The CHANGELOG entry sits outside ## [Unreleased]'s existing sections with a stray blank line; fold it into the Unreleased block.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants