Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rpc): Cookie auth system for the RPC endpoint #8900

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
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
2 changes: 2 additions & 0 deletions Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6172,6 +6172,7 @@ dependencies = [
name = "zebra-node-services"
version = "1.0.0-beta.39"
dependencies = [
"base64 0.22.1",
"color-eyre",
"jsonrpc-core",
"reqwest",
Expand All @@ -6185,6 +6186,7 @@ dependencies = [
name = "zebra-rpc"
version = "1.0.0-beta.39"
dependencies = [
"base64 0.22.1",
"chrono",
"futures",
"hex",
Expand Down
3 changes: 3 additions & 0 deletions zebra-node-services/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ getblocktemplate-rpcs = [
# Tool and test features

rpc-client = [
"base64",
"color-eyre",
"jsonrpc-core",
"reqwest",
Expand All @@ -42,6 +43,7 @@ zebra-chain = { path = "../zebra-chain" , version = "1.0.0-beta.39" }
# Optional dependencies

# Tool and test feature rpc-client
base64 = { version = "0.22.1", optional = true }
color-eyre = { version = "0.6.3", optional = true }
jsonrpc-core = { version = "18.0.0", optional = true }
# Security: avoid default dependency on openssl
Expand All @@ -52,6 +54,7 @@ tokio = { version = "1.40.0", features = ["time", "sync"] }

[dev-dependencies]

base64 = "0.22.1"
color-eyre = "0.6.3"
jsonrpc-core = "18.0.0"
reqwest = { version = "0.11.26", default-features = false, features = ["rustls-tls"] }
Expand Down
12 changes: 11 additions & 1 deletion zebra-node-services/src/rpc_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use std::net::SocketAddr;

use base64::{engine::general_purpose::URL_SAFE, Engine as _};
use reqwest::Client;

use crate::BoxError;
Expand All @@ -13,14 +14,16 @@ use crate::BoxError;
pub struct RpcRequestClient {
client: Client,
rpc_address: SocketAddr,
auth_cookie: String,
}

impl RpcRequestClient {
/// Creates new RPCRequestSender
pub fn new(rpc_address: SocketAddr) -> Self {
pub fn new(rpc_address: SocketAddr, auth_cookie: String) -> Self {
Self {
client: Client::new(),
rpc_address,
auth_cookie,
}
}

Expand All @@ -39,6 +42,13 @@ impl RpcRequestClient {
r#"{{"jsonrpc": "2.0", "method": "{method}", "params": {params}, "id":123 }}"#
))
.header("Content-Type", "application/json")
.header(
"Authorization",
format!(
"Basic {}",
URL_SAFE.encode(format!("__cookie__:{}", &self.auth_cookie))
),
)
.send()
.await
}
Expand Down
7 changes: 4 additions & 3 deletions zebra-rpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ indexer-rpcs = [

# Mining RPC support
getblocktemplate-rpcs = [
"rand",
"zcash_address",
"zebra-consensus/getblocktemplate-rpcs",
"zebra-state/getblocktemplate-rpcs",
Expand Down Expand Up @@ -68,6 +67,10 @@ jsonrpc-http-server = "18.0.0"
serde_json = { version = "1.0.128", features = ["preserve_order"] }
indexmap = { version = "2.5.0", features = ["serde"] }

# RPC endpoint basic auth
base64 = "0.22.1"
rand = "0.8.5"

tokio = { version = "1.40.0", features = [
"time",
"rt-multi-thread",
Expand All @@ -92,8 +95,6 @@ nix = { version = "0.29.0", features = ["signal"] }

zcash_primitives = { workspace = true, features = ["transparent-inputs"] }

# Experimental feature getblocktemplate-rpcs
rand = { version = "0.8.5", optional = true }
# ECC deps used by getblocktemplate-rpcs feature
zcash_address = { workspace = true, optional = true}

Expand Down
10 changes: 9 additions & 1 deletion zebra-rpc/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
//! User-configurable RPC settings.

use std::net::SocketAddr;
use std::{net::SocketAddr, path::PathBuf};

use serde::{Deserialize, Serialize};

use zebra_chain::common::default_cache_dir;

pub mod mining;

/// RPC configuration section.
Expand Down Expand Up @@ -71,6 +73,9 @@ pub struct Config {
/// Test-only option that makes Zebra say it is at the chain tip,
/// no matter what the estimated height or local clock is.
pub debug_force_finished_sync: bool,

/// The directory where Zebra stores RPC cookies.
pub cookie_dir: PathBuf,
}

// This impl isn't derivable because it depends on features.
Expand All @@ -94,6 +99,9 @@ impl Default for Config {

// Debug options are always off by default.
debug_force_finished_sync: false,

//
Copy link
Member

@upbqdn upbqdn Oct 7, 2024

Choose a reason for hiding this comment

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

Suggested change
//
// Use the default cache dir for the auth cookie.

cookie_dir: default_cache_dir(),
}
}
}
12 changes: 12 additions & 0 deletions zebra-rpc/src/methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,10 @@ pub trait Rpc {
/// tags: control
#[rpc(name = "stop")]
fn stop(&self) -> Result<String>;

#[rpc(name = "unauthenticated")]
/// A dummy RPC method that just returns a non-authenticated RPC error.
Comment on lines +318 to +319
Copy link
Member

Choose a reason for hiding this comment

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

Optional
Suggested change
#[rpc(name = "unauthenticated")]
/// A dummy RPC method that just returns a non-authenticated RPC error.
/// A dummy RPC method that just returns a non-authenticated RPC error.
#[rpc(name = "unauthenticated")]

fn unauthenticated(&self) -> Result<()>;
}

/// RPC method implementations.
Expand Down Expand Up @@ -1383,6 +1387,14 @@ where
data: None,
})
}

fn unauthenticated(&self) -> Result<()> {
Err(Error {
code: ErrorCode::ServerError(401),
message: "unauthenticated method".to_string(),
data: None,
})
}
}

/// Returns the best chain tip height of `latest_chain_tip`,
Expand Down
7 changes: 6 additions & 1 deletion zebra-rpc/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ use crate::{
#[cfg(feature = "getblocktemplate-rpcs")]
use crate::methods::{GetBlockTemplateRpc, GetBlockTemplateRpcImpl};

pub mod cookie;
pub mod http_request_compatibility;
pub mod rpc_call_compatibility;

Expand Down Expand Up @@ -193,6 +194,9 @@ impl RpcServer {
parallel_cpu_threads = available_parallelism().map(usize::from).unwrap_or(1);
}

// generate a cookie
cookie::generate(config.cookie_dir.clone());

// The server is a blocking task, which blocks on executor shutdown.
// So we need to start it in a std::thread.
// (Otherwise tokio panics on RPC port conflict, which shuts down the RPC server.)
Expand All @@ -205,7 +209,7 @@ impl RpcServer {
.threads(parallel_cpu_threads)
// TODO: disable this security check if we see errors from lightwalletd
//.allowed_hosts(DomainsValidation::Disabled)
.request_middleware(FixHttpRequestMiddleware)
.request_middleware(FixHttpRequestMiddleware(config.clone()))
.start_http(&listen_addr)
.expect("Unable to start RPC server");

Expand Down Expand Up @@ -299,6 +303,7 @@ impl RpcServer {
span.in_scope(|| {
info!("Stopping RPC server");
close_handle.clone().close();
cookie::delete(); // delete the auth cookie
debug!("Stopped RPC server");
})
};
Expand Down
47 changes: 47 additions & 0 deletions zebra-rpc/src/server/cookie.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
//! Cookie-based authentication for the RPC server.

use base64::{engine::general_purpose::URL_SAFE, Engine as _};
use rand::RngCore;

use std::{
fs::{remove_file, File},
io::{Read, Write},
path::PathBuf,
};

/// The user field in the cookie (arbitrary, only for recognizability in debugging/logging purposes)
pub const COOKIEAUTH_USER: &str = "__cookie__";
/// Default name for auth cookie file */
pub const COOKIEAUTH_FILE: &str = ".cookie";

/// Generate a new auth cookie and return the encoded password.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Generate a new auth cookie and return the encoded password.
/// Generate a new auth cookie and store it in the given `cookie_dir`.

pub fn generate(cookie_dir: PathBuf) -> Option<()> {
Copy link
Member

Choose a reason for hiding this comment

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

Optional

Returning Result would be more expressive.

let mut data = [0u8; 32];
rand::thread_rng().fill_bytes(&mut data);
let encoded_password = URL_SAFE.encode(data);
let cookie_content = format!("{}:{}", COOKIEAUTH_USER, encoded_password);

let mut file = File::create(cookie_dir.join(COOKIEAUTH_FILE)).ok()?;
file.write_all(cookie_content.as_bytes()).ok()?;

tracing::info!("RPC auth cookie generated successfully");

Some(())
}

/// Get the encoded password from the auth cookie.
pub fn get(cookie_dir: PathBuf) -> Option<String> {
let mut file = File::open(cookie_dir.join(COOKIEAUTH_FILE)).ok()?;
let mut contents = String::new();
file.read_to_string(&mut contents).ok()?;

let parts: Vec<&str> = contents.split(":").collect();
Some(parts[1].to_string())
}

/// Delete the auth cookie.
pub fn delete() -> Option<()> {
Copy link
Member

Choose a reason for hiding this comment

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

Optional

Similarly here.

remove_file(COOKIEAUTH_FILE).ok()?;
tracing::info!("RPC auth cookie deleted successfully");
Some(())
}
59 changes: 57 additions & 2 deletions zebra-rpc/src/server/http_request_compatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@
//!
//! These fixes are applied at the HTTP level, before the RPC request is parsed.

use base64::{engine::general_purpose::URL_SAFE, Engine as _};
use futures::TryStreamExt;
use jsonrpc_http_server::{
hyper::{body::Bytes, header, Body, Request},
RequestMiddleware, RequestMiddlewareAction,
};

use crate::server::cookie;

/// HTTP [`RequestMiddleware`] with compatibility workarounds.
///
/// This middleware makes the following changes to HTTP requests:
Expand All @@ -34,13 +37,18 @@ use jsonrpc_http_server::{
/// Any user-specified data in RPC requests is hex or base58check encoded.
/// We assume lightwalletd validates data encodings before sending it on to Zebra.
/// So any fixes Zebra performs won't change user-specified data.
#[derive(Copy, Clone, Debug)]
pub struct FixHttpRequestMiddleware;
#[derive(Clone, Debug)]
pub struct FixHttpRequestMiddleware(pub crate::config::Config);

impl RequestMiddleware for FixHttpRequestMiddleware {
fn on_request(&self, mut request: Request<Body>) -> RequestMiddlewareAction {
tracing::trace!(?request, "original HTTP request");

// Check if the request is authenticated
if !self.check_credentials(request.headers_mut()) {
request = Self::unauthenticated(request);
}

// Fix the request headers if needed and we can do so.
FixHttpRequestMiddleware::insert_or_replace_content_type_header(request.headers_mut());

Expand Down Expand Up @@ -141,4 +149,51 @@ impl FixHttpRequestMiddleware {
);
}
}

/// Change the method name in the JSON request.
Copy link
Member

Choose a reason for hiding this comment

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

Optional

Could we move the contents of this fn to unauthenticated and remove the fn since we call it only from there, and it only uses a single-use, hard-coded method name?

fn change_method_name(data: String) -> String {
let mut json_data: serde_json::Value = serde_json::from_str(&data).expect("Invalid JSON");

if let Some(method) = json_data.get_mut("method") {
*method = serde_json::json!("unauthenticated");
}

serde_json::to_string(&json_data).expect("Failed to serialize JSON")
}

/// Modify the request name to be `unauthenticated`.
fn unauthenticated(request: Request<Body>) -> Request<Body> {
request.map(|body| {
let body = body.map_ok(|data| {
let mut data = String::from_utf8_lossy(data.as_ref()).to_string();
data = Self::change_method_name(data);
Bytes::from(data)
});

Body::wrap_stream(body)
})
}

/// Check if the request is authenticated.
pub fn check_credentials(&self, headers: &header::HeaderMap) -> bool {
headers
.get(header::AUTHORIZATION)
.and_then(|auth_header| auth_header.to_str().ok())
.and_then(|auth| auth.split_whitespace().nth(1))
.and_then(|token| URL_SAFE.decode(token).ok())
.and_then(|decoded| String::from_utf8(decoded).ok())
.and_then(|decoded_str| {
decoded_str
.split(':')
.nth(1)
.map(|password| password.to_string())
})
.map_or(false, |password| {
if let Some(cookie_password) = cookie::get(self.0.cookie_dir.clone()) {
Copy link
Member

Choose a reason for hiding this comment

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

We read the cookie from the disk on each check here and only rely on the OS caching the disk reads. We could keep the cookie in the memory instead. For example, by using lazy_static! in cookie::get.

cookie_password == password
} else {
false
}
})
}
}
4 changes: 4 additions & 0 deletions zebra-rpc/src/server/tests/vectors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ fn rpc_server_spawn(parallel_cpu_threads: bool) {
indexer_listen_addr: None,
parallel_cpu_threads: if parallel_cpu_threads { 2 } else { 1 },
debug_force_finished_sync: false,
cookie_dir: Default::default(),
};

let rt = tokio::runtime::Runtime::new().unwrap();
Expand Down Expand Up @@ -134,6 +135,7 @@ fn rpc_server_spawn_unallocated_port(parallel_cpu_threads: bool, do_shutdown: bo
indexer_listen_addr: None,
parallel_cpu_threads: if parallel_cpu_threads { 0 } else { 1 },
debug_force_finished_sync: false,
cookie_dir: Default::default(),
};

let rt = tokio::runtime::Runtime::new().unwrap();
Expand Down Expand Up @@ -215,6 +217,7 @@ fn rpc_server_spawn_port_conflict() {
indexer_listen_addr: None,
parallel_cpu_threads: 1,
debug_force_finished_sync: false,
cookie_dir: Default::default(),
};

let rt = tokio::runtime::Runtime::new().unwrap();
Expand Down Expand Up @@ -326,6 +329,7 @@ fn rpc_server_spawn_port_conflict_parallel_auto() {
indexer_listen_addr: None,
parallel_cpu_threads: 2,
debug_force_finished_sync: false,
cookie_dir: Default::default(),
};

let rt = tokio::runtime::Runtime::new().unwrap();
Expand Down
Loading
Loading