Merge branch 'release-0.6.0'

6 files changed, 252 insertions, 117 deletions

diff --git a/src/config.rs b/src/config.rs
index cb678d7..120a51b 100644
--- a/src/config.rs
+++ b/src/config.rs
@@ -83,13 +83,13 @@ impl convert::TryFrom<Config> for RawConfig {
     }
 }
 
-impl From<[u8; 5]> for RawConfig {
-    fn from(data: [u8; 5]) -> Self {
-        RawConfig {
-            numlock: data[0],
-            capslock: data[1],
-            scrollock: data[2],
-            user_password: data[3] != 0,
+impl From<&nitrokey_sys::NK_status> for RawConfig {
+    fn from(status: &nitrokey_sys::NK_status) -> Self {
+        Self {
+            numlock: status.config_numlock,
+            capslock: status.config_capslock,
+            scrollock: status.config_scrolllock,
+            user_password: status.otp_user_password,
         }
     }
 }
diff --git a/src/device/mod.rs b/src/device/mod.rs
index 0234bf0..067fdf6 100644
--- a/src/device/mod.rs
+++ b/src/device/mod.rs
@@ -8,18 +8,17 @@ mod wrapper;
 use std::convert::{TryFrom, TryInto};
 use std::ffi;
 use std::fmt;
+use std::str;
 
-use libc;
 use nitrokey_sys;
 
 use crate::auth::Authenticate;
 use crate::config::{Config, RawConfig};
-use crate::error::{CommunicationError, Error};
+use crate::error::{CommunicationError, Error, LibraryError};
 use crate::otp::GenerateOtp;
 use crate::pws::GetPasswordSafe;
 use crate::util::{
-    get_command_result, get_cstring, get_last_error, owned_str_from_ptr, result_from_string,
-    result_or_error,
+    get_command_result, get_cstring, owned_str_from_ptr, result_or_error, run_with_string,
 };
 
 pub use pro::Pro;
@@ -71,6 +70,98 @@ impl TryFrom<nitrokey_sys::NK_device_model> for Model {
     }
 }
 
+/// Serial number of a Nitrokey device.
+///
+/// The serial number can be formatted as a string using the [`ToString`][] trait, and it can be
+/// parsed from a string using the [`FromStr`][] trait.  It can also be represented as a 32-bit
+/// unsigned integer using [`as_u32`][].  This integer is the ID of the smartcard of the Nitrokey
+/// device.
+///
+/// Neither the format of the string representation nor the integer representation are guaranteed
+/// to stay the same for new firmware versions.
+///
+/// [`as_u32`]: #method.as_u32
+/// [`FromStr`]: #impl-FromStr
+/// [`ToString`]: #impl-ToString
+#[derive(Clone, Copy, Debug, PartialEq)]
+pub struct SerialNumber {
+    value: u32,
+}
+
+impl SerialNumber {
+    /// Creates an emtpty serial number.
+    ///
+    /// This function can be used to create a placeholder value or to compare a `SerialNumber`
+    /// instance with an empty serial number.
+    pub fn empty() -> Self {
+        SerialNumber::new(0)
+    }
+
+    fn new(value: u32) -> Self {
+        SerialNumber { value }
+    }
+
+    /// Returns the integer reprensentation of this serial number.
+    ///
+    /// This integer currently is the ID of the smartcard of the Nitrokey device.  Upcoming
+    /// firmware versions might change the meaning of this representation, or add additional
+    /// components to the serial number.
+    // To provide a stable API even if the internal representation of SerialNumber changes, we want
+    // to borrow SerialNumber instead of copying it even if it might be less efficient.
+    #[allow(clippy::trivially_copy_pass_by_ref)]
+    pub fn as_u32(&self) -> u32 {
+        self.value
+    }
+}
+
+impl fmt::Display for SerialNumber {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{:#010x}", self.value)
+    }
+}
+
+impl str::FromStr for SerialNumber {
+    type Err = Error;
+
+    /// Try to parse a serial number from a hex string.
+    ///
+    /// The input string must be a valid hex string.  Optionally, it can include a `0x` prefix.
+    ///
+    /// # Errors
+    ///
+    /// - [`InvalidHexString`][] if the given string is not a valid hex string
+    ///
+    /// # Example
+    ///
+    /// ```no_run
+    /// use std::convert::TryFrom;
+    /// use nitrokey::{DeviceInfo, Error, SerialNumber};
+    ///
+    /// fn find_device(serial_number: &str) -> Result<Option<DeviceInfo>, Error> {
+    ///     let serial_number: SerialNumber = serial_number.parse()?;
+    ///     Ok(nitrokey::list_devices()?
+    ///         .into_iter()
+    ///         .filter(|device| device.serial_number == Some(serial_number))
+    ///         .next())
+    /// }
+    ///
+    /// ```
+    ///
+    /// [`InvalidHexString`]: enum.LibraryError.html#variant.InvalidHexString
+    fn from_str(s: &str) -> Result<SerialNumber, Error> {
+        // ignore leading 0x
+        let hex_string = if s.starts_with("0x") {
+            s.split_at(2).1
+        } else {
+            s
+        };
+
+        u32::from_str_radix(hex_string, 16)
+            .map(SerialNumber::new)
+            .map_err(|_| LibraryError::InvalidHexString.into())
+    }
+}
+
 /// Connection information for a Nitrokey device.
 #[derive(Clone, Debug, PartialEq)]
 pub struct DeviceInfo {
@@ -78,9 +169,8 @@ pub struct DeviceInfo {
     pub model: Option<Model>,
     /// The USB device path.
     pub path: String,
-    /// The serial number as a 8-character hex string, or `None` if the device does not expose its
-    /// serial number.
-    pub serial_number: Option<String>,
+    /// The serial number of the device, or `None` if the device does not expose its serial number.
+    pub serial_number: Option<SerialNumber>,
 }
 
 impl TryFrom<&nitrokey_sys::NK_device_info> for DeviceInfo {
@@ -110,45 +200,42 @@ impl fmt::Display for DeviceInfo {
             None => write!(f, "Unsupported Nitrokey model")?,
         }
         write!(f, " at {} with ", self.path)?;
-        match &self.serial_number {
-            Some(ref serial_number) => write!(f, "serial no. {}", serial_number),
+        match self.serial_number {
+            Some(serial_number) => write!(f, "serial no. {}", serial_number),
             None => write!(f, "an unknown serial number"),
         }
     }
 }
 
-/// Parses a serial number returned by hidapi and transforms it to the Nitrokey format.
+/// Parses a serial number returned by hidapi.
 ///
 /// If the serial number is all zero, this function returns `None`.  Otherwise, it uses the last
-/// eight characters.  If these are all zero, the first eight characters are used instead.  This
-/// function also makes sure that the returned string is lowercase, consistent with libnitrokey’s
-/// hex string formatting.
+/// eight characters.  If these are all zero, the first eight characters are used instead.  The
+/// selected substring is parse as a hex string and its integer value is returned from the
+/// function.  If the string cannot be parsed, this function returns `None`.
 ///
 /// The reason for this behavior is that the Nitrokey Storage does not report its serial number at
 /// all (all zero value), while the Nitrokey Pro with firmware 0.9 or later writes its serial
 /// number to the last eight characters.  Nitrokey Pro devices with firmware 0.8 or earlier wrote
 /// their serial number to the first eight characters.
-fn get_hidapi_serial_number(serial_number: &str) -> Option<String> {
+fn get_hidapi_serial_number(serial_number: &str) -> Option<SerialNumber> {
     let len = serial_number.len();
     if len < 8 {
-        // The serial number in the USB descriptor has 12 bytes, we need at least four of them
+        // The serial number in the USB descriptor has 12 bytes, we need at least four
         return None;
     }
 
     let iter = serial_number.char_indices().rev();
     let first_non_null = iter.skip_while(|(_, c)| *c == '0').next();
     if let Some((i, _)) = first_non_null {
-        if len - i < 8 {
+        let substr = if len - i < 8 {
             // The last eight characters contain at least one non-zero character --> use them
-            let mut serial_number = serial_number.split_at(len - 8).1.to_string();
-            serial_number.make_ascii_lowercase();
-            Some(serial_number)
+            serial_number.split_at(len - 8).1
         } else {
             // The last eight characters are all zero --> use the first eight
-            let mut serial_number = serial_number.split_at(8).0.to_string();
-            serial_number.make_ascii_lowercase();
-            Some(serial_number)
-        }
+            serial_number.split_at(8).0
+        };
+        substr.parse().ok()
     } else {
         // The serial number is all zero
         None
@@ -176,7 +263,7 @@ pub struct Status {
     /// The firmware version of the device.
     pub firmware_version: FirmwareVersion,
     /// The serial number of the device.
-    pub serial_number: u32,
+    pub serial_number: SerialNumber,
     /// The configuration of the device.
     pub config: Config,
 }
@@ -188,14 +275,8 @@ impl From<nitrokey_sys::NK_status> for Status {
                 major: status.firmware_version_major,
                 minor: status.firmware_version_minor,
             },
-            serial_number: status.serial_number_smart_card,
-            config: RawConfig {
-                numlock: status.config_numlock,
-                capslock: status.config_capslock,
-                scrollock: status.config_scrolllock,
-                user_password: status.otp_user_password,
-            }
-            .into(),
+            serial_number: SerialNumber::new(status.serial_number_smart_card),
+            config: RawConfig::from(&status).into(),
         }
     }
 }
@@ -263,7 +344,7 @@ pub trait Device<'a>: Authenticate<'a> + GetPasswordSafe<'a> + GenerateOtp + fmt
     /// let device = manager.connect()?;
     /// let status = device.get_status()?;
     /// println!("Firmware version: {}", status.firmware_version);
-    /// println!("Serial number:    {:x}", status.serial_number);
+    /// println!("Serial number:    {}", status.serial_number);
     /// # Ok::<(), nitrokey::Error>(())
     /// ```
     ///
@@ -273,8 +354,9 @@ pub trait Device<'a>: Authenticate<'a> + GetPasswordSafe<'a> + GenerateOtp + fmt
     /// [`StorageStatus`]: struct.StorageStatus.html
     fn get_status(&self) -> Result<Status, Error>;
 
-    /// Returns the serial number of the Nitrokey device.  The serial number is the string
-    /// representation of a hex number.
+    /// Returns the serial number of the Nitrokey device.
+    ///
+    /// For display purpuses, the serial number should be formatted as an 8-digit hex string.
     ///
     /// # Example
     ///
@@ -292,8 +374,10 @@ pub trait Device<'a>: Authenticate<'a> + GetPasswordSafe<'a> + GenerateOtp + fmt
     /// #     Ok(())
     /// # }
     /// ```
-    fn get_serial_number(&self) -> Result<String, Error> {
-        result_from_string(unsafe { nitrokey_sys::NK_device_serial_number() })
+    fn get_serial_number(&self) -> Result<SerialNumber, Error> {
+        run_with_string(unsafe { nitrokey_sys::NK_device_serial_number() }, |s| {
+            s.parse()
+        })
     }
 
     /// Returns the number of remaining authentication attempts for the user.  The total number of
@@ -388,14 +472,17 @@ pub trait Device<'a>: Authenticate<'a> + GetPasswordSafe<'a> + GenerateOtp + fmt
     /// # }
     /// ```
     fn get_config(&self) -> Result<Config, Error> {
-        let config_ptr = unsafe { nitrokey_sys::NK_read_config() };
-        if config_ptr.is_null() {
-            return Err(get_last_error());
-        }
-        let config_array_ptr = config_ptr as *const [u8; 5];
-        let raw_config = unsafe { RawConfig::from(*config_array_ptr) };
-        unsafe { libc::free(config_ptr as *mut libc::c_void) };
-        Ok(raw_config.into())
+        let mut raw_status = nitrokey_sys::NK_status {
+            firmware_version_major: 0,
+            firmware_version_minor: 0,
+            serial_number_smart_card: 0,
+            config_numlock: 0,
+            config_capslock: 0,
+            config_scrolllock: 0,
+            otp_user_password: false,
+        };
+        get_command_result(unsafe { nitrokey_sys::NK_get_status(&mut raw_status) })?;
+        Ok(RawConfig::from(&raw_status).into())
     }
 
     /// Changes the administrator PIN.
@@ -625,44 +712,77 @@ pub(crate) fn connect_enum(model: Model) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use super::get_hidapi_serial_number;
+    use std::str::FromStr;
+
+    use super::{get_hidapi_serial_number, LibraryError, SerialNumber};
 
     #[test]
-    fn hidapi_serial_number() {
-        assert_eq!(None, get_hidapi_serial_number(""));
-        assert_eq!(None, get_hidapi_serial_number("00000000000000000"));
-        assert_eq!(None, get_hidapi_serial_number("1234"));
-        assert_eq!(
-            Some("00001234".to_string()),
-            get_hidapi_serial_number("00001234")
-        );
-        assert_eq!(
-            Some("00001234".to_string()),
-            get_hidapi_serial_number("000000001234")
-        );
-        assert_eq!(
-            Some("00001234".to_string()),
-            get_hidapi_serial_number("100000001234")
-        );
-        assert_eq!(
-            Some("12340000".to_string()),
-            get_hidapi_serial_number("123400000000")
-        );
-        assert_eq!(
-            Some("00005678".to_string()),
-            get_hidapi_serial_number("000000000000000000005678")
-        );
-        assert_eq!(
-            Some("00001234".to_string()),
-            get_hidapi_serial_number("000012340000000000000000")
-        );
-        assert_eq!(
-            Some("0000ffff".to_string()),
-            get_hidapi_serial_number("00000000000000000000FFFF")
-        );
-        assert_eq!(
-            Some("0000ffff".to_string()),
-            get_hidapi_serial_number("00000000000000000000ffff")
-        );
+    fn test_serial_number_display() {
+        fn assert_str(s: &str, n: u32) {
+            assert_eq!(s.to_owned(), SerialNumber::new(n).to_string());
+        }
+
+        assert_str("0x00000000", 0);
+        assert_str("0x00001000", 0x1000);
+        assert_str("0x12345678", 0x12345678);
+    }
+
+    #[test]
+    fn test_serial_number_try_from() {
+        fn assert_ok(v: u32, s: &str) {
+            assert_eq!(SerialNumber::new(v), SerialNumber::from_str(s).unwrap());
+            assert_eq!(
+                SerialNumber::new(v),
+                SerialNumber::from_str(format!("0x{}", s).as_ref()).unwrap()
+            );
+        }
+
+        fn assert_err(s: &str) {
+            match SerialNumber::from_str(s).unwrap_err() {
+                super::Error::LibraryError(LibraryError::InvalidHexString) => {}
+                err => assert!(
+                    false,
+                    "expected InvalidHexString error, got {} (input {})",
+                    err, s
+                ),
+            }
+        }
+
+        assert_ok(0x1234, "1234");
+        assert_ok(0x1234, "01234");
+        assert_ok(0x1234, "001234");
+        assert_ok(0x1234, "0001234");
+
+        assert_ok(0, "0");
+        assert_ok(0xdeadbeef, "deadbeef");
+
+        assert_err("deadpork");
+        assert_err("blubb");
+        assert_err("");
+    }
+
+    #[test]
+    fn test_get_hidapi_serial_number() {
+        fn assert_none(s: &str) {
+            assert_eq!(None, get_hidapi_serial_number(s));
+        }
+
+        fn assert_some(n: u32, s: &str) {
+            assert_eq!(Some(SerialNumber::new(n)), get_hidapi_serial_number(s));
+        }
+
+        assert_none("");
+        assert_none("00000000000000000");
+        assert_none("blubb");
+        assert_none("1234");
+
+        assert_some(0x1234, "00001234");
+        assert_some(0x1234, "000000001234");
+        assert_some(0x1234, "100000001234");
+        assert_some(0x12340000, "123400000000");
+        assert_some(0x5678, "000000000000000000005678");
+        assert_some(0x1234, "000012340000000000000000");
+        assert_some(0xffff, "00000000000000000000FFFF");
+        assert_some(0xffff, "00000000000000000000ffff");
     }
 }
diff --git a/src/device/storage.rs b/src/device/storage.rs
index deb2844..5669a91 100644
--- a/src/device/storage.rs
+++ b/src/device/storage.rs
@@ -7,7 +7,7 @@ use std::ops;
 
 use nitrokey_sys;
 
-use crate::device::{Device, FirmwareVersion, Model, Status};
+use crate::device::{Device, FirmwareVersion, Model, SerialNumber, Status};
 use crate::error::{CommandError, Error};
 use crate::otp::GenerateOtp;
 use crate::util::{get_command_result, get_cstring, get_last_error};
@@ -678,7 +678,7 @@ impl<'a> Storage<'a> {
                 if usage_data.write_level_min > usage_data.write_level_max
                     || usage_data.write_level_max > 100
                 {
-                    Err(Error::UnexpectedError)
+                    Err(Error::UnexpectedError("Invalid write levels".to_owned()))
                 } else {
                     Ok(ops::Range {
                         start: usage_data.write_level_min,
@@ -708,10 +708,14 @@ impl<'a> Storage<'a> {
         match status {
             0..=100 => u8::try_from(status)
                 .map(OperationStatus::Ongoing)
-                .map_err(|_| Error::UnexpectedError),
+                .map_err(|_| {
+                    Error::UnexpectedError("Cannot create u8 from operation status".to_owned())
+                }),
             -1 => Ok(OperationStatus::Idle),
             -2 => Err(get_last_error()),
-            _ => Err(Error::UnexpectedError),
+            _ => Err(Error::UnexpectedError(
+                "Invalid operation status".to_owned(),
+            )),
         }
     }
 
@@ -823,7 +827,7 @@ impl<'a> Device<'a> for Storage<'a> {
 
         let storage_status = self.get_storage_status()?;
         status.firmware_version = storage_status.firmware_version;
-        status.serial_number = storage_status.serial_number_smart_card;
+        status.serial_number = SerialNumber::new(storage_status.serial_number_smart_card);
 
         Ok(status)
     }
diff --git a/src/error.rs b/src/error.rs
index f9af594..7bea3f2 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -25,7 +25,7 @@ pub enum Error {
     /// An error that occurred during random number generation.
     RandError(Box<dyn error::Error>),
     /// An error that is caused by an unexpected value returned by libnitrokey.
-    UnexpectedError,
+    UnexpectedError(String),
     /// An unknown error returned by libnitrokey.
     UnknownError(i64),
     /// An error caused by a Nitrokey model that is not supported by this crate.
@@ -102,7 +102,7 @@ impl error::Error for Error {
             Error::LibraryError(ref err) => Some(err),
             Error::PoisonError(ref err) => Some(err),
             Error::RandError(ref err) => Some(err.as_ref()),
-            Error::UnexpectedError => None,
+            Error::UnexpectedError(_) => None,
             Error::UnknownError(_) => None,
             Error::UnsupportedModelError => None,
             Error::Utf8Error(ref err) => Some(err),
@@ -119,7 +119,7 @@ impl fmt::Display for Error {
             Error::LibraryError(ref err) => write!(f, "Library error: {}", err),
             Error::PoisonError(_) => write!(f, "Internal error: poisoned lock"),
             Error::RandError(ref err) => write!(f, "RNG error: {}", err),
-            Error::UnexpectedError => write!(f, "An unexpected error occurred"),
+            Error::UnexpectedError(ref s) => write!(f, "An unexpected error occurred: {}", s),
             Error::UnknownError(ref err) => write!(f, "Unknown error: {}", err),
             Error::UnsupportedModelError => write!(f, "Unsupported Nitrokey model"),
             Error::Utf8Error(ref err) => write!(f, "UTF-8 error: {}", err),
diff --git a/src/lib.rs b/src/lib.rs
index 9efad91..92247d7 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -138,8 +138,8 @@ use nitrokey_sys;
 pub use crate::auth::{Admin, Authenticate, User};
 pub use crate::config::Config;
 pub use crate::device::{
-    Device, DeviceInfo, DeviceWrapper, Model, OperationStatus, Pro, SdCardData, Status, Storage,
-    StorageProductionInfo, StorageStatus, VolumeMode, VolumeStatus,
+    Device, DeviceInfo, DeviceWrapper, Model, OperationStatus, Pro, SdCardData, SerialNumber,
+    Status, Storage, StorageProductionInfo, StorageStatus, VolumeMode, VolumeStatus,
 };
 pub use crate::error::{CommandError, CommunicationError, Error, LibraryError};
 pub use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData};
diff --git a/src/util.rs b/src/util.rs
index a0d0d1b..b17b071 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -30,26 +30,38 @@ pub enum LogLevel {
     DebugL2,
 }
 
+pub fn str_from_ptr<'a>(ptr: *const c_char) -> Result<&'a str, Error> {
+    unsafe { CStr::from_ptr(ptr) }.to_str().map_err(Error::from)
+}
+
 pub fn owned_str_from_ptr(ptr: *const c_char) -> Result<String, Error> {
-    unsafe { CStr::from_ptr(ptr) }
-        .to_str()
-        .map(String::from)
-        .map_err(Error::from)
+    str_from_ptr(ptr).map(ToOwned::to_owned)
 }
 
-pub fn result_from_string(ptr: *const c_char) -> Result<String, Error> {
+pub fn run_with_string<R, F>(ptr: *const c_char, op: F) -> Result<R, Error>
+where
+    F: FnOnce(&str) -> Result<R, Error>,
+{
     if ptr.is_null() {
-        return Err(Error::UnexpectedError);
+        return Err(Error::UnexpectedError(
+            "libnitrokey returned a null pointer".to_owned(),
+        ));
     }
-    let s = owned_str_from_ptr(ptr)?;
+    let result = str_from_ptr(ptr).and_then(op);
     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)
-    }
+    result
+}
+
+pub fn result_from_string(ptr: *const c_char) -> Result<String, Error> {
+    run_with_string(ptr, |s| {
+        // 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.to_owned())
+        } else {
+            Ok(s.to_owned())
+        }
+    })
 }
 
 pub fn result_or_error<T>(value: T) -> Result<T, Error> {
@@ -69,10 +81,9 @@ pub fn get_last_result() -> Result<(), Error> {
 }
 
 pub fn get_last_error() -> Error {
-    match get_last_result() {
-        Ok(()) => Error::UnexpectedError,
-        Err(err) => err,
-    }
+    get_last_result().err().unwrap_or_else(|| {
+        Error::UnexpectedError("Expected an error, but command status is zero".to_owned())
+    })
 }
 
 pub fn generate_password(length: usize) -> Result<CString, Error> {