From db9c559247dcd2cce2d19decd79b8e3a313c91f3 Mon Sep 17 00:00:00 2001 From: Quentin V <35984401+quentinv72@users.noreply.github.com> Date: Sun, 19 May 2024 08:36:24 -0400 Subject: [PATCH] refactor: Replace JwtSecret with alloy's version of it (#8299) Co-authored-by: Matthias Seitz --- Cargo.lock | 7 +- crates/node-core/src/args/rpc_server.rs | 2 +- crates/node-core/src/utils.rs | 4 +- crates/rpc/rpc-builder/src/auth.rs | 8 +- crates/rpc/rpc-builder/src/lib.rs | 8 +- crates/rpc/rpc-layer/Cargo.toml | 7 +- crates/rpc/rpc-layer/src/auth_client_layer.rs | 2 +- crates/rpc/rpc-layer/src/auth_layer.rs | 6 +- crates/rpc/rpc-layer/src/jwt_secret.rs | 425 ------------------ crates/rpc/rpc-layer/src/jwt_validator.rs | 2 +- crates/rpc/rpc-layer/src/lib.rs | 5 +- 11 files changed, 21 insertions(+), 455 deletions(-) delete mode 100644 crates/rpc/rpc-layer/src/jwt_secret.rs diff --git a/Cargo.lock b/Cargo.lock index 7097fb1e7..f482c94ce 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7783,19 +7783,14 @@ dependencies = [ name = "reth-rpc-layer" version = "0.2.0-beta.7" dependencies = [ + "alloy-rpc-types-engine", "assert_matches", "http 0.2.12", "http-body 0.4.6", "hyper 0.14.28", "jsonrpsee", - "jsonwebtoken 8.3.0", "pin-project", - "rand 0.8.5", - "reth-fs-util", - "reth-primitives", - "serde", "tempfile", - "thiserror", "tokio", "tower", "tracing", diff --git a/crates/node-core/src/args/rpc_server.rs b/crates/node-core/src/args/rpc_server.rs index c46446317..f67ef6acb 100644 --- a/crates/node-core/src/args/rpc_server.rs +++ b/crates/node-core/src/args/rpc_server.rs @@ -493,7 +493,7 @@ impl RethRpcConfig for RpcServerArgs { } fn rpc_secret_key(&self) -> Option { - self.rpc_jwtsecret.clone() + self.rpc_jwtsecret } } diff --git a/crates/node-core/src/utils.rs b/crates/node-core/src/utils.rs index 84a3bef7b..f9b4ff599 100644 --- a/crates/node-core/src/utils.rs +++ b/crates/node-core/src/utils.rs @@ -12,7 +12,7 @@ use reth_interfaces::p2p::{ use reth_network::NetworkManager; use reth_primitives::{BlockHashOrNumber, ChainSpec, HeadersDirection, SealedBlock, SealedHeader}; use reth_provider::BlockReader; -use reth_rpc_layer::{JwtError, JwtSecret}; +use reth_rpc_types::engine::{JwtError, JwtSecret}; use std::{ env::VarError, path::{Path, PathBuf}, @@ -33,7 +33,7 @@ pub fn get_or_create_jwt_secret_from_path(path: &Path) -> Result jsonrpsee::http_client::HttpClient> { // Create a middleware that adds a new JWT token to every request. - let secret_layer = AuthClientLayer::new(self.secret.clone()); + let secret_layer = AuthClientLayer::new(self.secret); let middleware = tower::ServiceBuilder::default().layer(secret_layer); jsonrpsee::http_client::HttpClientBuilder::default() .set_http_middleware(middleware) diff --git a/crates/rpc/rpc-builder/src/lib.rs b/crates/rpc/rpc-builder/src/lib.rs index 036127fdf..c8976f03c 100644 --- a/crates/rpc/rpc-builder/src/lib.rs +++ b/crates/rpc/rpc-builder/src/lib.rs @@ -1630,7 +1630,7 @@ impl RpcServerConfig { /// Creates the [AuthLayer] if any fn maybe_jwt_layer(&self) -> Option> { - self.jwt_secret.clone().map(|secret| AuthLayer::new(JwtAuthValidator::new(secret))) + self.jwt_secret.map(|secret| AuthLayer::new(JwtAuthValidator::new(secret))) } /// Builds the ws and http server(s). @@ -1701,7 +1701,7 @@ impl RpcServerConfig { http_local_addr: Some(addr), ws_local_addr: Some(addr), server: WsHttpServers::SamePort(server), - jwt_secret: self.jwt_secret.clone(), + jwt_secret: self.jwt_secret, }) } @@ -1760,7 +1760,7 @@ impl RpcServerConfig { http_local_addr, ws_local_addr, server: WsHttpServers::DifferentPort { http: http_server, ws: ws_server }, - jwt_secret: self.jwt_secret.clone(), + jwt_secret: self.jwt_secret, }) } @@ -2062,7 +2062,7 @@ impl RpcServer { } /// Return the JwtSecret of the server pub fn jwt(&self) -> Option { - self.ws_http.jwt_secret.clone() + self.ws_http.jwt_secret } /// Returns the [`SocketAddr`] of the ws server if started. diff --git a/crates/rpc/rpc-layer/Cargo.toml b/crates/rpc/rpc-layer/Cargo.toml index 546770f94..b08bb21e7 100644 --- a/crates/rpc/rpc-layer/Cargo.toml +++ b/crates/rpc/rpc-layer/Cargo.toml @@ -11,19 +11,14 @@ repository.workspace = true workspace = true [dependencies] -reth-primitives.workspace = true -reth-fs-util.workspace = true +alloy-rpc-types-engine.workspace = true http.workspace = true hyper.workspace = true tower.workspace = true http-body.workspace = true pin-project.workspace = true -jsonwebtoken = "8" -rand.workspace = true -serde.workspace = true -thiserror.workspace = true tracing.workspace = true [dev-dependencies] diff --git a/crates/rpc/rpc-layer/src/auth_client_layer.rs b/crates/rpc/rpc-layer/src/auth_client_layer.rs index 4c845796e..1b57608a7 100644 --- a/crates/rpc/rpc-layer/src/auth_client_layer.rs +++ b/crates/rpc/rpc-layer/src/auth_client_layer.rs @@ -24,7 +24,7 @@ impl Layer for AuthClientLayer { type Service = AuthClientService; fn layer(&self, inner: S) -> Self::Service { - AuthClientService::new(self.secret.clone(), inner) + AuthClientService::new(self.secret, inner) } } diff --git a/crates/rpc/rpc-layer/src/auth_layer.rs b/crates/rpc/rpc-layer/src/auth_layer.rs index 4803d2c98..bbf353f84 100644 --- a/crates/rpc/rpc-layer/src/auth_layer.rs +++ b/crates/rpc/rpc-layer/src/auth_layer.rs @@ -155,6 +155,9 @@ where #[cfg(test)] mod tests { + use super::*; + use crate::JwtAuthValidator; + use alloy_rpc_types_engine::{Claims, JwtError, JwtSecret}; use http::{header, Method, Request, StatusCode}; use hyper::{body, Body}; use jsonrpsee::{ @@ -166,9 +169,6 @@ mod tests { time::{SystemTime, UNIX_EPOCH}, }; - use super::AuthLayer; - use crate::{jwt_secret::Claims, JwtAuthValidator, JwtError, JwtSecret}; - const AUTH_PORT: u32 = 8551; const AUTH_ADDR: &str = "0.0.0.0"; const SECRET: &str = "f79ae8046bc11c9927afe911db7143c51a806c4a537cc08e0d37140b0192f430"; diff --git a/crates/rpc/rpc-layer/src/jwt_secret.rs b/crates/rpc/rpc-layer/src/jwt_secret.rs deleted file mode 100644 index b3d536078..000000000 --- a/crates/rpc/rpc-layer/src/jwt_secret.rs +++ /dev/null @@ -1,425 +0,0 @@ -use jsonwebtoken::{decode, errors::ErrorKind, Algorithm, DecodingKey, Validation}; -use rand::Rng; -use reth_fs_util::{self as fs, FsPathError}; -use reth_primitives::hex::{self, encode as hex_encode}; -use serde::{Deserialize, Serialize}; -use std::{ - path::Path, - str::FromStr, - time::{Duration, SystemTime, UNIX_EPOCH}, -}; -use thiserror::Error; - -/// Errors returned by the [`JwtSecret`] -#[derive(Error, Debug)] -pub enum JwtError { - /// An error encountered while decoding the hexadecimal string for the JWT secret. - #[error(transparent)] - JwtSecretHexDecodeError(#[from] hex::FromHexError), - - /// The JWT key length provided is invalid, expecting a specific length. - #[error("JWT key is expected to have a length of {0} digits. {1} digits key provided")] - InvalidLength(usize, usize), - - /// The signature algorithm used in the JWT is not supported. Only HS256 is supported. - #[error("unsupported signature algorithm. Only HS256 is supported")] - UnsupportedSignatureAlgorithm, - - /// The provided signature in the JWT is invalid. - #[error("provided signature is invalid")] - InvalidSignature, - - /// The "iat" (issued-at) claim in the JWT is not within the allowed ±60 seconds from the - /// current time. - #[error("IAT (issued-at) claim is not within ±60 seconds from the current time")] - InvalidIssuanceTimestamp, - - /// The Authorization header is missing or invalid in the context of JWT validation. - #[error("Authorization header is missing or invalid")] - MissingOrInvalidAuthorizationHeader, - - /// An error occurred during JWT decoding. - #[error("JWT decoding error: {0}")] - JwtDecodingError(String), - - /// An error related to file system path handling encountered during JWT operations. - #[error(transparent)] - JwtFsPathError(#[from] FsPathError), - - /// An I/O error occurred during JWT operations. - #[error(transparent)] - IOError(#[from] std::io::Error), -} - -/// Length of the hex-encoded 256 bit secret key. -/// A 256-bit encoded string in Rust has a length of 64 digits because each digit represents 4 bits -/// of data. In hexadecimal representation, each digit can have 16 possible values (0-9 and A-F), so -/// 4 bits can be represented using a single hex digit. Therefore, to represent a 256-bit string, -/// we need 64 hexadecimal digits (256 bits ÷ 4 bits per digit = 64 digits). -const JWT_SECRET_LEN: usize = 64; - -/// The JWT `iat` (issued-at) claim cannot exceed +-60 seconds from the current time. -const JWT_MAX_IAT_DIFF: Duration = Duration::from_secs(60); - -/// The execution layer client MUST support at least the following alg HMAC + SHA256 (HS256) -const JWT_SIGNATURE_ALGO: Algorithm = Algorithm::HS256; - -/// Value-object holding a reference to a hex-encoded 256-bit secret key. -/// A JWT secret key is used to secure JWT-based authentication. The secret key is -/// a shared secret between the server and the client and is used to calculate a digital signature -/// for the JWT, which is included in the JWT along with its payload. -/// -/// See also: [Secret key - Engine API specs](https://github.com/ethereum/execution-apis/blob/main/src/engine/authentication.md#key-distribution) -#[derive(Clone, PartialEq, Eq)] -pub struct JwtSecret([u8; 32]); - -impl JwtSecret { - /// Creates an instance of [`JwtSecret`]. - /// - /// Returns an error if one of the following applies: - /// - `hex` is not a valid hexadecimal string - /// - `hex` argument length is less than `JWT_SECRET_LEN` - /// - /// This strips the leading `0x`, if any. - pub fn from_hex>(hex: S) -> Result { - let hex: &str = hex.as_ref().trim().trim_start_matches("0x"); - if hex.len() != JWT_SECRET_LEN { - Err(JwtError::InvalidLength(JWT_SECRET_LEN, hex.len())) - } else { - let hex_bytes = hex::decode(hex)?; - // is 32bytes, see length check - let bytes = hex_bytes.try_into().expect("is expected len"); - Ok(JwtSecret(bytes)) - } - } - - /// Tries to load a [`JwtSecret`] from the specified file path. - /// I/O or secret validation errors might occur during read operations in the form of - /// a [`JwtError`]. - pub fn from_file(fpath: &Path) -> Result { - let hex = fs::read_to_string(fpath)?; - let secret = JwtSecret::from_hex(hex)?; - Ok(secret) - } - - /// Creates a random [`JwtSecret`] and tries to store it at the specified path. I/O errors might - /// occur during write operations in the form of a [`JwtError`] - pub fn try_create(fpath: &Path) -> Result { - if let Some(dir) = fpath.parent() { - // Create parent directory - fs::create_dir_all(dir)? - } - - let secret = JwtSecret::random(); - let bytes = &secret.0; - let hex = hex::encode(bytes); - fs::write(fpath, hex)?; - Ok(secret) - } - - /// Validates a JWT token along the following rules: - /// - The JWT signature is valid. - /// - The JWT is signed with the `HMAC + SHA256 (HS256)` algorithm. - /// - The JWT `iat` (issued-at) claim is a timestamp within +-60 seconds from the current time. - /// - /// See also: [JWT Claims - Engine API specs](https://github.com/ethereum/execution-apis/blob/main/src/engine/authentication.md#jwt-claims) - pub fn validate(&self, jwt: String) -> Result<(), JwtError> { - let mut validation = Validation::new(JWT_SIGNATURE_ALGO); - // ensure that the JWT has an `iat` claim - validation.set_required_spec_claims(&["iat"]); - let bytes = &self.0; - - match decode::(&jwt, &DecodingKey::from_secret(bytes), &validation) { - Ok(token) => { - if !token.claims.is_within_time_window() { - Err(JwtError::InvalidIssuanceTimestamp)? - } - } - Err(err) => match *err.kind() { - ErrorKind::InvalidSignature => Err(JwtError::InvalidSignature)?, - ErrorKind::InvalidAlgorithm => Err(JwtError::UnsupportedSignatureAlgorithm)?, - _ => { - let detail = format!("{err}"); - Err(JwtError::JwtDecodingError(detail))? - } - }, - }; - - Ok(()) - } - - /// Generates a random [`JwtSecret`] containing a hex-encoded 256 bit secret key. - pub fn random() -> Self { - let random_bytes: [u8; 32] = rand::thread_rng().gen(); - let secret = hex_encode(random_bytes); - JwtSecret::from_hex(secret).unwrap() - } - - /// Encode the header and claims given and sign the payload using the algorithm from the header - /// and the key. - /// - /// ```rust - /// use reth_rpc_layer::{Claims, JwtSecret}; - /// - /// let my_claims = Claims { iat: 0, exp: None }; - /// let secret = JwtSecret::random(); - /// let token = secret.encode(&my_claims).unwrap(); - /// ``` - pub fn encode(&self, claims: &Claims) -> Result { - let bytes = &self.0; - let key = jsonwebtoken::EncodingKey::from_secret(bytes); - let algo = jsonwebtoken::Header::new(Algorithm::HS256); - jsonwebtoken::encode(&algo, claims, &key) - } -} - -impl std::fmt::Debug for JwtSecret { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_tuple("JwtSecretHash").field(&"{{}}").finish() - } -} - -impl FromStr for JwtSecret { - type Err = JwtError; - - fn from_str(s: &str) -> Result { - JwtSecret::from_hex(s) - } -} - -/// Claims in JWT are used to represent a set of information about an entity. -/// Claims are essentially key-value pairs that are encoded as JSON objects and included in the -/// payload of a JWT. They are used to transmit information such as the identity of the entity, the -/// time the JWT was issued, and the expiration time of the JWT, among others. -/// -/// The Engine API spec requires that just the `iat` (issued-at) claim is provided. -/// It ignores claims that are optional or additional for this specification. -#[derive(Debug, Serialize, Deserialize)] -pub struct Claims { - /// The "iat" value MUST be a number containing a NumericDate value. - /// According to the RFC A NumericDate represents the number of seconds since - /// the UNIX_EPOCH. - /// - [`RFC-7519 - Spec`](https://www.rfc-editor.org/rfc/rfc7519#section-4.1.6) - /// - [`RFC-7519 - Notations`](https://www.rfc-editor.org/rfc/rfc7519#section-2) - pub iat: u64, - /// Expiration, if any - pub exp: Option, -} - -impl Claims { - fn is_within_time_window(&self) -> bool { - let now = SystemTime::now(); - let now_secs = now.duration_since(UNIX_EPOCH).unwrap().as_secs(); - now_secs.abs_diff(self.iat) <= JWT_MAX_IAT_DIFF.as_secs() - } -} - -#[cfg(test)] -mod tests { - use super::*; - use assert_matches::assert_matches; - use jsonwebtoken::{encode, EncodingKey, Header}; - use tempfile::tempdir; - - #[test] - fn from_hex() { - let key = "f79ae8046bc11c9927afe911db7143c51a806c4a537cc08e0d37140b0192f430"; - let secret: Result = JwtSecret::from_hex(key); - assert!(secret.is_ok()); - - let secret: Result = JwtSecret::from_hex(key); - assert!(secret.is_ok()); - } - - #[test] - fn original_key_integrity_across_transformations() { - let original = "f79ae8046bc11c9927afe911db7143c51a806c4a537cc08e0d37140b0192f430"; - let secret = JwtSecret::from_hex(original).unwrap(); - let bytes = &secret.0; - let computed = hex_encode(bytes); - assert_eq!(original, computed); - } - - #[test] - fn secret_has_64_hex_digits() { - let expected_len = 64; - let secret = JwtSecret::random(); - let hex = hex::encode(secret.0); - assert_eq!(hex.len(), expected_len); - } - - #[test] - fn creation_ok_hex_string_with_0x() { - let hex: String = - "0x7365637265747365637265747365637265747365637265747365637265747365".into(); - let result = JwtSecret::from_hex(hex); - assert!(result.is_ok()); - } - - #[test] - fn creation_error_wrong_len() { - let hex = "f79ae8046"; - let result = JwtSecret::from_hex(hex); - assert!(matches!(result, Err(JwtError::InvalidLength(_, _)))); - } - - #[test] - fn creation_error_wrong_hex_string() { - let hex: String = "This__________Is__________Not_______An____Hex_____________String".into(); - let result = JwtSecret::from_hex(hex); - assert!(matches!(result, Err(JwtError::JwtSecretHexDecodeError(_)))); - } - - #[test] - fn validation_ok() { - let secret = JwtSecret::random(); - let claims = Claims { iat: to_u64(SystemTime::now()), exp: Some(10000000000) }; - let jwt: String = secret.encode(&claims).unwrap(); - - let result = secret.validate(jwt); - - assert!(matches!(result, Ok(()))); - } - - #[test] - fn validation_error_iat_out_of_window() { - let secret = JwtSecret::random(); - - // Check past 'iat' claim more than 60 secs - let offset = Duration::from_secs(JWT_MAX_IAT_DIFF.as_secs() + 1); - let out_of_window_time = SystemTime::now().checked_sub(offset).unwrap(); - let claims = Claims { iat: to_u64(out_of_window_time), exp: Some(10000000000) }; - let jwt: String = secret.encode(&claims).unwrap(); - - let result = secret.validate(jwt); - - assert!(matches!(result, Err(JwtError::InvalidIssuanceTimestamp))); - - // Check future 'iat' claim more than 60 secs - let offset = Duration::from_secs(JWT_MAX_IAT_DIFF.as_secs() + 1); - let out_of_window_time = SystemTime::now().checked_add(offset).unwrap(); - let claims = Claims { iat: to_u64(out_of_window_time), exp: Some(10000000000) }; - let jwt: String = secret.encode(&claims).unwrap(); - - let result = secret.validate(jwt); - - assert!(matches!(result, Err(JwtError::InvalidIssuanceTimestamp))); - } - - #[test] - fn validation_error_wrong_signature() { - let secret_1 = JwtSecret::random(); - let claims = Claims { iat: to_u64(SystemTime::now()), exp: Some(10000000000) }; - let jwt: String = secret_1.encode(&claims).unwrap(); - - // A different secret will generate a different signature. - let secret_2 = JwtSecret::random(); - let result = secret_2.validate(jwt); - assert!(matches!(result, Err(JwtError::InvalidSignature))); - } - - #[test] - fn validation_error_unsupported_algorithm() { - let secret = JwtSecret::random(); - let bytes = &secret.0; - - let key = EncodingKey::from_secret(bytes); - let unsupported_algo = Header::new(Algorithm::HS384); - - let claims = Claims { iat: to_u64(SystemTime::now()), exp: Some(10000000000) }; - let jwt: String = encode(&unsupported_algo, &claims, &key).unwrap(); - let result = secret.validate(jwt); - - assert!(matches!(result, Err(JwtError::UnsupportedSignatureAlgorithm))); - } - - #[test] - fn valid_without_exp_claim() { - let secret = JwtSecret::random(); - - let claims = Claims { iat: to_u64(SystemTime::now()), exp: None }; - let jwt: String = secret.encode(&claims).unwrap(); - - let result = secret.validate(jwt); - - assert!(matches!(result, Ok(()))); - } - - #[test] - fn ephemeral_secret_created() { - let fpath: &Path = Path::new("secret0.hex"); - assert!(not_exists(fpath)); - JwtSecret::try_create(fpath).expect("A secret file should be created"); - assert!(exists(fpath)); - delete(fpath); - } - - #[test] - fn valid_secret_provided() { - let fpath = Path::new("secret1.hex"); - assert!(not_exists(fpath)); - - let secret = JwtSecret::random(); - write(fpath, &hex(&secret)); - - match JwtSecret::from_file(fpath) { - Ok(gen_secret) => { - delete(fpath); - assert_eq!(hex(&gen_secret), hex(&secret)); - } - Err(_) => { - delete(fpath); - } - } - } - - #[test] - fn invalid_hex_provided() { - let fpath = Path::new("secret2.hex"); - write(fpath, "invalid hex"); - let result = JwtSecret::from_file(fpath); - assert!(result.is_err()); - delete(fpath); - } - - #[test] - fn provided_file_not_exists() { - let fpath = Path::new("secret3.hex"); - let result = JwtSecret::from_file(fpath); - assert_matches!(result, - Err(JwtError::JwtFsPathError(FsPathError::Read { source: _, path })) if path == fpath.to_path_buf() - ); - assert!(!exists(fpath)); - } - - #[test] - fn provided_file_is_a_directory() { - let dir = tempdir().unwrap(); - let result = JwtSecret::from_file(dir.path()); - assert_matches!(result, Err(JwtError::JwtFsPathError(FsPathError::Read { source: _, path })) if path == dir.into_path()); - } - - fn hex(secret: &JwtSecret) -> String { - hex::encode(secret.0) - } - - fn delete(path: &Path) { - std::fs::remove_file(path).unwrap(); - } - - fn write(path: &Path, s: &str) { - std::fs::write(path, s).unwrap(); - } - - fn not_exists(path: &Path) -> bool { - !exists(path) - } - - fn exists(path: &Path) -> bool { - std::fs::metadata(path).is_ok() - } - - fn to_u64(time: SystemTime) -> u64 { - time.duration_since(UNIX_EPOCH).unwrap().as_secs() - } -} diff --git a/crates/rpc/rpc-layer/src/jwt_validator.rs b/crates/rpc/rpc-layer/src/jwt_validator.rs index 0f5124f9a..5bed6135d 100644 --- a/crates/rpc/rpc-layer/src/jwt_validator.rs +++ b/crates/rpc/rpc-layer/src/jwt_validator.rs @@ -26,7 +26,7 @@ impl AuthValidator for JwtAuthValidator { fn validate(&self, headers: &HeaderMap) -> Result<(), Response> { match get_bearer(headers) { - Some(jwt) => match self.secret.validate(jwt) { + Some(jwt) => match self.secret.validate(&jwt) { Ok(_) => Ok(()), Err(e) => { error!(target: "engine::jwt-validator", "Invalid JWT: {e}"); diff --git a/crates/rpc/rpc-layer/src/lib.rs b/crates/rpc/rpc-layer/src/lib.rs index dbe070096..a379461df 100644 --- a/crates/rpc/rpc-layer/src/lib.rs +++ b/crates/rpc/rpc-layer/src/lib.rs @@ -12,12 +12,13 @@ use http::{HeaderMap, Response}; mod auth_client_layer; mod auth_layer; -mod jwt_secret; mod jwt_validator; +// Export alloy JWT types +pub use alloy_rpc_types_engine::{Claims, JwtError, JwtSecret}; + pub use auth_client_layer::{secret_to_bearer_header, AuthClientLayer, AuthClientService}; pub use auth_layer::AuthLayer; -pub use jwt_secret::{Claims, JwtError, JwtSecret}; pub use jwt_validator::JwtAuthValidator; /// General purpose trait to validate Http Authorization headers. It's supposed to be integrated as