diff --git a/substrate/client/cli/src/commands/mod.rs b/substrate/client/cli/src/commands/mod.rs index ae2fe55467..ec36fb3b64 100644 --- a/substrate/client/cli/src/commands/mod.rs +++ b/substrate/client/cli/src/commands/mod.rs @@ -279,6 +279,12 @@ macro_rules! substrate_cli_subcommands { } } + fn unsafe_rpc_expose(&self) -> $crate::Result { + match self { + $($enum::$variant(cmd) => cmd.unsafe_rpc_expose()),* + } + } + fn rpc_ws_max_connections(&self) -> $crate::Result<::std::option::Option> { match self { $($enum::$variant(cmd) => cmd.rpc_ws_max_connections()),* diff --git a/substrate/client/cli/src/commands/runcmd.rs b/substrate/client/cli/src/commands/runcmd.rs index b3ce6ce6d1..3a3a17ea3f 100644 --- a/substrate/client/cli/src/commands/runcmd.rs +++ b/substrate/client/cli/src/commands/runcmd.rs @@ -27,7 +27,7 @@ use sc_service::{ ChainSpec, Role, }; use sc_telemetry::TelemetryEndpoints; -use std::net::SocketAddr; +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use structopt::{clap::arg_enum, StructOpt}; arg_enum! { @@ -93,6 +93,14 @@ pub struct RunCmd { #[structopt(long = "unsafe-rpc-external")] pub unsafe_rpc_external: bool, + /// Don't deny potentially unsafe RPCs when listening on external interfaces. + /// + /// 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, + /// Listen to all Websocket interfaces. /// /// Default is local. Note: not all RPC methods are safe to be exposed publicly. Use an RPC proxy @@ -361,22 +369,19 @@ impl CliConfiguration for RunCmd { } fn prometheus_config(&self) -> Result> { - if self.no_prometheus { - Ok(None) + Ok(if self.no_prometheus { + None } else { - let prometheus_interface: &str = if self.prometheus_external { - "0.0.0.0" + let interface = if self.prometheus_external { + Ipv4Addr::UNSPECIFIED } else { - "127.0.0.1" + Ipv4Addr::LOCALHOST }; - Ok(Some(PrometheusConfig::new_with_default_registry( - parse_address( - &format!("{}:{}", prometheus_interface, 9615), - self.prometheus_port, - )?, - ))) - } + Some(PrometheusConfig::new_with_default_registry( + SocketAddr::new(interface.into(), self.prometheus_port.unwrap_or(9615)) + )) + }) } fn disable_grandpa(&self) -> Result { @@ -409,23 +414,29 @@ impl CliConfiguration for RunCmd { } fn rpc_http(&self) -> Result> { - let rpc_interface: &str = - interface_str(self.rpc_external, self.unsafe_rpc_external, self.validator)?; + let interface = rpc_interface( + self.rpc_external, + self.unsafe_rpc_external, + self.unsafe_rpc_expose, + self.validator + )?; - Ok(Some(parse_address( - &format!("{}:{}", rpc_interface, 9933), - self.rpc_port, - )?)) + Ok(Some(SocketAddr::new(interface, self.rpc_port.unwrap_or(9933)))) } fn rpc_ws(&self) -> Result> { - let ws_interface: &str = - interface_str(self.ws_external, self.unsafe_ws_external, self.validator)?; + let interface = rpc_interface( + self.ws_external, + self.unsafe_ws_external, + self.unsafe_rpc_expose, + self.validator + )?; - Ok(Some(parse_address( - &format!("{}:{}", ws_interface, 9944), - self.ws_port, - )?)) + Ok(Some(SocketAddr::new(interface, self.ws_port.unwrap_or(9944)))) + } + + fn unsafe_rpc_expose(&self) -> Result { + Ok(self.unsafe_rpc_expose) } fn offchain_worker(&self, role: &Role) -> Result { @@ -468,23 +479,13 @@ pub fn is_node_name_valid(_name: &str) -> std::result::Result<(), &str> { Ok(()) } -fn parse_address(address: &str, port: Option) -> std::result::Result { - let mut address: SocketAddr = address - .parse() - .map_err(|_| format!("Invalid address: {}", address))?; - if let Some(port) = port { - address.set_port(port); - } - - Ok(address) -} - -fn interface_str( +fn rpc_interface( is_external: bool, is_unsafe_external: bool, + is_unsafe_rpc_expose: bool, is_validator: bool, -) -> Result<&'static str> { - if is_external && is_validator { +) -> Result { + if is_external && is_validator && !is_unsafe_rpc_expose { 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 \ @@ -499,9 +500,9 @@ fn interface_str( available set of RPC methods." ); - Ok("0.0.0.0") + Ok(Ipv4Addr::UNSPECIFIED.into()) } else { - Ok("127.0.0.1") + Ok(Ipv4Addr::LOCALHOST.into()) } } diff --git a/substrate/client/cli/src/config.rs b/substrate/client/cli/src/config.rs index 04a6647402..9de2022a59 100644 --- a/substrate/client/cli/src/config.rs +++ b/substrate/client/cli/src/config.rs @@ -249,6 +249,13 @@ pub trait CliConfiguration: Sized { Ok(Default::default()) } + /// Returns `Ok(true) if potentially unsafe RPC is to be exposed. + /// + /// By default this is `false`. + fn unsafe_rpc_expose(&self) -> Result { + Ok(Default::default()) + } + /// Get the RPC websockets maximum connections (`None` if unlimited). /// /// By default this is `None`. @@ -419,6 +426,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_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/rpc-api/src/author/error.rs b/substrate/client/rpc-api/src/author/error.rs index f1b5691008..dfd488e5da 100644 --- a/substrate/client/rpc-api/src/author/error.rs +++ b/substrate/client/rpc-api/src/author/error.rs @@ -57,6 +57,8 @@ pub enum Error { /// Invalid session keys encoding. #[display(fmt="Session keys are not encoded correctly")] InvalidSessionKeys, + /// Call to an unsafe RPC was denied. + UnsafeRpcCalled(crate::policy::UnsafeRpcError), } impl std::error::Error for Error { @@ -65,6 +67,7 @@ impl std::error::Error for Error { Error::Client(ref err) => Some(&**err), Error::Pool(ref err) => Some(err), Error::Verification(ref err) => Some(&**err), + Error::UnsafeRpcCalled(ref err) => Some(err), _ => None, } } @@ -152,6 +155,7 @@ impl From for rpc::Error { request to insert the key successfully.".into() ), }, + Error::UnsafeRpcCalled(e) => e.into(), e => errors::internal(e), } } diff --git a/substrate/client/rpc-api/src/lib.rs b/substrate/client/rpc-api/src/lib.rs index 8ad2d94bfd..2d541be2a7 100644 --- a/substrate/client/rpc-api/src/lib.rs +++ b/substrate/client/rpc-api/src/lib.rs @@ -22,11 +22,13 @@ mod errors; mod helpers; +mod policy; mod subscriptions; pub use jsonrpc_core::IoHandlerExtension as RpcExtension; pub use subscriptions::{Subscriptions, TaskExecutor}; pub use helpers::Receiver; +pub use policy::DenyUnsafe; pub mod author; pub mod chain; diff --git a/substrate/client/rpc-api/src/offchain/error.rs b/substrate/client/rpc-api/src/offchain/error.rs index c28a2a2f39..695c0cf41f 100644 --- a/substrate/client/rpc-api/src/offchain/error.rs +++ b/substrate/client/rpc-api/src/offchain/error.rs @@ -27,11 +27,16 @@ pub enum Error { /// Unavailable storage kind error. #[display(fmt="This storage kind is not available yet.")] UnavailableStorageKind, + /// Call to an unsafe RPC was denied. + UnsafeRpcCalled(crate::policy::UnsafeRpcError), } impl std::error::Error for Error { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { - None + match self { + Self::UnsafeRpcCalled(err) => Some(err), + _ => None, + } } } @@ -46,6 +51,7 @@ impl From for rpc::Error { message: "This storage kind is not available yet" .into(), data: None, }, + Error::UnsafeRpcCalled(e) => e.into(), } } } diff --git a/substrate/client/rpc-api/src/policy.rs b/substrate/client/rpc-api/src/policy.rs new file mode 100644 index 0000000000..c01b5232f3 --- /dev/null +++ b/substrate/client/rpc-api/src/policy.rs @@ -0,0 +1,60 @@ +// Copyright 2020 Parity Technologies (UK) Ltd. +// This file is part of Substrate. + +// Substrate is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. + +// Substrate is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License +// along with Substrate. If not, see . + +//! Policy-related types. +//! +//! Contains a `DenyUnsafe` type that can be used to deny potentially unsafe +//! RPC when accessed externally. + +use jsonrpc_core as rpc; + +/// Signifies whether a potentially unsafe RPC should be denied. +#[derive(Clone, Copy, Debug)] +pub enum DenyUnsafe { + /// Denies only potentially unsafe RPCs. + Yes, + /// Allows calling every RPCs. + No +} + +impl DenyUnsafe { + /// Returns `Ok(())` if the RPCs considered unsafe are safe to call, + /// otherwise returns `Err(UnsafeRpcError)`. + pub fn check_if_safe(self) -> Result<(), UnsafeRpcError> { + match self { + DenyUnsafe::Yes => Err(UnsafeRpcError), + DenyUnsafe::No => Ok(()) + } + } +} + +/// Signifies whether an RPC considered unsafe is denied to be called externally. +#[derive(Debug)] +pub struct UnsafeRpcError; + +impl std::fmt::Display for UnsafeRpcError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "RPC call is unsafe to be called externally") + } +} + +impl std::error::Error for UnsafeRpcError {} + +impl From for rpc::Error { + fn from(_: UnsafeRpcError) -> rpc::Error { + rpc::Error::method_not_found() + } +} diff --git a/substrate/client/rpc-api/src/system/mod.rs b/substrate/client/rpc-api/src/system/mod.rs index e66ac97a68..486623477e 100644 --- a/substrate/client/rpc-api/src/system/mod.rs +++ b/substrate/client/rpc-api/src/system/mod.rs @@ -72,14 +72,16 @@ pub trait SystemApi { /// Returns currently connected peers #[rpc(name = "system_peers", returns = "Vec>")] - fn system_peers(&self) -> Receiver>>; + fn system_peers(&self) + -> Compat>>>>; /// Returns current state of the network. /// /// **Warning**: This API is not stable. // TODO: make this stable and move structs https://github.com/paritytech/substrate/issues/1890 #[rpc(name = "system_networkState", returns = "jsonrpc_core::Value")] - fn system_network_state(&self) -> Receiver; + fn system_network_state(&self) + -> Compat>>; /// Adds a reserved peer. Returns the empty string or an error. The string /// parameter should encode a `p2p` multiaddr. diff --git a/substrate/client/rpc/src/author/mod.rs b/substrate/client/rpc/src/author/mod.rs index a3f23e8e14..23aed953d0 100644 --- a/substrate/client/rpc/src/author/mod.rs +++ b/substrate/client/rpc/src/author/mod.rs @@ -30,7 +30,7 @@ use rpc::futures::{ }; use futures::{StreamExt as _, compat::Compat}; use futures::future::{ready, FutureExt, TryFutureExt}; -use sc_rpc_api::Subscriptions; +use sc_rpc_api::{DenyUnsafe, Subscriptions}; use jsonrpc_pubsub::{typed::Subscriber, SubscriptionId}; use codec::{Encode, Decode}; use sp_core::{Bytes, traits::BareCryptoStorePtr}; @@ -56,6 +56,8 @@ pub struct Author { subscriptions: Subscriptions, /// The key store. keystore: BareCryptoStorePtr, + /// Whether to deny unsafe calls + deny_unsafe: DenyUnsafe, } impl Author { @@ -65,12 +67,14 @@ impl Author { pool: Arc

, subscriptions: Subscriptions, keystore: BareCryptoStorePtr, + deny_unsafe: DenyUnsafe, ) -> Self { Author { client, pool, subscriptions, keystore, + deny_unsafe, } } } @@ -97,6 +101,8 @@ impl AuthorApi, BlockHash

> for Author suri: String, public: Bytes, ) -> Result<()> { + self.deny_unsafe.check_if_safe()?; + let key_type = key_type.as_str().try_into().map_err(|_| Error::BadKeyType)?; let mut keystore = self.keystore.write(); keystore.insert_unknown(key_type, &suri, &public[..]) @@ -105,6 +111,8 @@ impl AuthorApi, BlockHash

> for Author } fn rotate_keys(&self) -> Result { + self.deny_unsafe.check_if_safe()?; + let best_block_hash = self.client.info().best_hash; self.client.runtime_api().generate_session_keys( &generic::BlockId::Hash(best_block_hash), @@ -113,6 +121,8 @@ impl AuthorApi, BlockHash

> for Author } fn has_session_keys(&self, session_keys: Bytes) -> Result { + self.deny_unsafe.check_if_safe()?; + let best_block_hash = self.client.info().best_hash; let keys = self.client.runtime_api().decode_session_keys( &generic::BlockId::Hash(best_block_hash), @@ -124,6 +134,8 @@ impl AuthorApi, BlockHash

> for Author } fn has_key(&self, public_key: Bytes, key_type: String) -> Result { + self.deny_unsafe.check_if_safe()?; + let key_type = key_type.as_str().try_into().map_err(|_| Error::BadKeyType)?; Ok(self.keystore.read().has_keys(&[(public_key.to_vec(), key_type)])) } @@ -151,6 +163,8 @@ impl AuthorApi, BlockHash

> for Author &self, bytes_or_hash: Vec>>, ) -> Result>> { + self.deny_unsafe.check_if_safe()?; + let hashes = bytes_or_hash.into_iter() .map(|x| match x { hash::ExtrinsicOrHash::Hash(h) => Ok(h), diff --git a/substrate/client/rpc/src/author/tests.rs b/substrate/client/rpc/src/author/tests.rs index 445888c523..de2ee92fe3 100644 --- a/substrate/client/rpc/src/author/tests.rs +++ b/substrate/client/rpc/src/author/tests.rs @@ -83,6 +83,7 @@ impl TestSetup { pool: self.pool.clone(), subscriptions: Subscriptions::new(Arc::new(self.runtime.executor())), keystore: self.keystore.clone(), + deny_unsafe: DenyUnsafe::No, } } } diff --git a/substrate/client/rpc/src/lib.rs b/substrate/client/rpc/src/lib.rs index ea65785c20..c4389913b4 100644 --- a/substrate/client/rpc/src/lib.rs +++ b/substrate/client/rpc/src/lib.rs @@ -22,7 +22,7 @@ mod metadata; -pub use sc_rpc_api::Subscriptions; +pub use sc_rpc_api::{DenyUnsafe, Subscriptions}; pub use self::metadata::Metadata; pub use rpc::IoHandlerExtension as RpcExtension; diff --git a/substrate/client/rpc/src/offchain/mod.rs b/substrate/client/rpc/src/offchain/mod.rs index 61984d4845..16c03395c7 100644 --- a/substrate/client/rpc/src/offchain/mod.rs +++ b/substrate/client/rpc/src/offchain/mod.rs @@ -21,6 +21,7 @@ mod tests; /// Re-export the API for backward compatibility. pub use sc_rpc_api::offchain::*; +use sc_rpc_api::DenyUnsafe; use self::error::{Error, Result}; use sp_core::{ Bytes, @@ -34,13 +35,15 @@ use std::sync::Arc; pub struct Offchain { /// Offchain storage storage: Arc>, + deny_unsafe: DenyUnsafe, } impl Offchain { /// Create new instance of Offchain API. - pub fn new(storage: T) -> Self { + pub fn new(storage: T, deny_unsafe: DenyUnsafe) -> Self { Offchain { storage: Arc::new(RwLock::new(storage)), + deny_unsafe, } } } @@ -48,6 +51,8 @@ impl Offchain { impl OffchainApi for Offchain { /// Set offchain local storage under given key and prefix. fn set_local_storage(&self, kind: StorageKind, key: Bytes, value: Bytes) -> Result<()> { + self.deny_unsafe.check_if_safe()?; + let prefix = match kind { StorageKind::PERSISTENT => sp_offchain::STORAGE_PREFIX, StorageKind::LOCAL => return Err(Error::UnavailableStorageKind), @@ -58,6 +63,8 @@ impl OffchainApi for Offchain { /// Get offchain local storage under given key and prefix. fn get_local_storage(&self, kind: StorageKind, key: Bytes) -> Result> { + self.deny_unsafe.check_if_safe()?; + let prefix = match kind { StorageKind::PERSISTENT => sp_offchain::STORAGE_PREFIX, StorageKind::LOCAL => return Err(Error::UnavailableStorageKind), diff --git a/substrate/client/rpc/src/offchain/tests.rs b/substrate/client/rpc/src/offchain/tests.rs index ac1a6a4de3..cb05f3d4db 100644 --- a/substrate/client/rpc/src/offchain/tests.rs +++ b/substrate/client/rpc/src/offchain/tests.rs @@ -21,7 +21,7 @@ use sp_core::{Bytes, offchain::storage::InMemOffchainStorage}; #[test] fn local_storage_should_work() { let storage = InMemOffchainStorage::default(); - let offchain = Offchain::new(storage); + let offchain = Offchain::new(storage, DenyUnsafe::No); let key = Bytes(b"offchain_storage".to_vec()); let value = Bytes(b"offchain_value".to_vec()); @@ -34,3 +34,20 @@ fn local_storage_should_work() { Ok(Some(ref v)) if *v == value ); } + +#[test] +fn offchain_calls_considered_unsafe() { + let storage = InMemOffchainStorage::default(); + let offchain = Offchain::new(storage, DenyUnsafe::Yes); + let key = Bytes(b"offchain_storage".to_vec()); + let value = Bytes(b"offchain_value".to_vec()); + + assert_matches!( + offchain.set_local_storage(StorageKind::PERSISTENT, key.clone(), value.clone()), + Err(Error::UnsafeRpcCalled(_)) + ); + assert_matches!( + offchain.get_local_storage(StorageKind::PERSISTENT, key), + Err(Error::UnsafeRpcCalled(_)) + ); +} diff --git a/substrate/client/rpc/src/system/mod.rs b/substrate/client/rpc/src/system/mod.rs index 84e06c20a6..2a19e5412e 100644 --- a/substrate/client/rpc/src/system/mod.rs +++ b/substrate/client/rpc/src/system/mod.rs @@ -21,7 +21,7 @@ mod tests; use futures::{future::BoxFuture, FutureExt, TryFutureExt}; use futures::{channel::oneshot, compat::Compat}; -use sc_rpc_api::Receiver; +use sc_rpc_api::{DenyUnsafe, Receiver}; use sp_utils::mpsc::TracingUnboundedSender; use sp_runtime::traits::{self, Header as HeaderT}; @@ -31,10 +31,19 @@ pub use sc_rpc_api::system::*; pub use self::helpers::{SystemInfo, Health, PeerInfo, NodeRole}; pub use self::gen_client::Client as SystemClient; +macro_rules! bail_if_unsafe { + ($value: expr) => { + if let Err(err) = $value.check_if_safe() { + return async move { Err(err.into()) }.boxed().compat(); + } + }; +} + /// System API implementation pub struct System { info: SystemInfo, send_back: TracingUnboundedSender>, + deny_unsafe: DenyUnsafe, } /// Request to be processed. @@ -66,10 +75,12 @@ impl System { pub fn new( info: SystemInfo, send_back: TracingUnboundedSender>, + deny_unsafe: DenyUnsafe, ) -> Self { System { info, send_back, + deny_unsafe, } } } @@ -113,21 +124,37 @@ impl SystemApi::Number> for Sy Receiver(Compat::new(rx)) } - fn system_peers(&self) -> Receiver::Number>>> { + fn system_peers(&self) + -> Compat::Number>>>>> + { + bail_if_unsafe!(self.deny_unsafe); + let (tx, rx) = oneshot::channel(); let _ = self.send_back.unbounded_send(Request::Peers(tx)); - Receiver(Compat::new(rx)) + + async move { + rx.await.map_err(|_| rpc::Error::internal_error()) + }.boxed().compat() } - fn system_network_state(&self) -> Receiver { + fn system_network_state(&self) + -> Compat>> + { + bail_if_unsafe!(self.deny_unsafe); + let (tx, rx) = oneshot::channel(); let _ = self.send_back.unbounded_send(Request::NetworkState(tx)); - Receiver(Compat::new(rx)) + + async move { + rx.await.map_err(|_| rpc::Error::internal_error()) + }.boxed().compat() } fn system_add_reserved_peer(&self, peer: String) -> Compat>> { + bail_if_unsafe!(self.deny_unsafe); + let (tx, rx) = oneshot::channel(); let _ = self.send_back.unbounded_send(Request::NetworkAddReservedPeer(peer, tx)); async move { @@ -142,6 +169,8 @@ impl SystemApi::Number> for Sy fn system_remove_reserved_peer(&self, peer: String) -> Compat>> { + bail_if_unsafe!(self.deny_unsafe); + let (tx, rx) = oneshot::channel(); let _ = self.send_back.unbounded_send(Request::NetworkRemoveReservedPeer(peer, tx)); async move { diff --git a/substrate/client/rpc/src/system/tests.rs b/substrate/client/rpc/src/system/tests.rs index 921d941a1c..d0ec695126 100644 --- a/substrate/client/rpc/src/system/tests.rs +++ b/substrate/client/rpc/src/system/tests.rs @@ -109,13 +109,17 @@ fn api>>(sync: T) -> System { future::ready(()) })) }); - System::new(SystemInfo { - impl_name: "testclient".into(), - impl_version: "0.2.0".into(), - chain_name: "testchain".into(), - properties: Default::default(), - chain_type: Default::default(), - }, tx) + System::new( + SystemInfo { + impl_name: "testclient".into(), + impl_version: "0.2.0".into(), + chain_name: "testchain".into(), + properties: Default::default(), + chain_type: Default::default(), + }, + tx, + sc_rpc_api::DenyUnsafe::No + ) } fn wait_receiver(rx: Receiver) -> T { @@ -238,14 +242,19 @@ fn system_local_listen_addresses_works() { #[test] fn system_peers() { + let mut runtime = tokio::runtime::current_thread::Runtime::new().unwrap(); + let peer_id = PeerId::random(); + let req = api(Status { + peer_id: peer_id.clone(), + peers: 1, + is_syncing: false, + is_dev: true, + }).system_peers(); + let res = runtime.block_on(req).unwrap(); + assert_eq!( - wait_receiver(api(Status { - peer_id: peer_id.clone(), - peers: 1, - is_syncing: false, - is_dev: true, - }).system_peers()), + res, vec![PeerInfo { peer_id: peer_id.to_base58(), roles: "FULL".into(), @@ -258,7 +267,10 @@ fn system_peers() { #[test] fn system_network_state() { - let res = wait_receiver(api(None).system_network_state()); + let mut runtime = tokio::runtime::current_thread::Runtime::new().unwrap(); + let req = api(None).system_network_state(); + let res = runtime.block_on(req).unwrap(); + assert_eq!( serde_json::from_value::(res).unwrap(), sc_network::network_state::NetworkState { diff --git a/substrate/client/service/src/builder.rs b/substrate/client/service/src/builder.rs index 90e644481f..200707dad4 100644 --- a/substrate/client/service/src/builder.rs +++ b/substrate/client/service/src/builder.rs @@ -46,6 +46,7 @@ use sp_runtime::traits::{ use sp_api::ProvideRuntimeApi; use sc_executor::{NativeExecutor, NativeExecutionDispatch}; use std::{ + collections::HashMap, io::{Read, Write, Seek}, marker::PhantomData, sync::Arc, pin::Pin }; @@ -1001,7 +1002,7 @@ ServiceBuilder< // RPC let (system_rpc_tx, system_rpc_rx) = tracing_unbounded("mpsc_system_rpc"); - let gen_handler = || { + let gen_handler = |deny_unsafe: sc_rpc::DenyUnsafe| { use sc_rpc::{chain, state, author, system, offchain}; let system_info = sc_rpc::system::SystemInfo { @@ -1043,32 +1044,31 @@ ServiceBuilder< transaction_pool.clone(), subscriptions, keystore.clone(), + deny_unsafe, ); - let system = system::System::new(system_info, system_rpc_tx.clone()); + let system = system::System::new(system_info, system_rpc_tx.clone(), deny_unsafe); - match offchain_storage.clone() { - Some(storage) => { - let offchain = sc_rpc::offchain::Offchain::new(storage); - sc_rpc_server::rpc_handler(( - state::StateApi::to_delegate(state), - chain::ChainApi::to_delegate(chain), - offchain::OffchainApi::to_delegate(offchain), - author::AuthorApi::to_delegate(author), - system::SystemApi::to_delegate(system), - rpc_extensions.clone(), - )) - }, - None => sc_rpc_server::rpc_handler(( - state::StateApi::to_delegate(state), - chain::ChainApi::to_delegate(chain), - author::AuthorApi::to_delegate(author), - system::SystemApi::to_delegate(system), - rpc_extensions.clone(), - )) - } + let maybe_offchain_rpc = offchain_storage.clone() + .map(|storage| { + let offchain = sc_rpc::offchain::Offchain::new(storage, deny_unsafe); + // FIXME: Use plain Option (don't collect into HashMap) when we upgrade to jsonrpc 14.1 + // https://github.com/paritytech/jsonrpc/commit/20485387ed06a48f1a70bf4d609a7cde6cf0accf + let delegate = offchain::OffchainApi::to_delegate(offchain); + delegate.into_iter().collect::>() + }).unwrap_or_default(); + + sc_rpc_server::rpc_handler(( + state::StateApi::to_delegate(state), + chain::ChainApi::to_delegate(chain), + maybe_offchain_rpc, + author::AuthorApi::to_delegate(author), + system::SystemApi::to_delegate(system), + rpc_extensions.clone(), + )) }; - let rpc_handlers = gen_handler(); let rpc = start_rpc_servers(&config, gen_handler)?; + // This is used internally, so don't restrict access to unsafe RPC + let rpc_handlers = gen_handler(sc_rpc::DenyUnsafe::No); spawn_handle.spawn( "network-worker", diff --git a/substrate/client/service/src/config.rs b/substrate/client/service/src/config.rs index b90bed723f..affb4aabfc 100644 --- a/substrate/client/service/src/config.rs +++ b/substrate/client/service/src/config.rs @@ -59,6 +59,8 @@ 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. diff --git a/substrate/client/service/src/lib.rs b/substrate/client/service/src/lib.rs index 97481fcc25..039e0257ab 100644 --- a/substrate/client/service/src/lib.rs +++ b/substrate/client/service/src/lib.rs @@ -511,7 +511,7 @@ mod waiting { /// Starts RPC servers that run in their own thread, and returns an opaque object that keeps them alive. #[cfg(not(target_os = "unknown"))] -fn start_rpc_servers sc_rpc_server::RpcHandler>( +fn start_rpc_servers sc_rpc_server::RpcHandler>( config: &Configuration, mut gen_handler: H ) -> Result, error::Error> { @@ -533,10 +533,23 @@ fn start_rpc_servers sc_rpc_server::RpcHandler>( }) } + fn deny_unsafe(addr: &Option, unsafe_rpc_expose: bool) -> 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 + } + } + Ok(Box::new(( maybe_start_server( config.rpc_http, - |address| sc_rpc_server::start_http(address, config.rpc_cors.as_ref(), gen_handler()), + |address| sc_rpc_server::start_http( + address, + config.rpc_cors.as_ref(), + gen_handler(deny_unsafe(&config.rpc_http, config.unsafe_rpc_expose)), + ), )?.map(|s| waiting::HttpServer(Some(s))), maybe_start_server( config.rpc_ws, @@ -544,15 +557,15 @@ fn start_rpc_servers sc_rpc_server::RpcHandler>( address, config.rpc_ws_max_connections, config.rpc_cors.as_ref(), - gen_handler(), + gen_handler(deny_unsafe(&config.rpc_ws, config.unsafe_rpc_expose)), ), - )?.map(|s| waiting::WsServer(Some(s))).map(Mutex::new), + )?.map(|s| waiting::WsServer(Some(s))), ))) } /// Starts RPC servers that run in their own thread, and returns an opaque object that keeps them alive. #[cfg(target_os = "unknown")] -fn start_rpc_servers sc_rpc_server::RpcHandler>( +fn start_rpc_servers sc_rpc_server::RpcHandler>( _: &Configuration, _: H ) -> Result, error::Error> { diff --git a/substrate/client/service/test/src/lib.rs b/substrate/client/service/test/src/lib.rs index 1e824cb273..9b9140dd8d 100644 --- a/substrate/client/service/test/src/lib.rs +++ b/substrate/client/service/test/src/lib.rs @@ -184,6 +184,7 @@ fn node_config