Skip to content

Commit

Permalink
Add --only-aes-key option to reset command
Browse files Browse the repository at this point in the history
This patch adds an --only-aes-key option to the reset command to only
build a new AES key without performing a full factory reset.

Fixes #69
  • Loading branch information
robinkrahl authored and d-e-s-o committed Apr 14, 2021
1 parent 420c277 commit 351280e
Show file tree
Hide file tree
Showing 6 changed files with 96 additions and 19 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ Unreleased
----------
- Enabled usage of empty PWS slot fields
- Changed error reporting format to make up only a single line
- 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
extensions
- Allowed entering of `base32` encoded strings containing spaces
Expand Down
8 changes: 6 additions & 2 deletions doc/nitrocli.1
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.TH NITROCLI 1 2021-04-14
.TH NITROCLI 1 2021-04-17
.SH NAME
nitrocli \- access Nitrokey devices
.SH SYNOPSIS
Expand Down 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
58 changes: 57 additions & 1 deletion src/tests/reset.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// reset.rs

// Copyright (C) 2019-2020 The Nitrocli Developers
// Copyright (C) 2019-2021 The Nitrocli Developers
// SPDX-License-Identifier: GPL-3.0-or-later

use nitrokey::Authenticate;
Expand Down Expand Up @@ -43,3 +43,59 @@ 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 user 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 user 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(())
}

0 comments on commit 351280e

Please sign in to comment.