From 656b8d42de2fc945c988094418c90d29d000be32 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Fri, 3 Feb 2023 15:27:39 +0100 Subject: [PATCH 1/2] secrets can be passed directly in config, as file, or as env --- doc/book/reference-manual/configuration.md | 40 ++++--- src/garage/main.rs | 45 ++++++-- src/garage/repair/offline.rs | 9 +- src/garage/server.rs | 5 +- src/model/garage.rs | 23 ++-- src/util/config.rs | 116 +++++++++++++-------- 6 files changed, 156 insertions(+), 82 deletions(-) diff --git a/doc/book/reference-manual/configuration.md b/doc/book/reference-manual/configuration.md index 7a829836..b72c43a3 100644 --- a/doc/book/reference-manual/configuration.md +++ b/doc/book/reference-manual/configuration.md @@ -3,6 +3,8 @@ title = "Configuration file format" weight = 20 +++ +## Full example + Here is an example `garage.toml` configuration file that illustrates all of the possible options: ```toml @@ -259,17 +261,17 @@ Compression is done synchronously, setting a value too high will add latency to This value can be different between nodes, compression is done by the node which receive the API call. -### `rpc_secret` +### `rpc_secret`, `rpc_secret_file` or `GARAGE_RPC_SECRET` (env) -Garage uses a secret key that is shared between all nodes of the cluster -in order to identify these nodes and allow them to communicate together. -This key should be specified here in the form of a 32-byte hex-encoded -random string. Such a string can be generated with a command -such as `openssl rand -hex 32`. +Garage uses a secret key, called an RPC secret, that is shared between all +nodes of the cluster in order to identify these nodes and allow them to +communicate together. The RPC secret is a 32-byte hex-encoded random string, +which can be generated with a command such as `openssl rand -hex 32`. -### `rpc_secret_file` - -Like `rpc_secret` above, just that this is the path to a file that Garage will try to read the secret from. +The RPC secret should be specified in the `rpc_secret` configuration variable. +Since Garage v0.8.2, the RPC secret can also be stored in a file whose path is +given in the configuration variable `rpc_secret_file`, or specified as an +environment variable `GARAGE_RPC_SECRET`. ### `rpc_bind_addr` @@ -411,22 +413,28 @@ If specified, Garage will bind an HTTP server to this port and address, on which it will listen to requests for administration features. See [administration API reference](@/documentation/reference-manual/admin-api.md) to learn more about these features. -### `metrics_token` (since version 0.7.2) +### `metrics_token`, `metrics_token_file` or `GARAGE_METRICS_TOKEN` (env) -The token for accessing the Metrics endpoint. If this token is not set in -the config file, the Metrics endpoint can be accessed without access -control. +The token for accessing the Metrics endpoint. If this token is not set, the +Metrics endpoint can be accessed without access control. You can use any random string for this value. We recommend generating a random token with `openssl rand -hex 32`. -### `admin_token` (since version 0.7.2) +`metrics_token` was introduced in Garage version 0.7.2. +`metrics_token_file` and the `GARAGE_METRICS_TOKEN` environment variable are supported since Garage version 0.8.2. + + +### `admin_token`, `admin_token_file` or `GARAGE_ADMIN_TOKEN` (env) The token for accessing all of the other administration endpoints. If this -token is not set in the config file, access to these endpoints is disabled -entirely. +token is not set, access to these endpoints is disabled entirely. You can use any random string for this value. We recommend generating a random token with `openssl rand -hex 32`. +`admin_token` was introduced in Garage version 0.7.2. +`admin_token_file` and the `GARAGE_ADMIN_TOKEN` environment variable are supported since Garage version 0.8.2. + + ### `trace_sink` Optionally, the address of an OpenTelemetry collector. If specified, diff --git a/src/garage/main.rs b/src/garage/main.rs index 736e11ec..2bd0164e 100644 --- a/src/garage/main.rs +++ b/src/garage/main.rs @@ -25,6 +25,7 @@ use structopt::StructOpt; use netapp::util::parse_and_resolve_peer_addr; use netapp::NetworkKey; +use garage_util::config::Config; use garage_util::error::*; use garage_rpc::system::*; @@ -46,11 +47,10 @@ struct Opt { #[structopt(short = "h", long = "rpc-host", env = "GARAGE_RPC_HOST")] pub rpc_host: Option, - /// RPC secret network key for admin operations - #[structopt(short = "s", long = "rpc-secret", env = "GARAGE_RPC_SECRET")] - pub rpc_secret: Option, + #[structopt(flatten)] + pub secrets: Secrets, - /// Configuration file (garage.toml) + /// Path to configuration file #[structopt( short = "c", long = "config", @@ -63,6 +63,23 @@ 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, + + /// Metrics API authentication token, replaces admin.metrics_token in config.toml when + /// running the Garage daemon + #[structopt(long = "admin-token", env = "GARAGE_ADMIN_TOKEN")] + pub admin_token: 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, +} + #[tokio::main] async fn main() { // Initialize version and features info @@ -145,9 +162,9 @@ async fn main() { sodiumoxide::init().expect("Unable to init sodiumoxide"); let res = match opt.cmd { - Command::Server => server::run_server(opt.config_file).await, + Command::Server => server::run_server(opt.config_file, opt.secrets).await, Command::OfflineRepair(repair_opt) => { - repair::offline::offline_repair(opt.config_file, repair_opt).await + repair::offline::offline_repair(opt.config_file, opt.secrets, repair_opt).await } Command::Node(NodeOperation::NodeId(node_id_opt)) => { node_id_command(opt.config_file, node_id_opt.quiet) @@ -162,7 +179,7 @@ async fn main() { } async fn cli_command(opt: Opt) -> Result<(), Error> { - let config = if opt.rpc_secret.is_none() || opt.rpc_host.is_none() { + let config = if opt.secrets.rpc_secret.is_none() || opt.rpc_host.is_none() { Some(garage_util::config::read_config(opt.config_file.clone()) .err_context(format!("Unable to read configuration file {}. Configuration file is needed because -h or -s is not provided on the command line.", opt.config_file.to_string_lossy()))?) } else { @@ -171,6 +188,7 @@ async fn cli_command(opt: Opt) -> Result<(), Error> { // Find and parse network RPC secret let net_key_hex_str = opt + .secrets .rpc_secret .as_ref() .or_else(|| config.as_ref().and_then(|c| c.rpc_secret.as_ref())) @@ -230,3 +248,16 @@ async fn cli_command(opt: Opt) -> Result<(), Error> { Ok(x) => Ok(x), } } + +fn fill_secrets(mut config: Config, secrets: Secrets) -> Config { + if secrets.rpc_secret.is_some() { + config.rpc_secret = secrets.rpc_secret; + } + if secrets.admin_token.is_some() { + config.admin.admin_token = secrets.admin_token; + } + if secrets.metrics_token.is_some() { + config.admin.metrics_token = secrets.metrics_token; + } + config +} diff --git a/src/garage/repair/offline.rs b/src/garage/repair/offline.rs index 25193e4a..f4edcf03 100644 --- a/src/garage/repair/offline.rs +++ b/src/garage/repair/offline.rs @@ -6,8 +6,13 @@ use garage_util::error::*; use garage_model::garage::Garage; use crate::cli::structs::*; +use crate::{fill_secrets, Secrets}; -pub async fn offline_repair(config_file: PathBuf, opt: OfflineRepairOpt) -> Result<(), Error> { +pub async fn offline_repair( + config_file: PathBuf, + secrets: Secrets, + opt: OfflineRepairOpt, +) -> Result<(), Error> { if !opt.yes { return Err(Error::Message( "Please add the --yes flag to launch repair operation".into(), @@ -15,7 +20,7 @@ pub async fn offline_repair(config_file: PathBuf, opt: OfflineRepairOpt) -> Resu } info!("Loading configuration..."); - let config = read_config(config_file)?; + 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 16f1b625..958089c6 100644 --- a/src/garage/server.rs +++ b/src/garage/server.rs @@ -17,6 +17,7 @@ use garage_api::k2v::api_server::K2VApiServer; use crate::admin::*; #[cfg(feature = "telemetry-otlp")] use crate::tracing_setup::*; +use crate::{fill_secrets, Secrets}; async fn wait_from(mut chan: watch::Receiver) { while !*chan.borrow() { @@ -26,9 +27,9 @@ async fn wait_from(mut chan: watch::Receiver) { } } -pub async fn run_server(config_file: PathBuf) -> Result<(), Error> { +pub async fn run_server(config_file: PathBuf, secrets: Secrets) -> Result<(), Error> { info!("Loading configuration..."); - let config = read_config(config_file)?; + let config = fill_secrets(read_config(config_file)?, secrets); // ---- Initialize Garage internals ---- diff --git a/src/model/garage.rs b/src/model/garage.rs index 92cab17c..0a9ec608 100644 --- a/src/model/garage.rs +++ b/src/model/garage.rs @@ -98,7 +98,7 @@ impl Garage { .cache_capacity(config.sled_cache_capacity) .flush_every_ms(Some(config.sled_flush_every_ms)) .open() - .expect("Unable to open sled DB"); + .ok_or_message("Unable to open sled DB")?; db::sled_adapter::SledDb::init(db) } #[cfg(not(feature = "sled"))] @@ -109,7 +109,7 @@ impl Garage { db_path.push("db.sqlite"); info!("Opening Sqlite database at: {}", db_path.display()); let db = db::sqlite_adapter::rusqlite::Connection::open(db_path) - .expect("Unable to open sqlite DB"); + .ok_or_message("Unable to open sqlite DB")?; db::sqlite_adapter::SqliteDb::init(db) } #[cfg(not(feature = "sqlite"))] @@ -123,7 +123,8 @@ impl Garage { "lmdb" | "heed" => { db_path.push("db.lmdb"); info!("Opening LMDB database at: {}", db_path.display()); - std::fs::create_dir_all(&db_path).expect("Unable to create LMDB data directory"); + std::fs::create_dir_all(&db_path) + .ok_or_message("Unable to create LMDB data directory")?; let map_size = garage_db::lmdb_adapter::recommended_map_size(); use db::lmdb_adapter::heed; @@ -135,7 +136,9 @@ impl Garage { env_builder.flag(heed::flags::Flags::MdbNoSync); env_builder.flag(heed::flags::Flags::MdbNoMetaSync); } - let db = env_builder.open(&db_path).expect("Unable to open LMDB DB"); + let db = env_builder + .open(&db_path) + .ok_or_message("Unable to open LMDB DB")?; db::lmdb_adapter::LmdbDb::init(db) } #[cfg(not(feature = "lmdb"))] @@ -158,13 +161,15 @@ impl Garage { } }; - let network_key = NetworkKey::from_slice( - &hex::decode(config.rpc_secret.as_ref().unwrap()).expect("Invalid RPC secret key")[..], - ) - .expect("Invalid RPC secret key"); + 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", + )?) + .ok() + .and_then(|x| NetworkKey::from_slice(&x)) + .ok_or_message("Invalid RPC secret key")?; let replication_mode = ReplicationMode::parse(&config.replication_mode) - .expect("Invalid replication_mode in config file."); + .ok_or_message("Invalid replication_mode in config file.")?; info!("Initialize membership management system..."); let system = System::new(network_key, replication_mode, &config)?; diff --git a/src/util/config.rs b/src/util/config.rs index f0a881aa..c59e9c1d 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -34,9 +34,7 @@ pub struct Config { pub compression_level: Option, /// RPC secret key: 32 bytes hex encoded - /// Note: When using `read_config` this should never be `None` pub rpc_secret: Option, - /// Optional file where RPC secret key is read from pub rpc_secret_file: Option, @@ -122,10 +120,17 @@ pub struct WebConfig { pub struct AdminConfig { /// Address and port to bind for admin API serving pub api_bind_addr: Option, + /// Bearer token to use to scrape metrics pub metrics_token: Option, + /// File to read metrics token from + pub metrics_token_file: Option, + /// Bearer token to use to access Admin API endpoints pub admin_token: Option, + /// File to read admin token from + pub admin_token_file: Option, + /// OTLP server to where to export traces pub trace_sink: Option, } @@ -183,29 +188,55 @@ pub fn read_config(config_file: PathBuf) -> Result { let mut parsed_config: Config = toml::from_str(&config)?; - match (&parsed_config.rpc_secret, &parsed_config.rpc_secret_file) { - (Some(_), None) => { + secret_from_file( + &mut parsed_config.rpc_secret, + &mut parsed_config.rpc_secret_file, + "rpc_secret", + )?; + secret_from_file( + &mut parsed_config.admin.metrics_token, + &mut parsed_config.admin.metrics_token_file, + "admin.metrics_token", + )?; + secret_from_file( + &mut parsed_config.admin.admin_token, + &mut parsed_config.admin.admin_token_file, + "admin.admin_token", + )?; + + Ok(parsed_config) +} + +fn secret_from_file( + secret: &mut Option, + secret_file: &mut Option, + name: &'static str, +) -> Result<(), Error> { + match (&secret, &secret_file) { + (_, None) => { // no-op } (Some(_), Some(_)) => { - return Err("only one of `rpc_secret` and `rpc_secret_file` can be set".into()) + return Err(format!("only one of `{}` and `{}_file` can be set", name, name).into()); } - (None, Some(rpc_secret_file_path_string)) => { - let mut rpc_secret_file = std::fs::OpenOptions::new() - .read(true) - .open(rpc_secret_file_path_string)?; - let mut rpc_secret_from_file = String::new(); - rpc_secret_file.read_to_string(&mut rpc_secret_from_file)?; + (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 - parsed_config.rpc_secret = Some(String::from(rpc_secret_from_file.trim_end())); + *secret = Some(String::from(secret_buf.trim_end())); } - (None, None) => { - return Err("either `rpc_secret` or `rpc_secret_file` needs to be set".into()) - } - }; - - Ok(parsed_config) + } + Ok(()) } fn default_compression() -> Option { @@ -269,31 +300,7 @@ mod tests { use std::io::Write; #[test] - fn test_rpc_secret_is_required() -> Result<(), Error> { - let path1 = mktemp::Temp::new_file()?; - let mut file1 = File::create(path1.as_path())?; - writeln!( - file1, - r#" - metadata_dir = "/tmp/garage/meta" - data_dir = "/tmp/garage/data" - replication_mode = "3" - rpc_bind_addr = "[::]:3901" - - [s3_api] - s3_region = "garage" - api_bind_addr = "[::]:3900" - "# - )?; - assert_eq!( - "either `rpc_secret` or `rpc_secret_file` needs to be set", - super::read_config(path1.to_path_buf()) - .unwrap_err() - .to_string() - ); - drop(path1); - drop(file1); - + fn test_rpc_secret() -> Result<(), Error> { let path2 = mktemp::Temp::new_file()?; let mut file2 = File::create(path2.as_path())?; writeln!( @@ -328,7 +335,7 @@ mod tests { 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().display(); + let path_secret_path = path_secret.as_path(); writeln!( file_config, r#" @@ -336,15 +343,32 @@ mod tests { data_dir = "/tmp/garage/data" replication_mode = "3" rpc_bind_addr = "[::]:3901" - rpc_secret_file = "{path_secret_path}" + rpc_secret_file = "{}" [s3_api] s3_region = "garage" api_bind_addr = "[::]:3900" - "# + "#, + path_secret_path.display() )?; let config = super::read_config(path_config.to_path_buf())?; assert_eq!("foo", config.rpc_secret.unwrap()); + + #[cfg(unix)] + { + 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()); + } + drop(path_config); drop(path_secret); drop(file_config); From 80e232699825c5c512e8714e08b6a80992a06498 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 6 Feb 2023 12:23:55 +0100 Subject: [PATCH 2/2] fixes for pr 499 --- doc/book/reference-manual/configuration.md | 10 +++++----- src/garage/main.rs | 3 ++- src/util/config.rs | 8 ++++---- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/doc/book/reference-manual/configuration.md b/doc/book/reference-manual/configuration.md index b72c43a3..38062bab 100644 --- a/doc/book/reference-manual/configuration.md +++ b/doc/book/reference-manual/configuration.md @@ -269,7 +269,7 @@ communicate together. The RPC secret is a 32-byte hex-encoded random string, which can be generated with a command such as `openssl rand -hex 32`. The RPC secret should be specified in the `rpc_secret` configuration variable. -Since Garage v0.8.2, the RPC secret can also be stored in a file whose path is +Since Garage `v0.8.2`, the RPC secret can also be stored in a file whose path is given in the configuration variable `rpc_secret_file`, or specified as an environment variable `GARAGE_RPC_SECRET`. @@ -420,8 +420,8 @@ Metrics endpoint can be accessed without access control. You can use any random string for this value. We recommend generating a random token with `openssl rand -hex 32`. -`metrics_token` was introduced in Garage version 0.7.2. -`metrics_token_file` and the `GARAGE_METRICS_TOKEN` environment variable are supported since Garage version 0.8.2. +`metrics_token` was introduced in Garage `v0.7.2`. +`metrics_token_file` and the `GARAGE_METRICS_TOKEN` environment variable are supported since Garage `v0.8.2`. ### `admin_token`, `admin_token_file` or `GARAGE_ADMIN_TOKEN` (env) @@ -431,8 +431,8 @@ token is not set, access to these endpoints is disabled entirely. You can use any random string for this value. We recommend generating a random token with `openssl rand -hex 32`. -`admin_token` was introduced in Garage version 0.7.2. -`admin_token_file` and the `GARAGE_ADMIN_TOKEN` environment variable are supported since Garage version 0.8.2. +`admin_token` was introduced in Garage `v0.7.2`. +`admin_token_file` and the `GARAGE_ADMIN_TOKEN` environment variable are supported since Garage `v0.8.2`. ### `trace_sink` diff --git a/src/garage/main.rs b/src/garage/main.rs index 2bd0164e..1ab18bb2 100644 --- a/src/garage/main.rs +++ b/src/garage/main.rs @@ -65,7 +65,8 @@ struct Opt { #[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 + /// 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, diff --git a/src/util/config.rs b/src/util/config.rs index c59e9c1d..2176353e 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -190,17 +190,17 @@ pub fn read_config(config_file: PathBuf) -> Result { secret_from_file( &mut parsed_config.rpc_secret, - &mut parsed_config.rpc_secret_file, + &parsed_config.rpc_secret_file, "rpc_secret", )?; secret_from_file( &mut parsed_config.admin.metrics_token, - &mut parsed_config.admin.metrics_token_file, + &parsed_config.admin.metrics_token_file, "admin.metrics_token", )?; secret_from_file( &mut parsed_config.admin.admin_token, - &mut parsed_config.admin.admin_token_file, + &parsed_config.admin.admin_token_file, "admin.admin_token", )?; @@ -209,7 +209,7 @@ pub fn read_config(config_file: PathBuf) -> Result { fn secret_from_file( secret: &mut Option, - secret_file: &mut Option, + secret_file: &Option, name: &'static str, ) -> Result<(), Error> { match (&secret, &secret_file) {