diff --git a/CLAUDE.md b/CLAUDE.md index 772ea70..5aa1911 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -10,7 +10,7 @@ For deeper context (data flow, components, ops): [docs/architecture.md](docs/arc - **Async runtime**: Tokio 1.35 (`full`). - **HTTP**: `actix-web 4.9`, `actix-rt 2.9`. - **Nostr**: `nostr-sdk 0.27`. -- **HTTP client**: shared `reqwest::Client` with explicit timeouts (2 s connect, 5 s total). +- **HTTP client**: shared `reqwest::Client` with explicit timeouts (2 s connect, 5 s total). UnifiedPush is the one exception: it builds its own via `UnifiedPushService::build_client()`, identical except that redirects are refused. Its endpoint URL is attacker-supplied, and the SSRF guard only inspects the first hop. - **Rate limiting**: `governor 0.6` (already approved, dual-keyed limiter). - **Privacy hash**: `blake3` (salted truncated keyed hash for log correlators). - **Other notable deps**: `jsonwebtoken` (FCM OAuth), `secp256k1`, `chacha20poly1305`, `hkdf`, `sha2` (gated `crypto` module reserved for future encrypted-token registration), `uuid` (UUIDv4 `x-request-id`). @@ -74,6 +74,7 @@ src/ ├── push/ │ ├── mod.rs # PushService trait │ ├── dispatcher.rs # PushDispatcher (lock-free) +│ ├── endpoint_guard.rs # SSRF guard for UnifiedPush endpoint URLs │ ├── fcm.rs # FCM v1, OAuth2 service-account JWT │ └── unifiedpush.rs # UnifiedPush backend, persistent endpoint store ├── store/mod.rs # In-memory TokenStore + TTL cleanup diff --git a/docs/api.md b/docs/api.md index 0365b5a..f5e05b1 100644 --- a/docs/api.md +++ b/docs/api.md @@ -77,7 +77,7 @@ Request: | Field | Type | Description | |-----------------|--------|------------------------------------------------------------------------------------------------------------------------| | `trade_pubkey` | string | 64 hex characters | -| `token` | string | FCM device token, or UnifiedPush endpoint URL | +| `token` | string | FCM device token, or UnifiedPush endpoint URL. Non-empty, at most 4096 bytes. If it parses as an `http`/`https` URL it must be `https` and point at a public address (see below). | | `platform` | string | `"android"` or `"ios"` | | `mostro_pubkey` | string | 64 hex characters. Optional on the wire; required when the trusted-instance whitelist is non-empty (see below). | @@ -226,10 +226,86 @@ curl -i -X POST http://localhost:8080/api/notify \ |--------|-------------------------------------------------------------------------------| | 200 | `/api/health`, `/api/info`, `/api/status`, `/api/register`, `/api/unregister` | | 202 | `/api/notify` on parse-valid input | -| 400 | Malformed body, invalid `trade_pubkey`, invalid `platform`, empty `token` | +| 400 | Malformed body, body over the size limit, invalid `trade_pubkey`, invalid `platform`, empty or oversized `token` | | 429 | `/api/register`, `/api/unregister`, `/api/notify` rate limits | | 500 | Rate-limited endpoints fail closed when the per-IP key cannot be extracted | +### Push endpoint validation + +The `token` field is overloaded: for FCM it is an opaque registration token, +for UnifiedPush it is the URL the server will POST to. The request carries no +field saying which, so the server inspects the value instead. + +A token that parses as an `http` or `https` URL is treated as a push endpoint +and must satisfy all of: + +- scheme is `https` +- the host is not a private, loopback, link-local, CGNAT, or otherwise + non-routable address, including the IPv4-mapped IPv6 spellings of those + (`https://[::ffff:169.254.169.254]/`) + +Anything that does not parse as an `http`/`https` URL is treated as an opaque +backend token and passed through untouched, so FCM registrations are +unaffected. A short list of clearly unusable schemes (`file`, `ftp`, `gopher`, +`data`, `dict`, `ldap`) is refused outright. + +Rejection — `400 Bad Request`: + +```json +{ + "success": false, + "message": "Invalid push endpoint" +} +``` + +The message is identical for every rejection reason on purpose. A caller must +not be able to use the response to distinguish "unsupported scheme" from +"internal address" and map the server's network. + +Registration performs the checks above without touching the network. The +authoritative check runs again immediately before the outbound POST and +additionally resolves domain hosts, refusing the endpoint if any resolved +address is non-public. + +## Request size limits + +Every endpoint that accepts a body caps it. Actix's own default is 2 MB, which +on unauthenticated endpoints is a free memory-amplification primitive. + +| Endpoint | Max body | Notes | +|-------------------|----------|--------------------------------------------------------------| +| `/api/register` | 8 KiB | Sized to fit a 4096-byte `token` plus the other fields | +| `/api/unregister` | 1 KiB | Body carries a single 64-char hex pubkey | +| `/api/notify` | 1 KiB | Body carries a single 64-char hex pubkey | + +The `token` field of a registration is bounded separately at **4096 bytes**. The +body cap stops an enormous request; the field cap stops a merely large one from +being retained in the in-memory token store for its whole TTL. + +Exceeding either limit is reported as `400 Bad Request`, **not** `413 Payload +Too Large`: + +```json +{ + "success": false, + "message": "Request body too large" +} +``` + +```json +{ + "success": false, + "message": "Token exceeds maximum length" +} +``` + +Returning `400` rather than `413` is deliberate. The response bodies of +`/api/register` and `/api/unregister` are frozen against pre-1.1 fixtures, and +`/api/notify` is contractually restricted to a single failure status, so the +size cap reuses the shape those endpoints already emit instead of introducing a +new one. Only the payload-overflow case is remapped; every other body-parsing +failure keeps its previous behaviour. + ## Rate limiting `/api/register` and `/api/unregister` share a per-IP limit to protect the diff --git a/docs/configuration.md b/docs/configuration.md index c1ed28b..08ff707 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -158,6 +158,27 @@ NOTIFY_TRUST_PROXY_HEADERS=true RUST_LOG=info ``` +## UnifiedPush endpoint policy + +When UnifiedPush is enabled, the registered device token *is* the URL the +server POSTs to, which makes it a request-forgery surface reachable from the +unauthenticated `/api/register` and `/api/notify` pair. `src/push/endpoint_guard.rs` +refuses any endpoint that is not `https`, or whose host is (or resolves to) a +non-routable address: loopback, RFC1918, link-local, CGNAT, unique-local and +the rest. + +There is no configuration knob to relax this. Two consequences for operators: + +- **A UnifiedPush distributor on a private range is not reachable.** Front it + with a public hostname and a TLS certificate. This is deliberate: the blast + radius of accidentally reaching a cloud metadata service or a container-local + admin port is far worse than that of an operator having to publish a name. +- **Local mock servers cannot be used to exercise the dispatch path**, since + they bind to loopback. + +Full detail, including the known DNS-rebinding limitation, is in +[unifiedpush.md](./unifiedpush.md#endpoint-validation). + ## Generating a Firebase service account 1. [Firebase Console](https://console.firebase.google.com/) → your project → Project Settings → Service accounts. diff --git a/docs/unifiedpush.md b/docs/unifiedpush.md index 877c1fd..da9d68c 100644 --- a/docs/unifiedpush.md +++ b/docs/unifiedpush.md @@ -38,11 +38,39 @@ The endpoint store is loaded once at startup. Failures to read or parse the file If `UNIFIEDPUSH_ENABLED=false`, the service is not added to the dispatcher slice. Existing entries in `data/unifiedpush_endpoints.json` are ignored at runtime but not deleted. +## Endpoint validation + +The registered device token *is* the URL the server POSTs to, which makes it a +request-forgery surface reachable from the unauthenticated `/api/register` and +`/api/notify` pair. `src/push/endpoint_guard.rs` is the single place that +decides whether an endpoint may be contacted. + +Two passes: + +1. **Registration** — static, no network. Refuses non-`https` schemes and hosts + that are IP literals outside the public internet. +2. **Dispatch** — runs immediately before the outbound POST, repeats the static + checks, and resolves domain hosts, refusing if *any* resolved address is + non-public. This is the authoritative gate; registration is defence in + depth and fast feedback. + +The guard only ever inspects the **first hop**, which is why this backend does +not use the shared HTTP client. `reqwest` follows up to 10 redirects by +default, so a registered endpoint answering `302 Location: http://169.254.169.254/` +would walk the request past the guard entirely. `UnifiedPushService::build_client` +refuses redirects outright: a push endpoint has no legitimate reason to issue +one. A regression test asserts the second hop is never requested. + +Known limitation: `reqwest` resolves the host again when it connects, so a DNS +record with a very short TTL can change between validation and connection. +Closing that race requires pinning the validated address into the connection; +tracked in [#39](https://github.com/MostroP2P/mostro-push-server/issues/39). + ## Operational notes - UnifiedPush has no per-payload distinction between silent and visible push. `send_silent_to_token` falls back to `send_to_token`, which is the same code path the Nostr listener uses. - There is no rate limiting on outbound UnifiedPush calls beyond what the server-wide `reqwest::Client` timeouts provide (2 s connect, 5 s total). -- The endpoint URL is fully attacker-controlled in the sense that the distributor can be any HTTP server. The shared `reqwest::Client` enforces TLS and the timeouts; the server does not pin certificates or restrict hostnames. +- The endpoint URL is fully attacker-controlled in the sense that the distributor can be any HTTP server. A dedicated `reqwest::Client` (`UnifiedPushService::build_client`) enforces TLS, the timeouts, and a no-redirect policy; the server does not pin certificates. ## Reference diff --git a/src/api/notify.rs b/src/api/notify.rs index e41525d..b2f53c5 100644 --- a/src/api/notify.rs +++ b/src/api/notify.rs @@ -1,9 +1,10 @@ use actix_web::{ body::MessageBody, dev::{ServiceRequest, ServiceResponse}, + error::{InternalError, JsonPayloadError}, http::header::{HeaderName, HeaderValue}, middleware::Next, - web, Error, HttpResponse, Responder, + web, Error, HttpRequest, HttpResponse, Responder, }; use log::{info, warn}; use serde::{Deserialize, Serialize}; @@ -27,6 +28,42 @@ pub struct NotifyRequest { pub trade_pubkey: String, } +/// Upper bound on the JSON body accepted by /api/notify. +/// +/// The body is a fixed shape carrying one 64-char hex pubkey, so a kilobyte is +/// already generous. Actix defaults to 2 MB, which on an unauthenticated +/// endpoint is a free memory-amplification primitive. +const MAX_BODY_BYTES: usize = 1024; + +/// JSON extractor config for /api/notify. +/// +/// Two jobs: cap the body, and keep the response contract intact while doing +/// it. D-12 / hard constraint 2 allow exactly two failure statuses on this +/// endpoint — 400 for a parse failure and 400 for a bad pubkey — so an +/// oversized body is reported as the same 400, not as actix's default 413. +/// Every other `JsonPayloadError` variant falls through to actix's own +/// handling, leaving the existing malformed-JSON behaviour untouched. +/// +/// The 400 carries no information about the pubkey, so it cannot be used to +/// distinguish a registered from an unregistered one. +pub fn json_config() -> web::JsonConfig { + web::JsonConfig::default() + .limit(MAX_BODY_BYTES) + .error_handler(|err, _req: &HttpRequest| -> Error { + if matches!( + err, + JsonPayloadError::Overflow { .. } | JsonPayloadError::OverflowKnownLength { .. } + ) { + let response = HttpResponse::BadRequest().json(NotifyError { + success: false, + message: "Request body too large".to_string(), + }); + return InternalError::from_response(err, response).into(); + } + err.into() + }) +} + /// 400-only response shape for /api/notify. /// /// Defined locally (NOT a re-import of routes::RegisterResponse) per @@ -303,4 +340,29 @@ mod tests { .unwrap(); assert!(Uuid::parse_str(id_value).is_ok()); } + /// Hard constraint 2 allows exactly one failure status on this endpoint. + /// An oversized body must therefore be a 400 in the endpoint's own shape, + /// not actix's default 413, and it must carry nothing about the pubkey. + #[actix_web::test] + async fn notify_oversized_body_returns_400_not_413() { + let c = make_test_components(); + let app = test::init_service(build_test_actix_app(c)).await; + + let req = test::TestRequest::post() + .uri("/api/notify") + .insert_header(("Fly-Client-IP", "8.8.8.8")) + .set_json(serde_json::json!({ + "trade_pubkey": TEST_PUBKEY, + "padding": "a".repeat(super::MAX_BODY_BYTES * 2) + })) + .to_request(); + let resp = test::call_service(&app, req).await; + + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let body = test::read_body(resp).await; + assert_eq!( + std::str::from_utf8(&body).unwrap(), + r#"{"success":false,"message":"Request body too large"}"# + ); + } } diff --git a/src/api/routes.rs b/src/api/routes.rs index 8eb4182..f8b6a14 100644 --- a/src/api/routes.rs +++ b/src/api/routes.rs @@ -1,5 +1,6 @@ +use actix_web::error::{InternalError, JsonPayloadError}; use actix_web::middleware::from_fn; -use actix_web::{web, HttpResponse, Responder}; +use actix_web::{web, Error, HttpRequest, HttpResponse, Responder}; use log::{info, warn}; use serde::{Deserialize, Serialize}; use std::collections::HashSet; @@ -8,6 +9,7 @@ use tokio::sync::Semaphore; use crate::api::notify::{notify_token, request_id_mw}; use crate::api::rate_limit::{per_ip_rate_limit_mw, register_ip_rate_limit_mw, PerPubkeyLimiter}; +use crate::push::endpoint_guard::{classify_token, TokenShape}; use crate::push::PushDispatcher; use crate::store::{Platform, TokenStore, TokenStoreStats}; use crate::utils::log_pubkey::log_pubkey; @@ -66,6 +68,53 @@ pub struct AppState { pub trusted_whitelist_enabled: bool, } +/// Upper bound on the JSON body accepted by /api/register. +/// +/// Sized to fit the largest legitimate registration: a 64-char pubkey, a +/// platform string, an optional 64-char Mostro pubkey and a device token of up +/// to `MAX_TOKEN_BYTES`, with room to spare for whitespace and future fields. +const MAX_REGISTER_BODY_BYTES: usize = 8 * 1024; + +/// Upper bound on the JSON body accepted by /api/unregister. +/// +/// The body carries a single 64-char hex pubkey and nothing else. +const MAX_UNREGISTER_BODY_BYTES: usize = 1024; + +/// Upper bound on the `token` field of a registration. +/// +/// FCM registration tokens sit around 160-200 characters; UnifiedPush +/// endpoints are URLs. 4 KB leaves generous headroom while keeping an +/// unbounded string out of the in-memory `TokenStore`, which has no on-disk +/// backing to bound its growth (hard constraint 4). +const MAX_TOKEN_BYTES: usize = 4096; + +/// JSON extractor config for /api/register and /api/unregister. +/// +/// Caps the body without introducing a new status code: hard constraint 3 +/// freezes these response bodies, and its documented exceptions are 403 +/// (whitelist) and 429 (rate limit) only. An oversized body is therefore +/// reported as the 400 shape both endpoints already use, rather than actix's +/// default 413. Every other `JsonPayloadError` variant falls through to +/// actix's own handling, so malformed-JSON behaviour is unchanged. +fn json_config(limit: usize) -> web::JsonConfig { + web::JsonConfig::default() + .limit(limit) + .error_handler(|err, _req: &HttpRequest| -> Error { + if matches!( + err, + JsonPayloadError::Overflow { .. } | JsonPayloadError::OverflowKnownLength { .. } + ) { + let response = HttpResponse::BadRequest().json(RegisterResponse { + success: false, + message: "Request body too large".to_string(), + platform: None, + }); + return InternalError::from_response(err, response).into(); + } + err.into() + }) +} + pub fn configure(cfg: &mut web::ServiceConfig) { cfg.service( web::scope("/api") @@ -73,17 +122,20 @@ pub fn configure(cfg: &mut web::ServiceConfig) { .route("/status", web::get().to(status)) .service( web::resource("/register") + .app_data(json_config(MAX_REGISTER_BODY_BYTES)) .wrap(from_fn(register_ip_rate_limit_mw)) .route(web::post().to(register_token)), ) .service( web::resource("/unregister") + .app_data(json_config(MAX_UNREGISTER_BODY_BYTES)) .wrap(from_fn(register_ip_rate_limit_mw)) .route(web::post().to(unregister_token)), ) .route("/info", web::get().to(server_info)) .service( web::resource("/notify") + .app_data(crate::api::notify::json_config()) // Order matters: actix-web wraps in reverse-registration order, so the // last `.wrap()` is the outermost. `request_id_mw` MUST be outermost so // it runs even when `per_ip_rate_limit_mw` short-circuits with 429, @@ -148,6 +200,36 @@ async fn register_token( }); } + // Bound the token independently of the body limit: the body cap stops a + // huge request, this stops a merely large one from being retained in the + // in-memory store for the whole TTL. + if req.token.len() > MAX_TOKEN_BYTES { + warn!("Token exceeds maximum length"); + return HttpResponse::BadRequest().json(RegisterResponse { + success: false, + message: "Token exceeds maximum length".to_string(), + platform: None, + }); + } + + // SSRF guard, static pass. `/api/register` carries no field saying which + // backend a token belongs to, so an FCM token and a UnifiedPush endpoint + // URL arrive indistinguishable. Only values that parse as http(s) URLs are + // inspected; everything else is opaque and left to its own backend. + // + // The message is deliberately identical for every rejection reason: the + // caller must not be able to use the response to map the server's network. + // The authoritative check, including DNS resolution, runs at dispatch time + // in `endpoint_guard::validate_endpoint`. + if let TokenShape::Rejected(reason) = classify_token(&req.token) { + warn!("Register denied: unusable push endpoint ({})", reason); + return HttpResponse::BadRequest().json(RegisterResponse { + success: false, + message: "Invalid push endpoint".to_string(), + platform: None, + }); + } + // Parse platform let platform = match req.platform.to_lowercase().as_str() { "android" => Platform::Android, @@ -886,4 +968,202 @@ mod tests { ); } } + /// The body cap must not introduce a new status code: hard constraint 3 + /// freezes these bodies and lists 403 and 429 as the only exceptions, so an + /// oversized payload is reported as the 400 shape the endpoint already uses + /// rather than as actix's default 413. + #[actix_web::test] + async fn register_oversized_body_returns_400_not_413() { + let c = make_test_components(); + let app = atest::init_service(build_test_actix_app(c)).await; + + let req = atest::TestRequest::post() + .uri("/api/register") + .insert_header(("Fly-Client-IP", "8.8.8.8")) + .set_json(serde_json::json!({ + "trade_pubkey": TEST_PUBKEY, + "token": "a".repeat(1024 * 1024), + "platform": "android" + })) + .to_request(); + let resp = atest::call_service(&app, req).await; + + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let body = atest::read_body(resp).await; + assert_eq!( + std::str::from_utf8(&body).unwrap(), + r#"{"success":false,"message":"Request body too large"}"# + ); + } + + /// A token that fits the body cap but exceeds `MAX_TOKEN_BYTES` is rejected by + /// the field check. Without it such a token would sit in the in-memory store + /// for the whole TTL. + #[actix_web::test] + async fn register_token_over_max_length_returns_400() { + let c = make_test_components(); + let app = atest::init_service(build_test_actix_app(c)).await; + + let req = atest::TestRequest::post() + .uri("/api/register") + .insert_header(("Fly-Client-IP", "8.8.8.8")) + .set_json(serde_json::json!({ + "trade_pubkey": TEST_PUBKEY, + "token": "a".repeat(super::MAX_TOKEN_BYTES + 1), + "platform": "android" + })) + .to_request(); + let resp = atest::call_service(&app, req).await; + + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let body = atest::read_body(resp).await; + assert_eq!( + std::str::from_utf8(&body).unwrap(), + r#"{"success":false,"message":"Token exceeds maximum length"}"# + ); + } + + /// Boundary: exactly `MAX_TOKEN_BYTES` is still accepted, and the 200 body + /// stays byte-identical to the pre-1.1 fixture. + #[actix_web::test] + async fn register_token_at_max_length_succeeds() { + let c = make_test_components(); + let app = atest::init_service(build_test_actix_app(c)).await; + + let req = atest::TestRequest::post() + .uri("/api/register") + .insert_header(("Fly-Client-IP", "8.8.8.8")) + .set_json(serde_json::json!({ + "trade_pubkey": TEST_PUBKEY, + "token": "a".repeat(super::MAX_TOKEN_BYTES), + "platform": "android" + })) + .to_request(); + let resp = atest::call_service(&app, req).await; + + assert_eq!(resp.status(), StatusCode::OK); + let body = atest::read_body(resp).await; + assert_eq!( + std::str::from_utf8(&body).unwrap(), + r#"{"success":true,"message":"Token registered successfully","platform":"android"}"# + ); + } + + #[actix_web::test] + async fn unregister_oversized_body_returns_400_not_413() { + let c = make_test_components(); + let app = atest::init_service(build_test_actix_app(c)).await; + + let req = atest::TestRequest::post() + .uri("/api/unregister") + .insert_header(("Fly-Client-IP", "8.8.8.8")) + .set_json(serde_json::json!({ + "trade_pubkey": TEST_PUBKEY, + "padding": "a".repeat(super::MAX_UNREGISTER_BODY_BYTES * 2) + })) + .to_request(); + let resp = atest::call_service(&app, req).await; + + assert_eq!(resp.status(), StatusCode::BAD_REQUEST); + let body = atest::read_body(resp).await; + assert_eq!( + std::str::from_utf8(&body).unwrap(), + r#"{"success":false,"message":"Request body too large"}"# + ); + } + + /// The SSRF guard must refuse an endpoint pointing at cloud metadata, and + /// the message must not reveal *why* it was refused. + #[actix_web::test] + async fn register_with_non_public_endpoint_returns_400() { + for token in [ + "http://169.254.169.254/latest/meta-data/", + "https://169.254.169.254/latest/meta-data/", + "https://127.0.0.1:8080/", + "https://[::ffff:169.254.169.254]/", + "https://10.0.0.1/push", + "file:///etc/passwd", + ] { + let c = make_test_components(); + let app = atest::init_service(build_test_actix_app(c)).await; + + let req = atest::TestRequest::post() + .uri("/api/register") + .insert_header(("Fly-Client-IP", "8.8.8.8")) + .set_json(serde_json::json!({ + "trade_pubkey": TEST_PUBKEY, + "token": token, + "platform": "android" + })) + .to_request(); + let resp = atest::call_service(&app, req).await; + + assert_eq!(resp.status(), StatusCode::BAD_REQUEST, "{token}"); + let body = atest::read_body(resp).await; + assert_eq!( + std::str::from_utf8(&body).unwrap(), + r#"{"success":false,"message":"Invalid push endpoint"}"#, + "{token}" + ); + } + } + + /// Regression guard for the guard itself. FCM is the only backend enabled + /// in production and `/api/register` cannot tell an FCM token from a + /// UnifiedPush URL, so an over-eager SSRF check here would break every + /// real registration. The 200 body must also stay byte-identical. + #[actix_web::test] + async fn register_with_fcm_token_is_unaffected_by_the_ssrf_guard() { + for token in [ + "cXY7bF2mRk2vQ1s:APA91bH8xYzKq3vN9pLmT4wRbC7dEfGhIjKlMnOpQrStUvWxYz", + "d1PxYz8QRk-2vQ1sAbCdEf", + ] { + let c = make_test_components(); + let app = atest::init_service(build_test_actix_app(c)).await; + + let req = atest::TestRequest::post() + .uri("/api/register") + .insert_header(("Fly-Client-IP", "8.8.8.8")) + .set_json(serde_json::json!({ + "trade_pubkey": TEST_PUBKEY, + "token": token, + "platform": "android" + })) + .to_request(); + let resp = atest::call_service(&app, req).await; + + assert_eq!(resp.status(), StatusCode::OK, "{token}"); + let body = atest::read_body(resp).await; + assert_eq!( + std::str::from_utf8(&body).unwrap(), + r#"{"success":true,"message":"Token registered successfully","platform":"android"}"#, + "{token}" + ); + } + } + + /// A legitimate public UnifiedPush endpoint is still accepted. + #[actix_web::test] + async fn register_with_public_unifiedpush_endpoint_succeeds() { + let c = make_test_components(); + let app = atest::init_service(build_test_actix_app(c)).await; + + let req = atest::TestRequest::post() + .uri("/api/register") + .insert_header(("Fly-Client-IP", "8.8.8.8")) + .set_json(serde_json::json!({ + "trade_pubkey": TEST_PUBKEY, + "token": "https://ntfy.sh/abcdef", + "platform": "android" + })) + .to_request(); + let resp = atest::call_service(&app, req).await; + + assert_eq!(resp.status(), StatusCode::OK); + let body = atest::read_body(resp).await; + assert_eq!( + std::str::from_utf8(&body).unwrap(), + r#"{"success":true,"message":"Token registered successfully","platform":"android"}"# + ); + } } diff --git a/src/main.rs b/src/main.rs index fa63527..64fb601 100644 --- a/src/main.rs +++ b/src/main.rs @@ -78,10 +78,15 @@ async fn main() -> std::io::Result<()> { // Initialize push services let mut push_services: Vec<(Arc, &'static str)> = Vec::new(); - // Keep UnifiedPush service separate for endpoint management + // Keep UnifiedPush service separate for endpoint management. + // + // It also gets its own HTTP client rather than the shared one: its endpoint + // URL is attacker-supplied, so redirect following would let a registered + // endpoint bounce the request to an internal address, past the guard that + // only inspects the first hop. See `UnifiedPushService::build_client`. let unifiedpush_service = Arc::new(UnifiedPushService::new( config.clone(), - Arc::clone(&http_client), + Arc::new(UnifiedPushService::build_client()), )); // Load existing endpoints from disk diff --git a/src/push/endpoint_guard.rs b/src/push/endpoint_guard.rs new file mode 100644 index 0000000..4c73ef0 --- /dev/null +++ b/src/push/endpoint_guard.rs @@ -0,0 +1,425 @@ +//! SSRF guard for UnifiedPush endpoint URLs. +//! +//! The UnifiedPush backend treats a registered device token as a URL and POSTs +//! to it, so an unvalidated token is a request-forgery primitive reachable from +//! the unauthenticated `/api/register` + `/api/notify` pair. This module is the +//! single place that decides whether such a URL may be contacted. +//! +//! Two entry points, deliberately different in strength: +//! +//! - [`classify_token`] is pure and never touches the network. It runs at +//! registration to keep obviously hostile values out of the token store. +//! - [`validate_endpoint`] runs on the dispatch path, immediately before the +//! outbound POST, and additionally resolves domain hosts so a name pointing +//! at an internal address is refused. This is the authoritative check. +//! +//! Both passes inspect only the **first hop**. That is only sufficient because +//! the UnifiedPush backend runs on a client that refuses redirects; see +//! `UnifiedPushService::build_client`. Reusing a redirect-following client here +//! would reduce this whole module to decoration. +//! +//! Registration cannot be the only gate: `/api/register` carries no field +//! saying which backend a token belongs to, so an FCM token and a UnifiedPush +//! URL arrive indistinguishable. `classify_token` therefore only inspects +//! values whose scheme is `http` or `https`; anything else is opaque and left +//! for its own backend to interpret. + +use std::fmt; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::str::FromStr; + +use reqwest::Url; +use tokio::net::lookup_host; + +/// Schemes refused outright at registration. None of them can produce an +/// outbound request through `reqwest`, so this is data hygiene rather than an +/// SSRF control: a token spelled this way is broken whichever backend claims +/// it, and there is no reason to hold it in the store until its TTL expires. +const DANGEROUS_SCHEMES: &[&str] = &["file", "ftp", "gopher", "data", "dict", "ldap"]; + +/// Why an endpoint URL was refused. Kept coarse on purpose: the HTTP layer +/// collapses every variant into one message so a caller cannot use the +/// response to map the server's internal network. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum EndpointRejection { + /// Scheme is `http`. Push endpoints must be TLS-protected. + InsecureScheme, + /// URL carries no host (e.g. `https:///path`). + MissingHost, + /// Host is, or resolves to, an address outside the public internet. + NonPublicAddress, + /// Host is a domain that could not be resolved. + UnresolvableHost, +} + +impl fmt::Display for EndpointRejection { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + let s = match self { + EndpointRejection::InsecureScheme => "scheme must be https", + EndpointRejection::MissingHost => "URL has no host", + EndpointRejection::NonPublicAddress => "host is not a public address", + EndpointRejection::UnresolvableHost => "host could not be resolved", + }; + f.write_str(s) + } +} + +/// What a registration `token` turned out to be. +#[derive(Debug, PartialEq, Eq)] +pub enum TokenShape { + /// Not an `http`/`https` URL, so not something this guard can reason + /// about: an FCM registration token, or a value no backend will accept. + /// `reqwest` refuses to issue a request for a non-HTTP scheme, so an + /// opaque value cannot become an outbound request by itself. + Opaque, + /// A syntactically acceptable https endpoint. Domain hosts still face DNS + /// validation at dispatch time. + Endpoint, + /// An http(s) URL that must not be contacted. + Rejected(EndpointRejection), +} + +/// Pure, network-free classification of a registration token. +pub fn classify_token(token: &str) -> TokenShape { + let url = match Url::parse(token) { + Ok(url) => url, + // Not a URL at all. FCM tokens without a `:` land here. + Err(_) => return TokenShape::Opaque, + }; + + // FCM tokens containing a `:` do parse, with the part before the colon + // taken as the scheme and no host. Only http(s) can become an outbound + // request — `reqwest` refuses every other scheme — so only those are this + // guard's SSRF business. + // + // `DANGEROUS_SCHEMES` is a separate, weaker concern: hygiene. Those values + // are neither a valid FCM token nor a usable push endpoint, so they are + // refused rather than stored, even though no backend would act on them. + match url.scheme() { + "https" => {} + "http" => return TokenShape::Rejected(EndpointRejection::InsecureScheme), + scheme if DANGEROUS_SCHEMES.contains(&scheme) => { + return TokenShape::Rejected(EndpointRejection::InsecureScheme) + } + _ => return TokenShape::Opaque, + } + + let host = match url.host_str() { + Some(host) if !host.is_empty() => host, + _ => return TokenShape::Rejected(EndpointRejection::MissingHost), + }; + + // A host that parses as an IP is settled here; anything else is a domain + // and is resolved at dispatch time. See `validate_endpoint`. + if let Some(ip) = parse_host_ip(host) { + if is_non_public(ip) { + return TokenShape::Rejected(EndpointRejection::NonPublicAddress); + } + } + + TokenShape::Endpoint +} + +/// Authoritative check, run immediately before the outbound POST. +/// +/// Applies [`classify_token`] and, for domain hosts, resolves the name and +/// refuses if **any** resolved address is non-public. A domain that resolves +/// to several addresses is refused if even one of them is internal, so a +/// round-robin record cannot be used to slip through. +/// +/// This narrows but does not eliminate DNS rebinding: `reqwest` performs its +/// own resolution when it connects, so a record with a very short TTL can +/// still change between this check and that connection. Closing that race +/// requires pinning the validated address into the connection itself, tracked +/// in #39. +pub async fn validate_endpoint(token: &str) -> Result<(), EndpointRejection> { + let url = match classify_token(token) { + TokenShape::Endpoint => Url::parse(token).map_err(|_| EndpointRejection::MissingHost)?, + TokenShape::Rejected(reason) => return Err(reason), + // Reached only if a caller hands a non-HTTP value straight to the + // dispatch path; refuse rather than let reqwest decide. + TokenShape::Opaque => return Err(EndpointRejection::InsecureScheme), + }; + + let host = match url.host_str() { + Some(host) if !host.is_empty() => host.to_string(), + _ => return Err(EndpointRejection::MissingHost), + }; + + // IP literals were already settled by the static pass. + if parse_host_ip(&host).is_some() { + return Ok(()); + } + let domain = host; + + let port = url.port_or_known_default().unwrap_or(443); + let resolved = lookup_host((domain.as_str(), port)) + .await + .map_err(|_| EndpointRejection::UnresolvableHost)?; + + let mut saw_address = false; + for addr in resolved { + saw_address = true; + if is_non_public(addr.ip()) { + return Err(EndpointRejection::NonPublicAddress); + } + } + + if saw_address { + Ok(()) + } else { + Err(EndpointRejection::UnresolvableHost) + } +} + +/// Parses a URL host component as an IP address, if it is one. +/// +/// `Url::host_str` serialises IPv6 hosts in their bracketed form (`[::1]`), +/// which `IpAddr::from_str` does not accept, so the brackets are stripped +/// first. A domain never parses as an IP, which makes the distinction +/// unambiguous. Alternative literal spellings (`0x7f.1`, octal, decimal) are +/// already normalised by `Url::parse` before reaching this function. +fn parse_host_ip(host: &str) -> Option { + let unbracketed = host + .strip_prefix('[') + .and_then(|rest| rest.strip_suffix(']')) + .unwrap_or(host); + IpAddr::from_str(unbracketed).ok() +} + +/// Whether an address is anything other than a routable public one. +/// +/// Deliberately conservative: everything not clearly on the public internet is +/// refused, because the blast radius of a false negative (reaching a cloud +/// metadata service, a container-local admin port) is far worse than that of a +/// false positive (a self-hosted push server on a private range, which the +/// operator can front with a public name). +fn is_non_public(ip: IpAddr) -> bool { + match ip { + IpAddr::V4(addr) => is_non_public_v4(addr), + IpAddr::V6(addr) => is_non_public_v6(addr), + } +} + +fn is_non_public_v4(addr: Ipv4Addr) -> bool { + let [a, b, ..] = addr.octets(); + + addr.is_loopback() // 127.0.0.0/8 + || addr.is_private() // 10/8, 172.16/12, 192.168/16 + || addr.is_link_local() // 169.254.0.0/16 — cloud metadata lives here + || addr.is_broadcast() + || addr.is_documentation() + || addr.is_multicast() + || a == 0 // 0.0.0.0/8 "this network" + || (a == 100 && (64..=127).contains(&b)) // 100.64.0.0/10 CGNAT + || (a == 192 && b == 0) // 192.0.0.0/24 IETF protocol assignments + || (a == 198 && (18..=19).contains(&b)) // 198.18.0.0/15 benchmarking + || a >= 240 // 240.0.0.0/4 reserved +} + +fn is_non_public_v6(addr: Ipv6Addr) -> bool { + // `https://[::ffff:169.254.169.254]/` parses as an Ipv6 host, so the v4 + // ranges must be applied to the embedded address or every v4 rule above is + // trivially bypassable. Covers both ::ffff:a.b.c.d and ::a.b.c.d. + if let Some(v4) = addr.to_ipv4() { + return is_non_public_v4(v4); + } + + let first = addr.segments()[0]; + + addr.is_loopback() + || addr.is_unspecified() + || addr.is_multicast() + || (first & 0xfe00) == 0xfc00 // fc00::/7 unique local + || (first & 0xffc0) == 0xfe80 // fe80::/10 link local +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Realistic FCM registration tokens. These MUST stay accepted: FCM is the + /// only backend enabled in production, and `/api/register` cannot tell an + /// FCM token from a UnifiedPush URL, so an over-eager guard here would + /// break every real registration. + const FCM_TOKENS: &[&str] = &[ + "cXY7bF2mRk2vQ1s:APA91bH8xYzKq3vN9pLmT4wRbC7dEfGhIjKlMnOpQrStUvWxYz", + "fMEP0vJhS9-abcdefghij:APA91bExampleTokenValue1234567890", + "d1PxYz8QRk-2vQ1sAbCdEf", + "", + ]; + + #[test] + fn fcm_tokens_are_opaque() { + for token in FCM_TOKENS { + assert_eq!( + classify_token(token), + TokenShape::Opaque, + "FCM token must not be treated as an endpoint: {token}" + ); + } + } + + #[test] + fn public_https_endpoint_is_allowed() { + for token in [ + "https://up.example.com/UP?token=abc", + "https://ntfy.sh/abcdef", + "https://8.8.8.8/push", + "https://[2606:4700:4700::1111]/push", + ] { + assert_eq!(classify_token(token), TokenShape::Endpoint, "{token}"); + } + } + + #[test] + fn plain_http_is_refused() { + assert_eq!( + classify_token("http://up.example.com/UP"), + TokenShape::Rejected(EndpointRejection::InsecureScheme) + ); + } + + #[test] + fn dangerous_schemes_are_refused() { + for token in [ + "file:///etc/passwd", + "ftp://internal.example.com/x", + "gopher://127.0.0.1:11211/", + ] { + assert_eq!( + classify_token(token), + TokenShape::Rejected(EndpointRejection::InsecureScheme), + "{token}" + ); + } + } + + #[test] + fn non_public_ipv4_literals_are_refused() { + for token in [ + "https://127.0.0.1:8080/", // loopback + "https://169.254.169.254/latest/", // cloud metadata + "https://10.0.0.1/", // RFC1918 + "https://172.16.0.1/", // RFC1918 + "https://192.168.1.1/", // RFC1918 + "https://100.64.0.1/", // CGNAT + "https://0.0.0.0/", // this network + "https://192.0.0.1/", // IETF assignments + "https://198.18.0.1/", // benchmarking + "https://240.0.0.1/", // reserved + ] { + assert_eq!( + classify_token(token), + TokenShape::Rejected(EndpointRejection::NonPublicAddress), + "{token}" + ); + } + } + + #[test] + fn non_public_ipv6_literals_are_refused() { + for token in [ + "https://[::1]:9000/", // loopback + "https://[fe80::1]/", // link local + "https://[fc00::1]/", // unique local + "https://[::]/", // unspecified + ] { + assert_eq!( + classify_token(token), + TokenShape::Rejected(EndpointRejection::NonPublicAddress), + "{token}" + ); + } + } + + /// `https://[::ffff:169.254.169.254]/` parses as an IPv6 host. Without + /// unmapping the embedded address, every IPv4 rule above is one bracket + /// away from being bypassed. + #[test] + fn ipv4_mapped_into_ipv6_is_refused() { + for token in [ + "https://[::ffff:169.254.169.254]/latest/", + "https://[::ffff:127.0.0.1]/", + "https://[::ffff:10.0.0.1]/", + ] { + assert_eq!( + classify_token(token), + TokenShape::Rejected(EndpointRejection::NonPublicAddress), + "{token}" + ); + } + } + + /// Alternative literal spellings are normalised by `Url::parse` before the + /// guard sees them, so they cannot be used to smuggle a loopback address. + #[test] + fn obfuscated_ipv4_spellings_are_normalised_and_refused() { + for token in [ + "https://0x7f.1/", + "https://2130706433/", + "https://017700000001/", + ] { + assert_eq!( + classify_token(token), + TokenShape::Rejected(EndpointRejection::NonPublicAddress), + "{token}" + ); + } + } + + #[test] + fn public_addresses_are_not_flagged() { + for ip in [ + "8.8.8.8", + "1.1.1.1", + "2606:4700:4700::1111", + "93.184.216.34", + ] { + let parsed: IpAddr = ip.parse().unwrap(); + assert!(!is_non_public(parsed), "{ip} should be public"); + } + } + + /// Exercises the DNS branch offline: `localhost` always resolves to a + /// loopback address, so a domain pointing at an internal host is refused + /// even though the name itself carries no hint of that. + #[tokio::test] + async fn domain_resolving_to_loopback_is_refused() { + assert_eq!( + validate_endpoint("https://localhost/up").await, + Err(EndpointRejection::NonPublicAddress) + ); + } + + /// `.invalid` is reserved by RFC 2606 and never resolves. + #[tokio::test] + async fn unresolvable_domain_is_refused() { + assert_eq!( + validate_endpoint("https://this-host-does-not-exist.invalid/up").await, + Err(EndpointRejection::UnresolvableHost) + ); + } + + #[tokio::test] + async fn dispatch_refuses_what_registration_refuses() { + assert_eq!( + validate_endpoint("https://169.254.169.254/latest/").await, + Err(EndpointRejection::NonPublicAddress) + ); + assert_eq!( + validate_endpoint("http://up.example.com/UP").await, + Err(EndpointRejection::InsecureScheme) + ); + } + + /// An opaque value must never reach the outbound POST: the dispatch path + /// refuses it rather than letting reqwest decide what to do with it. + #[tokio::test] + async fn dispatch_refuses_opaque_tokens() { + assert_eq!( + validate_endpoint(FCM_TOKENS[0]).await, + Err(EndpointRejection::InsecureScheme) + ); + } +} diff --git a/src/push/mod.rs b/src/push/mod.rs index 5a024f0..fa4a892 100644 --- a/src/push/mod.rs +++ b/src/push/mod.rs @@ -2,6 +2,7 @@ use async_trait::async_trait; use std::sync::Arc; pub mod dispatcher; +pub mod endpoint_guard; pub mod fcm; pub mod unifiedpush; diff --git a/src/push/unifiedpush.rs b/src/push/unifiedpush.rs index cea89ca..0f65d4b 100644 --- a/src/push/unifiedpush.rs +++ b/src/push/unifiedpush.rs @@ -4,6 +4,7 @@ use serde::{Deserialize, Serialize}; use std::collections::HashMap; use std::path::PathBuf; use std::sync::Arc; +use std::time::Duration; use tokio::fs; use tokio::sync::RwLock; @@ -29,6 +30,30 @@ pub struct UnifiedPushService { } impl UnifiedPushService { + /// Builds the HTTP client this backend must use. + /// + /// Deliberately NOT the shared client from `main.rs`. `reqwest` defaults to + /// `Policy::limited(10)`, and the endpoint URL is attacker-supplied, so a + /// registered endpoint could answer `302 Location: http://169.254.169.254/` + /// and walk the request straight past `endpoint_guard`, which only ever + /// sees the first hop. A push endpoint has no legitimate reason to + /// redirect, so redirects are refused outright. + /// + /// FCM stays on the shared client: it talks to a fixed Google endpoint with + /// a token this server mints, not to a URL a caller chose. + /// + /// Timeouts mirror the shared client (2 s connect, 5 s total) so this + /// backend keeps the same bound on tying up worker threads. + pub fn build_client() -> reqwest::Client { + reqwest::Client::builder() + .redirect(reqwest::redirect::Policy::none()) + .connect_timeout(Duration::from_secs(2)) + .timeout(Duration::from_secs(5)) + .pool_idle_timeout(Some(Duration::from_secs(90))) + .build() + .expect("reqwest::Client build never fails on this config") + } + pub fn new(config: Config, client: Arc) -> Self { let storage_path = PathBuf::from("data/unifiedpush_endpoints.json"); @@ -140,7 +165,14 @@ impl PushService for UnifiedPushService { device_token: &str, _platform: &Platform, ) -> Result<(), Box> { - // For UnifiedPush, the device_token IS the endpoint URL + // For UnifiedPush, the device_token IS the endpoint URL, so this is the + // request-forgery boundary: everything reachable from here was supplied + // by an unauthenticated caller. Validate before any outbound traffic. + if let Err(reason) = crate::push::endpoint_guard::validate_endpoint(device_token).await { + warn!("UnifiedPush endpoint refused: {}", reason); + return Err(format!("UnifiedPush endpoint refused: {}", reason).into()); + } + let payload = serde_json::json!({ "type": "silent_wake", "timestamp": chrono::Utc::now().timestamp() @@ -169,3 +201,153 @@ impl PushService for UnifiedPushService { matches!(platform, Platform::Android) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::{ + Config, CryptoConfig, NostrConfig, NotifyRateLimitConfig, PushConfig, RateLimitConfig, + ServerConfig, StoreConfig, + }; + + /// Minimal config built by hand rather than through `Config::from_env`, + /// which would race with the env-mutating tests in `src/config.rs`. + fn test_config() -> Config { + Config { + nostr: NostrConfig { + relays: vec!["wss://relay.example.com".to_string()], + subscription_id: "test".to_string(), + event_kinds: vec![1059, 14], + }, + push: PushConfig { + fcm_enabled: false, + unifiedpush_enabled: true, + batch_delay_ms: 5000, + cooldown_ms: 60000, + }, + server: ServerConfig { + host: "0.0.0.0".to_string(), + port: 8080, + }, + rate_limit: RateLimitConfig { max_per_minute: 60 }, + crypto: CryptoConfig { + server_private_key: "00".repeat(32), + }, + store: StoreConfig { + token_ttl_hours: 48, + cleanup_interval_hours: 1, + }, + notify_rate_limit: NotifyRateLimitConfig { + per_pubkey_per_min: 30, + per_ip_per_min: 120, + cleanup_interval_secs: 60, + pubkey_limiter_soft_cap: 100_000, + trust_proxy_headers: false, + }, + trusted_whitelist_enabled: false, + } + } + + fn test_service() -> UnifiedPushService { + UnifiedPushService::new(test_config(), Arc::new(UnifiedPushService::build_client())) + } + + /// The guard living in `endpoint_guard` is only useful if the dispatch path + /// actually calls it. The unit tests over `validate_endpoint` would all + /// still pass if this call site were deleted, so assert on the refusal + /// message: a missing guard would surface as a reqwest connection error + /// instead, which reads very differently. + #[tokio::test] + async fn send_to_token_refuses_non_public_endpoints() { + let service = test_service(); + + for token in [ + "http://169.254.169.254/latest/meta-data/", + "https://169.254.169.254/latest/meta-data/", + "https://127.0.0.1:8080/", + "https://[::ffff:169.254.169.254]/", + "https://10.0.0.1/push", + "https://localhost/push", + ] { + let err = service + .send_to_token(token, &Platform::Android) + .await + .expect_err("dispatch MUST refuse a non-public endpoint"); + + assert!( + err.to_string().starts_with("UnifiedPush endpoint refused"), + "expected the guard to reject {token}, got: {err}" + ); + } + } + + /// An opaque value (an FCM token that reached the wrong backend) must not + /// be handed to reqwest to interpret. + #[tokio::test] + async fn send_to_token_refuses_opaque_tokens() { + let service = test_service(); + let err = service + .send_to_token("d1PxYz8QRk-2vQ1sAbCdEf", &Platform::Android) + .await + .expect_err("dispatch MUST refuse an opaque token"); + + assert!(err.to_string().starts_with("UnifiedPush endpoint refused")); + } + + /// `endpoint_guard` only ever inspects the first hop, so the client must + /// not chase a second one. Without this, a registered endpoint answering + /// `302 Location: http://169.254.169.254/` walks the request straight past + /// the guard and the whole SSRF fix is one HTTP header away from useless. + /// + /// The guard refuses loopback endpoints, so this exercises the client + /// configuration directly rather than going through `send_to_token`: the + /// client policy is the property under test. + #[tokio::test] + async fn dispatch_client_refuses_to_follow_redirects() { + let mut server = mockito::Server::new_async().await; + let base = server.url(); + + let second_hop = server + .mock("GET", "/internal-secret") + .with_status(200) + .with_body("METADATA_LEAKED") + .expect(0) + .create_async() + .await; + + let _redirector = server + .mock("POST", "/push") + .with_status(302) + .with_header("location", &format!("{base}/internal-secret")) + .create_async() + .await; + + let response = UnifiedPushService::build_client() + .post(format!("{base}/push")) + .json(&serde_json::json!({"type": "silent_wake"})) + .send() + .await + .expect("the request itself must still succeed"); + + assert_eq!( + response.status(), + 302, + "the redirect must be surfaced, not followed" + ); + let body = response.text().await.unwrap_or_default(); + assert!( + !body.contains("METADATA_LEAKED"), + "the second hop's body must never be reached, got: {body}" + ); + + // The strongest assertion: the second hop was never requested at all. + second_hop.assert_async().await; + } + + #[test] + fn unifiedpush_supports_android_only() { + let service = test_service(); + assert!(service.supports_platform(&Platform::Android)); + assert!(!service.supports_platform(&Platform::Ios)); + } +}