diff --git a/Cargo.lock b/Cargo.lock index a7744194..ec159374 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2366,12 +2366,14 @@ dependencies = [ "once_cell", "prost 0.14.4", "rand 0.8.6", + "rcgen", "reqwest", "secp256k1 0.30.0", "secrecy", "serde", "serde_json", "sqlx", + "subtle", "tokio", "toml", "tonic 0.14.6", @@ -3103,6 +3105,19 @@ version = "0.10.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "63b8176103e19a2643978565ca18b50549f6101881c443590420e4dc998a3c69" +[[package]] +name = "rcgen" +version = "0.13.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75e669e5202259b5314d1ea5397316ad400819437857b90861765f24c4cf80a2" +dependencies = [ + "pem", + "ring", + "rustls-pki-types", + "time", + "yasna", +] + [[package]] name = "redox_syscall" version = "0.5.18" @@ -4319,6 +4334,7 @@ dependencies = [ "socket2 0.6.4", "sync_wrapper", "tokio", + "tokio-rustls", "tokio-stream", "tower", "tower-layer", @@ -5066,6 +5082,15 @@ version = "0.6.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1ffae5123b2d3fc086436f8834ae3ab053a283cfac8fe0a0b8eaae044768a4c4" +[[package]] +name = "yasna" +version = "0.5.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e17bb3549cc1321ae1296b9cdc2698e2b6cb1992adfa19a8c72e5b7a738f44cd" +dependencies = [ + "time", +] + [[package]] name = "yoke" version = "0.8.3" diff --git a/Cargo.toml b/Cargo.toml index 3623faa7..3624836e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,16 +84,18 @@ dialoguer = "0.11" dirs = "6.0.0" dotenvy = "0.15.7" clearscreen = "4.0.1" -tonic = "0.14.2" +tonic = { version = "0.14.2", features = ["tls-ring"] } prost = "0.14.1" tonic-prost = "0.14.1" cdk = { version = "0.17.2", default-features = false, features = ["wallet"] } secp256k1 = { version = "0.30", features = ["serde"] } secrecy = { version = "0.10", features = ["serde"] } +subtle = "2.6" zeroize = "1.8" [dev-dependencies] tokio = { version = "1.47.1", features = ["full", "test-util", "macros"] } +rcgen = "0.13" axum = "0.8.4" tower-http = { version = "0.6.6", features = ["cors"] } bech32 = "0.11.0" diff --git a/README.md b/README.md index b4e4e110..fdb6fb00 100644 --- a/README.md +++ b/README.md @@ -653,8 +653,9 @@ bitcoin_price_api_url = "https://api.yadio.io" ```toml [rpc] enabled = false # Set to true to enable gRPC admin interface -listen_address = "127.0.0.1" +listen_address = "127.0.0.1" # IP literal, IPv6 bracketed; "localhost" is not resolved port = 50051 +allow_remote = false # Required to bind a non-loopback address ``` When enabled, exposes gRPC interface on `127.0.0.1:50051` for: @@ -662,6 +663,18 @@ When enabled, exposes gRPC interface on `127.0.0.1:50051` for: - Dispute settlement - Solver management +Enabling it **requires** a bearer token in the `MOSTRO_RPC_TOKEN` environment +variable (or `~/.mostro/.env`); the daemon refuses to start without one — or +with a hand-typed one: at least 32 characters and 16 distinct characters are +enforced — and refuses a non-loopback bind unless `allow_remote = true`. +Reaching this port is equivalent to holding the operator key — see the security +notes in [docs/RPC.md](docs/RPC.md). + +```bash +# ~/.mostro/.env +MOSTRO_RPC_TOKEN= +``` + --- #### Database @@ -773,19 +786,26 @@ For client documentation, see the respective client repositories. ### Admin Operations (RPC Interface) -If RPC is enabled, use admin tools for dispute resolution: +If RPC is enabled, use admin tools for dispute resolution. Every call must carry +the bearer token; `-plaintext` is only safe over loopback: ```bash +AUTH="authorization: Bearer $MOSTRO_RPC_TOKEN" + # Cancel an order (admin override) -grpcurl -plaintext -d '{"order_id": "abc123"}' localhost:50051 mostro.admin.v1.AdminService/CancelOrder +grpcurl -plaintext -H "$AUTH" -d '{"order_id": "abc123"}' localhost:50051 mostro.admin.v1.AdminService/CancelOrder # Settle disputed order -grpcurl -plaintext -d '{"order_id": "abc123"}' localhost:50051 mostro.admin.v1.AdminService/SettleOrder +grpcurl -plaintext -H "$AUTH" -d '{"order_id": "abc123"}' localhost:50051 mostro.admin.v1.AdminService/SettleOrder # Add dispute solver -grpcurl -plaintext -d '{"solver_pubkey": "npub1..."}' localhost:50051 mostro.admin.v1.AdminService/AddSolver +grpcurl -plaintext -H "$AUTH" -d '{"solver_pubkey": "npub1..."}' localhost:50051 mostro.admin.v1.AdminService/AddSolver ``` +The shell expands `$AUTH` into `grpcurl`'s arguments, so any local user can read +the token with `ps`. On a host you do not have to yourself, use the Rust client +in [docs/RPC.md](docs/RPC.md) instead, which keeps the token in the environment. + --- ### Querying Audit Events diff --git a/docs/ADMIN_RPC_AND_DISPUTES.md b/docs/ADMIN_RPC_AND_DISPUTES.md index 83e292bb..e68910d5 100644 --- a/docs/ADMIN_RPC_AND_DISPUTES.md +++ b/docs/ADMIN_RPC_AND_DISPUTES.md @@ -4,8 +4,9 @@ Admin capabilities and dispute resolution paths. ## RPC Server - Source: `src/rpc/server.rs` -- Enable: `settings.toml` → `[rpc] enabled = true` +- Enable: `settings.toml` → `[rpc] enabled = true`, plus a `MOSTRO_RPC_TOKEN` bearer token in the environment (startup is fatal without it). - Binds `listen_address:port`; injects Keys, `Arc>`, `Arc>`. +- Auth: `src/rpc/auth.rs` intercepts every method and rejects any request without the token. This is the *only* caller authorization on the RPC path — the handlers run as the daemon identity, which every downstream check treats as fully privileged. - Uses `tonic`; see `docs/RPC.md` and `proto/admin.proto`. ## Dispute Lifecycle @@ -41,7 +42,7 @@ sequenceDiagram ``` ## Audit and Safety -- Require admin authentication/authorization at message level. +- Nostr admin messages are authorized at message level; the gRPC path is authorized at the transport instead (bearer token), because its synthesized events always carry the daemon identity. - Enforce solver permission levels in the daemon: `read` solvers can assist but cannot execute `admin-settle` or `admin-cancel`. - Record solver, timestamps, and decisions in DB for traceability. - Avoid leaking sensitive data in logs; scrub invoices and keys. diff --git a/docs/RPC.md b/docs/RPC.md index d18ce530..2df49899 100644 --- a/docs/RPC.md +++ b/docs/RPC.md @@ -12,7 +12,7 @@ The RPC interface provides a direct communication method for admin operations, c ## Configuration -Add the following section to your `settings.toml` (keys are required; fields have Rust Default implementations but must be present): +Add the following section to your `settings.toml` (`enabled`, `listen_address` and `port` are required keys; fields have Rust Default implementations but must be present): ```toml [rpc] @@ -20,10 +20,40 @@ Add the following section to your `settings.toml` (keys are required; fields hav enabled = true # RPC server listen address (required key; default="127.0.0.1") listen_address = "127.0.0.1" -# RPC server port (required key; default=50051) +# RPC server port (required key; default=50051; 0 is refused at startup) port = 50051 +# Optional: acknowledge a non-loopback bind (default=false) +allow_remote = false +# Optional: serve TLS. Both paths are required together. +# tls_cert_path = "/etc/mostro/rpc-cert.pem" +# tls_key_path = "/etc/mostro/rpc-key.pem" ``` +`listen_address` must be an IP literal, with IPv6 bracketed: `127.0.0.1`, +`[::1]` or `0.0.0.0`. Hostnames are never resolved, so `localhost` and +unbracketed `::1` are refused at startup rather than accepted and then failed on +at bind time: `fn validate_rpc_settings` in `src/config/util.rs` and `fn bind` +for `RpcServer` in `src/rpc/server.rs` both resolve the address through +`fn listen_socket_addr` in `src/config/util.rs`. `port` must not be 0: an +ephemeral port changes on every restart, so no client could be configured +against it. + +The bearer token is **not** configured here. It is read from the `MOSTRO_RPC_TOKEN` +environment variable, which the daemon also picks up from `/.env`: + +```bash +# ~/.mostro/.env +MOSTRO_RPC_TOKEN= +``` + +`settings.toml` is the file operators paste into bug reports, so it never holds +the credential. The daemon **refuses to start** when `enabled = true` and the +variable is unset, shorter than 32 characters, or built from fewer than 16 +distinct characters. The last gate rejects hand-typed values such as a repeated +word: the decision not to rate-limit authentication assumes a randomly +generated token, so length alone is not enough. Anything produced by +`openssl rand -base64 32` passes all three. + ## Available Admin Operations The RPC interface supports the following admin operations: @@ -124,12 +154,40 @@ service AdminService { } ``` +## Authentication + +Every method, `GetVersion` included, requires an `authorization: Bearer ` +header carrying the value of `MOSTRO_RPC_TOKEN`. The scheme is matched +case-insensitively per RFC 7235; the token itself must match exactly. A missing, +malformed or incorrect token is answered with `UNAUTHENTICATED` before the +handler runs, and the token comparison itself is constant-time +(`fn credentials_match` in `src/rpc/auth.rs`), so it does not leak how many +bytes matched. Total request latency is not claimed to be constant: header +parsing, logging and transport all contribute to it. + +The token must be printable ASCII with no spaces, since it is sent verbatim as +an HTTP header. The daemon rejects anything else at startup. + +```bash +grpcurl -plaintext \ + -H "authorization: Bearer $MOSTRO_RPC_TOKEN" \ + -d '{"order_id": "550e8400-e29b-41d4-a716-446655440000"}' \ + localhost:50051 mostro.admin.v1.AdminService/CancelOrder +``` + +> **Shared hosts:** the shell expands `$MOSTRO_RPC_TOKEN` into `grpcurl`'s +> arguments, where any local user can read it with `ps`. On a host with other +> users, drive the API from the Rust client below instead, which keeps the token +> in the process environment. + ## Client Implementation Example Here's an example of how to create a gRPC client for the Mostro admin RPC: ```rust +use tonic::metadata::MetadataValue; use tonic::transport::Channel; +use tonic::Request; use mostro::rpc::admin::{admin_service_client::AdminServiceClient, CancelOrderRequest}; #[tokio::main] @@ -137,32 +195,78 @@ async fn main() -> Result<(), Box> { let channel = Channel::from_static("http://127.0.0.1:50051") .connect() .await?; - - let mut client = AdminServiceClient::new(channel); - + + let token: MetadataValue<_> = + format!("Bearer {}", std::env::var("MOSTRO_RPC_TOKEN")?).parse()?; + + let mut client = AdminServiceClient::with_interceptor(channel, move |mut req: Request<()>| { + req.metadata_mut().insert("authorization", token.clone()); + Ok(req) + }); + let request = tonic::Request::new(CancelOrderRequest { order_id: "550e8400-e29b-41d4-a716-446655440000".to_string(), request_id: Some("12345".to_string()), }); - + let response = client.cancel_order(request).await?; - + if response.get_ref().success { println!("Order cancelled successfully"); } else { println!("Failed to cancel order: {:?}", response.get_ref().error_message); } - + Ok(()) } ``` +The plaintext `http://` channel above is the loopback case. Against a server +configured with `tls_cert_path`/`tls_key_path` — which the Security +Considerations below require for any non-loopback exposure — the channel must +speak TLS, or `connect` fails with a transport error that never mentions TLS: + +```rust +use tonic::transport::{Certificate, Channel, ClientTlsConfig}; + +let tls = ClientTlsConfig::new() + .ca_certificate(Certificate::from_pem(std::fs::read("/etc/mostro/rpc-cert.pem")?)) + .domain_name("mostro.example.com"); +let channel = Channel::from_static("https://mostro.example.com:50051") + .tls_config(tls)? + .connect() + .await?; +``` + ## Security Considerations -- The RPC server listens on localhost by default for security -- Consider implementing authentication/authorization for production use -- The RPC interface provides the same admin capabilities as Nostr-based commands -- Only enable the RPC server in trusted environments +Treat reaching this port as equivalent to holding the Mostro operator key. + +Every RPC is executed under the daemon's own Nostr identity, and the daemon +identity is fully privileged downstream: `fn ensure_dispute_finalize_permission` +in `src/db.rs` waives its solver-category check for that key, and +`fn admin_add_solver_action` in `src/app/admin_add_solver.rs` accepts it +outright. The handlers apply no caller authorization of their own, so the bearer +token (`fn call` for `BearerAuth` in `src/rpc/auth.rs`) is the only thing between +the network and a settled dispute. + +- **Never expose this port beyond loopback without TLS.** The daemon refuses to + start on a non-loopback `listen_address` unless `allow_remote = true`, and + warns when such a bind runs without TLS. Over plaintext, anyone on the path + reads the bearer token and replays it. +- **The token is a credential, not a setting.** Keep it in `MOSTRO_RPC_TOKEN` + (environment or `/.env`, which the wizard writes with + owner-only permissions), rotate it by restarting with a new value, and never + commit it to `settings.toml`. +- **Container and appliance images publish ports easily.** Wrappers such as + Start9 or Umbrel map container ports to the host or LAN. Verify the mapping + before enabling the RPC; binding `0.0.0.0` inside a container whose port is + published hands the admin API to every device on the network. +- **A compromised token is a compromised node.** An attacker who holds it can + settle disputed orders to their own invoice or grant themselves permanent + solver rights over Nostr, which survives a token rotation. +- The RPC interface provides the same admin capabilities as Nostr-based + commands, without the Nostr-side key requirement. ## Debugging diff --git a/docs/RPC_RATE_LIMITING.md b/docs/RPC_RATE_LIMITING.md index d0519d28..cc39864e 100644 --- a/docs/RPC_RATE_LIMITING.md +++ b/docs/RPC_RATE_LIMITING.md @@ -68,8 +68,8 @@ How the original issue’s ideas map to the codebase today: | Per-IP rate limiting | **`check_rate_limit`** runs before the handler body. | | Exponential backoff / lockout | Implemented inside **`RateLimiter`**; **not** triggered by **`ValidateDbPassword`** (no **`record_failure`**). | | Audit logging | **tracing** in service + limiter. | -| Localhost-only | Default RPC bind **`127.0.0.1`** (see `settings.toml` / `docs/RPC.md`). | -| Strong auth | Out of scope for this stub; would need API keys or similar. | +| Localhost-only | Default RPC bind **`127.0.0.1`**, and a non-loopback bind now requires an explicit **`allow_remote = true`** (see `settings.toml` / `docs/RPC.md`). | +| Strong auth | Implemented by **`fn call`** for **`BearerAuth`** in **`src/rpc/auth.rs`**: a **`MOSTRO_RPC_TOKEN`** bearer token, compared in constant time (**`fn credentials_match`**, same file) on every method. The interceptor is deliberately **not** rate-limited — the token's entropy, not a counter, is what defeats guessing, and startup enforces that entropy: **`fn validate_rpc_settings`** in **`src/config/util.rs`** refuses tokens under 32 characters or with fewer than 16 distinct characters, so a hand-typed passphrase never reaches this trade-off. Because a rejected request never reaches the limiter, its *log line* is throttled instead (**`fn should_warn`**, same file): the first rejection from a peer within 60s is logged at `warn!` and the rest at `debug!`, so an unauthenticated flood cannot fill the disk. | ## Testing diff --git a/docs/STARTUP_AND_CONFIG.md b/docs/STARTUP_AND_CONFIG.md index 2efad905..902fffba 100644 --- a/docs/STARTUP_AND_CONFIG.md +++ b/docs/STARTUP_AND_CONFIG.md @@ -148,11 +148,14 @@ Configuration is loaded from `~/.mostro/settings.toml` (template: `settings.tpl. - `picture` (Option\): URL to avatar image, recommended square max 128x128px (default: None) - `website` (Option\): Operator website URL (default: None) -**RPC** (`src/config/types.rs:55-74`): +**RPC** (`RpcSettings` in `src/config/types.rs`): - `enabled` (bool): Enable RPC server (Rust Default: false) -- `listen_address` (String): Bind address (Rust Default: "127.0.0.1") +- `listen_address` (String): Bind address, as an IP literal with IPv6 bracketed - `127.0.0.1`, `[::1]`, `0.0.0.0`. Hostnames such as `localhost` are not resolved (Rust Default: "127.0.0.1") - `port` (u16): Listen port (Rust Default: 50051) -- Note: These fields have a Rust Default implementation, but `settings.toml` must still include these keys. If a key is present but empty or omitted by tooling, the daemon falls back to the Rust Default value. +- `allow_remote` (bool): Acknowledge a non-loopback bind (Rust Default: false) +- `tls_cert_path` / `tls_key_path` (Option\): PEM material for TLS; required together (Rust Default: None) +- Note: `enabled`, `listen_address` and `port` are required in `settings.toml`. `RpcSettings` has a Rust `Default` implementation, but neither those fields nor `rpc` on `Settings` carry `#[serde(default)]`, so an omitted key makes `toml::from_str` (`fn init_configuration_file` in `src/config/util.rs`) fail with a missing-field error rather than fall back, and an empty value is preserved as-is only for the string-typed `listen_address`; an empty value for `enabled` or `port` fails deserialization against their `bool` and `u16` types. The remaining fields are optional. +- The bearer token lives in the `MOSTRO_RPC_TOKEN` environment variable, never in `settings.toml`. `validate_rpc_settings` (`src/config/util.rs`) makes startup fatal when `enabled = true` and the token is missing, under 32 characters, or built from fewer than 16 distinct characters, when `listen_address` is not an address the server can bind, when `port` is 0, when a non-loopback address is bound without `allow_remote = true`, or when only one half of the TLS pair is configured. See `docs/RPC.md`. ## Global Variables diff --git a/settings.tpl.toml b/settings.tpl.toml index 8ad5fc4e..ce9fdbbf 100644 --- a/settings.tpl.toml +++ b/settings.tpl.toml @@ -126,14 +126,31 @@ fee_audit_days = 365 dm_days = 30 [rpc] -# Enable RPC server for direct admin communication +# Enable RPC server for direct admin communication. +# +# The admin RPC settles disputes, cancels orders and grants solver rights, and +# every call runs with the daemon's own privileges. Enabling it REQUIRES the +# MOSTRO_RPC_TOKEN environment variable (set it in the environment or in +# /.env, never here); the daemon refuses to start otherwise. +# Generate one with: openssl rand -base64 32 enabled = false -# RPC server listen address +# RPC server listen address. Must be an IP literal, with IPv6 bracketed: +# "127.0.0.1", "[::1]" or "0.0.0.0". Hostnames such as "localhost" are not +# resolved and the daemon refuses to start on one. listen_address = "127.0.0.1" # RPC server port port = 50051 # Duration in seconds after which inactive rate-limiter entries are evicted # rate_limiter_stale_duration = 3600 +# Acknowledge binding to a non-loopback address. The daemon refuses to start on +# a routable address unless this is true, so the admin API is never published to +# a LAN by accident. +# allow_remote = false +# Serve the admin RPC over TLS. Both paths are required together; without them +# the bearer token crosses the network in cleartext, so configure them (or a +# TLS-terminating reverse proxy) whenever allow_remote is true. +# tls_cert_path = "/etc/mostro/rpc-cert.pem" +# tls_key_path = "/etc/mostro/rpc-key.pem" # Multi-source price providers (see docs/PRICE_PROVIDERS.md). # Absent section ≡ legacy single-source behaviour synthesised from diff --git a/src/config/constants.rs b/src/config/constants.rs index ce5d3b5f..1e0b9862 100644 --- a/src/config/constants.rs +++ b/src/config/constants.rs @@ -30,3 +30,22 @@ pub const ENV_FILENAME: &str = ".env"; /// Environment variable name used to override the Nostr private key from the /// process environment. Shared between the wizard and the loader. pub const NSEC_ENV_VAR: &str = "MOSTRO_NSEC_PRIVKEY"; + +/// Environment variable holding the shared bearer token that authenticates +/// admin gRPC callers. Deliberately env-only (like `NSEC_ENV_VAR`): the token +/// never lives in `settings.toml`, only in the process environment or +/// `/.env`. +pub const RPC_TOKEN_ENV_VAR: &str = "MOSTRO_RPC_TOKEN"; + +/// Minimum accepted length for `MOSTRO_RPC_TOKEN`. 32 characters is the +/// shortest base64 encoding of 24 random bytes, well past the point where +/// online guessing against a single daemon is meaningful. +pub const MIN_RPC_TOKEN_LEN: usize = 32; + +/// Minimum number of distinct characters in `MOSTRO_RPC_TOKEN`. The length +/// floor alone is satisfied by `"a"` repeated 32 times, and the decision not +/// to rate-limit authentication (`crate::rpc::auth`) leans on the token being +/// randomly generated, not merely long. Random tokens clear this easily — +/// `openssl rand -base64 32` yields ~30 distinct characters on average — +/// while hand-typed passphrases do not. +pub const MIN_RPC_TOKEN_DISTINCT_CHARS: usize = 16; diff --git a/src/config/secret.rs b/src/config/secret.rs index c54e4537..da7c47bc 100644 --- a/src/config/secret.rs +++ b/src/config/secret.rs @@ -1,7 +1,7 @@ //! Helpers for loading and parsing the Mostro Nostr private key with //! zeroization of transient buffers. -use crate::config::constants::NSEC_ENV_VAR; +use crate::config::constants::{NSEC_ENV_VAR, RPC_TOKEN_ENV_VAR}; use crate::config::types::NostrSettings; use mostro_core::error::MostroError::{self, *}; use mostro_core::error::ServiceError; @@ -18,20 +18,36 @@ where serializer.serialize_str(secret.expose_secret()) } -/// Read `MOSTRO_NSEC_PRIVKEY` from the process environment, trim whitespace, -/// and wrap in a [`SecretString`]. Returns `None` when unset or blank. -pub fn read_nsec_env_var() -> Option { - let mut nsec_from_env = std::env::var(NSEC_ENV_VAR).ok()?; - let trimmed = nsec_from_env.trim(); +/// Read `name` from the process environment, trim whitespace, and wrap in a +/// [`SecretString`], zeroizing the transient buffer. Returns `None` when the +/// variable is unset or blank. +fn read_secret_env_var(name: &str) -> Option { + let mut value_from_env = std::env::var(name).ok()?; + let trimmed = value_from_env.trim(); if trimmed.is_empty() { - nsec_from_env.zeroize(); + value_from_env.zeroize(); return None; } let secret = SecretString::from(trimmed.to_owned()); - nsec_from_env.zeroize(); + value_from_env.zeroize(); Some(secret) } +/// Read `MOSTRO_NSEC_PRIVKEY` from the process environment, trim whitespace, +/// and wrap in a [`SecretString`]. Returns `None` when unset or blank. +pub fn read_nsec_env_var() -> Option { + read_secret_env_var(NSEC_ENV_VAR) +} + +/// Read `MOSTRO_RPC_TOKEN` from the process environment, trim whitespace, and +/// wrap in a [`SecretString`]. Returns `None` when unset or blank. +/// +/// The admin RPC bearer token is env-only by design: `settings.toml` is the +/// file operators paste into issues when asking for help. +pub fn read_rpc_token_env_var() -> Option { + read_secret_env_var(RPC_TOKEN_ENV_VAR) +} + /// Parse a bech32 nsec into [`Keys`], exposing the secret only in this scope. pub fn parse_mostro_keys(secret: &SecretString) -> Result { let nsec = secret.expose_secret(); diff --git a/src/config/types.rs b/src/config/types.rs index 3a9e56f2..c267237b 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -468,6 +468,18 @@ pub struct RpcSettings { /// Duration in seconds after which inactive rate-limiter entries are evicted #[serde(default = "default_rate_limiter_stale_duration")] pub rate_limiter_stale_duration: u64, + /// Acknowledge binding the admin RPC to a non-loopback address. The daemon + /// refuses to start on a routable address unless this is set, so an + /// operator cannot publish the admin surface to a LAN by accident. + #[serde(default)] + pub allow_remote: bool, + /// Path to the PEM-encoded TLS certificate chain served by the admin RPC. + /// Must be set together with `tls_key_path`; absent means plaintext. + #[serde(default)] + pub tls_cert_path: Option, + /// Path to the PEM-encoded TLS private key matching `tls_cert_path`. + #[serde(default)] + pub tls_key_path: Option, } fn default_rate_limiter_stale_duration() -> u64 { @@ -481,6 +493,22 @@ impl Default for RpcSettings { listen_address: "127.0.0.1".to_string(), port: 50051, rate_limiter_stale_duration: default_rate_limiter_stale_duration(), + allow_remote: false, + tls_cert_path: None, + tls_key_path: None, + } + } +} + +impl RpcSettings { + /// TLS certificate/key pair when both are configured. + /// + /// `validate_rpc_settings` rejects a half-configured pair at startup, so a + /// `None` here means plaintext was chosen, never that one path was lost. + pub fn tls_paths(&self) -> Option<(&str, &str)> { + match (self.tls_cert_path.as_deref(), self.tls_key_path.as_deref()) { + (Some(cert), Some(key)) => Some((cert, key)), + _ => None, } } } diff --git a/src/config/util.rs b/src/config/util.rs index 635a5ce3..f601faac 100644 --- a/src/config/util.rs +++ b/src/config/util.rs @@ -2,12 +2,17 @@ /// This module provides utility functions for the config module. /// It includes functions to initialize the default settings directory and create a settings file from the template if it doesn't exist. /// It also includes functions to add a trailing slash to a path if it doesn't already have one. -use crate::config::constants::{ENV_FILENAME, MAX_DEV_FEE_PERCENTAGE, MIN_DEV_FEE_PERCENTAGE}; -use crate::config::secret::read_nsec_env_var; +use crate::config::constants::{ + ENV_FILENAME, MAX_DEV_FEE_PERCENTAGE, MIN_DEV_FEE_PERCENTAGE, MIN_RPC_TOKEN_DISTINCT_CHARS, + MIN_RPC_TOKEN_LEN, RPC_TOKEN_ENV_VAR, +}; +use crate::config::secret::{read_nsec_env_var, read_rpc_token_env_var}; +use crate::config::types::RpcSettings; use crate::config::wizard; use crate::config::{init_mostro_settings, Settings}; use mostro_core::error::MostroError::{self, *}; use mostro_core::error::ServiceError; +use secrecy::{ExposeSecret, SecretString}; use std::fs; use std::io::IsTerminal; use std::path::PathBuf; @@ -15,6 +20,42 @@ use zeroize::Zeroizing; const DB_FILENAME: &str = "mostro.db"; +/// Serializes every test that mutates or reads the process environment. One +/// lock for the whole environment rather than one per variable: glibc's +/// `setenv` can reallocate `environ` with no synchronization against a +/// concurrent `getenv`, so two tests holding two *different* locks still race. +/// Async-aware because `rpc::server::tests` holds it across `.await`; sync +/// tests take it through `blocking_lock`. +#[cfg(test)] +pub(crate) static ENV_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + +/// Resolve `[rpc].listen_address` and `[rpc].port` into the address the server +/// binds. +/// +/// `SocketAddr` only parses IP literals, with IPv6 bracketed. Hostnames such as +/// `localhost` and bare `::1` are therefore not bindable addresses, however +/// natural they look in a config file. +/// +/// `RpcServer::bind` resolves its address through this function too, so a +/// `listen_address` that passes validation is guaranteed to be one `bind` can +/// use: the two must never disagree, or the daemon accepts a config at startup +/// and then dies on it. It lives here rather than in `rpc::server` so the +/// dependency keeps pointing the usual way, `rpc` → `config`. +pub(crate) fn listen_socket_addr( + listen_address: &str, + port: u16, +) -> Result { + format!("{listen_address}:{port}") + .parse::() + .map_err(|e| { + format!( + "Invalid address {listen_address:?}: {e}. Expected an IP literal, with IPv6 \ + bracketed — for example 127.0.0.1, [::1] or 0.0.0.0. Hostnames such as \ + \"localhost\" are not resolved." + ) + }) +} + /// Loads the optional `/.env` file so that values placed there /// become available through `std::env::var`. Variables already set in the /// process environment take precedence and are never overwritten. @@ -73,6 +114,177 @@ fn validate_mostro_settings(settings: &Settings) -> Result<(), MostroError> { .is_some_and(|bond| bond.enabled), )?; + validate_rpc_settings(&settings.rpc, read_rpc_token_env_var().as_ref())?; + + Ok(()) +} + +/// True when `addr` can only be reached from the host itself. +/// +/// Parses through `listen_socket_addr`, the same function `RpcServer::bind` +/// uses, so this can never call an address loopback that the server would then +/// refuse to bind. Callers must reject unparseable addresses +/// first — `false` here means "not loopback", not "not an address". +fn is_loopback_address(addr: &str) -> bool { + listen_socket_addr(addr, 0).is_ok_and(|socket| socket.ip().is_loopback()) +} + +/// Validate the `[rpc]` block (finding 1.5, issue #807). +/// +/// The admin gRPC surface settles disputes, moves escrowed funds, and grants +/// permanent solver rights. Worse, every RPC is executed under the daemon's own +/// identity, which downstream authorization treats as fully privileged +/// (`db::ensure_dispute_finalize_permission`). Reaching the port therefore *is* +/// the authorization, so both guards below are startup-fatal rather than +/// warnings: +/// +/// - `enabled = true` requires `MOSTRO_RPC_TOKEN`, so the interceptor always +/// has a credential to check. A daemon that boots without one would serve an +/// admin API that nothing gates. +/// - A non-loopback `listen_address` requires an explicit `allow_remote = true`. +/// The defaults are safe, but nothing used to stop `0.0.0.0` from publishing +/// the admin API to the LAN silently. +/// - `listen_address` must be an address `RpcServer::bind` can actually bind. +/// Validation and binding share `listen_socket_addr` so the two cannot +/// drift: a config accepted here is one the server will accept. +/// +/// A half-configured TLS pair is also fatal: it reads as "TLS is on" while +/// serving plaintext. +fn validate_rpc_settings( + rpc: &RpcSettings, + token: Option<&SecretString>, +) -> Result<(), MostroError> { + if !rpc.enabled { + return Ok(()); + } + + match token { + None => { + return Err(MostroInternalErr(ServiceError::IOError(format!( + "[rpc].enabled = true but {RPC_TOKEN_ENV_VAR} is not set: the admin RPC would \ + accept every caller that can reach the port. Set {RPC_TOKEN_ENV_VAR} in the \ + environment or /.env, or set [rpc].enabled = false." + )))); + } + Some(token) if token.expose_secret().chars().count() < MIN_RPC_TOKEN_LEN => { + return Err(MostroInternalErr(ServiceError::IOError(format!( + "{RPC_TOKEN_ENV_VAR} is shorter than {MIN_RPC_TOKEN_LEN} characters: generate a \ + high-entropy token, e.g. `openssl rand -base64 32`." + )))); + } + // The token travels verbatim inside an HTTP/2 `authorization` header. + // Anything outside printable ASCII cannot be carried there, so a daemon + // that accepted it would boot and then refuse every client — a failure + // that looks like a broken build rather than a typo in the token. + Some(token) if !token.expose_secret().chars().all(|c| c.is_ascii_graphic()) => { + return Err(MostroInternalErr(ServiceError::IOError(format!( + "{RPC_TOKEN_ENV_VAR} must contain only printable ASCII characters and no spaces: \ + it is sent as an HTTP header, so any other value can never authenticate a \ + client. `openssl rand -base64 32` produces a valid token." + )))); + } + // The decision not to rate-limit authentication (`rpc::auth`) leans on + // the token being randomly generated, and length alone does not make + // it so: `"a".repeat(32)` clears the length gate with almost no + // entropy. A floor on distinct characters rejects hand-typed values + // while passing every randomly generated token. + Some(token) + if token + .expose_secret() + .chars() + .collect::>() + .len() + < MIN_RPC_TOKEN_DISTINCT_CHARS => + { + return Err(MostroInternalErr(ServiceError::IOError(format!( + "{RPC_TOKEN_ENV_VAR} has fewer than {MIN_RPC_TOKEN_DISTINCT_CHARS} distinct \ + characters: it looks hand-typed rather than randomly generated, and the admin \ + RPC relies on token entropy instead of a guess counter. Generate it, e.g. \ + `openssl rand -base64 32`." + )))); + } + Some(_) => {} + } + + // Before the loopback check, or an unbindable address would be reported as + // a remote-exposure problem: `localhost` and bare `::1` read as loopback to + // an operator, so "set allow_remote = true" would be actively misleading + // advice for a daemon that is about to die on `Invalid address` instead. + listen_socket_addr(&rpc.listen_address, rpc.port).map_err(|e| { + MostroInternalErr(ServiceError::IOError(format!("[rpc].listen_address: {e}"))) + })?; + + if rpc.port == 0 { + return Err(MostroInternalErr(ServiceError::IOError( + "[rpc].port = 0 asks the kernel for an ephemeral port, which changes on every \ + restart and no client can be configured against. Set a fixed port." + .to_string(), + ))); + } + + if !is_loopback_address(&rpc.listen_address) && !rpc.allow_remote { + return Err(MostroInternalErr(ServiceError::IOError(format!( + "[rpc].listen_address ({:?}) is not a loopback address: this publishes the admin API \ + beyond this host. Set [rpc].allow_remote = true to confirm this is intended, or bind \ + 127.0.0.1.", + rpc.listen_address + )))); + } + + match (rpc.tls_cert_path.as_deref(), rpc.tls_key_path.as_deref()) { + (Some(_), None) => { + return Err(MostroInternalErr(ServiceError::IOError( + "[rpc].tls_cert_path is set without [rpc].tls_key_path: TLS needs both, and the \ + server would otherwise fall back to plaintext." + .to_string(), + ))); + } + (None, Some(_)) => { + return Err(MostroInternalErr(ServiceError::IOError( + "[rpc].tls_key_path is set without [rpc].tls_cert_path: TLS needs both, and the \ + server would otherwise fall back to plaintext." + .to_string(), + ))); + } + (Some(cert), Some(key)) => { + for (field, path) in [("tls_cert_path", cert), ("tls_key_path", key)] { + // Open rather than stat: `fs::metadata` succeeds for a file the + // daemon has no permission to read. Opening is still not + // enough on its own — on Linux a directory opens fine and only + // fails on read — so the file type is checked through the + // handle, which is the capability `RpcServer::start` needs. + let opened = fs::File::open(path).map_err(|e| { + MostroInternalErr(ServiceError::IOError(format!( + "[rpc].{field} ({path:?}) is not readable: {e}" + ))) + })?; + let is_regular_file = opened + .metadata() + .map(|metadata| metadata.is_file()) + .map_err(|e| { + MostroInternalErr(ServiceError::IOError(format!( + "[rpc].{field} ({path:?}) could not be inspected: {e}" + ))) + })?; + if !is_regular_file { + return Err(MostroInternalErr(ServiceError::IOError(format!( + "[rpc].{field} ({path:?}) is not a regular file" + )))); + } + } + } + (None, None) => { + if !is_loopback_address(&rpc.listen_address) { + tracing::warn!( + "[rpc] is bound to {} without TLS: admin bearer tokens and dispute data cross \ + the network in cleartext. Set [rpc].tls_cert_path and [rpc].tls_key_path, or \ + terminate TLS in a reverse proxy.", + rpc.listen_address + ); + } + } + } + Ok(()) } @@ -216,11 +428,6 @@ mod tests { DatabaseSettings, LightningSettings, MostroSettings, NostrSettings, RpcSettings, }; use secrecy::{ExposeSecret, SecretString}; - use std::sync::Mutex; - - // Tests that read/write MOSTRO_NSEC_PRIVKEY must run serially because the - // process environment is shared across threads. - static ENV_LOCK: Mutex<()> = Mutex::new(()); /// RAII guard that saves the current value of an env var and restores it /// on drop, so tests don't leak state into each other. @@ -269,7 +476,7 @@ mod tests { #[test] fn env_var_overrides_toml_nsec() { - let _lock = ENV_LOCK.lock().unwrap(); + let _lock = ENV_LOCK.blocking_lock(); let guard = EnvVarGuard::new(NSEC_ENV_VAR); guard.set("nsec_from_env"); @@ -281,7 +488,7 @@ mod tests { #[test] fn empty_env_var_falls_back_to_toml() { - let _lock = ENV_LOCK.lock().unwrap(); + let _lock = ENV_LOCK.blocking_lock(); let guard = EnvVarGuard::new(NSEC_ENV_VAR); guard.set(""); @@ -296,7 +503,7 @@ mod tests { #[test] fn no_env_var_keeps_toml() { - let _lock = ENV_LOCK.lock().unwrap(); + let _lock = ENV_LOCK.blocking_lock(); let _guard = EnvVarGuard::new(NSEC_ENV_VAR); let mut settings = make_settings("nsec_from_toml"); @@ -310,7 +517,7 @@ mod tests { #[test] fn whitespace_only_env_is_ignored() { - let _lock = ENV_LOCK.lock().unwrap(); + let _lock = ENV_LOCK.blocking_lock(); let guard = EnvVarGuard::new(NSEC_ENV_VAR); guard.set(" \t "); @@ -328,7 +535,7 @@ mod tests { // When the env var already held a value, the guard must restore that // exact value on drop (the `Some(previous)` restore arm), not leave // the test's override leaking into sibling tests. - let _lock = ENV_LOCK.lock().unwrap(); + let _lock = ENV_LOCK.blocking_lock(); std::env::set_var(NSEC_ENV_VAR, "preexisting_value"); { let guard = EnvVarGuard::new(NSEC_ENV_VAR); @@ -348,7 +555,7 @@ mod tests { #[test] fn env_var_value_is_trimmed() { - let _lock = ENV_LOCK.lock().unwrap(); + let _lock = ENV_LOCK.blocking_lock(); let guard = EnvVarGuard::new(NSEC_ENV_VAR); guard.set(" nsec_from_env "); @@ -457,11 +664,16 @@ mod startup_validation_tests { #[test] fn default_settings_pass_validation() { + // `validate_mostro_settings` reads MOSTRO_RPC_TOKEN from the + // environment, so these tests hold the crate-wide lock like every + // other reader: a concurrent `setenv` elsewhere is a data race. + let _lock = ENV_LOCK.blocking_lock(); assert!(validate_mostro_settings(&base_settings()).is_ok()); } #[test] fn dev_fee_below_minimum_is_rejected() { + let _lock = ENV_LOCK.blocking_lock(); let mut settings = base_settings(); settings.mostro.dev_fee_percentage = MIN_DEV_FEE_PERCENTAGE - 0.01; let err = validate_mostro_settings(&settings).expect_err("below-min dev fee must fail"); @@ -470,6 +682,7 @@ mod startup_validation_tests { #[test] fn dev_fee_above_maximum_is_rejected() { + let _lock = ENV_LOCK.blocking_lock(); let mut settings = base_settings(); settings.mostro.dev_fee_percentage = MAX_DEV_FEE_PERCENTAGE + 0.01; let err = validate_mostro_settings(&settings).expect_err("above-max dev fee must fail"); @@ -478,6 +691,7 @@ mod startup_validation_tests { #[test] fn cashu_and_bond_conflict_is_rejected_through_full_validation() { + let _lock = ENV_LOCK.blocking_lock(); let mut settings = base_settings(); settings.anti_abuse_bond = Some(AntiAbuseBondSettings { enabled: true, @@ -492,6 +706,268 @@ mod startup_validation_tests { } } +#[cfg(test)] +mod rpc_validation_tests { + use super::*; + use crate::config::types::RpcSettings; + + fn valid_token() -> SecretString { + // 32 distinct characters: clears both the length and the entropy + // floors the validator enforces. + SecretString::from("0123456789abcdefghijklmnopqrstuv") + } + + fn enabled_rpc() -> RpcSettings { + RpcSettings { + enabled: true, + ..Default::default() + } + } + + fn temp_pem(tag: &str) -> String { + let dir = std::env::temp_dir().join(format!("mostro-rpc-tls-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("create temp dir"); + let path = dir.join(format!("{tag}.pem")); + std::fs::write(&path, b"not a real certificate").expect("write pem"); + path.to_string_lossy().into_owned() + } + + #[test] + fn disabled_rpc_needs_no_token() { + // The whole block is inert when the server never starts, including a + // deliberately unsafe bind. + let rpc = RpcSettings { + listen_address: "0.0.0.0".to_string(), + ..Default::default() + }; + assert!(validate_rpc_settings(&rpc, None).is_ok()); + } + + #[test] + fn enabled_rpc_without_a_token_is_rejected() { + let err = validate_rpc_settings(&enabled_rpc(), None) + .expect_err("an ungated admin RPC must not boot"); + assert!(err.to_string().contains(RPC_TOKEN_ENV_VAR)); + } + + #[test] + fn enabled_rpc_with_a_short_token_is_rejected() { + let short = SecretString::from("a".repeat(MIN_RPC_TOKEN_LEN - 1)); + let err = validate_rpc_settings(&enabled_rpc(), Some(&short)) + .expect_err("a guessable token must not boot"); + assert!(err.to_string().contains("shorter than")); + } + + #[test] + fn enabled_rpc_on_loopback_with_a_token_is_accepted() { + assert!(validate_rpc_settings(&enabled_rpc(), Some(&valid_token())).is_ok()); + } + + #[test] + fn a_token_that_cannot_travel_in_a_header_is_rejected() { + // Long enough to clear the length gate, but unusable as an HTTP header + // value: accepting it would boot a daemon that refuses every client. + for unusable in ["é".repeat(MIN_RPC_TOKEN_LEN), "a".repeat(31) + " b"] { + let token = SecretString::from(unusable.clone()); + let err = validate_rpc_settings(&enabled_rpc(), Some(&token)) + .expect_err("a token that cannot be sent must not boot"); + assert!( + err.to_string().contains("printable ASCII"), + "{unusable:?} should have been refused as unsendable, got: {err}" + ); + } + } + + #[test] + fn a_low_entropy_token_is_rejected() { + // Long enough and ASCII, but visibly hand-typed: the length gate + // alone would accept both and quietly void the no-rate-limit + // argument in `rpc::auth`. + for guessable in ["a".repeat(MIN_RPC_TOKEN_LEN), "abcdefg1".repeat(4)] { + let token = SecretString::from(guessable.clone()); + let err = validate_rpc_settings(&enabled_rpc(), Some(&token)) + .expect_err("a low-entropy token must not boot"); + assert!( + err.to_string().contains("distinct"), + "{guessable:?} should have been refused as low-entropy, got: {err}" + ); + } + } + + #[test] + fn port_zero_is_rejected() { + let rpc = RpcSettings { + port: 0, + ..enabled_rpc() + }; + let err = validate_rpc_settings(&rpc, Some(&valid_token())) + .expect_err("an ephemeral admin port must not boot"); + assert!(err.to_string().contains("ephemeral")); + } + + #[test] + fn listen_socket_addr_accepts_only_bindable_literals() { + for address in ["127.0.0.1", "[::1]", "0.0.0.0", "[::]"] { + assert!( + listen_socket_addr(address, 50051).is_ok(), + "{address} is a bindable literal" + ); + } + for address in ["localhost", "::1", "::", "mostro.example.com", ""] { + assert!( + listen_socket_addr(address, 50051).is_err(), + "{address} is not a bindable literal" + ); + } + } + + #[test] + fn a_directory_is_not_accepted_as_tls_material() { + // Both `fs::metadata` and `File::open` succeed on a directory, so only + // the file-type check rejects this. + let dir = std::env::temp_dir().join(format!("mostro-rpc-tls-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("create temp dir"); + let rpc = RpcSettings { + tls_cert_path: Some(dir.to_string_lossy().into_owned()), + tls_key_path: Some(temp_pem("dir-case-key")), + ..enabled_rpc() + }; + let err = validate_rpc_settings(&rpc, Some(&valid_token())) + .expect_err("a directory is not a certificate"); + assert!(err.to_string().contains("not a regular file")); + } + + #[test] + fn loopback_is_recognised_in_every_bindable_form() { + for address in ["127.0.0.1", "127.0.0.53", "[::1]"] { + let rpc = RpcSettings { + enabled: true, + listen_address: address.to_string(), + ..Default::default() + }; + assert!( + validate_rpc_settings(&rpc, Some(&valid_token())).is_ok(), + "{address} should be treated as loopback" + ); + } + } + + /// The contract this pins: validation and `RpcServer::bind` share one + /// parser, so anything the server cannot bind is refused here with an + /// actionable message instead of at startup with `Invalid address`. + /// + /// `localhost` and bare `::1` are the cases that matter — they look like + /// valid loopback spellings, and reporting them through the `allow_remote` + /// branch would send the operator to fix the wrong setting. + #[test] + fn an_unbindable_listen_address_is_rejected() { + for address in ["localhost", "LOCALHOST", "::1", "::", "mostro.example.com"] { + let rpc = RpcSettings { + enabled: true, + listen_address: address.to_string(), + // Set so the failure cannot be attributed to the remote-bind + // guard: only the parse check can refuse these. + allow_remote: true, + ..Default::default() + }; + let err = validate_rpc_settings(&rpc, Some(&valid_token())) + .expect_err("an address the server cannot bind must not boot"); + let message = err.to_string(); + assert!( + message.contains("IP literal") && message.contains("[::1]"), + "{address} should name the accepted spellings, got: {message}" + ); + } + } + + #[test] + fn non_loopback_bind_without_allow_remote_is_rejected() { + for address in ["0.0.0.0", "192.168.1.10", "[::]"] { + let rpc = RpcSettings { + enabled: true, + listen_address: address.to_string(), + ..Default::default() + }; + let err = validate_rpc_settings(&rpc, Some(&valid_token())) + .expect_err("a routable bind must require allow_remote"); + assert!( + err.to_string().contains("allow_remote"), + "{address} should have been refused, got: {err}" + ); + } + } + + #[test] + fn non_loopback_bind_with_allow_remote_is_accepted() { + let rpc = RpcSettings { + enabled: true, + listen_address: "0.0.0.0".to_string(), + allow_remote: true, + ..Default::default() + }; + assert!(validate_rpc_settings(&rpc, Some(&valid_token())).is_ok()); + } + + #[test] + fn half_configured_tls_is_rejected() { + let cert_only = RpcSettings { + tls_cert_path: Some(temp_pem("cert-only")), + ..enabled_rpc() + }; + assert!(validate_rpc_settings(&cert_only, Some(&valid_token())) + .expect_err("cert without key must fail") + .to_string() + .contains("tls_key_path")); + + let key_only = RpcSettings { + tls_key_path: Some(temp_pem("key-only")), + ..enabled_rpc() + }; + assert!(validate_rpc_settings(&key_only, Some(&valid_token())) + .expect_err("key without cert must fail") + .to_string() + .contains("tls_cert_path")); + } + + #[test] + fn unreadable_tls_material_is_rejected() { + let rpc = RpcSettings { + tls_cert_path: Some("/nonexistent/mostro-rpc.pem".to_string()), + tls_key_path: Some(temp_pem("readable-key")), + ..enabled_rpc() + }; + let err = validate_rpc_settings(&rpc, Some(&valid_token())) + .expect_err("unreadable TLS material must fail"); + assert!(err.to_string().contains("not readable")); + } + + #[test] + fn readable_tls_pair_is_accepted() { + let rpc = RpcSettings { + tls_cert_path: Some(temp_pem("pair-cert")), + tls_key_path: Some(temp_pem("pair-key")), + ..enabled_rpc() + }; + assert!(validate_rpc_settings(&rpc, Some(&valid_token())).is_ok()); + } + + #[test] + fn tls_paths_helper_requires_both_halves() { + let rpc = RpcSettings { + tls_cert_path: Some("cert.pem".to_string()), + ..Default::default() + }; + assert!(rpc.tls_paths().is_none()); + + let rpc = RpcSettings { + tls_cert_path: Some("cert.pem".to_string()), + tls_key_path: Some("key.pem".to_string()), + ..Default::default() + }; + assert_eq!(rpc.tls_paths(), Some(("cert.pem", "key.pem"))); + } +} + #[cfg(test)] mod env_file_tests { use super::*; @@ -513,8 +989,10 @@ mod env_file_tests { #[test] fn env_file_values_become_process_env() { + // dotenvy mutates the environment, so the crate-wide lock applies + // even though the variable name is unique to this test. + let _lock = ENV_LOCK.blocking_lock(); let dir = temp_dir("with-env"); - // A variable name no other test uses, so parallel runs can't race. std::fs::write( dir.join(ENV_FILENAME), "MOSTRO_TEST_ENV_FILE_MARKER=loaded\n", diff --git a/src/main.rs b/src/main.rs index 951ff5e2..0c7577e3 100644 --- a/src/main.rs +++ b/src/main.rs @@ -273,10 +273,36 @@ async fn main() -> Result<()> { let rpc_pool = get_db_pool(); let rpc_ln_client = Arc::new(tokio::sync::Mutex::new(ln_client.clone())); + // `[rpc].enabled = true` is an explicit request for the admin API, and + // every other misconfiguration in that block is startup-fatal (see + // `config::util::validate_rpc_settings`). `bind` resolves the listener + // and the TLS material here, before any of the startup below runs, so + // the daemon never advances past this point believing an admin + // interface is up that nothing is serving. + // `bind` logs the address it actually acquired, so it is not repeated + // here. + let (_bound, rpc_serving) = match rpc_server.bind(rpc_keys, rpc_pool, rpc_ln_client) { + Ok(bound) => bound, + Err(e) => { + tracing::error!("RPC server failed to start: {}", e); + exit(1); + } + }; + + // Only the accept loop is detached; it is already listening. tokio::spawn(async move { - match rpc_server.start(rpc_keys, rpc_pool, rpc_ln_client).await { - Ok(_) => tracing::info!("RPC server started successfully"), - Err(e) => tracing::error!("RPC server failed to start: {}", e), + match rpc_serving.await { + // The listener stream never ends, so this arm is unreachable + // today; treat it like any other loss of the admin API rather + // than letting `enabled = true` silently become false. + Ok(()) => { + tracing::error!("RPC server stopped accepting connections"); + exit(1); + } + Err(e) => { + tracing::error!("RPC server error: {}", e); + exit(1); + } } }); } diff --git a/src/rpc/auth.rs b/src/rpc/auth.rs new file mode 100644 index 00000000..821db1f2 --- /dev/null +++ b/src/rpc/auth.rs @@ -0,0 +1,336 @@ +//! Bearer-token authentication for the admin gRPC surface. +//! +//! Every admin RPC is executed under the daemon's own Nostr identity (see +//! `crate::rpc::service`), and downstream authorization grants that identity +//! full privilege — `db::ensure_dispute_finalize_permission` short-circuits its +//! solver-category check for the daemon key. There is therefore no +//! message-level authorization left to fall back on: reaching the port is the +//! authorization, so the transport has to be the gate. +//! +//! The authentication *decision* is deliberately not rate-limited. +//! `MIN_RPC_TOKEN_LEN` and `MIN_RPC_TOKEN_DISTINCT_CHARS` — enforced at +//! startup by `config::util::validate_rpc_settings` — keep the search space +//! of accepted tokens far out of reach of online guessing, and tonic's +//! [`Interceptor`] is synchronous while +//! [`crate::rpc::rate_limiter::RateLimiter`] is async, so wiring one in would +//! mean a second, parallel limiter for no security gain. +//! +//! The *logging* is throttled, which is a different problem. The rate limiter +//! runs inside the handlers (`check_rate_limit` in [`crate::rpc::service`]), so +//! a request rejected here never reaches it. One `warn!` per rejected request +//! is harmless on loopback, but under `[rpc].allow_remote = true` it hands any +//! host that can reach the port a free log line per request — unbounded disk +//! growth, and enough noise to bury the genuine audit lines. So the first +//! rejection from a peer within [`LOG_WINDOW`] is logged at `warn!` and the rest +//! at `debug!`. The peer table that makes this possible is itself capped, or it +//! would just move the unbounded growth from the log to memory. + +use secrecy::{ExposeSecret, SecretString}; +use std::collections::HashMap; +use std::net::IpAddr; +use std::sync::{Arc, Mutex, PoisonError}; +use std::time::{Duration, Instant}; +use subtle::ConstantTimeEq; +use tonic::service::Interceptor; +use tonic::{Request, Status}; +use tracing::{debug, warn}; + +const AUTHORIZATION_HEADER: &str = "authorization"; +/// RFC 7235 defines the auth-scheme as case-insensitive, and proxies do +/// normalize it, so the scheme is matched without regard to case. The +/// credential that follows is still compared byte for byte. +const BEARER_SCHEME: &str = "Bearer"; +/// How long one rejected peer stays quiet after its `warn!` line. Short enough +/// that a real operator debugging a wrong token still sees a line per attempt +/// once they pause, long enough that a flood costs one line per minute. +const LOG_WINDOW: Duration = Duration::from_secs(60); +/// Ceiling on tracked peers. Without it a caller that spoofs a fresh source +/// address per request would grow this table forever, which is the same +/// resource exhaustion the throttle exists to prevent. +const MAX_TRACKED_PEERS: usize = 1024; + +/// Rejects any request that does not carry the configured bearer token. +#[derive(Clone)] +pub struct BearerAuth { + token: Arc, + /// Last time each peer was logged at `warn!`. `None` keys the peers tonic + /// could not attribute to an address, so they are throttled as one bucket + /// rather than escaping the cap. + /// + /// A `std::sync::Mutex` because [`Interceptor::call`] is synchronous; the + /// critical section is a hash lookup and never awaits. `Arc` because tonic + /// clones the interceptor per connection and the table has to be shared. + warned: Arc, Instant>>>, +} + +impl BearerAuth { + pub fn new(token: SecretString) -> Self { + Self { + token: Arc::new(token), + warned: Arc::new(Mutex::new(HashMap::new())), + } + } + + /// True when this rejection deserves a `warn!` rather than a `debug!`. + /// + /// `now` is a parameter so the window can be tested without sleeping. + fn should_warn(&self, peer: Option, now: Instant) -> bool { + // A poisoned lock means another thread panicked mid-update. The table + // is a logging heuristic, so recovering the map beats propagating a + // panic out of an interceptor and killing the connection. + let mut warned = self.warned.lock().unwrap_or_else(PoisonError::into_inner); + + if let Some(last) = warned.get(&peer) { + if now.duration_since(*last) < LOG_WINDOW { + return false; + } + } else if warned.len() >= MAX_TRACKED_PEERS { + warned.retain(|_, last| now.duration_since(*last) < LOG_WINDOW); + // Still full: every slot belongs to a peer inside its window, so + // this one is part of a flood. Stay quiet rather than grow. + if warned.len() >= MAX_TRACKED_PEERS { + return false; + } + } + + warned.insert(peer, now); + true + } +} + +impl Interceptor for BearerAuth { + fn call(&mut self, request: Request<()>) -> Result, Status> { + let presented = request + .metadata() + .get(AUTHORIZATION_HEADER) + .and_then(|value| value.to_str().ok()) + .and_then(|value| { + let (scheme, credential) = value.split_once(' ')?; + // RFC 7235 allows `1*SP` between the scheme and the credential, + // and token68 contains no spaces, so trimming the rest is + // lossless. + let credential = credential.trim_start_matches(' '); + scheme + .eq_ignore_ascii_case(BEARER_SCHEME) + .then_some(credential) + }); + + match presented { + Some(candidate) if credentials_match(candidate, self.token.expose_secret()) => { + Ok(request) + } + // One message for every failure mode: a caller learns whether the + // port is an admin RPC, never whether it guessed part of a token. + _ => { + let peer = request.remote_addr().map(|addr| addr.ip()); + if self.should_warn(peer, Instant::now()) { + match peer { + Some(ip) => warn!("Rejected unauthenticated admin RPC from {}", ip), + None => warn!("Rejected unauthenticated admin RPC from an unknown peer"), + } + } else { + match peer { + Some(ip) => debug!( + "Rejected unauthenticated admin RPC from {} (further rejections from \ + this peer are logged at debug for up to {}s)", + ip, + LOG_WINDOW.as_secs() + ), + None => debug!("Rejected unauthenticated admin RPC from an unknown peer"), + } + } + Err(Status::unauthenticated("missing or invalid credentials")) + } + } + } +} + +/// Compare the presented credential against the configured one without leaking +/// how far the two matched. +/// +/// `subtle` is used rather than a hand-written loop because only its +/// optimization barrier makes the constant-time property a guarantee the +/// compiler must honour. `ct_eq` on slices already answers `false` for +/// mismatched lengths, and the token's length is set by the operator's config +/// rather than being itself a secret. +fn credentials_match(presented: &str, configured: &str) -> bool { + presented + .as_bytes() + .ct_eq(configured.as_bytes()) + .unwrap_u8() + == 1 +} + +#[cfg(test)] +mod tests { + use super::*; + use tonic::metadata::MetadataValue; + + const TOKEN: &str = "0123456789abcdef0123456789abcdef"; + + fn interceptor() -> BearerAuth { + BearerAuth::new(SecretString::from(TOKEN)) + } + + fn request_with_authorization(value: &str) -> Request<()> { + let mut request = Request::new(()); + request.metadata_mut().insert( + AUTHORIZATION_HEADER, + MetadataValue::try_from(value).expect("header value is ASCII"), + ); + request + } + + #[test] + fn accepts_the_configured_token() { + let result = interceptor().call(request_with_authorization(&format!("Bearer {TOKEN}"))); + assert!(result.is_ok()); + } + + #[test] + fn accepts_extra_spaces_after_the_scheme() { + // RFC 7235 auth-param grammar is `scheme 1*SP token68`, and proxies do + // rewrite the separator. + let result = interceptor().call(request_with_authorization(&format!("Bearer {TOKEN}"))); + assert!(result.is_ok()); + } + + #[test] + fn accepts_any_casing_of_the_bearer_scheme() { + // RFC 7235: the auth-scheme is case-insensitive, and proxies rewrite it. + for scheme in ["Bearer", "bearer", "BEARER", "BeArEr"] { + let result = + interceptor().call(request_with_authorization(&format!("{scheme} {TOKEN}"))); + assert!(result.is_ok(), "{scheme} should be accepted"); + } + } + + #[test] + fn rejects_another_scheme_carrying_the_right_token() { + let status = interceptor() + .call(request_with_authorization(&format!("Basic {TOKEN}"))) + .expect_err("only the Bearer scheme is accepted"); + assert_eq!(status.code(), tonic::Code::Unauthenticated); + } + + #[test] + fn rejects_a_missing_header() { + let status = interceptor() + .call(Request::new(())) + .expect_err("no credentials must be refused"); + assert_eq!(status.code(), tonic::Code::Unauthenticated); + } + + #[test] + fn rejects_a_token_without_the_bearer_prefix() { + let status = interceptor() + .call(request_with_authorization(TOKEN)) + .expect_err("a bare token must be refused"); + assert_eq!(status.code(), tonic::Code::Unauthenticated); + } + + #[test] + fn rejects_a_wrong_token_of_equal_length() { + let mut wrong = TOKEN.to_string(); + wrong.pop(); + wrong.push('0'); + assert_eq!(wrong.len(), TOKEN.len()); + + let status = interceptor() + .call(request_with_authorization(&format!("Bearer {wrong}"))) + .expect_err("a wrong token must be refused"); + assert_eq!(status.code(), tonic::Code::Unauthenticated); + } + + #[test] + fn rejects_a_token_that_is_a_prefix_of_the_real_one() { + let status = interceptor() + .call(request_with_authorization(&format!( + "Bearer {}", + &TOKEN[..TOKEN.len() - 1] + ))) + .expect_err("a truncated token must be refused"); + assert_eq!(status.code(), tonic::Code::Unauthenticated); + } + + #[test] + fn credentials_match_only_on_exact_equality() { + assert!(credentials_match("abc", "abc")); + assert!(!credentials_match("abc", "abd")); + assert!(!credentials_match("abc", "ab")); + assert!(!credentials_match("ab", "abc")); + // The credential itself stays case-sensitive even though the scheme + // is not. + assert!(!credentials_match("ABC", "abc")); + } + + fn peer(last_octet: u8) -> Option { + Some(IpAddr::from([192, 0, 2, last_octet])) + } + + #[test] + fn a_flooding_peer_is_warned_about_once_per_window() { + let auth = interceptor(); + let start = Instant::now(); + + assert!(auth.should_warn(peer(1), start)); + for attempt in 1..100 { + assert!( + !auth.should_warn(peer(1), start + Duration::from_millis(attempt)), + "attempt {attempt} must not produce a second warn line" + ); + } + // Once the window closes the peer is audible again, so a slow retry + // loop still leaves a trail. + assert!(auth.should_warn(peer(1), start + LOG_WINDOW)); + } + + #[test] + fn each_peer_gets_its_own_window() { + let auth = interceptor(); + let now = Instant::now(); + assert!(auth.should_warn(peer(1), now)); + assert!(auth.should_warn(peer(2), now)); + assert!(auth.should_warn(None, now)); + // ...and the second attempt from each is throttled independently. + assert!(!auth.should_warn(peer(1), now)); + assert!(!auth.should_warn(peer(2), now)); + assert!(!auth.should_warn(None, now)); + } + + #[test] + fn the_peer_table_cannot_grow_without_bound() { + // A caller spoofing a fresh source address per request must not be able + // to turn the log throttle into a memory leak. + let auth = interceptor(); + let now = Instant::now(); + for octet in 0..=u8::MAX { + for third in 0..=u8::MAX { + auth.should_warn(Some(IpAddr::from([192, 0, third, octet])), now); + } + } + let tracked = auth + .warned + .lock() + .unwrap_or_else(PoisonError::into_inner) + .len(); + assert!( + tracked <= MAX_TRACKED_PEERS, + "{tracked} peers tracked, cap is {MAX_TRACKED_PEERS}" + ); + } + + #[test] + fn expired_entries_are_reclaimed_when_the_table_fills() { + let auth = interceptor(); + let start = Instant::now(); + for third in 0..=u8::MAX { + for fourth in 0..=u8::MAX { + auth.should_warn(Some(IpAddr::from([198, 51, third, fourth])), start); + } + } + // Every tracked peer is now outside its window, so a new peer is both + // admitted and audible rather than silently dropped. + assert!(auth.should_warn(peer(7), start + LOG_WINDOW)); + } +} diff --git a/src/rpc/mod.rs b/src/rpc/mod.rs index 60f80d72..7cfc7c59 100644 --- a/src/rpc/mod.rs +++ b/src/rpc/mod.rs @@ -4,6 +4,7 @@ //! for admin operations without going through the Nostr protocol. This is useful //! for local development and admin applications that need low-latency access. +pub mod auth; pub mod rate_limiter; pub mod server; pub mod service; diff --git a/src/rpc/server.rs b/src/rpc/server.rs index 46ff79f7..5333d7d5 100644 --- a/src/rpc/server.rs +++ b/src/rpc/server.rs @@ -1,13 +1,19 @@ //! RPC server implementation for admin operations +use crate::config::constants::RPC_TOKEN_ENV_VAR; +use crate::config::secret::read_rpc_token_env_var; use crate::config::settings::Settings; +use crate::config::util::listen_socket_addr; use crate::lightning::LndConnector; +use crate::rpc::auth::BearerAuth; use crate::rpc::service::AdminServiceImpl; use nostr_sdk::prelude::Keys; use sqlx::{Pool, Sqlite}; use std::sync::Arc; -use tonic::transport::Server; -use tracing::{error, info}; +use std::time::Duration; +use tonic::transport::server::TcpIncoming; +use tonic::transport::{Identity, Server, ServerTlsConfig}; +use tracing::info; use super::admin::admin_service_server::AdminServiceServer; @@ -15,6 +21,9 @@ use super::admin::admin_service_server::AdminServiceServer; pub struct RpcServer { listen_address: String, port: u16, + /// Certificate and key, or plaintext. Pairing them here keeps + /// "both or neither" a property of the type rather than a runtime check. + tls: Option<(String, String)>, } impl RpcServer { @@ -24,34 +33,95 @@ impl RpcServer { Self { listen_address: rpc_config.listen_address.clone(), port: rpc_config.port, + tls: rpc_config + .tls_paths() + .map(|(cert, key)| (cert.to_string(), key.to_string())), } } - /// Start the RPC server - pub async fn start( + /// Acquire the listener and return the bound address with the future that + /// serves it. + /// + /// Everything that can fail on the way up happens here, before the caller + /// gets anything to detach: a missing bearer token, unusable TLS material, + /// an address already in use. The returned future only accepts connections, + /// so a caller that awaits this function knows the admin API is listening + /// and gated before it lets the rest of the daemon proceed — `[rpc].enabled + /// = true` becomes an invariant rather than a hope. + /// + /// The address comes back from the listener rather than from the config, so + /// it is the port actually in use: `port = 0` reports the ephemeral port the + /// kernel picked instead of a literal `:0`. + /// + /// Refusing to serve without a token is deliberately redundant with + /// `config::util::validate_rpc_settings`: a code path that reached here + /// without one would expose an ungated admin API, and no RPC at all is the + /// safer failure. + pub fn bind( &self, my_keys: Keys, pool: Arc>, ln_client: Arc>, - ) -> Result<(), Box> { - let addr = format!("{}:{}", self.listen_address, self.port) - .parse() - .map_err(|e| format!("Invalid address: {}", e))?; + ) -> Result< + ( + std::net::SocketAddr, + impl std::future::Future> + Send + 'static, + ), + Box, + > { + let addr = listen_socket_addr(&self.listen_address, self.port)?; - let admin_service = AdminServiceImpl::new(my_keys, pool, ln_client); + let token = read_rpc_token_env_var().ok_or_else(|| { + format!("Refusing to start the admin RPC server: {RPC_TOKEN_ENV_VAR} is not set") + })?; - info!("Starting RPC server on {}", addr); + let admin_service = AdminServiceImpl::new(my_keys, pool, ln_client); - let server = Server::builder() - .add_service(AdminServiceServer::new(admin_service)) - .serve(addr); + // The admin surface is reachable off-host under `allow_remote`, and + // the interceptor rejects requests without closing connections, so the + // transport carries its own limits. + let mut builder = Server::builder() + .timeout(Duration::from_secs(30)) + .max_concurrent_streams(Some(64)) + .http2_keepalive_interval(Some(Duration::from_secs(30))) + .http2_keepalive_timeout(Some(Duration::from_secs(10))); + let transport = match &self.tls { + Some((cert_path, key_path)) => { + let cert = std::fs::read(cert_path) + .map_err(|e| format!("Failed to read {cert_path}: {e}"))?; + let key = std::fs::read(key_path) + .map_err(|e| format!("Failed to read {key_path}: {e}"))?; + // Malformed PEM is rejected by `tls_config`, so it surfaces + // here rather than inside the detached serving future. + builder = builder + .tls_config(ServerTlsConfig::new().identity(Identity::from_pem(cert, key)))?; + "TLS" + } + None => "plaintext", + }; - if let Err(e) = server.await { - error!("RPC server error: {}", e); - return Err(Box::new(e)); - } + // Binds eagerly: an occupied port is a startup error, not a surprise + // discovered later by whoever happens to read the logs. + // `serve_with_incoming` ignores the builder's TCP settings, so the + // nodelay default `Server::serve` would have applied is set on the + // listener here — without it Nagle delays every small admin frame. + let incoming = TcpIncoming::bind(addr) + .map_err(|e| format!("Failed to bind {addr}: {e}"))? + .with_nodelay(Some(true)); + let bound = incoming + .local_addr() + .map_err(|e| format!("Failed to read the address bound to {addr}: {e}"))?; + info!("RPC server listening on {} ({})", bound, transport); - Ok(()) + Ok(( + bound, + builder + .add_service(AdminServiceServer::with_interceptor( + admin_service, + BearerAuth::new(token), + )) + .serve_with_incoming(incoming), + )) } /// Check if RPC server is enabled @@ -82,10 +152,7 @@ mod tests { #[test] fn test_rpc_server_structure() { // Test that RpcServer can be created with explicit values - let server = RpcServer { - listen_address: "localhost".to_string(), - port: 8080, - }; + let server = server_at("localhost", 8080); assert_eq!(server.listen_address, "localhost"); assert_eq!(server.port, 8080); @@ -93,10 +160,7 @@ mod tests { #[test] fn test_address_formatting() { - let server = RpcServer { - listen_address: "127.0.0.1".to_string(), - port: 50051, - }; + let server = server_at("127.0.0.1", 50051); let expected_addr = format!("{}:{}", server.listen_address, server.port); assert_eq!(expected_addr, "127.0.0.1:50051"); @@ -111,6 +175,51 @@ mod tests { let _ = MOSTRO_CONFIG.set(test_settings()); } + /// Plaintext server bound to an explicit address, so the tests below stay + /// readable as `RpcServer` grows optional fields. + fn server_at(listen_address: &str, port: u16) -> RpcServer { + RpcServer { + listen_address: listen_address.to_string(), + port, + tls: None, + } + } + + // `MOSTRO_RPC_TOKEN` is process-wide state, so the tests that touch it + // serialize on the crate-wide environment lock. One lock for the whole + // environment, not one per variable: two tests holding two different locks + // would still race glibc's `setenv` against `getenv` elsewhere. + use crate::config::util::ENV_LOCK; + + /// Sets `MOSTRO_RPC_TOKEN` for the duration of a test and restores the + /// previous value on drop. + struct RpcTokenGuard { + previous: Option, + } + + impl RpcTokenGuard { + fn set(value: &str) -> Self { + let previous = std::env::var(RPC_TOKEN_ENV_VAR).ok(); + std::env::set_var(RPC_TOKEN_ENV_VAR, value); + Self { previous } + } + + fn unset() -> Self { + let previous = std::env::var(RPC_TOKEN_ENV_VAR).ok(); + std::env::remove_var(RPC_TOKEN_ENV_VAR); + Self { previous } + } + } + + impl Drop for RpcTokenGuard { + fn drop(&mut self) { + match &self.previous { + Some(value) => std::env::set_var(RPC_TOKEN_ENV_VAR, value), + None => std::env::remove_var(RPC_TOKEN_ENV_VAR), + } + } + } + /// Offline `LndConnector` (lazy connect, no network until first RPC). async fn offline_ln_client() -> Arc> { let dir = std::env::temp_dir().join(format!("mostro-rpcsrv-{}", std::process::id())); @@ -146,33 +255,236 @@ mod tests { } #[tokio::test] - async fn start_rejects_unparseable_address() { + async fn bind_rejects_unparseable_address() { + init_test_settings(); + let _lock = ENV_LOCK.lock().await; + let _token = RpcTokenGuard::set(&"t".repeat(32)); + // `localhost` and bare `::1` are in here on purpose: they read like + // valid loopback spellings, and `config::util` rejects them for exactly + // this reason — `SocketAddr` cannot parse either. + for address in ["not an address", "localhost", "::1"] { + let server = server_at(address, 50051); + let pool = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap(); + let error = server + .bind(Keys::generate(), Arc::new(pool), offline_ln_client().await) + .err() + .expect("an address that cannot be parsed must not serve"); + assert!( + error.to_string().contains("Invalid address"), + "{address} should have been refused, got: {error}" + ); + } + } + + /// The startup invariant the daemon depends on: a listener that cannot be + /// acquired is reported by `bind` itself, so `main` learns about it before + /// it detaches anything or continues booting. + #[tokio::test] + async fn bind_surfaces_bind_failure_before_returning() { + init_test_settings(); + let _lock = ENV_LOCK.lock().await; + let _token = RpcTokenGuard::set(&"t".repeat(32)); + // 8.8.8.8 is not a local interface, so the bind fails immediately. + let server = server_at("8.8.8.8", 1); + let pool = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap(); + let error = server + .bind(Keys::generate(), Arc::new(pool), offline_ln_client().await) + .err() + .expect("an unavailable address must fail before serving"); + assert!(error.to_string().contains("Failed to bind")); + } + + #[tokio::test] + async fn bind_refuses_to_serve_without_a_token() { + init_test_settings(); + let _lock = ENV_LOCK.lock().await; + let _token = RpcTokenGuard::unset(); + // 127.0.0.1:0 would otherwise bind successfully, so reaching the error + // path proves the token check runs before anything starts listening. + let server = server_at("127.0.0.1", 0); + let pool = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap(); + let error = server + .bind(Keys::generate(), Arc::new(pool), offline_ln_client().await) + .err() + .expect("an admin RPC without a token must never serve"); + assert!(error.to_string().contains(RPC_TOKEN_ENV_VAR)); + } + + #[tokio::test] + async fn bind_rejects_malformed_tls_material() { init_test_settings(); + let _lock = ENV_LOCK.lock().await; + let _token = RpcTokenGuard::set(&"t".repeat(32)); + // Readable files that are not valid PEM: config validation accepts + // them, so `bind` is the layer that has to catch this — and it must do + // so before returning, or the daemon boots without the API it was told + // to serve. + let dir = std::env::temp_dir().join(format!("mostro-rpc-badtls-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("create temp dir"); + let cert = dir.join("cert.pem"); + let key = dir.join("key.pem"); + std::fs::write(&cert, b"not a certificate").expect("write cert"); + std::fs::write(&key, b"not a key").expect("write key"); + let server = RpcServer { - listen_address: "not an address".to_string(), - port: 50051, + listen_address: "127.0.0.1".to_string(), + port: 0, + tls: Some(( + cert.to_string_lossy().into_owned(), + key.to_string_lossy().into_owned(), + )), }; let pool = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap(); - let result = server - .start(Keys::generate(), Arc::new(pool), offline_ln_client().await) - .await; - assert!(result.is_err()); + assert!(server + .bind(Keys::generate(), Arc::new(pool), offline_ln_client().await) + .is_err()); } + /// End-to-end proof that the interceptor gates the service that is actually + /// served. The unit tests in `crate::rpc::auth` only cover the interceptor + /// in isolation, so they would stay green if a refactor registered the + /// service without it — which is precisely the regression that would + /// reopen the hole this module exists to close. + /// + /// `GetVersion` is the probe: it touches neither the database nor LND, so + /// the only thing under test is the authentication decision. #[tokio::test] - async fn start_surfaces_bind_failure() { + async fn served_rpc_rejects_calls_without_the_token() { + use crate::rpc::admin::{admin_service_client::AdminServiceClient, GetVersionRequest}; + use tonic::metadata::MetadataValue; + use tonic::transport::Channel; + use tonic::Request; + init_test_settings(); - // 8.8.8.8 is not a local interface: the bind fails immediately, so - // the server error path is exercised without serving traffic. + let _lock = ENV_LOCK.lock().await; + let token = "t".repeat(32); + let _guard = RpcTokenGuard::set(&token); + + let pool = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap(); + let ln_client = offline_ln_client().await; + // Port 0: the listener `bind` returns owns the ephemeral port for as + // long as the test needs it. Reserving a port and releasing it first + // would leave a window in which the kernel can hand it to another + // process — a flake, not a failure of what is under test. + let server = server_at("127.0.0.1", 0); + let (bound, serving) = server + .bind(Keys::generate(), Arc::new(pool), ln_client) + .expect("bind must succeed on a free loopback port"); + let serving = tokio::spawn(serving); + + // No retry loop: `bind` returned, so the listener already exists and + // the connection below must succeed on the first attempt. A retry here + // would hide exactly the regression this asserts against. + let channel = Channel::from_shared(format!("http://{bound}")) + .expect("valid endpoint") + .connect() + .await + .expect("the listener is open as soon as bind returns"); + + let status = AdminServiceClient::new(channel.clone()) + .get_version(GetVersionRequest {}) + .await + .expect_err("an anonymous call must be refused"); + assert_eq!(status.code(), tonic::Code::Unauthenticated); + + let credential: MetadataValue<_> = format!("Bearer {token}") + .parse() + .expect("token is a valid header value"); + let mut authenticated = + AdminServiceClient::with_interceptor(channel, move |mut request: Request<()>| { + request + .metadata_mut() + .insert("authorization", credential.clone()); + Ok(request) + }); + let version = authenticated + .get_version(GetVersionRequest {}) + .await + .expect("an authenticated call must go through") + .into_inner() + .version; + assert_eq!(version, env!("CARGO_PKG_VERSION")); + + serving.abort(); + } + + /// The mirror image of `bind_rejects_malformed_tls_material`: valid PEM + /// must produce a server that actually speaks TLS. The regression this + /// pins is "reads as TLS is on while serving plaintext" — for example + /// dropping the `builder = builder.tls_config(...)` reassignment — which + /// every plaintext test in this file would miss, and which the client + /// below turns into a failed handshake instead of a green run. + #[tokio::test] + async fn served_rpc_over_tls_authenticates_the_token() { + use crate::rpc::admin::{admin_service_client::AdminServiceClient, GetVersionRequest}; + use tonic::metadata::MetadataValue; + use tonic::transport::{Certificate, Channel, ClientTlsConfig}; + use tonic::Request; + + init_test_settings(); + let _lock = ENV_LOCK.lock().await; + let token = "t".repeat(32); + let _guard = RpcTokenGuard::set(&token); + + let identity = rcgen::generate_simple_self_signed(vec!["localhost".to_string()]) + .expect("generate a self-signed certificate"); + let dir = std::env::temp_dir().join(format!("mostro-rpc-tls-ok-{}", std::process::id())); + std::fs::create_dir_all(&dir).expect("create temp dir"); + let cert_path = dir.join("cert.pem"); + let key_path = dir.join("key.pem"); + std::fs::write(&cert_path, identity.cert.pem()).expect("write cert"); + std::fs::write(&key_path, identity.key_pair.serialize_pem()).expect("write key"); + let server = RpcServer { - listen_address: "8.8.8.8".to_string(), - port: 1, + listen_address: "127.0.0.1".to_string(), + port: 0, + tls: Some(( + cert_path.to_string_lossy().into_owned(), + key_path.to_string_lossy().into_owned(), + )), }; let pool = sqlx::SqlitePool::connect("sqlite::memory:").await.unwrap(); - let result = server - .start(Keys::generate(), Arc::new(pool), offline_ln_client().await) - .await; - assert!(result.is_err()); + let (bound, serving) = server + .bind(Keys::generate(), Arc::new(pool), offline_ln_client().await) + .expect("bind must succeed with valid TLS material"); + let serving = tokio::spawn(serving); + + let tls = ClientTlsConfig::new() + .ca_certificate(Certificate::from_pem(identity.cert.pem())) + .domain_name("localhost"); + let channel = Channel::from_shared(format!("https://{bound}")) + .expect("valid endpoint") + .tls_config(tls) + .expect("client TLS config") + .connect() + .await + .expect("the TLS handshake must succeed against the served certificate"); + + let status = AdminServiceClient::new(channel.clone()) + .get_version(GetVersionRequest {}) + .await + .expect_err("an anonymous call must be refused over TLS too"); + assert_eq!(status.code(), tonic::Code::Unauthenticated); + + let credential: MetadataValue<_> = format!("Bearer {token}") + .parse() + .expect("token is a valid header value"); + let mut authenticated = + AdminServiceClient::with_interceptor(channel, move |mut request: Request<()>| { + request + .metadata_mut() + .insert("authorization", credential.clone()); + Ok(request) + }); + let version = authenticated + .get_version(GetVersionRequest {}) + .await + .expect("an authenticated TLS call must go through") + .into_inner() + .version; + assert_eq!(version, env!("CARGO_PKG_VERSION")); + + serving.abort(); } #[test] diff --git a/src/rpc/service.rs b/src/rpc/service.rs index 854230a0..921e4eb0 100644 --- a/src/rpc/service.rs +++ b/src/rpc/service.rs @@ -62,11 +62,14 @@ impl AdminServiceImpl { ); // Admin RPC flows synthesize the inbound event with the node's own - // pubkey in both `identity` and `sender` slots. Authorization is then - // enforced downstream: the caller must be the assigned solver - // (`is_assigned_solver`), with `ensure_dispute_finalize_permission` - // bypassing solver category checks for the daemon key (same as - // `admin_take_dispute`). gRPC transport authenticates the operator. + // pubkey in both `identity` and `sender` slots, so every downstream + // check sees the daemon itself: `ensure_dispute_finalize_permission` + // waives the solver-category check for the daemon key (same as + // `admin_take_dispute`), and `admin_add_solver_action` accepts it + // outright. In other words the handlers below apply *no* caller + // authorization of their own — the bearer-token interceptor in + // `crate::rpc::auth` is the only thing standing between the network + // and these actions. let event = UnwrappedMessage { message: msg.clone(), signature: None,