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/util.rs | 113 +----------------------------------------------------------- 1 file changed, 1 insertion(+), 112 deletions(-) (limited to 'src/util.rs') 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.1 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. --- src/util.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) (limited to 'src/util.rs') 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.1 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. --- src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/util.rs') 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.1 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. --- src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/util.rs') 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)), } } -- cgit v1.2.1 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. --- src/util.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/util.rs') 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 { -- cgit v1.2.1 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/util.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/util.rs') 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, }; } -- cgit v1.2.1 From 601bc22ae18838ff56b64c15b365bcf7f93006be Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Wed, 23 Jan 2019 02:44:20 +0000 Subject: Prefer into() over numeric casting Numeric casting might truncate an integer, while into() is only implemented for numeric types if the cast is possible without truncation. --- src/util.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/util.rs') diff --git a/src/util.rs b/src/util.rs index 79b8c34..2542a7b 100644 --- a/src/util.rs +++ b/src/util.rs @@ -59,8 +59,7 @@ pub fn get_command_result(value: c_int) -> Result<(), Error> { } pub fn get_last_result() -> Result<(), Error> { - let value = unsafe { nitrokey_sys::NK_get_last_command_status() } as c_int; - get_command_result(value) + get_command_result(unsafe { nitrokey_sys::NK_get_last_command_status() }.into()) } pub fn get_last_error() -> Error { -- cgit v1.2.1 From 5540ca5e76ffe5efe27d8819efb9e62066a10219 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Wed, 23 Jan 2019 03:46:09 +0000 Subject: Refactor and clean up all code This includes: - using idiomatic Rust - limiting the scope of unsafe blocks - simplifying code --- src/util.rs | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) (limited to 'src/util.rs') diff --git a/src/util.rs b/src/util.rs index 2542a7b..f8ad9c9 100644 --- a/src/util.rs +++ b/src/util.rs @@ -29,32 +29,31 @@ pub enum LogLevel { } pub fn owned_str_from_ptr(ptr: *const c_char) -> String { - unsafe { - return CStr::from_ptr(ptr).to_string_lossy().into_owned(); - } + unsafe { CStr::from_ptr(ptr) } + .to_string_lossy() + .into_owned() } pub fn result_from_string(ptr: *const c_char) -> Result { if ptr.is_null() { return Err(Error::UnexpectedError); } - unsafe { - let s = owned_str_from_ptr(ptr); - free(ptr as *mut c_void); - // An empty string can both indicate an error or be a valid return value. In this case, we - // have to check the last command status to decide what to return. - if s.is_empty() { - get_last_result().map(|_| s) - } else { - Ok(s) - } + let s = owned_str_from_ptr(ptr); + unsafe { free(ptr as *mut c_void) }; + // An empty string can both indicate an error or be a valid return value. In this case, we + // have to check the last command status to decide what to return. + if s.is_empty() { + get_last_result().map(|_| s) + } else { + Ok(s) } } pub fn get_command_result(value: c_int) -> Result<(), Error> { - match value { - 0 => Ok(()), - other => Err(Error::from(other)), + if value == 0 { + Ok(()) + } else { + Err(Error::from(value)) } } @@ -63,10 +62,10 @@ pub fn get_last_result() -> Result<(), Error> { } pub fn get_last_error() -> Error { - return match get_last_result() { + match get_last_result() { Ok(()) => Error::UnexpectedError, Err(err) => err, - }; + } } pub fn generate_password(length: usize) -> Result, Error> { -- cgit v1.2.1 From d4663961c41a0fb6f81f4a54aefd0fedce49d350 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Wed, 23 Jan 2019 04:27:14 +0000 Subject: Return UTF-8 error if libnitrokey returns an invalid string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, we used lossy UTF-8 conversion. Yet the user should be notified if we have a problem instead of silently changing the data. Therefore, we now return an error if we enocunter an invalid UTF-8 string. This leads to a change in `get_library_version`’s signature. --- src/util.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'src/util.rs') diff --git a/src/util.rs b/src/util.rs index f8ad9c9..64dde39 100644 --- a/src/util.rs +++ b/src/util.rs @@ -28,17 +28,18 @@ pub enum LogLevel { DebugL2, } -pub fn owned_str_from_ptr(ptr: *const c_char) -> String { +pub fn owned_str_from_ptr(ptr: *const c_char) -> Result { unsafe { CStr::from_ptr(ptr) } - .to_string_lossy() - .into_owned() + .to_str() + .map(String::from) + .map_err(Error::from) } pub fn result_from_string(ptr: *const c_char) -> Result { if ptr.is_null() { return Err(Error::UnexpectedError); } - let s = owned_str_from_ptr(ptr); + let s = owned_str_from_ptr(ptr)?; unsafe { free(ptr as *mut c_void) }; // An empty string can both indicate an error or be a valid return value. In this case, we // have to check the last command status to decide what to return. -- cgit v1.2.1 From fdb7bac3063e62776bfc13f184cf786da19f42d1 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Wed, 23 Jan 2019 16:33:26 +0100 Subject: Add license and copyright information This patch adds license and copyright information to all files to make nitrokey-rs compliant with the REUSE practices [0]. [0] https://reuse.software/practices/2.0/ --- src/util.rs | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/util.rs') diff --git a/src/util.rs b/src/util.rs index 64dde39..5f25655 100644 --- a/src/util.rs +++ b/src/util.rs @@ -1,3 +1,6 @@ +// Copyright (C) 2018-2019 Robin Krahl +// SPDX-License-Identifier: MIT + use std::ffi::{CStr, CString}; use std::os::raw::{c_char, c_int}; -- cgit v1.2.1 From 809d31a4273505487febb2dd281376d2bb3766ab Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Fri, 25 Jan 2019 18:46:57 +0000 Subject: Remove rand_core::Error from public API rand_core does not have a stable release yet, and it is unlikely that there will be one soon. To be able to stabilize nitrokey without waiting for a stable rand_core version, we remove the rand_core::Error type from the public API and replace it with a Box. --- src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/util.rs') diff --git a/src/util.rs b/src/util.rs index 5f25655..d87cd7c 100644 --- a/src/util.rs +++ b/src/util.rs @@ -73,7 +73,7 @@ pub fn get_last_error() -> Error { } pub fn generate_password(length: usize) -> Result, Error> { - let mut rng = OsRng::new()?; + let mut rng = OsRng::new().map_err(|err| Error::RandError(Box::new(err)))?; let mut data = vec![0u8; length]; rng.fill_bytes(&mut data[..]); Ok(data) -- cgit v1.2.1 From 3cab3525e9c0bd743aa419d286de38d346776fbd Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 27 Jan 2019 13:42:12 +0000 Subject: Replace or with or_else in get_cstring To avoid unnecessary function calls, we replace the or with an or_else in get_cstring. --- src/util.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/util.rs') diff --git a/src/util.rs b/src/util.rs index d87cd7c..b7e8cd3 100644 --- a/src/util.rs +++ b/src/util.rs @@ -80,7 +80,7 @@ pub fn generate_password(length: usize) -> Result, Error> { } pub fn get_cstring>>(s: T) -> Result { - CString::new(s).or(Err(LibraryError::InvalidString.into())) + CString::new(s).or_else(|_| Err(LibraryError::InvalidString.into())) } impl Into for LogLevel { -- cgit v1.2.1 From c30cbd35ba187cd6e5055d3beb8420b11fb030ec Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 27 Jan 2019 23:23:00 +0000 Subject: Always return a Result when communicating with a device Previously, we sometimes returned a value without wrapping it in a result if the API method did not indicate errors in the return value. But we can detect errors using the NK_get_last_command_status function. This patch changes the return types of these methods to Result<_, Error> and adds error checks. --- src/util.rs | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/util.rs') diff --git a/src/util.rs b/src/util.rs index b7e8cd3..fdb73c3 100644 --- a/src/util.rs +++ b/src/util.rs @@ -53,6 +53,10 @@ pub fn result_from_string(ptr: *const c_char) -> Result { } } +pub fn result_or_error(value: T) -> Result { + get_last_result().and(Ok(value)) +} + pub fn get_command_result(value: c_int) -> Result<(), Error> { if value == 0 { Ok(()) -- cgit v1.2.1 From 6c138eaa850c745b97b7e48a201db0cbaad8e1e0 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Tue, 16 Jul 2019 08:58:37 +0000 Subject: Update rand_{core,os} dependencies This patch updates the rand_core dependency to version 0.5 and the rand_os dependency to version 0.2. This causes a change in util.rs: Instead of constructing an OsRng instance using OsRng::new(), we can directly instantiate the (now empty) struct. --- src/util.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/util.rs') diff --git a/src/util.rs b/src/util.rs index fdb73c3..a5dd1e5 100644 --- a/src/util.rs +++ b/src/util.rs @@ -77,9 +77,8 @@ pub fn get_last_error() -> Error { } pub fn generate_password(length: usize) -> Result, Error> { - let mut rng = OsRng::new().map_err(|err| Error::RandError(Box::new(err)))?; let mut data = vec![0u8; length]; - rng.fill_bytes(&mut data[..]); + OsRng.fill_bytes(&mut data[..]); Ok(data) } -- cgit v1.2.1 From 0bffc7931e011b4c0c046ed7608fbe9b7a1ffd19 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Fri, 27 Dec 2019 23:07:26 +0100 Subject: Replace rand_os::OsRng with rand_core::OsRng rand_os::OsRng has been deprecated. Instead we can use rand_core with the getrandom feature. --- src/util.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/util.rs') diff --git a/src/util.rs b/src/util.rs index a5dd1e5..5a56c55 100644 --- a/src/util.rs +++ b/src/util.rs @@ -5,8 +5,7 @@ use std::ffi::{CStr, CString}; use std::os::raw::{c_char, c_int}; use libc::{c_void, free}; -use rand_core::RngCore; -use rand_os::OsRng; +use rand_core::{OsRng, RngCore}; use crate::error::{Error, LibraryError}; -- cgit v1.2.1