From 1d749ed33d6ca4e6e69f9d833405543f0d8829fa Mon Sep 17 00:00:00 2001 From: Bryan Stitt Date: Fri, 3 Feb 2023 10:56:05 -0800 Subject: [PATCH] block all admin_ commands --- TODO.md | 1 + web3_proxy/src/app/mod.rs | 47 +++++++++++++++++-------------- web3_proxy/src/frontend/errors.rs | 26 ++++++++--------- 3 files changed, 40 insertions(+), 34 deletions(-) diff --git a/TODO.md b/TODO.md index 6a1d8a58..53849742 100644 --- a/TODO.md +++ b/TODO.md @@ -327,6 +327,7 @@ These are not yet ordered. There might be duplicates. We might not actually need - [x] short lived cache on /health - [x] cache /status for longer - [x] sort connections during eth_sendRawTransaction +- [x] block all admin_ rpc commands - [-] proxy mode for benchmarking all backends - [-] proxy mode for sending to multiple backends - [-] let users choose a % of reverts to log (or maybe x/second). someone like curve logging all reverts will be a BIG database very quickly diff --git a/web3_proxy/src/app/mod.rs b/web3_proxy/src/app/mod.rs index 80561eee..d26bd243 100644 --- a/web3_proxy/src/app/mod.rs +++ b/web3_proxy/src/app/mod.rs @@ -212,7 +212,7 @@ pub struct Web3ProxyApp { pub db_conn: Option, pub db_replica: Option, /// prometheus metrics - app_metrics: Arc, + // app_metrics: Arc, open_request_handle_metrics: Arc, /// store pending transactions that we've seen so that we don't send duplicates to subscribers pub pending_transactions: Cache, @@ -365,7 +365,7 @@ pub struct Web3ProxyAppSpawn { pub background_handles: FuturesUnordered>, } -#[metered(registry = Web3ProxyAppMetrics, registry_expr = self.app_metrics, visibility = pub)] +// #[metered(registry = Web3ProxyAppMetrics, registry_expr = self.app_metrics, visibility = pub)] impl Web3ProxyApp { /// The main entrypoint. pub async fn spawn( @@ -396,7 +396,7 @@ impl Web3ProxyApp { } // setup metrics - let app_metrics = Default::default(); + // let app_metrics = Default::default(); let open_request_handle_metrics: Arc = Default::default(); let mut db_conn = None::; @@ -736,7 +736,7 @@ impl Web3ProxyApp { db_conn, db_replica, vredis_pool, - app_metrics, + // app_metrics, open_request_handle_metrics, rpc_secret_key_cache, bearer_token_semaphores, @@ -912,7 +912,7 @@ impl Web3ProxyApp { #[derive(Serialize)] struct CombinedMetrics<'a> { - app: &'a Web3ProxyAppMetrics, + // app: &'a Web3ProxyAppMetrics, backend_rpc: &'a OpenRequestHandleMetrics, recent_ip_counts: RecentCounts, recent_user_id_counts: RecentCounts, @@ -921,7 +921,7 @@ impl Web3ProxyApp { } let metrics = CombinedMetrics { - app: &self.app_metrics, + // app: &self.app_metrics, backend_rpc: &self.open_request_handle_metrics, recent_ip_counts, recent_user_id_counts, @@ -979,7 +979,8 @@ impl Web3ProxyApp { authorization: &Arc, requests: Vec, proxy_mode: ProxyMode, - ) -> anyhow::Result<(Vec, Vec>)> { + ) -> Result<(Vec, Vec>), FrontendErrorResponse> + { // TODO: we should probably change ethers-rs to support this directly. they pushed this off to v2 though let num_requests = requests.len(); @@ -1031,13 +1032,13 @@ impl Web3ProxyApp { } } - #[measure([ErrorCount, HitCount, ResponseTime, Throughput])] + // #[measure([ErrorCount, HitCount, ResponseTime, Throughput])] async fn proxy_cached_request( self: &Arc, authorization: &Arc, mut request: JsonRpcRequest, proxy_mode: ProxyMode, - ) -> anyhow::Result<(JsonRpcForwardedResponse, Vec>)> { + ) -> Result<(JsonRpcForwardedResponse, Vec>), FrontendErrorResponse> { // trace!("Received request: {:?}", request); let request_metadata = Arc::new(RequestMetadata::new(REQUEST_PERIOD, request.num_bytes())?); @@ -1051,13 +1052,7 @@ impl Web3ProxyApp { // TODO: don't clone? let partial_response: serde_json::Value = match request_method.as_ref() { // lots of commands are blocked - method @ ("admin_addPeer" - | "admin_datadir" - | "admin_startRPC" - | "admin_startWS" - | "admin_stopRPC" - | "admin_stopWS" - | "db_getHex" + method @ ("db_getHex" | "db_getString" | "db_putHex" | "db_putString" @@ -1157,9 +1152,10 @@ impl Web3ProxyApp { } None => { // TODO: what does geth do if this happens? - return Err(anyhow::anyhow!( - "no servers synced. unknown eth_blockNumber" - )); + // TODO: i think we want a 502 so that haproxy retries on another server + return Err( + anyhow::anyhow!("no servers synced. unknown eth_blockNumber").into(), + ); } } } @@ -1395,12 +1391,16 @@ impl Web3ProxyApp { )); } - // TODO: don't return with ? here. send a jsonrpc invalid request let param = Bytes::from_str( params[0] .as_str() .context("parsing params 0 into str then bytes")?, - )?; + ) + .map_err(|x| { + FrontendErrorResponse::BadRequest( + "param 0 could not be read as H256".to_string(), + ) + })?; let hash = H256::from(keccak256(param)); @@ -1432,6 +1432,11 @@ impl Web3ProxyApp { } // anything else gets sent to backend rpcs and cached method => { + if method.starts_with("admin_") { + // TODO: emit a stat? will probably just be noise + return Err(FrontendErrorResponse::AccessDenied); + } + // emit stats // TODO: if no servers synced, wait for them to be synced? probably better to error and let haproxy retry another server diff --git a/web3_proxy/src/frontend/errors.rs b/web3_proxy/src/frontend/errors.rs index 22f048ee..162bf255 100644 --- a/web3_proxy/src/frontend/errors.rs +++ b/web3_proxy/src/frontend/errors.rs @@ -11,7 +11,7 @@ use axum::{ use derive_more::From; use http::header::InvalidHeaderValue; use ipnet::AddrParseError; -use log::{trace, warn}; +use log::{debug, trace, warn}; use migration::sea_orm::DbErr; use redis_rate_limiter::redis::RedisError; use reqwest::header::ToStrError; @@ -25,6 +25,7 @@ pub type FrontendResult = Result; pub enum FrontendErrorResponse { AccessDenied, Anyhow(anyhow::Error), + BadRequest(String), SemaphoreAcquireError(AcquireError), Database(DbErr), HeadersError(headers::Error), @@ -71,18 +72,17 @@ impl FrontendErrorResponse { ), ) } - // Self::(err) => { - // warn!("boxed err={:?}", err); - // ( - // StatusCode::INTERNAL_SERVER_ERROR, - // JsonRpcForwardedResponse::from_str( - // // TODO: make this better. maybe include the error type? - // "boxed error!", - // Some(StatusCode::INTERNAL_SERVER_ERROR.as_u16().into()), - // None, - // ), - // ) - // } + Self::BadRequest(err) => { + debug!("BAD_REQUEST: {}", err); + ( + StatusCode::BAD_REQUEST, + JsonRpcForwardedResponse::from_str( + &format!("bad request: {}", err), + Some(StatusCode::BAD_REQUEST.as_u16().into()), + None, + ), + ) + } Self::Database(err) => { warn!("database err={:?}", err); (