Add pws update subcommand

This patch adds the pws update subcommand that overwrites some fields of
a programmed PWS slot. We always have to specify the values for all
fields when calling writing to a slot, so we first query the current
values of the fields that remain unchanged and then execute the write
command. This is a potential race condition, but concurrent access to a
Nitrokey device is unsupported and highly likely to mess up the
communication anyway.

CHANGELOG.md

@@ -2,6 +2,7 @@ Unreleased
 ----------
 - Enabled usage of empty PWS slot fields
 - Changed error reporting format to make up only a single line
+- Added the `pws update` subcommand to update an existing PWS slot
 - Added the `--only-aes-key` option to the `reset` command to build a new AES
  key without performing a factory reset
 - Added `NITROCLI_RESOLVED_USB_PATH` environment variable to be used by

doc/nitrocli.1

@@ -279,6 +279,16 @@ Set the content of a PWS slot.
 \fIname\fR, \fIlogin\fR, and \fIpassword\fR represent the data to write to the
 slot.
 .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]
+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.
+.TP
 \fBnitrocli pws clear \fIslot\fR
 Delete the data stored in a PWS slot.
 \fIslot\fR is the number of the slot clear.

src/args.rs

@@ -398,6 +398,16 @@ Command! {PwsCommand, [
  Set(PwsSetArgs) => |ctx, args: PwsSetArgs| {
  crate::commands::pws_set(ctx, args.slot, &args.name, &args.login, &args.password)
  },
+ /// Updates a password safe slot
+ Update(PwsUpdateArgs) => |ctx, args: PwsUpdateArgs| {
+ crate::commands::pws_update(
+ ctx,
+ args.slot,
+ args.name.as_deref(),
+ args.login.as_deref(),
+ args.password.as_deref()
+ )
+ },
  /// Prints the status of the password safe slots
  Status(PwsStatusArgs) => |ctx, args: PwsStatusArgs| crate::commands::pws_status(ctx, args.all),
 ]}
@@ -438,6 +448,21 @@ pub struct PwsSetArgs {
  pub password: String,
 }
 
+#[derive(Debug, PartialEq, structopt::StructOpt)]
+pub struct PwsUpdateArgs {
+ /// The PWS slot to update
+ pub slot: u8,
+ /// The new name to store on the slot
+ #[structopt(short, long)]
+ pub name: Option<String>,
+ /// The new login to store on the slot
+ #[structopt(short, long)]
+ pub login: Option<String>,
+ /// The new password to store on the slot
+ #[structopt(short, long)]
+ pub password: Option<String>,
+}
+
 #[derive(Debug, PartialEq, structopt::StructOpt)]
 pub struct PwsStatusArgs {
  /// Shows slots that are not programmed

src/commands.rs

@@ -1,6 +1,6 @@
 // commands.rs
 
-// Copyright (C) 2018-2020 The Nitrocli Developers
+// Copyright (C) 2018-2021 The Nitrocli Developers
 // SPDX-License-Identifier: GPL-3.0-or-later
 
 use std::borrow;
@@ -1046,6 +1046,37 @@ pub fn pws_set(
  })
 }
 
+/// Update a PWS slot.
+pub fn pws_update(
+ ctx: &mut Context<'_>,
+ slot_idx: u8,
+ name: Option<&str>,
+ login: Option<&str>,
+ password: Option<&str>,
+) -> anyhow::Result<()> {
+ if name.is_none() && login.is_none() && password.is_none() {
+ anyhow::bail!("You have to set at least one of --name, --login, or --password");
+ }
+ with_password_safe(ctx, |_ctx, mut pws| {
+ let slot = pws.get_slot(slot_idx).context("Failed to query PWS slot")?;
+ let name = name
+ .map(|s| Ok(borrow::Cow::from(s)))
+ .unwrap_or_else(|| slot.get_name().map(borrow::Cow::from))
+ .context("Failed to query current slot name")?;
+ let login = login
+ .map(|s| Ok(borrow::Cow::from(s)))
+ .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)))
+ .unwrap_or_else(|| slot.get_password().map(borrow::Cow::from))
+ .context("Failed to query current slot password")?;
+ pws
+ .write_slot(slot_idx, name.as_ref(), login.as_ref(), password.as_ref())
+ .context("Failed to write PWS slot")
+ })
+}
+
 /// Clear a PWS slot.
 pub fn pws_clear(ctx: &mut Context<'_>, slot: u8) -> anyhow::Result<()> {
  with_password_safe(ctx, |_ctx, mut pws| {

src/tests/pws.rs

@@ -5,6 +5,19 @@
 
 use super::*;
 
+fn assert_slot(
+ model: nitrokey::Model,
+ slot: u8,
+ name: &str,
+ login: &str,
+ password: &str,
+) -> anyhow::Result<()> {
+ let mut ncli = Nitrocli::new().model(model);
+ let out = ncli.handle(&["pws", "get", &slot.to_string(), "--quiet"])?;
+ assert_eq!(format!("{}\n{}\n{}\n", name, login, password), out);
+ Ok(())
+}
+
 #[test_device]
 fn set_invalid_slot(model: nitrokey::Model) {
  let err = Nitrocli::new()
@@ -52,8 +65,7 @@ fn set_get(model: nitrokey::Model) -> anyhow::Result<()> {
  let out = ncli.handle(&["pws", "get", "1", "--quiet", "--password"])?;
  assert_eq!(out, format!("{}\n", PASSWORD));
 
- let out = ncli.handle(&["pws", "get", "1", "--quiet"])?;
- assert_eq!(out, format!("{}\n{}\n{}\n", NAME, LOGIN, PASSWORD));
+ assert_slot(model, 1, NAME, LOGIN, PASSWORD)?;
 
  let out = ncli.handle(&["pws", "get", "1"])?;
  assert_eq!(
@@ -80,8 +92,7 @@ fn set_empty(model: nitrokey::Model) -> anyhow::Result<()> {
  let out = ncli.handle(&["pws", "get", "1", "--quiet", "--password"])?;
  assert_eq!(out, "\n");
 
- let out = ncli.handle(&["pws", "get", "1", "--quiet"])?;
- assert_eq!(out, "\n\n\n");
+ assert_slot(model, 1, "", "", "")?;
 
  let out = ncli.handle(&["pws", "get", "1"])?;
  assert_eq!(out, "name: \nlogin: \npassword: \n",);
@@ -117,3 +128,74 @@ fn clear(model: nitrokey::Model) -> anyhow::Result<()> {
  assert_eq!(err, "Failed to access PWS slot");
  Ok(())
 }
+
+#[test_device]
+fn update_unprogrammed(model: nitrokey::Model) -> anyhow::Result<()> {
+ let mut ncli = Nitrocli::new().model(model);
+ let _ = ncli.handle(&["pws", "set", "10", "clear-test", "some-login", "abcdef"])?;
+ let _ = ncli.handle(&["pws", "clear", "10"])?;
+ let res = ncli.handle(&["pws", "update", "10", "--name", "test"]);
+
+ let err = res.unwrap_err().to_string();
+ assert_eq!(err, "Failed to query PWS slot");
+ Ok(())
+}
+
+#[test_device]
+fn update_no_options(model: nitrokey::Model) -> anyhow::Result<()> {
+ let mut ncli = Nitrocli::new().model(model);
+ let res = ncli.handle(&["pws", "update", "10"]);
+
+ let err = res.unwrap_err().to_string();
+ assert_eq!(
+ err,
+ "You have to set at least one of --name, --login, or --password"
+ );
+ Ok(())
+}
+
+#[test_device]
+fn update(model: nitrokey::Model) -> anyhow::Result<()> {
+ const NAME_BEFORE: &str = "name-before";
+ const NAME_AFTER: &str = "name-after";
+ const LOGIN_BEFORE: &str = "login-before";
+ const LOGIN_AFTER: &str = "login-after";
+ const PASSWORD_BEFORE: &str = "password-before";
+ const PASSWORD_AFTER: &str = "password-after";
+
+ let mut ncli = Nitrocli::new().model(model);
+ let _ = ncli.handle(&[
+ "pws",
+ "set",
+ "10",
+ NAME_BEFORE,
+ LOGIN_BEFORE,
+ PASSWORD_BEFORE,
+ ])?;
+
+ assert_slot(model, 10, NAME_BEFORE, LOGIN_BEFORE, PASSWORD_BEFORE)?;
+
+ let _ = ncli.handle(&["pws", "update", "10", "--name", NAME_AFTER])?;
+ assert_slot(model, 10, NAME_AFTER, LOGIN_BEFORE, PASSWORD_BEFORE)?;
+
+ let _ = ncli.handle(&["pws", "update", "10", "--login", LOGIN_AFTER])?;
+ assert_slot(model, 10, NAME_AFTER, LOGIN_AFTER, PASSWORD_BEFORE)?;
+
+ let _ = ncli.handle(&["pws", "update", "10", "--password", PASSWORD_AFTER])?;
+ assert_slot(model, 10, NAME_AFTER, LOGIN_AFTER, PASSWORD_AFTER)?;
+
+ let _ = ncli.handle(&[
+ "pws",
+ "update",
+ "10",
+ "--name",
+ NAME_BEFORE,
+ "--login",
+ LOGIN_BEFORE,
+ "--password",
+ PASSWORD_BEFORE,
+ ])?;
+ assert_slot(model, 10, NAME_BEFORE, LOGIN_BEFORE, PASSWORD_BEFORE)?;
+
+ Ok(())
+}