diff --git a/server/src/at_record_server/covers.gleam b/server/src/at_record_server/covers.gleam index 12d8383..94a5f20 100644 --- a/server/src/at_record_server/covers.gleam +++ b/server/src/at_record_server/covers.gleam @@ -4,6 +4,7 @@ import at_record/gen/client as generated_client import at_record_server/discogs_client import at_record_server/promotion +import at_record_server/url_guard.{is_safe_pds_url} import atproto/blob.{type Blob} import atproto/repo import atproto/xrpc.{type Client} @@ -52,8 +53,10 @@ pub fn fetch_and_upload_any( }) } -/// Best-effort: any failure (fetch, size, upload) logs and returns None so the -/// caller keeps the hotlink fallback instead of failing the write. +/// Best-effort: any failure (unsafe url, fetch, size, upload) logs and +/// returns None so the caller keeps the hotlink fallback instead of failing +/// the write. Every caller in this module (candidate lists, foreign-repo +/// copies) funnels through here, so the URL guard lives in one place. pub fn fetch_and_upload( fetch_client: Client, pds_client: Client, @@ -61,26 +64,33 @@ pub fn fetch_and_upload( token: String, url: String, ) -> Option(Blob) { - { - use resp <- result.try( - xrpc.get_bits(fetch_client, url, option.None) - |> result.map_error(fn(e) { "fetch failed: " <> string.inspect(e) }), - ) - let mime = - response.get_header(resp, "content-type") - |> result.unwrap("image/jpeg") - |> strip_params - case bit_array.byte_size(resp.body) <= max_bytes { - True -> - repo.upload_blob(pds_client, pds, token, resp.body, mime) - |> result.map_error(fn(e) { "upload failed: " <> string.inspect(e) }) - False -> Error("image exceeds the lexicon maxSize") + case is_safe_pds_url(url) { + False -> { + wisp.log_warning("cover blob rejected (" <> url <> "): unsafe endpoint") + option.None } + True -> + { + use resp <- result.try( + xrpc.get_bits(fetch_client, url, option.None) + |> result.map_error(fn(e) { "fetch failed: " <> string.inspect(e) }), + ) + let mime = + response.get_header(resp, "content-type") + |> result.unwrap("image/jpeg") + |> strip_params + case bit_array.byte_size(resp.body) <= max_bytes { + True -> + repo.upload_blob(pds_client, pds, token, resp.body, mime) + |> result.map_error(fn(e) { "upload failed: " <> string.inspect(e) }) + False -> Error("image exceeds the lexicon maxSize") + } + } + |> result.map_error(fn(reason) { + wisp.log_warning("cover blob skipped (" <> url <> "): " <> reason) + }) + |> option.from_result } - |> result.map_error(fn(reason) { - wisp.log_warning("cover blob skipped (" <> url <> "): " <> reason) - }) - |> option.from_result } /// Copy a foreign repo's cover blob into our own: a record of ours can only diff --git a/server/src/at_record_server/handlers/cover_proxy.gleam b/server/src/at_record_server/handlers/cover_proxy.gleam index 5aa0e94..9d8b838 100644 --- a/server/src/at_record_server/handlers/cover_proxy.gleam +++ b/server/src/at_record_server/handlers/cover_proxy.gleam @@ -8,6 +8,7 @@ import at_record/gen/client as generated_client import at_record_server/config_env import at_record_server/context.{type Context} +import at_record_server/url_guard.{is_safe_pds_url} import atproto/xrpc import gleam/bytes_tree import gleam/http/response @@ -65,7 +66,13 @@ fn fetch_and_cache( ) { Error(_) -> upstream_failure() - Ok(doc) -> fetch_blob(ctx, doc.pds, did, cid, dir, base) + Ok(doc) -> + case is_safe_pds_url(doc.pds) { + // `doc.pds` comes from the did's document, so an unsafe endpoint is + // treated the same as an unresolvable one rather than being fetched. + False -> upstream_failure() + True -> fetch_blob(ctx, doc.pds, did, cid, dir, base) + } } } diff --git a/server/src/at_record_server/oauth/flow.gleam b/server/src/at_record_server/oauth/flow.gleam index 12e7462..e4c5a3c 100644 --- a/server/src/at_record_server/oauth/flow.gleam +++ b/server/src/at_record_server/oauth/flow.gleam @@ -8,6 +8,7 @@ import at_record_server/oauth/config.{type Config} import at_record_server/oauth/pkce.{type Pkce} import at_record_server/oauth/store.{PendingFlow} import at_record_server/oauth/transport +import at_record_server/url_guard.{is_safe_pds_url} import atproto/oauth/metadata.{type AuthServerMetadata} import atproto/xrpc import gleam/bit_array @@ -45,7 +46,7 @@ pub fn start_login( ) |> result.map_error(fn(e) { ResolveFailed(string.inspect(e)) }), ) - let pds = doc.pds + use pds <- result.try(require_safe_endpoint(doc.pds, ResolveFailed)) use meta <- result.try( metadata.discover(cfg.client, pds) |> result.map_error(fn(e) { DiscoverFailed(string.inspect(e)) }), @@ -55,6 +56,10 @@ pub fn start_login( let dpop_key = gose.generate_ec(ec.P256) let state = b64(crypto.strong_random_bytes(16)) + use token_endpoint <- result.try(require_safe_endpoint( + meta.token_endpoint, + DiscoverFailed, + )) use request_uri <- result.try(push_par(cfg, meta, handle, pk, dpop_key, state)) store.put( @@ -64,7 +69,7 @@ pub fn start_login( handle:, pds:, issuer: meta.issuer, - token_endpoint: meta.token_endpoint, + token_endpoint:, dpop_key:, pkce_verifier: pk.verifier, client_id: cfg.client_id, @@ -90,14 +95,13 @@ fn push_par( dpop_key: gose.Key(String), state: String, ) -> Result(String, FlowError) { + use par_endpoint <- result.try(require_safe_endpoint( + meta.pushed_authorization_request_endpoint, + ParFailed, + )) use form <- result.try(par_form(cfg, meta, handle, pk, state)) use resp <- result.try( - transport.post_form_with_dpop( - cfg.client, - meta.pushed_authorization_request_endpoint, - form, - dpop_key, - ) + transport.post_form_with_dpop(cfg.client, par_endpoint, form, dpop_key) |> result.map_error(ParFailed), ) @@ -149,3 +153,16 @@ fn par_form( fn b64(bits: BitArray) -> String { bit_array.base64_url_encode(bits, False) } + +/// `url` is attacker-influenced (a resolved PDS, or an endpoint the PDS's own +/// discovery metadata pointed at), so it is checked before every outbound +/// request the login flow makes rather than trusted wherever it points. +fn require_safe_endpoint( + url: String, + on_unsafe: fn(String) -> FlowError, +) -> Result(String, FlowError) { + case is_safe_pds_url(url) { + True -> Ok(url) + False -> Error(on_unsafe("unsafe endpoint: " <> url)) + } +} diff --git a/server/src/at_record_server/oauth/tokens.gleam b/server/src/at_record_server/oauth/tokens.gleam index f8eb59a..dfd25d1 100644 --- a/server/src/at_record_server/oauth/tokens.gleam +++ b/server/src/at_record_server/oauth/tokens.gleam @@ -7,6 +7,7 @@ import at_record_server/oauth/config.{type Config} import at_record_server/oauth/sessions.{type OauthSession} import at_record_server/oauth/store.{type PendingFlow} import at_record_server/oauth/transport +import at_record_server/url_guard.{is_safe_pds_url} import atproto/oauth/metadata import atproto/xrpc import gleam/dynamic/decode @@ -70,34 +71,41 @@ pub fn revoke(cfg: Config, session: OauthSession) -> Nil { case metadata.fetch_authorization_server(cfg.client, session.issuer) { Ok(meta) -> case meta.revocation_endpoint { - Some(endpoint) -> { - let base = [ - #("token", session.refresh_token), - #("token_type_hint", "refresh_token"), - #("client_id", session.client_id), - ] - case - with_auth( - cfg, - base, - session.confidential, - session.client_id, - session.issuer, - ) - { - Ok(form) -> { - let _ = - transport.post_form_with_dpop( - cfg.client, - endpoint, - form, - session.dpop_key, + // The revocation endpoint comes from the same untrusted AS-discovery + // metadata as the PAR endpoint, so it gets the same check before + // anything is posted to it. + Some(endpoint) -> + case is_safe_pds_url(endpoint) { + False -> Nil + True -> { + let base = [ + #("token", session.refresh_token), + #("token_type_hint", "refresh_token"), + #("client_id", session.client_id), + ] + case + with_auth( + cfg, + base, + session.confidential, + session.client_id, + session.issuer, ) - Nil + { + Ok(form) -> { + let _ = + transport.post_form_with_dpop( + cfg.client, + endpoint, + form, + session.dpop_key, + ) + Nil + } + Error(_) -> Nil + } } - Error(_) -> Nil } - } None -> Nil } Error(_) -> Nil diff --git a/server/src/at_record_server/shelf_owner.gleam b/server/src/at_record_server/shelf_owner.gleam index 7d4bff5..e1e1045 100644 --- a/server/src/at_record_server/shelf_owner.gleam +++ b/server/src/at_record_server/shelf_owner.gleam @@ -10,14 +10,13 @@ import at_record/gen/repo/list_records.{type RecordEntry} import at_record/gen/shelf/entry import at_record/storage.{type StoredItem} import at_record_server/context.{type Context} +import at_record_server/url_guard.{is_safe_pds_url} import atproto/xrpc import gleam/dynamic/decode import gleam/int import gleam/list import gleam/option.{type Option, None, Some} import gleam/result -import gleam/string -import gleam/uri import wisp /// Handle or DID -> did/handle/pds via the Slingshot resolver. Every @@ -64,55 +63,6 @@ pub fn resolve_actor( } } -/// `doc.pds` comes straight off an attacker-controllable DID document, so a -/// rejected endpoint (non-https, loopback, link-local, RFC1918) behaves like -/// an unresolvable actor to the caller rather than being fetched. -pub fn is_safe_pds_url(url: String) -> Bool { - case uri.parse(url) { - Error(_) -> False - Ok(parsed) -> - case parsed.scheme, parsed.host { - Some("https"), Some(host) -> is_safe_host(host) - _, _ -> False - } - } -} - -// URL-level check only: a hostname that looks fine here could still resolve -// to a private address at request time (DNS rebinding), which this can't see. -fn is_safe_host(host: String) -> Bool { - case string.lowercase(host) { - "localhost" -> False - lower -> - case ipv4_octets(lower) { - Some(octets) -> !is_private_ipv4(octets) - None -> True - } - } -} - -fn ipv4_octets(host: String) -> Option(List(Int)) { - case string.split(host, ".") { - [a, b, c, d] -> - case int.parse(a), int.parse(b), int.parse(c), int.parse(d) { - Ok(oa), Ok(ob), Ok(oc), Ok(od) -> Some([oa, ob, oc, od]) - _, _, _, _ -> None - } - _ -> None - } -} - -fn is_private_ipv4(octets: List(Int)) -> Bool { - case octets { - [10, _, _, _] -> True - [127, _, _, _] -> True - [169, 254, _, _] -> True - [172, b, _, _] if b >= 16 && b <= 31 -> True - [192, 168, _, _] -> True - _ -> False - } -} - /// Three outcomes the handler must tell apart: a genuine transport/decode /// failure (`FetchFailed`, 502-worthy), a foreign PDS answering 400/404 /// because the repo is missing or gone (`RepoNotFound`, 404-worthy, not the diff --git a/server/src/at_record_server/url_guard.gleam b/server/src/at_record_server/url_guard.gleam new file mode 100644 index 0000000..8ef6bfd --- /dev/null +++ b/server/src/at_record_server/url_guard.gleam @@ -0,0 +1,113 @@ +//// Shared guard for outbound fetches whose target is attacker-influenced: +//// a PDS from a DID document, a discovered OAuth endpoint, a user-supplied +//// cover URL. Same rule everywhere, so it lives in one place rather than +//// being re-derived per call site. + +import gleam/int +import gleam/list +import gleam/option.{Some} +import gleam/string +import gleam/uri + +/// Rejects anything that isn't a plain `https://` URL whose resolved +/// address (or addresses) all land outside loopback/link-local/RFC1918/ULA/ +/// CGNAT ranges: the caller should treat a `False` the same way it treats an +/// unresolvable or unreachable target. +pub fn is_safe_pds_url(url: String) -> Bool { + case uri.parse(url) { + Error(_) -> False + Ok(parsed) -> + case parsed.scheme, parsed.host { + Some("https"), Some(host) -> + !has_bracketed_authority(url) + && !string.starts_with(host, "[") + && is_safe_host(host) + _, _ -> False + } + } +} + +// A real atproto PDS is always a DNS hostname, never a bare IP literal, so +// IPv6 is rejected wholesale rather than range-classified (no loopback/ULA/ +// link-local carve-out to get right). That sidesteps a real bug in the +// vendored gleam_stdlib's bracket-host parser (an inverted range check in +// `is_valid_host_within_brackets_char`) that mangles a bracketed IPv6 host +// down to a bare "[" for any address containing a letter or a digit above +// "0": `parsed.host` can't be trusted to classify IPv6 by range, so instead +// this checks for "[" both on whatever `parsed.host` came out as (catching +// the mangled "[" form too) and on the raw authority text straight out of +// the url, in case a differently-behaved parser lets one slip through. +fn has_bracketed_authority(url: String) -> Bool { + case string.split_once(url, "://") { + Error(Nil) -> False + Ok(#(_, rest)) -> + case string.split_once(rest, "/") { + Ok(#(authority, _)) -> string.contains(authority, "[") + Error(Nil) -> string.contains(rest, "[") + } + } +} + +// Classifying the host as an IP by hand-parsing its text (four dotted +// decimal octets) misses the encodings a resolver still accepts: a bare +// decimal integer, hex (`0xA9FEA9FE`), and octal-dotted (`0251.0376...`) all +// resolve straight to an address with no DNS involved. Resolving the host +// with the same `inet` calls the transport ends up using sidesteps having to +// re-derive every encoding the resolver understands, and also catches DNS +// rebinding at request time: any address a hostname resolves to (not just +// the first) is range-checked, and a host that resolves to nothing is +// rejected rather than assumed safe. +fn is_safe_host(host: String) -> Bool { + let #(v4_addrs, v6_addrs) = resolve_addresses(string.lowercase(host)) + case v4_addrs, v6_addrs { + [], [] -> False + _, _ -> + list.all(v4_addrs, fn(addr) { !is_private_ipv4(addr) }) + && list.all(v6_addrs, fn(addr) { !is_private_ipv6(addr) }) + } +} + +@external(erlang, "url_guard_ffi", "resolve_addresses") +fn resolve_addresses( + host: String, +) -> #( + List(#(Int, Int, Int, Int)), + List(#(Int, Int, Int, Int, Int, Int, Int, Int)), +) + +fn is_private_ipv4(octets: #(Int, Int, Int, Int)) -> Bool { + case octets { + #(0, _, _, _) -> True + #(10, _, _, _) -> True + #(127, _, _, _) -> True + #(169, 254, _, _) -> True + #(172, b, _, _) if b >= 16 && b <= 31 -> True + #(192, 168, _, _) -> True + #(100, b, _, _) if b >= 64 && b <= 127 -> True + #(255, 255, 255, 255) -> True + _ -> False + } +} + +fn is_private_ipv6(groups: #(Int, Int, Int, Int, Int, Int, Int, Int)) -> Bool { + case groups { + #(0, 0, 0, 0, 0, 0, 0, 0) -> True + #(0, 0, 0, 0, 0, 0, 0, 1) -> True + #(0, 0, 0, 0, 0, 0xffff, hi, lo) -> is_private_ipv4(mapped_ipv4(hi, lo)) + #(a, _, _, _, _, _, _, _) -> is_ula_or_link_local(a) + } +} + +// fc00::/7 (ULA) and fe80::/10 (link-local): both ranges are pinned down by +// the first group's high bits, so a mask-and-compare on that one group is +// enough rather than range-checking the full address. +fn is_ula_or_link_local(first_group: Int) -> Bool { + int.bitwise_and(first_group, 0xfe00) == 0xfc00 + || int.bitwise_and(first_group, 0xffc0) == 0xfe80 +} + +// The last two 16-bit groups of an IPv4-mapped address (`::ffff:a.b.c.d`) +// pack the four octets two-per-group, high byte first. +fn mapped_ipv4(hi: Int, lo: Int) -> #(Int, Int, Int, Int) { + #(hi / 256, hi % 256, lo / 256, lo % 256) +} diff --git a/server/src/at_record_server/url_guard_ffi.erl b/server/src/at_record_server/url_guard_ffi.erl new file mode 100644 index 0000000..f057c5a --- /dev/null +++ b/server/src/at_record_server/url_guard_ffi.erl @@ -0,0 +1,19 @@ +%% The guard classifies a host by resolving it the same way gleam_httpc's +%% transport eventually will (inet:getaddr), rather than hand-parsing the +%% decimal/hex/octal encodings a resolver accepts: numeric literals resolve +%% locally with no DNS, so this stays deterministic for the SSRF bypass +%% inputs while still doing a real lookup for actual hostnames. +-module(url_guard_ffi). +-export([resolve_addresses/1]). + +resolve_addresses(Host) -> + HostList = binary_to_list(Host), + {resolve(HostList, inet), resolve(HostList, inet6)}. + +resolve(Host, Family) -> + try inet:getaddrs(Host, Family) of + {ok, Addrs} -> Addrs; + {error, _} -> [] + catch + _:_ -> [] + end. diff --git a/server/test/actor_shelf_test.gleam b/server/test/actor_shelf_test.gleam index caef85a..a64c820 100644 --- a/server/test/actor_shelf_test.gleam +++ b/server/test/actor_shelf_test.gleam @@ -25,7 +25,12 @@ const actor_did = "did:plc:pub" const actor_handle = "pub.test" -const pds_host = "pds.pub.test" +// url_guard now resolves the host for real (fail-closed on nxdomain), so +// this can't be an RFC 2606 reserved, non-resolving name; a TEST-NET-1 +// (RFC 5737) literal resolves locally with no DNS and stays outside every +// private range, so the suite stays offline. The xrpc client below is +// still a stub, no real fetch happens. +const pds_host = "192.0.2.10" fn entry_uri(rkey: String) -> String { "at://" <> actor_did <> "/dev.mokkenstorm.crate.shelf.entry/" <> rkey diff --git a/server/test/cover_proxy_test.gleam b/server/test/cover_proxy_test.gleam index ad248da..9fe16a1 100644 --- a/server/test/cover_proxy_test.gleam +++ b/server/test/cover_proxy_test.gleam @@ -20,7 +20,12 @@ const cid = "bafycover123" const image_bytes = "totally-a-jpeg" -const pds_host = "pds.test" +// url_guard now resolves the host for real (fail-closed on nxdomain), so +// this can't be an RFC 2606 reserved, non-resolving name; a TEST-NET-2 +// (RFC 5737) literal resolves locally with no DNS and stays outside every +// private range, so the suite stays offline. The xrpc client below is +// still a stub, no real fetch happens. +const pds_host = "198.51.100.10" fn resolve_body() -> String { "{\"did\":\"" @@ -37,7 +42,7 @@ fn network_client() -> xrpc.Client { case req.host { "resolver.test" -> Ok(response.Response(200, [], bit_array.from_string(resolve_body()))) - "pds.test" -> + host if host == pds_host -> Ok(response.Response( 200, [#("content-type", "image/png; charset=binary")], @@ -54,6 +59,27 @@ fn failing_resolver_client() -> xrpc.Client { }) } +/// Resolves to a loopback "pds": the did document is attacker-controllable, +/// so this must be rejected the same way a resolve failure is, without ever +/// reaching the network for the blob. +fn resolves_to_a_private_pds_client() -> xrpc.Client { + xrpc.Client(send: fn(req) { + case req.host { + "resolver.test" -> + Ok(response.Response( + 200, + [], + bit_array.from_string( + "{\"did\":\"" + <> did + <> "\",\"handle\":\"pub.test\",\"pds\":\"https://127.0.0.1\",\"signing_key\":\"zTest\"}", + ), + )) + _ -> panic as "an unsafe pds must not be fetched" + } + }) +} + fn resolves_but_blob_fetch_fails_client() -> xrpc.Client { xrpc.Client(send: fn(req) { case req.host { @@ -128,6 +154,16 @@ pub fn resolve_failure_returns_502_and_caches_nothing_test() { let _ = simplifile.delete(dir) } +pub fn unsafe_pds_returns_502_and_caches_nothing_test() { + let dir = fresh_cache_dir("unsafe-pds") + let ctx = test_context(resolves_to_a_private_pds_client()) + let resp = cover_proxy.serve(did, cid, ctx) + assert resp.status == 502 + assert simplifile.is_file(dir <> "/" <> did <> "-" <> cid <> ".bin") + == Ok(False) + let _ = simplifile.delete(dir) +} + pub fn blob_fetch_failure_returns_502_and_caches_nothing_test() { let dir = fresh_cache_dir("blob-fail") let ctx = test_context(resolves_but_blob_fetch_fails_client()) diff --git a/server/test/covers_test.gleam b/server/test/covers_test.gleam new file mode 100644 index 0000000..d3b3f2e --- /dev/null +++ b/server/test/covers_test.gleam @@ -0,0 +1,37 @@ +//// A `coverUrl`/`thumbUrl` on `addEntry` is user-supplied and unauthenticated +//// against the target host, so `fetch_and_upload` (and everything that funnels +//// through it: `fetch_and_upload_any`, the foreign-repo copy path) must reject +//// a private-address candidate before ever touching the network. + +import at_record_server/covers +import atproto/xrpc +import gleam/option.{None, Some} + +fn panics_if_called_client() -> xrpc.Client { + xrpc.Client(send: fn(_req) { + panic as "an unsafe cover url must not be fetched" + }) +} + +pub fn fetch_and_upload_rejects_a_private_address_test() { + let client = panics_if_called_client() + let cover = + covers.fetch_and_upload( + client, + client, + "https://pds.example", + "token", + "https://169.254.169.254/secret", + ) + assert cover == None +} + +pub fn fetch_and_upload_any_degrades_to_no_cover_on_unsafe_candidates_test() { + let client = panics_if_called_client() + let cover = + covers.fetch_and_upload_any(client, client, "https://pds.example", "token", [ + Some("http://127.0.0.1/cover.jpg"), + Some("https://localhost/thumb.jpg"), + ]) + assert cover == None +} diff --git a/server/test/crate_overlap_test.gleam b/server/test/crate_overlap_test.gleam index 85896e8..17d1748 100644 --- a/server/test/crate_overlap_test.gleam +++ b/server/test/crate_overlap_test.gleam @@ -114,7 +114,12 @@ const actor_did = "did:plc:pub" const actor_handle = "pub.test" -const actor_pds_host = "pds.pub.test" +// url_guard now resolves the host for real (fail-closed on nxdomain), so +// this can't be an RFC 2606 reserved, non-resolving name; a TEST-NET-3 +// (RFC 5737) literal resolves locally with no DNS and stays outside every +// private range, so the suite stays offline. The xrpc client below is +// still a stub, no real fetch happens. +const actor_pds_host = "203.0.113.10" const session_cookie = "ar_oauth_sid" diff --git a/server/test/oauth_test.gleam b/server/test/oauth_test.gleam index 0c43dc0..eab5b4c 100644 --- a/server/test/oauth_test.gleam +++ b/server/test/oauth_test.gleam @@ -5,6 +5,7 @@ import at_record_server/oauth/assertion import at_record_server/oauth/authed import at_record_server/oauth/config import at_record_server/oauth/dpop +import at_record_server/oauth/flow import at_record_server/oauth/keys import at_record_server/oauth/pkce import at_record_server/oauth/session_store @@ -130,6 +131,164 @@ pub fn config_https_is_confidential_client_test() { assert cfg.redirect_uri == "https://app.example/api/oauth/callback" } +// -- SSRF guard on the login path ----------------------------------------- +// +// Both the resolved PDS and the PAR endpoint the PDS's own discovery +// metadata points at are attacker-influenced (the DID document is the +// entered handle's, and a malicious PDS controls what its own +// well-known responses say), so both must be checked before any outbound +// request. A client that panics on the unsafe host proves it never gets +// that far. + +fn resolve_mini_doc_response(pds: String) -> String { + "{\"did\":\"did:plc:abc\",\"handle\":\"h.test\",\"pds\":\"" + <> pds + <> "\",\"signing_key\":\"zTest\"}" +} + +pub fn start_login_rejects_a_private_pds_test() { + let stub = + xrpc.Client(send: fn(req) { + case req.host { + "resolver.test" -> + Ok(response.Response( + 200, + [], + bit_array.from_string(resolve_mini_doc_response("https://127.0.0.1")), + )) + _ -> panic as "an unsafe pds must not be fetched" + } + }) + let cfg = + support.stub_config_with( + stub, + "https://resolver.test", + "http://localhost:8080", + ) + let assert Error(_) = flow.start_login(cfg, "h.test") +} + +const protected_resource_response = "{\"authorization_servers\":[\"https://198.51.100.20\"]}" + +const auth_server_response_with_unsafe_par = "{\"issuer\":\"https://198.51.100.20\",\"authorization_endpoint\":\"https://198.51.100.20/authorize\",\"token_endpoint\":\"https://198.51.100.20/token\",\"pushed_authorization_request_endpoint\":\"https://127.0.0.1/par\"}" + +pub fn start_login_rejects_an_unsafe_par_endpoint_test() { + // The pds and issuer hosts have to resolve to a public address here: + // url_guard now resolves the host for real (fail-closed on nxdomain), so + // an RFC 2606 reserved, non-resolving hostname would get rejected before + // ever reaching the PAR-endpoint check this test pins. TEST-NET-1/2 + // literals (RFC 5737) resolve locally with no DNS, so this stays offline. + let stub = + xrpc.Client(send: fn(req) { + case req.host { + "resolver.test" -> + Ok(response.Response( + 200, + [], + bit_array.from_string(resolve_mini_doc_response( + "https://192.0.2.10", + )), + )) + "192.0.2.10" -> + Ok(response.Response( + 200, + [], + bit_array.from_string(protected_resource_response), + )) + "198.51.100.20" -> + Ok(response.Response( + 200, + [], + bit_array.from_string(auth_server_response_with_unsafe_par), + )) + _ -> panic as "an unsafe PAR endpoint must not be posted to" + } + }) + let cfg = + support.stub_config_with( + stub, + "https://resolver.test", + "http://localhost:8080", + ) + let assert Error(_) = flow.start_login(cfg, "h.test") +} + +const auth_server_response_with_unsafe_token = "{\"issuer\":\"https://198.51.100.20\",\"authorization_endpoint\":\"https://198.51.100.20/authorize\",\"token_endpoint\":\"https://127.0.0.1/token\",\"pushed_authorization_request_endpoint\":\"https://198.51.100.20/par\"}" + +pub fn start_login_rejects_an_unsafe_token_endpoint_test() { + // The token endpoint comes from the same untrusted AS-discovery metadata + // as the PAR endpoint (a malicious PDS controls both), so it must be + // rejected before ever being stored, the same way an unsafe PAR endpoint + // is: otherwise it would later be POSTed to on code exchange and refresh. + let stub = + xrpc.Client(send: fn(req) { + case req.host { + "resolver.test" -> + Ok(response.Response( + 200, + [], + bit_array.from_string(resolve_mini_doc_response( + "https://192.0.2.10", + )), + )) + "192.0.2.10" -> + Ok(response.Response( + 200, + [], + bit_array.from_string(protected_resource_response), + )) + "198.51.100.20" -> + Ok(response.Response( + 200, + [], + bit_array.from_string(auth_server_response_with_unsafe_token), + )) + _ -> panic as "an unsafe token endpoint must not be reached" + } + }) + let cfg = + support.stub_config_with( + stub, + "https://resolver.test", + "http://localhost:8080", + ) + let assert Error(_) = flow.start_login(cfg, "h.test") +} + +const auth_server_response_with_unsafe_revocation = "{\"issuer\":\"https://as.example\",\"authorization_endpoint\":\"https://as.example/authorize\",\"token_endpoint\":\"https://as.example/token\",\"pushed_authorization_request_endpoint\":\"https://as.example/par\",\"revocation_endpoint\":\"https://127.0.0.1/revoke\"}" + +/// `revoke` is best-effort logout cleanup, re-discovering the session's +/// authorization server rather than trusting a stored endpoint, so a +/// revocation endpoint pointing at a private address must be skipped the +/// same way the PAR and token endpoints are. +pub fn revoke_does_not_post_to_an_unsafe_revocation_endpoint_test() { + let stub = + xrpc.Client(send: fn(req) { + case req.host { + "as.example" -> + Ok(response.Response( + 200, + [], + bit_array.from_string(auth_server_response_with_unsafe_revocation), + )) + _ -> panic as "an unsafe revocation endpoint must not be posted to" + } + }) + let cfg = + support.stub_config_with( + stub, + "https://resolver.test", + "http://localhost:8080", + ) + let session = + support.stub_session_with( + access_token: "at", + refresh_token: "rt", + expires_at: far_future, + ) + tokens.revoke(cfg, session) +} + const token_response = "{\"access_token\":\"at\",\"refresh_token\":\"rt\",\"token_type\":\"DPoP\",\"expires_in\":3600,\"sub\":\"did:plc:abc\",\"scope\":\"atproto\"}" pub fn token_exchange_parses_dpop_bound_tokens_test() { diff --git a/server/test/shelf_owner_test.gleam b/server/test/shelf_owner_test.gleam deleted file mode 100644 index 2df3d00..0000000 --- a/server/test/shelf_owner_test.gleam +++ /dev/null @@ -1,40 +0,0 @@ -//// `resolve_actor` hands `doc.pds` straight from an attacker-controllable DID -//// document to an outbound fetch, so `is_safe_pds_url` is the SSRF gate: pin -//// it directly rather than only through the handler. - -import at_record_server/shelf_owner -import gleam/list - -pub fn https_is_required_test() { - assert !shelf_owner.is_safe_pds_url("http://pds.example") -} - -pub fn loopback_is_rejected_test() { - ["https://127.0.0.1", "https://localhost", "https://127.1.2.3:8080"] - |> list.each(fn(url) { - assert !shelf_owner.is_safe_pds_url(url) - }) -} - -pub fn link_local_is_rejected_test() { - assert !shelf_owner.is_safe_pds_url("https://169.254.169.254") -} - -pub fn rfc1918_ranges_are_rejected_test() { - ["https://10.0.0.5", "https://172.16.0.1", "https://192.168.1.1"] - |> list.each(fn(url) { - assert !shelf_owner.is_safe_pds_url(url) - }) -} - -pub fn adjacent_ranges_are_not_confused_with_rfc1918_test() { - // 172.15.x and 172.32.x are outside the 172.16.0.0/12 private block. - ["https://172.15.0.1", "https://172.32.0.1"] - |> list.each(fn(url) { - assert shelf_owner.is_safe_pds_url(url) - }) -} - -pub fn ordinary_https_host_is_accepted_test() { - assert shelf_owner.is_safe_pds_url("https://pds.example.com") -} diff --git a/server/test/url_guard_test.gleam b/server/test/url_guard_test.gleam new file mode 100644 index 0000000..495422b --- /dev/null +++ b/server/test/url_guard_test.gleam @@ -0,0 +1,98 @@ +//// `is_safe_pds_url` gates every outbound fetch whose target is derived from +//// an attacker-controllable source (a DID document's `pds`, a discovered +//// OAuth endpoint, a user-supplied cover URL): pin it directly here rather +//// than only through each handler that calls it. +//// +//// Every host below is either a numeric literal (resolves locally through +//// `inet`, no DNS) or a bracketed/loopback form rejected before resolution +//// is even attempted, so this suite never touches the network. + +import at_record_server/url_guard +import gleam/list + +pub fn https_is_required_test() { + assert !url_guard.is_safe_pds_url("http://pds.example") +} + +pub fn loopback_is_rejected_test() { + ["https://127.0.0.1", "https://localhost", "https://127.1.2.3:8080"] + |> list.each(fn(url) { + assert !url_guard.is_safe_pds_url(url) + }) +} + +pub fn link_local_is_rejected_test() { + assert !url_guard.is_safe_pds_url("https://169.254.169.254") +} + +pub fn rfc1918_ranges_are_rejected_test() { + ["https://10.0.0.5", "https://172.16.0.1", "https://192.168.1.1"] + |> list.each(fn(url) { + assert !url_guard.is_safe_pds_url(url) + }) +} + +pub fn adjacent_ranges_are_not_confused_with_rfc1918_test() { + // 172.15.x and 172.32.x are outside the 172.16.0.0/12 private block. + ["https://172.15.0.1", "https://172.32.0.1"] + |> list.each(fn(url) { + assert url_guard.is_safe_pds_url(url) + }) +} + +pub fn ordinary_public_address_is_accepted_test() { + // A numeric public literal, not a hostname: a real hostname would resolve + // through DNS, which would make this suite dependent on the network. + assert url_guard.is_safe_pds_url("https://8.8.8.8") +} + +pub fn ipv6_literals_are_rejected_wholesale_test() { + // No range-classification: a real PDS is a hostname, never a bare IP + // literal, so every bracketed IPv6 form is rejected outright. + ["https://[::1]", "https://[fd00::1]", "https://[fe80::1]"] + |> list.each(fn(url) { + assert !url_guard.is_safe_pds_url(url) + }) +} + +// -- alternate IPv4 encodings that a hand-rolled dotted-decimal parser ----- +// -- misses, but that `inet:getaddr` (and so the real transport) resolves -- +// +// The old check only recognised exactly four base-10 dotted octets, so a +// bare decimal integer, hex, and octal-dotted forms all fell through to +// "not IP-shaped, assume safe" and reached 169.254.169.254 / 127.0.0.1. +// Resolving the host the same way the transport does closes all of them +// at once instead of adding one more hand-parsed special case per format. + +pub fn bare_decimal_integer_bypass_is_rejected_test() { + // 2852039166 == 169.254.169.254 packed into a single 32-bit integer. + assert !url_guard.is_safe_pds_url("https://2852039166") +} + +pub fn hex_encoded_bypass_is_rejected_test() { + // 0xA9FEA9FE == 169.254.169.254. + assert !url_guard.is_safe_pds_url("https://0xA9FEA9FE") +} + +pub fn octal_dotted_bypass_is_rejected_test() { + // Each octet is octal: 0177 == 127, so this is 127.0.0.1. + assert !url_guard.is_safe_pds_url("https://0177.0.0.1") +} + +pub fn mixed_hex_octet_bypass_is_rejected_test() { + // 0x7f == 127, so this is also 127.0.0.1, mixing a hex first octet with + // plain-decimal remaining octets. + assert !url_guard.is_safe_pds_url("https://0x7f.0.0.1") +} + +pub fn this_network_address_is_rejected_test() { + assert !url_guard.is_safe_pds_url("https://0.0.0.0") +} + +pub fn cgnat_range_is_rejected_test() { + assert !url_guard.is_safe_pds_url("https://100.64.0.1") +} + +pub fn limited_broadcast_is_rejected_test() { + assert !url_guard.is_safe_pds_url("https://255.255.255.255") +}