diff --git a/crates/malachite-app/README.md b/crates/malachite-app/README.md index 5198969..6f5a548 100644 --- a/crates/malachite-app/README.md +++ b/crates/malachite-app/README.md @@ -78,7 +78,7 @@ arc-node-consensus start \ --p2p.addr=/ip4/172.19.0.5/tcp/27000 \ --p2p.persistent-peers=/ip4/172.19.0.6/tcp/27000,/ip4/172.19.0.7/tcp/27000 \ --metrics=172.19.0.5:29000 \ - --rpc.addr=0.0.0.0:31000 \ + --rpc.addr=172.19.0.5:31000 \ --eth-socket=/tmp/reth.ipc \ --execution-socket=/tmp/auth.ipc \ --minimal @@ -95,13 +95,15 @@ arc-node-consensus start \ --p2p.addr=/ip4/172.19.0.5/tcp/27000 \ --p2p.persistent-peers=/ip4/172.19.0.6/tcp/27000,/ip4/172.19.0.7/tcp/27000 \ --metrics=0.0.0.0:29000 \ - --rpc.addr=0.0.0.0:31000 \ + --rpc.addr=127.0.0.1:31000 \ --eth-rpc-endpoint=http://localhost:8545 \ --execution-endpoint=http://localhost:8551 \ --execution-jwt=jwtsecret \ --minimal ``` +The CL RPC port is an internal interface. These examples bind it to loopback or to the node's private interface, never to `0.0.0.0`. See the port table in [running-an-arc-node.md](../../docs/running-an-arc-node.md). + Note: to generate a JWT (JSON web token), use the following command: ```bash @@ -162,7 +164,8 @@ https://example.com,wss=ws.example.com:1212 - `--discovery.num-inbound-peers` - Number of inbound peers (default: 20) - `--value-sync` - Enable value sync (default: true) - `--metrics` - Enable metrics and set listen address (e.g., "0.0.0.0:29000") -- `--rpc.addr` - Enable RPC and set listen address (e.g., "0.0.0.0:31000") +- `--rpc.addr` - Enable RPC and set listen address (e.g., "127.0.0.1:31000"). The CL RPC port is an internal interface: bind it to loopback or a private interface and firewall it off from the public internet (see the port table in [running-an-arc-node.md](../../docs/running-an-arc-node.md)) +- `--rpc.admin-token-file` - Path to a file holding the bearer token required by the privileged RPC routes (adding and removing persistent peers). Without it those routes are not served at all. Generate one with `openssl rand -hex 32 > admin-token` - `--full` - Arc full-node pruning preset; sets `--prune.certificates.distance 237600`; mutually exclusive with `--minimal` and the individual `--prune.certificates.*` flags - `--minimal` - Arc minimal-storage pruning preset; sets `--prune.certificates.distance 237600`; mutually exclusive with `--full` and the individual `--prune.certificates.*` flags - `--prune.certificates.distance` - Keep certificates for the last N heights (default: 0, disabled/archive node); mutually exclusive with `--prune.certificates.before` and `--full/--minimal` presets @@ -274,7 +277,16 @@ The following environment variables can be used to modify behavior: ## REST API -The consensus layer exposes a REST API for monitoring and querying consensus state when `--rpc.addr` is set (e.g., `--rpc.addr=0.0.0.0:26658`). +The consensus layer exposes a REST API for monitoring and querying consensus state when `--rpc.addr` is set (e.g., `--rpc.addr=127.0.0.1:26658`). + +### Public and privileged routes + +The API has two classes of route, and the split is the security boundary of this listener: + +- **Public, read-only.** Everything under [Available Endpoints](#available-endpoints). These only report state. +- **Privileged.** `POST /persistent-peers` and `DELETE /persistent-peers` change the node's peer set while it is running. They are served only when `--rpc.admin-token-file` is set, and a request must then carry that token as `Authorization: Bearer `. Without the flag the paths are not routed at all and `GET /` does not list them. + +Peer mutation is an operator action. An unauthenticated caller that could reach it would be able to add its own peer or remove the peers a validator depends on, which matters most for a node run with `--p2p.persistent-peers-only`. Keep the RPC port internal either way: the token is the second line of defence, not a licence to expose the port. ### API Versioning @@ -333,6 +345,11 @@ All endpoints support versioning: - `GET /commit?height=N` - Commit certificate for specific height - `GET /network-state` - Network peer information +Privileged, and served only with `--rpc.admin-token-file` (see [Public and privileged routes](#public-and-privileged-routes)): + +- `POST /persistent-peers` - Add a persistent peer at runtime +- `DELETE /persistent-peers` - Remove a persistent peer at runtime + #### Example API Usage **Get Status:** @@ -356,6 +373,18 @@ curl http://localhost:26658/health curl http://localhost:26658/ ``` +**Add a persistent peer (privileged):** +```bash +curl -X POST \ + -H "Accept: application/vnd.arc.v1+json" \ + -H "Authorization: Bearer $(cat /etc/arc/rpc-admin-token)" \ + -H "Content-Type: application/json" \ + -d '{"addr":"/ip4/10.0.0.2/tcp/27000/p2p/12D3KooW..."}' \ + http://localhost:26658/persistent-peers +``` + +Without the header the node answers `401` with `{"error":"Missing bearer token"}`. On a node started without `--rpc.admin-token-file` the route does not exist and the node answers `404`. + ### Deprecation Policy When breaking changes are introduced: diff --git a/crates/malachite-app/src/main.rs b/crates/malachite-app/src/main.rs index d69e8b2..8bbc030 100644 --- a/crates/malachite-app/src/main.rs +++ b/crates/malachite-app/src/main.rs @@ -22,11 +22,11 @@ const PRESETS_PRUNE_CERTIFICATES_DISTANCE: u64 = 237_600; use bytesize::ByteSize; use eyre::{eyre, Result}; -use tracing::{info, trace}; +use tracing::{info, trace, warn}; use arc_consensus_types::{ - Config, ExecutionConfig, Height, MetricsConfig, PruningConfig, RpcConfig, RuntimeConfig, - SigningConfig, + AdminToken, Config, ExecutionConfig, Height, MetricsConfig, PruningConfig, RpcConfig, + RuntimeConfig, SigningConfig, }; use arc_node_consensus::hardcoded_config; use arc_node_consensus::node::{App, StartConfig}; @@ -122,6 +122,29 @@ fn build_signing_config(cmd: &StartCmd) -> Result { } } +/// Read the bearer token that the privileged RPC routes require. +/// +/// Returns `None` when `--rpc.admin-token-file` is not set, which leaves those +/// routes unregistered. A path that cannot be read, or a file with no token in +/// it, is a startup error rather than a silently open RPC surface. +fn build_rpc_admin_token(cmd: &StartCmd) -> Result> { + let Some(path) = cmd.rpc_admin_token_file.as_ref() else { + return Ok(None); + }; + + let contents = std::fs::read_to_string(path).map_err(|e| { + eyre!( + "Failed to read --rpc.admin-token-file '{}': {e}", + path.display() + ) + })?; + + let token = AdminToken::from_file_contents(&contents) + .map_err(|e| eyre!("Invalid --rpc.admin-token-file '{}': {e}", path.display()))?; + + Ok(Some(token)) +} + /// Build configuration from CLI arguments fn build_config_from_cli(cmd: &StartCmd, logging: config::LoggingConfig) -> Result { let p2p_listen_addr = cmd.p2p_listen_addr()?; @@ -174,8 +197,17 @@ fn build_config_from_cli(cmd: &StartCmd, logging: config::LoggingConfig) -> Resu listen_addr: cmd .rpc_addr .unwrap_or_else(|| "0.0.0.0:31000".parse().expect("valid socket address")), + admin_token: build_rpc_admin_token(cmd)?, }; + if rpc.enabled && !rpc.listen_addr.ip().is_loopback() { + warn!( + listen_addr = %rpc.listen_addr, + "CL RPC is bound to a non-loopback address. It is an internal interface, so keep it \ + off the public internet with a firewall or a private network" + ); + } + let certificates_distance = if cmd.full || cmd.minimal { PRESETS_PRUNE_CERTIFICATES_DISTANCE } else { diff --git a/crates/malachite-app/src/node.rs b/crates/malachite-app/src/node.rs index ae51a75..e408263 100644 --- a/crates/malachite-app/src/node.rs +++ b/crates/malachite-app/src/node.rs @@ -578,11 +578,13 @@ impl App { let listen_addr = self.config.rpc.listen_addr; let request_handle = channels.requests.clone(); let net_request_handle = channels.net_requests.clone(); + let admin_token = self.config.rpc.admin_token.clone(); crate::rpc::serve( listen_addr, request_handle, tx_rpc_req.clone(), net_request_handle, + admin_token, ) }); Some(join_handle) diff --git a/crates/malachite-app/src/rpc/middleware.rs b/crates/malachite-app/src/rpc/middleware.rs index d7b62c1..1554874 100644 --- a/crates/malachite-app/src/rpc/middleware.rs +++ b/crates/malachite-app/src/rpc/middleware.rs @@ -14,17 +14,76 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Middleware for API version extraction and negotiation +//! Middleware for API version extraction and negotiation, and for the admin +//! credential the privileged routes require. -use axum::extract::Request; +use axum::extract::{Request, State}; use axum::http::{header, StatusCode}; use axum::middleware::Next; use axum::response::{IntoResponse, Response}; use serde_json::json; use tracing::debug; +use arc_consensus_types::AdminToken; + use super::version::ApiVersion; +/// Axum middleware that rejects a request unless it presents the configured admin +/// token as `Authorization: Bearer `. +/// +/// This is applied with `Router::route_layer`, so it only runs for the privileged +/// routes. Those routes are not registered at all when no token is configured, +/// which keeps peer mutation off a node that has not opted in. +pub async fn require_admin_token( + State(expected): State, + req: Request, + next: Next, +) -> Response { + let Some(presented) = bearer_token(&req) else { + debug!( + path = %req.uri().path(), + "Privileged RPC request without a bearer token, returning 401" + ); + return unauthorized("Missing bearer token"); + }; + + if !expected.matches(presented) { + debug!( + path = %req.uri().path(), + "Privileged RPC request with a bearer token that does not match, returning 401" + ); + return unauthorized("Invalid admin token"); + } + + next.run(req).await +} + +/// Extract the credential from an `Authorization: Bearer ` header. +fn bearer_token(req: &Request) -> Option<&str> { + let value = req.headers().get(header::AUTHORIZATION)?.to_str().ok()?; + let (scheme, token) = value.split_once(' ')?; + + if !scheme.eq_ignore_ascii_case("bearer") { + return None; + } + + let token = token.trim(); + if token.is_empty() { + return None; + } + + Some(token) +} + +fn unauthorized(message: &'static str) -> Response { + ( + StatusCode::UNAUTHORIZED, + [(header::WWW_AUTHENTICATE, "Bearer")], + axum::Json(json!({ "error": message })), + ) + .into_response() +} + /// Axum middleware that extracts the API version from the Accept header /// and stores it in the request extensions. /// @@ -114,4 +173,25 @@ mod tests { None ); } + + #[test] + fn test_bearer_token_parsing() { + let with_auth = |value: &str| { + Request::builder() + .header(header::AUTHORIZATION, value) + .body(axum::body::Body::empty()) + .unwrap() + }; + + assert_eq!(bearer_token(&with_auth("Bearer s3cret")), Some("s3cret")); + assert_eq!(bearer_token(&with_auth("bearer s3cret")), Some("s3cret")); + assert_eq!(bearer_token(&with_auth("Bearer s3cret ")), Some("s3cret")); + assert_eq!(bearer_token(&with_auth("Basic s3cret")), None); + assert_eq!(bearer_token(&with_auth("Bearer")), None); + assert_eq!(bearer_token(&with_auth("Bearer ")), None); + assert_eq!( + bearer_token(&Request::builder().body(axum::body::Body::empty()).unwrap()), + None + ); + } } diff --git a/crates/malachite-app/src/rpc/routes.rs b/crates/malachite-app/src/rpc/routes.rs index 16a4f8c..8edc394 100644 --- a/crates/malachite-app/src/rpc/routes.rs +++ b/crates/malachite-app/src/rpc/routes.rs @@ -25,7 +25,9 @@ use serde_json::json; use tokio::net::{TcpListener, ToSocketAddrs}; use tracing::{error, info}; -use super::middleware::extract_version; +use arc_consensus_types::AdminToken; + +use super::middleware::{extract_version, require_admin_token}; use super::types::{EndpointInfo, RpcState, TxConsensusReq, TxNetworkReq}; use super::version::ApiVersion; use crate::request::TxAppReq; @@ -105,19 +107,21 @@ routes![ "Get the current network state (peers, topics, scores)" ), route!( + admin, post, "/persistent-peers", crate::rpc::handlers::add_persistent_peer, - "Add a persistent peer at runtime.", + "Add a persistent peer at runtime. Requires the admin bearer token.", params = { "body" => "JSON object with \"addr\" (string): multiaddr of the peer, e.g. \"/ip4/127.0.0.1/tcp/26656/p2p/12D3KooW...\"." } ), route!( + admin, delete, "/persistent-peers", crate::rpc::handlers::remove_persistent_peer, - "Remove a persistent peer at runtime.", + "Remove a persistent peer at runtime. Requires the admin bearer token.", params = { "body" => "JSON object with \"addr\" (string): multiaddr of the peer to remove, e.g. \"/ip4/127.0.0.1/tcp/26656/p2p/12D3KooW...\"." } @@ -130,8 +134,17 @@ pub async fn serve( tx_consensus_req: TxConsensusReq, tx_app_req: TxAppReq, tx_network_req: TxNetworkReq, + admin_token: Option, ) { - if let Err(e) = inner(listen_addr, tx_consensus_req, tx_app_req, tx_network_req).await { + if let Err(e) = inner( + listen_addr, + tx_consensus_req, + tx_app_req, + tx_network_req, + admin_token, + ) + .await + { error!("RPC server failed: {e}"); } } @@ -140,10 +153,17 @@ pub async fn serve( /// /// This is exposed publicly for testing purposes, allowing integration tests /// to create a server with the actual production router. +/// +/// Routes marked `admin` change node state at runtime. They are registered only +/// when `admin_token` is set, and then they sit behind +/// [`require_admin_token`](super::middleware::require_admin_token). With no token +/// configured the listener serves read-only monitoring routes and the privileged +/// paths do not exist. pub fn build_router( tx_consensus_req: TxConsensusReq, tx_app_req: TxAppReq, tx_network_req: TxNetworkReq, + admin_token: Option, ) -> Router { let rpc_state = RpcState { tx_consensus_req, @@ -151,15 +171,37 @@ pub fn build_router( tx_network_req, }; - let routes = build_routes(); + let (admin_routes, public_routes): (Vec<_>, Vec<_>) = + build_routes().into_iter().partition(|route| route.admin); let mut router = Router::new(); - for route in &routes { + for route in &public_routes { router = router.route(route.path, (route.handler)()); } - let docs = routes + let admin_enabled = admin_token.is_some(); + if let Some(token) = admin_token { + let mut admin_router = Router::new(); + for route in &admin_routes { + admin_router = admin_router.route(route.path, (route.handler)()); + } + + // route_layer only runs for requests that match one of these routes, so + // unmatched paths still fall through to the public router. + router = router.merge( + admin_router.route_layer(axum::middleware::from_fn_with_state( + token, + require_admin_token, + )), + ); + } else { + info!("RPC admin routes disabled: no --rpc.admin-token-file configured"); + } + + // Document only what this node actually serves. + let docs = public_routes .into_iter() + .chain(admin_routes.into_iter().filter(|_| admin_enabled)) .map(|r| (format!("{} {}", r.method, r.path), r.doc)) .collect::>(); @@ -179,8 +221,9 @@ async fn inner( tx_consensus_req: TxConsensusReq, tx_app_req: TxAppReq, tx_network_req: TxNetworkReq, + admin_token: Option, ) -> Result<()> { - let app = build_router(tx_consensus_req, tx_app_req, tx_network_req); + let app = build_router(tx_consensus_req, tx_app_req, tx_network_req, admin_token); let listener = TcpListener::bind(listen_addr).await?; let address = listener.local_addr()?; @@ -515,6 +558,12 @@ mod tests { serde_json::from_slice(&bytes).unwrap() } + const TEST_ADMIN_TOKEN: &str = "test-admin-token"; + + fn test_admin_token() -> AdminToken { + AdminToken::from_file_contents(TEST_ADMIN_TOKEN).unwrap() + } + async fn build_no_backend_router_and_request(uri: &str) -> (StatusCode, serde_json::Value) { let (tx_dummy_cons_req, _dummy_rx_c) = mpsc::channel::>(1); let (tx_dummy_app_req, _dummy_rx_a) = mpsc::channel::(1); @@ -528,7 +577,7 @@ mod tests { tx_network_req: mpsc::Sender, uri: &str, ) -> (StatusCode, serde_json::Value) { - let app = build_router(tx_consensus_req, tx_app_req, tx_network_req); + let app = build_router(tx_consensus_req, tx_app_req, tx_network_req, None); let req = Request::builder() .method("GET") .uri(uri) @@ -540,6 +589,8 @@ mod tests { (status, val) } + /// Request with a JSON body against a router that serves the admin routes, + /// presenting the configured token. async fn build_router_and_request_with_body( method: &str, tx_consensus_req: mpsc::Sender>, @@ -548,19 +599,89 @@ mod tests { uri: &str, body: serde_json::Value, ) -> (StatusCode, serde_json::Value) { - let app = build_router(tx_consensus_req, tx_app_req, tx_network_req); - let req = Request::builder() + request_with_body( + method, + tx_consensus_req, + tx_app_req, + tx_network_req, + uri, + body, + Some(test_admin_token()), + Some(TEST_ADMIN_TOKEN), + ) + .await + } + + /// Same, with explicit control over the token the node is configured with and + /// the token the caller presents. + #[allow(clippy::too_many_arguments)] + async fn request_with_body( + method: &str, + tx_consensus_req: mpsc::Sender>, + tx_app_req: mpsc::Sender, + tx_network_req: mpsc::Sender, + uri: &str, + body: serde_json::Value, + configured: Option, + presented: Option<&str>, + ) -> (StatusCode, serde_json::Value) { + let app = build_router(tx_consensus_req, tx_app_req, tx_network_req, configured); + let mut builder = Request::builder() .method(method) .uri(uri) - .header("content-type", "application/json") + .header("content-type", "application/json"); + if let Some(token) = presented { + builder = builder.header("authorization", format!("Bearer {token}")); + } + let req = builder .body(Body::from(serde_json::to_vec(&body).unwrap())) .unwrap(); let resp = app.oneshot(req).await.unwrap(); let status = resp.status(); - let val = response_to_json(resp).await; + let val = response_to_json_lenient(resp).await; (status, val) } + /// Like `response_to_json`, but tolerates an empty body. A router that does + /// not know a path answers 404 with no body at all. + async fn response_to_json_lenient(resp: Response) -> serde_json::Value { + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + + if bytes.is_empty() { + return serde_json::Value::Null; + } + + serde_json::from_slice(&bytes).unwrap() + } + + /// Peer-mutation request with no backend listening at all. The receivers are + /// dropped, so a request that reaches a handler fails fast instead of waiting + /// for a reply that will never come. + async fn peer_request_without_backend( + method: &str, + configured: Option, + presented: Option<&str>, + ) -> (StatusCode, serde_json::Value) { + let (tx_cons_req, rx_c) = mpsc::channel::>(1); + let (tx_app_req, rx_a) = mpsc::channel::(1); + let (tx_nw_req, rx_n) = mpsc::channel::(1); + drop((rx_c, rx_a, rx_n)); + + request_with_body( + method, + tx_cons_req, + tx_app_req, + tx_nw_req, + "/persistent-peers", + valid_add_persistent_peer_addr(), + configured, + presented, + ) + .await + } + #[test] fn test_build_routes_contains_expected_paths() { let mut paths: Vec<_> = build_routes().iter().map(|r| r.path).collect(); @@ -642,14 +763,133 @@ mod tests { ); assert_eq!( endpoints["POST /persistent-peers"]["desc"], - "Add a persistent peer at runtime." + "Add a persistent peer at runtime. Requires the admin bearer token." ); assert_eq!( endpoints["DELETE /persistent-peers"]["desc"], - "Remove a persistent peer at runtime." + "Remove a persistent peer at runtime. Requires the admin bearer token." + ); + } + + #[test] + fn test_only_peer_mutation_routes_are_privileged() { + let admin: Vec<_> = build_routes() + .iter() + .filter(|r| r.admin) + .map(|r| format!("{} {}", r.method, r.path)) + .collect(); + assert_eq!( + admin, + vec![ + "POST /persistent-peers".to_string(), + "DELETE /persistent-peers".to_string(), + ] ); } + /// The index of a node with no admin token must not advertise routes it does + /// not serve. + #[tokio::test] + async fn test_index_omits_admin_routes_without_a_token() { + let (_, val) = build_no_backend_router_and_request("/").await; + let endpoints = &val["endpoints"]; + assert!(endpoints.get("POST /persistent-peers").is_none()); + assert!(endpoints.get("DELETE /persistent-peers").is_none()); + assert!(endpoints.get("GET /network-state").is_some()); + } + + #[tokio::test] + async fn test_index_lists_admin_routes_with_a_token() { + let (tx_cons_req, rx_c) = mpsc::channel::>(1); + let (tx_app_req, rx_a) = mpsc::channel::(1); + let (tx_nw_req, rx_n) = mpsc::channel::(1); + drop((rx_c, rx_a, rx_n)); + + let app = build_router(tx_cons_req, tx_app_req, tx_nw_req, Some(test_admin_token())); + let req = Request::builder() + .method("GET") + .uri("/") + .body(Body::empty()) + .unwrap(); + let resp = app.oneshot(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + let val = response_to_json(resp).await; + let endpoints = &val["endpoints"]; + assert!(endpoints.get("POST /persistent-peers").is_some()); + assert!(endpoints.get("DELETE /persistent-peers").is_some()); + } + + #[tokio::test] + async fn test_add_persistent_peer_without_token_is_unauthorized() { + let (status, val) = + peer_request_without_backend("POST", Some(test_admin_token()), None).await; + assert_eq!(status, StatusCode::UNAUTHORIZED); + assert_eq!(val, json!({"error": "Missing bearer token"})); + } + + #[tokio::test] + async fn test_remove_persistent_peer_without_token_is_unauthorized() { + let (status, val) = + peer_request_without_backend("DELETE", Some(test_admin_token()), None).await; + assert_eq!(status, StatusCode::UNAUTHORIZED); + assert_eq!(val, json!({"error": "Missing bearer token"})); + } + + #[tokio::test] + async fn test_add_persistent_peer_with_wrong_token_is_unauthorized() { + let (status, val) = + peer_request_without_backend("POST", Some(test_admin_token()), Some("not-the-token")) + .await; + assert_eq!(status, StatusCode::UNAUTHORIZED); + assert_eq!(val, json!({"error": "Invalid admin token"})); + } + + #[tokio::test] + async fn test_remove_persistent_peer_with_wrong_token_is_unauthorized() { + let (status, val) = + peer_request_without_backend("DELETE", Some(test_admin_token()), Some("not-the-token")) + .await; + assert_eq!(status, StatusCode::UNAUTHORIZED); + assert_eq!(val, json!({"error": "Invalid admin token"})); + } + + /// With no admin token configured the peer-mutation paths are not registered, + /// so they are not there to be called even by a caller that guesses a token. + #[tokio::test] + async fn test_peer_mutation_is_not_served_without_an_admin_token() { + for method in ["POST", "DELETE"] { + let (status, _) = peer_request_without_backend(method, None, None).await; + assert_eq!( + status, + StatusCode::NOT_FOUND, + "{method} /persistent-peers must not be routed without an admin token" + ); + + let (status, _) = + peer_request_without_backend(method, None, Some(TEST_ADMIN_TOKEN)).await; + assert_eq!( + status, + StatusCode::NOT_FOUND, + "{method} /persistent-peers must not be routed without an admin token" + ); + } + } + + /// Read-only routes stay public on a node that has admin routes enabled. + #[tokio::test] + async fn test_public_routes_need_no_admin_token() { + let (tx_cons_req, tx_app_req, tx_nw_req) = + MockBackend::spawn_new(MockConfig::NetworkDumpState(MockValue::Present)); + let app = build_router(tx_cons_req, tx_app_req, tx_nw_req, Some(test_admin_token())); + let req = Request::builder() + .method("GET") + .uri("/network-state") + .body(Body::empty()) + .unwrap(); + let resp = app.oneshot(req).await.unwrap(); + assert_eq!(resp.status(), StatusCode::OK); + } + #[tokio::test] async fn test_version_success() { // '/version' endpoint does not use the backend diff --git a/crates/malachite-app/src/rpc/types.rs b/crates/malachite-app/src/rpc/types.rs index be05ab2..b18b2db 100644 --- a/crates/malachite-app/src/rpc/types.rs +++ b/crates/malachite-app/src/rpc/types.rs @@ -67,6 +67,9 @@ pub(crate) struct RouteDef { pub path: &'static str, pub handler: fn() -> axum::routing::MethodRouter, pub doc: EndpointInfo, + /// Privileged route. It is registered only when an admin token is configured, + /// and then it is only reachable by a caller that presents that token. + pub admin: bool, } macro_rules! method_str { @@ -90,13 +93,16 @@ macro_rules! routes { } macro_rules! route { + (admin, $method:ident, $path:expr, $handler_fn:path, $desc:expr, params = { $( $pkey:expr => $pval:expr ),* $(,)? }) => { + route!(@build $method, $path, $handler_fn, $desc, Some(::std::collections::BTreeMap::from([ $( ($pkey, $pval) ),* ])), true) + }; ($method:ident, $path:expr, $handler_fn:path, $desc:expr) => { - route!(@build $method, $path, $handler_fn, $desc, None) + route!(@build $method, $path, $handler_fn, $desc, None, false) }; ($method:ident, $path:expr, $handler_fn:path, $desc:expr, params = { $( $pkey:expr => $pval:expr ),* $(,)? }) => { - route!(@build $method, $path, $handler_fn, $desc, Some(::std::collections::BTreeMap::from([ $( ($pkey, $pval) ),* ]))) + route!(@build $method, $path, $handler_fn, $desc, Some(::std::collections::BTreeMap::from([ $( ($pkey, $pval) ),* ])), false) }; - (@build $method:ident, $path:expr, $handler_fn:path, $desc:expr, $params:expr) => { + (@build $method:ident, $path:expr, $handler_fn:path, $desc:expr, $params:expr, $admin:expr) => { crate::rpc::types::RouteDef { method: method_str!($method), path: $path, @@ -105,6 +111,7 @@ macro_rules! route { desc: $desc, params: $params, }, + admin: $admin, } }; } diff --git a/crates/malachite-app/tests/common/mod.rs b/crates/malachite-app/tests/common/mod.rs index 29c500e..8d565f4 100644 --- a/crates/malachite-app/tests/common/mod.rs +++ b/crates/malachite-app/tests/common/mod.rs @@ -19,7 +19,7 @@ use std::net::SocketAddr; use std::sync::Arc; -use arc_consensus_types::ArcContext; +use arc_consensus_types::{AdminToken, ArcContext}; use arc_node_consensus::request::AppRequest; use malachitebft_app_channel::{ConsensusRequest, NetworkRequest}; use tokio::net::TcpListener; @@ -43,6 +43,17 @@ impl TestServer { /// Start a new test server with specified channel capacity pub async fn start_with_capacity(capacity: usize) -> Self { + Self::start_with(capacity, None).await + } + + /// Start a server that serves the privileged routes behind `token`, the way a + /// node started with `--rpc.admin-token-file` does. + pub async fn start_with_admin_token(token: &str) -> Self { + let token = AdminToken::from_file_contents(token).expect("valid admin token"); + Self::start_with(100, Some(token)).await + } + + async fn start_with(capacity: usize, admin_token: Option) -> Self { // Create channels for communication let (app_tx, app_rx) = mpsc::channel(capacity); let (consensus_tx, consensus_rx) = mpsc::channel(capacity); @@ -55,7 +66,8 @@ impl TestServer { let addr = listener.local_addr().expect("Failed to get local address"); // Build the actual production router - let router = arc_node_consensus::rpc::build_router(consensus_tx, app_tx, network_tx); + let router = + arc_node_consensus::rpc::build_router(consensus_tx, app_tx, network_tx, admin_token); // Spawn the server let server_handle = tokio::spawn(async move { diff --git a/crates/malachite-app/tests/rpc_integration.rs b/crates/malachite-app/tests/rpc_integration.rs index 498ad5e..367cb00 100644 --- a/crates/malachite-app/tests/rpc_integration.rs +++ b/crates/malachite-app/tests/rpc_integration.rs @@ -500,3 +500,149 @@ async fn test_no_misbehavior_evidence_endpoint_without_height() { assert_eq!(response.status(), 404); // Not found since we returned None } + +const ADMIN_TOKEN: &str = "integration-admin-token"; + +fn a_peer_addr() -> serde_json::Value { + serde_json::json!({ + "addr": "/ip4/127.0.0.1/tcp/26656/p2p/QmNnooDu7bfjPFoTZYxMNLWUQJyrVwtbZg5gBMjTezGAJN" + }) +} + +/// An unauthenticated caller must not be able to add a persistent peer on a node +/// that serves the privileged routes. +#[tokio::test] +async fn test_add_persistent_peer_unauthenticated_is_rejected() { + let server = TestServer::start_with_admin_token(ADMIN_TOKEN).await; + let client = reqwest::Client::new(); + + let response = client + .post(format!("{}/persistent-peers", server.url())) + .json(&a_peer_addr()) + .send() + .await + .expect("Failed to send request"); + + assert_eq!(response.status(), 401); + assert_eq!( + response + .headers() + .get("www-authenticate") + .and_then(|v| v.to_str().ok()), + Some("Bearer") + ); + + let body: serde_json::Value = response.json().await.expect("Failed to parse JSON"); + assert_eq!(body.get("error").unwrap(), "Missing bearer token"); +} + +/// Same for removing a peer, which is the direction that can partition a node. +#[tokio::test] +async fn test_remove_persistent_peer_unauthenticated_is_rejected() { + let server = TestServer::start_with_admin_token(ADMIN_TOKEN).await; + let client = reqwest::Client::new(); + + let response = client + .delete(format!("{}/persistent-peers", server.url())) + .json(&a_peer_addr()) + .send() + .await + .expect("Failed to send request"); + + assert_eq!(response.status(), 401); +} + +#[tokio::test] +async fn test_persistent_peer_with_wrong_token_is_rejected() { + let server = TestServer::start_with_admin_token(ADMIN_TOKEN).await; + let client = reqwest::Client::new(); + + let response = client + .post(format!("{}/persistent-peers", server.url())) + .bearer_auth("guessed-token") + .json(&a_peer_addr()) + .send() + .await + .expect("Failed to send request"); + + assert_eq!(response.status(), 401); + let body: serde_json::Value = response.json().await.expect("Failed to parse JSON"); + assert_eq!(body.get("error").unwrap(), "Invalid admin token"); +} + +/// The operator's own request still works, and the multiaddr still reaches the +/// networking layer. +#[tokio::test] +async fn test_add_persistent_peer_with_admin_token_succeeds() { + let server = TestServer::start_with_admin_token(ADMIN_TOKEN).await; + let client = reqwest::Client::new(); + + server.expect_network_request(|req| match req { + NetworkRequest::UpdatePersistentPeers(_, reply) => { + reply.send(Ok(())).ok(); + } + _ => panic!("Unexpected request type"), + }); + + let response = client + .post(format!("{}/persistent-peers", server.url())) + .bearer_auth(ADMIN_TOKEN) + .json(&a_peer_addr()) + .send() + .await + .expect("Failed to send request"); + + assert_eq!(response.status(), 200); + let body: serde_json::Value = response.json().await.expect("Failed to parse JSON"); + assert_eq!(body.get("status").unwrap(), "ok"); +} + +#[tokio::test] +async fn test_remove_persistent_peer_with_admin_token_succeeds() { + let server = TestServer::start_with_admin_token(ADMIN_TOKEN).await; + let client = reqwest::Client::new(); + + server.expect_network_request(|req| match req { + NetworkRequest::UpdatePersistentPeers(_, reply) => { + reply.send(Ok(())).ok(); + } + _ => panic!("Unexpected request type"), + }); + + let response = client + .delete(format!("{}/persistent-peers", server.url())) + .bearer_auth(ADMIN_TOKEN) + .json(&a_peer_addr()) + .send() + .await + .expect("Failed to send request"); + + assert_eq!(response.status(), 200); +} + +/// A node started without `--rpc.admin-token-file` does not route the peer +/// mutation methods at all. +#[tokio::test] +async fn test_persistent_peers_absent_without_admin_token() { + let server = TestServer::start().await; + let client = reqwest::Client::new(); + + let response = client + .post(format!("{}/persistent-peers", server.url())) + .bearer_auth(ADMIN_TOKEN) + .json(&a_peer_addr()) + .send() + .await + .expect("Failed to send request"); + + assert_eq!(response.status(), 404); + + let response = client + .delete(format!("{}/persistent-peers", server.url())) + .json(&a_peer_addr()) + .send() + .await + .expect("Failed to send request"); + + assert_eq!(response.status(), 404); +} diff --git a/crates/malachite-cli/src/cmd/start.rs b/crates/malachite-cli/src/cmd/start.rs index 8ac42a9..20a7080 100644 --- a/crates/malachite-cli/src/cmd/start.rs +++ b/crates/malachite-cli/src/cmd/start.rs @@ -276,11 +276,28 @@ pub struct StartCmd { /// If omitted, RPC is disabled. /// If provided, RPC is enabled on the given address. /// - /// Example: 0.0.0.0:31000 + /// The CL RPC port is an internal interface. Bind it to loopback or to a + /// private interface, and firewall it off from the public internet. + /// + /// Example: 127.0.0.1:31000 #[clap(long = "rpc.addr", value_name = "ADDR")] #[serde(skip)] pub rpc_addr: Option, + /// Path to a file holding the bearer token that the privileged RPC routes + /// require. Those are the routes that change node state at runtime: adding + /// and removing persistent peers. + /// + /// If omitted, the privileged routes are not served at all and the RPC + /// listener answers read-only monitoring requests only. + /// + /// Generate a token with: openssl rand -hex 32 > admin-token + /// + /// Example: /etc/arc/rpc-admin-token + #[clap(long = "rpc.admin-token-file", value_name = "PATH")] + #[serde(skip)] + pub rpc_admin_token_file: Option, + // ===== Runtime ===== /// Tokio runtime flavor to use. #[clap( @@ -511,6 +528,7 @@ impl Default for StartCmd { execution_jwt: None, metrics: None, rpc_addr: None, + rpc_admin_token_file: None, runtime_flavor: RUNTIME_MULTI_THREADED.to_string(), worker_threads: None, full: false, @@ -626,6 +644,9 @@ impl StartCmd { push_if_some!("execution-jwt", self.execution_jwt); push_if_some!("metrics", self.metrics); push_if_some!("rpc.addr", self.rpc_addr); + if let Some(ref path) = self.rpc_admin_token_file { + flags.push(format!("--rpc.admin-token-file={}", path.display())); + } push_if!("full", self.full); push_if!("minimal", self.minimal); if let Some(ref path) = self.private_key { @@ -1283,6 +1304,38 @@ mod tests { let cmd = new_start_cmd(); assert_eq!(cmd.metrics, None); assert_eq!(cmd.rpc_addr, None); + assert_eq!(cmd.rpc_admin_token_file, None); + } + + #[test] + fn rpc_admin_token_file_flag_parses_a_path() { + let args = vec![ + "arc-node-consensus", + "--moniker", + "test", + "--p2p.addr", + "/ip4/127.0.0.1/tcp/27000", + "--rpc.addr", + "127.0.0.1:31000", + "--rpc.admin-token-file", + "/etc/arc/rpc-admin-token", + ]; + let cmd = StartCmd::try_parse_from(args).unwrap(); + assert_eq!( + cmd.rpc_admin_token_file, + Some(PathBuf::from("/etc/arc/rpc-admin-token")) + ); + } + + #[test] + fn rpc_admin_token_file_round_trips_through_flags() { + let cmd = StartCmd { + rpc_admin_token_file: Some(PathBuf::from("/etc/arc/rpc-admin-token")), + ..new_start_cmd() + }; + assert!(cmd + .to_cli_flags() + .contains(&"--rpc.admin-token-file=/etc/arc/rpc-admin-token".to_string())); } // Pruning tests @@ -1603,6 +1656,7 @@ mod tests { execution_jwt: None, metrics: Some("127.0.0.1:9000".parse().unwrap()), rpc_addr: Some("127.0.0.1:31000".parse().unwrap()), + rpc_admin_token_file: Some(PathBuf::from("/etc/arc/rpc-admin-token")), runtime_flavor: RUNTIME_SINGLE_THREADED.to_string(), worker_threads: Some(8), full: false, diff --git a/crates/quake/src/setup.rs b/crates/quake/src/setup.rs index 3493738..d9b082f 100644 --- a/crates/quake/src/setup.rs +++ b/crates/quake/src/setup.rs @@ -648,6 +648,8 @@ fn generate_legacy_consensus_config( listen_addr: format!("0.0.0.0:{APP_RPC_DEFAULT_PORT}") .parse() .context("failed to parse RPC listen address")?, + // Local devnet: no privileged RPC routes are served. + admin_token: None, }, signing: if node.remote_signer.is_some() { SigningConfig::Remote(RemoteSigningConfig { diff --git a/crates/test/integration/src/runner.rs b/crates/test/integration/src/runner.rs index 97cf00d..2a3093b 100644 --- a/crates/test/integration/src/runner.rs +++ b/crates/test/integration/src/runner.rs @@ -607,6 +607,7 @@ fn build_node_consensus_config( rpc: arc_consensus_types::RpcConfig { enabled: true, listen_addr: rpc_listen_addr, + admin_token: None, }, ..Default::default() }; diff --git a/crates/types/src/config.rs b/crates/types/src/config.rs index 5167eda..cff2b19 100644 --- a/crates/types/src/config.rs +++ b/crates/types/src/config.rs @@ -160,6 +160,15 @@ pub struct RpcConfig { /// Address to bind the RPC server to pub listen_addr: SocketAddr, + + /// Bearer token required by the privileged RPC routes. + /// + /// Set from `--rpc.admin-token-file`. While it is `None` the routes that + /// mutate node state are not registered at all, so the listener serves only + /// the read-only monitoring endpoints. It is never read from or written to a + /// config file, so the credential lives in the token file alone. + #[serde(skip)] + pub admin_token: Option, } impl Default for RpcConfig { @@ -169,8 +178,65 @@ impl Default for RpcConfig { listen_addr: format!("127.0.0.1:{RPC_BASE_PORT}") .parse() .expect("valid socket address"), + admin_token: None, + } + } +} + +/// Bearer credential a caller must present to reach the privileged RPC routes. +/// +/// `Debug` is redacted, so dumping the configuration (`trace!(?config)`) cannot +/// leak the token, and comparisons do not exit early on the first wrong byte. +#[derive(Clone)] +pub struct AdminToken(String); + +impl AdminToken { + /// Build a token from the contents of an admin token file. + /// + /// Surrounding whitespace is trimmed, which is what an operator gets from + /// `openssl rand -hex 32 > token`. An empty file is rejected rather than + /// accepted as an empty credential. + pub fn from_file_contents(contents: &str) -> eyre::Result { + let token = contents.trim(); + + if token.is_empty() { + bail!("admin token file is empty"); } + + Ok(Self(token.to_owned())) + } + + /// Whether a presented credential matches this token. + pub fn matches(&self, presented: &str) -> bool { + bytes_eq_no_early_exit(self.0.as_bytes(), presented.as_bytes()) + } +} + +impl std::fmt::Debug for AdminToken { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("AdminToken(redacted)") + } +} + +impl PartialEq for AdminToken { + fn eq(&self, other: &Self) -> bool { + bytes_eq_no_early_exit(self.0.as_bytes(), other.0.as_bytes()) + } +} + +/// Compare two byte strings without returning early on the first difference, so +/// the count of matching leading bytes does not show up in the response time. +fn bytes_eq_no_early_exit(a: &[u8], b: &[u8]) -> bool { + if a.len() != b.len() { + return false; + } + + let mut diff = 0u8; + for (x, y) in a.iter().zip(b.iter()) { + diff |= x ^ y; } + + diff == 0 } /// Execution-layer tuning parameters. @@ -268,6 +334,65 @@ impl Default for RetryConfig { mod tests { use super::*; + mod admin_token { + use super::AdminToken; + use crate::config::RpcConfig; + + #[test] + fn trims_surrounding_whitespace() { + let token = AdminToken::from_file_contents(" s3cret\n").unwrap(); + assert!(token.matches("s3cret")); + assert!(!token.matches(" s3cret\n")); + } + + #[test] + fn rejects_an_empty_file() { + assert!(AdminToken::from_file_contents("").is_err()); + assert!(AdminToken::from_file_contents(" \n\t").is_err()); + } + + #[test] + fn does_not_match_a_prefix_or_a_different_token() { + let token = AdminToken::from_file_contents("s3cret").unwrap(); + assert!(!token.matches("s3cre")); + assert!(!token.matches("s3cretx")); + assert!(!token.matches("")); + assert!(!token.matches("S3CRET")); + } + + #[test] + fn debug_output_is_redacted() { + let token = AdminToken::from_file_contents("s3cret").unwrap(); + let rendered = format!("{token:?}"); + assert!(!rendered.contains("s3cret"), "rendered: {rendered}"); + + let config = RpcConfig { + admin_token: Some(token), + ..RpcConfig::default() + }; + let rendered = format!("{config:?}"); + assert!(!rendered.contains("s3cret"), "rendered: {rendered}"); + } + + #[test] + fn is_never_serialised_into_a_config_file() { + let config = RpcConfig { + admin_token: Some(AdminToken::from_file_contents("s3cret").unwrap()), + ..RpcConfig::default() + }; + + let serialised = serde_json::to_string(&config).unwrap(); + assert!(!serialised.contains("s3cret"), "serialised: {serialised}"); + assert!( + !serialised.contains("admin_token"), + "serialised: {serialised}" + ); + + let round_tripped: RpcConfig = serde_json::from_str(&serialised).unwrap(); + assert_eq!(round_tripped.admin_token, None); + } + } + mod pruning { use super::Config; use crate::config::PruningConfig; diff --git a/docs/running-an-arc-node.md b/docs/running-an-arc-node.md index 93f04f9..eed81f0 100644 --- a/docs/running-an-arc-node.md +++ b/docs/running-an-arc-node.md @@ -579,6 +579,19 @@ EL: --rpc.addr=127.0.0.1:31000 ``` +The CL RPC listener serves read-only monitoring routes. The routes that change +the node's persistent peer set at runtime (`POST` and `DELETE +/persistent-peers`) are not served unless you point the CL at a token file: + +```bash +--rpc.admin-token-file=/etc/arc/rpc-admin-token +``` + +Requests to those routes must then carry `Authorization: Bearer `. +Generate the token with `openssl rand -hex 32 > /etc/arc/rpc-admin-token` and +keep it readable only by the node user. Leave the flag off if you do not manage +peers over RPC. + #### Sync-only mode By default the CL participates in the consensus protocol. To run a node that