From 219d47227076268fbaa9da0edfc5aa074a0c25f3 Mon Sep 17 00:00:00 2001 From: Alfredo Garcia Date: Wed, 14 Jun 2023 16:02:55 -0300 Subject: [PATCH] fix(compatibility): Replace or add RPC content type header when applicable (#6885) * ignore client supplied content-type header and use json always * rename method * add one more check to test * Add missing proptest-impl dependency from zebrad to zebra-rpc * change to replace only specific content type * remove cargo mods * refactor `insert_or_replace_content_type_header` * add security comments * allow variants of text/plain ocntent_type --------- Co-authored-by: teor --- zebra-node-services/src/rpc_client.rs | 38 +++++++++ .../src/server/http_request_compatibility.rs | 49 +++++++++-- zebrad/tests/acceptance.rs | 82 +++++++++++++++++++ 3 files changed, 161 insertions(+), 8 deletions(-) diff --git a/zebra-node-services/src/rpc_client.rs b/zebra-node-services/src/rpc_client.rs index e214af735..350b373aa 100644 --- a/zebra-node-services/src/rpc_client.rs +++ b/zebra-node-services/src/rpc_client.rs @@ -43,6 +43,44 @@ impl RpcRequestClient { .await } + /// Builds rpc request with a variable `content-type`. + pub async fn call_with_content_type( + &self, + method: impl AsRef, + params: impl AsRef, + content_type: String, + ) -> reqwest::Result { + let method = method.as_ref(); + let params = params.as_ref(); + + self.client + .post(format!("http://{}", &self.rpc_address)) + .body(format!( + r#"{{"jsonrpc": "2.0", "method": "{method}", "params": {params}, "id":123 }}"# + )) + .header("Content-Type", content_type) + .send() + .await + } + + /// Builds rpc request with no content type. + pub async fn call_with_no_content_type( + &self, + method: impl AsRef, + params: impl AsRef, + ) -> reqwest::Result { + let method = method.as_ref(); + let params = params.as_ref(); + + self.client + .post(format!("http://{}", &self.rpc_address)) + .body(format!( + r#"{{"jsonrpc": "2.0", "method": "{method}", "params": {params}, "id":123 }}"# + )) + .send() + .await + } + /// Builds rpc request and gets text from response pub async fn text_from_call( &self, diff --git a/zebra-rpc/src/server/http_request_compatibility.rs b/zebra-rpc/src/server/http_request_compatibility.rs index 63f445e91..99e604843 100644 --- a/zebra-rpc/src/server/http_request_compatibility.rs +++ b/zebra-rpc/src/server/http_request_compatibility.rs @@ -43,8 +43,8 @@ impl RequestMiddleware for FixHttpRequestMiddleware { ) -> jsonrpc_http_server::RequestMiddlewareAction { tracing::trace!(?request, "original HTTP request"); - // Fix the request headers - FixHttpRequestMiddleware::add_missing_content_type_header(request.headers_mut()); + // Fix the request headers if needed and we can do so. + FixHttpRequestMiddleware::insert_or_replace_content_type_header(request.headers_mut()); // Fix the request body let request = request.map(|body| { @@ -103,11 +103,44 @@ impl FixHttpRequestMiddleware { .replace(", \"jsonrpc\": \"1.0\"", "") } - /// If the `content-type` HTTP header is not present, - /// add an `application/json` content type header. - pub fn add_missing_content_type_header(headers: &mut hyper::header::HeaderMap) { - headers - .entry(hyper::header::CONTENT_TYPE) - .or_insert(hyper::header::HeaderValue::from_static("application/json")); + /// Insert or replace client supplied `content-type` HTTP header to `application/json` in the following cases: + /// + /// - no `content-type` supplied. + /// - supplied `content-type` start with `text/plain`, for example: + /// - `text/plain` + /// - `text/plain;` + /// - `text/plain; charset=utf-8` + /// + /// `application/json` is the only `content-type` accepted by the Zebra rpc endpoint: + /// + /// + /// + /// # Security + /// + /// - `content-type` headers exist so that applications know they are speaking the correct protocol with the correct format. + /// We can be a bit flexible, but there are some types (such as binary) we shouldn't allow. + /// In particular, the "application/x-www-form-urlencoded" header should be rejected, so browser forms can't be used to attack + /// a local RPC port. See "The Role of Routers in the CSRF Attack" in + /// + /// - Checking all the headers is secure, but only because hyper has custom code that just reads the first content-type header. + /// + pub fn insert_or_replace_content_type_header(headers: &mut hyper::header::HeaderMap) { + if !headers.contains_key(hyper::header::CONTENT_TYPE) + || headers + .get(hyper::header::CONTENT_TYPE) + .filter(|value| { + value + .to_str() + .ok() + .unwrap_or_default() + .starts_with("text/plain") + }) + .is_some() + { + headers.insert( + hyper::header::CONTENT_TYPE, + hyper::header::HeaderValue::from_static("application/json"), + ); + } } } diff --git a/zebrad/tests/acceptance.rs b/zebrad/tests/acceptance.rs index c9953fba1..57062d598 100644 --- a/zebrad/tests/acceptance.rs +++ b/zebrad/tests/acceptance.rs @@ -1467,6 +1467,88 @@ async fn rpc_endpoint(parallel_cpu_threads: bool) -> Result<()> { Ok(()) } +/// Test that the JSON-RPC endpoint responds to requests with different content types. +/// +/// This test ensures that the curl examples of zcashd rpc methods will also work in Zebra. +/// +/// https://zcash.github.io/rpc/getblockchaininfo.html +#[tokio::test] +async fn rpc_endpoint_client_content_type() -> Result<()> { + let _init_guard = zebra_test::init(); + if zebra_test::net::zebra_skip_network_tests() { + return Ok(()); + } + + // Write a configuration that has RPC listen_addr set + // [Note on port conflict](#Note on port conflict) + let mut config = random_known_rpc_port_config(true)?; + + let dir = testdir()?.with_config(&mut config)?; + let mut child = dir.spawn_child(args!["start"])?; + + // Wait until port is open. + child.expect_stdout_line_matches( + format!("Opened RPC endpoint at {}", config.rpc.listen_addr.unwrap()).as_str(), + )?; + + // Create an http client + let client = RpcRequestClient::new(config.rpc.listen_addr.unwrap()); + + // Call to `getinfo` RPC method with a no content type. + let res = client + .call_with_no_content_type("getinfo", "[]".to_string()) + .await?; + + // Zebra will insert valid `application/json` content type and succeed. + assert!(res.status().is_success()); + + // Call to `getinfo` RPC method with a `text/plain`. + let res = client + .call_with_content_type("getinfo", "[]".to_string(), "text/plain".to_string()) + .await?; + + // Zebra will replace to the valid `application/json` content type and succeed. + assert!(res.status().is_success()); + + // Call to `getinfo` RPC method with a `text/plain` content type as the zcashd rpc docs. + let res = client + .call_with_content_type("getinfo", "[]".to_string(), "text/plain;".to_string()) + .await?; + + // Zebra will replace to the valid `application/json` content type and succeed. + assert!(res.status().is_success()); + + // Call to `getinfo` RPC method with a `text/plain; other string` content type. + let res = client + .call_with_content_type( + "getinfo", + "[]".to_string(), + "text/plain; other string".to_string(), + ) + .await?; + + // Zebra will replace to the valid `application/json` content type and succeed. + assert!(res.status().is_success()); + + // Call to `getinfo` RPC method with a valid `application/json` content type. + let res = client + .call_with_content_type("getinfo", "[]".to_string(), "application/json".to_string()) + .await?; + + // Zebra will not replace valid content type and succeed. + assert!(res.status().is_success()); + + // Call to `getinfo` RPC method with invalid string as content type. + let res = client + .call_with_content_type("getinfo", "[]".to_string(), "whatever".to_string()) + .await?; + + // Zebra will not replace unrecognized content type and fail. + assert!(res.status().is_client_error()); + + Ok(()) +} + /// Test that Zebra's non-blocking logger works, by creating lots of debug output, but not reading the logs. /// Then make sure Zebra drops excess log lines. (Previously, it would block waiting for logs to be read.) ///