diff --git a/src/config.rs b/src/config.rs index 5b07b168..32792487 100644 --- a/src/config.rs +++ b/src/config.rs @@ -17,6 +17,7 @@ use nextcloud_config_parser::{ }; use redis::{ConnectionAddr, ConnectionInfo}; use sqlx::any::AnyConnectOptions; +use sqlx::ConnectOptions; use std::convert::{TryFrom, TryInto}; use std::env::var; use std::fmt::{Debug, Display, Formatter}; @@ -24,6 +25,19 @@ use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, TcpListener}; use std::path::{Path, PathBuf}; use std::str::FromStr; +const REDACTED: &str = "REDACTED"; + +/// Parses a url given on the command line. +/// +/// Both url options are taken as plain strings so that clap never has to format +/// a parse error: it puts the raw argument into its own message, password and all. +fn parse_url_option(option: &'static str, url: &str) -> Result +where + T::Err: std::error::Error + Send + Sync + 'static, +{ + T::from_str(url).map_err(|e| ConfigError::UrlOption(option, Box::new(e))) +} + fn styles() -> Styles { Styles::styled() .header(AnsiColor::Yellow.on_default() | Effects::BOLD) @@ -32,15 +46,15 @@ fn styles() -> Styles { .placeholder(AnsiColor::Green.on_default()) } -#[derive(Parser, Debug)] +#[derive(Parser)] #[command(name = "notify_push", styles = styles())] pub struct Opt { /// The database connect url #[clap(long)] - pub database_url: Option, + pub database_url: Option, /// The redis connect url #[clap(long)] - pub redis_url: Vec, + pub redis_url: Vec, /// The client certificate to use when connecting to redis over TLS #[clap(long)] pub redis_tls_cert: Option, @@ -118,7 +132,6 @@ pub struct Opt { pub max_connection_time: Option, } -#[derive(Debug)] pub struct Config { pub database: AnyConnectOptions, pub database_prefix: String, @@ -135,6 +148,66 @@ pub struct Config { pub max_connection_time: usize, } +/// Formats a database url without its password +struct RedactedDatabase<'a>(&'a AnyConnectOptions); + +impl Debug for RedactedDatabase<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + let mut url = self.0.to_url_lossy(); + if url.password().is_some() && url.set_password(Some(REDACTED)).is_err() { + // never fall through to printing the url we failed to redact + return f.write_str(REDACTED); + } + Display::fmt(&url, f) + } +} + +/// Formats a redis config without its password +struct RedactedRedis<'a>(&'a RedisConfig); + +impl Debug for RedactedRedis<'_> { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + match self.0 { + RedisConfig::Single(single) => f + .debug_struct("Single") + .field("addr", &single.addr) + .field("db", &single.db) + .field("username", &single.username) + .field("password", &single.password.as_ref().map(|_| REDACTED)) + .field("tls_params", &single.tls_params) + .finish(), + RedisConfig::Cluster(cluster) => f + .debug_struct("Cluster") + .field("addr", &cluster.addr) + .field("db", &cluster.db) + .field("username", &cluster.username) + .field("password", &cluster.password.as_ref().map(|_| REDACTED)) + .field("tls_params", &cluster.tls_params) + .finish(), + } + } +} + +impl Debug for Config { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Config") + .field("database", &RedactedDatabase(&self.database)) + .field("database_prefix", &self.database_prefix) + .field("redis", &RedactedRedis(&self.redis)) + .field("nextcloud_url", &self.nextcloud_url) + .field("metrics_bind", &self.metrics_bind) + .field("log_level", &self.log_level) + .field("bind", &self.bind) + .field("allow_self_signed", &self.allow_self_signed) + .field("user_agent", &self.user_agent) + .field("no_ansi", &self.no_ansi) + .field("tls", &self.tls) + .field("max_debounce_time", &self.max_debounce_time) + .field("max_connection_time", &self.max_connection_time) + .finish() + } +} + #[derive(Debug, Clone)] pub struct TlsConfig { pub key: PathBuf, @@ -247,13 +320,13 @@ impl Config { .transpose()? .unwrap_or_default(); let from_env = PartialConfig::from_env()?; - let from_opt = PartialConfig::from_opt(opt); + let from_opt = PartialConfig::from_opt(opt)?; from_opt.merge(from_env).merge(from_config).try_into() } } -#[derive(Debug, Default)] +#[derive(Default)] struct PartialConfig { pub database: Option, pub database_prefix: Option, @@ -359,17 +432,23 @@ impl PartialConfig { Ok(parse_config_file(file, glob)?) } - fn from_opt(opt: Opt) -> Self { + fn from_opt(opt: Opt) -> Result { let tls = if let (Some(cert), Some(key)) = (opt.tls_cert, opt.tls_key) { Some(TlsConfig { cert, key }) } else { None }; - let redis = match opt.redis_url.len() { + let redis_url = opt + .redis_url + .iter() + .map(|url| parse_url_option::("redis-url", url)) + .collect::, _>>()?; + + let redis = match redis_url.len() { 0 => None, 1 => { - let redis = opt.redis_url.into_iter().next().unwrap(); + let redis = redis_url.into_iter().next().unwrap(); let addr = map_redis_addr(redis.addr().clone()); let redis_tls = matches!(addr, RedisConnectionAddr::Tcp { tls: true, .. }); @@ -391,8 +470,7 @@ impl PartialConfig { })) } _ => { - let addr: Vec<_> = opt - .redis_url + let addr: Vec<_> = redis_url .iter() .map(|redis| map_redis_addr(redis.addr().clone())) .collect(); @@ -410,7 +488,7 @@ impl PartialConfig { insecure: opt.redis_tls_insecure, }); - let redis = opt.redis_url.into_iter().next().unwrap(); + let redis = redis_url.into_iter().next().unwrap(); let redis = redis.redis_settings(); Some(RedisConfig::Cluster(RedisClusterConnectionInfo { addr, @@ -422,8 +500,12 @@ impl PartialConfig { } }; - PartialConfig { - database: opt.database_url, + Ok(PartialConfig { + database: opt + .database_url + .as_deref() + .map(|url| parse_url_option("database-url", url)) + .transpose()?, database_prefix: opt.database_prefix, redis, nextcloud_url: opt.nextcloud_url, @@ -444,7 +526,7 @@ impl PartialConfig { tls, max_debounce_time: opt.max_debounce_time, max_connection_time: opt.max_connection_time, - } + }) } fn merge(self, fallback: Self) -> Self { @@ -502,3 +584,94 @@ fn map_redis_addr(addr: ConnectionAddr) -> RedisConnectionAddr { _ => unreachable!("unknown redis address"), } } + +#[cfg(test)] +mod tests { + use super::*; + + fn test_config(database: &str, redis_password: Option<&str>) -> Config { + sqlx::any::install_default_drivers(); + Config { + database: database.parse().unwrap(), + database_prefix: "oc_".into(), + redis: RedisConfig::Single(RedisConnectionInfo { + addr: RedisConnectionAddr::Tcp { + host: "127.0.0.1".into(), + port: 6379, + tls: false, + }, + db: 0, + username: Some("redis_user".into()), + password: redis_password.map(String::from), + tls_params: None, + }), + nextcloud_url: "https://cloud.example.com/".into(), + metrics_bind: None, + log_level: "warn".into(), + bind: Bind::Tcp(([127, 0, 0, 1], 7867).into()), + allow_self_signed: false, + user_agent: None, + no_ansi: false, + tls: None, + max_debounce_time: 15, + max_connection_time: 0, + } + } + + /// Renders an error together with its whole source chain + fn full_error(err: &dyn std::error::Error) -> String { + let mut rendered = err.to_string(); + let mut source = err.source(); + while let Some(inner) = source { + rendered.push_str(" / "); + rendered.push_str(&inner.to_string()); + source = inner.source(); + } + rendered + } + + #[test] + fn test_malformed_urls_are_rejected_without_echoing_them() { + let opt = Opt::try_parse_from([ + "notify_push", + "--database-url", + "postgres://ncuser:db-hunter2@host:not_a_port/nextcloud", + "--redis-url", + "redis://redisuser:redis-hunter2@host:not_a_port/0", + ]) + .expect("clap has to accept the raw value and leave validation to us"); + + let err = Config::from_opt(opt).expect_err("a malformed url still has to be rejected"); + let rendered = full_error(&err); + assert!(!rendered.contains("db-hunter2"), "{rendered}"); + assert!(!rendered.contains("redis-hunter2"), "{rendered}"); + } + + #[test] + fn test_config_debug_does_not_leak_passwords() { + // `--dump-config` and `log::trace!` both format the whole config + let config = test_config( + "postgres://nextcloud:db-hunter2@localhost/nextcloud", + Some("redis-hunter2"), + ); + for formatted in [format!("{config:?}"), format!("{config:#?}")] { + assert!( + !formatted.contains("db-hunter2"), + "database password leaked into config debug output: {formatted}" + ); + assert!( + !formatted.contains("redis-hunter2"), + "redis password leaked into config debug output: {formatted}" + ); + } + } + + #[test] + fn test_config_debug_keeps_non_secret_details() { + let config = test_config("postgres://nextcloud@localhost/nextcloud", None); + let formatted = format!("{config:#?}"); + assert!(formatted.contains("localhost"), "{formatted}"); + assert!(formatted.contains("cloud.example.com"), "{formatted}"); + assert!(formatted.contains("redis_user"), "{formatted}"); + } +} diff --git a/src/error.rs b/src/error.rs index b4364e45..937e2e09 100644 --- a/src/error.rs +++ b/src/error.rs @@ -100,6 +100,11 @@ pub enum ConfigError { #[error("Error while parsing nextcloud config.php")] #[diagnostic(transparent)] Parse(#[from] nextcloud_config_parser::Error), + #[error("Invalid --{0} value")] + UrlOption( + &'static str, + #[source] Box, + ), #[error("Invalid {0} environment variable")] Env( &'static str,