Merge pull request 'Add allow_world_readable_secrets option to config file' (#663) from PicNoir/garage:nin/world-readable-conf-file into main-0.8.x

Reviewed-on: #663

doc/book/reference-manual/configuration.md

@@ -323,6 +323,18 @@ be obtained by running `garage node id` and then included directly in the
 key will be returned by `garage node id` and you will have to add the IP
 yourself.
 
+### `allow_world_readable_secrets`
+
+Garage checks the permissions of your secret files to make sure
+they're not world-readable. In some cases, the check might fail and
+consider your files as world-readable even if they're not. Such as
+when using Posix ACLs.
+
+Setting `allow_world_readable_secrets` to `true` bypass this
+permission verification.
+
+Alternatively, you can set the `GARAGE_ALLOW_WORLD_READABLE_SECRETS`
+environment variable to `true` to bypass the permissions check.
 
 ## The `[consul_discovery]` section
 

src/util/config.rs

@@ -38,11 +38,15 @@ pub struct Config {
 	)]
 	pub compression_level: Option<i32>,
 
+	/// 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<String>,
 	/// Optional file where RPC secret key is read from
 	pub rpc_secret_file: Option<String>,
-
 	/// 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<Config, Error> {
 		&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<String>,
 	secret_file: &Option<String>,
 	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);