~ireas/nitrokey-rs

d87859975dc158919ecd5bf11a1111a2da5fcb30 — Robin Krahl 2 years ago 17f9c30
Check specific error codes in the tests

If possible, check specific error codes instead of `is_err()`.  This
makes the code more readable and catches bugs resulting in the wrong
error code.  Also, using the assert_*_err and assert_ok macros yields
error messages containing the expected and the actual value.

To be able to use these macros with the `get_password_safe` method, we
also have to implement `Debug` for `PasswordSafe` and `Device`.
7 files changed, 32 insertions(+), 51 deletions(-)

M TODO.md
M src/device.rs
M src/pws.rs
M tests/device.rs
M tests/otp.rs
M tests/pws.rs
M tests/util/mod.rs
M TODO.md => TODO.md +0 -1
@@ 9,7 9,6 @@
- Clear passwords from memory.
- Find a nicer syntax for the `write_config` test.
- Prevent construction of internal types.
- More specific error checking in the tests.
- Check integer conversions.
- Consider implementing `Into<CommandError>` for `(Device, CommandError)`
- Lock password safe in `PasswordSafe::drop()` (see [nitrokey-storage-firmware

M src/device.rs => src/device.rs +1 -1
@@ 286,7 286,7 @@ pub struct StorageStatus {
///
/// This trait provides the commands that can be executed without authentication and that are
/// present on all supported Nitrokey devices.
pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp {
pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp + fmt::Debug {
    /// Returns the model of the connected Nitrokey device.
    ///
    /// # Example

M src/pws.rs => src/pws.rs +1 -0
@@ 52,6 52,7 @@ pub const SLOT_COUNT: u8 = 16;
/// [`get_password_safe`]: trait.GetPasswordSafe.html#method.get_password_safe
/// [`lock`]: trait.Device.html#method.lock
/// [`GetPasswordSafe`]: trait.GetPasswordSafe.html
#[derive(Debug)]
pub struct PasswordSafe<'a> {
    _device: &'a dyn Device,
}

M tests/device.rs => tests/device.rs +15 -31
@@ 5,8 5,8 @@ use std::process::Command;
use std::{thread, time};

use nitrokey::{
    Authenticate, CommandError, Config, ConfigureOtp, Device, Error, GenerateOtp, GetPasswordSafe,
    LibraryError, OtpMode, OtpSlotData, Storage, VolumeMode,
    Authenticate, CommandError, CommunicationError, Config, ConfigureOtp, Device, Error,
    GenerateOtp, GetPasswordSafe, LibraryError, OtpMode, OtpSlotData, Storage, VolumeMode,
};
use nitrokey_test::test as test_device;



@@ 31,11 31,11 @@ fn count_nitrokey_block_devices() -> usize {

#[test_device]
fn connect_no_device() {
    assert!(nitrokey::connect().is_err());
    assert!(nitrokey::connect_model(nitrokey::Model::Pro).is_err());
    assert!(nitrokey::connect_model(nitrokey::Model::Storage).is_err());
    assert!(nitrokey::Pro::connect().is_err());
    assert!(nitrokey::Storage::connect().is_err());
    assert_cmu_err!(CommunicationError::NotConnected, nitrokey::connect());
    assert_cmu_err!(CommunicationError::NotConnected, nitrokey::connect_model(nitrokey::Model::Pro));
    assert_cmu_err!(CommunicationError::NotConnected, nitrokey::connect_model(nitrokey::Model::Storage));
    assert_cmu_err!(CommunicationError::NotConnected, nitrokey::Pro::connect());
    assert_cmu_err!(CommunicationError::NotConnected, nitrokey::Storage::connect());
}

#[test_device]


@@ 148,9 148,7 @@ fn change_user_pin(device: DeviceWrapper) {
    let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
    let device = device.authenticate_user(USER_NEW_PASSWORD).unwrap_err().0;

    assert!(device
        .change_user_pin(USER_PASSWORD, USER_NEW_PASSWORD)
        .is_ok());
    assert_ok!((), device.change_user_pin(USER_PASSWORD, USER_NEW_PASSWORD));

    let device = device.authenticate_user(USER_PASSWORD).unwrap_err().0;
    let device = device


@@ 161,9 159,7 @@ fn change_user_pin(device: DeviceWrapper) {
    let result = device.change_user_pin(USER_PASSWORD, USER_PASSWORD);
    assert_cmd_err!(CommandError::WrongPassword, result);

    assert!(device
        .change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD)
        .is_ok());
    assert_ok!((), device.change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD));

    let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
    assert!(device.authenticate_user(USER_NEW_PASSWORD).is_err());


@@ 174,9 170,7 @@ fn change_admin_pin(device: DeviceWrapper) {
    let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap().device();
    let device = device.authenticate_admin(ADMIN_NEW_PASSWORD).unwrap_err().0;

    assert!(device
        .change_admin_pin(ADMIN_PASSWORD, ADMIN_NEW_PASSWORD)
        .is_ok());
    assert_ok!((), device.change_admin_pin(ADMIN_PASSWORD, ADMIN_NEW_PASSWORD));

    let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap_err().0;
    let device = device


@@ 189,9 183,7 @@ fn change_admin_pin(device: DeviceWrapper) {
        device.change_admin_pin(ADMIN_PASSWORD, ADMIN_PASSWORD)
    );

    assert!(device
        .change_admin_pin(ADMIN_NEW_PASSWORD, ADMIN_PASSWORD)
        .is_ok());
    assert_ok!((), device.change_admin_pin(ADMIN_NEW_PASSWORD, ADMIN_PASSWORD));

    let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap().device();
    device.authenticate_admin(ADMIN_NEW_PASSWORD).unwrap_err();


@@ 215,9 207,7 @@ where
#[test_device]
fn unlock_user_pin(device: DeviceWrapper) {
    let device = device.authenticate_user(USER_PASSWORD).unwrap().device();
    assert!(device
        .unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD)
        .is_ok());
    assert_ok!((), device.unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD));
    assert_cmd_err!(
        CommandError::WrongPassword,
        device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD)


@@ 235,9 225,7 @@ fn unlock_user_pin(device: DeviceWrapper) {
        CommandError::WrongPassword,
        device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD)
    );
    assert!(device
        .unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD)
        .is_ok());
    assert_ok!((), device.unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD));
    let device = device.authenticate_user(USER_PASSWORD).unwrap().device();

    // block user PIN


@@ 251,14 239,10 @@ fn unlock_user_pin(device: DeviceWrapper) {
        CommandError::WrongPassword,
        device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD)
    );
    assert!(device
        .unlock_user_pin(ADMIN_PASSWORD, USER_NEW_PASSWORD)
        .is_ok());
    assert_ok!((), device.unlock_user_pin(ADMIN_PASSWORD, USER_NEW_PASSWORD));

    // reset user PIN
    assert!(device
        .change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD)
        .is_ok());
    assert_ok!((), device.change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD));
}

#[test_device]

M tests/otp.rs => tests/otp.rs +6 -14
@@ 93,7 93,7 @@ fn hotp_pin(device: DeviceWrapper) {
    let user = admin.device().authenticate_user(USER_PASSWORD).unwrap();
    check_hotp_codes(&user, 0);

    assert!(user.device().get_hotp_code(1).is_err());
    assert_cmd_err!(CommandError::NotAuthorized, user.device().get_hotp_code(1));
}

#[test_device]


@@ 156,7 156,7 @@ fn configure_totp(admin: &ConfigureOtp, factor: u64) {
}

fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimestampSize) {
    for (i, &(base_time, code)) in TOTP_CODES.iter().enumerate() {
    for (base_time, code) in TOTP_CODES {
        let time = base_time.checked_mul(factor).unwrap();
        let is_u64 = time > u32::max_value() as u64;
        if is_u64 != (timestamp_size == TotpTimestampSize::U64) {


@@ 164,14 164,7 @@ fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimes
        }

        assert_ok!((), device.set_time(time, true));
        let result = device.get_totp_code(1);
        assert!(result.is_ok());
        let result_code = result.unwrap();
        assert_eq!(
            code, result_code,
            "TOTP code {} should be {} but is {}",
            i, code, result_code
        );
        assert_ok!(code.to_string(), device.get_totp_code(1));
    }
}



@@ 221,7 214,7 @@ fn totp_pin(device: DeviceWrapper) {
    let user = admin.device().authenticate_user(USER_PASSWORD).unwrap();
    check_totp_codes(&user, 1, TotpTimestampSize::U32);

    assert!(user.device().get_totp_code(1).is_err());
    assert_cmd_err!(CommandError::NotAuthorized, user.device().get_totp_code(1));
}

#[test_device]


@@ 235,7 228,7 @@ fn totp_pin_64(device: Pro) {
    let user = admin.device().authenticate_user(USER_PASSWORD).unwrap();
    check_totp_codes(&user, 1, TotpTimestampSize::U64);

    assert!(user.device().get_totp_code(1).is_err());
    assert_cmd_err!(CommandError::NotAuthorized, user.device().get_totp_code(1));
}

#[test_device]


@@ 246,8 239,7 @@ fn totp_slot_name(device: DeviceWrapper) {

    let device = admin.device();
    let result = device.get_totp_slot_name(1);
    assert!(result.is_ok());
    assert_eq!("test-totp", result.unwrap());
    assert_ok!("test-totp", result);
    let result = device.get_totp_slot_name(16);
    assert_lib_err!(LibraryError::InvalidSlot, result);
}

M tests/pws.rs => tests/pws.rs +2 -4
@@ 39,11 39,9 @@ where

#[test_device]
fn enable(device: DeviceWrapper) {
    assert!(device
        .get_password_safe(&(USER_PASSWORD.to_owned() + "123"))
        .is_err());
    assert_cmd_err!(CommandError::WrongPassword, device.get_password_safe(&(USER_PASSWORD.to_owned() + "123")));
    assert!(device.get_password_safe(USER_PASSWORD).is_ok());
    assert!(device.get_password_safe(ADMIN_PASSWORD).is_err());
    assert_cmd_err!(CommandError::WrongPassword, device.get_password_safe(ADMIN_PASSWORD));
    assert!(device.get_password_safe(USER_PASSWORD).is_ok());
}


M tests/util/mod.rs => tests/util/mod.rs +7 -0
@@ 69,6 69,13 @@ macro_rules! assert_cmd_err {
}

#[macro_export]
macro_rules! assert_cmu_err {
    ($left:expr, $right:expr) => {
        assert_err!(::nitrokey::Error::CommunicationError, $left, $right);
    };
}

#[macro_export]
macro_rules! assert_lib_err {
    ($left:expr, $right:expr) => {
        assert_err!(::nitrokey::Error::LibraryError, $left, $right);