From c43b63b70ee32f9fa8e980d89eff5383931f5c39 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 03:49:13 +0000 Subject: Add assert_cmd_err and assert_ok macros to tests These macros allow easier comparisions using the new error type. This patch fixes all tests and updates nitrokey-test to 0.2.0 so that it integrates with the new error structure. Some tests may still fail until CommunicationError::NotConnected is actually returned. --- tests/util/mod.rs | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) (limited to 'tests/util') diff --git a/tests/util/mod.rs b/tests/util/mod.rs index cbf6b93..f0d0bb5 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -1,2 +1,69 @@ pub static ADMIN_PASSWORD: &str = "12345678"; pub static USER_PASSWORD: &str = "123456"; + +#[macro_export] +macro_rules! assert_ok { + ($left:expr, $right:expr) => {{ + match &$right { + Ok(right) => match &$left { + left => { + if !(*left == *right) { + panic!( + r#"assertion failed: `(left == right)` + left: `{:?}`, + right: `{:?}`"#, + left, right + ) + } + } + }, + Err(right_err) => panic!( + r#"assertion failed: `(left == right)` + left: `Ok({:?})`, + right: `Err({:?})`"#, + $left, right_err + ), + } + }}; +} + +#[macro_export] +macro_rules! assert_err { + ($err:path, $left:expr, $right:expr) => { + match &$right { + Err($err(ref right_err)) => match &$left { + left_err => { + if !(*left_err == *right_err) { + panic!( + r#"assertion failed: `(left == right)` + left: `{:?}`, + right: `{:?}`"#, + left_err, right_err + ) + } + } + }, + Err(ref right_err) => panic!( + r#"assertion failed: `(left == right)` + left: `{:?}`, + right: `{:?}`"#, + $err($left), + right_err + ), + Ok(right_ok) => panic!( + r#"assertion failed: `(left == right)` + left: `Err({:?})`, + right: `Ok({:?})`"#, + $err($left), + right_ok + ), + } + }; +} + +#[macro_export] +macro_rules! assert_cmd_err { + ($left:expr, $right:expr) => { + assert_err!(::nitrokey::Error::CommandError, $left, $right); + }; +} -- cgit v1.2.3 From 5e258d26b55af6bed7c316b1c7ac12e20946702d Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 12:47:52 +0000 Subject: Refactor library errors into LibraryError enum Previously, library errors were part of the CommandError enum. As command errors and library errors are two different error types, they should be split into two enums. --- CHANGELOG.md | 2 ++ src/auth.rs | 6 ++--- src/config.rs | 4 +-- src/device.rs | 22 ++++++++--------- src/error.rs | 74 +++++++++++++++++++++++++++++++++++++++---------------- src/lib.rs | 2 +- src/otp.rs | 20 +++++++-------- src/pws.rs | 14 +++++------ src/util.rs | 4 +-- tests/device.rs | 4 +-- tests/otp.rs | 28 ++++++++++----------- tests/pws.rs | 16 ++++++------ tests/util/mod.rs | 7 ++++++ 13 files changed, 123 insertions(+), 80 deletions(-) (limited to 'tests/util') diff --git a/CHANGELOG.md b/CHANGELOG.md index def5273..c34175e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ - Refactor the error handling code: - Implement `std::error::Error` for `CommandError`. - Add the `Error` enum and the `Result` typedef. + - Add the `LibraryError` enum and move the library error variants from + `CommandError` to `LibraryError`. - Return `Error` instead of `CommandError` in all public functions. - Move the `CommandError::RngError` variant to `Error::RandError` and the `CommandError::Unknown` variant to `Error::Unknown`. diff --git a/src/auth.rs b/src/auth.rs index e05f6b3..d1eb049 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -55,7 +55,7 @@ pub trait Authenticate { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword fn authenticate_user(self, password: &str) -> Result, (Self, Error)> @@ -101,7 +101,7 @@ pub trait Authenticate { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> @@ -287,7 +287,7 @@ impl Admin { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot pub fn write_config(&self, config: Config) -> Result<(), Error> { let raw_config = RawConfig::try_from(config)?; unsafe { diff --git a/src/config.rs b/src/config.rs index 741d67e..6aa6d10 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,4 +1,4 @@ -use crate::error::{CommandError, Error}; +use crate::error::{Error, LibraryError}; /// The configuration for a Nitrokey. #[derive(Clone, Copy, Debug, PartialEq)] @@ -41,7 +41,7 @@ fn option_to_config_otp_slot(value: Option) -> Result { if value < 3 { Ok(value) } else { - Err(CommandError::InvalidSlot.into()) + Err(LibraryError::InvalidSlot.into()) } } None => Ok(255), diff --git a/src/device.rs b/src/device.rs index ccd0597..5c4014b 100644 --- a/src/device.rs +++ b/src/device.rs @@ -461,7 +461,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword fn change_admin_pin(&self, current: &str, new: &str) -> Result<(), Error> { let current_string = get_cstring(current)?; @@ -497,7 +497,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword fn change_user_pin(&self, current: &str, new: &str) -> Result<(), Error> { let current_string = get_cstring(current)?; @@ -533,7 +533,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword fn unlock_user_pin(&self, admin_pin: &str, user_pin: &str) -> Result<(), Error> { let admin_pin_string = get_cstring(admin_pin)?; @@ -867,7 +867,7 @@ impl Storage { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword pub fn change_update_pin(&self, current: &str, new: &str) -> Result<(), Error> { let current_string = get_cstring(current)?; @@ -907,7 +907,7 @@ impl Storage { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword pub fn enable_firmware_update(&self, update_pin: &str) -> Result<(), Error> { let update_pin_string = get_cstring(update_pin)?; @@ -943,7 +943,7 @@ impl Storage { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword pub fn enable_encrypted_volume(&self, user_pin: &str) -> Result<(), Error> { let user_pin = get_cstring(user_pin)?; @@ -1021,7 +1021,7 @@ impl Storage { /// /// [`enable_encrypted_volume`]: #method.enable_encrypted_volume /// [`AesDecryptionFailed`]: enum.CommandError.html#variant.AesDecryptionFailed - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString pub fn enable_hidden_volume(&self, volume_password: &str) -> Result<(), Error> { let volume_password = get_cstring(volume_password)?; unsafe { @@ -1099,7 +1099,7 @@ impl Storage { /// ``` /// /// [`AesDecryptionFailed`]: enum.CommandError.html#variant.AesDecryptionFailed - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString pub fn create_hidden_volume( &self, slot: u8, @@ -1145,7 +1145,7 @@ impl Storage { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword pub fn set_unencrypted_volume_mode( &self, @@ -1276,7 +1276,7 @@ impl Storage { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword pub fn clear_new_sd_card_warning(&self, admin_pin: &str) -> Result<(), Error> { let admin_pin = get_cstring(admin_pin)?; @@ -1304,7 +1304,7 @@ impl Storage { /// - [`InvalidString`][] if one of the provided passwords contains a null byte /// - [`WrongPassword`][] if the admin password is wrong /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword pub fn export_firmware(&self, admin_pin: &str) -> Result<(), Error> { let admin_pin_string = get_cstring(admin_pin)?; diff --git a/src/error.rs b/src/error.rs index c5a975e..f40d07f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -11,6 +11,8 @@ pub enum Error { CommandError(CommandError), /// Placeholder for testing. CommunicationError(CommunicationError), + /// A library usage error. + LibraryError(LibraryError), /// An error that occured during random number generation. RandError(rand_core::Error), /// An unknown error returned by libnitrokey. @@ -21,6 +23,8 @@ impl From for Error { fn from(code: raw::c_int) -> Self { if let Some(err) = CommandError::try_from(code) { Error::CommandError(err) + } else if let Some(err) = LibraryError::try_from(code) { + Error::LibraryError(err) } else { Error::Unknown(code.into()) } @@ -33,6 +37,12 @@ impl From for Error { } } +impl From for Error { + fn from(err: LibraryError) -> Self { + Error::LibraryError(err) + } +} + impl From for Error { fn from(error: rand_core::Error) -> Self { Error::RandError(error) @@ -44,6 +54,7 @@ impl error::Error for Error { match *self { Error::CommandError(ref err) => Some(err), Error::CommunicationError(_) => None, + Error::LibraryError(ref err) => Some(err), Error::RandError(ref err) => Some(err), Error::Unknown(_) => None, } @@ -55,6 +66,7 @@ impl fmt::Display for Error { match *self { Error::CommandError(ref err) => write!(f, "Command error: {}", err), Error::CommunicationError(_) => write!(f, "Placeholder"), + Error::LibraryError(ref err) => write!(f, "Library error: {}", err), Error::RandError(ref err) => write!(f, "RNG error: {}", err), Error::Unknown(ref err) => write!(f, "Unknown error: {}", err), } @@ -90,16 +102,6 @@ pub enum CommandError { AesDecryptionFailed, /// An unspecified error occurred. Undefined, - /// You passed a string containing a null byte. - InvalidString, - /// A supplied string exceeded a length limit. - StringTooLong, - /// You passed an invalid slot. - InvalidSlot, - /// The supplied string was not in hexadecimal format. - InvalidHexString, - /// The target buffer was smaller than the source. - TargetBufferTooSmall, } /// Placeholder for testing. @@ -122,10 +124,6 @@ impl CommandError { 8 => Some(CommandError::NotSupported), 9 => Some(CommandError::UnknownCommand), 10 => Some(CommandError::AesDecryptionFailed), - 200 => Some(CommandError::StringTooLong), - 201 => Some(CommandError::InvalidSlot), - 202 => Some(CommandError::InvalidHexString), - 203 => Some(CommandError::TargetBufferTooSmall), _ => None, } } @@ -149,13 +147,6 @@ impl CommandError { CommandError::UnknownCommand => "This command is unknown".into(), CommandError::AesDecryptionFailed => "AES decryption failed".into(), CommandError::Undefined => "An unspecified error occurred".into(), - CommandError::InvalidString => "You passed a string containing a null byte".into(), - CommandError::StringTooLong => "The supplied string is too long".into(), - CommandError::InvalidSlot => "The given slot is invalid".into(), - CommandError::InvalidHexString => { - "The supplied string is not in hexadecimal format".into() - } - CommandError::TargetBufferTooSmall => "The target buffer is too small".into(), } } } @@ -167,3 +158,44 @@ impl fmt::Display for CommandError { write!(f, "{}", self.as_str()) } } + +/// A library usage error. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum LibraryError { + /// A supplied string exceeded a length limit. + StringTooLong, + /// You passed an invalid slot. + InvalidSlot, + /// The supplied string was not in hexadecimal format. + InvalidHexString, + /// The target buffer was smaller than the source. + TargetBufferTooSmall, + /// You passed a string containing a null byte. + InvalidString, +} + +impl LibraryError { + fn try_from(value: raw::c_int) -> Option { + match value { + 200 => Some(LibraryError::StringTooLong), + 201 => Some(LibraryError::InvalidSlot), + 202 => Some(LibraryError::InvalidHexString), + 203 => Some(LibraryError::TargetBufferTooSmall), + _ => None, + } + } +} + +impl error::Error for LibraryError {} + +impl fmt::Display for LibraryError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match *self { + LibraryError::StringTooLong => "The supplied string is too long", + LibraryError::InvalidSlot => "The given slot is invalid", + LibraryError::InvalidHexString => "The supplied string is not in hexadecimal format", + LibraryError::TargetBufferTooSmall => "The target buffer is too small", + LibraryError::InvalidString => "You passed a string containing a null byte", + }) + } +} diff --git a/src/lib.rs b/src/lib.rs index 8522e83..993ec92 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -104,7 +104,7 @@ pub use crate::device::{ connect, connect_model, Device, DeviceWrapper, Model, Pro, SdCardData, Storage, StorageProductionInfo, StorageStatus, VolumeMode, VolumeStatus, }; -pub use crate::error::{CommandError, CommunicationError, Error, Result}; +pub use crate::error::{CommandError, CommunicationError, Error, LibraryError, Result}; pub use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData}; pub use crate::pws::{GetPasswordSafe, PasswordSafe, SLOT_COUNT}; pub use crate::util::LogLevel; diff --git a/src/otp.rs b/src/otp.rs index 5dfe8b1..7535a77 100644 --- a/src/otp.rs +++ b/src/otp.rs @@ -47,8 +47,8 @@ pub trait ConfigureOtp { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`NoName`]: enum.CommandError.html#variant.NoName fn write_hotp_slot(&self, data: OtpSlotData, counter: u64) -> Result<(), Error>; @@ -83,8 +83,8 @@ pub trait ConfigureOtp { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`NoName`]: enum.CommandError.html#variant.NoName fn write_totp_slot(&self, data: OtpSlotData, time_window: u16) -> Result<(), Error>; @@ -115,7 +115,7 @@ pub trait ConfigureOtp { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot fn erase_hotp_slot(&self, slot: u8) -> Result<(), Error>; /// Erases a TOTP slot. @@ -145,7 +145,7 @@ pub trait ConfigureOtp { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot fn erase_totp_slot(&self, slot: u8) -> Result<(), Error>; } @@ -216,7 +216,7 @@ pub trait GenerateOtp { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed fn get_hotp_slot_name(&self, slot: u8) -> Result { unsafe { result_from_string(nitrokey_sys::NK_get_hotp_slot_name(slot)) } @@ -245,7 +245,7 @@ pub trait GenerateOtp { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed fn get_totp_slot_name(&self, slot: u8) -> Result { unsafe { result_from_string(nitrokey_sys::NK_get_totp_slot_name(slot)) } @@ -275,7 +275,7 @@ pub trait GenerateOtp { /// ``` /// /// [`get_config`]: trait.Device.html#method.get_config - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot /// [`NotAuthorized`]: enum.CommandError.html#variant.NotAuthorized /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed fn get_hotp_code(&self, slot: u8) -> Result { @@ -320,7 +320,7 @@ pub trait GenerateOtp { /// /// [`set_time`]: #method.set_time /// [`get_config`]: trait.Device.html#method.get_config - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot /// [`NotAuthorized`]: enum.CommandError.html#variant.NotAuthorized /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed fn get_totp_code(&self, slot: u8) -> Result { diff --git a/src/pws.rs b/src/pws.rs index e974737..47965d7 100644 --- a/src/pws.rs +++ b/src/pws.rs @@ -109,7 +109,7 @@ pub trait GetPasswordSafe { /// [`lock`]: trait.Device.html#method.lock /// [`AesDecryptionFailed`]: enum.CommandError.html#variant.AesDecryptionFailed /// [`Device::build_aes_key`]: trait.Device.html#method.build_aes_key - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`Unknown`]: enum.CommandError.html#variant.Unknown /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword fn get_password_safe(&self, user_pin: &str) -> Result, Error>; @@ -207,7 +207,7 @@ impl<'a> PasswordSafe<'a> { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed pub fn get_slot_name(&self, slot: u8) -> Result { unsafe { result_from_string(nitrokey_sys::NK_get_password_safe_slot_name(slot)) } @@ -240,7 +240,7 @@ impl<'a> PasswordSafe<'a> { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed pub fn get_slot_login(&self, slot: u8) -> Result { unsafe { result_from_string(nitrokey_sys::NK_get_password_safe_slot_login(slot)) } @@ -273,7 +273,7 @@ impl<'a> PasswordSafe<'a> { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed pub fn get_slot_password(&self, slot: u8) -> Result { unsafe { result_from_string(nitrokey_sys::NK_get_password_safe_slot_password(slot)) } @@ -304,8 +304,8 @@ impl<'a> PasswordSafe<'a> { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString pub fn write_slot( &self, slot: u8, @@ -350,7 +350,7 @@ impl<'a> PasswordSafe<'a> { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot pub fn erase_slot(&self, slot: u8) -> Result<(), Error> { unsafe { get_command_result(nitrokey_sys::NK_erase_password_safe_slot(slot)) } } diff --git a/src/util.rs b/src/util.rs index 3b9904f..2738fce 100644 --- a/src/util.rs +++ b/src/util.rs @@ -5,7 +5,7 @@ use libc::{c_void, free}; use rand_core::RngCore; use rand_os::OsRng; -use crate::error::{CommandError, Error}; +use crate::error::{CommandError, Error, LibraryError}; /// Log level for libnitrokey. /// @@ -78,7 +78,7 @@ pub fn generate_password(length: usize) -> Result, Error> { } pub fn get_cstring>>(s: T) -> Result { - CString::new(s).or(Err(CommandError::InvalidString.into())) + CString::new(s).or(Err(LibraryError::InvalidString.into())) } impl Into for LogLevel { diff --git a/tests/device.rs b/tests/device.rs index 174624f..ee5dae1 100644 --- a/tests/device.rs +++ b/tests/device.rs @@ -6,7 +6,7 @@ use std::{thread, time}; use nitrokey::{ Authenticate, CommandError, Config, ConfigureOtp, Device, Error, GenerateOtp, GetPasswordSafe, - OtpMode, OtpSlotData, Storage, VolumeMode, + LibraryError, OtpMode, OtpSlotData, Storage, VolumeMode, }; use nitrokey_test::test as test_device; @@ -130,7 +130,7 @@ fn config(device: DeviceWrapper) { assert_eq!(config, get_config); let config = Config::new(None, Some(9), None, true); - assert_cmd_err!(CommandError::InvalidSlot, admin.write_config(config)); + assert_lib_err!(LibraryError::InvalidSlot, admin.write_config(config)); let config = Config::new(Some(1), None, Some(0), false); assert_ok!((), admin.write_config(config)); diff --git a/tests/otp.rs b/tests/otp.rs index d328351..51a6539 100644 --- a/tests/otp.rs +++ b/tests/otp.rs @@ -4,8 +4,8 @@ use std::fmt::Debug; use std::ops::Deref; use nitrokey::{ - Admin, Authenticate, CommandError, Config, ConfigureOtp, Device, GenerateOtp, OtpMode, - OtpSlotData, + Admin, Authenticate, CommandError, Config, ConfigureOtp, Device, GenerateOtp, LibraryError, + OtpMode, OtpSlotData, }; use nitrokey_test::test as test_device; @@ -106,7 +106,7 @@ fn hotp_slot_name(device: DeviceWrapper) { let result = device.get_hotp_slot_name(1); assert_eq!("test-hotp", result.unwrap()); let result = device.get_hotp_slot_name(4); - assert_cmd_err!(CommandError::InvalidSlot, result); + assert_lib_err!(LibraryError::InvalidSlot, result); } #[test_device] @@ -115,17 +115,17 @@ fn hotp_error(device: DeviceWrapper) { let slot_data = OtpSlotData::new(1, "", HOTP_SECRET, OtpMode::SixDigits); assert_cmd_err!(CommandError::NoName, admin.write_hotp_slot(slot_data, 0)); let slot_data = OtpSlotData::new(4, "test", HOTP_SECRET, OtpMode::SixDigits); - assert_cmd_err!( - CommandError::InvalidSlot, + assert_lib_err!( + LibraryError::InvalidSlot, admin.write_hotp_slot(slot_data, 0) ); let slot_data = OtpSlotData::new(1, "test", "foobar", OtpMode::SixDigits); - assert_cmd_err!( - CommandError::InvalidHexString, + assert_lib_err!( + LibraryError::InvalidHexString, admin.write_hotp_slot(slot_data, 0) ); let code = admin.get_hotp_code(4); - assert_cmd_err!(CommandError::InvalidSlot, code); + assert_lib_err!(LibraryError::InvalidSlot, code); } #[test_device] @@ -249,7 +249,7 @@ fn totp_slot_name(device: DeviceWrapper) { assert!(result.is_ok()); assert_eq!("test-totp", result.unwrap()); let result = device.get_totp_slot_name(16); - assert_cmd_err!(CommandError::InvalidSlot, result); + assert_lib_err!(LibraryError::InvalidSlot, result); } #[test_device] @@ -258,17 +258,17 @@ fn totp_error(device: DeviceWrapper) { let slot_data = OtpSlotData::new(1, "", TOTP_SECRET, OtpMode::SixDigits); assert_cmd_err!(CommandError::NoName, admin.write_totp_slot(slot_data, 0)); let slot_data = OtpSlotData::new(20, "test", TOTP_SECRET, OtpMode::SixDigits); - assert_cmd_err!( - CommandError::InvalidSlot, + assert_lib_err!( + LibraryError::InvalidSlot, admin.write_totp_slot(slot_data, 0) ); let slot_data = OtpSlotData::new(4, "test", "foobar", OtpMode::SixDigits); - assert_cmd_err!( - CommandError::InvalidHexString, + assert_lib_err!( + LibraryError::InvalidHexString, admin.write_totp_slot(slot_data, 0) ); let code = admin.get_totp_code(20); - assert_cmd_err!(CommandError::InvalidSlot, code); + assert_lib_err!(LibraryError::InvalidSlot, code); } #[test_device] diff --git a/tests/pws.rs b/tests/pws.rs index a4647bd..51e6189 100644 --- a/tests/pws.rs +++ b/tests/pws.rs @@ -3,7 +3,9 @@ mod util; use std::ffi::CStr; use libc::{c_int, c_void, free}; -use nitrokey::{CommandError, Device, Error, GetPasswordSafe, PasswordSafe, SLOT_COUNT}; +use nitrokey::{ + CommandError, Device, Error, GetPasswordSafe, LibraryError, PasswordSafe, SLOT_COUNT, +}; use nitrokey_sys; use nitrokey_test::test as test_device; @@ -104,17 +106,17 @@ fn get_data(device: DeviceWrapper) { assert_eq!(login, pws.get_slot_login(1).unwrap()); assert_eq!(password, pws.get_slot_password(1).unwrap()); - assert_cmd_err!(CommandError::InvalidSlot, pws.get_slot_name(SLOT_COUNT)); - assert_cmd_err!(CommandError::InvalidSlot, pws.get_slot_login(SLOT_COUNT)); - assert_cmd_err!(CommandError::InvalidSlot, pws.get_slot_password(SLOT_COUNT)); + assert_lib_err!(LibraryError::InvalidSlot, pws.get_slot_name(SLOT_COUNT)); + assert_lib_err!(LibraryError::InvalidSlot, pws.get_slot_login(SLOT_COUNT)); + assert_lib_err!(LibraryError::InvalidSlot, pws.get_slot_password(SLOT_COUNT)); } #[test_device] fn write(device: DeviceWrapper) { let pws = get_pws(&device); - assert_cmd_err!( - CommandError::InvalidSlot, + assert_lib_err!( + LibraryError::InvalidSlot, pws.write_slot(SLOT_COUNT, "name", "login", "password") ); @@ -137,7 +139,7 @@ fn write(device: DeviceWrapper) { #[test_device] fn erase(device: DeviceWrapper) { let pws = get_pws(&device); - assert_cmd_err!(CommandError::InvalidSlot, pws.erase_slot(SLOT_COUNT)); + assert_lib_err!(LibraryError::InvalidSlot, pws.erase_slot(SLOT_COUNT)); assert_ok!((), pws.write_slot(0, "name", "login", "password")); assert_ok!((), pws.erase_slot(0)); diff --git a/tests/util/mod.rs b/tests/util/mod.rs index f0d0bb5..b1d3ea3 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -67,3 +67,10 @@ macro_rules! assert_cmd_err { assert_err!(::nitrokey::Error::CommandError, $left, $right); }; } + +#[macro_export] +macro_rules! assert_lib_err { + ($left:expr, $right:expr) => { + assert_err!(::nitrokey::Error::LibraryError, $left, $right); + }; +} -- cgit v1.2.3 From d87859975dc158919ecd5bf11a1111a2da5fcb30 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 14:21:44 +0000 Subject: 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`. --- TODO.md | 1 - src/device.rs | 2 +- src/pws.rs | 1 + tests/device.rs | 46 +++++++++++++++------------------------------- tests/otp.rs | 20 ++++++-------------- tests/pws.rs | 6 ++---- tests/util/mod.rs | 7 +++++++ 7 files changed, 32 insertions(+), 51 deletions(-) (limited to 'tests/util') diff --git a/TODO.md b/TODO.md index 53de7e9..487f56d 100644 --- a/TODO.md +++ b/TODO.md @@ -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` for `(Device, CommandError)` - Lock password safe in `PasswordSafe::drop()` (see [nitrokey-storage-firmware diff --git a/src/device.rs b/src/device.rs index 1cf9da9..16064c3 100644 --- a/src/device.rs +++ b/src/device.rs @@ -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 diff --git a/src/pws.rs b/src/pws.rs index 47965d7..a21527c 100644 --- a/src/pws.rs +++ b/src/pws.rs @@ -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, } diff --git a/tests/device.rs b/tests/device.rs index ee5dae1..c502945 100644 --- a/tests/device.rs +++ b/tests/device.rs @@ -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] diff --git a/tests/otp.rs b/tests/otp.rs index 51a6539..96da371 100644 --- a/tests/otp.rs +++ b/tests/otp.rs @@ -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); } diff --git a/tests/pws.rs b/tests/pws.rs index b89d7f6..7a97983 100644 --- a/tests/pws.rs +++ b/tests/pws.rs @@ -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()); } diff --git a/tests/util/mod.rs b/tests/util/mod.rs index b1d3ea3..4a00a66 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -68,6 +68,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) => { -- cgit v1.2.3