diff --git a/src/controllers/cluster_probe.rs b/src/controllers/cluster_probe.rs index 2987e286e..d9f293dd2 100644 --- a/src/controllers/cluster_probe.rs +++ b/src/controllers/cluster_probe.rs @@ -134,22 +134,44 @@ pub async fn probe_nodes( /// Credential for the node's health server, resolved INSIDE the container the /// way the image resolves it: `HEALTH_API_PASSWORD` gates the mutating routes -/// (blank = open) and `HEALTH_API_USERNAME` defaults to `railway`. Leaves `$@` -/// holding curl's `-u user:pass`, or nothing when the node carries no password -/// -- an open node ignores the header and an enforcing one requires it, so one -/// command spans a cluster mid-rollout, and no secret enters the exec payload, -/// this process, or a log line. +/// (blank = open) and `HEALTH_API_USERNAME` defaults to `railway`. Stages it +/// for curl as a one-line config document in `$HEALTH_API_CFG` +/// (`user = "user:pass"`) with `$@` holding `-K -`, which makes curl read +/// that document from stdin -- or leaves both empty when the node carries no +/// password. An open node ignores the header and an enforcing one requires +/// it, so one command spans a cluster mid-rollout, and no secret enters the +/// exec payload, this process, or a log line. +/// +/// Stdin rather than `-u user:pass` because argv is public inside the +/// container: `/proc//cmdline` (`ps`) shows every process's arguments +/// to every other process in the PID namespace for as long as the request +/// runs. `printf` is a builtin of the shells the data images ship (dash, +/// bash), so the pipe spawns no process carrying the secret either. +/// +/// `curl_cfg_quote` escapes a string for a double-quoted value in curl's +/// config syntax the way curl's parser reads it back: `\` and `"` are +/// backslashed and a newline becomes `\n`, since a raw newline would end +/// the value. Everything else (`$`, `'`, `#`, whitespace) is literal inside +/// the quotes. Pure POSIX sh, so it asks nothing of the image beyond the +/// shell itself: `s` is the unread rest of the input, `c` the character in +/// hand, `r` the input after it, `q` the escaped output. Twin of the Patroni +/// prelude in [`super::patroni`]. const HEALTH_API_AUTH_PRELUDE: &str = concat!( r#"HEALTH_API_PW="${HEALTH_API_PASSWORD:-}"; "#, r#"HEALTH_API_USER="${HEALTH_API_USERNAME:-railway}"; "#, - r#"if [ -n "$HEALTH_API_PW" ]; then set -- -u "$HEALTH_API_USER:$HEALTH_API_PW"; else set --; fi; "#, + r#"curl_cfg_quote() { s=$1; q=; nl=$(printf '\nx'); nl=${nl%x}; "#, + r#"while [ -n "$s" ]; do r=${s#?}; c=${s%"$r"}; s=$r; "#, + r#"case $c in \\) q="$q\\\\";; \") q="$q\\\"";; "$nl") q="$q\\n";; *) q="$q$c";; esac; done; "#, + r#"printf '%s' "$q"; }; "#, + r#"if [ -n "$HEALTH_API_PW" ]; then HEALTH_API_CFG="user = \"$(curl_cfg_quote "$HEALTH_API_USER:$HEALTH_API_PW")\""; set -- -K -; else HEALTH_API_CFG=; set --; fi; "#, ); /// The exact shell text the switchover runs, so a test can pin both halves: -/// the credential resolution and the request itself. +/// the credential resolution and the request itself. The config document is +/// piped into curl on every run; curl reads it only when `$@` says `-K -`. fn switchover_command(endpoint: &ResolvedEndpoint) -> String { format!( - "{prelude}curl -s --max-time 8 -w '\\nHTTP_STATUS:%{{http_code}}' \"$@\" -X POST localhost:{port}{path}", + r#"{prelude}printf '%s\n' "$HEALTH_API_CFG" | curl -s --max-time 8 -w '\nHTTP_STATUS:%{{http_code}}' "$@" -X POST localhost:{port}{path}"#, prelude = HEALTH_API_AUTH_PRELUDE, port = endpoint.port, path = endpoint.path, @@ -266,14 +288,24 @@ mod tests { .unwrap() } + /// What the `curl` shim saw: its argv, one entry per element, and the + /// config document it was handed on stdin when that argv said `-K -`. + #[cfg(unix)] + struct CurlCall { + argv: Vec, + config: Option, + } + #[cfg(unix)] /// Runs the emitted switchover text through a real `sh`, with a `curl` - /// shim that prints its argv one per line, so the assertions are about + /// shim that prints its argv one per line and, when told to read a + /// config from stdin, that config too -- so the assertions are about /// what curl is actually handed rather than about string shapes. The /// command only ever runs inside a Linux container, and Windows has no - /// shell to check it against -- `switchover_command_resolves_the_credential_inside_the_container` + /// shell to check it against -- + /// `switchover_command_resolves_the_credential_inside_the_container` /// covers what can be asserted everywhere. - fn curl_argv_for(env: &[(&str, &str)]) -> Vec { + fn curl_call_for(env: &[(&str, &str)]) -> CurlCall { use std::io::Write; let dir = std::env::temp_dir().join(format!( "cli-health-api-auth-{}-{:?}", @@ -283,7 +315,19 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); std::fs::create_dir_all(&dir).unwrap(); let shim = dir.join("curl"); - std::fs::write(&shim, "#!/bin/sh\nprintf '%s\\n' \"$@\"\n").unwrap(); + std::fs::write( + &shim, + concat!( + "#!/bin/sh\n", + "printf '%s\\n' \"$@\"\n", + "prev=\n", + "for a in \"$@\"; do\n", + " if [ \"$prev\" = -K ] && [ \"$a\" = - ]; then printf '%s\\n' '@@CONFIG@@'; cat; fi\n", + " prev=$a\n", + "done\n", + ), + ) + .unwrap(); #[cfg(unix)] { use std::os::unix::fs::PermissionsExt; @@ -323,11 +367,42 @@ mod tests { "{}", String::from_utf8_lossy(&out.stderr) ); - String::from_utf8(out.stdout) - .unwrap() - .lines() - .map(str::to_string) - .collect() + let stdout = String::from_utf8(out.stdout).unwrap(); + let (argv, config) = match stdout.split_once("@@CONFIG@@\n") { + Some((argv, config)) => (argv.to_string(), Some(config.to_string())), + None => (stdout, None), + }; + CurlCall { + argv: argv.lines().map(str::to_string).collect(), + config, + } + } + + /// How curl's config parser (`unslashquote` in `src/tool_parsecfg.c`) + /// reads a double-quoted value: `\\`, `\"`, `\t`, `\n`, `\r` and `\v` + /// are escapes, a backslash before anything else is dropped, and the + /// value ends at the closing quote. Pins that the shell-side escaper + /// speaks the dialect curl actually parses. + #[cfg(unix)] + fn curl_config_value(line: &str) -> String { + let (_, quoted) = line.split_once('"').expect("a double-quoted value"); + let mut out = String::new(); + let mut chars = quoted.chars(); + while let Some(c) = chars.next() { + match c { + '"' => break, + '\\' => match chars.next() { + Some('t') => out.push('\t'), + Some('n') => out.push('\n'), + Some('r') => out.push('\r'), + Some('v') => out.push('\u{0B}'), + Some(other) => out.push(other), + None => break, + }, + c => out.push(c), + } + } + out } #[test] @@ -336,44 +411,86 @@ mod tests { assert!(cmd.starts_with(HEALTH_API_AUTH_PRELUDE)); assert!(cmd.contains("${HEALTH_API_PASSWORD:-}")); assert!(cmd.contains("${HEALTH_API_USERNAME:-railway}")); - assert!(cmd.contains(r#"set -- -u "$HEALTH_API_USER:$HEALTH_API_PW""#)); + assert!(cmd.contains( + r#"HEALTH_API_CFG="user = \"$(curl_cfg_quote "$HEALTH_API_USER:$HEALTH_API_PW")\""; set -- -K -"# + )); + assert!(cmd.contains("else HEALTH_API_CFG=; set --; fi")); + assert!( + !cmd.contains(" -u "), + "the credential must never be a curl argument" + ); + // The config document is piped into curl, ahead of the request. + assert!(cmd.contains(r#"printf '%s\n' "$HEALTH_API_CFG" | curl "#)); assert!(cmd.ends_with(r#""$@" -X POST localhost:8080/switchover"#)); } #[cfg(unix)] #[test] fn an_open_node_gets_no_credential_flag_at_all() { - let argv = curl_argv_for(&[]); - // No password in the container => no `-u` at all; `-u "railway:"` - // would be a malformed credential rather than "none". - assert!(!argv.iter().any(|a| a == "-u"), "{argv:?}"); - assert_eq!(argv.last().unwrap(), "localhost:8080/switchover"); + let call = curl_call_for(&[]); + // No password in the container => no config document and no `-K` at + // all; `user = "railway:"` would be a malformed credential rather + // than "none". + assert!(call.config.is_none(), "{:?}", call.config); + assert!( + !call.argv.iter().any(|a| a == "-K" || a == "-u"), + "{:?}", + call.argv + ); + assert_eq!(call.argv.last().unwrap(), "localhost:8080/switchover"); } #[cfg(unix)] #[test] fn a_password_alone_authenticates_as_the_default_user() { - let argv = curl_argv_for(&[("HEALTH_API_PASSWORD", "s3cret")]); - let at = argv + let call = curl_call_for(&[("HEALTH_API_PASSWORD", "s3cret")]); + assert_eq!(call.config.as_deref(), Some("user = \"railway:s3cret\"\n")); + // argv is public inside the container; the credential never rides it. + assert!( + call.argv.iter().all(|a| !a.contains("s3cret")), + "{:?}", + call.argv + ); + assert!(!call.argv.iter().any(|a| a == "-u"), "{:?}", call.argv); + // curl reads the config ahead of the request itself. + let k = call + .argv .iter() - .position(|a| a == "-u") - .expect("credential passed to curl"); - assert_eq!(argv[at + 1], "railway:s3cret"); - // curl carries the credential ahead of the request itself. - assert!(argv.iter().position(|a| a == "-X").unwrap() > at); + .position(|a| a == "-K") + .expect("curl told to read its config from stdin"); + assert_eq!(call.argv[k + 1], "-"); + assert!(call.argv.iter().position(|a| a == "-X").unwrap() > k); } #[cfg(unix)] #[test] fn an_explicit_username_wins_over_the_default() { - let argv = curl_argv_for(&[ + let call = curl_call_for(&[ ("HEALTH_API_PASSWORD", "s3cret"), ("HEALTH_API_USERNAME", "ops"), ]); - let at = argv - .iter() - .position(|a| a == "-u") - .expect("credential passed to curl"); - assert_eq!(argv[at + 1], "ops:s3cret"); + assert_eq!(call.config.as_deref(), Some("user = \"ops:s3cret\"\n")); + } + + /// A password is arbitrary text, and curl's config parser gives `\` and + /// `"` meaning inside a double-quoted value and ends the value at a + /// newline -- so the shell escapes exactly those, and what curl reads + /// back (`curl_config_value`, its parser's rules) is the original. + #[cfg(unix)] + #[test] + fn the_credential_is_escaped_for_curls_config_parser() { + let password = "p\"a\\s$s' w#rd\nnext\ttab"; + let call = curl_call_for(&[("HEALTH_API_PASSWORD", password)]); + let config = call.config.expect("config document piped to curl"); + assert_eq!( + config, + "user = \"railway:p\\\"a\\\\s$s' w#rd\\nnext\ttab\"\n" + ); + assert_eq!(curl_config_value(&config), format!("railway:{password}")); + assert!( + call.argv.iter().all(|a| !a.contains("w#rd")), + "{:?}", + call.argv + ); } }