From 33d50666b569aa844026a66d6bcde657da80477f Mon Sep 17 00:00:00 2001 From: trinity-1686a Date: Sat, 5 Sep 2026 16:25:49 +0200 Subject: [PATCH] wrap secrets in new type --- Cargo.lock | 1 + Cargo.toml | 1 + src/api/admin/admin_token.rs | 4 ++-- src/api/admin/api_server.rs | 10 +++++++-- src/garage/main.rs | 3 ++- src/garage/secrets.rs | 13 +++++++----- src/model/garage.rs | 2 +- src/rpc/consul.rs | 2 +- src/util/Cargo.toml | 1 + src/util/config.rs | 39 ++++++++++++++++++++++++++++++++---- 10 files changed, 60 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c2d32773..50ee732b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1890,6 +1890,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.10.9", + "subtle", "thiserror 2.0.18", "tokio", "toml", diff --git a/Cargo.toml b/Cargo.toml index 704f0264..8f58e8ee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -76,6 +76,7 @@ pnet_datalink = "0.35" rand = "0.9" sha1 = "0.10" sha2 = "0.10" +subtle = "2.6.1" timeago = { version = "0.5", default-features = false } xxhash-rust = { version = "0.8", default-features = false, features = ["xxh3"] } diff --git a/src/api/admin/admin_token.rs b/src/api/admin/admin_token.rs index b954b53e..471ec2d2 100644 --- a/src/api/admin/admin_token.rs +++ b/src/api/admin/admin_token.rs @@ -191,7 +191,7 @@ impl RequestHandler for GetCurrentAdminTokenInfoRequest { .admin .metrics_token .as_ref() - .is_some_and(|s| s == &self.admin_token) + .is_some_and(|s| s.eq_ct(&self.admin_token)) { return Ok(GetCurrentAdminTokenInfoResponse( GetAdminTokenInfoResponse { @@ -210,7 +210,7 @@ impl RequestHandler for GetCurrentAdminTokenInfoRequest { .admin .admin_token .as_ref() - .is_some_and(|s| s == &self.admin_token) + .is_some_and(|s| s.eq_ct(&self.admin_token)) { return Ok(GetCurrentAdminTokenInfoResponse( GetAdminTokenInfoResponse { diff --git a/src/api/admin/api_server.rs b/src/api/admin/api_server.rs index aa8d8e96..75ba2e65 100644 --- a/src/api/admin/api_server.rs +++ b/src/api/admin/api_server.rs @@ -117,8 +117,14 @@ impl AdminApiServer { #[cfg(feature = "metrics")] exporter: PrometheusExporter, ) -> Arc { let cfg = &garage.config.admin; - let metrics_token = cfg.metrics_token.as_deref().map(hash_bearer_token); - let admin_token = cfg.admin_token.as_deref().map(hash_bearer_token); + let metrics_token = cfg + .metrics_token + .as_ref() + .map(|token| hash_bearer_token(token.extract_secret())); + let admin_token = cfg + .admin_token + .as_ref() + .map(|token| hash_bearer_token(token.extract_secret())); let metrics_require_token = cfg.metrics_require_token; let endpoint = garage.system.netapp.endpoint(ADMIN_RPC_PATH.into()); diff --git a/src/garage/main.rs b/src/garage/main.rs index 27577560..4397776c 100644 --- a/src/garage/main.rs +++ b/src/garage/main.rs @@ -307,7 +307,8 @@ async fn cli_command(opt: Opt) -> Result<(), Error> { let net_key_hex_str = rpc_secret.ok_or("No RPC secret provided")?; let network_key = NetworkKey::from_slice( - &hex::decode(&net_key_hex_str).err_context("Invalid RPC secret key (bad hex)")?[..], + &hex::decode(net_key_hex_str.extract_secret()) + .err_context("Invalid RPC secret key (bad hex)")?[..], ) .ok_or("Invalid RPC secret provided (wrong length)")?; diff --git a/src/garage/secrets.rs b/src/garage/secrets.rs index 66cc84dc..4ff19387 100644 --- a/src/garage/secrets.rs +++ b/src/garage/secrets.rs @@ -2,7 +2,7 @@ use std::path::PathBuf; use structopt::StructOpt; -use garage_util::config::Config; +use garage_util::config::{Config, Secret}; use garage_util::error::Error; /// Structure for secret values or paths that are passed as CLI arguments or environment @@ -99,7 +99,7 @@ pub fn fill_secrets(mut config: Config, secrets: Secrets) -> Result, + config_secret: &mut Option>, config_secret_file: &Option, cli_secret: &Option, cli_secret_file: &Option, @@ -110,7 +110,7 @@ pub(crate) fn fill_secret( (Some(_), Some(_)) => { return Err(format!("only one of `{}` and `{}_file` can be set", name, name).into()); } - (Some(secret), None) => Some(secret.to_string()), + (Some(secret), None) => Some(Secret::new(secret.to_string())), (None, Some(file)) => Some(read_secret_file(file, allow_world_readable)?), (None, None) => None, }; @@ -132,7 +132,10 @@ pub(crate) fn fill_secret( Ok(()) } -fn read_secret_file(file_path: &PathBuf, allow_world_readable: bool) -> Result { +fn read_secret_file( + file_path: &PathBuf, + allow_world_readable: bool, +) -> Result, Error> { if !allow_world_readable { #[cfg(unix)] { @@ -152,7 +155,7 @@ fn read_secret_file(file_path: &PathBuf, allow_world_readable: bool) -> Result somefile`. // also editors sometimes add a trailing newline - Ok(String::from(secret_buf.trim_end())) + Ok(Secret::new(String::from(secret_buf.trim_end()))) } #[cfg(test)] diff --git a/src/model/garage.rs b/src/model/garage.rs index 9f88fa87..b026c2d7 100644 --- a/src/model/garage.rs +++ b/src/model/garage.rs @@ -137,7 +137,7 @@ impl Garage { info!("Initializing RPC..."); let network_key = hex::decode(config.rpc_secret.as_ref().ok_or_message( "rpc_secret value is missing, not present in config file or in environment", - )?) + )?.extract_secret()) .ok() .and_then(|x| NetworkKey::from_slice(&x)) .ok_or_message("Invalid RPC secret key: expected 32 bytes of random hex, please check the documentation for requirements")?; diff --git a/src/rpc/consul.rs b/src/rpc/consul.rs index dea49995..38f56e48 100644 --- a/src/rpc/consul.rs +++ b/src/rpc/consul.rs @@ -115,7 +115,7 @@ impl ConsulDiscovery { let mut headers = reqwest::header::HeaderMap::new(); headers.insert( "x-consul-token", - reqwest::header::HeaderValue::from_str(token)?, + reqwest::header::HeaderValue::from_str(token.extract_secret())?, ); builder = builder.default_headers(headers); } diff --git a/src/util/Cargo.toml b/src/util/Cargo.toml index 11e5a40e..2712b956 100644 --- a/src/util/Cargo.toml +++ b/src/util/Cargo.toml @@ -32,6 +32,7 @@ lazy_static.workspace = true tracing.workspace = true rand.workspace = true sha2.workspace = true +subtle.workspace = true chrono.workspace = true rmp-serde.workspace = true diff --git a/src/util/config.rs b/src/util/config.rs index 0395549e..716b5ecf 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -90,7 +90,7 @@ pub struct Config { pub allow_world_readable_secrets: bool, /// RPC secret key: 32 bytes hex encoded - pub rpc_secret: Option, + pub rpc_secret: Option>, /// Optional file where RPC secret key is read from pub rpc_secret_file: Option, /// Address to bind for RPC @@ -205,6 +205,37 @@ pub struct WebConfig { pub add_host_to_metrics: bool, } +#[derive(Deserialize, Clone)] +#[serde(transparent)] +pub struct Secret(T); + +impl Secret { + pub fn new(secret: T) -> Self { + Secret(secret) + } + + pub fn extract_secret(&self) -> &T { + &self.0 + } +} + +impl> Secret { + pub fn eq_ct(&self, other: &T) -> bool { + use subtle::ConstantTimeEq; + self.0 + .deref() + .as_bytes() + .ct_eq(other.deref().as_bytes()) + .into() + } +} + +impl std::fmt::Debug for Secret { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Secret").finish_non_exhaustive() + } +} + /// Configuration for the admin and monitoring HTTP API #[derive(Deserialize, Debug, Clone, Default)] pub struct AdminConfig { @@ -212,7 +243,7 @@ pub struct AdminConfig { pub api_bind_addr: Option, /// Bearer token to use to scrape metrics - pub metrics_token: Option, + pub metrics_token: Option>, /// File to read metrics token from pub metrics_token_file: Option, /// Whether to require an access token for accessing the metrics endpoint @@ -220,7 +251,7 @@ pub struct AdminConfig { pub metrics_require_token: bool, /// Bearer token to use to access Admin API endpoints - pub admin_token: Option, + pub admin_token: Option>, /// File to read admin token from pub admin_token_file: Option, @@ -252,7 +283,7 @@ pub struct ConsulDiscoveryConfig { /// Client TLS key to use when connecting to Consul pub client_key: Option, /// /// Token to use for connecting to consul - pub token: Option, + pub token: Option>, /// Skip TLS hostname verification #[serde(default)] pub tls_skip_verify: bool,