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/util') 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 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/util') 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/util/config.rs | 220 ++--------------------------------------------------- 1 file changed, 7 insertions(+), 213 deletions(-) (limited to 'src/util') 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 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/util/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/util') 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" -- cgit v1.2.3 From ee57dd922b9c396298473b41e4046c8d00ee77d5 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Tue, 16 Jan 2024 16:28:17 +0100 Subject: Bump version to 0.9.1 --- src/util/Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/util') diff --git a/src/util/Cargo.toml b/src/util/Cargo.toml index 6554ac13..85317ec1 100644 --- a/src/util/Cargo.toml +++ b/src/util/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "garage_util" -version = "0.9.0" +version = "0.9.1" authors = ["Alex Auvolat "] edition = "2018" license = "AGPL-3.0" -- cgit v1.2.3 From fe1af5d98b8e6c87f39f1d94586e61bff100e7d2 Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 5 Feb 2024 13:02:02 +0100 Subject: [dep-upgrade-202402] refactor dependencies: move all as workspace deps --- src/util/Cargo.toml | 64 ++++++++++++++++++++++++++--------------------------- 1 file changed, 32 insertions(+), 32 deletions(-) (limited to 'src/util') diff --git a/src/util/Cargo.toml b/src/util/Cargo.toml index 85317ec1..f4e53e1a 100644 --- a/src/util/Cargo.toml +++ b/src/util/Cargo.toml @@ -16,42 +16,42 @@ path = "lib.rs" [dependencies] garage_db.workspace = true -arc-swap = "1.0" -async-trait = "0.1" -blake2 = "0.10" -bytes = "1.0" -bytesize = "1.2" -digest = "0.10" -err-derive = "0.3" -hexdump = "0.1" -xxhash-rust = { version = "0.8", default-features = false, features = ["xxh3"] } -hex = "0.4" -lazy_static = "1.4" -tracing = "0.1" -rand = "0.8" -sha2 = "0.10" - -chrono = "0.4" -rmp-serde = "1.1.2" -serde = { version = "1.0", default-features = false, features = ["derive", "rc"] } -serde_json = "1.0" -toml = "0.6" - -futures = "0.3" -tokio = { version = "1.0", default-features = false, features = ["rt", "rt-multi-thread", "io-util", "net", "time", "macros", "sync", "signal", "fs"] } - -netapp = "0.10" - -http = "0.2" -hyper = "0.14" - -opentelemetry = { version = "0.17", features = [ "rt-tokio", "metrics", "trace" ] } +arc-swap.workspace = true +async-trait.workspace = true +blake2.workspace = true +bytes.workspace = true +bytesize.workspace = true +digest.workspace = true +err-derive.workspace = true +hexdump.workspace = true +xxhash-rust.workspace = true +hex.workspace = true +lazy_static.workspace = true +tracing.workspace = true +rand.workspace = true +sha2.workspace = true + +chrono.workspace = true +rmp-serde.workspace = true +serde.workspace = true +serde_json.workspace = true +toml.workspace = true + +futures.workspace = true +tokio.workspace = true + +netapp.workspace = true + +http.workspace = true +hyper.workspace = true + +opentelemetry.workspace = true [build-dependencies] -rustc_version = "0.4.0" +rustc_version.workspace = true [dev-dependencies] -mktemp = "0.5" +mktemp.workspace = true [features] k2v = [] -- cgit v1.2.3 From bf283c99240787c7ffeba8ff3222283ee9fc61aa Mon Sep 17 00:00:00 2001 From: Alex Auvolat Date: Mon, 12 Feb 2024 11:35:57 +0100 Subject: [fix-secrets-695] config: replace String by PathBuf for *_file --- src/util/config.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/util') diff --git a/src/util/config.rs b/src/util/config.rs index 65c0b5c0..a9a72110 100644 --- a/src/util/config.rs +++ b/src/util/config.rs @@ -52,7 +52,7 @@ pub struct Config { /// 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, + pub rpc_secret_file: Option, /// Address to bind for RPC pub rpc_bind_addr: SocketAddr, /// Public IP address of this node @@ -166,12 +166,12 @@ pub struct AdminConfig { /// Bearer token to use to scrape metrics pub metrics_token: Option, /// File to read metrics token from - pub metrics_token_file: Option, + 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, + pub admin_token_file: Option, /// OTLP server to where to export traces pub trace_sink: Option, -- cgit v1.2.3