From d0f63513bb935d3d931c86a1ab7b68d6ed44bf27 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 01:53:08 +0000 Subject: Move util::CommandError to the new error module This prepares the refactoring of util::CommandError into multiple enums. --- src/auth.rs | 5 ++- src/config.rs | 2 +- src/device.rs | 5 ++- src/error.rs | 114 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 4 ++- src/otp.rs | 3 +- src/pws.rs | 5 ++- src/util.rs | 113 +-------------------------------------------------------- 8 files changed, 127 insertions(+), 124 deletions(-) create mode 100644 src/error.rs diff --git a/src/auth.rs b/src/auth.rs index 2d61d4b..e805e54 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -6,10 +6,9 @@ use nitrokey_sys; use crate::config::{Config, RawConfig}; use crate::device::{Device, DeviceWrapper, Pro, Storage}; +use crate::error::CommandError; use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData, RawOtpSlotData}; -use crate::util::{ - generate_password, get_command_result, get_cstring, result_from_string, CommandError, -}; +use crate::util::{generate_password, get_command_result, get_cstring, result_from_string}; static TEMPORARY_PASSWORD_LENGTH: usize = 25; diff --git a/src/config.rs b/src/config.rs index 2ce6f77..277dc5e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,4 +1,4 @@ -use crate::util::CommandError; +use crate::error::CommandError; /// The configuration for a Nitrokey. #[derive(Clone, Copy, Debug, PartialEq)] diff --git a/src/device.rs b/src/device.rs index d794e1b..603a986 100644 --- a/src/device.rs +++ b/src/device.rs @@ -5,11 +5,10 @@ use nitrokey_sys; use crate::auth::Authenticate; use crate::config::{Config, RawConfig}; +use crate::error::CommandError; use crate::otp::GenerateOtp; use crate::pws::GetPasswordSafe; -use crate::util::{ - get_command_result, get_cstring, get_last_error, result_from_string, CommandError, -}; +use crate::util::{get_command_result, get_cstring, get_last_error, result_from_string}; /// Available Nitrokey models. #[derive(Clone, Copy, Debug, PartialEq)] diff --git a/src/error.rs b/src/error.rs new file mode 100644 index 0000000..6aeeef8 --- /dev/null +++ b/src/error.rs @@ -0,0 +1,114 @@ +use std::borrow; +use std::fmt; +use std::os::raw; + +/// Error types returned by Nitrokey device or by the library. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum CommandError { + /// A packet with a wrong checksum has been sent or received. + WrongCrc, + /// A command tried to access an OTP slot that does not exist. + WrongSlot, + /// A command tried to generate an OTP on a slot that is not configured. + SlotNotProgrammed, + /// The provided password is wrong. + WrongPassword, + /// You are not authorized for this command or provided a wrong temporary + /// password. + NotAuthorized, + /// An error occurred when getting or setting the time. + Timestamp, + /// You did not provide a name for the OTP slot. + NoName, + /// This command is not supported by this device. + NotSupported, + /// This command is unknown. + UnknownCommand, + /// AES decryption failed. + AesDecryptionFailed, + /// An unknown error occurred. + Unknown(i64), + /// 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, + /// An error occurred during random number generation. + RngError, +} + +impl CommandError { + fn as_str(&self) -> borrow::Cow<'static, str> { + match *self { + CommandError::WrongCrc => { + "A packet with a wrong checksum has been sent or received".into() + } + CommandError::WrongSlot => "The given OTP slot does not exist".into(), + CommandError::SlotNotProgrammed => "The given OTP slot is not programmed".into(), + CommandError::WrongPassword => "The given password is wrong".into(), + CommandError::NotAuthorized => { + "You are not authorized for this command or provided a wrong temporary \ + password" + .into() + } + CommandError::Timestamp => "An error occurred when getting or setting the time".into(), + CommandError::NoName => "You did not provide a name for the OTP slot".into(), + CommandError::NotSupported => "This command is not supported by this device".into(), + CommandError::UnknownCommand => "This command is unknown".into(), + CommandError::AesDecryptionFailed => "AES decryption failed".into(), + CommandError::Unknown(x) => { + borrow::Cow::from(format!("An unknown error occurred ({})", x)) + } + 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(), + CommandError::RngError => "An error occurred during random number generation".into(), + } + } +} + +impl fmt::Display for CommandError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.as_str()) + } +} + +impl From for CommandError { + fn from(value: raw::c_int) -> Self { + match value { + 1 => CommandError::WrongCrc, + 2 => CommandError::WrongSlot, + 3 => CommandError::SlotNotProgrammed, + 4 => CommandError::WrongPassword, + 5 => CommandError::NotAuthorized, + 6 => CommandError::Timestamp, + 7 => CommandError::NoName, + 8 => CommandError::NotSupported, + 9 => CommandError::UnknownCommand, + 10 => CommandError::AesDecryptionFailed, + 200 => CommandError::StringTooLong, + 201 => CommandError::InvalidSlot, + 202 => CommandError::InvalidHexString, + 203 => CommandError::TargetBufferTooSmall, + x => CommandError::Unknown(x.into()), + } + } +} + +impl From for CommandError { + fn from(_error: rand_core::Error) -> Self { + CommandError::RngError + } +} diff --git a/src/lib.rs b/src/lib.rs index 93a9894..cec5db7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -89,6 +89,7 @@ mod auth; mod config; mod device; +mod error; mod otp; mod pws; mod util; @@ -103,9 +104,10 @@ pub use crate::device::{ connect, connect_model, Device, DeviceWrapper, Model, Pro, SdCardData, Storage, StorageProductionInfo, StorageStatus, VolumeMode, VolumeStatus, }; +pub use crate::error::CommandError; pub use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData}; pub use crate::pws::{GetPasswordSafe, PasswordSafe, SLOT_COUNT}; -pub use crate::util::{CommandError, LogLevel}; +pub use crate::util::LogLevel; /// The default admin PIN for all Nitrokey devices. pub const DEFAULT_ADMIN_PIN: &'static str = "12345678"; diff --git a/src/otp.rs b/src/otp.rs index 901bef9..784149a 100644 --- a/src/otp.rs +++ b/src/otp.rs @@ -2,7 +2,8 @@ use std::ffi::CString; use nitrokey_sys; -use crate::util::{get_command_result, get_cstring, result_from_string, CommandError}; +use crate::error::CommandError; +use crate::util::{get_command_result, get_cstring, result_from_string}; /// Modes for one-time password generation. #[derive(Clone, Copy, Debug, PartialEq)] diff --git a/src/pws.rs b/src/pws.rs index 28f0681..615e47c 100644 --- a/src/pws.rs +++ b/src/pws.rs @@ -2,9 +2,8 @@ use libc; use nitrokey_sys; use crate::device::{Device, DeviceWrapper, Pro, Storage}; -use crate::util::{ - get_command_result, get_cstring, get_last_error, result_from_string, CommandError, -}; +use crate::error::CommandError; +use crate::util::{get_command_result, get_cstring, get_last_error, result_from_string}; /// The number of slots in a [`PasswordSafe`][]. /// diff --git a/src/util.rs b/src/util.rs index 567c478..88a381c 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,53 +1,11 @@ -use std::borrow; use std::ffi::{CStr, CString}; -use std::fmt; use std::os::raw::{c_char, c_int}; use libc::{c_void, free}; use rand_core::RngCore; use rand_os::OsRng; -/// Error types returned by Nitrokey device or by the library. -#[derive(Clone, Copy, Debug, PartialEq)] -pub enum CommandError { - /// A packet with a wrong checksum has been sent or received. - WrongCrc, - /// A command tried to access an OTP slot that does not exist. - WrongSlot, - /// A command tried to generate an OTP on a slot that is not configured. - SlotNotProgrammed, - /// The provided password is wrong. - WrongPassword, - /// You are not authorized for this command or provided a wrong temporary - /// password. - NotAuthorized, - /// An error occurred when getting or setting the time. - Timestamp, - /// You did not provide a name for the OTP slot. - NoName, - /// This command is not supported by this device. - NotSupported, - /// This command is unknown. - UnknownCommand, - /// AES decryption failed. - AesDecryptionFailed, - /// An unknown error occurred. - Unknown(i64), - /// 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, - /// An error occurred during random number generation. - RngError, -} +use crate::error::CommandError; /// Log level for libnitrokey. /// @@ -123,75 +81,6 @@ pub fn get_cstring>>(s: T) -> Result { CString::new(s).or(Err(CommandError::InvalidString)) } -impl CommandError { - fn as_str(&self) -> borrow::Cow<'static, str> { - match *self { - CommandError::WrongCrc => { - "A packet with a wrong checksum has been sent or received".into() - } - CommandError::WrongSlot => "The given OTP slot does not exist".into(), - CommandError::SlotNotProgrammed => "The given OTP slot is not programmed".into(), - CommandError::WrongPassword => "The given password is wrong".into(), - CommandError::NotAuthorized => { - "You are not authorized for this command or provided a wrong temporary \ - password" - .into() - } - CommandError::Timestamp => "An error occurred when getting or setting the time".into(), - CommandError::NoName => "You did not provide a name for the OTP slot".into(), - CommandError::NotSupported => "This command is not supported by this device".into(), - CommandError::UnknownCommand => "This command is unknown".into(), - CommandError::AesDecryptionFailed => "AES decryption failed".into(), - CommandError::Unknown(x) => { - borrow::Cow::from(format!("An unknown error occurred ({})", x)) - } - 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(), - CommandError::RngError => "An error occurred during random number generation".into(), - } - } -} - -impl fmt::Display for CommandError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.as_str()) - } -} - -impl From for CommandError { - fn from(value: c_int) -> Self { - match value { - 1 => CommandError::WrongCrc, - 2 => CommandError::WrongSlot, - 3 => CommandError::SlotNotProgrammed, - 4 => CommandError::WrongPassword, - 5 => CommandError::NotAuthorized, - 6 => CommandError::Timestamp, - 7 => CommandError::NoName, - 8 => CommandError::NotSupported, - 9 => CommandError::UnknownCommand, - 10 => CommandError::AesDecryptionFailed, - 200 => CommandError::StringTooLong, - 201 => CommandError::InvalidSlot, - 202 => CommandError::InvalidHexString, - 203 => CommandError::TargetBufferTooSmall, - x => CommandError::Unknown(x.into()), - } - } -} - -impl From for CommandError { - fn from(_error: rand_core::Error) -> Self { - CommandError::RngError - } -} - impl Into for LogLevel { fn into(self) -> i32 { match self { -- cgit v1.2.3 From 591c55ff294ee12812e4be1b90f03a093f83a4f5 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 01:58:42 +0000 Subject: Implement std::error::Error for error::CommandError --- CHANGELOG.md | 1 + src/error.rs | 3 +++ 2 files changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c5ebf3..d2982a9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ - Remove the `test-pro` and `test-storage` features. - Implement `Display` for `Version`. - Introduce `DEFAULT_ADMIN_PIN` and `DEFAULT_USER_PIN` constants. +- Implement `std::error::Error` for `CommandError`. # v0.3.4 (2019-01-20) - Fix authentication methods that assumed that `char` is signed. diff --git a/src/error.rs b/src/error.rs index 6aeeef8..dcaa4d2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,5 @@ use std::borrow; +use std::error; use std::fmt; use std::os::raw; @@ -79,6 +80,8 @@ impl CommandError { } } +impl error::Error for CommandError {} + impl fmt::Display for CommandError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", self.as_str()) -- cgit v1.2.3 From db198936be1a80f1735731d9e95eb6f4c48a5329 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 02:05:52 +0000 Subject: Add the Error enum and the Result typedef The Error enum is a wrapper for the possible error types (currently only CommandError). Result is defined as Result. --- CHANGELOG.md | 4 +++- src/error.rs | 35 ++++++++++++++++++++++++++++++++++- src/lib.rs | 2 +- 3 files changed, 38 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d2982a9..c69fd25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,9 @@ - Remove the `test-pro` and `test-storage` features. - Implement `Display` for `Version`. - Introduce `DEFAULT_ADMIN_PIN` and `DEFAULT_USER_PIN` constants. -- Implement `std::error::Error` for `CommandError`. +- Refactor the error handling code: + - Implement `std::error::Error` for `CommandError`. + - Add the `Error` enum and the `Result` typedef. # v0.3.4 (2019-01-20) - Fix authentication methods that assumed that `char` is signed. diff --git a/src/error.rs b/src/error.rs index dcaa4d2..89c4c82 100644 --- a/src/error.rs +++ b/src/error.rs @@ -2,8 +2,41 @@ use std::borrow; use std::error; use std::fmt; use std::os::raw; +use std::result; -/// Error types returned by Nitrokey device or by the library. +/// An error returned by the nitrokey crate. +#[derive(Debug)] +pub enum Error { + /// An error reported by the Nitrokey device in the response packet. + CommandError(CommandError), +} + +impl From for Error { + fn from(err: CommandError) -> Self { + Error::CommandError(err) + } +} + +impl error::Error for Error { + fn source(&self) -> Option<&(dyn error::Error + 'static)> { + match *self { + Error::CommandError(ref err) => Some(err), + } + } +} + +impl fmt::Display for Error { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match *self { + Error::CommandError(ref err) => write!(f, "Command error: {}", err), + } + } +} + +/// A result returned by the nitrokey crate. +pub type Result = result::Result; + +/// An error reported by the Nitrokey device in the response packet. #[derive(Clone, Copy, Debug, PartialEq)] pub enum CommandError { /// A packet with a wrong checksum has been sent or received. diff --git a/src/lib.rs b/src/lib.rs index cec5db7..df11cc3 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; +pub use crate::error::{CommandError, Error, Result}; pub use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData}; pub use crate::pws::{GetPasswordSafe, PasswordSafe, SLOT_COUNT}; pub use crate::util::LogLevel; -- cgit v1.2.3 From 94390aadc8a3997d379bf5e4c0bc00c2a9669a34 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 20 Jan 2019 20:58:18 +0000 Subject: Return Error instead of CommandError This patch changes all public functions to return the Error enum instead of the CommandError enum. This breaks the tests which will be fixed with the next patch. This patch also adds a placeholder variant Error::CommandError and a placeholder enum CommandError to make the transition to a new nitrokey-test version easier. --- CHANGELOG.md | 1 + src/auth.rs | 54 ++++++++--------- src/config.rs | 8 +-- src/device.rs | 182 +++++++++++++++++++++++++++++----------------------------- src/error.rs | 11 ++++ src/lib.rs | 14 ++--- src/otp.rs | 62 ++++++++++---------- src/pws.rs | 62 ++++++++++---------- src/util.rs | 24 ++++---- 9 files changed, 215 insertions(+), 203 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c69fd25..bbb3202 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Refactor the error handling code: - Implement `std::error::Error` for `CommandError`. - Add the `Error` enum and the `Result` typedef. + - Return `Error` instead of `CommandError` in all public functions. # v0.3.4 (2019-01-20) - Fix authentication methods that assumed that `char` is signed. diff --git a/src/auth.rs b/src/auth.rs index e805e54..509d3aa 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -6,7 +6,7 @@ use nitrokey_sys; use crate::config::{Config, RawConfig}; use crate::device::{Device, DeviceWrapper, Pro, Storage}; -use crate::error::CommandError; +use crate::error::{CommandError, Error}; use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData, RawOtpSlotData}; use crate::util::{generate_password, get_command_result, get_cstring, result_from_string}; @@ -33,12 +33,12 @@ pub trait Authenticate { /// /// ```no_run /// use nitrokey::{Authenticate, DeviceWrapper, User}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// /// fn perform_user_task(device: &User) {} /// fn perform_other_task(device: &DeviceWrapper) {} /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let device = match device.authenticate_user("123456") { /// Ok(user) => { @@ -58,7 +58,7 @@ pub trait Authenticate { /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn authenticate_user(self, password: &str) -> Result, (Self, CommandError)> + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> where Self: Device + Sized; @@ -79,12 +79,12 @@ pub trait Authenticate { /// /// ```no_run /// use nitrokey::{Authenticate, Admin, DeviceWrapper}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// /// fn perform_admin_task(device: &Admin) {} /// fn perform_other_task(device: &DeviceWrapper) {} /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let device = match device.authenticate_admin("123456") { /// Ok(admin) => { @@ -104,7 +104,7 @@ pub trait Authenticate { /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn authenticate_admin(self, password: &str) -> Result, (Self, CommandError)> + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> where Self: Device + Sized; } @@ -143,7 +143,7 @@ pub struct Admin { temp_password: Vec, } -fn authenticate(device: D, password: &str, callback: T) -> Result +fn authenticate(device: D, password: &str, callback: T) -> Result where D: Device, A: AuthenticatedDevice, @@ -161,7 +161,7 @@ where let temp_password_ptr = temp_password.as_ptr() as *const c_char; return match callback(password_ptr, temp_password_ptr) { 0 => Ok(A::new(device, temp_password)), - rv => Err((device, CommandError::from(rv))), + rv => Err((device, CommandError::from(rv).into())), }; } @@ -169,7 +169,7 @@ fn authenticate_user_wrapper( device: T, constructor: C, password: &str, -) -> Result, (DeviceWrapper, CommandError)> +) -> Result, (DeviceWrapper, Error)> where T: Device, C: Fn(T) -> DeviceWrapper, @@ -185,7 +185,7 @@ fn authenticate_admin_wrapper( device: T, constructor: C, password: &str, -) -> Result, (DeviceWrapper, CommandError)> +) -> Result, (DeviceWrapper, Error)> where T: Device, C: Fn(T) -> DeviceWrapper, @@ -215,14 +215,14 @@ impl Deref for User { } impl GenerateOtp for User { - fn get_hotp_code(&self, slot: u8) -> Result { + fn get_hotp_code(&self, slot: u8) -> Result { unsafe { let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; return result_from_string(nitrokey_sys::NK_get_hotp_code_PIN(slot, temp_password_ptr)); } } - fn get_totp_code(&self, slot: u8) -> Result { + fn get_totp_code(&self, slot: u8) -> Result { unsafe { let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; return result_from_string(nitrokey_sys::NK_get_totp_code_PIN( @@ -271,9 +271,9 @@ impl Admin { /// /// ```no_run /// use nitrokey::{Authenticate, Config}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let config = Config::new(None, None, None, false); /// match device.authenticate_admin("12345678") { @@ -288,7 +288,7 @@ impl Admin { /// ``` /// /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot - pub fn write_config(&self, config: Config) -> Result<(), CommandError> { + pub fn write_config(&self, config: Config) -> Result<(), Error> { let raw_config = RawConfig::try_from(config)?; unsafe { get_command_result(nitrokey_sys::NK_write_config( @@ -302,7 +302,7 @@ impl Admin { } } - fn write_otp_slot(&self, data: OtpSlotData, callback: C) -> Result<(), CommandError> + fn write_otp_slot(&self, data: OtpSlotData, callback: C) -> Result<(), Error> where C: Fn(RawOtpSlotData, *const c_char) -> c_int, { @@ -313,7 +313,7 @@ impl Admin { } impl ConfigureOtp for Admin { - fn write_hotp_slot(&self, data: OtpSlotData, counter: u64) -> Result<(), CommandError> { + fn write_hotp_slot(&self, data: OtpSlotData, counter: u64) -> Result<(), Error> { self.write_otp_slot(data, |raw_data: RawOtpSlotData, temp_password_ptr| unsafe { nitrokey_sys::NK_write_hotp_slot( raw_data.number, @@ -329,7 +329,7 @@ impl ConfigureOtp for Admin { }) } - fn write_totp_slot(&self, data: OtpSlotData, time_window: u16) -> Result<(), CommandError> { + fn write_totp_slot(&self, data: OtpSlotData, time_window: u16) -> Result<(), Error> { self.write_otp_slot(data, |raw_data: RawOtpSlotData, temp_password_ptr| unsafe { nitrokey_sys::NK_write_totp_slot( raw_data.number, @@ -345,12 +345,12 @@ impl ConfigureOtp for Admin { }) } - fn erase_hotp_slot(&self, slot: u8) -> Result<(), CommandError> { + fn erase_hotp_slot(&self, slot: u8) -> Result<(), Error> { let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; unsafe { get_command_result(nitrokey_sys::NK_erase_hotp_slot(slot, temp_password_ptr)) } } - fn erase_totp_slot(&self, slot: u8) -> Result<(), CommandError> { + fn erase_totp_slot(&self, slot: u8) -> Result<(), Error> { let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; unsafe { get_command_result(nitrokey_sys::NK_erase_totp_slot(slot, temp_password_ptr)) } } @@ -366,7 +366,7 @@ impl AuthenticatedDevice for Admin { } impl Authenticate for DeviceWrapper { - fn authenticate_user(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { match self { DeviceWrapper::Storage(storage) => { authenticate_user_wrapper(storage, DeviceWrapper::Storage, password) @@ -375,7 +375,7 @@ impl Authenticate for DeviceWrapper { } } - fn authenticate_admin(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { match self { DeviceWrapper::Storage(storage) => { authenticate_admin_wrapper(storage, DeviceWrapper::Storage, password) @@ -388,13 +388,13 @@ impl Authenticate for DeviceWrapper { } impl Authenticate for Pro { - fn authenticate_user(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) }) } - fn authenticate_admin(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_first_authenticate(password_ptr, temp_password_ptr) }) @@ -402,13 +402,13 @@ impl Authenticate for Pro { } impl Authenticate for Storage { - fn authenticate_user(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) }) } - fn authenticate_admin(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_first_authenticate(password_ptr, temp_password_ptr) }) diff --git a/src/config.rs b/src/config.rs index 277dc5e..741d67e 100644 --- a/src/config.rs +++ b/src/config.rs @@ -1,4 +1,4 @@ -use crate::error::CommandError; +use crate::error::{CommandError, Error}; /// The configuration for a Nitrokey. #[derive(Clone, Copy, Debug, PartialEq)] @@ -35,13 +35,13 @@ fn config_otp_slot_to_option(value: u8) -> Option { None } -fn option_to_config_otp_slot(value: Option) -> Result { +fn option_to_config_otp_slot(value: Option) -> Result { match value { Some(value) => { if value < 3 { Ok(value) } else { - Err(CommandError::InvalidSlot) + Err(CommandError::InvalidSlot.into()) } } None => Ok(255), @@ -66,7 +66,7 @@ impl Config { } impl RawConfig { - pub fn try_from(config: Config) -> Result { + pub fn try_from(config: Config) -> Result { Ok(RawConfig { numlock: option_to_config_otp_slot(config.numlock)?, capslock: option_to_config_otp_slot(config.capslock)?, diff --git a/src/device.rs b/src/device.rs index 603a986..ccd0597 100644 --- a/src/device.rs +++ b/src/device.rs @@ -5,7 +5,7 @@ use nitrokey_sys; use crate::auth::Authenticate; use crate::config::{Config, RawConfig}; -use crate::error::CommandError; +use crate::error::{CommandError, Error}; use crate::otp::GenerateOtp; use crate::pws::GetPasswordSafe; use crate::util::{get_command_result, get_cstring, get_last_error, result_from_string}; @@ -63,12 +63,12 @@ impl fmt::Display for VolumeMode { /// /// ```no_run /// use nitrokey::{Authenticate, DeviceWrapper, User}; -/// # use nitrokey::CommandError; +/// # use nitrokey::Error; /// /// fn perform_user_task(device: &User) {} /// fn perform_other_task(device: &DeviceWrapper) {} /// -/// # fn try_main() -> Result<(), CommandError> { +/// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let device = match device.authenticate_user("123456") { /// Ok(user) => { @@ -89,12 +89,12 @@ impl fmt::Display for VolumeMode { /// /// ```no_run /// use nitrokey::{DeviceWrapper, Storage}; -/// # use nitrokey::CommandError; +/// # use nitrokey::Error; /// /// fn perform_common_task(device: &DeviceWrapper) {} /// fn perform_storage_task(device: &Storage) {} /// -/// # fn try_main() -> Result<(), CommandError> { +/// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// perform_common_task(&device); /// match device { @@ -127,12 +127,12 @@ pub enum DeviceWrapper { /// /// ```no_run /// use nitrokey::{Authenticate, User, Pro}; -/// # use nitrokey::CommandError; +/// # use nitrokey::Error; /// /// fn perform_user_task(device: &User) {} /// fn perform_other_task(device: &Pro) {} /// -/// # fn try_main() -> Result<(), CommandError> { +/// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Pro::connect()?; /// let device = match device.authenticate_user("123456") { /// Ok(user) => { @@ -169,12 +169,12 @@ pub struct Pro {} /// /// ```no_run /// use nitrokey::{Authenticate, User, Storage}; -/// # use nitrokey::CommandError; +/// # use nitrokey::Error; /// /// fn perform_user_task(device: &User) {} /// fn perform_other_task(device: &Storage) {} /// -/// # fn try_main() -> Result<(), CommandError> { +/// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// let device = match device.authenticate_user("123456") { /// Ok(user) => { @@ -293,9 +293,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// println!("Connected to a Nitrokey {}", device.get_model()); /// # Ok(()) @@ -309,9 +309,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.get_serial_number() { /// Ok(number) => println!("serial no: {}", number), @@ -320,7 +320,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// # Ok(()) /// # } /// ``` - fn get_serial_number(&self) -> Result { + fn get_serial_number(&self) -> Result { unsafe { result_from_string(nitrokey_sys::NK_device_serial_number()) } } @@ -331,9 +331,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let count = device.get_user_retry_count(); /// println!("{} remaining authentication attempts (user)", count); @@ -351,9 +351,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let count = device.get_admin_retry_count(); /// println!("{} remaining authentication attempts (admin)", count); @@ -370,9 +370,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// println!( /// "Firmware version: {}.{}", @@ -392,9 +392,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// println!( /// "Firmware version: {}.{}", @@ -413,9 +413,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let config = device.get_config()?; /// println!("numlock binding: {:?}", config.numlock); @@ -425,7 +425,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// # Ok(()) /// # } /// ``` - fn get_config(&self) -> Result { + fn get_config(&self) -> Result { unsafe { let config_ptr = nitrokey_sys::NK_read_config(); if config_ptr.is_null() { @@ -449,9 +449,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.change_admin_pin("12345678", "12345679") { /// Ok(()) => println!("Updated admin PIN."), @@ -463,7 +463,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn change_admin_pin(&self, current: &str, new: &str) -> Result<(), CommandError> { + fn change_admin_pin(&self, current: &str, new: &str) -> Result<(), Error> { let current_string = get_cstring(current)?; let new_string = get_cstring(new)?; unsafe { @@ -485,9 +485,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.change_user_pin("123456", "123457") { /// Ok(()) => println!("Updated admin PIN."), @@ -499,7 +499,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn change_user_pin(&self, current: &str, new: &str) -> Result<(), CommandError> { + fn change_user_pin(&self, current: &str, new: &str) -> Result<(), Error> { let current_string = get_cstring(current)?; let new_string = get_cstring(new)?; unsafe { @@ -521,9 +521,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.unlock_user_pin("12345678", "123456") { /// Ok(()) => println!("Unlocked user PIN."), @@ -535,7 +535,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn unlock_user_pin(&self, admin_pin: &str, user_pin: &str) -> Result<(), CommandError> { + fn unlock_user_pin(&self, admin_pin: &str, user_pin: &str) -> Result<(), Error> { let admin_pin_string = get_cstring(admin_pin)?; let user_pin_string = get_cstring(user_pin)?; unsafe { @@ -555,9 +555,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.lock() { /// Ok(()) => println!("Locked the Nitrokey device."), @@ -566,7 +566,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// # Ok(()) /// # } /// ``` - fn lock(&self) -> Result<(), CommandError> { + fn lock(&self) -> Result<(), Error> { unsafe { get_command_result(nitrokey_sys::NK_lock_device()) } } @@ -586,9 +586,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.factory_reset("12345678") { /// Ok(()) => println!("Performed a factory reset."), @@ -599,7 +599,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// ``` /// /// [`build_aes_key`]: #method.build_aes_key - fn factory_reset(&self, admin_pin: &str) -> Result<(), CommandError> { + fn factory_reset(&self, admin_pin: &str) -> Result<(), Error> { let admin_pin_string = get_cstring(admin_pin)?; unsafe { get_command_result(nitrokey_sys::NK_factory_reset(admin_pin_string.as_ptr())) } } @@ -620,9 +620,9 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// ```no_run /// use nitrokey::Device; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.build_aes_key("12345678") { /// Ok(()) => println!("New AES keys have been built."), @@ -633,7 +633,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// ``` /// /// [`factory_reset`]: #method.factory_reset - fn build_aes_key(&self, admin_pin: &str) -> Result<(), CommandError> { + fn build_aes_key(&self, admin_pin: &str) -> Result<(), Error> { let admin_pin_string = get_cstring(admin_pin)?; unsafe { get_command_result(nitrokey_sys::NK_build_aes_key(admin_pin_string.as_ptr())) } } @@ -660,14 +660,14 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// ``` /// /// [`Undefined`]: enum.CommandError.html#variant.Undefined -pub fn connect() -> Result { +pub fn connect() -> Result { unsafe { match nitrokey_sys::NK_login_auto() { 1 => match get_connected_device() { Some(wrapper) => Ok(wrapper), - None => Err(CommandError::Undefined), + None => Err(CommandError::Undefined.into()), }, - _ => Err(CommandError::Undefined), + _ => Err(CommandError::Undefined.into()), } } } @@ -693,11 +693,11 @@ pub fn connect() -> Result { /// ``` /// /// [`Undefined`]: enum.CommandError.html#variant.Undefined -pub fn connect_model(model: Model) -> Result { +pub fn connect_model(model: Model) -> Result { if connect_enum(model) { Ok(create_device_wrapper(model)) } else { - Err(CommandError::Undefined) + Err(CommandError::Undefined.into()) } } @@ -740,19 +740,19 @@ impl DeviceWrapper { } impl GenerateOtp for DeviceWrapper { - fn get_hotp_slot_name(&self, slot: u8) -> Result { + fn get_hotp_slot_name(&self, slot: u8) -> Result { self.device().get_hotp_slot_name(slot) } - fn get_totp_slot_name(&self, slot: u8) -> Result { + fn get_totp_slot_name(&self, slot: u8) -> Result { self.device().get_totp_slot_name(slot) } - fn get_hotp_code(&self, slot: u8) -> Result { + fn get_hotp_code(&self, slot: u8) -> Result { self.device().get_hotp_code(slot) } - fn get_totp_code(&self, slot: u8) -> Result { + fn get_totp_code(&self, slot: u8) -> Result { self.device().get_totp_code(slot) } } @@ -787,11 +787,11 @@ impl Pro { /// ``` /// /// [`Undefined`]: enum.CommandError.html#variant.Undefined - pub fn connect() -> Result { + pub fn connect() -> Result { // TODO: maybe Option instead of Result? match connect_enum(Model::Pro) { true => Ok(Pro {}), - false => Err(CommandError::Undefined), + false => Err(CommandError::Undefined.into()), } } } @@ -833,11 +833,11 @@ impl Storage { /// ``` /// /// [`Undefined`]: enum.CommandError.html#variant.Undefined - pub fn connect() -> Result { + pub fn connect() -> Result { // TODO: maybe Option instead of Result? match connect_enum(Model::Storage) { true => Ok(Storage {}), - false => Err(CommandError::Undefined), + false => Err(CommandError::Undefined.into()), } } @@ -855,9 +855,9 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// match device.change_update_pin("12345678", "87654321") { /// Ok(()) => println!("Updated update PIN."), @@ -869,7 +869,7 @@ impl Storage { /// /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - pub fn change_update_pin(&self, current: &str, new: &str) -> Result<(), CommandError> { + pub fn change_update_pin(&self, current: &str, new: &str) -> Result<(), Error> { let current_string = get_cstring(current)?; let new_string = get_cstring(new)?; unsafe { @@ -895,9 +895,9 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// match device.enable_firmware_update("12345678") { /// Ok(()) => println!("Nitrokey entered update mode."), @@ -909,7 +909,7 @@ impl Storage { /// /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - pub fn enable_firmware_update(&self, update_pin: &str) -> Result<(), CommandError> { + pub fn enable_firmware_update(&self, update_pin: &str) -> Result<(), Error> { let update_pin_string = get_cstring(update_pin)?; unsafe { get_command_result(nitrokey_sys::NK_enable_firmware_update( @@ -931,9 +931,9 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// match device.enable_encrypted_volume("123456") { /// Ok(()) => println!("Enabled the encrypted volume."), @@ -945,7 +945,7 @@ impl Storage { /// /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - pub fn enable_encrypted_volume(&self, user_pin: &str) -> Result<(), CommandError> { + pub fn enable_encrypted_volume(&self, user_pin: &str) -> Result<(), Error> { let user_pin = get_cstring(user_pin)?; unsafe { get_command_result(nitrokey_sys::NK_unlock_encrypted_volume(user_pin.as_ptr())) } } @@ -958,11 +958,11 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// /// fn use_volume() {} /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// match device.enable_encrypted_volume("123456") { /// Ok(()) => { @@ -980,7 +980,7 @@ impl Storage { /// # Ok(()) /// # } /// ``` - pub fn disable_encrypted_volume(&self) -> Result<(), CommandError> { + pub fn disable_encrypted_volume(&self) -> Result<(), Error> { unsafe { get_command_result(nitrokey_sys::NK_lock_encrypted_volume()) } } @@ -1006,9 +1006,9 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// device.enable_encrypted_volume("123445")?; /// match device.enable_hidden_volume("hidden-pw") { @@ -1022,7 +1022,7 @@ impl Storage { /// [`enable_encrypted_volume`]: #method.enable_encrypted_volume /// [`AesDecryptionFailed`]: enum.CommandError.html#variant.AesDecryptionFailed /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString - pub fn enable_hidden_volume(&self, volume_password: &str) -> Result<(), CommandError> { + pub fn enable_hidden_volume(&self, volume_password: &str) -> Result<(), Error> { let volume_password = get_cstring(volume_password)?; unsafe { get_command_result(nitrokey_sys::NK_unlock_hidden_volume( @@ -1039,11 +1039,11 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// /// fn use_volume() {} /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// device.enable_encrypted_volume("123445")?; /// match device.enable_hidden_volume("hidden-pw") { @@ -1062,7 +1062,7 @@ impl Storage { /// # Ok(()) /// # } /// ``` - pub fn disable_hidden_volume(&self) -> Result<(), CommandError> { + pub fn disable_hidden_volume(&self) -> Result<(), Error> { unsafe { get_command_result(nitrokey_sys::NK_lock_hidden_volume()) } } @@ -1088,9 +1088,9 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// device.enable_encrypted_volume("123445")?; /// device.create_hidden_volume(0, 0, 100, "hidden-pw")?; @@ -1106,7 +1106,7 @@ impl Storage { start: u8, end: u8, password: &str, - ) -> Result<(), CommandError> { + ) -> Result<(), Error> { let password = get_cstring(password)?; unsafe { get_command_result(nitrokey_sys::NK_create_hidden_volume( @@ -1132,10 +1132,10 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// use nitrokey::VolumeMode; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// match device.set_unencrypted_volume_mode("123456", VolumeMode::ReadWrite) { /// Ok(()) => println!("Set the unencrypted volume to read-write mode."), @@ -1151,7 +1151,7 @@ impl Storage { &self, admin_pin: &str, mode: VolumeMode, - ) -> Result<(), CommandError> { + ) -> Result<(), Error> { let admin_pin = get_cstring(admin_pin)?; let result = match mode { VolumeMode::ReadOnly => unsafe { @@ -1169,11 +1169,11 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// /// fn use_volume() {} /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// match device.get_status() { /// Ok(status) => { @@ -1184,7 +1184,7 @@ impl Storage { /// # Ok(()) /// # } /// ``` - pub fn get_status(&self) -> Result { + pub fn get_status(&self) -> Result { let mut raw_status = nitrokey_sys::NK_storage_status { unencrypted_volume_read_only: false, unencrypted_volume_active: false, @@ -1213,11 +1213,11 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// /// fn use_volume() {} /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// match device.get_production_info() { /// Ok(data) => { @@ -1229,7 +1229,7 @@ impl Storage { /// # Ok(()) /// # } /// ``` - pub fn get_production_info(&self) -> Result { + pub fn get_production_info(&self) -> Result { let mut raw_data = nitrokey_sys::NK_storage_ProductionTest { FirmwareVersion_au8: [0, 2], FirmwareVersionInternal_u8: 0, @@ -1264,9 +1264,9 @@ impl Storage { /// # Example /// /// ```no_run - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::Storage::connect()?; /// match device.clear_new_sd_card_warning("12345678") { /// Ok(()) => println!("Cleared the new SD card warning."), @@ -1278,7 +1278,7 @@ impl Storage { /// /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - pub fn clear_new_sd_card_warning(&self, admin_pin: &str) -> Result<(), CommandError> { + pub fn clear_new_sd_card_warning(&self, admin_pin: &str) -> Result<(), Error> { let admin_pin = get_cstring(admin_pin)?; get_command_result(unsafe { nitrokey_sys::NK_clear_new_sd_card_warning(admin_pin.as_ptr()) @@ -1286,7 +1286,7 @@ impl Storage { } /// Blinks the red and green LED alternatively and infinitely until the device is reconnected. - pub fn wink(&self) -> Result<(), CommandError> { + pub fn wink(&self) -> Result<(), Error> { get_command_result(unsafe { nitrokey_sys::NK_wink() }) } @@ -1306,7 +1306,7 @@ impl Storage { /// /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - pub fn export_firmware(&self, admin_pin: &str) -> Result<(), CommandError> { + pub fn export_firmware(&self, admin_pin: &str) -> Result<(), Error> { let admin_pin_string = get_cstring(admin_pin)?; get_command_result(unsafe { nitrokey_sys::NK_export_firmware(admin_pin_string.as_ptr()) }) } diff --git a/src/error.rs b/src/error.rs index 89c4c82..3f60af2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -9,6 +9,8 @@ use std::result; pub enum Error { /// An error reported by the Nitrokey device in the response packet. CommandError(CommandError), + /// Placeholder for testing. + CommunicationError(CommunicationError), } impl From for Error { @@ -21,6 +23,7 @@ impl error::Error for Error { fn source(&self) -> Option<&(dyn error::Error + 'static)> { match *self { Error::CommandError(ref err) => Some(err), + Error::CommunicationError(_) => None, } } } @@ -29,6 +32,7 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { Error::CommandError(ref err) => write!(f, "Command error: {}", err), + Error::CommunicationError(_) => write!(f, "Placeholder"), } } } @@ -78,6 +82,13 @@ pub enum CommandError { RngError, } +/// Placeholder for testing. +#[derive(Debug)] +pub enum CommunicationError { + /// Placeholder for testing. + NotConnected, +} + impl CommandError { fn as_str(&self) -> borrow::Cow<'static, str> { match *self { diff --git a/src/lib.rs b/src/lib.rs index df11cc3..8522e83 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -25,9 +25,9 @@ //! //! ```no_run //! use nitrokey::Device; -//! # use nitrokey::CommandError; +//! # use nitrokey::Error; //! -//! # fn try_main() -> Result<(), CommandError> { +//! # fn try_main() -> Result<(), Error> { //! let device = nitrokey::connect()?; //! println!("{}", device.get_serial_number()?); //! # Ok(()) @@ -38,9 +38,9 @@ //! //! ```no_run //! use nitrokey::{Authenticate, ConfigureOtp, OtpMode, OtpSlotData}; -//! # use nitrokey::CommandError; +//! # use nitrokey::Error; //! -//! # fn try_main() -> Result<(), (CommandError)> { +//! # fn try_main() -> Result<(), Error> { //! let device = nitrokey::connect()?; //! let slot_data = OtpSlotData::new(1, "test", "01234567890123456689", OtpMode::SixDigits); //! match device.authenticate_admin("12345678") { @@ -60,9 +60,9 @@ //! //! ```no_run //! use nitrokey::{Device, GenerateOtp}; -//! # use nitrokey::CommandError; +//! # use nitrokey::Error; //! -//! # fn try_main() -> Result<(), (CommandError)> { +//! # fn try_main() -> Result<(), Error> { //! let device = nitrokey::connect()?; //! match device.get_hotp_code(1) { //! Ok(code) => println!("Generated HOTP code: {}", code), @@ -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, Error, Result}; +pub use crate::error::{CommandError, CommunicationError, Error, 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 784149a..5dfe8b1 100644 --- a/src/otp.rs +++ b/src/otp.rs @@ -2,7 +2,7 @@ use std::ffi::CString; use nitrokey_sys; -use crate::error::CommandError; +use crate::error::Error; use crate::util::{get_command_result, get_cstring, result_from_string}; /// Modes for one-time password generation. @@ -29,9 +29,9 @@ pub trait ConfigureOtp { /// /// ```no_run /// use nitrokey::{Authenticate, ConfigureOtp, OtpMode, OtpSlotData}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), (CommandError)> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let slot_data = OtpSlotData::new(1, "test", "01234567890123456689", OtpMode::SixDigits); /// match device.authenticate_admin("12345678") { @@ -50,7 +50,7 @@ pub trait ConfigureOtp { /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`NoName`]: enum.CommandError.html#variant.NoName - fn write_hotp_slot(&self, data: OtpSlotData, counter: u64) -> Result<(), CommandError>; + fn write_hotp_slot(&self, data: OtpSlotData, counter: u64) -> Result<(), Error>; /// Configure a TOTP slot with the given data and set the TOTP time window to the given value /// (default 30). @@ -65,9 +65,9 @@ pub trait ConfigureOtp { /// /// ```no_run /// use nitrokey::{Authenticate, ConfigureOtp, OtpMode, OtpSlotData}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), (CommandError)> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let slot_data = OtpSlotData::new(1, "test", "01234567890123456689", OtpMode::EightDigits); /// match device.authenticate_admin("12345678") { @@ -86,7 +86,7 @@ pub trait ConfigureOtp { /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`NoName`]: enum.CommandError.html#variant.NoName - fn write_totp_slot(&self, data: OtpSlotData, time_window: u16) -> Result<(), CommandError>; + fn write_totp_slot(&self, data: OtpSlotData, time_window: u16) -> Result<(), Error>; /// Erases an HOTP slot. /// @@ -98,9 +98,9 @@ pub trait ConfigureOtp { /// /// ```no_run /// use nitrokey::{Authenticate, ConfigureOtp}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), (CommandError)> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.authenticate_admin("12345678") { /// Ok(admin) => { @@ -116,7 +116,7 @@ pub trait ConfigureOtp { /// ``` /// /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot - fn erase_hotp_slot(&self, slot: u8) -> Result<(), CommandError>; + fn erase_hotp_slot(&self, slot: u8) -> Result<(), Error>; /// Erases a TOTP slot. /// @@ -128,9 +128,9 @@ pub trait ConfigureOtp { /// /// ```no_run /// use nitrokey::{Authenticate, ConfigureOtp}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), (CommandError)> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.authenticate_admin("12345678") { /// Ok(admin) => { @@ -146,7 +146,7 @@ pub trait ConfigureOtp { /// ``` /// /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot - fn erase_totp_slot(&self, slot: u8) -> Result<(), CommandError>; + fn erase_totp_slot(&self, slot: u8) -> Result<(), Error>; } /// Provides methods to generate OTP codes and to query OTP slots on a Nitrokey @@ -165,9 +165,9 @@ pub trait GenerateOtp { /// ```no_run /// use std::time; /// use nitrokey::GenerateOtp; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let time = time::SystemTime::now().duration_since(time::UNIX_EPOCH); /// match time { @@ -184,7 +184,7 @@ pub trait GenerateOtp { /// /// [`get_totp_code`]: #method.get_totp_code /// [`Timestamp`]: enum.CommandError.html#variant.Timestamp - fn set_time(&self, time: u64, force: bool) -> Result<(), CommandError> { + fn set_time(&self, time: u64, force: bool) -> Result<(), Error> { let result = if force { unsafe { nitrokey_sys::NK_totp_set_time(time) } } else { @@ -203,13 +203,13 @@ pub trait GenerateOtp { /// # Example /// /// ```no_run - /// use nitrokey::{CommandError, GenerateOtp}; + /// use nitrokey::{CommandError, Error, GenerateOtp}; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.get_hotp_slot_name(1) { /// Ok(name) => println!("HOTP slot 1: {}", name), - /// Err(CommandError::SlotNotProgrammed) => println!("HOTP slot 1 not programmed"), + /// Err(Error::CommandError(CommandError::SlotNotProgrammed)) => println!("HOTP slot 1 not programmed"), /// Err(err) => println!("Could not get slot name: {}", err), /// }; /// # Ok(()) @@ -218,7 +218,7 @@ pub trait GenerateOtp { /// /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed - fn get_hotp_slot_name(&self, slot: u8) -> Result { + fn get_hotp_slot_name(&self, slot: u8) -> Result { unsafe { result_from_string(nitrokey_sys::NK_get_hotp_slot_name(slot)) } } @@ -232,13 +232,13 @@ pub trait GenerateOtp { /// # Example /// /// ```no_run - /// use nitrokey::{CommandError, GenerateOtp}; + /// use nitrokey::{CommandError, Error, GenerateOtp}; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.get_totp_slot_name(1) { /// Ok(name) => println!("TOTP slot 1: {}", name), - /// Err(CommandError::SlotNotProgrammed) => println!("TOTP slot 1 not programmed"), + /// Err(Error::CommandError(CommandError::SlotNotProgrammed)) => println!("TOTP slot 1 not programmed"), /// Err(err) => println!("Could not get slot name: {}", err), /// }; /// # Ok(()) @@ -247,7 +247,7 @@ pub trait GenerateOtp { /// /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed - fn get_totp_slot_name(&self, slot: u8) -> Result { + fn get_totp_slot_name(&self, slot: u8) -> Result { unsafe { result_from_string(nitrokey_sys::NK_get_totp_slot_name(slot)) } } @@ -264,9 +264,9 @@ pub trait GenerateOtp { /// /// ```no_run /// use nitrokey::GenerateOtp; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let code = device.get_hotp_code(1)?; /// println!("Generated HOTP code on slot 1: {}", code); @@ -278,7 +278,7 @@ pub trait GenerateOtp { /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot /// [`NotAuthorized`]: enum.CommandError.html#variant.NotAuthorized /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed - fn get_hotp_code(&self, slot: u8) -> Result { + fn get_hotp_code(&self, slot: u8) -> Result { unsafe { return result_from_string(nitrokey_sys::NK_get_hotp_code(slot)); } @@ -301,9 +301,9 @@ pub trait GenerateOtp { /// ```no_run /// use std::time; /// use nitrokey::GenerateOtp; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let time = time::SystemTime::now().duration_since(time::UNIX_EPOCH); /// match time { @@ -323,7 +323,7 @@ pub trait GenerateOtp { /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot /// [`NotAuthorized`]: enum.CommandError.html#variant.NotAuthorized /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed - fn get_totp_code(&self, slot: u8) -> Result { + fn get_totp_code(&self, slot: u8) -> Result { unsafe { return result_from_string(nitrokey_sys::NK_get_totp_code(slot, 0, 0, 0)); } @@ -396,7 +396,7 @@ impl OtpSlotData { } impl RawOtpSlotData { - pub fn new(data: OtpSlotData) -> Result { + pub fn new(data: OtpSlotData) -> Result { let name = get_cstring(data.name)?; let secret = get_cstring(data.secret)?; let use_token_id = data.token_id.is_some(); diff --git a/src/pws.rs b/src/pws.rs index 615e47c..e974737 100644 --- a/src/pws.rs +++ b/src/pws.rs @@ -2,7 +2,7 @@ use libc; use nitrokey_sys; use crate::device::{Device, DeviceWrapper, Pro, Storage}; -use crate::error::CommandError; +use crate::error::{CommandError, Error}; use crate::util::{get_command_result, get_cstring, get_last_error, result_from_string}; /// The number of slots in a [`PasswordSafe`][]. @@ -29,9 +29,9 @@ pub const SLOT_COUNT: u8 = 16; /// /// ```no_run /// use nitrokey::{Device, GetPasswordSafe, PasswordSafe}; -/// # use nitrokey::CommandError; +/// # use nitrokey::Error; /// -/// fn use_password_safe(pws: &PasswordSafe) -> Result<(), CommandError> { +/// fn use_password_safe(pws: &PasswordSafe) -> Result<(), Error> { /// let name = pws.get_slot_name(0)?; /// let login = pws.get_slot_login(0)?; /// let password = pws.get_slot_login(0)?; @@ -39,7 +39,7 @@ pub const SLOT_COUNT: u8 = 16; /// Ok(()) /// } /// -/// # fn try_main() -> Result<(), CommandError> { +/// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let pws = device.get_password_safe("123456")?; /// use_password_safe(&pws); @@ -88,11 +88,11 @@ pub trait GetPasswordSafe { /// /// ```no_run /// use nitrokey::{Device, GetPasswordSafe, PasswordSafe}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// /// fn use_password_safe(pws: &PasswordSafe) {} /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.get_password_safe("123456") { /// Ok(pws) => { @@ -112,13 +112,13 @@ pub trait GetPasswordSafe { /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`Unknown`]: enum.CommandError.html#variant.Unknown /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn get_password_safe(&self, user_pin: &str) -> Result, CommandError>; + fn get_password_safe(&self, user_pin: &str) -> Result, Error>; } fn get_password_safe<'a>( device: &'a dyn Device, user_pin: &str, -) -> Result, CommandError> { +) -> Result, Error> { let user_pin_string = get_cstring(user_pin)?; let result = unsafe { get_command_result(nitrokey_sys::NK_enable_password_safe( @@ -128,9 +128,9 @@ fn get_password_safe<'a>( result.map(|()| PasswordSafe { _device: device }) } -fn get_pws_result(s: String) -> Result { +fn get_pws_result(s: String) -> Result { if s.is_empty() { - Err(CommandError::SlotNotProgrammed) + Err(CommandError::SlotNotProgrammed.into()) } else { Ok(s) } @@ -145,9 +145,9 @@ impl<'a> PasswordSafe<'a> { /// /// ```no_run /// use nitrokey::{GetPasswordSafe, SLOT_COUNT}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let pws = device.get_password_safe("123456")?; /// pws.get_slot_status()?.iter().enumerate().for_each(|(slot, programmed)| { @@ -160,7 +160,7 @@ impl<'a> PasswordSafe<'a> { /// # Ok(()) /// # } /// ``` - pub fn get_slot_status(&self) -> Result<[bool; SLOT_COUNT as usize], CommandError> { + pub fn get_slot_status(&self) -> Result<[bool; SLOT_COUNT as usize], Error> { let status_ptr = unsafe { nitrokey_sys::NK_get_password_safe_slot_status() }; if status_ptr.is_null() { return Err(get_last_error()); @@ -190,9 +190,9 @@ impl<'a> PasswordSafe<'a> { /// /// ```no_run /// use nitrokey::GetPasswordSafe; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// match device.get_password_safe("123456") { /// Ok(pws) => { @@ -209,7 +209,7 @@ impl<'a> PasswordSafe<'a> { /// /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed - pub fn get_slot_name(&self, slot: u8) -> Result { + pub fn get_slot_name(&self, slot: u8) -> Result { unsafe { result_from_string(nitrokey_sys::NK_get_password_safe_slot_name(slot)) } .and_then(get_pws_result) } @@ -227,9 +227,9 @@ impl<'a> PasswordSafe<'a> { /// /// ```no_run /// use nitrokey::GetPasswordSafe; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let pws = device.get_password_safe("123456")?; /// let name = pws.get_slot_name(0)?; @@ -242,7 +242,7 @@ impl<'a> PasswordSafe<'a> { /// /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed - pub fn get_slot_login(&self, slot: u8) -> Result { + pub fn get_slot_login(&self, slot: u8) -> Result { unsafe { result_from_string(nitrokey_sys::NK_get_password_safe_slot_login(slot)) } .and_then(get_pws_result) } @@ -260,9 +260,9 @@ impl<'a> PasswordSafe<'a> { /// /// ```no_run /// use nitrokey::GetPasswordSafe; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let pws = device.get_password_safe("123456")?; /// let name = pws.get_slot_name(0)?; @@ -275,7 +275,7 @@ impl<'a> PasswordSafe<'a> { /// /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot /// [`SlotNotProgrammed`]: enum.CommandError.html#variant.SlotNotProgrammed - pub fn get_slot_password(&self, slot: u8) -> Result { + pub fn get_slot_password(&self, slot: u8) -> Result { unsafe { result_from_string(nitrokey_sys::NK_get_password_safe_slot_password(slot)) } .and_then(get_pws_result) } @@ -291,9 +291,9 @@ impl<'a> PasswordSafe<'a> { /// /// ```no_run /// use nitrokey::GetPasswordSafe; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let pws = device.get_password_safe("123456")?; /// let name = pws.get_slot_name(0)?; @@ -312,7 +312,7 @@ impl<'a> PasswordSafe<'a> { name: &str, login: &str, password: &str, - ) -> Result<(), CommandError> { + ) -> Result<(), Error> { let name_string = get_cstring(name)?; let login_string = get_cstring(login)?; let password_string = get_cstring(password)?; @@ -337,9 +337,9 @@ impl<'a> PasswordSafe<'a> { /// /// ```no_run /// use nitrokey::GetPasswordSafe; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let pws = device.get_password_safe("123456")?; /// match pws.erase_slot(0) { @@ -351,7 +351,7 @@ impl<'a> PasswordSafe<'a> { /// ``` /// /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot - pub fn erase_slot(&self, slot: u8) -> Result<(), CommandError> { + pub fn erase_slot(&self, slot: u8) -> Result<(), Error> { unsafe { get_command_result(nitrokey_sys::NK_erase_password_safe_slot(slot)) } } } @@ -364,19 +364,19 @@ impl<'a> Drop for PasswordSafe<'a> { } impl GetPasswordSafe for Pro { - fn get_password_safe(&self, user_pin: &str) -> Result, CommandError> { + fn get_password_safe(&self, user_pin: &str) -> Result, Error> { get_password_safe(self, user_pin) } } impl GetPasswordSafe for Storage { - fn get_password_safe(&self, user_pin: &str) -> Result, CommandError> { + fn get_password_safe(&self, user_pin: &str) -> Result, Error> { get_password_safe(self, user_pin) } } impl GetPasswordSafe for DeviceWrapper { - fn get_password_safe(&self, user_pin: &str) -> Result, CommandError> { + fn get_password_safe(&self, user_pin: &str) -> Result, Error> { get_password_safe(self, user_pin) } } diff --git a/src/util.rs b/src/util.rs index 88a381c..8855275 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; +use crate::error::{CommandError, Error}; /// Log level for libnitrokey. /// @@ -34,9 +34,9 @@ pub fn owned_str_from_ptr(ptr: *const c_char) -> String { } } -pub fn result_from_string(ptr: *const c_char) -> Result { +pub fn result_from_string(ptr: *const c_char) -> Result { if ptr.is_null() { - return Err(CommandError::Undefined); + return Err(CommandError::Undefined.into()); } unsafe { let s = owned_str_from_ptr(ptr); @@ -51,34 +51,34 @@ pub fn result_from_string(ptr: *const c_char) -> Result { } } -pub fn get_command_result(value: c_int) -> Result<(), CommandError> { +pub fn get_command_result(value: c_int) -> Result<(), Error> { match value { 0 => Ok(()), - other => Err(CommandError::from(other)), + other => Err(CommandError::from(other).into()), } } -pub fn get_last_result() -> Result<(), CommandError> { +pub fn get_last_result() -> Result<(), Error> { let value = unsafe { nitrokey_sys::NK_get_last_command_status() } as c_int; get_command_result(value) } -pub fn get_last_error() -> CommandError { +pub fn get_last_error() -> Error { return match get_last_result() { - Ok(()) => CommandError::Undefined, + Ok(()) => CommandError::Undefined.into(), Err(err) => err, }; } -pub fn generate_password(length: usize) -> Result, CommandError> { - let mut rng = OsRng::new()?; +pub fn generate_password(length: usize) -> Result, Error> { + let mut rng = OsRng::new().map_err(CommandError::from)?; let mut data = vec![0u8; length]; rng.fill_bytes(&mut data[..]); Ok(data) } -pub fn get_cstring>>(s: T) -> Result { - CString::new(s).or(Err(CommandError::InvalidString)) +pub fn get_cstring>>(s: T) -> Result { + CString::new(s).or(Err(CommandError::InvalidString.into())) } impl Into for LogLevel { -- cgit v1.2.3 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. --- Cargo.toml | 2 +- tests/device.rs | 157 +++++++++++++++++++++++++----------------------------- tests/otp.rs | 93 +++++++++++++++----------------- tests/pws.rs | 99 +++++++++++++++------------------- tests/util/mod.rs | 67 +++++++++++++++++++++++ 5 files changed, 226 insertions(+), 192 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 99ddd0c..cfe1579 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,4 +19,4 @@ rand_core = {version = "0.3", default-features = false} rand_os = {version = "0.1"} [dev-dependencies] -nitrokey-test = {version = "0.1"} +nitrokey-test = {version = "0.2"} diff --git a/tests/device.rs b/tests/device.rs index abede67..174624f 100644 --- a/tests/device.rs +++ b/tests/device.rs @@ -5,7 +5,7 @@ use std::process::Command; use std::{thread, time}; use nitrokey::{ - Authenticate, CommandError, Config, ConfigureOtp, Device, GenerateOtp, GetPasswordSafe, + Authenticate, CommandError, Config, ConfigureOtp, Device, Error, GenerateOtp, GetPasswordSafe, OtpMode, OtpSlotData, Storage, VolumeMode, }; use nitrokey_test::test as test_device; @@ -125,20 +125,20 @@ fn get_retry_count(device: DeviceWrapper) { fn config(device: DeviceWrapper) { let admin = device.authenticate_admin(ADMIN_PASSWORD).unwrap(); let config = Config::new(None, None, None, true); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); let get_config = admin.get_config().unwrap(); assert_eq!(config, get_config); let config = Config::new(None, Some(9), None, true); - assert_eq!(Err(CommandError::InvalidSlot), admin.write_config(config)); + assert_cmd_err!(CommandError::InvalidSlot, admin.write_config(config)); let config = Config::new(Some(1), None, Some(0), false); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); let get_config = admin.get_config().unwrap(); assert_eq!(config, get_config); let config = Config::new(None, None, None, false); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); let get_config = admin.get_config().unwrap(); assert_eq!(config, get_config); } @@ -159,7 +159,7 @@ fn change_user_pin(device: DeviceWrapper) { .device(); let result = device.change_user_pin(USER_PASSWORD, USER_PASSWORD); - assert_eq!(Err(CommandError::WrongPassword), result); + assert_cmd_err!(CommandError::WrongPassword, result); assert!(device .change_user_pin(USER_NEW_PASSWORD, USER_PASSWORD) @@ -184,8 +184,8 @@ fn change_admin_pin(device: DeviceWrapper) { .unwrap() .device(); - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.change_admin_pin(ADMIN_PASSWORD, ADMIN_PASSWORD) ); @@ -205,7 +205,10 @@ where let result = device.authenticate_user(password); assert!(result.is_err()); let err = result.unwrap_err(); - assert_eq!(error, err.1); + match err.1 { + Error::CommandError(err) => assert_eq!(error, err), + _ => assert!(false), + }; err.0 } @@ -215,8 +218,8 @@ fn unlock_user_pin(device: DeviceWrapper) { assert!(device .unlock_user_pin(ADMIN_PASSWORD, USER_PASSWORD) .is_ok()); - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD) ); @@ -228,8 +231,8 @@ fn unlock_user_pin(device: DeviceWrapper) { let device = require_failed_user_login(device, USER_PASSWORD, CommandError::WrongPassword); // unblock with current PIN - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD) ); assert!(device @@ -244,8 +247,8 @@ fn unlock_user_pin(device: DeviceWrapper) { let device = require_failed_user_login(device, USER_PASSWORD, CommandError::WrongPassword); // unblock with new PIN - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.unlock_user_pin(USER_PASSWORD, USER_PASSWORD) ); assert!(device @@ -262,39 +265,33 @@ fn unlock_user_pin(device: DeviceWrapper) { fn factory_reset(device: DeviceWrapper) { let admin = device.authenticate_admin(ADMIN_PASSWORD).unwrap(); let otp_data = OtpSlotData::new(1, "test", "0123468790", OtpMode::SixDigits); - assert_eq!(Ok(()), admin.write_totp_slot(otp_data, 30)); + assert_ok!((), admin.write_totp_slot(otp_data, 30)); let device = admin.device(); let pws = device.get_password_safe(USER_PASSWORD).unwrap(); - assert_eq!(Ok(()), pws.write_slot(0, "test", "testlogin", "testpw")); + assert_ok!((), pws.write_slot(0, "test", "testlogin", "testpw")); drop(pws); - assert_eq!( - Ok(()), - device.change_user_pin(USER_PASSWORD, USER_NEW_PASSWORD) - ); - assert_eq!( - Ok(()), + assert_ok!((), device.change_user_pin(USER_PASSWORD, USER_NEW_PASSWORD)); + assert_ok!( + (), device.change_admin_pin(ADMIN_PASSWORD, ADMIN_NEW_PASSWORD) ); - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.factory_reset(USER_NEW_PASSWORD) ); - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.factory_reset(ADMIN_PASSWORD) ); - assert_eq!(Ok(()), device.factory_reset(ADMIN_NEW_PASSWORD)); + assert_ok!((), device.factory_reset(ADMIN_NEW_PASSWORD)); let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap().device(); let user = device.authenticate_user(USER_PASSWORD).unwrap(); - assert_eq!( - Err(CommandError::SlotNotProgrammed), - user.get_totp_slot_name(1) - ); + assert_cmd_err!(CommandError::SlotNotProgrammed, user.get_totp_slot_name(1)); let device = user.device(); let pws = device.get_password_safe(USER_PASSWORD).unwrap(); @@ -302,20 +299,20 @@ fn factory_reset(device: DeviceWrapper) { assert_ne!("testlogin".to_string(), pws.get_slot_login(0).unwrap()); assert_ne!("testpw".to_string(), pws.get_slot_password(0).unwrap()); - assert_eq!(Ok(()), device.build_aes_key(ADMIN_PASSWORD)); + assert_ok!((), device.build_aes_key(ADMIN_PASSWORD)); } #[test_device] fn build_aes_key(device: DeviceWrapper) { let pws = device.get_password_safe(USER_PASSWORD).unwrap(); - assert_eq!(Ok(()), pws.write_slot(0, "test", "testlogin", "testpw")); + assert_ok!((), pws.write_slot(0, "test", "testlogin", "testpw")); drop(pws); - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.build_aes_key(USER_PASSWORD) ); - assert_eq!(Ok(()), device.build_aes_key(ADMIN_PASSWORD)); + assert_ok!((), device.build_aes_key(ADMIN_PASSWORD)); let device = device.authenticate_admin(ADMIN_PASSWORD).unwrap().device(); @@ -327,74 +324,71 @@ fn build_aes_key(device: DeviceWrapper) { #[test_device] fn change_update_pin(device: Storage) { - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.change_update_pin(UPDATE_NEW_PIN, UPDATE_PIN) ); - assert_eq!(Ok(()), device.change_update_pin(UPDATE_PIN, UPDATE_NEW_PIN)); - assert_eq!(Ok(()), device.change_update_pin(UPDATE_NEW_PIN, UPDATE_PIN)); + assert_ok!((), device.change_update_pin(UPDATE_PIN, UPDATE_NEW_PIN)); + assert_ok!((), device.change_update_pin(UPDATE_NEW_PIN, UPDATE_PIN)); } #[test_device] fn encrypted_volume(device: Storage) { - assert_eq!(Ok(()), device.lock()); + assert_ok!((), device.lock()); assert_eq!(1, count_nitrokey_block_devices()); - assert_eq!(Ok(()), device.disable_encrypted_volume()); + assert_ok!((), device.disable_encrypted_volume()); assert_eq!(1, count_nitrokey_block_devices()); - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.enable_encrypted_volume("123") ); assert_eq!(1, count_nitrokey_block_devices()); - assert_eq!(Ok(()), device.enable_encrypted_volume(USER_PASSWORD)); + assert_ok!((), device.enable_encrypted_volume(USER_PASSWORD)); assert_eq!(2, count_nitrokey_block_devices()); - assert_eq!(Ok(()), device.disable_encrypted_volume()); + assert_ok!((), device.disable_encrypted_volume()); assert_eq!(1, count_nitrokey_block_devices()); } #[test_device] fn hidden_volume(device: Storage) { - assert_eq!(Ok(()), device.lock()); + assert_ok!((), device.lock()); assert_eq!(1, count_nitrokey_block_devices()); - assert_eq!(Ok(()), device.disable_hidden_volume()); + assert_ok!((), device.disable_hidden_volume()); assert_eq!(1, count_nitrokey_block_devices()); - assert_eq!(Ok(()), device.enable_encrypted_volume(USER_PASSWORD)); + assert_ok!((), device.enable_encrypted_volume(USER_PASSWORD)); assert_eq!(2, count_nitrokey_block_devices()); // TODO: why this error code? - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.create_hidden_volume(5, 0, 100, "hiddenpw") ); - assert_eq!(Ok(()), device.create_hidden_volume(0, 20, 21, "hidden-pw")); - assert_eq!( - Ok(()), - device.create_hidden_volume(0, 20, 21, "hiddenpassword") - ); - assert_eq!(Ok(()), device.create_hidden_volume(1, 0, 1, "otherpw")); + assert_ok!((), device.create_hidden_volume(0, 20, 21, "hidden-pw")); + assert_ok!((), device.create_hidden_volume(0, 20, 21, "hiddenpassword")); + assert_ok!((), device.create_hidden_volume(1, 0, 1, "otherpw")); // TODO: test invalid range (not handled by libnitrokey) assert_eq!(2, count_nitrokey_block_devices()); - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.enable_hidden_volume("blubb") ); - assert_eq!(Ok(()), device.enable_hidden_volume("hiddenpassword")); + assert_ok!((), device.enable_hidden_volume("hiddenpassword")); assert_eq!(2, count_nitrokey_block_devices()); - assert_eq!(Ok(()), device.enable_hidden_volume("otherpw")); + assert_ok!((), device.enable_hidden_volume("otherpw")); assert_eq!(2, count_nitrokey_block_devices()); - assert_eq!(Ok(()), device.disable_hidden_volume()); + assert_ok!((), device.disable_hidden_volume()); assert_eq!(1, count_nitrokey_block_devices()); } #[test_device] fn lock(device: Storage) { - assert_eq!(Ok(()), device.enable_encrypted_volume(USER_PASSWORD)); - assert_eq!(Ok(()), device.lock()); + assert_ok!((), device.enable_encrypted_volume(USER_PASSWORD)); + assert_ok!((), device.lock()); assert_eq!(1, count_nitrokey_block_devices()); } @@ -410,17 +404,14 @@ fn set_unencrypted_volume_mode(device: Storage) { } fn assert_success(device: &Storage, mode: VolumeMode) { - assert_eq!( - Ok(()), - device.set_unencrypted_volume_mode(ADMIN_PASSWORD, mode) - ); + assert_ok!((), device.set_unencrypted_volume_mode(ADMIN_PASSWORD, mode)); assert_mode(&device, mode); } assert_success(&device, VolumeMode::ReadOnly); - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.set_unencrypted_volume_mode(USER_PASSWORD, VolumeMode::ReadOnly) ); assert_mode(&device, VolumeMode::ReadOnly); @@ -460,17 +451,17 @@ fn get_production_info(device: Storage) { #[test_device] fn clear_new_sd_card_warning(device: Storage) { - assert_eq!(Ok(()), device.factory_reset(ADMIN_PASSWORD)); + assert_ok!((), device.factory_reset(ADMIN_PASSWORD)); thread::sleep(time::Duration::from_secs(3)); - assert_eq!(Ok(()), device.build_aes_key(ADMIN_PASSWORD)); + assert_ok!((), device.build_aes_key(ADMIN_PASSWORD)); // We have to perform an SD card operation to reset the new_sd_card_found field - assert_eq!(Ok(()), device.lock()); + assert_ok!((), device.lock()); let status = device.get_status().unwrap(); assert!(status.new_sd_card_found); - assert_eq!(Ok(()), device.clear_new_sd_card_warning(ADMIN_PASSWORD)); + assert_ok!((), device.clear_new_sd_card_warning(ADMIN_PASSWORD)); let status = device.get_status().unwrap(); assert!(!status.new_sd_card_found); @@ -478,18 +469,18 @@ fn clear_new_sd_card_warning(device: Storage) { #[test_device] fn export_firmware(device: Storage) { - assert_eq!( - Err(CommandError::WrongPassword), + assert_cmd_err!( + CommandError::WrongPassword, device.export_firmware("someadminpn") ); - assert_eq!(Ok(()), device.export_firmware(ADMIN_PASSWORD)); - assert_eq!( - Ok(()), + assert_ok!((), device.export_firmware(ADMIN_PASSWORD)); + assert_ok!( + (), device.set_unencrypted_volume_mode(ADMIN_PASSWORD, VolumeMode::ReadWrite) ); - assert_eq!(Ok(()), device.export_firmware(ADMIN_PASSWORD)); - assert_eq!( - Ok(()), + assert_ok!((), device.export_firmware(ADMIN_PASSWORD)); + assert_ok!( + (), device.set_unencrypted_volume_mode(ADMIN_PASSWORD, VolumeMode::ReadOnly) ); } diff --git a/tests/otp.rs b/tests/otp.rs index 712f7a2..d328351 100644 --- a/tests/otp.rs +++ b/tests/otp.rs @@ -38,7 +38,7 @@ enum TotpTimestampSize { fn make_admin_test_device(device: T) -> Admin where T: Device, - (T, nitrokey::CommandError): Debug, + (T, nitrokey::Error): Debug, { device .authenticate_admin(ADMIN_PASSWORD) @@ -47,7 +47,7 @@ where fn configure_hotp(admin: &ConfigureOtp, counter: u8) { let slot_data = OtpSlotData::new(1, "test-hotp", HOTP_SECRET, OtpMode::SixDigits); - assert_eq!(Ok(()), admin.write_hotp_slot(slot_data, counter.into())); + assert_ok!((), admin.write_hotp_slot(slot_data, counter.into())); } fn check_hotp_codes(device: &GenerateOtp, offset: u8) { @@ -61,20 +61,17 @@ fn check_hotp_codes(device: &GenerateOtp, offset: u8) { #[test_device] fn set_time(device: DeviceWrapper) { - assert_eq!(Ok(()), device.set_time(1546385382, true)); - assert_eq!(Ok(()), device.set_time(1546385392, false)); - assert_eq!( - Err(CommandError::Timestamp), - device.set_time(1546385292, false) - ); - assert_eq!(Ok(()), device.set_time(1546385382, true)); + assert_ok!((), device.set_time(1546385382, true)); + assert_ok!((), device.set_time(1546385392, false)); + assert_cmd_err!(CommandError::Timestamp, device.set_time(1546385292, false)); + assert_ok!((), device.set_time(1546385382, true)); } #[test_device] fn hotp_no_pin(device: DeviceWrapper) { let admin = make_admin_test_device(device); let config = Config::new(None, None, None, false); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); configure_hotp(&admin, 0); check_hotp_codes(admin.deref(), 0); @@ -90,7 +87,7 @@ fn hotp_no_pin(device: DeviceWrapper) { fn hotp_pin(device: DeviceWrapper) { let admin = make_admin_test_device(device); let config = Config::new(None, None, None, true); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); configure_hotp(&admin, 0); let user = admin.device().authenticate_user(USER_PASSWORD).unwrap(); @@ -103,54 +100,51 @@ fn hotp_pin(device: DeviceWrapper) { fn hotp_slot_name(device: DeviceWrapper) { let admin = make_admin_test_device(device); let slot_data = OtpSlotData::new(1, "test-hotp", HOTP_SECRET, OtpMode::SixDigits); - assert_eq!(Ok(()), admin.write_hotp_slot(slot_data, 0)); + assert_ok!((), admin.write_hotp_slot(slot_data, 0)); let device = admin.device(); let result = device.get_hotp_slot_name(1); assert_eq!("test-hotp", result.unwrap()); let result = device.get_hotp_slot_name(4); - assert_eq!(CommandError::InvalidSlot, result.unwrap_err()); + assert_cmd_err!(CommandError::InvalidSlot, result); } #[test_device] fn hotp_error(device: DeviceWrapper) { let admin = make_admin_test_device(device); let slot_data = OtpSlotData::new(1, "", HOTP_SECRET, OtpMode::SixDigits); - assert_eq!( - Err(CommandError::NoName), - admin.write_hotp_slot(slot_data, 0) - ); + assert_cmd_err!(CommandError::NoName, admin.write_hotp_slot(slot_data, 0)); let slot_data = OtpSlotData::new(4, "test", HOTP_SECRET, OtpMode::SixDigits); - assert_eq!( - Err(CommandError::InvalidSlot), + assert_cmd_err!( + CommandError::InvalidSlot, admin.write_hotp_slot(slot_data, 0) ); let slot_data = OtpSlotData::new(1, "test", "foobar", OtpMode::SixDigits); - assert_eq!( - Err(CommandError::InvalidHexString), + assert_cmd_err!( + CommandError::InvalidHexString, admin.write_hotp_slot(slot_data, 0) ); let code = admin.get_hotp_code(4); - assert_eq!(CommandError::InvalidSlot, code.unwrap_err()); + assert_cmd_err!(CommandError::InvalidSlot, code); } #[test_device] fn hotp_erase(device: DeviceWrapper) { let admin = make_admin_test_device(device); let config = Config::new(None, None, None, false); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); let slot_data = OtpSlotData::new(1, "test1", HOTP_SECRET, OtpMode::SixDigits); - assert_eq!(Ok(()), admin.write_hotp_slot(slot_data, 0)); + assert_ok!((), admin.write_hotp_slot(slot_data, 0)); let slot_data = OtpSlotData::new(2, "test2", HOTP_SECRET, OtpMode::SixDigits); - assert_eq!(Ok(()), admin.write_hotp_slot(slot_data, 0)); + assert_ok!((), admin.write_hotp_slot(slot_data, 0)); - assert_eq!(Ok(()), admin.erase_hotp_slot(1)); + assert_ok!((), admin.erase_hotp_slot(1)); let device = admin.device(); let result = device.get_hotp_slot_name(1); - assert_eq!(CommandError::SlotNotProgrammed, result.unwrap_err()); + assert_cmd_err!(CommandError::SlotNotProgrammed, result); let result = device.get_hotp_code(1); - assert_eq!(CommandError::SlotNotProgrammed, result.unwrap_err()); + assert_cmd_err!(CommandError::SlotNotProgrammed, result); assert_eq!("test2", device.get_hotp_slot_name(2).unwrap()); } @@ -158,7 +152,7 @@ fn hotp_erase(device: DeviceWrapper) { fn configure_totp(admin: &ConfigureOtp, factor: u64) { let slot_data = OtpSlotData::new(1, "test-totp", TOTP_SECRET, OtpMode::EightDigits); let time_window = 30u64.checked_mul(factor).unwrap(); - assert_eq!(Ok(()), admin.write_totp_slot(slot_data, time_window as u16)); + assert_ok!((), admin.write_totp_slot(slot_data, time_window as u16)); } fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimestampSize) { @@ -169,7 +163,7 @@ fn check_totp_codes(device: &GenerateOtp, factor: u64, timestamp_size: TotpTimes continue; } - assert_eq!(Ok(()), device.set_time(time, true)); + assert_ok!((), device.set_time(time, true)); let result = device.get_totp_code(1); assert!(result.is_ok()); let result_code = result.unwrap(); @@ -186,7 +180,7 @@ fn totp_no_pin(device: DeviceWrapper) { // TODO: this test may fail due to bad timing --> find solution let admin = make_admin_test_device(device); let config = Config::new(None, None, None, false); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); configure_totp(&admin, 1); check_totp_codes(admin.deref(), 1, TotpTimestampSize::U32); @@ -204,7 +198,7 @@ fn totp_no_pin(device: DeviceWrapper) { fn totp_no_pin_64(device: Pro) { let admin = make_admin_test_device(device); let config = Config::new(None, None, None, false); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); configure_totp(&admin, 1); check_totp_codes(admin.deref(), 1, TotpTimestampSize::U64); @@ -221,7 +215,7 @@ fn totp_pin(device: DeviceWrapper) { // TODO: this test may fail due to bad timing --> find solution let admin = make_admin_test_device(device); let config = Config::new(None, None, None, true); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); configure_totp(&admin, 1); let user = admin.device().authenticate_user(USER_PASSWORD).unwrap(); @@ -235,7 +229,7 @@ fn totp_pin(device: DeviceWrapper) { fn totp_pin_64(device: Pro) { let admin = make_admin_test_device(device); let config = Config::new(None, None, None, true); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); configure_totp(&admin, 1); let user = admin.device().authenticate_user(USER_PASSWORD).unwrap(); @@ -248,55 +242,52 @@ fn totp_pin_64(device: Pro) { fn totp_slot_name(device: DeviceWrapper) { let admin = make_admin_test_device(device); let slot_data = OtpSlotData::new(1, "test-totp", TOTP_SECRET, OtpMode::EightDigits); - assert_eq!(Ok(()), admin.write_totp_slot(slot_data, 0)); + assert_ok!((), admin.write_totp_slot(slot_data, 0)); let device = admin.device(); let result = device.get_totp_slot_name(1); assert!(result.is_ok()); assert_eq!("test-totp", result.unwrap()); let result = device.get_totp_slot_name(16); - assert_eq!(CommandError::InvalidSlot, result.unwrap_err()); + assert_cmd_err!(CommandError::InvalidSlot, result); } #[test_device] fn totp_error(device: DeviceWrapper) { let admin = make_admin_test_device(device); let slot_data = OtpSlotData::new(1, "", TOTP_SECRET, OtpMode::SixDigits); - assert_eq!( - Err(CommandError::NoName), - admin.write_totp_slot(slot_data, 0) - ); + assert_cmd_err!(CommandError::NoName, admin.write_totp_slot(slot_data, 0)); let slot_data = OtpSlotData::new(20, "test", TOTP_SECRET, OtpMode::SixDigits); - assert_eq!( - Err(CommandError::InvalidSlot), + assert_cmd_err!( + CommandError::InvalidSlot, admin.write_totp_slot(slot_data, 0) ); let slot_data = OtpSlotData::new(4, "test", "foobar", OtpMode::SixDigits); - assert_eq!( - Err(CommandError::InvalidHexString), + assert_cmd_err!( + CommandError::InvalidHexString, admin.write_totp_slot(slot_data, 0) ); let code = admin.get_totp_code(20); - assert_eq!(CommandError::InvalidSlot, code.unwrap_err()); + assert_cmd_err!(CommandError::InvalidSlot, code); } #[test_device] fn totp_erase(device: DeviceWrapper) { let admin = make_admin_test_device(device); let config = Config::new(None, None, None, false); - assert_eq!(Ok(()), admin.write_config(config)); + assert_ok!((), admin.write_config(config)); let slot_data = OtpSlotData::new(1, "test1", TOTP_SECRET, OtpMode::SixDigits); - assert_eq!(Ok(()), admin.write_totp_slot(slot_data, 0)); + assert_ok!((), admin.write_totp_slot(slot_data, 0)); let slot_data = OtpSlotData::new(2, "test2", TOTP_SECRET, OtpMode::SixDigits); - assert_eq!(Ok(()), admin.write_totp_slot(slot_data, 0)); + assert_ok!((), admin.write_totp_slot(slot_data, 0)); - assert_eq!(Ok(()), admin.erase_totp_slot(1)); + assert_ok!((), admin.erase_totp_slot(1)); let device = admin.device(); let result = device.get_totp_slot_name(1); - assert_eq!(CommandError::SlotNotProgrammed, result.unwrap_err()); + assert_cmd_err!(CommandError::SlotNotProgrammed, result); let result = device.get_totp_code(1); - assert_eq!(CommandError::SlotNotProgrammed, result.unwrap_err()); + assert_cmd_err!(CommandError::SlotNotProgrammed, result); assert_eq!("test2", device.get_totp_slot_name(2).unwrap()); } diff --git a/tests/pws.rs b/tests/pws.rs index fbcc0c1..f12af48 100644 --- a/tests/pws.rs +++ b/tests/pws.rs @@ -3,16 +3,16 @@ mod util; use std::ffi::CStr; use libc::{c_int, c_void, free}; -use nitrokey::{CommandError, Device, GetPasswordSafe, PasswordSafe, SLOT_COUNT}; +use nitrokey::{CommandError, Device, Error, GetPasswordSafe, PasswordSafe, SLOT_COUNT}; use nitrokey_sys; use nitrokey_test::test as test_device; use crate::util::{ADMIN_PASSWORD, USER_PASSWORD}; -fn get_slot_name_direct(slot: u8) -> Result { +fn get_slot_name_direct(slot: u8) -> Result { let ptr = unsafe { nitrokey_sys::NK_get_password_safe_slot_name(slot) }; if ptr.is_null() { - return Err(CommandError::Undefined); + return Err(CommandError::Undefined.into()); } let s = unsafe { CStr::from_ptr(ptr).to_string_lossy().into_owned() }; unsafe { free(ptr as *mut c_void) }; @@ -21,7 +21,7 @@ fn get_slot_name_direct(slot: u8) -> Result { let error = unsafe { nitrokey_sys::NK_get_last_command_status() } as c_int; match error { 0 => Ok(s), - other => Err(CommandError::from(other)), + other => Err(CommandError::from(other).into()), } } false => Ok(s), @@ -49,35 +49,35 @@ fn enable(device: DeviceWrapper) { fn drop(device: DeviceWrapper) { { let pws = get_pws(&device); - assert_eq!(Ok(()), pws.write_slot(1, "name", "login", "password")); + assert_ok!((), pws.write_slot(1, "name", "login", "password")); assert_eq!("name", pws.get_slot_name(1).unwrap()); let result = get_slot_name_direct(1); - assert_eq!(Ok(String::from("name")), result); + assert_ok!(String::from("name"), result); } let result = get_slot_name_direct(1); - assert_eq!(Ok(String::from("name")), result); - assert_eq!(Ok(()), device.lock()); + assert_ok!(String::from("name"), result); + assert_ok!((), device.lock()); let result = get_slot_name_direct(1); - assert_eq!(Err(CommandError::NotAuthorized), result); + assert_cmd_err!(CommandError::NotAuthorized, result); } #[test_device] fn get_status(device: DeviceWrapper) { let pws = get_pws(&device); for i in 0..SLOT_COUNT { - assert_eq!(Ok(()), pws.erase_slot(i), "Could not erase slot {}", i); + assert_ok!((), pws.erase_slot(i)); } let status = pws.get_slot_status().unwrap(); assert_eq!(status, [false; SLOT_COUNT as usize]); - assert_eq!(Ok(()), pws.write_slot(1, "name", "login", "password")); + assert_ok!((), pws.write_slot(1, "name", "login", "password")); let status = pws.get_slot_status().unwrap(); for i in 0..SLOT_COUNT { assert_eq!(i == 1, status[i as usize]); } for i in 0..SLOT_COUNT { - assert_eq!(Ok(()), pws.write_slot(i, "name", "login", "password")); + assert_ok!((), pws.write_slot(i, "name", "login", "password")); } let status = pws.get_slot_status().unwrap(); assert_eq!(status, [true; SLOT_COUNT as usize]); @@ -86,76 +86,61 @@ fn get_status(device: DeviceWrapper) { #[test_device] fn get_data(device: DeviceWrapper) { let pws = get_pws(&device); - assert_eq!(Ok(()), pws.write_slot(1, "name", "login", "password")); + assert_ok!((), pws.write_slot(1, "name", "login", "password")); assert_eq!("name", pws.get_slot_name(1).unwrap()); assert_eq!("login", pws.get_slot_login(1).unwrap()); assert_eq!("password", pws.get_slot_password(1).unwrap()); - assert_eq!(Ok(()), pws.erase_slot(1)); - assert_eq!(Err(CommandError::SlotNotProgrammed), pws.get_slot_name(1)); - assert_eq!(Err(CommandError::SlotNotProgrammed), pws.get_slot_login(1)); - assert_eq!( - Err(CommandError::SlotNotProgrammed), - pws.get_slot_password(1) - ); + assert_ok!((), pws.erase_slot(1)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_name(1)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_login(1)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_password(1)); let name = "with å"; let login = "pär@test.com"; let password = "'i3lJc[09?I:,[u7dWz9"; - assert_eq!(Ok(()), pws.write_slot(1, name, login, password)); + assert_ok!((), pws.write_slot(1, name, login, password)); assert_eq!(name, pws.get_slot_name(1).unwrap()); assert_eq!(login, pws.get_slot_login(1).unwrap()); assert_eq!(password, pws.get_slot_password(1).unwrap()); - assert_eq!( - Err(CommandError::InvalidSlot), - pws.get_slot_name(SLOT_COUNT) - ); - assert_eq!( - Err(CommandError::InvalidSlot), - pws.get_slot_login(SLOT_COUNT) - ); - assert_eq!( - Err(CommandError::InvalidSlot), - pws.get_slot_password(SLOT_COUNT) - ); + 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)); } #[test_device] fn write(device: DeviceWrapper) { let pws = get_pws(&device); - assert_eq!( - Err(CommandError::InvalidSlot), + assert_cmd_err!( + CommandError::InvalidSlot, pws.write_slot(SLOT_COUNT, "name", "login", "password") ); - assert_eq!(Ok(()), pws.write_slot(0, "", "login", "password")); - assert_eq!(Err(CommandError::SlotNotProgrammed), pws.get_slot_name(0)); - assert_eq!(Ok(String::from("login")), pws.get_slot_login(0)); - assert_eq!(Ok(String::from("password")), pws.get_slot_password(0)); - - assert_eq!(Ok(()), pws.write_slot(0, "name", "", "password")); - assert_eq!(Ok(String::from("name")), pws.get_slot_name(0)); - assert_eq!(Err(CommandError::SlotNotProgrammed), pws.get_slot_login(0)); - assert_eq!(Ok(String::from("password")), pws.get_slot_password(0)); - - assert_eq!(Ok(()), pws.write_slot(0, "name", "login", "")); - assert_eq!(Ok(String::from("name")), pws.get_slot_name(0)); - assert_eq!(Ok(String::from("login")), pws.get_slot_login(0)); - assert_eq!( - Err(CommandError::SlotNotProgrammed), - pws.get_slot_password(0) - ); + assert_ok!((), pws.write_slot(0, "", "login", "password")); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_name(0)); + assert_ok!(String::from("login"), pws.get_slot_login(0)); + assert_ok!(String::from("password"), pws.get_slot_password(0)); + + assert_ok!((), pws.write_slot(0, "name", "", "password")); + assert_ok!(String::from("name"), pws.get_slot_name(0)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_login(0)); + assert_ok!(String::from("password"), pws.get_slot_password(0)); + + assert_ok!((), pws.write_slot(0, "name", "login", "")); + assert_ok!(String::from("name"), pws.get_slot_name(0)); + assert_ok!(String::from("login"), pws.get_slot_login(0)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_password(0)); } #[test_device] fn erase(device: DeviceWrapper) { let pws = get_pws(&device); - assert_eq!(Err(CommandError::InvalidSlot), pws.erase_slot(SLOT_COUNT)); + assert_cmd_err!(CommandError::InvalidSlot, pws.erase_slot(SLOT_COUNT)); - assert_eq!(Ok(()), pws.write_slot(0, "name", "login", "password")); - assert_eq!(Ok(()), pws.erase_slot(0)); - assert_eq!(Ok(()), pws.erase_slot(0)); - assert_eq!(Err(CommandError::SlotNotProgrammed), pws.get_slot_name(0)); + assert_ok!((), pws.write_slot(0, "name", "login", "password")); + assert_ok!((), pws.erase_slot(0)); + assert_ok!((), pws.erase_slot(0)); + assert_cmd_err!(CommandError::SlotNotProgrammed, pws.get_slot_name(0)); } 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 5c5644d2e11af0cbf4df5c5ddbc22cf0b12af4a6 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 03:58:17 +0000 Subject: Enable std feature for rand_core With the std feature enabled, rand_core::Error implements std::error::Error, which we require for the error types wrapped in the Error enum. --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index cfe1579..ceaa57d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,7 +15,7 @@ license = "MIT" [dependencies] libc = "0.2" nitrokey-sys = "3.4" -rand_core = {version = "0.3", default-features = false} +rand_core = {version = "0.3", default-features = false, features = ["std"] } rand_os = {version = "0.1"} [dev-dependencies] -- cgit v1.2.3 From cafc3a6f8cfb9f82343c1d3fe843c7f8d7ef1136 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 20 Jan 2019 21:05:58 +0000 Subject: Refactor CommandError::RngError into Error::RandError We reserve CommandError for errors returned by the Nitrokey device. Errors during random number generation should have their own type. --- CHANGELOG.md | 1 + src/error.rs | 19 ++++++++++--------- src/util.rs | 2 +- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bbb3202..5a78724 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Implement `std::error::Error` for `CommandError`. - Add the `Error` enum and the `Result` typedef. - Return `Error` instead of `CommandError` in all public functions. + - Move the `CommandError::RngError` variant to `Error::RandError`. # v0.3.4 (2019-01-20) - Fix authentication methods that assumed that `char` is signed. diff --git a/src/error.rs b/src/error.rs index 3f60af2..dfe1680 100644 --- a/src/error.rs +++ b/src/error.rs @@ -11,6 +11,8 @@ pub enum Error { CommandError(CommandError), /// Placeholder for testing. CommunicationError(CommunicationError), + /// An error that occured during random number generation. + RandError(rand_core::Error), } impl From for Error { @@ -19,11 +21,18 @@ impl From for Error { } } +impl From for Error { + fn from(error: rand_core::Error) -> Self { + Error::RandError(error) + } +} + impl error::Error for Error { fn source(&self) -> Option<&(dyn error::Error + 'static)> { match *self { Error::CommandError(ref err) => Some(err), Error::CommunicationError(_) => None, + Error::RandError(ref err) => Some(err), } } } @@ -33,6 +42,7 @@ impl fmt::Display for Error { match *self { Error::CommandError(ref err) => write!(f, "Command error: {}", err), Error::CommunicationError(_) => write!(f, "Placeholder"), + Error::RandError(ref err) => write!(f, "RNG error: {}", err), } } } @@ -78,8 +88,6 @@ pub enum CommandError { InvalidHexString, /// The target buffer was smaller than the source. TargetBufferTooSmall, - /// An error occurred during random number generation. - RngError, } /// Placeholder for testing. @@ -119,7 +127,6 @@ impl CommandError { "The supplied string is not in hexadecimal format".into() } CommandError::TargetBufferTooSmall => "The target buffer is too small".into(), - CommandError::RngError => "An error occurred during random number generation".into(), } } } @@ -153,9 +160,3 @@ impl From for CommandError { } } } - -impl From for CommandError { - fn from(_error: rand_core::Error) -> Self { - CommandError::RngError - } -} diff --git a/src/util.rs b/src/util.rs index 8855275..06bb854 100644 --- a/src/util.rs +++ b/src/util.rs @@ -71,7 +71,7 @@ pub fn get_last_error() -> Error { } pub fn generate_password(length: usize) -> Result, Error> { - let mut rng = OsRng::new().map_err(CommandError::from)?; + let mut rng = OsRng::new()?; let mut data = vec![0u8; length]; rng.fill_bytes(&mut data[..]); Ok(data) -- cgit v1.2.3 From 944e1fa0d51e547dde2a9368d2b8431b109f63c4 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 04:15:31 +0000 Subject: Move the CommandError::Unknown to Error An error code can not only indiciate a command error, but also a library or device communication error. Therefore, the variant for an unknown error code should be placed in the top-level Error enum instead of the CommandError enum. --- CHANGELOG.md | 3 ++- src/auth.rs | 4 ++-- src/error.rs | 61 +++++++++++++++++++++++++++++++++--------------------------- src/util.rs | 2 +- tests/pws.rs | 2 +- 5 files changed, 40 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a78724..def5273 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ - Implement `std::error::Error` for `CommandError`. - Add the `Error` enum and the `Result` typedef. - Return `Error` instead of `CommandError` in all public functions. - - Move the `CommandError::RngError` variant to `Error::RandError`. + - Move the `CommandError::RngError` variant to `Error::RandError` and the + `CommandError::Unknown` variant to `Error::Unknown`. # v0.3.4 (2019-01-20) - Fix authentication methods that assumed that `char` is signed. diff --git a/src/auth.rs b/src/auth.rs index 509d3aa..e05f6b3 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -6,7 +6,7 @@ use nitrokey_sys; use crate::config::{Config, RawConfig}; use crate::device::{Device, DeviceWrapper, Pro, Storage}; -use crate::error::{CommandError, Error}; +use crate::error::Error; use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData, RawOtpSlotData}; use crate::util::{generate_password, get_command_result, get_cstring, result_from_string}; @@ -161,7 +161,7 @@ where let temp_password_ptr = temp_password.as_ptr() as *const c_char; return match callback(password_ptr, temp_password_ptr) { 0 => Ok(A::new(device, temp_password)), - rv => Err((device, CommandError::from(rv).into())), + rv => Err((device, Error::from(rv))), }; } diff --git a/src/error.rs b/src/error.rs index dfe1680..c5a975e 100644 --- a/src/error.rs +++ b/src/error.rs @@ -13,6 +13,18 @@ pub enum Error { CommunicationError(CommunicationError), /// An error that occured during random number generation. RandError(rand_core::Error), + /// An unknown error returned by libnitrokey. + Unknown(i64), +} + +impl From for Error { + fn from(code: raw::c_int) -> Self { + if let Some(err) = CommandError::try_from(code) { + Error::CommandError(err) + } else { + Error::Unknown(code.into()) + } + } } impl From for Error { @@ -33,6 +45,7 @@ impl error::Error for Error { Error::CommandError(ref err) => Some(err), Error::CommunicationError(_) => None, Error::RandError(ref err) => Some(err), + Error::Unknown(_) => None, } } } @@ -43,6 +56,7 @@ impl fmt::Display for Error { Error::CommandError(ref err) => write!(f, "Command error: {}", err), Error::CommunicationError(_) => write!(f, "Placeholder"), Error::RandError(ref err) => write!(f, "RNG error: {}", err), + Error::Unknown(ref err) => write!(f, "Unknown error: {}", err), } } } @@ -74,8 +88,6 @@ pub enum CommandError { UnknownCommand, /// AES decryption failed. AesDecryptionFailed, - /// An unknown error occurred. - Unknown(i64), /// An unspecified error occurred. Undefined, /// You passed a string containing a null byte. @@ -98,6 +110,26 @@ pub enum CommunicationError { } impl CommandError { + fn try_from(value: raw::c_int) -> Option { + match value { + 1 => Some(CommandError::WrongCrc), + 2 => Some(CommandError::WrongSlot), + 3 => Some(CommandError::SlotNotProgrammed), + 4 => Some(CommandError::WrongPassword), + 5 => Some(CommandError::NotAuthorized), + 6 => Some(CommandError::Timestamp), + 7 => Some(CommandError::NoName), + 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, + } + } + fn as_str(&self) -> borrow::Cow<'static, str> { match *self { CommandError::WrongCrc => { @@ -116,9 +148,6 @@ impl CommandError { CommandError::NotSupported => "This command is not supported by this device".into(), CommandError::UnknownCommand => "This command is unknown".into(), CommandError::AesDecryptionFailed => "AES decryption failed".into(), - CommandError::Unknown(x) => { - borrow::Cow::from(format!("An unknown error occurred ({})", x)) - } 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(), @@ -138,25 +167,3 @@ impl fmt::Display for CommandError { write!(f, "{}", self.as_str()) } } - -impl From for CommandError { - fn from(value: raw::c_int) -> Self { - match value { - 1 => CommandError::WrongCrc, - 2 => CommandError::WrongSlot, - 3 => CommandError::SlotNotProgrammed, - 4 => CommandError::WrongPassword, - 5 => CommandError::NotAuthorized, - 6 => CommandError::Timestamp, - 7 => CommandError::NoName, - 8 => CommandError::NotSupported, - 9 => CommandError::UnknownCommand, - 10 => CommandError::AesDecryptionFailed, - 200 => CommandError::StringTooLong, - 201 => CommandError::InvalidSlot, - 202 => CommandError::InvalidHexString, - 203 => CommandError::TargetBufferTooSmall, - x => CommandError::Unknown(x.into()), - } - } -} diff --git a/src/util.rs b/src/util.rs index 06bb854..3b9904f 100644 --- a/src/util.rs +++ b/src/util.rs @@ -54,7 +54,7 @@ pub fn result_from_string(ptr: *const c_char) -> Result { pub fn get_command_result(value: c_int) -> Result<(), Error> { match value { 0 => Ok(()), - other => Err(CommandError::from(other).into()), + other => Err(Error::from(other)), } } diff --git a/tests/pws.rs b/tests/pws.rs index f12af48..a4647bd 100644 --- a/tests/pws.rs +++ b/tests/pws.rs @@ -21,7 +21,7 @@ fn get_slot_name_direct(slot: u8) -> Result { let error = unsafe { nitrokey_sys::NK_get_last_command_status() } as c_int; match error { 0 => Ok(s), - other => Err(CommandError::from(other).into()), + other => Err(Error::from(other)), } } false => Ok(s), -- 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(-) 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 27138c4b799248d2d39e9681337a620c89636557 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 13:10:01 +0000 Subject: Add the CommunicationError enum Communication errors returned by libnitrokey were previously not mapped to an error type in the nitrokey crate. We introduce the CommunicationError enum to represent these errors. --- CHANGELOG.md | 2 ++ src/error.rs | 59 +++++++++++++++++++++++++++++++++++++++++++++++++---------- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c34175e..413c626 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ - Add the `Error` enum and the `Result` typedef. - Add the `LibraryError` enum and move the library error variants from `CommandError` to `LibraryError`. + - Add the `CommunicationError` enum and move the communication error variants + from `CommandError` to `CommunicationError`. - 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/error.rs b/src/error.rs index f40d07f..a2b3848 100644 --- a/src/error.rs +++ b/src/error.rs @@ -9,7 +9,7 @@ use std::result; pub enum Error { /// An error reported by the Nitrokey device in the response packet. CommandError(CommandError), - /// Placeholder for testing. + /// A device communication. CommunicationError(CommunicationError), /// A library usage error. LibraryError(LibraryError), @@ -23,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) = CommunicationError::try_from(256 - code) { + Error::CommunicationError(err) } else if let Some(err) = LibraryError::try_from(code) { Error::LibraryError(err) } else { @@ -37,6 +39,12 @@ impl From for Error { } } +impl From for Error { + fn from(err: CommunicationError) -> Self { + Error::CommunicationError(err) + } +} + impl From for Error { fn from(err: LibraryError) -> Self { Error::LibraryError(err) @@ -53,7 +61,7 @@ impl error::Error for Error { fn source(&self) -> Option<&(dyn error::Error + 'static)> { match *self { Error::CommandError(ref err) => Some(err), - Error::CommunicationError(_) => None, + Error::CommunicationError(ref err) => Some(err), Error::LibraryError(ref err) => Some(err), Error::RandError(ref err) => Some(err), Error::Unknown(_) => None, @@ -65,7 +73,7 @@ impl fmt::Display for Error { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match *self { Error::CommandError(ref err) => write!(f, "Command error: {}", err), - Error::CommunicationError(_) => write!(f, "Placeholder"), + Error::CommunicationError(ref err) => write!(f, "Communication error: {}", err), 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), @@ -104,13 +112,6 @@ pub enum CommandError { Undefined, } -/// Placeholder for testing. -#[derive(Debug)] -pub enum CommunicationError { - /// Placeholder for testing. - NotConnected, -} - impl CommandError { fn try_from(value: raw::c_int) -> Option { match value { @@ -159,6 +160,44 @@ impl fmt::Display for CommandError { } } +/// A device communication error. +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum CommunicationError { + /// Could not connect to a Nitrokey device. + NotConnected, + /// Sending a packet failed. + SendingFailure, + /// Receiving a packet failed. + ReceivingFailure, + /// A packet with a wrong checksum was received. + InvalidCrc, +} + +impl CommunicationError { + fn try_from(value: raw::c_int) -> Option { + match value { + 2 => Some(CommunicationError::NotConnected), + 3 => Some(CommunicationError::SendingFailure), + 4 => Some(CommunicationError::ReceivingFailure), + 5 => Some(CommunicationError::InvalidCrc), + _ => None, + } + } +} + +impl error::Error for CommunicationError {} + +impl fmt::Display for CommunicationError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str(match *self { + CommunicationError::NotConnected => "Could not connect to a Nitrokey device", + CommunicationError::SendingFailure => "Sending a packet failed", + CommunicationError::ReceivingFailure => "Receiving a packet failed", + CommunicationError::InvalidCrc => "A packet with a wrong checksum was received", + }) + } +} + /// A library usage error. #[derive(Clone, Copy, Debug, PartialEq)] pub enum LibraryError { -- cgit v1.2.3 From 391cfd03edafd6e857d6cdbee1347f38e7a02b3f Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 13:28:03 +0000 Subject: Remove CommandError::as_str method AsStr is automatically implementeded if Display is implemented, so having a manual as_str() method is not necessary. --- src/error.rs | 40 ++++++++++++++++------------------------ 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/src/error.rs b/src/error.rs index a2b3848..1aaf21f 100644 --- a/src/error.rs +++ b/src/error.rs @@ -1,4 +1,3 @@ -use std::borrow; use std::error; use std::fmt; use std::os::raw; @@ -128,35 +127,28 @@ impl CommandError { _ => None, } } - - fn as_str(&self) -> borrow::Cow<'static, str> { - match *self { - CommandError::WrongCrc => { - "A packet with a wrong checksum has been sent or received".into() - } - CommandError::WrongSlot => "The given OTP slot does not exist".into(), - CommandError::SlotNotProgrammed => "The given OTP slot is not programmed".into(), - CommandError::WrongPassword => "The given password is wrong".into(), - CommandError::NotAuthorized => { - "You are not authorized for this command or provided a wrong temporary \ - password" - .into() - } - CommandError::Timestamp => "An error occurred when getting or setting the time".into(), - CommandError::NoName => "You did not provide a name for the OTP slot".into(), - CommandError::NotSupported => "This command is not supported by this device".into(), - CommandError::UnknownCommand => "This command is unknown".into(), - CommandError::AesDecryptionFailed => "AES decryption failed".into(), - CommandError::Undefined => "An unspecified error occurred".into(), - } - } } impl error::Error for CommandError {} impl fmt::Display for CommandError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self.as_str()) + f.write_str(match *self { + CommandError::WrongCrc => "A packet with a wrong checksum has been sent or received", + CommandError::WrongSlot => "The given OTP slot does not exist", + CommandError::SlotNotProgrammed => "The given OTP slot is not programmed", + CommandError::WrongPassword => "The given password is wrong", + CommandError::NotAuthorized => { + "You are not authorized for this command or provided a wrong temporary \ + password" + } + CommandError::Timestamp => "An error occurred when getting or setting the time", + CommandError::NoName => "You did not provide a name for the OTP slot", + CommandError::NotSupported => "This command is not supported by this device", + CommandError::UnknownCommand => "This command is unknown", + CommandError::AesDecryptionFailed => "AES decryption failed", + CommandError::Undefined => "An unspecified error occurred", + }) } } -- cgit v1.2.3 From c3e551dd40142bcd2552972d549f31ad7483621d Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 13:30:20 +0000 Subject: Make CommandError messages more general For example, the WrongSlot error may also be returned for a PWS slot. --- src/error.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/error.rs b/src/error.rs index 1aaf21f..ef9b149 100644 --- a/src/error.rs +++ b/src/error.rs @@ -135,15 +135,15 @@ impl fmt::Display for CommandError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str(match *self { CommandError::WrongCrc => "A packet with a wrong checksum has been sent or received", - CommandError::WrongSlot => "The given OTP slot does not exist", - CommandError::SlotNotProgrammed => "The given OTP slot is not programmed", + CommandError::WrongSlot => "The given slot does not exist", + CommandError::SlotNotProgrammed => "The given slot is not programmed", CommandError::WrongPassword => "The given password is wrong", CommandError::NotAuthorized => { "You are not authorized for this command or provided a wrong temporary \ password" } CommandError::Timestamp => "An error occurred when getting or setting the time", - CommandError::NoName => "You did not provide a name for the OTP slot", + CommandError::NoName => "You did not provide a name for the slot", CommandError::NotSupported => "This command is not supported by this device", CommandError::UnknownCommand => "This command is unknown", CommandError::AesDecryptionFailed => "AES decryption failed", -- cgit v1.2.3 From c191e875492ff8aeab1b4493b87486cd265f0edc Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 13:38:28 +0000 Subject: Introduce the Error::UnexpectedError variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The UnexpectedError variant is used when a libnitrokey function returns a value that violates the function’s contract, for example if a function returns a null pointer although it guarantees to never return null. Previously, we returned a CommandError::Unspecified in these cases. --- src/error.rs | 4 ++++ src/util.rs | 6 +++--- tests/pws.rs | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/error.rs b/src/error.rs index ef9b149..b27124c 100644 --- a/src/error.rs +++ b/src/error.rs @@ -14,6 +14,8 @@ pub enum Error { LibraryError(LibraryError), /// An error that occured during random number generation. RandError(rand_core::Error), + /// An error that is caused by an unexpected value returned by libnitrokey. + UnexpectedError, /// An unknown error returned by libnitrokey. Unknown(i64), } @@ -63,6 +65,7 @@ impl error::Error for Error { Error::CommunicationError(ref err) => Some(err), Error::LibraryError(ref err) => Some(err), Error::RandError(ref err) => Some(err), + Error::UnexpectedError => None, Error::Unknown(_) => None, } } @@ -75,6 +78,7 @@ impl fmt::Display for Error { Error::CommunicationError(ref err) => write!(f, "Communication error: {}", err), Error::LibraryError(ref err) => write!(f, "Library error: {}", err), Error::RandError(ref err) => write!(f, "RNG error: {}", err), + Error::UnexpectedError => write!(f, "An unexpected error occurred"), Error::Unknown(ref err) => write!(f, "Unknown error: {}", err), } } diff --git a/src/util.rs b/src/util.rs index 2738fce..79b8c34 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, LibraryError}; +use crate::error::{Error, LibraryError}; /// Log level for libnitrokey. /// @@ -36,7 +36,7 @@ pub fn owned_str_from_ptr(ptr: *const c_char) -> String { pub fn result_from_string(ptr: *const c_char) -> Result { if ptr.is_null() { - return Err(CommandError::Undefined.into()); + return Err(Error::UnexpectedError); } unsafe { let s = owned_str_from_ptr(ptr); @@ -65,7 +65,7 @@ pub fn get_last_result() -> Result<(), Error> { pub fn get_last_error() -> Error { return match get_last_result() { - Ok(()) => CommandError::Undefined.into(), + Ok(()) => Error::UnexpectedError, Err(err) => err, }; } diff --git a/tests/pws.rs b/tests/pws.rs index 51e6189..b89d7f6 100644 --- a/tests/pws.rs +++ b/tests/pws.rs @@ -14,7 +14,7 @@ use crate::util::{ADMIN_PASSWORD, USER_PASSWORD}; fn get_slot_name_direct(slot: u8) -> Result { let ptr = unsafe { nitrokey_sys::NK_get_password_safe_slot_name(slot) }; if ptr.is_null() { - return Err(CommandError::Undefined.into()); + return Err(Error::UnexpectedError); } let s = unsafe { CStr::from_ptr(ptr).to_string_lossy().into_owned() }; unsafe { free(ptr as *mut c_void) }; -- cgit v1.2.3 From 70e886d3ca487c306b8eced9f0e067a67ba9c1bb Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 13:53:15 +0000 Subject: Return CommunicationError::NotConnected from connect functions Previously, we returned a CommandError::Undefined if a connect function failed. A CommunicationError::NotConnected is a more specific and better fitting choice. Once the Try trait has been stabilized, we should return an Option<_> instead of a Result<_, Error> from the connect functions. --- CHANGELOG.md | 2 ++ src/device.rs | 28 ++++++++++++++-------------- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 413c626..1856336 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,8 @@ - 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`. + - Return `CommunicationError::NotConnected` instead of + `CommandError::Undefined` from the connect functions. # v0.3.4 (2019-01-20) - Fix authentication methods that assumed that `char` is signed. diff --git a/src/device.rs b/src/device.rs index 5c4014b..1cf9da9 100644 --- a/src/device.rs +++ b/src/device.rs @@ -5,7 +5,7 @@ use nitrokey_sys; use crate::auth::Authenticate; use crate::config::{Config, RawConfig}; -use crate::error::{CommandError, Error}; +use crate::error::{CommunicationError, Error}; use crate::otp::GenerateOtp; use crate::pws::GetPasswordSafe; use crate::util::{get_command_result, get_cstring, get_last_error, result_from_string}; @@ -644,7 +644,7 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// /// # Errors /// -/// - [`Undefined`][] if no Nitrokey device is connected +/// - [`NotConnected`][] if no Nitrokey device is connected /// /// # Example /// @@ -659,15 +659,15 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp { /// } /// ``` /// -/// [`Undefined`]: enum.CommandError.html#variant.Undefined +/// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected pub fn connect() -> Result { unsafe { match nitrokey_sys::NK_login_auto() { 1 => match get_connected_device() { Some(wrapper) => Ok(wrapper), - None => Err(CommandError::Undefined.into()), + None => Err(CommunicationError::NotConnected.into()), }, - _ => Err(CommandError::Undefined.into()), + _ => Err(CommunicationError::NotConnected.into()), } } } @@ -676,7 +676,7 @@ pub fn connect() -> Result { /// /// # Errors /// -/// - [`Undefined`][] if no Nitrokey device of the given model is connected +/// - [`NotConnected`][] if no Nitrokey device of the given model is connected /// /// # Example /// @@ -692,12 +692,12 @@ pub fn connect() -> Result { /// } /// ``` /// -/// [`Undefined`]: enum.CommandError.html#variant.Undefined +/// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected pub fn connect_model(model: Model) -> Result { if connect_enum(model) { Ok(create_device_wrapper(model)) } else { - Err(CommandError::Undefined.into()) + Err(CommunicationError::NotConnected.into()) } } @@ -771,7 +771,7 @@ impl Pro { /// /// # Errors /// - /// - [`Undefined`][] if no Nitrokey device of the given model is connected + /// - [`NotConnected`][] if no Nitrokey device of the given model is connected /// /// # Example /// @@ -786,12 +786,12 @@ impl Pro { /// } /// ``` /// - /// [`Undefined`]: enum.CommandError.html#variant.Undefined + /// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected pub fn connect() -> Result { // TODO: maybe Option instead of Result? match connect_enum(Model::Pro) { true => Ok(Pro {}), - false => Err(CommandError::Undefined.into()), + false => Err(CommunicationError::NotConnected.into()), } } } @@ -817,7 +817,7 @@ impl Storage { /// /// # Errors /// - /// - [`Undefined`][] if no Nitrokey device of the given model is connected + /// - [`NotConnected`][] if no Nitrokey device of the given model is connected /// /// # Example /// @@ -832,12 +832,12 @@ impl Storage { /// } /// ``` /// - /// [`Undefined`]: enum.CommandError.html#variant.Undefined + /// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected pub fn connect() -> Result { // TODO: maybe Option instead of Result? match connect_enum(Model::Storage) { true => Ok(Storage {}), - false => Err(CommandError::Undefined.into()), + false => Err(CommunicationError::NotConnected.into()), } } -- cgit v1.2.3 From 17f9c30a0ace070cba856e4e89fcccedcab5e8e6 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 14:00:54 +0000 Subject: Remove the unused CommandError::Undefined variant The CommandError::Undefined variant has been refactored into Error::UnexpectedError and CommunicationError::NotConnected and is therefore no longer needed. --- CHANGELOG.md | 1 + src/error.rs | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1856336..a4df5c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ `CommandError::Unknown` variant to `Error::Unknown`. - Return `CommunicationError::NotConnected` instead of `CommandError::Undefined` from the connect functions. + - Remove the `CommandError::Undefined` variant. # v0.3.4 (2019-01-20) - Fix authentication methods that assumed that `char` is signed. diff --git a/src/error.rs b/src/error.rs index b27124c..cde9a34 100644 --- a/src/error.rs +++ b/src/error.rs @@ -111,8 +111,6 @@ pub enum CommandError { UnknownCommand, /// AES decryption failed. AesDecryptionFailed, - /// An unspecified error occurred. - Undefined, } impl CommandError { @@ -151,7 +149,6 @@ impl fmt::Display for CommandError { CommandError::NotSupported => "This command is not supported by this device", CommandError::UnknownCommand => "This command is unknown", CommandError::AesDecryptionFailed => "AES decryption failed", - CommandError::Undefined => "An unspecified error occurred", }) } } -- 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(-) 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