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 <teor@riseup.net>
This commit is contained in:
Alfredo Garcia 2023-06-14 16:02:55 -03:00 committed by GitHub
parent 17bd7884ea
commit 219d472270
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 161 additions and 8 deletions

View File

@ -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<str>,
params: impl AsRef<str>,
content_type: String,
) -> reqwest::Result<reqwest::Response> {
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<str>,
params: impl AsRef<str>,
) -> reqwest::Result<reqwest::Response> {
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,

View File

@ -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:
///
/// <https://github.com/paritytech/jsonrpc/blob/38af3c9439aa75481805edf6c05c6622a5ab1e70/http/src/handler.rs#L582-L584>
///
/// # 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
/// <https://www.invicti.com/blog/web-security/importance-content-type-header-http-requests/>
/// - Checking all the headers is secure, but only because hyper has custom code that just reads the first content-type header.
/// <https://github.com/hyperium/headers/blob/f01cc90cf8d601a716856bc9d29f47df92b779e4/src/common/content_type.rs#L102-L108>
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"),
);
}
}
}

View File

@ -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.)
///