diff --git a/CHANGELOG.md b/CHANGELOG.md index 3758f1d1..926b128f 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/doc/nitrocli.1 b/doc/nitrocli.1 index 24bff555..14cf0490 100644 --- a/doc/nitrocli.1 +++ b/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 \ diff --git a/doc/nitrocli.1.pdf b/doc/nitrocli.1.pdf index 5cd6baaf..bf761f6b 100644 Binary files a/doc/nitrocli.1.pdf and b/doc/nitrocli.1.pdf differ diff --git a/src/args.rs b/src/args.rs index b9a85789..dc6bcea0 100644 --- a/src/args.rs +++ b/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, } #[derive(Debug, PartialEq, structopt::StructOpt)] diff --git a/src/commands.rs b/src/commands.rs index 9d54e592..18206b19 100644 --- a/src/commands.rs +++ b/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, ) -> 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(()) }) } diff --git a/src/tests/pws.rs b/src/tests/pws.rs index b01e127f..68dde9e6 100644 --- a/src/tests/pws.rs +++ b/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);