From 65ead1426e1e9864b7e245fd729a66e689e549b4 Mon Sep 17 00:00:00 2001 From: Daniel Mueller Date: Wed, 2 Jan 2019 15:53:18 -0800 Subject: Preserve unknown error code values The CommandError::Unknown variant, which is used whenever a reported error code is not known, makes it close to impossible to determine the root cause of, say, a one-off error, because all information explaining what went wrong is discarded. With this change we adjust the Unknown variant to include the error report. In addition, we introduce a new CommandError variant, Undefined, that is used when no error code is available. --- src/device.rs | 8 ++++---- src/util.rs | 59 +++++++++++++++++++++++++++++++++++++---------------------- tests/pws.rs | 18 +++++++++--------- 3 files changed, 50 insertions(+), 35 deletions(-) diff --git a/src/device.rs b/src/device.rs index 9c6608d..33e5410 100644 --- a/src/device.rs +++ b/src/device.rs @@ -532,9 +532,9 @@ pub fn connect() -> Result { match nitrokey_sys::NK_login_auto() { 1 => match get_connected_device() { Some(wrapper) => Ok(wrapper), - None => Err(CommandError::Unknown), + None => Err(CommandError::Undefined), }, - _ => Err(CommandError::Unknown), + _ => Err(CommandError::Undefined), } } } @@ -623,7 +623,7 @@ impl Pro { // TODO: maybe Option instead of Result? match connect_model(Model::Pro) { true => Ok(Pro {}), - false => Err(CommandError::Unknown), + false => Err(CommandError::Undefined), } } } @@ -663,7 +663,7 @@ impl Storage { // TODO: maybe Option instead of Result? match connect_model(Model::Storage) { true => Ok(Storage {}), - false => Err(CommandError::Unknown), + false => Err(CommandError::Undefined), } } diff --git a/src/util.rs b/src/util.rs index 315cb34..1ecc0b7 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,3 +1,4 @@ +use std::borrow; use std::ffi::{CStr, CString}; use std::fmt; use std::os::raw::{c_char, c_int}; @@ -30,7 +31,9 @@ pub enum CommandError { /// AES decryption failed. AesDecryptionFailed, /// An unknown error occurred. - Unknown, + Unknown(i64), + /// An unspecified error occurred. + Undefined, /// You passed a string containing a null byte. InvalidString, /// You passed an invalid slot. @@ -66,7 +69,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::Unknown); + return Err(CommandError::Undefined); } unsafe { let s = owned_str_from_ptr(ptr); @@ -92,7 +95,7 @@ pub fn get_last_result() -> Result<(), CommandError> { pub fn get_last_error() -> CommandError { return match get_last_result() { - Ok(()) => CommandError::Unknown, + Ok(()) => CommandError::Undefined, Err(err) => err, }; } @@ -107,26 +110,38 @@ pub fn get_cstring>>(s: T) -> Result { CString::new(s).or(Err(CommandError::InvalidString)) } -impl fmt::Display for CommandError { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - let msg = 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", +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" + "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", - 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::Unknown => "An unknown error occurred", - CommandError::InvalidString => "You passed a string containing a null byte", - CommandError::InvalidSlot => "The given slot is invalid", - }; - write!(f, "{}", msg) + 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::InvalidSlot => "The given slot is invalid".into(), + } + } +} + +impl fmt::Display for CommandError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.as_str()) } } @@ -144,7 +159,7 @@ impl From for CommandError { 9 => CommandError::UnknownCommand, 10 => CommandError::AesDecryptionFailed, 201 => CommandError::InvalidSlot, - _ => CommandError::Unknown, + x => CommandError::Unknown(x.into()), } } } diff --git a/tests/pws.rs b/tests/pws.rs index 875324b..5061298 100644 --- a/tests/pws.rs +++ b/tests/pws.rs @@ -11,7 +11,7 @@ use crate::util::{Target, 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::Unknown); + return Err(CommandError::Undefined); } let s = unsafe { CStr::from_ptr(ptr).to_string_lossy().into_owned() }; unsafe { free(ptr as *mut c_void) }; @@ -19,7 +19,7 @@ fn get_slot_name_direct(slot: u8) -> Result { true => { let error = unsafe { nitrokey_sys::NK_get_last_command_status() } as c_int; match error { - 0 => Err(CommandError::Unknown), + 0 => Err(CommandError::Undefined), other => Err(CommandError::from(other)), } } @@ -97,9 +97,9 @@ fn get_data() { assert!(pws.erase_slot(1).is_ok()); // TODO: check error codes - assert_eq!(Err(CommandError::Unknown), pws.get_slot_name(1)); - assert_eq!(Err(CommandError::Unknown), pws.get_slot_login(1)); - assert_eq!(Err(CommandError::Unknown), pws.get_slot_password(1)); + assert_eq!(Err(CommandError::Undefined), pws.get_slot_name(1)); + assert_eq!(Err(CommandError::Undefined), pws.get_slot_login(1)); + assert_eq!(Err(CommandError::Undefined), pws.get_slot_password(1)); let name = "with å"; let login = "pär@test.com"; @@ -135,19 +135,19 @@ fn write() { ); assert!(pws.write_slot(0, "", "login", "password").is_ok()); - assert_eq!(Err(CommandError::Unknown), pws.get_slot_name(0)); + assert_eq!(Err(CommandError::Undefined), 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!(pws.write_slot(0, "name", "", "password").is_ok()); assert_eq!(Ok(String::from("name")), pws.get_slot_name(0)); - assert_eq!(Err(CommandError::Unknown), pws.get_slot_login(0)); + assert_eq!(Err(CommandError::Undefined), pws.get_slot_login(0)); assert_eq!(Ok(String::from("password")), pws.get_slot_password(0)); assert!(pws.write_slot(0, "name", "login", "").is_ok()); 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::Unknown), pws.get_slot_password(0)); + assert_eq!(Err(CommandError::Undefined), pws.get_slot_password(0)); } #[test] @@ -160,5 +160,5 @@ fn erase() { assert!(pws.write_slot(0, "name", "login", "password").is_ok()); assert!(pws.erase_slot(0).is_ok()); assert!(pws.erase_slot(0).is_ok()); - assert_eq!(Err(CommandError::Unknown), pws.get_slot_name(0)); + assert_eq!(Err(CommandError::Undefined), pws.get_slot_name(0)); } -- cgit v1.2.1