Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --only-aes-key option to reset command #149

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ Unreleased
- Allowed entering of `base32` encoded strings containing spaces
- Fixed pinentry dialog highlighting some messages incorrectly as errors
- Bumped `nitrokey` dependency to `0.9.0`
- Added the `--only-aes-key` option to the `reset` command to build a new AES
key without performing a factory reset


0.4.0
Expand Down
6 changes: 5 additions & 1 deletion doc/nitrocli.1
Original file line number Diff line number Diff line change
Expand Up @@ -79,12 +79,16 @@ This command locks the password safe (see the Password safe section). On the
Nitrokey Storage, it will also close any active encrypted or hidden volumes (see
the Storage section).
.TP
.B nitrocli reset
.B nitrocli reset \fR[\fB\-\-only-aes-key\fR]
Perform a factory reset on the Nitrokey.
This command performs a factory reset on the OpenPGP smart card, clears the
flash storage and builds a new AES key.
The user PIN is reset to 123456, the admin PIN to 12345678.

If the \fB\-\-only-aes-key\fR option is set, the command does not perform a
full factory reset but only creates a new AES key.
The AES key is for example used to encrypt the password safe.

This command requires the admin PIN.
To avoid accidental calls of this command, the user has to enter the PIN even
if it has been cached.
Expand Down
Binary file modified doc/nitrocli.1.pdf
Binary file not shown.
9 changes: 8 additions & 1 deletion src/args.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ Command! {
/// Accesses the password safe
Pws(PwsArgs) => |ctx, args: PwsArgs| args.subcmd.execute(ctx),
/// Performs a factory reset
Reset => crate::commands::reset,
Reset(ResetArgs) => |ctx, args: ResetArgs| crate::commands::reset(ctx, args.only_aes_key),
/// Prints the status of the connected Nitrokey device
Status => crate::commands::status,
/// Interacts with the device's unencrypted volume
Expand Down Expand Up @@ -445,6 +445,13 @@ pub struct PwsStatusArgs {
pub all: bool,
}

#[derive(Debug, PartialEq, structopt::StructOpt)]
pub struct ResetArgs {
/// Only build a new AES key instead of performing a full factory reset.
#[structopt(long)]
pub only_aes_key: bool,
}

#[derive(Debug, PartialEq, structopt::StructOpt)]
pub struct UnencryptedArgs {
#[structopt(subcommand)]
Expand Down
38 changes: 23 additions & 15 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ pub fn fill(ctx: &mut Context<'_>, attach: bool) -> anyhow::Result<()> {
}

/// Perform a factory reset.
pub fn reset(ctx: &mut Context<'_>) -> anyhow::Result<()> {
pub fn reset(ctx: &mut Context<'_>, only_aes_key: bool) -> anyhow::Result<()> {
with_device(ctx, |ctx, mut device| {
let pin_entry = pinentry::PinEntry::from(args::PinType::Admin, &device)?;

Expand All @@ -522,20 +522,28 @@ pub fn reset(ctx: &mut Context<'_>) -> anyhow::Result<()> {
pinentry::clear(&pin_entry).context("Failed to clear cached secret")?;

try_with_pin(ctx, &pin_entry, |pin| {
device
.factory_reset(&pin)
.context("Failed to reset to factory settings")?;
// Work around for a timing issue between factory_reset and
// build_aes_key, see
// https://github.com/Nitrokey/nitrokey-storage-firmware/issues/80
thread::sleep(time::Duration::from_secs(3));
// Another work around for spurious WrongPassword returns of
// build_aes_key after a factory reset on Pro devices.
// https://github.com/Nitrokey/nitrokey-pro-firmware/issues/57
let _ = device.get_user_retry_count();
device
.build_aes_key(nitrokey::DEFAULT_ADMIN_PIN)
.context("Failed to rebuild AES key")
if only_aes_key {
// Similar to the else arm, we have to execute this command to avoid WrongPassword errors
let _ = device.get_user_retry_count();
device
.build_aes_key(&pin)
.context("Failed to rebuild AES key")
} else {
device
.factory_reset(&pin)
.context("Failed to reset to factory settings")?;
// Work around for a timing issue between factory_reset and
// build_aes_key, see
// https://github.com/Nitrokey/nitrokey-storage-firmware/issues/80
thread::sleep(time::Duration::from_secs(3));
// Another work around for spurious WrongPassword returns of
// build_aes_key after a factory reset on Pro devices.
// https://github.com/Nitrokey/nitrokey-pro-firmware/issues/57
let _ = device.get_user_retry_count();
device
.build_aes_key(nitrokey::DEFAULT_ADMIN_PIN)
.context("Failed to rebuild AES key")
}
})
})
}
Expand Down
53 changes: 53 additions & 0 deletions src/tests/reset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,3 +43,56 @@ fn reset(model: nitrokey::Model) -> anyhow::Result<()> {

Ok(())
}

#[test_device]
fn reset_only_aes_key(model: nitrokey::Model) -> anyhow::Result<()> {
const NEW_USER_PIN: &str = "654321";
const NAME: &str = "slotname";
const LOGIN: &str = "sloglogin";
const PASSWORD: &str = "slotpassword";

let mut ncli = Nitrocli::new().model(model).new_user_pin(NEW_USER_PIN);

// Change the user PIN
let _ = ncli.handle(&["pin", "set", "user"])?;

// Add an entry to the PWS
{
let mut manager = nitrokey::force_take()?;
let mut device = manager.connect_model(model)?;
let mut pws = device.get_password_safe(NEW_USER_PIN)?;
pws.write_slot(0, NAME, LOGIN, PASSWORD)?;
}

// Build AES key
let mut ncli = Nitrocli::new().model(model);
let out = ncli.handle(&["reset", "--only-aes-key"])?;
assert!(out.is_empty());

// Check that 1) the password store works, i.e., there is an AES key, that 2) we can no longer
// access the stored data, i. e. the AES has been replaced, and that 3) the changed admin PIN
// still works, i. e. we did not perform a factory reset.
{
let mut manager = nitrokey::force_take()?;
let mut device = manager.connect_model(model)?;
let pws = device.get_password_safe(NEW_USER_PIN)?;
let slot = pws.get_slot_unchecked(0)?;

if let Ok(name) = slot.get_name() {
assert_ne!(NAME, &name);
}
if let Ok(login) = slot.get_login() {
assert_ne!(LOGIN, &login);
}
if let Ok(password) = slot.get_password() {
assert_ne!(PASSWORD, &password);
}
}

// Reset the admin PIN for other tests
let mut ncli = ncli.user_pin(NEW_USER_PIN).new_user_pin(nitrokey::DEFAULT_USER_PIN);
let out = ncli.handle(&["pin", "set", "user"])?;
assert!(out.is_empty());

Ok(())
}