diff --git a/substrate/client/cli/src/arg_enums.rs b/substrate/client/cli/src/arg_enums.rs index f0eeca4c8b..c146a16886 100644 --- a/substrate/client/cli/src/arg_enums.rs +++ b/substrate/client/cli/src/arg_enums.rs @@ -122,6 +122,31 @@ impl ExecutionStrategy { } } +arg_enum! { + /// Available RPC methods. + #[allow(missing_docs)] + #[derive(Debug, Copy, Clone, PartialEq)] + pub enum RpcMethods { + // Expose every RPC method only when RPC is listening on `localhost`, + // otherwise serve only safe RPC methods. + Auto, + // Allow only a safe subset of RPC methods. + Safe, + // Expose every RPC method (even potentially unsafe ones). + Unsafe, + } +} + +impl Into for RpcMethods { + fn into(self) -> sc_service::config::RpcMethods { + match self { + RpcMethods::Auto => sc_service::config::RpcMethods::Auto, + RpcMethods::Safe => sc_service::config::RpcMethods::Safe, + RpcMethods::Unsafe => sc_service::config::RpcMethods::Unsafe, + } + } +} + arg_enum! { /// Database backend #[allow(missing_docs)] diff --git a/substrate/client/cli/src/commands/mod.rs b/substrate/client/cli/src/commands/mod.rs index 5312336c76..d2bab5bca0 100644 --- a/substrate/client/cli/src/commands/mod.rs +++ b/substrate/client/cli/src/commands/mod.rs @@ -296,9 +296,9 @@ macro_rules! substrate_cli_subcommands { } } - fn unsafe_rpc_expose(&self) -> $crate::Result { + fn rpc_methods(&self) -> $crate::Result { match self { - $($enum::$variant(cmd) => cmd.unsafe_rpc_expose()),* + $($enum::$variant(cmd) => cmd.rpc_methods()),* } } diff --git a/substrate/client/cli/src/commands/run_cmd.rs b/substrate/client/cli/src/commands/run_cmd.rs index 3365c7a27a..f7cec61df1 100644 --- a/substrate/client/cli/src/commands/run_cmd.rs +++ b/substrate/client/cli/src/commands/run_cmd.rs @@ -14,6 +14,7 @@ // You should have received a copy of the GNU General Public License // along with Substrate. If not, see . +use crate::arg_enums::RpcMethods; use crate::error::{Error, Result}; use crate::params::ImportParams; use crate::params::KeystoreParams; @@ -80,16 +81,24 @@ pub struct RunCmd { /// Listen to all RPC interfaces. /// /// Same as `--rpc-external`. - #[structopt(long = "unsafe-rpc-external")] + #[structopt(long)] pub unsafe_rpc_external: bool, - /// Don't deny potentially unsafe RPCs when listening on external interfaces. + /// RPC methods to expose. /// - /// Default is false. This allows exposing RPC methods publicly (same as `--unsafe-{rpc,ws}-external` ) - /// but will allow doing so even on validator nodes, which is prohibited by default. - /// Please do this if you know what you're doing. - #[structopt(long = "unsafe-rpc-expose")] - pub unsafe_rpc_expose: bool, + /// - `Unsafe`: Exposes every RPC method. + /// - `Safe`: Exposes only a safe subset of RPC methods, denying unsafe RPC methods. + /// - `Auto`: Acts as `Safe` if RPC is served externally, e.g. when `--{rpc,ws}-external` is passed, + /// otherwise acts as `Unsafe`. + #[structopt( + long, + value_name = "METHOD SET", + possible_values = &RpcMethods::variants(), + case_insensitive = true, + default_value = "Auto", + verbatim_doc_comment + )] + pub rpc_methods: RpcMethods, /// Listen to all Websocket interfaces. /// @@ -406,7 +415,7 @@ impl CliConfiguration for RunCmd { let interface = rpc_interface( self.rpc_external, self.unsafe_rpc_external, - self.unsafe_rpc_expose, + self.rpc_methods, self.validator )?; @@ -417,15 +426,15 @@ impl CliConfiguration for RunCmd { let interface = rpc_interface( self.ws_external, self.unsafe_ws_external, - self.unsafe_rpc_expose, + self.rpc_methods, self.validator )?; Ok(Some(SocketAddr::new(interface, self.ws_port.unwrap_or(9944)))) } - fn unsafe_rpc_expose(&self) -> Result { - Ok(self.unsafe_rpc_expose) + fn rpc_methods(&self) -> Result { + Ok(self.rpc_methods.into()) } fn transaction_pool(&self) -> Result { @@ -462,23 +471,26 @@ pub fn is_node_name_valid(_name: &str) -> std::result::Result<(), &str> { fn rpc_interface( is_external: bool, is_unsafe_external: bool, - is_unsafe_rpc_expose: bool, + rpc_methods: RpcMethods, is_validator: bool, ) -> Result { - if is_external && is_validator && !is_unsafe_rpc_expose { + if is_external && is_validator && rpc_methods != RpcMethods::Unsafe { return Err(Error::Input( "--rpc-external and --ws-external options shouldn't be \ - used if the node is running as a validator. Use `--unsafe-rpc-external` if you understand \ - the risks. See the options description for more information." + used if the node is running as a validator. Use `--unsafe-rpc-external` \ + or `--rpc-methods=unsafe` if you understand the risks. See the options \ + description for more information." .to_owned(), )); } if is_external || is_unsafe_external { - log::warn!( - "It isn't safe to expose RPC publicly without a proxy server that filters \ - available set of RPC methods." - ); + if rpc_methods == RpcMethods::Unsafe { + log::warn!( + "It isn't safe to expose RPC publicly without a proxy server that filters \ + available set of RPC methods." + ); + } Ok(Ipv4Addr::UNSPECIFIED.into()) } else { diff --git a/substrate/client/cli/src/config.rs b/substrate/client/cli/src/config.rs index 5c4a84247b..f2a6715cf2 100644 --- a/substrate/client/cli/src/config.rs +++ b/substrate/client/cli/src/config.rs @@ -27,8 +27,8 @@ use names::{Generator, Name}; use sc_client_api::execution_extensions::ExecutionStrategies; use sc_service::config::{ Configuration, DatabaseConfig, ExtTransport, KeystoreConfig, NetworkConfiguration, - NodeKeyConfig, OffchainWorkerConfig, PrometheusConfig, PruningMode, Role, TaskType, - TelemetryEndpoints, TransactionPoolOptions, WasmExecutionMethod, + NodeKeyConfig, OffchainWorkerConfig, PrometheusConfig, PruningMode, Role, RpcMethods, + TaskType, TelemetryEndpoints, TransactionPoolOptions, WasmExecutionMethod, }; use sc_service::{ChainSpec, TracingReceiver}; use std::future::Future; @@ -265,10 +265,11 @@ pub trait CliConfiguration: Sized { Ok(Default::default()) } - /// Returns `Ok(true) if potentially unsafe RPC is to be exposed. + /// Returns the RPC method set to expose. /// - /// By default this is `false`. - fn unsafe_rpc_expose(&self) -> Result { + /// By default this is `RpcMethods::Auto` (unsafe RPCs are denied iff + /// `{rpc,ws}_external` returns true, respectively). + fn rpc_methods(&self) -> Result { Ok(Default::default()) } @@ -449,7 +450,7 @@ pub trait CliConfiguration: Sized { execution_strategies: self.execution_strategies(is_dev)?, rpc_http: self.rpc_http()?, rpc_ws: self.rpc_ws()?, - unsafe_rpc_expose: self.unsafe_rpc_expose()?, + rpc_methods: self.rpc_methods()?, rpc_ws_max_connections: self.rpc_ws_max_connections()?, rpc_cors: self.rpc_cors(is_dev)?, prometheus_config: self.prometheus_config()?, diff --git a/substrate/client/service/src/config.rs b/substrate/client/service/src/config.rs index e1adc02e9a..e0de85b56d 100644 --- a/substrate/client/service/src/config.rs +++ b/substrate/client/service/src/config.rs @@ -59,8 +59,6 @@ pub struct Configuration { pub wasm_method: WasmExecutionMethod, /// Execution strategies. pub execution_strategies: ExecutionStrategies, - /// Whether potentially unsafe RPC is considered safe to be exposed. - pub unsafe_rpc_expose: bool, /// RPC over HTTP binding address. `None` if disabled. pub rpc_http: Option, /// RPC over Websockets binding address. `None` if disabled. @@ -69,6 +67,8 @@ pub struct Configuration { pub rpc_ws_max_connections: Option, /// CORS settings for HTTP & WS servers. `None` if all origins are allowed. pub rpc_cors: Option>, + /// RPC methods to expose (by default only a safe subset or all of them). + pub rpc_methods: RpcMethods, /// Prometheus endpoint configuration. `None` if disabled. pub prometheus_config: Option, /// Telemetry service URL. `None` if disabled. @@ -171,3 +171,21 @@ impl Configuration { self.role.to_string() } } + +/// Available RPC methods. +#[derive(Debug, Copy, Clone)] +pub enum RpcMethods { + /// Expose every RPC method only when RPC is listening on `localhost`, + /// otherwise serve only safe RPC methods. + Auto, + /// Allow only a safe subset of RPC methods. + Safe, + /// Expose every RPC method (even potentially unsafe ones). + Unsafe, +} + +impl Default for RpcMethods { + fn default() -> RpcMethods { + RpcMethods::Auto + } +} diff --git a/substrate/client/service/src/lib.rs b/substrate/client/service/src/lib.rs index 742e5b29aa..4e979ebb17 100644 --- a/substrate/client/service/src/lib.rs +++ b/substrate/client/service/src/lib.rs @@ -64,7 +64,7 @@ pub use self::builder::{ ServiceBuilder, ServiceBuilderCommand, TFullClient, TLightClient, TFullBackend, TLightBackend, TFullCallExecutor, TLightCallExecutor, }; -pub use config::{Configuration, Role, PruningMode, DatabaseConfig, TaskType}; +pub use config::{Configuration, DatabaseConfig, PruningMode, Role, RpcMethods, TaskType}; pub use sc_chain_spec::{ ChainSpec, GenericChainSpec, Properties, RuntimeGenesis, Extension as ChainSpecExtension, NoExtension, ChainType, @@ -551,12 +551,12 @@ fn start_rpc_servers sc_rpc_server::RpcHandler, unsafe_rpc_expose: bool) -> sc_rpc::DenyUnsafe { + fn deny_unsafe(addr: &Option, methods: &RpcMethods) -> sc_rpc::DenyUnsafe { let is_exposed_addr = addr.map(|x| x.ip().is_loopback()).unwrap_or(false); - if is_exposed_addr && !unsafe_rpc_expose { - sc_rpc::DenyUnsafe::Yes - } else { - sc_rpc::DenyUnsafe::No + match (is_exposed_addr, methods) { + | (_, RpcMethods::Unsafe) + | (false, RpcMethods::Auto) => sc_rpc::DenyUnsafe::No, + _ => sc_rpc::DenyUnsafe::Yes } } @@ -566,7 +566,7 @@ fn start_rpc_servers sc_rpc_server::RpcHandler sc_rpc_server::RpcHandler