From c99cb58d712fcfdcc9ec03095c0cac25e0c2b934 Mon Sep 17 00:00:00 2001 From: networkException Date: Thu, 19 Oct 2023 03:29:48 +0200 Subject: util: move reading secret file into seperate helper this patch moves the logic to read a secret file (and check for correct permissions) from `secret_from_file` into a new `read_secret_file` helper. --- src/util/config.rs | 35 +++++++++++++++++++---------------- 1 file changed, 19 insertions(+), 16 deletions(-) (limited to 'src') diff --git a/src/util/config.rs b/src/util/config.rs index ebcb5fbe..756f7ec5 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -238,6 +238,24 @@ pub fn read_config(config_file: PathBuf) -> Result { Ok(parsed_config) } +pub fn read_secret_file(file_path: &String) -> Result { + #[cfg(unix)] + if std::env::var("GARAGE_ALLOW_WORLD_READABLE_SECRETS").as_deref() != Ok("true") { + use std::os::unix::fs::MetadataExt; + let metadata = std::fs::metadata(file_path)?; + if metadata.mode() & 0o077 != 0 { + return Err(format!("File {} is world-readable! (mode: 0{:o}, expected 0600)\nRefusing to start until this is fixed, or environment variable GARAGE_ALLOW_WORLD_READABLE_SECRETS is set to true.", file_path, metadata.mode()).into()); + } + } + let mut file = std::fs::OpenOptions::new().read(true).open(file_path)?; + let mut secret_buf = String::new(); + file.read_to_string(&mut secret_buf)?; + + // trim_end: allows for use case such as `echo "$(openssl rand -hex 32)" > somefile`. + // also editors sometimes add a trailing newline + Ok(String::from(secret_buf.trim_end())) +} + fn secret_from_file( secret: &mut Option, secret_file: &Option, @@ -250,22 +268,7 @@ fn secret_from_file( (Some(_), Some(_)) => { return Err(format!("only one of `{}` and `{}_file` can be set", name, name).into()); } - (None, Some(file_path)) => { - #[cfg(unix)] - if std::env::var("GARAGE_ALLOW_WORLD_READABLE_SECRETS").as_deref() != Ok("true") { - use std::os::unix::fs::MetadataExt; - let metadata = std::fs::metadata(file_path)?; - if metadata.mode() & 0o077 != 0 { - return Err(format!("File {} is world-readable! (mode: 0{:o}, expected 0600)\nRefusing to start until this is fixed, or environment variable GARAGE_ALLOW_WORLD_READABLE_SECRETS is set to true.", file_path, metadata.mode()).into()); - } - } - let mut file = std::fs::OpenOptions::new().read(true).open(file_path)?; - let mut secret_buf = String::new(); - file.read_to_string(&mut secret_buf)?; - // trim_end: allows for use case such as `echo "$(openssl rand -hex 32)" > somefile`. - // also editors sometimes add a trailing newline - *secret = Some(String::from(secret_buf.trim_end())); - } + (None, Some(file_path)) => *secret = Some(read_secret_file(file_path)?), } Ok(()) } -- cgit v1.2.3 From 4a19ee94bb9c846af1c74db8ba501b4ff625a3f6 Mon Sep 17 00:00:00 2001 From: networkException Date: Thu, 19 Oct 2023 03:31:50 +0200 Subject: garage: fix admin-token description --- src/garage/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/garage/main.rs b/src/garage/main.rs index e8aee892..09e77b35 100644 --- a/src/garage/main.rs +++ b/src/garage/main.rs @@ -70,7 +70,7 @@ pub struct Secrets { #[structopt(short = "s", long = "rpc-secret", env = "GARAGE_RPC_SECRET")] pub rpc_secret: Option, - /// Metrics API authentication token, replaces admin.metrics_token in config.toml when + /// Admin API authentication token, replaces admin.admin_token in config.toml when /// running the Garage daemon #[structopt(long = "admin-token", env = "GARAGE_ADMIN_TOKEN")] pub admin_token: Option, -- cgit v1.2.3 From 8599051c492d7df22305e4c65659395d9102955c Mon Sep 17 00:00:00 2001 From: networkException Date: Thu, 19 Oct 2023 03:39:02 +0200 Subject: garage: support specifying token / secret as environment variables this patch adds support for specifying the `rpc_secret_file`, `metrics_token_file` and `admin_token_file` as environment variables. --- src/garage/main.rs | 30 +++++++++++++++++++++++++++--- src/garage/repair/offline.rs | 2 +- src/garage/server.rs | 2 +- 3 files changed, 29 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/garage/main.rs b/src/garage/main.rs index 09e77b35..ab84fa11 100644 --- a/src/garage/main.rs +++ b/src/garage/main.rs @@ -25,7 +25,7 @@ use structopt::StructOpt; use netapp::util::parse_and_resolve_peer_addr; use netapp::NetworkKey; -use garage_util::config::Config; +use garage_util::config::{read_secret_file, Config}; use garage_util::error::*; use garage_rpc::system::*; @@ -70,15 +70,30 @@ pub struct Secrets { #[structopt(short = "s", long = "rpc-secret", env = "GARAGE_RPC_SECRET")] pub rpc_secret: Option, + /// RPC secret network key, used to replace rpc_secret in config.toml and rpc-secret + /// when running the daemon or doing admin operations + #[structopt(long = "rpc-secret-file", env = "GARAGE_RPC_SECRET_FILE")] + pub rpc_secret_file: Option, + /// Admin API authentication token, replaces admin.admin_token in config.toml when /// running the Garage daemon #[structopt(long = "admin-token", env = "GARAGE_ADMIN_TOKEN")] pub admin_token: Option, + /// Admin API authentication token file path, replaces admin.admin_token in config.toml + /// and admin-token when running the Garage daemon + #[structopt(long = "admin-token-file", env = "GARAGE_ADMIN_TOKEN_FILE")] + pub admin_token_file: Option, + /// Metrics API authentication token, replaces admin.metrics_token in config.toml when /// running the Garage daemon #[structopt(long = "metrics-token", env = "GARAGE_METRICS_TOKEN")] pub metrics_token: Option, + + /// Metrics API authentication token file path, replaces admin.metrics_token in config.toml + /// and metrics-token when running the Garage daemon + #[structopt(long = "metrics-token-file", env = "GARAGE_METRICS_TOKEN_FILE")] + pub metrics_token_file: Option, } #[tokio::main] @@ -256,15 +271,24 @@ async fn cli_command(opt: Opt) -> Result<(), Error> { } } -fn fill_secrets(mut config: Config, secrets: Secrets) -> Config { +fn fill_secrets(mut config: Config, secrets: Secrets) -> Result { if secrets.rpc_secret.is_some() { config.rpc_secret = secrets.rpc_secret; + } else if secrets.rpc_secret_file.is_some() { + config.rpc_secret = Some(read_secret_file(&secrets.rpc_secret_file.unwrap())?); } + if secrets.admin_token.is_some() { config.admin.admin_token = secrets.admin_token; + } else if secrets.admin_token_file.is_some() { + config.admin.admin_token = Some(read_secret_file(&secrets.admin_token_file.unwrap())?); } + if secrets.metrics_token.is_some() { config.admin.metrics_token = secrets.metrics_token; + } else if secrets.metrics_token_file.is_some() { + config.admin.metrics_token = Some(read_secret_file(&secrets.metrics_token_file.unwrap())?); } - config + + Ok(config) } diff --git a/src/garage/repair/offline.rs b/src/garage/repair/offline.rs index f4edcf03..beb48d65 100644 --- a/src/garage/repair/offline.rs +++ b/src/garage/repair/offline.rs @@ -20,7 +20,7 @@ pub async fn offline_repair( } info!("Loading configuration..."); - let config = fill_secrets(read_config(config_file)?, secrets); + let config = fill_secrets(read_config(config_file)?, secrets)?; info!("Initializing Garage main data store..."); let garage = Garage::new(config)?; diff --git a/src/garage/server.rs b/src/garage/server.rs index 3ad10b72..96ea900d 100644 --- a/src/garage/server.rs +++ b/src/garage/server.rs @@ -29,7 +29,7 @@ async fn wait_from(mut chan: watch::Receiver) { pub async fn run_server(config_file: PathBuf, secrets: Secrets) -> Result<(), Error> { info!("Loading configuration..."); - let config = fill_secrets(read_config(config_file)?, secrets); + let config = fill_secrets(read_config(config_file)?, secrets)?; // ---- Initialize Garage internals ---- -- cgit v1.2.3 From ac04934daefe48ac4f41d22b9129d1fe2ce44833 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 20 Oct 2023 10:29:03 +0200 Subject: s3 api: add missing CORS headers to PostObject responses (fix #609) --- src/api/s3/post_object.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'src') diff --git a/src/api/s3/post_object.rs b/src/api/s3/post_object.rs index 542b7a81..f9eccb7f 100644 --- a/src/api/s3/post_object.rs +++ b/src/api/s3/post_object.rs @@ -15,6 +15,7 @@ use serde::Deserialize; use garage_model::garage::Garage; +use crate::s3::cors::*; use crate::s3::error::*; use crate::s3::put::{get_headers, save_stream}; use crate::s3::xml as s3_xml; @@ -242,7 +243,7 @@ pub async fn handle_post_object( let etag = format!("\"{}\"", md5); - let resp = if let Some(mut target) = params + let mut resp = if let Some(mut target) = params .get("success_action_redirect") .and_then(|h| h.to_str().ok()) .and_then(|u| url::Url::parse(u).ok()) @@ -262,8 +263,7 @@ pub async fn handle_post_object( } else { let path = head .uri - .into_parts() - .path_and_query + .path_and_query() .map(|paq| paq.path().to_string()) .unwrap_or_else(|| "/".to_string()); let authority = head @@ -308,6 +308,13 @@ pub async fn handle_post_object( } }; + let matching_cors_rule = + find_matching_cors_rule(&bucket, &Request::from_parts(head, Body::empty()))?; + if let Some(rule) = matching_cors_rule { + add_cors_headers(&mut resp, rule) + .ok_or_internal_error("Invalid bucket CORS configuration")?; + } + Ok(resp) } -- cgit v1.2.3 From f83fa021937978e79c917c08b3499ba866120284 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Baylac=20Jacqu=C3=A9?= Date: Wed, 25 Oct 2023 11:34:39 +0200 Subject: Add allow_world_readable_secrets option to config file Sometimes, the secret files permissions checks gets in the way. It's by no mean complete, it doesn't take the Posix ACLs into account among other things. Correctly checking the ACLs would be too involving (see https://git.deuxfleurs.fr/Deuxfleurs/garage/issues/658#issuecomment-7102) and would likely still fail in some weird chmod settings. We're adding a new configuration file key allowing the user to disable this permission check altogether. The (already existing) env variable counterpart always take precedence to this config file option. That's useful in cases where the configuration file is static and cannot be easily altered. Fixes https://git.deuxfleurs.fr/Deuxfleurs/garage/issues/658 Co-authored-by: Florian Klink --- src/util/config.rs | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 77 insertions(+), 5 deletions(-) (limited to 'src') diff --git a/src/util/config.rs b/src/util/config.rs index 756f7ec5..2271dd1c 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -38,11 +38,15 @@ pub struct Config { )] pub compression_level: Option, + /// Skip the permission check of secret files. Useful when + /// POSIX ACLs (or more complex chmods) are used. + #[serde(default)] + pub allow_world_readable_secrets: bool, + /// RPC secret key: 32 bytes hex encoded pub rpc_secret: Option, /// Optional file where RPC secret key is read from pub rpc_secret_file: Option, - /// Address to bind for RPC pub rpc_bind_addr: SocketAddr, /// Public IP address of this node @@ -223,16 +227,19 @@ pub fn read_config(config_file: PathBuf) -> Result { &mut parsed_config.rpc_secret, &parsed_config.rpc_secret_file, "rpc_secret", + parsed_config.allow_world_readable_secrets, )?; secret_from_file( &mut parsed_config.admin.metrics_token, &parsed_config.admin.metrics_token_file, "admin.metrics_token", + parsed_config.allow_world_readable_secrets, )?; secret_from_file( &mut parsed_config.admin.admin_token, &parsed_config.admin.admin_token_file, "admin.admin_token", + parsed_config.allow_world_readable_secrets, )?; Ok(parsed_config) @@ -260,6 +267,7 @@ fn secret_from_file( secret: &mut Option, secret_file: &Option, name: &'static str, + conf_allow_world_readable: bool, ) -> Result<(), Error> { match (&secret, &secret_file) { (_, None) => { @@ -268,7 +276,37 @@ fn secret_from_file( (Some(_), Some(_)) => { return Err(format!("only one of `{}` and `{}_file` can be set", name, name).into()); } - (None, Some(file_path)) => *secret = Some(read_secret_file(file_path)?), + (None, Some(file_path)) => { + #[cfg(unix)] + // decide whether to ignore or check permission + // bits. GARAGE_ALLOW_WORLD_READABLE_SECRETS + // always takes precedence over what's specified + // in the config file, to allow overriding it in + // case the config file is read-only. + let ignore_perms = { + let ignore_perms_env = std::env::var("GARAGE_ALLOW_WORLD_READABLE_SECRETS"); + if ignore_perms_env.as_deref() == Ok("true") { + true + } else if ignore_perms_env.as_deref() == Ok("false") { + false + } else { + conf_allow_world_readable + } + }; + if !ignore_perms { + use std::os::unix::fs::MetadataExt; + let metadata = std::fs::metadata(file_path)?; + if metadata.mode() & 0o077 != 0 { + return Err(format!("File {} is world-readable! (mode: 0{:o}, expected 0600)\nRefusing to start until this is fixed, or environment variable GARAGE_ALLOW_WORLD_READABLE_SECRETS is set to true.", file_path, metadata.mode()).into()); + } + } + let mut file = std::fs::OpenOptions::new().read(true).open(file_path)?; + let mut secret_buf = String::new(); + file.read_to_string(&mut secret_buf)?; + // trim_end: allows for use case such as `echo "$(openssl rand -hex 32)" > somefile`. + // also editors sometimes add a trailing newline + *secret = Some(String::from(secret_buf.trim_end())); + } } Ok(()) } @@ -427,11 +465,34 @@ mod tests { "#, path_secret_path.display() )?; - let config = super::read_config(path_config.to_path_buf())?; - assert_eq!("foo", config.rpc_secret.unwrap()); + // Second configuration file, same as previous one + // except it allows world-readable secrets. + let path_config_allow_world_readable = mktemp::Temp::new_file()?; + let mut file_config_allow_world_readable = + File::create(path_config_allow_world_readable.as_path())?; + writeln!( + file_config_allow_world_readable, + r#" + metadata_dir = "/tmp/garage/meta" + data_dir = "/tmp/garage/data" + replication_mode = "3" + rpc_bind_addr = "[::]:3901" + rpc_secret_file = "{}" + allow_world_readable_secrets = true + + [s3_api] + s3_region = "garage" + api_bind_addr = "[::]:3900" + "#, + path_secret_path.display() + )?; + + let mut config = super::read_config(path_config.to_path_buf())?; + assert_eq!("foo", config.rpc_secret.unwrap()); #[cfg(unix)] { + // Check non world-readable secrets config use std::os::unix::fs::PermissionsExt; let metadata = std::fs::metadata(&path_secret_path)?; let mut perm = metadata.permissions(); @@ -443,8 +504,19 @@ mod tests { std::env::set_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS", "true"); assert!(super::read_config(path_config.to_path_buf()).is_ok()); - } + std::env::remove_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS"); + + // Check world-readable secrets config. + assert!(super::read_config(path_config_allow_world_readable.to_path_buf()).is_ok()); + + std::env::set_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS", "false"); + assert!(super::read_config(path_config_allow_world_readable.to_path_buf()).is_err()); + + std::env::set_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS", "true"); + assert!(super::read_config(path_config_allow_world_readable.to_path_buf()).is_ok()); + } + std::env::remove_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS"); drop(path_config); drop(path_secret); drop(file_config); -- cgit v1.2.3 From 7228695ee288012103355589caa1ab5dd666b164 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 15 Jan 2024 17:18:46 +0100 Subject: config: refactor secret sourcing --- src/garage/Cargo.toml | 1 + src/garage/main.rs | 58 +-------- src/garage/repair/offline.rs | 2 +- src/garage/secrets.rs | 280 +++++++++++++++++++++++++++++++++++++++++++ src/garage/server.rs | 2 +- src/util/config.rs | 220 ++-------------------------------- 6 files changed, 292 insertions(+), 271 deletions(-) create mode 100644 src/garage/secrets.rs (limited to 'src') diff --git a/src/garage/Cargo.toml b/src/garage/Cargo.toml index 35d87a3e..00975738 100644 --- a/src/garage/Cargo.toml +++ b/src/garage/Cargo.toml @@ -67,6 +67,7 @@ chrono = "0.4" http = "0.2" hmac = "0.12" hyper = { version = "0.14", features = ["client", "http1", "runtime"] } +mktemp = "0.5" sha2 = "0.10" static_init = "1.0" diff --git a/src/garage/main.rs b/src/garage/main.rs index a9f1ad29..d89762e4 100644 --- a/src/garage/main.rs +++ b/src/garage/main.rs @@ -7,6 +7,7 @@ extern crate tracing; mod admin; mod cli; mod repair; +mod secrets; mod server; #[cfg(feature = "telemetry-otlp")] mod tracing_setup; @@ -25,7 +26,6 @@ use structopt::StructOpt; use netapp::util::parse_and_resolve_peer_addr; use netapp::NetworkKey; -use garage_util::config::{read_secret_file, Config}; use garage_util::error::*; use garage_rpc::system::*; @@ -35,6 +35,7 @@ use garage_model::helper::error::Error as HelperError; use admin::*; use cli::*; +use secrets::Secrets; #[derive(StructOpt, Debug)] #[structopt( @@ -63,39 +64,6 @@ struct Opt { cmd: Command, } -#[derive(StructOpt, Debug)] -pub struct Secrets { - /// RPC secret network key, used to replace rpc_secret in config.toml when running the - /// daemon or doing admin operations - #[structopt(short = "s", long = "rpc-secret", env = "GARAGE_RPC_SECRET")] - pub rpc_secret: Option, - - /// RPC secret network key, used to replace rpc_secret in config.toml and rpc-secret - /// when running the daemon or doing admin operations - #[structopt(long = "rpc-secret-file", env = "GARAGE_RPC_SECRET_FILE")] - pub rpc_secret_file: Option, - - /// Admin API authentication token, replaces admin.admin_token in config.toml when - /// running the Garage daemon - #[structopt(long = "admin-token", env = "GARAGE_ADMIN_TOKEN")] - pub admin_token: Option, - - /// Admin API authentication token file path, replaces admin.admin_token in config.toml - /// and admin-token when running the Garage daemon - #[structopt(long = "admin-token-file", env = "GARAGE_ADMIN_TOKEN_FILE")] - pub admin_token_file: Option, - - /// Metrics API authentication token, replaces admin.metrics_token in config.toml when - /// running the Garage daemon - #[structopt(long = "metrics-token", env = "GARAGE_METRICS_TOKEN")] - pub metrics_token: Option, - - /// Metrics API authentication token file path, replaces admin.metrics_token in config.toml - /// and metrics-token when running the Garage daemon - #[structopt(long = "metrics-token-file", env = "GARAGE_METRICS_TOKEN_FILE")] - pub metrics_token_file: Option, -} - #[tokio::main] async fn main() { // Initialize version and features info @@ -273,25 +241,3 @@ async fn cli_command(opt: Opt) -> Result<(), Error> { Ok(x) => Ok(x), } } - -fn fill_secrets(mut config: Config, secrets: Secrets) -> Result { - if secrets.rpc_secret.is_some() { - config.rpc_secret = secrets.rpc_secret; - } else if secrets.rpc_secret_file.is_some() { - config.rpc_secret = Some(read_secret_file(&secrets.rpc_secret_file.unwrap())?); - } - - if secrets.admin_token.is_some() { - config.admin.admin_token = secrets.admin_token; - } else if secrets.admin_token_file.is_some() { - config.admin.admin_token = Some(read_secret_file(&secrets.admin_token_file.unwrap())?); - } - - if secrets.metrics_token.is_some() { - config.admin.metrics_token = secrets.metrics_token; - } else if secrets.metrics_token_file.is_some() { - config.admin.metrics_token = Some(read_secret_file(&secrets.metrics_token_file.unwrap())?); - } - - Ok(config) -} diff --git a/src/garage/repair/offline.rs b/src/garage/repair/offline.rs index beb48d65..45024e71 100644 --- a/src/garage/repair/offline.rs +++ b/src/garage/repair/offline.rs @@ -6,7 +6,7 @@ use garage_util::error::*; use garage_model::garage::Garage; use crate::cli::structs::*; -use crate::{fill_secrets, Secrets}; +use crate::secrets::{fill_secrets, Secrets}; pub async fn offline_repair( config_file: PathBuf, diff --git a/src/garage/secrets.rs b/src/garage/secrets.rs new file mode 100644 index 00000000..c40c3cc9 --- /dev/null +++ b/src/garage/secrets.rs @@ -0,0 +1,280 @@ +use structopt::StructOpt; + +use garage_util::config::Config; +use garage_util::error::Error; + +/// Structure for secret values or paths that are passed as CLI arguments or environment +/// variables, instead of in the config file. +#[derive(StructOpt, Debug, Default, Clone)] +pub struct Secrets { + /// Skip permission check on files containing secrets + #[cfg(unix)] + #[structopt( + long = "allow-world-readable-secrets", + env = "GARAGE_ALLOW_WORLD_READABLE_SECRETS" + )] + pub allow_world_readable_secrets: Option, + + /// RPC secret network key, used to replace rpc_secret in config.toml when running the + /// daemon or doing admin operations + #[structopt(short = "s", long = "rpc-secret", env = "GARAGE_RPC_SECRET")] + pub rpc_secret: Option, + + /// RPC secret network key, used to replace rpc_secret in config.toml and rpc-secret + /// when running the daemon or doing admin operations + #[structopt(long = "rpc-secret-file", env = "GARAGE_RPC_SECRET_FILE")] + pub rpc_secret_file: Option, + + /// Admin API authentication token, replaces admin.admin_token in config.toml when + /// running the Garage daemon + #[structopt(long = "admin-token", env = "GARAGE_ADMIN_TOKEN")] + pub admin_token: Option, + + /// Admin API authentication token file path, replaces admin.admin_token in config.toml + /// and admin-token when running the Garage daemon + #[structopt(long = "admin-token-file", env = "GARAGE_ADMIN_TOKEN_FILE")] + pub admin_token_file: Option, + + /// Metrics API authentication token, replaces admin.metrics_token in config.toml when + /// running the Garage daemon + #[structopt(long = "metrics-token", env = "GARAGE_METRICS_TOKEN")] + pub metrics_token: Option, + + /// Metrics API authentication token file path, replaces admin.metrics_token in config.toml + /// and metrics-token when running the Garage daemon + #[structopt(long = "metrics-token-file", env = "GARAGE_METRICS_TOKEN_FILE")] + pub metrics_token_file: Option, +} + +/// Single function to fill all secrets in the Config struct from their correct source (value +/// from config or CLI param or env variable or read from a file specified in config or CLI +/// param or env variable) +pub fn fill_secrets(mut config: Config, secrets: Secrets) -> Result { + let allow_world_readable = secrets + .allow_world_readable_secrets + .unwrap_or(config.allow_world_readable_secrets); + + fill_secret( + &mut config.rpc_secret, + &config.rpc_secret_file, + &secrets.rpc_secret, + &secrets.rpc_secret_file, + "rpc_secret", + allow_world_readable, + )?; + + fill_secret( + &mut config.admin.admin_token, + &config.admin.admin_token_file, + &secrets.admin_token, + &secrets.admin_token_file, + "admin.admin_token", + allow_world_readable, + )?; + fill_secret( + &mut config.admin.metrics_token, + &config.admin.metrics_token_file, + &secrets.metrics_token, + &secrets.metrics_token_file, + "admin.metrics_token", + allow_world_readable, + )?; + + Ok(config) +} + +fn fill_secret( + config_secret: &mut Option, + config_secret_file: &Option, + cli_secret: &Option, + cli_secret_file: &Option, + name: &'static str, + allow_world_readable: bool, +) -> Result<(), Error> { + let cli_value = match (&cli_secret, &cli_secret_file) { + (Some(_), Some(_)) => { + return Err(format!("only one of `{}` and `{}_file` can be set", name, name).into()); + } + (Some(secret), None) => Some(secret.to_string()), + (None, Some(file)) => Some(read_secret_file(file, allow_world_readable)?), + (None, None) => None, + }; + + if let Some(val) = cli_value { + if config_secret.is_some() || config_secret_file.is_some() { + debug!("Overriding secret `{}` using value specified using CLI argument or environnement variable.", name); + } + + *config_secret = Some(val); + } else if let Some(file_path) = &config_secret_file { + if config_secret.is_some() { + return Err(format!("only one of `{}` and `{}_file` can be set", name, name).into()); + } + + *config_secret = Some(read_secret_file(file_path, allow_world_readable)?); + } + + Ok(()) +} + +fn read_secret_file(file_path: &String, allow_world_readable: bool) -> Result { + if !allow_world_readable { + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let metadata = std::fs::metadata(file_path)?; + if metadata.mode() & 0o077 != 0 { + return Err(format!("File {} is world-readable! (mode: 0{:o}, expected 0600)\nRefusing to start until this is fixed, or environment variable GARAGE_ALLOW_WORLD_READABLE_SECRETS is set to true.", file_path, metadata.mode()).into()); + } + } + } + + let secret_buf = std::fs::read_to_string(file_path)?; + + // trim_end: allows for use case such as `echo "$(openssl rand -hex 32)" > somefile`. + // also editors sometimes add a trailing newline + Ok(String::from(secret_buf.trim_end())) +} + +#[cfg(test)] +mod tests { + use std::fs::File; + use std::io::Write; + + use garage_util::config::read_config; + use garage_util::error::Error; + + use super::*; + + #[test] + fn test_rpc_secret_file_works() -> Result<(), Error> { + let path_secret = mktemp::Temp::new_file()?; + let mut file_secret = File::create(path_secret.as_path())?; + writeln!(file_secret, "foo")?; + drop(file_secret); + + let path_config = mktemp::Temp::new_file()?; + let mut file_config = File::create(path_config.as_path())?; + let path_secret_path = path_secret.as_path(); + writeln!( + file_config, + r#" + metadata_dir = "/tmp/garage/meta" + data_dir = "/tmp/garage/data" + replication_mode = "3" + rpc_bind_addr = "[::]:3901" + rpc_secret_file = "{}" + + [s3_api] + s3_region = "garage" + api_bind_addr = "[::]:3900" + "#, + path_secret_path.display() + )?; + drop(file_config); + + // Second configuration file, same as previous one + // except it allows world-readable secrets. + let path_config_allow_world_readable = mktemp::Temp::new_file()?; + let mut file_config_allow_world_readable = + File::create(path_config_allow_world_readable.as_path())?; + writeln!( + file_config_allow_world_readable, + r#" + metadata_dir = "/tmp/garage/meta" + data_dir = "/tmp/garage/data" + replication_mode = "3" + rpc_bind_addr = "[::]:3901" + rpc_secret_file = "{}" + allow_world_readable_secrets = true + + [s3_api] + s3_region = "garage" + api_bind_addr = "[::]:3900" + "#, + path_secret_path.display() + )?; + drop(file_config_allow_world_readable); + + let config = read_config(path_config.to_path_buf())?; + let config = fill_secrets(config, Secrets::default())?; + assert_eq!("foo", config.rpc_secret.unwrap()); + + #[cfg(unix)] + { + // Check non world-readable secrets config + + let secrets_allow_world_readable = Secrets { + allow_world_readable_secrets: Some(true), + ..Default::default() + }; + let secrets_no_allow_world_readable = Secrets { + allow_world_readable_secrets: Some(false), + ..Default::default() + }; + + use std::os::unix::fs::PermissionsExt; + let metadata = std::fs::metadata(&path_secret_path)?; + let mut perm = metadata.permissions(); + perm.set_mode(0o660); + std::fs::set_permissions(&path_secret_path, perm)?; + + // Config file that just specifies the path + let config = read_config(path_config.to_path_buf())?; + assert!(fill_secrets(config, Secrets::default()).is_err()); + + let config = read_config(path_config.to_path_buf())?; + assert!(fill_secrets(config, secrets_allow_world_readable.clone()).is_ok()); + + let config = read_config(path_config.to_path_buf())?; + assert!(fill_secrets(config, secrets_no_allow_world_readable.clone()).is_err()); + + // Config file that also specifies to allow world_readable_secrets + let config = read_config(path_config_allow_world_readable.to_path_buf())?; + assert!(fill_secrets(config, Secrets::default()).is_ok()); + + let config = read_config(path_config_allow_world_readable.to_path_buf())?; + assert!(fill_secrets(config, secrets_allow_world_readable).is_ok()); + + let config = read_config(path_config_allow_world_readable.to_path_buf())?; + assert!(fill_secrets(config, secrets_no_allow_world_readable).is_err()); + } + + drop(path_secret); + drop(path_config); + drop(path_config_allow_world_readable); + + Ok(()) + } + + #[test] + fn test_rcp_secret_and_rpc_secret_file_cannot_be_set_both() -> Result<(), Error> { + let path_config = mktemp::Temp::new_file()?; + let mut file_config = File::create(path_config.as_path())?; + writeln!( + file_config, + r#" + metadata_dir = "/tmp/garage/meta" + data_dir = "/tmp/garage/data" + replication_mode = "3" + rpc_bind_addr = "[::]:3901" + rpc_secret= "dummy" + rpc_secret_file = "dummy" + + [s3_api] + s3_region = "garage" + api_bind_addr = "[::]:3900" + "# + )?; + let config = read_config(path_config.to_path_buf())?; + assert_eq!( + "only one of `rpc_secret` and `rpc_secret_file` can be set", + fill_secrets(config, Secrets::default()) + .unwrap_err() + .to_string() + ); + drop(path_config); + drop(file_config); + Ok(()) + } +} diff --git a/src/garage/server.rs b/src/garage/server.rs index 96ea900d..25d4b845 100644 --- a/src/garage/server.rs +++ b/src/garage/server.rs @@ -15,9 +15,9 @@ use garage_web::WebServer; use garage_api::k2v::api_server::K2VApiServer; use crate::admin::*; +use crate::secrets::{fill_secrets, Secrets}; #[cfg(feature = "telemetry-otlp")] use crate::tracing_setup::*; -use crate::{fill_secrets, Secrets}; async fn wait_from(mut chan: watch::Receiver) { while !*chan.borrow() { diff --git a/src/util/config.rs b/src/util/config.rs index 2271dd1c..add78278 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -1,6 +1,5 @@ //! Contains type and functions related to Garage configuration file use std::convert::TryFrom; -use std::io::Read; use std::net::SocketAddr; use std::path::PathBuf; @@ -198,6 +197,13 @@ pub struct KubernetesDiscoveryConfig { pub skip_crd: bool, } +/// Read and parse configuration +pub fn read_config(config_file: PathBuf) -> Result { + let config = std::fs::read_to_string(config_file)?; + + Ok(toml::from_str(&config)?) +} + fn default_db_engine() -> String { "sled".into() } @@ -212,105 +218,6 @@ fn default_block_size() -> usize { 1048576 } -/// Read and parse configuration -pub fn read_config(config_file: PathBuf) -> Result { - let mut file = std::fs::OpenOptions::new() - .read(true) - .open(config_file.as_path())?; - - let mut config = String::new(); - file.read_to_string(&mut config)?; - - let mut parsed_config: Config = toml::from_str(&config)?; - - secret_from_file( - &mut parsed_config.rpc_secret, - &parsed_config.rpc_secret_file, - "rpc_secret", - parsed_config.allow_world_readable_secrets, - )?; - secret_from_file( - &mut parsed_config.admin.metrics_token, - &parsed_config.admin.metrics_token_file, - "admin.metrics_token", - parsed_config.allow_world_readable_secrets, - )?; - secret_from_file( - &mut parsed_config.admin.admin_token, - &parsed_config.admin.admin_token_file, - "admin.admin_token", - parsed_config.allow_world_readable_secrets, - )?; - - Ok(parsed_config) -} - -pub fn read_secret_file(file_path: &String) -> Result { - #[cfg(unix)] - if std::env::var("GARAGE_ALLOW_WORLD_READABLE_SECRETS").as_deref() != Ok("true") { - use std::os::unix::fs::MetadataExt; - let metadata = std::fs::metadata(file_path)?; - if metadata.mode() & 0o077 != 0 { - return Err(format!("File {} is world-readable! (mode: 0{:o}, expected 0600)\nRefusing to start until this is fixed, or environment variable GARAGE_ALLOW_WORLD_READABLE_SECRETS is set to true.", file_path, metadata.mode()).into()); - } - } - let mut file = std::fs::OpenOptions::new().read(true).open(file_path)?; - let mut secret_buf = String::new(); - file.read_to_string(&mut secret_buf)?; - - // trim_end: allows for use case such as `echo "$(openssl rand -hex 32)" > somefile`. - // also editors sometimes add a trailing newline - Ok(String::from(secret_buf.trim_end())) -} - -fn secret_from_file( - secret: &mut Option, - secret_file: &Option, - name: &'static str, - conf_allow_world_readable: bool, -) -> Result<(), Error> { - match (&secret, &secret_file) { - (_, None) => { - // no-op - } - (Some(_), Some(_)) => { - return Err(format!("only one of `{}` and `{}_file` can be set", name, name).into()); - } - (None, Some(file_path)) => { - #[cfg(unix)] - // decide whether to ignore or check permission - // bits. GARAGE_ALLOW_WORLD_READABLE_SECRETS - // always takes precedence over what's specified - // in the config file, to allow overriding it in - // case the config file is read-only. - let ignore_perms = { - let ignore_perms_env = std::env::var("GARAGE_ALLOW_WORLD_READABLE_SECRETS"); - if ignore_perms_env.as_deref() == Ok("true") { - true - } else if ignore_perms_env.as_deref() == Ok("false") { - false - } else { - conf_allow_world_readable - } - }; - if !ignore_perms { - use std::os::unix::fs::MetadataExt; - let metadata = std::fs::metadata(file_path)?; - if metadata.mode() & 0o077 != 0 { - return Err(format!("File {} is world-readable! (mode: 0{:o}, expected 0600)\nRefusing to start until this is fixed, or environment variable GARAGE_ALLOW_WORLD_READABLE_SECRETS is set to true.", file_path, metadata.mode()).into()); - } - } - let mut file = std::fs::OpenOptions::new().read(true).open(file_path)?; - let mut secret_buf = String::new(); - file.read_to_string(&mut secret_buf)?; - // trim_end: allows for use case such as `echo "$(openssl rand -hex 32)" > somefile`. - // also editors sometimes add a trailing newline - *secret = Some(String::from(secret_buf.trim_end())); - } - } - Ok(()) -} - fn default_compression() -> Option { Some(1) } @@ -439,117 +346,4 @@ mod tests { Ok(()) } - - #[test] - fn test_rpc_secret_file_works() -> Result<(), Error> { - let path_secret = mktemp::Temp::new_file()?; - let mut file_secret = File::create(path_secret.as_path())?; - writeln!(file_secret, "foo")?; - drop(file_secret); - - let path_config = mktemp::Temp::new_file()?; - let mut file_config = File::create(path_config.as_path())?; - let path_secret_path = path_secret.as_path(); - writeln!( - file_config, - r#" - metadata_dir = "/tmp/garage/meta" - data_dir = "/tmp/garage/data" - replication_mode = "3" - rpc_bind_addr = "[::]:3901" - rpc_secret_file = "{}" - - [s3_api] - s3_region = "garage" - api_bind_addr = "[::]:3900" - "#, - path_secret_path.display() - )?; - - // Second configuration file, same as previous one - // except it allows world-readable secrets. - let path_config_allow_world_readable = mktemp::Temp::new_file()?; - let mut file_config_allow_world_readable = - File::create(path_config_allow_world_readable.as_path())?; - writeln!( - file_config_allow_world_readable, - r#" - metadata_dir = "/tmp/garage/meta" - data_dir = "/tmp/garage/data" - replication_mode = "3" - rpc_bind_addr = "[::]:3901" - rpc_secret_file = "{}" - allow_world_readable_secrets = true - - [s3_api] - s3_region = "garage" - api_bind_addr = "[::]:3900" - "#, - path_secret_path.display() - )?; - - let mut config = super::read_config(path_config.to_path_buf())?; - assert_eq!("foo", config.rpc_secret.unwrap()); - #[cfg(unix)] - { - // Check non world-readable secrets config - use std::os::unix::fs::PermissionsExt; - let metadata = std::fs::metadata(&path_secret_path)?; - let mut perm = metadata.permissions(); - perm.set_mode(0o660); - std::fs::set_permissions(&path_secret_path, perm)?; - - std::env::set_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS", "false"); - assert!(super::read_config(path_config.to_path_buf()).is_err()); - - std::env::set_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS", "true"); - assert!(super::read_config(path_config.to_path_buf()).is_ok()); - - std::env::remove_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS"); - - // Check world-readable secrets config. - assert!(super::read_config(path_config_allow_world_readable.to_path_buf()).is_ok()); - - std::env::set_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS", "false"); - assert!(super::read_config(path_config_allow_world_readable.to_path_buf()).is_err()); - - std::env::set_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS", "true"); - assert!(super::read_config(path_config_allow_world_readable.to_path_buf()).is_ok()); - } - std::env::remove_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS"); - drop(path_config); - drop(path_secret); - drop(file_config); - Ok(()) - } - - #[test] - fn test_rcp_secret_and_rpc_secret_file_cannot_be_set_both() -> Result<(), Error> { - let path_config = mktemp::Temp::new_file()?; - let mut file_config = File::create(path_config.as_path())?; - writeln!( - file_config, - r#" - metadata_dir = "/tmp/garage/meta" - data_dir = "/tmp/garage/data" - replication_mode = "3" - rpc_bind_addr = "[::]:3901" - rpc_secret= "dummy" - rpc_secret_file = "dummy" - - [s3_api] - s3_region = "garage" - api_bind_addr = "[::]:3900" - "# - )?; - assert_eq!( - "only one of `rpc_secret` and `rpc_secret_file` can be set", - super::read_config(path_config.to_path_buf()) - .unwrap_err() - .to_string() - ); - drop(path_config); - drop(file_config); - Ok(()) - } } -- cgit v1.2.3 From 97bae7213aa214022b68b65094c3e152826de408 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 15 Jan 2024 17:30:30 +0100 Subject: config: additional tests for secret sourcing --- src/garage/secrets.rs | 42 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) (limited to 'src') diff --git a/src/garage/secrets.rs b/src/garage/secrets.rs index c40c3cc9..e96be9e4 100644 --- a/src/garage/secrets.rs +++ b/src/garage/secrets.rs @@ -200,10 +200,9 @@ mod tests { let config = fill_secrets(config, Secrets::default())?; assert_eq!("foo", config.rpc_secret.unwrap()); + // ---- Check non world-readable secrets config ---- #[cfg(unix)] { - // Check non world-readable secrets config - let secrets_allow_world_readable = Secrets { allow_world_readable_secrets: Some(true), ..Default::default() @@ -240,7 +239,46 @@ mod tests { assert!(fill_secrets(config, secrets_no_allow_world_readable).is_err()); } + // ---- Check alternative secrets specified on CLI ---- + + let path_secret2 = mktemp::Temp::new_file()?; + let mut file_secret2 = File::create(path_secret2.as_path())?; + writeln!(file_secret2, "bar")?; + drop(file_secret2); + + let config = read_config(path_config.to_path_buf())?; + let config = fill_secrets( + config, + Secrets { + rpc_secret: Some("baz".into()), + ..Default::default() + }, + )?; + assert_eq!(config.rpc_secret.as_deref(), Some("baz")); + + let config = read_config(path_config.to_path_buf())?; + let config = fill_secrets( + config, + Secrets { + rpc_secret_file: Some(path_secret2.display().to_string()), + ..Default::default() + }, + )?; + assert_eq!(config.rpc_secret.as_deref(), Some("bar")); + + let config = read_config(path_config.to_path_buf())?; + assert!(fill_secrets( + config, + Secrets { + rpc_secret: Some("baz".into()), + rpc_secret_file: Some(path_secret2.display().to_string()), + ..Default::default() + } + ) + .is_err()); + drop(path_secret); + drop(path_secret2); drop(path_config); drop(path_config_allow_world_readable); -- cgit v1.2.3 From f512609123fdf374839ca2a8385ddda8694d09fa Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 15 Jan 2024 17:33:35 +0100 Subject: monitoring: finer histogram boundaries in prometheus metrics (fix #531) --- src/garage/server.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'src') diff --git a/src/garage/server.rs b/src/garage/server.rs index 96ea900d..b6570301 100644 --- a/src/garage/server.rs +++ b/src/garage/server.rs @@ -34,7 +34,14 @@ pub async fn run_server(config_file: PathBuf, secrets: Secrets) -> Result<(), Er // ---- Initialize Garage internals ---- #[cfg(feature = "metrics")] - let metrics_exporter = opentelemetry_prometheus::exporter().init(); + let metrics_exporter = opentelemetry_prometheus::exporter() + .with_default_summary_quantiles(vec![0.25, 0.5, 0.75, 0.9, 0.95, 0.99]) + .with_default_histogram_boundaries(vec![ + 0.001, 0.0015, 0.002, 0.003, 0.005, 0.007, 0.01, 0.015, 0.02, 0.03, 0.05, 0.07, 0.1, + 0.15, 0.2, 0.3, 0.5, 0.7, 1., 1.5, 2., 3., 5., 7., 10., 15., 20., 30., 40., 50., 60., + 70., 100., + ]) + .init(); info!("Initializing Garage main data store..."); let garage = Garage::new(config.clone())?; -- cgit v1.2.3 From 50643e61bfb4ef0782e4364ed368bdfa62be7e5e Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 16 Jan 2024 10:47:33 +0100 Subject: Bump version to 0.8.5 --- src/api/Cargo.toml | 2 +- src/block/Cargo.toml | 2 +- src/db/Cargo.toml | 2 +- src/garage/Cargo.toml | 2 +- src/model/Cargo.toml | 2 +- src/rpc/Cargo.toml | 2 +- src/table/Cargo.toml | 2 +- src/util/Cargo.toml | 2 +- src/web/Cargo.toml | 2 +- 9 files changed, 9 insertions(+), 9 deletions(-) (limited to 'src') diff --git a/src/api/Cargo.toml b/src/api/Cargo.toml index cb9e2e55..43167fdb 100644 --- a/src/api/Cargo.toml +++ b/src/api/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_api" -version = "0.8.4" +version = "0.8.5" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/block/Cargo.toml b/src/block/Cargo.toml index 963086ac..8fd1e0c5 100644 --- a/src/block/Cargo.toml +++ b/src/block/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_block" -version = "0.8.4" +version = "0.8.5" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/db/Cargo.toml b/src/db/Cargo.toml index fa1a4419..edd4eaf4 100644 --- a/src/db/Cargo.toml +++ b/src/db/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_db" -version = "0.8.4" +version = "0.8.5" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/garage/Cargo.toml b/src/garage/Cargo.toml index 00975738..88c4c731 100644 --- a/src/garage/Cargo.toml +++ b/src/garage/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage" -version = "0.8.4" +version = "0.8.5" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/model/Cargo.toml b/src/model/Cargo.toml index 1d3600a6..1f2624f0 100644 --- a/src/model/Cargo.toml +++ b/src/model/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_model" -version = "0.8.4" +version = "0.8.5" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/rpc/Cargo.toml b/src/rpc/Cargo.toml index 0cda723e..b7a340f3 100644 --- a/src/rpc/Cargo.toml +++ b/src/rpc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_rpc" -version = "0.8.4" +version = "0.8.5" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/table/Cargo.toml b/src/table/Cargo.toml index 0d2a3103..aff77369 100644 --- a/src/table/Cargo.toml +++ b/src/table/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_table" -version = "0.8.4" +version = "0.8.5" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/util/Cargo.toml b/src/util/Cargo.toml index 2efb0270..444254d1 100644 --- a/src/util/Cargo.toml +++ b/src/util/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_util" -version = "0.8.4" +version = "0.8.5" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" diff --git a/src/web/Cargo.toml b/src/web/Cargo.toml index eec47bcd..aa1cf4fe 100644 --- a/src/web/Cargo.toml +++ b/src/web/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_web" -version = "0.8.4" +version = "0.8.5" authors = ["Alex Auvolat ", "Quentin Dufour "] edition = "2018" license = "AGPL-3.0" -- cgit v1.2.3