Add --slot option to pws add

This patch adds the --slot option the pws add command so that we can add
data on a specific slot without using pws set (which might override a
programmed slot).

CHANGELOG.md

@@ -2,7 +2,7 @@ Unreleased
 ----------
 - Enabled usage of empty PWS slot fields
 - Changed error reporting format to make up only a single line
-- Added the `pws add` subcommand to write to the first unprogrammed slot
+- Added the `pws add` subcommand to write to a new slot
 - 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

doc/nitrocli.1

@@ -1,4 +1,4 @@
-.TH NITROCLI 1 2021-04-17
+.TH NITROCLI 1 2021-04-18
 .SH NAME
 nitrocli \- access Nitrokey devices
 .SH SYNOPSIS
@@ -279,10 +279,13 @@ 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 add \fIslot name login password\fR
+\fBnitrocli pws add \fR[\fB\-s\fR|\fB\-\-slot \fIslot\fR] \
+\fIname login password\fR
 Add a new PWS slot.
-This command locates the first free PWS slot and sets its content to the given
-values.
+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.
 .TP
 \fBnitrocli pws update \fIslot\fR \

src/args.rs

@@ -398,9 +398,9 @@ Command! {PwsCommand, [
  Set(PwsSetArgs) => |ctx, args: PwsSetArgs| {
  crate::commands::pws_set(ctx, args.slot, &args.name, &args.login, &args.password)
  },
- /// Writes data to the first free password safe slot
+ /// Adds a new password safe slot
  Add(PwsAddArgs) => |ctx, args: PwsAddArgs| {
- crate::commands::pws_add(ctx, &args.name, &args.login, &args.password)
+ crate::commands::pws_add(ctx, &args.name, &args.login, &args.password, args.slot)
  },
  /// Updates a password safe slot
  Update(PwsUpdateArgs) => |ctx, args: PwsUpdateArgs| {
@@ -460,6 +460,11 @@ pub struct PwsAddArgs {
  pub login: String,
  /// The password to store on the slot
  pub password: String,
+ /// The number of the slot to write
+ ///
+ /// If this option is not set, the first unprogrammed slot is used.
+ #[structopt(short, long)]
+ pub slot: Option<u8>,
 }
 
 #[derive(Debug, PartialEq, structopt::StructOpt)]

src/commands.rs

@@ -1047,25 +1047,48 @@ pub fn pws_set(
  })
 }
 
-/// Write data to the first unprogrammed PWS slot.
+/// Add a new PWS slot.
 pub fn pws_add(
  ctx: &mut Context<'_>,
  name: &str,
  login: &str,
  password: &str,
+ slot_idx: Option<u8>,
 ) -> anyhow::Result<()> {
  with_password_safe(ctx, |ctx, mut pws| {
  let slots = pws.get_slots()?;
- if let Some(slot) = slots.iter().position(Option::is_none) {
- let slot = u8::try_from(slot).context("Unexpected number of password slots")?;
- pws
- .write_slot(slot, name, login, password)
- .context("Failed to write PWS slot")?;
- println!(ctx, "Added PWS slot {}", slot)?;
- Ok(())
+
+ let slot_idx = if let Some(slot_idx) = slot_idx {
+ // If the user specified a slot, make sure that it is not programmed
+ if let Some(slot) = slots.get(usize::from(slot_idx)) {
+ if slot.is_some() {
+ Err(anyhow::anyhow!(
+ "The PWS slot {} is already programmed",
+ slot_idx
+ ))
+ } else {
+ Ok(slot_idx)
+ }
+ } else {
+ Err(anyhow::anyhow!(
+ "Encountered invalid slot index: {}",
+ slot_idx
+ ))
+ }
  } else {
- Err(anyhow::anyhow!("All PWS slots are already programmed"))
- }
+ // If the user did not specify a slot, we try to find the first unprogrammed slot
+ if let Some(slot_idx) = slots.iter().position(Option::is_none) {
+ u8::try_from(slot_idx).context("Unexpected number of PWS slots")
+ } else {
+ Err(anyhow::anyhow!("All PWS slots are already programmed"))
+ }
+ }?;
+
+ pws
+ .write_slot(slot_idx, name, login, password)
+ .context("Failed to write PWS slot")?;
+ println!(ctx, "Added PWS slot {}", slot_idx)?;
+ Ok(())
  })
 }
 

src/tests/pws.rs

@@ -29,6 +29,17 @@ fn set_invalid_slot(model: nitrokey::Model) {
  assert_eq!(err, "Failed to write PWS slot");
 }
 
+#[test_device]
+fn add_invalid_slot(model: nitrokey::Model) {
+ let err = Nitrocli::new()
+ .model(model)
+ .handle(&["pws", "add", "--slot", "100", "name", "login", "1234"])
+ .unwrap_err()
+ .to_string();
+
+ assert_eq!(err, "Encountered invalid slot index: 100");
+}
+
 #[test_device]
 fn status(model: nitrokey::Model) -> anyhow::Result<()> {
  let re = regex::Regex::new(
@@ -227,6 +238,43 @@ fn add_full(model: nitrokey::Model) -> anyhow::Result<()> {
  Ok(())
 }
 
+#[test_device]
+fn add_existing(model: nitrokey::Model) -> anyhow::Result<()> {
+ let mut ncli = Nitrocli::new().model(model);
+
+ // Fill slot 0
+ let _ = ncli.handle(&["pws", "set", "0", "name0", "login0", "pass0rd"])?;
+
+ // Try to add slot 0
+ let res = ncli.handle(&["pws", "add", "--slot", "0", "name", "login", "passw0rd"]);
+
+ let err = res.unwrap_err().to_string();
+ assert_eq!(err, "The PWS slot 0 is already programmed");
+ Ok(())
+}
+
+#[test_device]
+fn add_slot(model: nitrokey::Model) -> anyhow::Result<()> {
+ let mut ncli = Nitrocli::new().model(model);
+
+ // Fill slots 0 and 5
+ let _ = ncli.handle(&["pws", "set", "0", "name0", "login0", "passw0rd"])?;
+ let _ = ncli.handle(&["pws", "set", "5", "name5", "login5", "passw5rd"])?;
+
+ // Clear slot 1 (in case it was written to by other slots)
+ let _ = ncli.handle(&["pws", "clear", "1"])?;
+
+ // Try to add slot 1
+ let out = ncli.handle(&["pws", "add", "--slot", "1", "name1", "login1", "passw1rd"])?;
+ assert_eq!("Added PWS slot 1\n", out);
+
+ assert_slot(model, 0, "name0", "login0", "passw0rd")?;
+ assert_slot(model, 1, "name1", "login1", "passw1rd")?;
+ assert_slot(model, 5, "name5", "login5", "passw5rd")?;
+
+ Ok(())
+}
+
 #[test_device]
 fn add(model: nitrokey::Model) -> anyhow::Result<()> {
  let mut ncli = Nitrocli::new().model(model);