Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

jsonrpsee: add host and origin filtering #9787

Merged
merged 6 commits into from
Sep 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 14 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions client/rpc-api/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ pub use policy::DenyUnsafe;

pub mod author;
pub mod chain;
/// Child state API
pub mod child_state;
pub mod offchain;
pub mod state;
Expand Down
50 changes: 39 additions & 11 deletions client/rpc-servers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
#![warn(missing_docs)]

use jsonrpsee::{
http_server::{HttpServerBuilder, HttpStopHandle},
http_server::{AccessControlBuilder, Host, HttpServerBuilder, HttpStopHandle},
ws_server::{WsServerBuilder, WsStopHandle},
RpcModule,
};
Expand Down Expand Up @@ -90,7 +90,7 @@ pub type WsServer = WsStopHandle;
/// Start HTTP server listening on given address.
pub fn start_http<M: Send + Sync + 'static>(
addr: std::net::SocketAddr,
_cors: Option<&Vec<String>>,
cors: Option<&Vec<String>>,
maybe_max_payload_mb: Option<usize>,
module: RpcModule<M>,
rt: tokio::runtime::Handle,
Expand All @@ -99,8 +99,26 @@ pub fn start_http<M: Send + Sync + 'static>(
.map(|mb| mb.saturating_mul(MEGABYTE))
.unwrap_or(RPC_MAX_PAYLOAD_DEFAULT);

let mut acl = AccessControlBuilder::new();

log::info!("Starting JSONRPC HTTP server: addr={}, allowed origins={:?}", addr, cors);

if let Some(cors) = cors {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't we put localhost/127.0.0.1 on the allowlist even if corse is None?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All hosts/origins are enabled by default: https://github.com/paritytech/jsonrpsee/blob/master/http-server/src/access_control/mod.rs#L115

We should probably document it clearly in jsonrpsee I guess

// Whitelist listening address.
let host = Host::parse(&format!("localhost:{}", addr.port()));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incredible ugly and annoying, we should fix this API.

acl = acl.allow_host(host);
let host = Host::parse(&format!("127.0.0.1:{}", addr.port()));
acl = acl.allow_host(host);

// Set allowed origins.
for origin in cors {
acl = acl.cors_allow_origin(origin.into());
}
};

let server = HttpServerBuilder::default()
.max_request_body_size(max_request_body_size as u32)
.set_access_control(acl.build())
.build(addr)?;

let handle = server.stop_handle();
Expand All @@ -117,7 +135,7 @@ pub fn start_http<M: Send + Sync + 'static>(
pub fn start_ws<M: Send + Sync + 'static>(
addr: std::net::SocketAddr,
max_connections: Option<usize>,
_cors: Option<&Vec<String>>,
cors: Option<&Vec<String>>,
maybe_max_payload_mb: Option<usize>,
module: RpcModule<M>,
rt: tokio::runtime::Handle,
Expand All @@ -127,14 +145,24 @@ pub fn start_ws<M: Send + Sync + 'static>(
.unwrap_or(RPC_MAX_PAYLOAD_DEFAULT);
let max_connections = max_connections.unwrap_or(WS_MAX_CONNECTIONS);

let server = tokio::task::block_in_place(|| {
rt.block_on(
WsServerBuilder::default()
.max_request_body_size(max_request_body_size as u32)
.max_connections(max_connections as u64)
.build(addr),
)
})?;
let mut builder = WsServerBuilder::default()
.max_request_body_size(max_request_body_size as u32)
.max_connections(max_connections as u64);

log::info!("Starting JSONRPC WS server: addr={}, allowed origins={:?}", addr, cors);

if let Some(cors) = cors {
// Whitelist listening address.
builder = builder.set_allowed_hosts([
format!("localhost:{}", addr.port()),
format!("127.0.0.1:{}", addr.port()),
])?;

// Set allowed origins.
builder = builder.set_allowed_origins(cors)?;
}

let server = tokio::task::block_in_place(|| rt.block_on(builder.build(addr)))?;

let handle = server.stop_handle();
let rpc_api = build_rpc_api(module);
Expand Down
7 changes: 5 additions & 2 deletions client/rpc/src/offchain/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ impl<T: OffchainStorage> Offchain<T> {
#[async_trait]
impl<T: OffchainStorage + 'static> OffchainApiServer for Offchain<T> {
fn set_local_storage(&self, kind: StorageKind, key: Bytes, value: Bytes) -> JsonRpcResult<()> {
self.deny_unsafe.check_if_safe()?;

let prefix = match kind {
StorageKind::PERSISTENT => sp_offchain::STORAGE_PREFIX,
StorageKind::LOCAL =>
Expand All @@ -61,13 +63,14 @@ impl<T: OffchainStorage + 'static> OffchainApiServer for Offchain<T> {
}

fn get_local_storage(&self, kind: StorageKind, key: Bytes) -> JsonRpcResult<Option<Bytes>> {
self.deny_unsafe.check_if_safe()?;

let prefix = match kind {
StorageKind::PERSISTENT => sp_offchain::STORAGE_PREFIX,
StorageKind::LOCAL =>
return Err(JsonRpseeError::to_call_error(Error::UnavailableStorageKind)),
};

let bytes: Option<Bytes> = self.storage.read().get(prefix, &*key).map(Into::into);
Ok(bytes)
Ok(self.storage.read().get(prefix, &*key).map(Into::into))
}
}
5 changes: 1 addition & 4 deletions client/rpc/src/state/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,7 @@ where
return Err(JsonRpseeError::to_call_error(Error::InvalidCount {
value: count,
max: STORAGE_KEYS_PAGED_MAX_COUNT,
}))
}));
}
self.backend
.storage_keys_paged(block, prefix, count, start_key)
Expand Down Expand Up @@ -342,7 +342,6 @@ where
}

async fn runtime_version(&self, at: Option<Block::Hash>) -> JsonRpcResult<RuntimeVersion> {
self.deny_unsafe.check_if_safe()?;
self.backend
.runtime_version(at)
.await
Expand All @@ -367,7 +366,6 @@ where
keys: Vec<StorageKey>,
at: Option<Block::Hash>,
) -> JsonRpcResult<Vec<StorageChangeSet<Block::Hash>>> {
self.deny_unsafe.check_if_safe()?;
self.backend
.query_storage_at(keys, at)
.await
Expand All @@ -379,7 +377,6 @@ where
keys: Vec<StorageKey>,
block: Option<Block::Hash>,
) -> JsonRpcResult<ReadProof<Block::Hash>> {
self.deny_unsafe.check_if_safe()?;
self.backend
.read_proof(block, keys)
.await
Expand Down
2 changes: 0 additions & 2 deletions client/rpc/src/system/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,14 +164,12 @@ impl<B: traits::Block> SystemApiServer<B::Hash, <B::Header as HeaderT>::Number>
}

async fn system_node_roles(&self) -> JsonRpcResult<Vec<NodeRole>> {
self.deny_unsafe.check_if_safe()?;
let (tx, rx) = oneshot::channel();
let _ = self.send_back.unbounded_send(Request::NodeRoles(tx));
rx.await.map_err(|e| JsonRpseeError::to_call_error(e))
}

async fn system_sync_state(&self) -> JsonRpcResult<SyncState<<B::Header as HeaderT>::Number>> {
self.deny_unsafe.check_if_safe()?;
let (tx, rx) = oneshot::channel();
let _ = self.send_back.unbounded_send(Request::SyncState(tx));
rx.await.map_err(|e| JsonRpseeError::to_call_error(e))
Expand Down
7 changes: 1 addition & 6 deletions client/service/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,8 +592,6 @@ where
),
);

// NOTE(niklasad1): we spawn jsonrpsee in seperate thread now.
// this will not shutdown the server.
task_manager.keep_alive((config.base_path, rpc));

Ok(())
Expand Down Expand Up @@ -656,7 +654,7 @@ fn init_telemetry<TBl: BlockT, TCl: BlockBackend<TBl>>(
// Maciej: This is very WIP, mocking the original `gen_handler`. All of the `jsonrpsee`
// specific logic should be merged back to `gen_handler` down the road.
fn gen_rpc_module<TBl, TBackend, TCl, TExPool>(
_deny_unsafe: DenyUnsafe,
deny_unsafe: DenyUnsafe,
spawn_handle: SpawnTaskHandle,
client: Arc<TCl>,
on_demand: Option<Arc<OnDemand<TBl>>>,
Expand Down Expand Up @@ -690,9 +688,6 @@ where
{
const UNIQUE_METHOD_NAMES_PROOF: &str = "Method names are unique; qed";

// TODO(niklasad1): fix CORS.
let deny_unsafe = DenyUnsafe::No;

let system_info = sc_rpc::system::SystemInfo {
chain_name: config.chain_spec.name().into(),
impl_name: config.impl_name.clone(),
Expand Down
1 change: 1 addition & 0 deletions client/sync-state-rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ targets = ["x86_64-unknown-linux-gnu"]
thiserror = "1.0.21"
anyhow = "1"
jsonrpsee = { git = "https://github.com/paritytech/jsonrpsee", branch = "master", features = ["server"] }
log = "0.4"

sc-chain-spec = { version = "4.0.0-dev", path = "../chain-spec" }
sc-client-api = { version = "4.0.0-dev", path = "../api" }
Expand Down
1 change: 1 addition & 0 deletions frame/contracts/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ targets = ["x86_64-unknown-linux-gnu"]
[dependencies]
codec = { package = "parity-scale-codec", version = "2" }
jsonrpsee = { git = "https://github.com/paritytech/jsonrpsee", branch = "master", features = ["server"] }
log = "0.4"
serde = { version = "1", features = ["derive"] }
serde_json = "1"

Expand Down
1 change: 1 addition & 0 deletions frame/merkle-mountain-range/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ codec = { package = "parity-scale-codec", version = "2.0.0" }
jsonrpsee = { git = "https://github.com/paritytech/jsonrpsee", branch = "master", features = ["server"] }
serde_json = "1"
serde = { version = "1.0.126", features = ["derive"] }
log = "0.4"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah so this is what you meant by the issue on log? Yeah this is a bit awkward. :/

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah :)


sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" }
sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" }
Expand Down
2 changes: 2 additions & 0 deletions frame/transaction-payment/rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,11 @@ readme = "README.md"
targets = ["x86_64-unknown-linux-gnu"]

[dependencies]
anyhow = "1"
codec = { package = "parity-scale-codec", version = "2.0.0" }

jsonrpsee = { git = "https://github.com/paritytech/jsonrpsee", branch = "master", features = ["server"] }
log = "0.4"

sp-api = { version = "4.0.0-dev", path = "../../../primitives/api" }
sp-blockchain = { version = "4.0.0-dev", path = "../../../primitives/blockchain" }
Expand Down
11 changes: 8 additions & 3 deletions frame/transaction-payment/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

use std::{convert::TryInto, sync::Arc};

use anyhow::anyhow;
use codec::{Codec, Decode};
use jsonrpsee::{
proc_macros::rpc,
Expand All @@ -28,7 +29,6 @@ use jsonrpsee::{
JsonRpcResult,
},
};
pub use pallet_transaction_payment_rpc_runtime_api::TransactionPaymentApi as TransactionPaymentRuntimeApi;
use pallet_transaction_payment_rpc_runtime_api::{FeeDetails, InclusionFee, RuntimeDispatchInfo};
use sp_api::ProvideRuntimeApi;
use sp_blockchain::HeaderBackend;
Expand All @@ -39,6 +39,8 @@ use sp_runtime::{
traits::{Block as BlockT, MaybeDisplay},
};

pub use pallet_transaction_payment_rpc_runtime_api::TransactionPaymentApi as TransactionPaymentRuntimeApi;

#[rpc(client, server, namespace = "payment")]
pub trait TransactionPaymentApi<BlockHash, ResponseType> {
#[method(name = "queryInfo")]
Expand Down Expand Up @@ -109,8 +111,11 @@ where
.query_fee_details(&at, uxt, encoded_len)
.map_err(|api_err| CallError::from_std_error(api_err))?;

let try_into_rpc_balance =
|value: Balance| value.try_into().map_err(|_try_err| CallError::InvalidParams);
let try_into_rpc_balance = |value: Balance| {
value
.try_into()
.map_err(|_| anyhow!("{} doesn't fit in NumberOrHex representation", value))
};

Ok(FeeDetails {
inclusion_fee: if let Some(inclusion_fee) = fee_details.inclusion_fee {
Expand Down