From 28956a17438b0f5bb3309b056c65688528d8ec38 Mon Sep 17 00:00:00 2001 From: zkasuran <289388318+zkasuran@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:00:57 +0530 Subject: [PATCH 1/3] wip(rpc): AdminToken credential type on RpcConfig Redacted Debug, never serialised into a config file, comparison without an early exit. Wiring follows in the next commit. --- crates/types/src/config.rs | 122 +++++++++++++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/crates/types/src/config.rs b/crates/types/src/config.rs index 5167eda..b12d1a3 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,10 +178,67 @@ 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. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct ExecutionConfig { @@ -268,6 +334,62 @@ 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; From f6c1de35459337250dc1911fca7de0487b364c3f Mon Sep 17 00:00:00 2001 From: zkasuran <289388318+zkasuran@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:19:31 +0530 Subject: [PATCH 2/3] fix(rpc): require an admin token for runtime peer mutation POST and DELETE /persistent-peers changed the node's persistent peer set with no caller identity of any kind. build_router applied only extract_version, so any client that could reach the RPC listener could add its own peer or remove the node's configured peers. The operator guide tells operators to keep the CL RPC port internal, but crates/malachite-app/README.md shows --rpc.addr=0.0.0.0:31000 in two validator examples and the CLI flag's own doc comment used the same address, so the reachable case is documented rather than exotic. Route definitions now carry an `admin` flag. Admin routes are registered only when --rpc.admin-token-file is set, and then they sit behind a route_layer that requires Authorization: Bearer . With no token configured the peer mutation paths are not routed at all and the index does not advertise them, so a node has to opt in before it can be told to change its peer set. Read-only monitoring routes are untouched in both modes. The credential is a small AdminToken type: redacted Debug so trace!(?config) cannot leak it, skipped by serde so it never lands in a config file, and compared without an early exit. An unreadable or empty token file is a startup error rather than a silently open RPC surface. Binding the RPC listener to a non-loopback address now warns at startup. --- crates/malachite-app/src/main.rs | 38 ++- crates/malachite-app/src/node.rs | 2 + crates/malachite-app/src/rpc/middleware.rs | 84 +++++- crates/malachite-app/src/rpc/routes.rs | 267 +++++++++++++++++- crates/malachite-app/src/rpc/types.rs | 13 +- crates/malachite-app/tests/common/mod.rs | 16 +- crates/malachite-app/tests/rpc_integration.rs | 146 ++++++++++ crates/malachite-cli/src/cmd/start.rs | 56 +++- crates/quake/src/setup.rs | 2 + crates/test/integration/src/runner.rs | 1 + 10 files changed, 599 insertions(+), 26 deletions(-) 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..e867ec4 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,34 @@ 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 +218,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 +555,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 +574,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 +586,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 +596,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 +760,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() }; From 8f8ea0f33af1385e8c63655b9941b0bdf82ea998 Mon Sep 17 00:00:00 2001 From: zkasuran <289388318+zkasuran@users.noreply.github.com> Date: Sun, 2 Aug 2026 15:23:04 +0530 Subject: [PATCH 3/3] docs(rpc): document the admin token and the public versus privileged split crates/malachite-app/README.md gains the --rpc.admin-token-file flag, a section naming the security boundary of the CL RPC listener, the two privileged routes in the endpoint list and a worked curl example. The two validator examples bound the CL RPC to 0.0.0.0:31000 while docs/running-an-arc-node.md says port 31000 must never be exposed; they now bind loopback or the node's private interface, which is what those examples already use for every other address. The operator guide gains the flag next to the --rpc.addr requirement it belongs with. --- crates/malachite-app/README.md | 37 +++++++++++++++++++++++--- crates/malachite-app/src/rpc/routes.rs | 9 ++++--- crates/types/src/config.rs | 5 +++- docs/running-an-arc-node.md | 13 +++++++++ 4 files changed, 56 insertions(+), 8 deletions(-) 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/rpc/routes.rs b/crates/malachite-app/src/rpc/routes.rs index e867ec4..8edc394 100644 --- a/crates/malachite-app/src/rpc/routes.rs +++ b/crates/malachite-app/src/rpc/routes.rs @@ -188,9 +188,12 @@ pub fn build_router( // 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), - )); + 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"); } diff --git a/crates/types/src/config.rs b/crates/types/src/config.rs index b12d1a3..cff2b19 100644 --- a/crates/types/src/config.rs +++ b/crates/types/src/config.rs @@ -383,7 +383,10 @@ mod tests { let serialised = serde_json::to_string(&config).unwrap(); assert!(!serialised.contains("s3cret"), "serialised: {serialised}"); - assert!(!serialised.contains("admin_token"), "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); 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