Merge pull request 'Secrets can be passed directly in config, as file, or as env' (#499) from config-files-env into main

Reviewed-on: https://git.deuxfleurs.fr/Deuxfleurs/garage/pulls/499

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
+Garage uses a secret key, called an RPC secret, that is shared between all
-in order to identify these nodes and allow them to communicate together.
+nodes of the cluster in order to identify these nodes and allow them to
-This key should be specified here in the form of a 32-byte hex-encoded
+communicate together. The RPC secret is a 32-byte hex-encoded random string,
-random string. Such a string can be generated with a command
+which can be generated with a command such as `openssl rand -hex 32`.
-such as `openssl rand -hex 32`.
 
-### `rpc_secret_file`
+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
-Like `rpc_secret` above, just that this is the path to a file that Garage will try to read the secret from.
+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 token for accessing the Metrics endpoint. If this token is not set, the
-the config file, the Metrics endpoint can be accessed without access
+Metrics endpoint can be accessed without access control.
-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 `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)
 
 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
+token is not set, access to these endpoints is disabled entirely.
-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 `v0.7.2`.
+`admin_token_file` and the `GARAGE_ADMIN_TOKEN` environment variable are supported since Garage `v0.8.2`.
+
+
 ### `trace_sink`
 
 Optionally, the address of an OpenTelemetry collector.  If specified,

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<String>,
 
-	/// RPC secret network key for admin operations
+	#[structopt(flatten)]
-	#[structopt(short = "s", long = "rpc-secret", env = "GARAGE_RPC_SECRET")]
+	pub secrets: Secrets,
-	pub rpc_secret: Option<String>,
 
-	/// Configuration file (garage.toml)
+	/// Path to configuration file
 	#[structopt(
 		short = "c",
 		long = "config",
@@ -63,6 +63,24 @@ 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<String>,
+
+	/// 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<String>,
+
+	/// 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<String>,
+}
+
 #[tokio::main]
 async fn main() {
 	// Initialize version and features info
@@ -145,9 +163,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 +180,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 +189,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 +249,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
+}

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)?;

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<bool>) {
 	while !*chan.borrow() {
@@ -26,9 +27,9 @@ async fn wait_from(mut chan: watch::Receiver<bool>) {
 	}
 }
 
-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 ----
 

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(
+		let network_key = hex::decode(config.rpc_secret.as_ref().ok_or_message(
-			&hex::decode(config.rpc_secret.as_ref().unwrap()).expect("Invalid RPC secret key")[..],
+			"rpc_secret value is missing, not present in config file or in environment",
-		)
+		)?)
-		.expect("Invalid RPC secret key");
+		.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)?;

src/util/config.rs

@@ -34,9 +34,7 @@ pub struct Config {
 	pub compression_level: Option<i32>,
 
 	/// RPC secret key: 32 bytes hex encoded
-	/// Note: When using `read_config` this should never be `None`
 	pub rpc_secret: Option<String>,
-
 	/// Optional file where RPC secret key is read from
 	pub rpc_secret_file: Option<String>,
 
@@ -122,10 +120,17 @@ pub struct WebConfig {
 pub struct AdminConfig {
 	/// Address and port to bind for admin API serving
 	pub api_bind_addr: Option<SocketAddr>,
+
 	/// Bearer token to use to scrape metrics
 	pub metrics_token: Option<String>,
+	/// File to read metrics token from
+	pub metrics_token_file: Option<String>,
+
 	/// Bearer token to use to access Admin API endpoints
 	pub admin_token: Option<String>,
+	/// File to read admin token from
+	pub admin_token_file: Option<String>,
+
 	/// OTLP server to where to export traces
 	pub trace_sink: Option<String>,
 }
@@ -183,29 +188,55 @@ pub fn read_config(config_file: PathBuf) -> Result<Config, Error> {
 
 	let mut parsed_config: Config = toml::from_str(&config)?;
 
-	match (&parsed_config.rpc_secret, &parsed_config.rpc_secret_file) {
+	secret_from_file(
-		(Some(_), None) => {
+		&mut parsed_config.rpc_secret,
+		&parsed_config.rpc_secret_file,
+		"rpc_secret",
+	)?;
+	secret_from_file(
+		&mut parsed_config.admin.metrics_token,
+		&parsed_config.admin.metrics_token_file,
+		"admin.metrics_token",
+	)?;
+	secret_from_file(
+		&mut parsed_config.admin.admin_token,
+		&parsed_config.admin.admin_token_file,
+		"admin.admin_token",
+	)?;
+
+	Ok(parsed_config)
+}
+
+fn secret_from_file(
+	secret: &mut Option<String>,
+	secret_file: &Option<String>,
+	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)) => {
+		(None, Some(file_path)) => {
-			let mut rpc_secret_file = std::fs::OpenOptions::new()
+			#[cfg(unix)]
-				.read(true)
+			if std::env::var("GARAGE_ALLOW_WORLD_READABLE_SECRETS").as_deref() != Ok("true") {
-				.open(rpc_secret_file_path_string)?;
+				use std::os::unix::fs::MetadataExt;
-			let mut rpc_secret_from_file = String::new();
+				let metadata = std::fs::metadata(&file_path)?;
-			rpc_secret_file.read_to_string(&mut rpc_secret_from_file)?;
+				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(())
-		}
-	};
-
-	Ok(parsed_config)
 }
 
 fn default_compression() -> Option<i32> {
@@ -269,31 +300,7 @@ mod tests {
 	use std::io::Write;
 
 	#[test]
-	fn test_rpc_secret_is_required() -> Result<(), Error> {
+	fn test_rpc_secret() -> 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);
-
 		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);