feat(pairing-cli): source --nsec - reads the key from stdin, never argv - #6849
feat(pairing-cli): source --nsec - reads the key from stdin, never argv#6849lodar wants to merge 1 commit into
source --nsec - reads the key from stdin, never argv#6849Conversation
…argv
`buzz-pair source` transfers a real secret key. Today its only form is
`--nsec <bech32>`: an argv element, and argv is world-readable in
`/proc/<pid>/cmdline` for the entire life of the process — which for a `source`
session is up to the 120s it spends waiting for the target to show up. Any
local user, and anything that snapshots the process table, gets the key.
`--nsec -` takes the key from the first line of stdin instead. It is read before
any session I/O and through the same buffered `io::stdin()` handle the SAS
prompt already uses, a line at a time, so the interactive y/n answer is simply
the next line and the existing flow is unchanged:
{ printf '%s\n' "$nsec"; cat; } | buzz-pair source --nsec - --relay wss://...
`--nsec <bech32>` keeps working exactly as before; this only adds the `-` form.
The clap help text names both `-` and the word stdin so a caller can probe
`source --help` and refuse to hand a key to a build that does not support it.
Signed-off-by: lodar <markounik@gmail.com>
Chessing234
left a comment
There was a problem hiding this comment.
the threat here is real and the fix is the right shape — a source session waits up to 120s for the target, and argv is readable from /proc/<pid>/cmdline for that whole window. i checked the stdin claim too: read_line goes through io::stdin(), which is the process-global buffered handle, and read_yes_no calls the same function, so the SAS answer really is just the next line. the doc example works.
the gap is that the key still ends up in memory the crate doesn't zeroize.
fn read_line() -> Result<String, CliError> {
let stdin = io::stdin();
let mut line = String::new();
stdin.lock().read_line(&mut line)?;
Ok(line.trim_end_matches('\n').trim_end_matches('\r').to_string())
}Zeroizing::new(read_line()?) wraps only the value that comes back. by then the nsec exists in at least two other places that are dropped without wiping: line itself, and — because read_line grows the String as it reads — any earlier buffer it reallocated away from. on top of that Stdin's internal BufReader holds the raw bytes it read (its buffer is 8 KiB and lives for the process), so the nsec sits in that buffer until enough later input overwrites it, which for this tool is one short y.
a crate that already reaches for Zeroizing and SecretKey::parse(s.as_str()) is clearly trying to get this right, so it's worth closing: read into a Zeroizing<String> directly and zero the tail explicitly rather than round-tripping through to_string():
fn read_line_secret() -> Result<Zeroizing<String>, CliError> {
let mut line = Zeroizing::new(String::with_capacity(128));
io::stdin().lock().read_line(&mut line)?;
let end = line.trim_end_matches(['\n', '\r']).len();
line.truncate(end); // truncate leaves the tail bytes, but Zeroizing wipes the whole buffer on drop
Ok(line)
}with_capacity up front matters — without it the realloc during read leaves a copy behind that nothing owns. leave read_line as-is for the y/n prompt and use the secret variant only for --nsec -.
two smaller ones:
an interactive --nsec - echoes the key. if someone runs it without a pipe (which the flag invites — it's the obvious thing to try), the terminal echoes the nsec as they paste it and it lands in the scrollback. worth either detecting a tty and reading with echo off, or refusing a tty with a message pointing at the pipe form from the doc comment.
empty stdin gives a confusing error. read_line on a closed stdin returns Ok(""), which then fails as InvalidNsec("...") — the user reads that as "my key is malformed" when the real problem is that nothing arrived. a if s.is_empty() { return Err(...) } with its own message would save a debugging session.
Summary
buzz-pair sourcetransfers a real secret key, and today its only form is--nsec <bech32>— an argv element. argv is world-readable in/proc/<pid>/cmdlinefor the entire life of the process, which for asourcesession is up to the 120s it spends waiting for the target to appear. Any local user, and anything that snapshots the process table, gets the key.--nsec -takes the key from the first line of stdin instead. It is read before any session I/O and through the same bufferedio::stdin()handle the SAS prompt already uses, a line at a time, so the interactive y/n answer is simply the next line and the existing flow is unchanged:--nsec <bech32>keeps working exactly as before; this only adds the-form. The clap help text names both-and the word stdin, so a caller can probesource --helpand refuse to hand a key to a build that does not support it.Related issue
None found — I searched open issues and PRs for stdin/argv key handling in the pairing CLI and turned up nothing. The closest neighbour is #2610 (
test(pairing-cli): add unit coverage for buzz-pairing-cli), which is adjacent rather than overlapping.Note for whoever triages: this touches the same
resolve_payloadfunction as my other pairing-cli PR (the HTTPS pairing fix). They are independent and either can be taken alone — whichever lands second takes a small textual conflict, and I will rebase it.Testing
--nsec -pairs normally and the SAS prompt still reads the next stdin line;--nsec <bech32>is byte-for-byte the previous behaviour;ps//proc/<pid>/cmdlineduring a livesourcesession holds the literal-and no key material.rustfmt --checkclean; lint, unit tests, the Windows build and the cross-compile matrix ran green on this exact tree in my own CI before opening.io::stdin()handle, so covering it means either injecting aBufReadintoresolve_payload— a refactor that would collide with the other pairing-cli PR — or driving the built binary from an integration test. Say which you would prefer and I will add it here.No UI surface is touched.
The branch is based on
f88cda9eb, which is behindmain;crates/buzz-pairing-cli/src/main.rshas not been touched upstream since, so it applies cleanly.