Merge sigp-audit-fixes (#348)

Co-authored-by: Manuel Iñaki Bilbao <manuel.bilbao@lambdaclass.com>
Co-authored-by: ltitanb <163874448+ltitanb@users.noreply.github.com>

Author: 3 people

Cargo.lock

@@ -6,16 +6,16 @@ use std::{
 
 use cb_common::{
     config::{
-        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, 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_ENDPOINT_ENV, SIGNER_KEYS_ENV, SIGNER_MODULE_NAME,
-        SIGNER_PORT_DEFAULT, SIGNER_URL_ENV,
+        CommitBoostConfig, LogsSettings, ModuleKind, SignerConfig, 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_ENDPOINT_ENV, SIGNER_KEYS_ENV,
+        SIGNER_MODULE_NAME, SIGNER_PORT_DEFAULT, SIGNER_URL_ENV,
     },
     pbs::{BUILDER_API_PATH, GET_STATUS_PATH},
     signer::{ProxyStore, SignerLoader},
@@ -333,6 +333,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),
                 ]);
 
                 // Bind the signer API to 0.0.0.0
@@ -366,6 +367,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()];

crates/cli/src/docker_init.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_MODULE_PATH: &str = "/revoke_jwt";

crates/common/src/commit/constants.rs

@@ -1,4 +1,5 @@
 use std::{
+    collections::HashMap,
     fmt::{self, Debug, Display},
     str::FromStr,
 };
@@ -9,13 +10,17 @@ 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::{
-    constants::COMMIT_BOOST_DOMAIN, error::BlstErrorWrapper, signature::verify_signed_message,
-    signer::BlsPublicKey, types::Chain,
+    config::decode_string_to_map,
+    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 {}
@@ -199,6 +204,31 @@ pub struct GetPubkeysResponse {
     pub keys: Vec<ConsensusProxyMap>,
 }
 
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct ReloadRequest {
+    #[serde(default, deserialize_with = "deserialize_jwt_secrets")]
+    pub jwt_secrets: Option<HashMap<ModuleId, String>>,
+    pub admin_secret: Option<String>,
+}
+
+pub fn deserialize_jwt_secrets<'de, D>(
+    deserializer: D,
+) -> Result<Option<HashMap<ModuleId, String>>, D::Error>
+where
+    D: Deserializer<'de>,
+{
+    let raw: String = Deserialize::deserialize(deserializer)?;
+
+    decode_string_to_map(&raw)
+        .map(Some)
+        .map_err(|_| serde::de::Error::custom("Invalid format".to_string()))
+}
+
+#[derive(Debug, Clone, Serialize, Deserialize)]
+pub struct RevokeModuleRequest {
+    pub module_id: ModuleId,
+}
+
 /// Map of consensus pubkeys to proxies
 #[derive(Debug, Clone, Deserialize, Serialize)]
 pub struct ConsensusProxyMap {
@@ -289,7 +319,7 @@ mod tests {
 
         let _: SignedProxyDelegationBls = serde_json::from_str(data).unwrap();
 
-        let data = r#"{ 
+        let data = r#"{
             "message": {
                 "delegator": "0xa3366b54f28e4bf1461926a3c70cdb0ec432b5c92554ecaae3742d33fb33873990cbed1761c68020e6d3c14d30a22050",
                 "proxy": "0x4ca9939a8311a7cab3dde201b70157285fa81a9d"

crates/common/src/commit/request.rs

@@ -47,6 +47,7 @@ pub const SIGNER_JWT_AUTH_FAIL_TIMEOUT_SECONDS_DEFAULT: u32 = 5 * 60;
 
 /// Comma separated list module_id=jwt_secret
 pub const JWTS_ENV: &str = "CB_JWTS";
+pub const ADMIN_JWT_ENV: &str = "CB_SIGNER_ADMIN_JWT";
 
 /// Path to json file with plaintext keys (testing only)
 pub const SIGNER_KEYS_ENV: &str = "CB_SIGNER_LOADER_FILE";

crates/common/src/config/constants.rs

@@ -165,6 +165,7 @@ pub struct StartSignerConfig {
     pub store: Option<ProxyStore>,
     pub endpoint: SocketAddr,
     pub mod_signing_configs: HashMap<ModuleId, ModuleSigningConfig>,
+    pub admin_secret: String,
     pub jwt_auth_fail_limit: u32,
     pub jwt_auth_fail_timeout_seconds: u32,
     pub dirk: Option<DirkConfig>,
@@ -174,7 +175,7 @@ impl StartSignerConfig {
     pub fn load_from_env() -> Result<Self> {
         let config = CommitBoostConfig::from_env_path()?;
 
-        let jwt_secrets = load_jwt_secrets()?;
+        let (admin_secret, jwt_secrets) = load_jwt_secrets()?;
 
         // Load the module signing configs
         let mod_signing_configs = load_module_signing_configs(&config, &jwt_secrets)
@@ -213,6 +214,7 @@ impl StartSignerConfig {
                 loader: Some(loader),
                 endpoint,
                 mod_signing_configs,
+                admin_secret,
                 jwt_auth_fail_limit,
                 jwt_auth_fail_timeout_seconds,
                 store,
@@ -243,6 +245,7 @@ impl StartSignerConfig {
                     chain: config.chain,
                     endpoint,
                     mod_signing_configs,
+                    admin_secret,
                     jwt_auth_fail_limit,
                     jwt_auth_fail_timeout_seconds,
                     loader: None,

crates/common/src/config/signer.rs

@@ -5,7 +5,7 @@ use eyre::{bail, Context, Result};
 use serde::de::DeserializeOwned;
 
 use crate::{
-    config::{JWTS_ENV, MUXER_HTTP_MAX_LENGTH},
+    config::{ADMIN_JWT_ENV, JWTS_ENV, MUXER_HTTP_MAX_LENGTH},
     types::ModuleId,
     utils::read_chunked_body_with_max,
 };
@@ -29,9 +29,10 @@ pub fn load_file_from_env<T: DeserializeOwned>(env: &str) -> Result<T> {
 }
 
 /// Loads a map of module id -> jwt secret from a json env
-pub fn load_jwt_secrets() -> Result<HashMap<ModuleId, String>> {
+pub fn load_jwt_secrets() -> Result<(String, HashMap<ModuleId, String>)> {
+    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))
 }
 
 /// Reads an HTTP response safely, erroring out if it failed or if the body is
@@ -74,7 +75,7 @@ pub fn remove_duplicate_keys(keys: Vec<BlsPublicKey>) -> Vec<BlsPublicKey> {
     unique_keys
 }
 
-fn decode_string_to_map(raw: &str) -> Result<HashMap<ModuleId, String>> {
+pub fn decode_string_to_map(raw: &str) -> Result<HashMap<ModuleId, String>> {
     // trim the string and split for comma
     raw.trim()
         .split(',')

crates/common/src/config/utils.rs

@@ -24,6 +24,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,

crates/common/src/types.rs

@@ -30,7 +30,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;
@@ -405,6 +405,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::<JwtAdmin>(
+        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()

crates/common/src/utils.rs

@@ -25,11 +25,17 @@ pub enum SignerModuleError {
     #[error("Dirk signer does not support this operation")]
     DirkNotSupported,
 
+    #[error("module id not found")]
+    ModuleIdNotFound,
+
     #[error("internal error: {0}")]
     Internal(String),
 
     #[error("rate limited for {0} more seconds")]
     RateLimited(f64),
+
+    #[error("request error: {0}")]
+    RequestError(String),
 }
 
 impl IntoResponse for SignerModuleError {
@@ -48,9 +54,13 @@ 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()),
             SignerModuleError::RateLimited(duration) => {
                 (StatusCode::TOO_MANY_REQUESTS, format!("rate limited for {duration:?}"))
             }
+            SignerModuleError::RequestError(err) => {
+                (StatusCode::BAD_REQUEST, format!("bad request: {err}"))
+            }
         }
         .into_response()
     }