mirror of
https://git.deuxfleurs.fr/Deuxfleurs/garage.git
synced 2024-11-22 08:01:02 +00:00
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 <flokli@flokli.de>
This commit is contained in:
parent
4b3dee2ca3
commit
f83fa02193
2 changed files with 89 additions and 5 deletions
|
@ -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
|
key will be returned by `garage node id` and you will have to add the IP
|
||||||
yourself.
|
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
|
## The `[consul_discovery]` section
|
||||||
|
|
||||||
|
|
|
@ -38,11 +38,15 @@ pub struct Config {
|
||||||
)]
|
)]
|
||||||
pub compression_level: Option<i32>,
|
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
|
/// RPC secret key: 32 bytes hex encoded
|
||||||
pub rpc_secret: Option<String>,
|
pub rpc_secret: Option<String>,
|
||||||
/// Optional file where RPC secret key is read from
|
/// Optional file where RPC secret key is read from
|
||||||
pub rpc_secret_file: Option<String>,
|
pub rpc_secret_file: Option<String>,
|
||||||
|
|
||||||
/// Address to bind for RPC
|
/// Address to bind for RPC
|
||||||
pub rpc_bind_addr: SocketAddr,
|
pub rpc_bind_addr: SocketAddr,
|
||||||
/// Public IP address of this node
|
/// 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,
|
&mut parsed_config.rpc_secret,
|
||||||
&parsed_config.rpc_secret_file,
|
&parsed_config.rpc_secret_file,
|
||||||
"rpc_secret",
|
"rpc_secret",
|
||||||
|
parsed_config.allow_world_readable_secrets,
|
||||||
)?;
|
)?;
|
||||||
secret_from_file(
|
secret_from_file(
|
||||||
&mut parsed_config.admin.metrics_token,
|
&mut parsed_config.admin.metrics_token,
|
||||||
&parsed_config.admin.metrics_token_file,
|
&parsed_config.admin.metrics_token_file,
|
||||||
"admin.metrics_token",
|
"admin.metrics_token",
|
||||||
|
parsed_config.allow_world_readable_secrets,
|
||||||
)?;
|
)?;
|
||||||
secret_from_file(
|
secret_from_file(
|
||||||
&mut parsed_config.admin.admin_token,
|
&mut parsed_config.admin.admin_token,
|
||||||
&parsed_config.admin.admin_token_file,
|
&parsed_config.admin.admin_token_file,
|
||||||
"admin.admin_token",
|
"admin.admin_token",
|
||||||
|
parsed_config.allow_world_readable_secrets,
|
||||||
)?;
|
)?;
|
||||||
|
|
||||||
Ok(parsed_config)
|
Ok(parsed_config)
|
||||||
|
@ -260,6 +267,7 @@ fn secret_from_file(
|
||||||
secret: &mut Option<String>,
|
secret: &mut Option<String>,
|
||||||
secret_file: &Option<String>,
|
secret_file: &Option<String>,
|
||||||
name: &'static str,
|
name: &'static str,
|
||||||
|
conf_allow_world_readable: bool,
|
||||||
) -> Result<(), Error> {
|
) -> Result<(), Error> {
|
||||||
match (&secret, &secret_file) {
|
match (&secret, &secret_file) {
|
||||||
(_, None) => {
|
(_, None) => {
|
||||||
|
@ -268,7 +276,37 @@ fn secret_from_file(
|
||||||
(Some(_), Some(_)) => {
|
(Some(_), Some(_)) => {
|
||||||
return Err(format!("only one of `{}` and `{}_file` can be set", name, name).into());
|
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(())
|
Ok(())
|
||||||
}
|
}
|
||||||
|
@ -427,11 +465,34 @@ mod tests {
|
||||||
"#,
|
"#,
|
||||||
path_secret_path.display()
|
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)]
|
#[cfg(unix)]
|
||||||
{
|
{
|
||||||
|
// Check non world-readable secrets config
|
||||||
use std::os::unix::fs::PermissionsExt;
|
use std::os::unix::fs::PermissionsExt;
|
||||||
let metadata = std::fs::metadata(&path_secret_path)?;
|
let metadata = std::fs::metadata(&path_secret_path)?;
|
||||||
let mut perm = metadata.permissions();
|
let mut perm = metadata.permissions();
|
||||||
|
@ -443,8 +504,19 @@ mod tests {
|
||||||
|
|
||||||
std::env::set_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS", "true");
|
std::env::set_var("GARAGE_ALLOW_WORLD_READABLE_SECRETS", "true");
|
||||||
assert!(super::read_config(path_config.to_path_buf()).is_ok());
|
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_config);
|
||||||
drop(path_secret);
|
drop(path_secret);
|
||||||
drop(file_config);
|
drop(file_config);
|
||||||
|
|
Loading…
Reference in a new issue