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

Refactor otp set command #167

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: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ Unreleased
- Removed the `pws set` subcommand
- Added the `--only-aes-key` option to the `reset` command to build a new AES
key without performing a factory reset
- Added support for reading PWS passwords from stdin
- Added support for reading PWS passwords and OTP secrets from stdin
- Added `NITROCLI_RESOLVED_USB_PATH` environment variable to be used by
extensions
- Allowed entering of `base32` encoded strings containing spaces
Expand Down
4 changes: 3 additions & 1 deletion doc/nitrocli.1
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ If \fB\-\-time\fR is set, it is set to \fItime\fR instead, which must be a Unix
timestamp (i.e., the number of seconds since 1970-01-01 00:00:00 UTC).
This command might require the user PIN (see the Configuration section).
.TP
\fBnitrocli otp set \fIslot name secret \
\fBnitrocli otp set \fIslot name secret\fR|\fB- \
\fR[\fB\-a\fR|\fB\-\-algorithm \fIalgorithm\fR] \
[\fB\-d\fR|\fB\-\-digits \fIdigits\fR] [\fB\-c\fR|\fB\-\-counter \fIcounter\fR] \
[\fB\-t\fR|\fB\-\-time-window \fItime-window\fR] \
Expand All @@ -191,6 +191,8 @@ Configure a one-time password slot.
\fIslot\fR is the number of the slot to configure.
\fIname\fR is the name of the slot (may not be empty).
\fIsecret\fR is the secret value to store in that slot.
If \fIsecret\fR is set to \fB-\fR, the secret is read from the standard
input.

The \fB\-\-format\fR option specifies the format of the secret.
If it is set to \fBascii\fR, each character of the given secret is interpreted
Expand Down
Binary file modified doc/nitrocli.1.pdf
Binary file not shown.
63 changes: 32 additions & 31 deletions src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use std::ffi;
use std::fmt;
use std::fs;
use std::io;
use std::mem;
use std::ops;
use std::ops::Deref as _;
use std::path;
Expand Down Expand Up @@ -824,39 +823,41 @@ fn prepare_base32_secret(secret: &str) -> anyhow::Result<String> {
.context("Failed to parse base32 secret")
}

/// Configure a one-time password slot on the Nitrokey device.
pub fn otp_set(ctx: &mut Context<'_>, mut args: args::OtpSetArgs) -> anyhow::Result<()> {
let mut data = nitrokey::OtpSlotData {
number: args.slot,
name: mem::take(&mut args.name),
secret: mem::take(&mut args.secret),
mode: args.digits.into(),
use_enter: false,
token_id: None,
};
/// Prepare a secret string in the given format for libnitrokey.
fn prepare_secret(
secret: borrow::Cow<'_, str>,
format: args::OtpSecretFormat,
) -> anyhow::Result<String> {
match format {
args::OtpSecretFormat::Ascii => prepare_ascii_secret(&secret),
args::OtpSecretFormat::Base32 => prepare_base32_secret(&secret),
args::OtpSecretFormat::Hex => {
// We need to ensure to provide a string with an even number of
// characters in it, just because that's what libnitrokey
// expects. So prepend a '0' if that is not the case.
// TODO: This code can be removed once upstream issue #164
// (https://github.com/Nitrokey/libnitrokey/issues/164) is
// addressed.
let mut secret = secret.into_owned();
if secret.len() % 2 != 0 {
secret.insert(0, '0')
}
Ok(secret)
}
}
}

/// Configure a one-time password slot on the Nitrokey device.
pub fn otp_set(ctx: &mut Context<'_>, args: args::OtpSetArgs) -> anyhow::Result<()> {
let secret = value_or_stdin(ctx, &args.secret)?;
let secret = prepare_secret(secret, args.format)?;
let data = nitrokey::OtpSlotData::new(args.slot, args.name, secret, args.digits.into());
let (algorithm, counter, time_window) = (args.algorithm, args.counter, args.time_window);
with_device(ctx, |ctx, device| {
let secret = match args.format {
args::OtpSecretFormat::Ascii => prepare_ascii_secret(&data.secret)?,
args::OtpSecretFormat::Base32 => prepare_base32_secret(&data.secret)?,
args::OtpSecretFormat::Hex => {
// We need to ensure to provide a string with an even number of
// characters in it, just because that's what libnitrokey
// expects. So prepend a '0' if that is not the case.
// TODO: This code can be removed once upstream issue #164
// (https://github.com/Nitrokey/libnitrokey/issues/164) is
// addressed.
if data.secret.len() % 2 != 0 {
data.secret.insert(0, '0')
}
data.secret
}
};
let data = nitrokey::OtpSlotData { secret, ..data };
let mut device = authenticate_admin(ctx, device)?;
match args.algorithm {
args::OtpAlgorithm::Hotp => device.write_hotp_slot(data, args.counter),
args::OtpAlgorithm::Totp => device.write_totp_slot(data, args.time_window),
match algorithm {
args::OtpAlgorithm::Hotp => device.write_hotp_slot(data, counter),
args::OtpAlgorithm::Totp => device.write_totp_slot(data, time_window),
}
.context("Failed to write OTP slot")?;
Ok(())
Expand Down
18 changes: 18 additions & 0 deletions src/tests/otp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,24 @@ fn set_totp_uneven_chars(model: nitrokey::Model) -> anyhow::Result<()> {
Ok(())
}

#[test_device]
fn set_stdin(model: nitrokey::Model) -> anyhow::Result<()> {
const SECRET: &str = "12345678901234567890";
const TIME: &str = stringify!(1111111111);
const OTP: &str = concat!(14050471, "\n");

let _ = Nitrocli::new()
.model(model)
.stdin(SECRET)
.handle(&["otp", "set", "-d", "8", "-f", "ascii", "2", "name", "-"])?;

let out = Nitrocli::new()
.model(model)
.handle(&["otp", "get", "-t", TIME, "2"])?;
assert_eq!(out, OTP);
Ok(())
}

#[test_device]
fn clear(model: nitrokey::Model) -> anyhow::Result<()> {
let mut ncli = Nitrocli::new().model(model);
Expand Down