From 576d9e022e27a9931130509130416e9039f9ced4 Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Thu, 10 Jun 2021 21:47:49 +0200 Subject: [PATCH 1/9] Initial implementation --- types/Cargo.toml | 3 ++- ws-server/Cargo.toml | 3 ++- ws-server/src/server.rs | 46 ++++++++++++++++++++++++++++++++++------- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/types/Cargo.toml b/types/Cargo.toml index 31c2c32779..f62a3573af 100644 --- a/types/Cargo.toml +++ b/types/Cargo.toml @@ -18,5 +18,6 @@ log = { version = "0.4", default-features = false } serde = { version = "1", default-features = false, features = ["derive"] } serde_json = { version = "1", default-features = false, features = ["alloc", "raw_value", "std"] } thiserror = "1.0" -soketto = "0.5" +# soketto = "0.5" +soketto = { path = "../../soketto" } hyper = "0.14" diff --git a/ws-server/Cargo.toml b/ws-server/Cargo.toml index 20ad2a1619..663f10cbe5 100644 --- a/ws-server/Cargo.toml +++ b/ws-server/Cargo.toml @@ -19,7 +19,8 @@ log = "0.4" rustc-hash = "1.1.0" serde = { version = "1", default-features = false, features = ["derive"] } serde_json = { version = "1", features = ["raw_value"] } -soketto = "0.5" +# soketto = "0.5" +soketto = { path = "../../soketto" } tokio = { version = "1", features = ["net", "rt-multi-thread", "macros"] } tokio-stream = { version = "0.1.1", features = ["net"] } tokio-util = { version = "0.6", features = ["compat"] } diff --git a/ws-server/src/server.rs b/ws-server/src/server.rs index 6e6b112d61..909fb5ff80 100644 --- a/ws-server/src/server.rs +++ b/ws-server/src/server.rs @@ -75,7 +75,7 @@ impl Server { pub async fn start(self) { let mut incoming = TcpListenerStream::new(self.listener); let methods = Arc::new(self.methods); - let cfg = self.cfg; + // let cfg = self.cfg; let mut id = 0; while let Some(socket) = incoming.next().await { @@ -88,7 +88,7 @@ impl Server { } let methods = methods.clone(); - tokio::spawn(background_task(socket, id, methods, cfg)); + tokio::spawn(background_task(socket, id, methods, self.cfg.clone())); id += 1; } @@ -105,13 +105,22 @@ async fn background_task( // For each incoming background_task we perform a handshake. let mut server = SokettoServer::new(BufReader::new(BufWriter::new(socket.compat()))); - let websocket_key = { + let key = { let req = server.receive_request().await?; - req.into_key() + + if let (Cors::AllowList(list), Some(origin)) = (&cfg.cors, req.headers().origin) { + if !list.iter().any(|o| o.as_bytes() == origin) { + let error = format!("Origin denied: {}", String::from_utf8_lossy(origin)); + log::warn!("{}", error); + return Err(Error::Request(error)); + } + } + + req.key() }; // Here we accept the client unconditionally. - let accept = Response::Accept { key: &websocket_key, protocol: None }; + let accept = Response::Accept { key, protocol: None }; server.send_response(&accept).await?; // And we can finally transition to a websocket background_task. @@ -179,18 +188,30 @@ async fn background_task( } } +#[derive(Debug, Clone)] +enum Cors { + AllowAny, + AllowList(Arc<[String]>), +} + /// JSON-RPC Websocket server settings. -#[derive(Debug, Clone, Copy)] +#[derive(Debug, Clone)] struct Settings { /// Maximum size in bytes of a request. max_request_body_size: u32, /// Maximum number of incoming connections allowed. max_connections: u64, + + cors: Cors, } impl Default for Settings { fn default() -> Self { - Self { max_request_body_size: TEN_MB_SIZE_BYTES, max_connections: MAX_CONNECTIONS } + Self { + max_request_body_size: TEN_MB_SIZE_BYTES, + max_connections: MAX_CONNECTIONS, + cors: Cors::AllowAny, + } } } @@ -213,6 +234,17 @@ impl Builder { self } + /// Set a list of allowet `Origin` headers, connections comming in with a different + /// origin will be denied. + /// + /// Values should include protocol: `"protocol://hostname"`. + /// + /// By default allows any Origin. + pub fn set_allowed_origins(mut self, list: impl AsRef<[String]>) -> Self { + self.settings.cors = Cors::AllowList(list.as_ref().into()); + self + } + /// Finalize the configuration of the server. Consumes the [`Builder`]. pub async fn build(self, addr: impl ToSocketAddrs) -> Result { let listener = TcpListener::bind(addr).await?; From c7173d131ddf2e258d481e38c191bd51094c685c Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Thu, 10 Jun 2021 21:54:34 +0200 Subject: [PATCH 2/9] Comments --- ws-server/src/server.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ws-server/src/server.rs b/ws-server/src/server.rs index 909fb5ff80..73b8516377 100644 --- a/ws-server/src/server.rs +++ b/ws-server/src/server.rs @@ -110,6 +110,8 @@ async fn background_task( if let (Cors::AllowList(list), Some(origin)) = (&cfg.cors, req.headers().origin) { if !list.iter().any(|o| o.as_bytes() == origin) { + // TODO: send `Response::Reject` with an appropriate status code + let error = format!("Origin denied: {}", String::from_utf8_lossy(origin)); log::warn!("{}", error); return Err(Error::Request(error)); @@ -201,7 +203,7 @@ struct Settings { max_request_body_size: u32, /// Maximum number of incoming connections allowed. max_connections: u64, - + /// Cross-origin policy by which to accept or deny incoming requests. cors: Cors, } From 1db6167b25ba5181a202a7c0985fe06ea2544cc9 Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Thu, 10 Jun 2021 22:10:05 +0200 Subject: [PATCH 3/9] Send a 403 on denied origin --- ws-server/src/server.rs | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/ws-server/src/server.rs b/ws-server/src/server.rs index 73b8516377..569755c0b1 100644 --- a/ws-server/src/server.rs +++ b/ws-server/src/server.rs @@ -108,22 +108,21 @@ async fn background_task( let key = { let req = server.receive_request().await?; - if let (Cors::AllowList(list), Some(origin)) = (&cfg.cors, req.headers().origin) { - if !list.iter().any(|o| o.as_bytes() == origin) { - // TODO: send `Response::Reject` with an appropriate status code - - let error = format!("Origin denied: {}", String::from_utf8_lossy(origin)); - log::warn!("{}", error); - return Err(Error::Request(error)); - } - } - - req.key() + cfg.cors.verify_origin(req.headers().origin).map(|_| req.key()) }; - // Here we accept the client unconditionally. - let accept = Response::Accept { key, protocol: None }; - server.send_response(&accept).await?; + match key { + Ok(key) => { + let accept = Response::Accept { key, protocol: None }; + server.send_response(&accept).await?; + }, + Err(error) => { + let reject = Response::Reject { status_code: 403 }; + server.send_response(&reject).await?; + + return Err(error); + } + } // And we can finally transition to a websocket background_task. let (mut sender, mut receiver) = server.into_builder().finish(); @@ -196,6 +195,20 @@ enum Cors { AllowList(Arc<[String]>), } +impl Cors { + fn verify_origin(&self, origin: Option<&[u8]>) -> Result<(), Error> { + if let (Cors::AllowList(list), Some(origin)) = (self, origin) { + if !list.iter().any(|o| o.as_bytes() == origin) { + let error = format!("Origin denied: {}", String::from_utf8_lossy(origin)); + log::warn!("{}", error); + return Err(Error::Request(error)); + } + } + + Ok(()) + } +} + /// JSON-RPC Websocket server settings. #[derive(Debug, Clone)] struct Settings { From aadd9b94b521c7b0a68e05ce0cb11b276e597078 Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Thu, 10 Jun 2021 22:51:02 +0200 Subject: [PATCH 4/9] Noodling around with `set_allowed_origins` --- ws-server/src/server.rs | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/ws-server/src/server.rs b/ws-server/src/server.rs index 569755c0b1..790ec7a726 100644 --- a/ws-server/src/server.rs +++ b/ws-server/src/server.rs @@ -250,13 +250,20 @@ impl Builder { } /// Set a list of allowet `Origin` headers, connections comming in with a different - /// origin will be denied. + /// origin will be denied. Values should include protocol. /// - /// Values should include protocol: `"protocol://hostname"`. + /// ```rust + /// # let mut builder = jsonrpsee_ws_server::WsServerBuilder::default(); + /// builder.set_allowed_origins(vec!["https://example.com"]); + /// ``` /// - /// By default allows any Origin. - pub fn set_allowed_origins(mut self, list: impl AsRef<[String]>) -> Self { - self.settings.cors = Cors::AllowList(list.as_ref().into()); + /// By default allows any `Origin`. + pub fn set_allowed_origins(mut self, list: List) -> Self + where + List: IntoIterator, + Origin: Into, + { + self.settings.cors = Cors::AllowList(list.into_iter().map(Into::into).collect()); self } From a046a9ab972a3e1b0aa99b8a7a513c9f7bbd44a4 Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Tue, 15 Jun 2021 12:03:51 +0100 Subject: [PATCH 5/9] Error on empty list --- types/src/error.rs | 3 +++ ws-server/src/server.rs | 18 +++++++++++++----- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/types/src/error.rs b/types/src/error.rs index 607e193fde..aff03c5579 100644 --- a/types/src/error.rs +++ b/types/src/error.rs @@ -78,6 +78,9 @@ pub enum Error { /// Configured max number of request slots exceeded. #[error("Configured max number of request slots exceeded")] MaxSlotsExceeded, + /// List passed into `set_allowed_origins` was empty + #[error("Must set at least one allowed origin")] + EmptyAllowedOrigins, /// Custom error. #[error("Custom error: {0}")] Custom(String), diff --git a/ws-server/src/server.rs b/ws-server/src/server.rs index 790ec7a726..8f8c752ccb 100644 --- a/ws-server/src/server.rs +++ b/ws-server/src/server.rs @@ -249,8 +249,9 @@ impl Builder { self } - /// Set a list of allowet `Origin` headers, connections comming in with a different - /// origin will be denied. Values should include protocol. + /// Set a list of allowed origins. During the handshake, the `Origin` header will be + /// checked against the list, connections without a matching origin will be denied. + /// Values should include protocol. /// /// ```rust /// # let mut builder = jsonrpsee_ws_server::WsServerBuilder::default(); @@ -258,13 +259,20 @@ impl Builder { /// ``` /// /// By default allows any `Origin`. - pub fn set_allowed_origins(mut self, list: List) -> Self + pub fn set_allowed_origins(mut self, list: List) -> Result where List: IntoIterator, Origin: Into, { - self.settings.cors = Cors::AllowList(list.into_iter().map(Into::into).collect()); - self + let list: Arc<_> = list.into_iter().map(Into::into).collect(); + + if list.len() == 0 { + return Err(Error::EmptyAllowedOrigins); + } + + self.settings.cors = Cors::AllowList(list); + + Ok(self) } /// Finalize the configuration of the server. Consumes the [`Builder`]. From 81f7b8358be674240ed48ab11a4525c8879c36f7 Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Tue, 15 Jun 2021 12:12:27 +0100 Subject: [PATCH 6/9] Soketto 0.6 --- test-utils/Cargo.toml | 2 +- test-utils/src/types.rs | 6 +++--- types/Cargo.toml | 3 +-- ws-client/Cargo.toml | 2 +- ws-server/Cargo.toml | 3 +-- 5 files changed, 7 insertions(+), 9 deletions(-) diff --git a/test-utils/Cargo.toml b/test-utils/Cargo.toml index 9915300453..aca58d3cc3 100644 --- a/test-utils/Cargo.toml +++ b/test-utils/Cargo.toml @@ -15,6 +15,6 @@ hyper = { version = "0.14", features = ["full"] } log = "0.4" serde = { version = "1", default-features = false, features = ["derive"] } serde_json = "1" -soketto = "0.5" +soketto = "0.6" tokio = { version = "1", features = ["net", "rt-multi-thread", "macros", "time"] } tokio-util = { version = "0.6", features = ["compat"] } diff --git a/test-utils/src/types.rs b/test-utils/src/types.rs index 419a937a41..5c5aa8c92c 100644 --- a/test-utils/src/types.rs +++ b/test-utils/src/types.rs @@ -199,12 +199,12 @@ async fn server_backend(listener: tokio::net::TcpListener, mut exit: Receiver<() async fn connection_task(socket: tokio::net::TcpStream, mode: ServerMode, mut exit: Receiver<()>) { let mut server = Server::new(socket.compat()); - let websocket_key = match server.receive_request().await { - Ok(req) => req.into_key(), + let key = match server.receive_request().await { + Ok(req) => req.key(), Err(_) => return, }; - let accept = server.send_response(&Response::Accept { key: &websocket_key, protocol: None }).await; + let accept = server.send_response(&Response::Accept { key, protocol: None }).await; if accept.is_err() { return; diff --git a/types/Cargo.toml b/types/Cargo.toml index f62a3573af..6964760be6 100644 --- a/types/Cargo.toml +++ b/types/Cargo.toml @@ -18,6 +18,5 @@ log = { version = "0.4", default-features = false } serde = { version = "1", default-features = false, features = ["derive"] } serde_json = { version = "1", default-features = false, features = ["alloc", "raw_value", "std"] } thiserror = "1.0" -# soketto = "0.5" -soketto = { path = "../../soketto" } +soketto = "0.6" hyper = "0.14" diff --git a/ws-client/Cargo.toml b/ws-client/Cargo.toml index 3a52d54bb7..04949e29bc 100644 --- a/ws-client/Cargo.toml +++ b/ws-client/Cargo.toml @@ -27,7 +27,7 @@ jsonrpsee-types = { path = "../types", version = "0.2.0" } log = "0.4" serde = "1" serde_json = "1" -soketto = "0.5" +soketto = "0.6" pin-project = "1" thiserror = "1" url = "2" diff --git a/ws-server/Cargo.toml b/ws-server/Cargo.toml index 663f10cbe5..6601735702 100644 --- a/ws-server/Cargo.toml +++ b/ws-server/Cargo.toml @@ -19,8 +19,7 @@ log = "0.4" rustc-hash = "1.1.0" serde = { version = "1", default-features = false, features = ["derive"] } serde_json = { version = "1", features = ["raw_value"] } -# soketto = "0.5" -soketto = { path = "../../soketto" } +soketto = "0.6" tokio = { version = "1", features = ["net", "rt-multi-thread", "macros"] } tokio-stream = { version = "0.1.1", features = ["net"] } tokio-util = { version = "0.6", features = ["compat"] } From 723958691fc5fe4d3ced4c87a752dd546243b055 Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Tue, 15 Jun 2021 12:18:55 +0100 Subject: [PATCH 7/9] fmt --- ws-server/src/server.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/ws-server/src/server.rs b/ws-server/src/server.rs index 8f8c752ccb..e0b0815a6f 100644 --- a/ws-server/src/server.rs +++ b/ws-server/src/server.rs @@ -115,7 +115,7 @@ async fn background_task( Ok(key) => { let accept = Response::Accept { key, protocol: None }; server.send_response(&accept).await?; - }, + } Err(error) => { let reject = Response::Reject { status_code: 403 }; server.send_response(&reject).await?; @@ -222,11 +222,7 @@ struct Settings { impl Default for Settings { fn default() -> Self { - Self { - max_request_body_size: TEN_MB_SIZE_BYTES, - max_connections: MAX_CONNECTIONS, - cors: Cors::AllowAny, - } + Self { max_request_body_size: TEN_MB_SIZE_BYTES, max_connections: MAX_CONNECTIONS, cors: Cors::AllowAny } } } From bdeeab96ce07b4b3b4a1611c7d72a32bad546e98 Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Tue, 15 Jun 2021 12:33:42 +0100 Subject: [PATCH 8/9] Add `Builder::allow_all_origins`, clarify doc comments --- ws-server/src/server.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/ws-server/src/server.rs b/ws-server/src/server.rs index e0b0815a6f..54ab9d1297 100644 --- a/ws-server/src/server.rs +++ b/ws-server/src/server.rs @@ -255,6 +255,8 @@ impl Builder { /// ``` /// /// By default allows any `Origin`. + /// + /// Will return an error if `list` is empty. Use [`allow_all_origins`](Builder::allow_all_origins) to restore the default. pub fn set_allowed_origins(mut self, list: List) -> Result where List: IntoIterator, @@ -271,6 +273,13 @@ impl Builder { Ok(self) } + /// Restores the default behavior of allowing connections with `Origin` header + /// containing any value. This will undo any list set by [`set_allowed_origins`](Builder::set_allowed_origins). + pub fn allow_all_origins(mut self) -> Self { + self.settings.cors = Cors::AllowAny; + self + } + /// Finalize the configuration of the server. Consumes the [`Builder`]. pub async fn build(self, addr: impl ToSocketAddrs) -> Result { let listener = TcpListener::bind(addr).await?; From b82edf75fbe3bb94c3c285f99e1e85283660a7fd Mon Sep 17 00:00:00 2001 From: Maciej Hirsz Date: Wed, 16 Jun 2021 10:57:09 +0100 Subject: [PATCH 9/9] Rename Cors -> AllowedOrigins, nits, no panic --- ws-server/src/server.rs | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/ws-server/src/server.rs b/ws-server/src/server.rs index 54ab9d1297..49365d25f0 100644 --- a/ws-server/src/server.rs +++ b/ws-server/src/server.rs @@ -75,12 +75,14 @@ impl Server { pub async fn start(self) { let mut incoming = TcpListenerStream::new(self.listener); let methods = Arc::new(self.methods); - // let cfg = self.cfg; let mut id = 0; while let Some(socket) = incoming.next().await { if let Ok(socket) = socket { - socket.set_nodelay(true).unwrap_or_else(|e| panic!("Could not set NODELAY on socket: {:?}", e)); + if let Err(e) = socket.set_nodelay(true) { + log::error!("Could not set NODELAY on socket: {:?}", e); + continue; + } if Arc::strong_count(&methods) > self.cfg.max_connections as usize { log::warn!("Too many connections. Try again in a while"); @@ -108,7 +110,7 @@ async fn background_task( let key = { let req = server.receive_request().await?; - cfg.cors.verify_origin(req.headers().origin).map(|_| req.key()) + cfg.allowed_origins.verify(req.headers().origin).map(|()| req.key()) }; match key { @@ -190,14 +192,14 @@ async fn background_task( } #[derive(Debug, Clone)] -enum Cors { - AllowAny, - AllowList(Arc<[String]>), +enum AllowedOrigins { + Any, + OneOf(Arc<[String]>), } -impl Cors { - fn verify_origin(&self, origin: Option<&[u8]>) -> Result<(), Error> { - if let (Cors::AllowList(list), Some(origin)) = (self, origin) { +impl AllowedOrigins { + fn verify(&self, origin: Option<&[u8]>) -> Result<(), Error> { + if let (AllowedOrigins::OneOf(list), Some(origin)) = (self, origin) { if !list.iter().any(|o| o.as_bytes() == origin) { let error = format!("Origin denied: {}", String::from_utf8_lossy(origin)); log::warn!("{}", error); @@ -217,12 +219,16 @@ struct Settings { /// Maximum number of incoming connections allowed. max_connections: u64, /// Cross-origin policy by which to accept or deny incoming requests. - cors: Cors, + allowed_origins: AllowedOrigins, } impl Default for Settings { fn default() -> Self { - Self { max_request_body_size: TEN_MB_SIZE_BYTES, max_connections: MAX_CONNECTIONS, cors: Cors::AllowAny } + Self { + max_request_body_size: TEN_MB_SIZE_BYTES, + max_connections: MAX_CONNECTIONS, + allowed_origins: AllowedOrigins::Any, + } } } @@ -268,7 +274,7 @@ impl Builder { return Err(Error::EmptyAllowedOrigins); } - self.settings.cors = Cors::AllowList(list); + self.settings.allowed_origins = AllowedOrigins::OneOf(list); Ok(self) } @@ -276,7 +282,7 @@ impl Builder { /// Restores the default behavior of allowing connections with `Origin` header /// containing any value. This will undo any list set by [`set_allowed_origins`](Builder::set_allowed_origins). pub fn allow_all_origins(mut self) -> Self { - self.settings.cors = Cors::AllowAny; + self.settings.allowed_origins = AllowedOrigins::Any; self }