From cc628fc0f350e43b856d87c4ea9e10e7e773ee7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Wed, 16 Apr 2025 15:12:46 -0300 Subject: [PATCH 01/12] Update JWTs on reload endpoint --- crates/signer/src/service.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/signer/src/service.rs b/crates/signer/src/service.rs index 28a1d934..4a985725 100644 --- a/crates/signer/src/service.rs +++ b/crates/signer/src/service.rs @@ -270,6 +270,8 @@ async fn handle_reload( } }; + state.jwts = config.jwts.clone().into(); + let new_manager = match start_manager(config).await { Ok(manager) => manager, Err(err) => { From 6aed0d514ed4e762e9c144f5326af08122528584 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Mon, 21 Apr 2025 14:35:04 -0300 Subject: [PATCH 02/12] Add revoke JWT endpoint --- crates/common/src/commit/constants.rs | 1 + crates/common/src/commit/request.rs | 14 +++++++++++--- crates/signer/src/error.rs | 4 ++++ crates/signer/src/service.rs | 27 ++++++++++++++++++++------- 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/crates/common/src/commit/constants.rs b/crates/common/src/commit/constants.rs index 7c9f948c..1e10975f 100644 --- a/crates/common/src/commit/constants.rs +++ b/crates/common/src/commit/constants.rs @@ -3,3 +3,4 @@ pub const REQUEST_SIGNATURE_PATH: &str = "/signer/v1/request_signature"; pub const GENERATE_PROXY_KEY_PATH: &str = "/signer/v1/generate_proxy_key"; pub const STATUS_PATH: &str = "/status"; pub const RELOAD_PATH: &str = "/reload"; +pub const REVOKE_JWT: &str = "/revoke_jwt"; diff --git a/crates/common/src/commit/request.rs b/crates/common/src/commit/request.rs index b8843234..5c2b058c 100644 --- a/crates/common/src/commit/request.rs +++ b/crates/common/src/commit/request.rs @@ -14,8 +14,11 @@ use tree_hash::TreeHash; use tree_hash_derive::TreeHash; use crate::{ - constants::COMMIT_BOOST_DOMAIN, error::BlstErrorWrapper, signature::verify_signed_message, - signer::BlsPublicKey, types::Chain, + constants::COMMIT_BOOST_DOMAIN, + error::BlstErrorWrapper, + signature::verify_signed_message, + signer::BlsPublicKey, + types::{Chain, ModuleId}, }; pub trait ProxyId: AsRef<[u8]> + Debug + Clone + Copy + TreeHash + Display {} @@ -198,6 +201,11 @@ pub struct GetPubkeysResponse { pub keys: Vec, } +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RevokeJWTRequest { + pub module_id: ModuleId, +} + /// Map of consensus pubkeys to proxies #[derive(Debug, Clone, Deserialize, Serialize)] pub struct ConsensusProxyMap { @@ -288,7 +296,7 @@ mod tests { let _: SignedProxyDelegationBls = serde_json::from_str(data).unwrap(); - let data = r#"{ + let data = r#"{ "message": { "delegator": "0xa3366b54f28e4bf1461926a3c70cdb0ec432b5c92554ecaae3742d33fb33873990cbed1761c68020e6d3c14d30a22050", "proxy": "0x4ca9939a8311a7cab3dde201b70157285fa81a9d" diff --git a/crates/signer/src/error.rs b/crates/signer/src/error.rs index 477e9e42..db319542 100644 --- a/crates/signer/src/error.rs +++ b/crates/signer/src/error.rs @@ -25,6 +25,9 @@ pub enum SignerModuleError { #[error("Dirk signer does not support this operation")] DirkNotSupported, + #[error("module id not found")] + ModuleIdNotFound, + #[error("internal error: {0}")] Internal(String), } @@ -45,6 +48,7 @@ impl IntoResponse for SignerModuleError { (StatusCode::INTERNAL_SERVER_ERROR, "internal error".to_string()) } SignerModuleError::SignerError(err) => (StatusCode::BAD_REQUEST, err.to_string()), + SignerModuleError::ModuleIdNotFound => (StatusCode::NOT_FOUND, self.to_string()), } .into_response() } diff --git a/crates/signer/src/service.rs b/crates/signer/src/service.rs index 4a985725..1cfde17f 100644 --- a/crates/signer/src/service.rs +++ b/crates/signer/src/service.rs @@ -13,11 +13,11 @@ use cb_common::{ commit::{ constants::{ GENERATE_PROXY_KEY_PATH, GET_PUBKEYS_PATH, RELOAD_PATH, REQUEST_SIGNATURE_PATH, - STATUS_PATH, + REVOKE_JWT, STATUS_PATH, }, request::{ - EncryptionScheme, GenerateProxyRequest, GetPubkeysResponse, SignConsensusRequest, - SignProxyRequest, SignRequest, + EncryptionScheme, GenerateProxyRequest, GetPubkeysResponse, RevokeJWTRequest, + SignConsensusRequest, SignProxyRequest, SignRequest, }, }, config::StartSignerConfig, @@ -47,7 +47,7 @@ struct SigningState { manager: Arc>, /// Map of modules ids to JWT secrets. This also acts as registry of all /// modules running - jwts: Arc>, + jwts: Arc>>, } impl SigningService { @@ -61,7 +61,7 @@ impl SigningService { let state = SigningState { manager: Arc::new(RwLock::new(start_manager(config.clone()).await?)), - jwts: config.jwts.into(), + jwts: Arc::new(RwLock::new(config.jwts)), }; let loaded_consensus = state.manager.read().await.available_consensus_signers(); @@ -77,6 +77,7 @@ impl SigningService { .route(GENERATE_PROXY_KEY_PATH, post(handle_generate_proxy)) .route_layer(middleware::from_fn_with_state(state.clone(), jwt_auth)) .route(RELOAD_PATH, post(handle_reload)) + .route(REVOKE_JWT, post(handle_revoke_jwt)) .with_state(state.clone()) .route_layer(middleware::from_fn(log_request)) .route(STATUS_PATH, get(handle_status)); @@ -108,7 +109,8 @@ async fn jwt_auth( SignerModuleError::Unauthorized })?; - let jwt_secret = state.jwts.get(&module_id).ok_or_else(|| { + let guard = state.jwts.read().await; + let jwt_secret = guard.get(&module_id).ok_or_else(|| { error!("Unauthorized request. Was the module started correctly?"); SignerModuleError::Unauthorized })?; @@ -270,7 +272,7 @@ async fn handle_reload( } }; - state.jwts = config.jwts.clone().into(); + state.jwts = Arc::new(RwLock::new(config.jwts.clone())); let new_manager = match start_manager(config).await { Ok(manager) => manager, @@ -285,6 +287,17 @@ async fn handle_reload( Ok(StatusCode::OK) } +async fn handle_revoke_jwt( + State(state): State, + Json(request): Json, +) -> Result { + let mut guard = state.jwts.write().await; + guard + .remove(&request.module_id) + .ok_or(SignerModuleError::ModuleIdNotFound) + .map(|_| StatusCode::OK) +} + async fn start_manager(config: StartSignerConfig) -> eyre::Result { let proxy_store = if let Some(store) = config.store.clone() { Some(store.init_from_env()?) From 2f9fb562593ae8b6043ca5e8415c4a8981a8719e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Mon, 21 Apr 2025 15:20:26 -0300 Subject: [PATCH 03/12] Add admin JWT for admin endpoints --- crates/cli/src/docker_init.rs | 12 +++++----- crates/common/src/config/constants.rs | 1 + crates/common/src/config/signer.rs | 5 ++++- crates/common/src/config/utils.rs | 7 +++--- crates/common/src/types.rs | 6 +++++ crates/common/src/utils.rs | 20 ++++++++++++++++- crates/signer/src/service.rs | 32 +++++++++++++++++++++++---- 7 files changed, 69 insertions(+), 14 deletions(-) diff --git a/crates/cli/src/docker_init.rs b/crates/cli/src/docker_init.rs index 4453f597..fcc79b98 100644 --- a/crates/cli/src/docker_init.rs +++ b/crates/cli/src/docker_init.rs @@ -7,11 +7,11 @@ use std::{ use cb_common::{ config::{ load_optional_env_var, CommitBoostConfig, LogsSettings, ModuleKind, SignerConfig, - SignerType, BUILDER_PORT_ENV, BUILDER_URLS_ENV, CHAIN_SPEC_ENV, CONFIG_DEFAULT, CONFIG_ENV, - DIRK_CA_CERT_DEFAULT, DIRK_CA_CERT_ENV, DIRK_CERT_DEFAULT, DIRK_CERT_ENV, - DIRK_DIR_SECRETS_DEFAULT, DIRK_DIR_SECRETS_ENV, DIRK_KEY_DEFAULT, DIRK_KEY_ENV, JWTS_ENV, - LOGS_DIR_DEFAULT, LOGS_DIR_ENV, METRICS_PORT_ENV, MODULE_ID_ENV, MODULE_JWT_ENV, - PBS_ENDPOINT_ENV, PBS_MODULE_NAME, PROXY_DIR_DEFAULT, PROXY_DIR_ENV, + SignerType, ADMIN_JWT_ENV, BUILDER_PORT_ENV, BUILDER_URLS_ENV, CHAIN_SPEC_ENV, + CONFIG_DEFAULT, CONFIG_ENV, DIRK_CA_CERT_DEFAULT, DIRK_CA_CERT_ENV, DIRK_CERT_DEFAULT, + DIRK_CERT_ENV, DIRK_DIR_SECRETS_DEFAULT, DIRK_DIR_SECRETS_ENV, DIRK_KEY_DEFAULT, + DIRK_KEY_ENV, JWTS_ENV, LOGS_DIR_DEFAULT, LOGS_DIR_ENV, METRICS_PORT_ENV, MODULE_ID_ENV, + MODULE_JWT_ENV, PBS_ENDPOINT_ENV, PBS_MODULE_NAME, PROXY_DIR_DEFAULT, PROXY_DIR_ENV, PROXY_DIR_KEYS_DEFAULT, PROXY_DIR_KEYS_ENV, PROXY_DIR_SECRETS_DEFAULT, PROXY_DIR_SECRETS_ENV, SIGNER_DEFAULT, SIGNER_DIR_KEYS_DEFAULT, SIGNER_DIR_KEYS_ENV, SIGNER_DIR_SECRETS_DEFAULT, SIGNER_DIR_SECRETS_ENV, SIGNER_JWT_SECRET_ENV, SIGNER_KEYS_ENV, @@ -334,6 +334,7 @@ pub async fn handle_docker_init(config_path: PathBuf, output_dir: PathBuf) -> Re let mut signer_envs = IndexMap::from([ get_env_val(CONFIG_ENV, CONFIG_DEFAULT), get_env_same(JWTS_ENV), + get_env_same(ADMIN_JWT_ENV), get_env_uval(SIGNER_PORT_ENV, signer_port as u64), ]); @@ -360,6 +361,7 @@ pub async fn handle_docker_init(config_path: PathBuf, output_dir: PathBuf) -> Re // write jwts to env envs.insert(JWTS_ENV.into(), format_comma_separated(&jwts)); + envs.insert(ADMIN_JWT_ENV.into(), random_jwt_secret()); // volumes let mut volumes = vec![config_volume.clone()]; diff --git a/crates/common/src/config/constants.rs b/crates/common/src/config/constants.rs index 422af7e7..d717dc70 100644 --- a/crates/common/src/config/constants.rs +++ b/crates/common/src/config/constants.rs @@ -37,6 +37,7 @@ pub const SIGNER_PORT_ENV: &str = "CB_SIGNER_PORT"; /// Comma separated list module_id=jwt_secret pub const JWTS_ENV: &str = "CB_JWTS"; +pub const ADMIN_JWT_ENV: &str = "CB_ADMIN_JWT"; /// The JWT secret for the signer to validate the modules requests pub const SIGNER_JWT_SECRET_ENV: &str = "CB_SIGNER_JWT_SECRET"; diff --git a/crates/common/src/config/signer.rs b/crates/common/src/config/signer.rs index 9df6b948..ba0f0f87 100644 --- a/crates/common/src/config/signer.rs +++ b/crates/common/src/config/signer.rs @@ -89,6 +89,7 @@ pub struct StartSignerConfig { pub store: Option, pub server_port: u16, pub jwts: HashMap, + pub admin_secret: String, pub dirk: Option, } @@ -96,7 +97,7 @@ impl StartSignerConfig { pub fn load_from_env() -> Result { let config = CommitBoostConfig::from_env_path()?; - let jwts = load_jwt_secrets()?; + let (admin_secret, jwts) = load_jwt_secrets()?; let server_port = load_env_var(SIGNER_PORT_ENV)?.parse()?; let signer = config.signer.ok_or_eyre("Signer config is missing")?.inner; @@ -107,6 +108,7 @@ impl StartSignerConfig { loader: Some(loader), server_port, jwts, + admin_secret, store, dirk: None, }), @@ -135,6 +137,7 @@ impl StartSignerConfig { chain: config.chain, server_port, jwts, + admin_secret, loader: None, store, dirk: Some(DirkConfig { diff --git a/crates/common/src/config/utils.rs b/crates/common/src/config/utils.rs index 67c367c5..d7c6b68f 100644 --- a/crates/common/src/config/utils.rs +++ b/crates/common/src/config/utils.rs @@ -3,7 +3,7 @@ use std::{collections::HashMap, path::Path}; use eyre::{bail, Context, Result}; use serde::de::DeserializeOwned; -use super::JWTS_ENV; +use super::{ADMIN_JWT_ENV, JWTS_ENV}; use crate::types::ModuleId; pub fn load_env_var(env: &str) -> Result { @@ -25,9 +25,10 @@ pub fn load_file_from_env(env: &str) -> Result { } /// Loads a map of module id -> jwt secret from a json env -pub fn load_jwt_secrets() -> Result> { +pub fn load_jwt_secrets() -> Result<(String, HashMap)> { + let admin_jwt = std::env::var(ADMIN_JWT_ENV).wrap_err(format!("{ADMIN_JWT_ENV} is not set"))?; let jwt_secrets = std::env::var(JWTS_ENV).wrap_err(format!("{JWTS_ENV} is not set"))?; - decode_string_to_map(&jwt_secrets) + decode_string_to_map(&jwt_secrets).map(|secrets| (admin_jwt, secrets)) } fn decode_string_to_map(raw: &str) -> Result> { diff --git a/crates/common/src/types.rs b/crates/common/src/types.rs index 5293a789..3d07e89c 100644 --- a/crates/common/src/types.rs +++ b/crates/common/src/types.rs @@ -23,6 +23,12 @@ pub struct JwtClaims { pub module: String, } +#[derive(Debug, Serialize, Deserialize)] +pub struct JwtAdmin { + pub exp: u64, + pub admin: bool, +} + #[derive(Clone, Copy, PartialEq, Eq)] pub enum Chain { Mainnet, diff --git a/crates/common/src/utils.rs b/crates/common/src/utils.rs index 37119580..4f40d09b 100644 --- a/crates/common/src/utils.rs +++ b/crates/common/src/utils.rs @@ -26,7 +26,7 @@ use crate::{ config::LogsSettings, constants::SIGNER_JWT_EXPIRATION, pbs::HEADER_VERSION_VALUE, - types::{Chain, Jwt, JwtClaims, ModuleId}, + types::{Chain, Jwt, JwtAdmin, JwtClaims, ModuleId}, }; const MILLIS_PER_SECOND: u64 = 1_000; @@ -320,6 +320,24 @@ pub fn validate_jwt(jwt: Jwt, secret: &str) -> eyre::Result<()> { .map_err(From::from) } +/// Validate an admin JWT with the given secret +pub fn validate_admin_jwt(jwt: Jwt, secret: &str) -> eyre::Result<()> { + let mut validation = jsonwebtoken::Validation::default(); + validation.leeway = 10; + + let token = jsonwebtoken::decode::( + jwt.as_str(), + &jsonwebtoken::DecodingKey::from_secret(secret.as_ref()), + &validation, + )?; + + if token.claims.admin { + Ok(()) + } else { + eyre::bail!("Token is not admin") + } +} + /// Generates a random string pub fn random_jwt_secret() -> String { rand::rng().sample_iter(&Alphanumeric).take(32).map(char::from).collect() diff --git a/crates/signer/src/service.rs b/crates/signer/src/service.rs index 1cfde17f..2be57193 100644 --- a/crates/signer/src/service.rs +++ b/crates/signer/src/service.rs @@ -23,7 +23,7 @@ use cb_common::{ config::StartSignerConfig, constants::{COMMIT_BOOST_COMMIT, COMMIT_BOOST_VERSION}, types::{Chain, Jwt, ModuleId}, - utils::{decode_jwt, validate_jwt}, + utils::{decode_jwt, validate_admin_jwt, validate_jwt}, }; use cb_metrics::provider::MetricsProvider; use eyre::Context; @@ -48,6 +48,8 @@ struct SigningState { /// Map of modules ids to JWT secrets. This also acts as registry of all /// modules running jwts: Arc>>, + /// Secret for the admin JWT + admin_secret: Arc>, } impl SigningService { @@ -62,6 +64,7 @@ impl SigningService { let state = SigningState { manager: Arc::new(RwLock::new(start_manager(config.clone()).await?)), jwts: Arc::new(RwLock::new(config.jwts)), + admin_secret: Arc::new(RwLock::new(config.admin_secret)), }; let loaded_consensus = state.manager.read().await.available_consensus_signers(); @@ -71,21 +74,25 @@ impl SigningService { SigningService::init_metrics(config.chain)?; - let app = axum::Router::new() + let signer_app = axum::Router::new() .route(REQUEST_SIGNATURE_PATH, post(handle_request_signature)) .route(GET_PUBKEYS_PATH, get(handle_get_pubkeys)) .route(GENERATE_PROXY_KEY_PATH, post(handle_generate_proxy)) .route_layer(middleware::from_fn_with_state(state.clone(), jwt_auth)) + .with_state(state.clone()) + .route_layer(middleware::from_fn(log_request)); + + let admin_app = axum::Router::new() .route(RELOAD_PATH, post(handle_reload)) .route(REVOKE_JWT, post(handle_revoke_jwt)) + .route_layer(middleware::from_fn_with_state(state.clone(), admin_auth)) .with_state(state.clone()) .route_layer(middleware::from_fn(log_request)) .route(STATUS_PATH, get(handle_status)); - let address = SocketAddr::from(([0, 0, 0, 0], config.server_port)); let listener = TcpListener::bind(address).await?; - axum::serve(listener, app).await.wrap_err("signer server exited") + axum::serve(listener, signer_app.merge(admin_app)).await.wrap_err("signer server exited") } fn init_metrics(network: Chain) -> eyre::Result<()> { @@ -125,6 +132,22 @@ async fn jwt_auth( Ok(next.run(req).await) } +async fn admin_auth( + State(state): State, + TypedHeader(auth): TypedHeader>, + req: Request, + next: Next, +) -> Result { + let jwt: Jwt = auth.token().to_string().into(); + + validate_admin_jwt(jwt, &state.admin_secret.read().await).map_err(|e| { + error!("Unauthorized request. Invalid JWT: {e}"); + SignerModuleError::Unauthorized + })?; + + Ok(next.run(req).await) +} + /// Requests logging middleware layer async fn log_request(req: Request, next: Next) -> Result { let url = &req.uri().clone(); @@ -273,6 +296,7 @@ async fn handle_reload( }; state.jwts = Arc::new(RwLock::new(config.jwts.clone())); + state.admin_secret = Arc::new(RwLock::new(config.admin_secret.clone())); let new_manager = match start_manager(config).await { Ok(manager) => manager, From 03e034a83ac76400167d52ed3495d9bc05528244 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Mon, 21 Apr 2025 15:40:11 -0300 Subject: [PATCH 04/12] Renames --- crates/common/src/commit/constants.rs | 2 +- crates/signer/src/service.rs | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/crates/common/src/commit/constants.rs b/crates/common/src/commit/constants.rs index 1e10975f..ea9cd9bb 100644 --- a/crates/common/src/commit/constants.rs +++ b/crates/common/src/commit/constants.rs @@ -3,4 +3,4 @@ pub const REQUEST_SIGNATURE_PATH: &str = "/signer/v1/request_signature"; pub const GENERATE_PROXY_KEY_PATH: &str = "/signer/v1/generate_proxy_key"; pub const STATUS_PATH: &str = "/status"; pub const RELOAD_PATH: &str = "/reload"; -pub const REVOKE_JWT: &str = "/revoke_jwt"; +pub const REVOKE_MODULE_PATH: &str = "/revoke_jwt"; diff --git a/crates/signer/src/service.rs b/crates/signer/src/service.rs index 2be57193..6afd59cc 100644 --- a/crates/signer/src/service.rs +++ b/crates/signer/src/service.rs @@ -13,7 +13,7 @@ use cb_common::{ commit::{ constants::{ GENERATE_PROXY_KEY_PATH, GET_PUBKEYS_PATH, RELOAD_PATH, REQUEST_SIGNATURE_PATH, - REVOKE_JWT, STATUS_PATH, + REVOKE_MODULE_PATH, STATUS_PATH, }, request::{ EncryptionScheme, GenerateProxyRequest, GetPubkeysResponse, RevokeJWTRequest, @@ -84,7 +84,7 @@ impl SigningService { let admin_app = axum::Router::new() .route(RELOAD_PATH, post(handle_reload)) - .route(REVOKE_JWT, post(handle_revoke_jwt)) + .route(REVOKE_MODULE_PATH, post(handle_revoke_module)) .route_layer(middleware::from_fn_with_state(state.clone(), admin_auth)) .with_state(state.clone()) .route_layer(middleware::from_fn(log_request)) @@ -311,7 +311,7 @@ async fn handle_reload( Ok(StatusCode::OK) } -async fn handle_revoke_jwt( +async fn handle_revoke_module( State(state): State, Json(request): Json, ) -> Result { From 5b8c4db94ea27f695788da5d911d8bbf6eab4eee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Mon, 21 Apr 2025 16:48:02 -0300 Subject: [PATCH 05/12] Receive module jwts and admin secrets from body in reload endpoint --- crates/common/src/commit/request.rs | 26 ++++++++++++++++++++++++-- crates/common/src/config/utils.rs | 2 +- crates/signer/src/service.rs | 16 +++++++++++----- 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/crates/common/src/commit/request.rs b/crates/common/src/commit/request.rs index 5c2b058c..ba6e8a6b 100644 --- a/crates/common/src/commit/request.rs +++ b/crates/common/src/commit/request.rs @@ -1,4 +1,5 @@ use std::{ + collections::HashMap, fmt::{self, Debug, Display}, str::FromStr, }; @@ -9,11 +10,12 @@ use alloy::{ rpc::types::beacon::BlsSignature, }; use derive_more::derive::From; -use serde::{Deserialize, Serialize}; +use serde::{Deserialize, Deserializer, Serialize}; use tree_hash::TreeHash; use tree_hash_derive::TreeHash; use crate::{ + config::decode_string_to_map, constants::COMMIT_BOOST_DOMAIN, error::BlstErrorWrapper, signature::verify_signed_message, @@ -202,7 +204,27 @@ pub struct GetPubkeysResponse { } #[derive(Debug, Clone, Serialize, Deserialize)] -pub struct RevokeJWTRequest { +pub struct ReloadRequest { + #[serde(default, deserialize_with = "deserialize_jwt_secrets")] + pub jwt_secrets: Option>, + pub admin_secret: Option, +} + +pub fn deserialize_jwt_secrets<'de, D>( + deserializer: D, +) -> Result>, D::Error> +where + D: Deserializer<'de>, +{ + let raw: String = Deserialize::deserialize(deserializer)?; + + decode_string_to_map(&raw) + .map(|x| Some(x)) + .map_err(|_| serde::de::Error::custom("Invalid format".to_string())) +} + +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct RevokeModuleRequest { pub module_id: ModuleId, } diff --git a/crates/common/src/config/utils.rs b/crates/common/src/config/utils.rs index d7c6b68f..5962500b 100644 --- a/crates/common/src/config/utils.rs +++ b/crates/common/src/config/utils.rs @@ -31,7 +31,7 @@ pub fn load_jwt_secrets() -> Result<(String, HashMap)> { decode_string_to_map(&jwt_secrets).map(|secrets| (admin_jwt, secrets)) } -fn decode_string_to_map(raw: &str) -> Result> { +pub fn decode_string_to_map(raw: &str) -> Result> { // trim the string and split for comma raw.trim() .split(',') diff --git a/crates/signer/src/service.rs b/crates/signer/src/service.rs index 6afd59cc..36df7f69 100644 --- a/crates/signer/src/service.rs +++ b/crates/signer/src/service.rs @@ -16,8 +16,8 @@ use cb_common::{ REVOKE_MODULE_PATH, STATUS_PATH, }, request::{ - EncryptionScheme, GenerateProxyRequest, GetPubkeysResponse, RevokeJWTRequest, - SignConsensusRequest, SignProxyRequest, SignRequest, + EncryptionScheme, GenerateProxyRequest, GetPubkeysResponse, ReloadRequest, + RevokeModuleRequest, SignConsensusRequest, SignProxyRequest, SignRequest, }, }, config::StartSignerConfig, @@ -282,6 +282,7 @@ async fn handle_generate_proxy( async fn handle_reload( State(mut state): State, + Json(request): Json, ) -> Result { let req_id = Uuid::new_v4(); @@ -295,8 +296,13 @@ async fn handle_reload( } }; - state.jwts = Arc::new(RwLock::new(config.jwts.clone())); - state.admin_secret = Arc::new(RwLock::new(config.admin_secret.clone())); + if let Some(jwt_secrets) = request.jwt_secrets { + *state.jwts.write().await = jwt_secrets; + } + + if let Some(admin_secret) = request.admin_secret { + *state.admin_secret.write().await = admin_secret; + } let new_manager = match start_manager(config).await { Ok(manager) => manager, @@ -313,7 +319,7 @@ async fn handle_reload( async fn handle_revoke_module( State(state): State, - Json(request): Json, + Json(request): Json, ) -> Result { let mut guard = state.jwts.write().await; guard From 53351cd824d6f1cf25ab635a4504715a1ade3b2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Mon, 21 Apr 2025 16:57:18 -0300 Subject: [PATCH 06/12] Update docs --- docs/docs/get_started/configuration.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/docs/get_started/configuration.md b/docs/docs/get_started/configuration.md index 4e642205..fc67fa9a 100644 --- a/docs/docs/get_started/configuration.md +++ b/docs/docs/get_started/configuration.md @@ -365,6 +365,15 @@ Commit-Boost supports hot-reloading the configuration file. This means that you docker compose -f cb.docker-compose.yml exec cb_signer curl -X POST http://localhost:20000/reload ``` +### Signer module reload + +The signer module takes 2 optional parameters in the JSON body: + +- `jwt_secrets`: a string with a comma-separated list of `=JWT_SECRET` for all modules. +- `admin_secret`: a string with the secret for the signer admin JWT. + +In the case that someone of those isn't present, that parameter won't be updated. + ### Notes - The hot reload feature is available for PBS modules (both default and custom) and signer module. From cc4d0d8d417da4a7897622aa3b684b409419256d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Mon, 21 Apr 2025 16:59:27 -0300 Subject: [PATCH 07/12] Update docs --- crates/common/src/config/constants.rs | 2 +- docs/docs/get_started/configuration.md | 2 +- docs/docs/get_started/running/binary.md | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/common/src/config/constants.rs b/crates/common/src/config/constants.rs index d717dc70..0cf9b3c3 100644 --- a/crates/common/src/config/constants.rs +++ b/crates/common/src/config/constants.rs @@ -37,7 +37,7 @@ pub const SIGNER_PORT_ENV: &str = "CB_SIGNER_PORT"; /// Comma separated list module_id=jwt_secret pub const JWTS_ENV: &str = "CB_JWTS"; -pub const ADMIN_JWT_ENV: &str = "CB_ADMIN_JWT"; +pub const ADMIN_JWT_ENV: &str = "CB_SIGNER_ADMIN_JWT"; /// The JWT secret for the signer to validate the modules requests pub const SIGNER_JWT_SECRET_ENV: &str = "CB_SIGNER_JWT_SECRET"; diff --git a/docs/docs/get_started/configuration.md b/docs/docs/get_started/configuration.md index fc67fa9a..ebdbef94 100644 --- a/docs/docs/get_started/configuration.md +++ b/docs/docs/get_started/configuration.md @@ -369,7 +369,7 @@ docker compose -f cb.docker-compose.yml exec cb_signer curl -X POST http://local The signer module takes 2 optional parameters in the JSON body: -- `jwt_secrets`: a string with a comma-separated list of `=JWT_SECRET` for all modules. +- `jwt_secrets`: a string with a comma-separated list of `=` for all modules. - `admin_secret`: a string with the secret for the signer admin JWT. In the case that someone of those isn't present, that parameter won't be updated. diff --git a/docs/docs/get_started/running/binary.md b/docs/docs/get_started/running/binary.md index 3708ab19..37110714 100644 --- a/docs/docs/get_started/running/binary.md +++ b/docs/docs/get_started/running/binary.md @@ -27,6 +27,7 @@ Modules need some environment variables to work correctly. ### Signer Module - `CB_SIGNER_JWT_SECRET`: secret to use for JWT authentication with the Signer module. +- `CB_SIGNER_ADMIN_JWT`: secret to use for admin JWT. - `CB_SIGNER_PORT`: required, port to open the signer server on. - For loading keys we currently support: - `CB_SIGNER_LOADER_FILE`: path to a `.json` with plaintext keys (for testing purposes only). From 4bef8ff2f68f154feb9020a188b5d5d54685c448 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Tue, 22 Apr 2025 10:08:12 -0300 Subject: [PATCH 08/12] Fix clippy --- crates/common/src/commit/request.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/common/src/commit/request.rs b/crates/common/src/commit/request.rs index ba6e8a6b..9a67dcc2 100644 --- a/crates/common/src/commit/request.rs +++ b/crates/common/src/commit/request.rs @@ -219,7 +219,7 @@ where let raw: String = Deserialize::deserialize(deserializer)?; decode_string_to_map(&raw) - .map(|x| Some(x)) + .map(Some) .map_err(|_| serde::de::Error::custom("Invalid format".to_string())) } From e1c10671a640c2c8a04725fe97a691c6de2a1aea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Tue, 8 Jul 2025 18:58:26 -0300 Subject: [PATCH 09/12] Use parking_lot::RwLock --- crates/signer/src/service.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/crates/signer/src/service.rs b/crates/signer/src/service.rs index db0d0d5d..2b4a088e 100644 --- a/crates/signer/src/service.rs +++ b/crates/signer/src/service.rs @@ -63,9 +63,9 @@ struct SigningState { /// Map of modules ids to JWT secrets. This also acts as registry of all /// modules running - jwts: Arc>>, + jwts: Arc>>, /// Secret for the admin JWT - admin_secret: Arc>, + admin_secret: Arc>, /// Map of JWT failures per peer jwt_auth_failures: Arc>>, @@ -86,8 +86,8 @@ impl SigningService { let state = SigningState { manager: Arc::new(RwLock::new(start_manager(config.clone()).await?)), - jwts: Arc::new(RwLock::new(config.jwts)), - admin_secret: Arc::new(RwLock::new(config.admin_secret)), + jwts: Arc::new(ParkingRwLock::new(config.jwts)), + admin_secret: Arc::new(ParkingRwLock::new(config.admin_secret)), jwt_auth_failures: Arc::new(ParkingRwLock::new(HashMap::new())), jwt_auth_fail_limit: config.jwt_auth_fail_limit, jwt_auth_fail_timeout: Duration::from_secs(config.jwt_auth_fail_timeout_seconds as u64), @@ -227,7 +227,7 @@ async fn check_jwt_auth( SignerModuleError::Unauthorized })?; - let guard = state.jwts.read().await; + let guard = state.jwts.read(); let jwt_secret = guard.get(&module_id).ok_or_else(|| { error!("Unauthorized request. Was the module started correctly?"); SignerModuleError::Unauthorized @@ -248,7 +248,7 @@ async fn admin_auth( ) -> Result { let jwt: Jwt = auth.token().to_string().into(); - validate_admin_jwt(jwt, &state.admin_secret.read().await).map_err(|e| { + validate_admin_jwt(jwt, &state.admin_secret.read()).map_err(|e| { error!("Unauthorized request. Invalid JWT: {e}"); SignerModuleError::Unauthorized })?; @@ -405,11 +405,11 @@ async fn handle_reload( }; if let Some(jwt_secrets) = request.jwt_secrets { - *state.jwts.write().await = jwt_secrets; + *state.jwts.write() = jwt_secrets; } if let Some(admin_secret) = request.admin_secret { - *state.admin_secret.write().await = admin_secret; + *state.admin_secret.write() = admin_secret; } let new_manager = match start_manager(config).await { @@ -429,7 +429,7 @@ async fn handle_revoke_module( State(state): State, Json(request): Json, ) -> Result { - let mut guard = state.jwts.write().await; + let mut guard = state.jwts.write(); guard .remove(&request.module_id) .ok_or(SignerModuleError::ModuleIdNotFound) From ce562dea62f111bfbf1ff0cdcb44dd588dd65ca6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Thu, 10 Jul 2025 10:57:47 -0300 Subject: [PATCH 10/12] Remove unneeded async-await --- crates/signer/src/service.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/signer/src/service.rs b/crates/signer/src/service.rs index 2b4a088e..59da3c3d 100644 --- a/crates/signer/src/service.rs +++ b/crates/signer/src/service.rs @@ -160,7 +160,7 @@ async fn jwt_auth( check_jwt_rate_limit(&state, &client_ip)?; // Process JWT authorization - match check_jwt_auth(&auth, &state).await { + match check_jwt_auth(&auth, &state) { Ok(module_id) => { req.extensions_mut().insert(module_id); Ok(next.run(req).await) @@ -214,7 +214,7 @@ fn check_jwt_rate_limit(state: &SigningState, client_ip: &IpAddr) -> Result<(), } /// Checks if a request can successfully authenticate with the JWT secret -async fn check_jwt_auth( +fn check_jwt_auth( auth: &Authorization, state: &SigningState, ) -> Result { From bebea646078befe8a752a6da25546c70ed0e151c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Thu, 10 Jul 2025 12:41:24 -0300 Subject: [PATCH 11/12] Add tests for revoke endpoint --- Cargo.lock | 1 + tests/Cargo.toml | 1 + tests/src/utils.rs | 5 +- tests/tests/signer_jwt_auth.rs | 92 ++++++++++++++++++++++++++++++++-- 4 files changed, 94 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5182a760..9e8fc8e5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1594,6 +1594,7 @@ dependencies = [ "cb-pbs", "cb-signer", "eyre", + "jsonwebtoken", "reqwest", "serde_json", "tempfile", diff --git a/tests/Cargo.toml b/tests/Cargo.toml index f1b5c9d9..573cfa20 100644 --- a/tests/Cargo.toml +++ b/tests/Cargo.toml @@ -11,6 +11,7 @@ cb-common.workspace = true cb-pbs.workspace = true cb-signer.workspace = true eyre.workspace = true +jsonwebtoken.workspace = true reqwest.workspace = true serde_json.workspace = true tempfile.workspace = true diff --git a/tests/src/utils.rs b/tests/src/utils.rs index 33741228..ef4d2873 100644 --- a/tests/src/utils.rs +++ b/tests/src/utils.rs @@ -16,7 +16,7 @@ use cb_common::{ DEFAULT_SIGNER_PORT, }, types::{Chain, ModuleId}, - utils::{default_host, random_jwt_secret}, + utils::default_host, }; use eyre::Result; @@ -117,6 +117,7 @@ pub fn get_start_signer_config( signer_config: SignerConfig, chain: Chain, jwts: HashMap, + admin_secret: String, ) -> StartSignerConfig { match signer_config.inner { SignerType::Local { loader, .. } => StartSignerConfig { @@ -125,7 +126,7 @@ pub fn get_start_signer_config( store: None, endpoint: SocketAddr::new(signer_config.host.into(), signer_config.port), jwts, - admin_secret: random_jwt_secret(), + admin_secret, jwt_auth_fail_limit: signer_config.jwt_auth_fail_limit, jwt_auth_fail_timeout_seconds: signer_config.jwt_auth_fail_timeout_seconds, dirk: None, diff --git a/tests/tests/signer_jwt_auth.rs b/tests/tests/signer_jwt_auth.rs index 90a0365f..820afbcc 100644 --- a/tests/tests/signer_jwt_auth.rs +++ b/tests/tests/signer_jwt_auth.rs @@ -2,10 +2,14 @@ use std::{collections::HashMap, time::Duration}; use alloy::{hex, primitives::FixedBytes}; use cb_common::{ - commit::{constants::GET_PUBKEYS_PATH, request::GetPubkeysResponse}, + commit::{ + constants::{GET_PUBKEYS_PATH, REVOKE_MODULE_PATH}, + request::GetPubkeysResponse, + }, config::StartSignerConfig, + constants::SIGNER_JWT_EXPIRATION, signer::{SignerLoader, ValidatorKeysFormat}, - types::{Chain, ModuleId}, + types::{Chain, Jwt, JwtAdmin, ModuleId}, utils::create_jwt, }; use cb_signer::service::SigningService; @@ -16,6 +20,7 @@ use tracing::info; const JWT_MODULE: &str = "test-module"; const JWT_SECRET: &str = "test-jwt-secret"; +const ADMIN_SECRET: &str = "test-admin-secret"; #[tokio::test] async fn test_signer_jwt_auth_success() -> Result<()> { @@ -86,6 +91,74 @@ async fn test_signer_jwt_rate_limit() -> Result<()> { Ok(()) } +#[tokio::test] +async fn test_signer_revoked_jwt_fail() -> Result<()> { + setup_test_env(); + let module_id = ModuleId(JWT_MODULE.to_string()); + let start_config = start_server(20400).await?; + + // Run as many pubkeys requests as the fail limit + let jwt = create_jwt(&module_id, JWT_SECRET)?; + let admin_jwt = create_admin_jwt()?; + let client = reqwest::Client::new(); + + // At first, test module should be allowed to request pubkeys + let url = format!("http://{}{}", start_config.endpoint, GET_PUBKEYS_PATH); + let response = client.get(&url).bearer_auth(&jwt).send().await?; + assert!(response.status() == StatusCode::OK); + + let revoke_url = format!("http://{}{}", start_config.endpoint, REVOKE_MODULE_PATH); + let response = client + .post(&revoke_url) + .header("content-type", "application/json") + .body(reqwest::Body::wrap(format!("{{\"module_id\": \"{JWT_MODULE}\"}}"))) + .bearer_auth(&admin_jwt) + .send() + .await?; + assert!(response.status() == StatusCode::OK); + + // After revoke, test module shouldn't be allowed anymore + let response = client.get(&url).bearer_auth(&jwt).send().await?; + assert!(response.status() == StatusCode::UNAUTHORIZED); + + Ok(()) +} + +#[tokio::test] +async fn test_signer_only_admin_can_revoke() -> Result<()> { + setup_test_env(); + let module_id = ModuleId(JWT_MODULE.to_string()); + let start_config = start_server(20500).await?; + + // Run as many pubkeys requests as the fail limit + let jwt = create_jwt(&module_id, JWT_SECRET)?; + let admin_jwt = create_admin_jwt()?; + let client = reqwest::Client::new(); + let url = format!("http://{}{}", start_config.endpoint, REVOKE_MODULE_PATH); + + // Module JWT shouldn't be able to revoke modules + let response = client + .post(&url) + .header("content-type", "application/json") + .body(reqwest::Body::wrap(format!("{{\"module_id\": \"{JWT_MODULE}\"}}"))) + .bearer_auth(&jwt) + .send() + .await?; + assert!(response.status() == StatusCode::UNAUTHORIZED); + + // Admin should be able to revoke modules + let response = client + .post(&url) + .header("content-type", "application/json") + .body(reqwest::Body::wrap(format!("{{\"module_id\": \"{JWT_MODULE}\"}}"))) + .bearer_auth(&admin_jwt) + .send() + .await?; + assert!(response.status() == StatusCode::OK); + + Ok(()) +} + // Starts the signer moduler server on a separate task and returns its // configuration async fn start_server(port: u16) -> Result { @@ -107,7 +180,7 @@ async fn start_server(port: u16) -> Result { config.port = port; config.jwt_auth_fail_limit = 3; // Set a low fail limit for testing config.jwt_auth_fail_timeout_seconds = 3; // Set a short timeout for testing - let start_config = get_start_signer_config(config, chain, jwts); + let start_config = get_start_signer_config(config, chain, jwts, ADMIN_SECRET.to_string()); // Run the Signer let server_handle = tokio::spawn(SigningService::run(start_config.clone())); @@ -144,3 +217,16 @@ async fn verify_pubkeys(response: Response) -> Result<()> { } Ok(()) } + +fn create_admin_jwt() -> Result { + jsonwebtoken::encode( + &jsonwebtoken::Header::default(), + &JwtAdmin { + admin: true, + exp: jsonwebtoken::get_current_timestamp() + SIGNER_JWT_EXPIRATION, + }, + &jsonwebtoken::EncodingKey::from_secret(ADMIN_SECRET.as_ref()), + ) + .map_err(Into::into) + .map(Jwt::from) +} From 36d8c8f75b9301ce9ef2aca4b58f4278910ff2de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20I=C3=B1aki=20Bilbao?= Date: Tue, 22 Jul 2025 16:13:33 -0300 Subject: [PATCH 12/12] Update docs --- docs/docs/get_started/configuration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/docs/get_started/configuration.md b/docs/docs/get_started/configuration.md index a09d347f..b65e73ad 100644 --- a/docs/docs/get_started/configuration.md +++ b/docs/docs/get_started/configuration.md @@ -405,7 +405,7 @@ The signer module takes 2 optional parameters in the JSON body: - `jwt_secrets`: a string with a comma-separated list of `=` for all modules. - `admin_secret`: a string with the secret for the signer admin JWT. -In the case that someone of those isn't present, that parameter won't be updated. +Parameters that are not provided will not be updated; they will be regenerated using their original on-disk data as though the signer service was being restarted. Note that any changes you made with calls to `/revoke_jwt` or `/reload` will be reverted, so make sure you provide any modifications again as part of this call. ### Notes