Skip to content

Commit

Permalink
Properly pad user supplied hexadecimal strings to otp set subcommand
Browse files Browse the repository at this point in the history
The library ultimately taking care of communicating with the Nitrokey
device, libnitrokey, unconditionally expects hexadecimal strings
supplied as part of the configuration of an OTP slot to have an even
number of bytes.
Users should not be aware of this detail and so with this change we take
care of padding the supplied string with a leading zero to make such a
configuration go through without an error.
  • Loading branch information
d-e-s-o committed Oct 13, 2019
1 parent c46803a commit 9f3991a
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 2 deletions.
15 changes: 13 additions & 2 deletions nitrocli/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -646,7 +646,7 @@ fn prepare_base32_secret(secret: &str) -> Result<String> {
/// Configure a one-time password slot on the Nitrokey device.
pub fn otp_set(
ctx: &mut args::ExecCtx<'_>,
data: nitrokey::OtpSlotData,
mut data: nitrokey::OtpSlotData,
algorithm: args::OtpAlgorithm,
counter: u64,
time_window: u16,
Expand All @@ -656,7 +656,18 @@ pub fn otp_set(
let secret = match secret_format {
args::OtpSecretFormat::Ascii => prepare_ascii_secret(&data.secret)?,
args::OtpSecretFormat::Base32 => prepare_base32_secret(&data.secret)?,
args::OtpSecretFormat::Hex => 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 device = authenticate_admin(ctx, device)?;
Expand Down
16 changes: 16 additions & 0 deletions nitrocli/src/tests/otp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@

use super::*;

use crate::args;

#[test_device]
fn set_invalid_slot_raw(device: nitrokey::DeviceWrapper) {
let (rc, out, err) = Nitrocli::with_dev(device).run(&["otp", "set", "100", "name", "1234"]);
Expand Down Expand Up @@ -96,6 +98,20 @@ fn set_get_totp(device: nitrokey::DeviceWrapper) -> crate::Result<()> {
Ok(())
}

#[test_device]
fn set_totp_uneven_chars(device: nitrokey::DeviceWrapper) -> crate::Result<()> {
let secrets = [
(args::OtpSecretFormat::Hex, "123"),
(args::OtpSecretFormat::Base32, "FBILDWWGA2"),
];

let mut ncli = Nitrocli::with_dev(device);
for (format, secret) in &secrets {
let _ = ncli.handle(&["otp", "set", "-f", format.as_ref(), "3", "foobar", &secret])?;
}
Ok(())
}

#[test_device]
fn clear(device: nitrokey::DeviceWrapper) -> crate::Result<()> {
let mut ncli = Nitrocli::with_dev(device);
Expand Down

0 comments on commit 9f3991a

Please sign in to comment.