Support reading PWS passwords from stdin

Specifying passwords as command-line arguments can be problematic: The
user has to be careful that the command is not recorded in the shell
history. Also, the password might be exposed over the process list.

This patch adds an alternative input method for the pws add and pws
update methods: reading the password from stdin. This is activated by
setting the password to "-". For the tests, we use a string as a
simulated stdin buffer.

CHANGELOG.md

@@ -9,6 +9,7 @@ Unreleased
 - Removed the `pws set` subcommand
 - Added the `--only-aes-key` option to the `reset` command to build a new AES
  key without performing a factory reset
+- Added support for reading PWS passwords from stdin
 - Added `NITROCLI_RESOLVED_USB_PATH` environment variable to be used by
  extensions
 - Allowed entering of `base32` encoded strings containing spaces

doc/nitrocli.1

@@ -274,23 +274,29 @@ Use the \fB\-\-quiet\fR option to suppress the labels and to only output the
 values stored in the PWS slot.
 .TP
 \fBnitrocli pws add \fR[\fB\-s\fR|\fB\-\-slot \fIslot\fR] \
-\fIname login password\fR
+\fIname login password\fR|\fB-\fR
 Add a new PWS slot.
 If the \fB\-\-slot\fR option is set, this command writes the data to the given
 slot and fails if the slot is already programmed.
 If the \fB\-\-slot\fR option is not set, this command locates the first free
 PWS slot and sets its content to the given values.
 It fails if all PWS slots are programmed.
+
+If \fIpassword\fR is set to \fB-\fR, the password is read from the standard
+input.
 .TP
 \fBnitrocli pws update \fIslot\fR \
 [\fB\-n\fR|\fB\-\-name \fIname\fR] \
 [\fB\-l\fR|\fB\-\-login \fIlogin\fR] \
-[\fB\-p\fR|\fB\-\-password \fIpassword\fR]
+[\fB\-p\fR|\fB\-\-password \fIpassword\fR|\fB-\fR]
 Update the content of a programmed PWS slot.
 \fIslot\fR is the number of the slot to write.
 This command only sets the data given with the \fB\-\-name\fR, \fB\-\-login\fR,
 and \fB\-\-password\fR options and does not overwrite the other fields of the
 slot.
+
+If \fIpassword\fR is set to \fB-\fR, the password is read from the standard
+input.
 .TP
 \fBnitrocli pws clear \fIslot\fR
 Delete the data stored in a PWS slot.

src/commands.rs

@@ -378,6 +378,20 @@ fn print_storage_status(
  Ok(())
 }
 
+/// Read a value from stdin if the given string is set to "-".
+fn value_or_stdin<'s>(ctx: &mut Context<'_>, s: &'s str) -> anyhow::Result<borrow::Cow<'s, str>> {
+ if s == "-" {
+ let mut s = String::new();
+ let _ = ctx
+ .stdin
+ .read_to_string(&mut s)
+ .context("Failed to read from stdin")?;
+ Ok(borrow::Cow::from(s))
+ } else {
+ Ok(borrow::Cow::from(s))
+ }
+}
+
 /// Query and pretty print the status that is common to all Nitrokey devices.
 fn print_status(ctx: &mut Context<'_>, device: &nitrokey::DeviceWrapper<'_>) -> anyhow::Result<()> {
  let serial_number = device
@@ -1040,6 +1054,7 @@ pub fn pws_add(
  password: &str,
  slot_idx: Option<u8>,
 ) -> anyhow::Result<()> {
+ let password = value_or_stdin(ctx, password)?;
  with_password_safe(ctx, |ctx, mut pws| {
  let slots = pws.get_slots()?;
 
@@ -1070,7 +1085,7 @@ pub fn pws_add(
  }?;
 
  pws
- .write_slot(slot_idx, name, login, password)
+ .write_slot(slot_idx, name, login, password.as_ref())
  .context("Failed to write PWS slot")?;
  println!(ctx, "Added PWS slot {}", slot_idx)?;
  Ok(())
@@ -1088,6 +1103,9 @@ pub fn pws_update(
  if name.is_none() && login.is_none() && password.is_none() {
  anyhow::bail!("You have to set at least one of --name, --login, or --password");
  }
+
+ let password = password.map(|s| value_or_stdin(ctx, s)).transpose()?;
+
  with_password_safe(ctx, |_ctx, mut pws| {
  let slot = pws.get_slot(slot_idx).context("Failed to query PWS slot")?;
  let name = name
@@ -1099,7 +1117,8 @@ pub fn pws_update(
  .unwrap_or_else(|| slot.get_login().map(borrow::Cow::from))
  .context("Failed to query current slot login")?;
  let password = password
- .map(|s| Ok(borrow::Cow::from(s)))
+ .as_ref()
+ .map(|s| Ok(borrow::Cow::from(s.as_ref())))
  .unwrap_or_else(|| slot.get_password().map(borrow::Cow::from))
  .context("Failed to query current slot password")?;
  pws

src/main.rs

@@ -173,6 +173,8 @@ fn get_version_string() -> String {
 /// The context used when running the program.
 #[allow(missing_debug_implementations)]
 pub struct Context<'io> {
+ /// The `Read` object used as standard input throughout the program.
+ pub stdin: &'io mut dyn io::Read,
  /// The `Write` object used as standard output throughout the program.
  pub stdout: &'io mut dyn io::Write,
  /// The `Write` object used as standard error throughout the program.
@@ -201,17 +203,20 @@ pub struct Context<'io> {
 }
 
 impl<'io> Context<'io> {
- fn from_env<O, E>(
+ fn from_env<I, O, E>(
+ stdin: &'io mut I,
  stdout: &'io mut O,
  stderr: &'io mut E,
  is_tty: bool,
  config: config::Config,
  ) -> Context<'io>
  where
+ I: io::Read,
  O: io::Write,
  E: io::Write,
  {
  Context {
+ stdin,
  stdout,
  stderr,
  is_tty,
@@ -247,14 +252,15 @@ fn run<'ctx, 'io: 'ctx>(ctx: &'ctx mut Context<'io>, args: Vec<String>) -> i32 {
 fn main() {
  use std::io::Write;
 
+ let mut stdin = io::stdin();
  let mut stdout = io::stdout();
  let mut stderr = io::stderr();
 
  let rc = match config::Config::load() {
  Ok(config) => {
  let is_tty = termion::is_tty(&stdout);
  let args = env::args().collect::<Vec<_>>();
- let ctx = &mut Context::from_env(&mut stdout, &mut stderr, is_tty, config);
+ let ctx = &mut Context::from_env(&mut stdin, &mut stdout, &mut stderr, is_tty, config);
 
  run(ctx, args)
  }

src/tests/mod.rs

@@ -23,6 +23,7 @@ mod status;
 mod unencrypted;
 
 struct Nitrocli {
+ stdin: String,
  model: Option<nitrokey::Model>,
  path: Option<ffi::OsString>,
  admin_pin: Option<ffi::OsString>,
@@ -35,6 +36,7 @@ struct Nitrocli {
 impl Nitrocli {
  pub fn new() -> Self {
  Self {
+ stdin: String::new(),
  model: None,
  path: None,
  admin_pin: Some(nitrokey::DEFAULT_ADMIN_PIN.into()),
@@ -63,6 +65,11 @@ impl Nitrocli {
  self
  }
 
+ pub fn stdin(mut self, stdin: impl Into<String>) -> Self {
+ self.stdin = stdin.into();
+ self
+ }
+
  pub fn admin_pin(mut self, pin: impl Into<ffi::OsString>) -> Self {
  self.admin_pin = Some(pin.into());
  self
@@ -104,10 +111,12 @@ impl Nitrocli {
  .map(ToOwned::to_owned)
  .collect();
 
+ let mut stdin = self.stdin.as_bytes();
  let mut stdout = Vec::new();
  let mut stderr = Vec::new();
 
  let ctx = &mut crate::Context {
+ stdin: &mut stdin,
  stdout: &mut stdout,
  stderr: &mut stderr,
  is_tty: false,

src/tests/pws.rs

@@ -240,6 +240,22 @@ fn update(model: nitrokey::Model) -> anyhow::Result<()> {
  Ok(())
 }
 
+#[test_device]
+fn update_stdin(model: nitrokey::Model) -> anyhow::Result<()> {
+ clear_pws(model)?;
+
+ let mut ncli = Nitrocli::new().model(model);
+
+ let _ = ncli.handle(&["pws", "add", "--slot", "0", "name0", "login0", "pass0rd"])?;
+ let _ = ncli
+ .stdin("passw1rd")
+ .handle(&["pws", "update", "0", "--password", "-"])?;
+
+ assert_slot(model, 0, "name0", "login0", "passw1rd")?;
+
+ Ok(())
+}
+
 #[test_device]
 fn add_full(model: nitrokey::Model) -> anyhow::Result<()> {
  clear_pws(model)?;
@@ -320,3 +336,24 @@ fn add(model: nitrokey::Model) -> anyhow::Result<()> {
 
  Ok(())
 }
+
+#[test_device]
+fn add_stdin(model: nitrokey::Model) -> anyhow::Result<()> {
+ clear_pws(model)?;
+
+ let mut ncli = Nitrocli::new().model(model);
+
+ // Fill slots 0 and 5
+ let _ = ncli.handle(&["pws", "add", "--slot", "0", "name0", "login0", "pass0rd"])?;
+ let _ = ncli.handle(&["pws", "add", "--slot", "5", "name5", "login5", "pass5rd"])?;
+
+ // Try to add another one
+ let out = ncli
+ .stdin("passw1rd")
+ .handle(&["pws", "add", "name1", "login1", "-"])?;
+ assert_eq!("Added PWS slot 1\n", out);
+
+ assert_slot(model, 1, "name1", "login1", "passw1rd")?;
+
+ Ok(())
+}