Always store serial numbers as integers

This patch consistently uses u32 integers to store and return the serial
number of a Nitrokey device.  This makes it easier to convert and
compare the serial number, as it is a unique representation and as
formatting an integer cannot fail.  For more details, see this RFC:
	https://lists.sr.ht/~ireas/nitrokey-rs-dev/%3C20200126074816.GA1314%40ireas.org%3E

1 files changed, 62 insertions, 46 deletions

diff --git a/src/device/mod.rs b/src/device/mod.rs
index 0234bf0..a25ad1b 100644
--- a/src/device/mod.rs
+++ b/src/device/mod.rs
@@ -78,9 +78,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<u32>,
 }
 
 impl TryFrom<&nitrokey_sys::NK_device_info> for DeviceInfo {
@@ -110,45 +109,48 @@ 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. 0x{:08x}", serial_number),
             None => write!(f, "an unknown serial number"),
         }
     }
 }
 
-/// Parses a serial number returned by hidapi and transforms it to the Nitrokey format.
+/// Parses the given hex string and returns its integer representation.
+fn parse_serial_number<S: AsRef<str>>(s: S) -> Result<u32, Error> {
+    u32::from_str_radix(s.as_ref(), 16)
+        .map_err(|_| Error::UnexpectedError("Could not parse hex string".to_owned()))
+}
+
+/// Parses a serial number returned by hidapi and returns its integer value.
 ///
 /// 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<u32> {
     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
+        };
+        parse_serial_number(substr).ok()
     } else {
         // The serial number is all zero
         None
@@ -263,7 +265,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:    0x{:08x}", status.serial_number);
     /// # Ok::<(), nitrokey::Error>(())
     /// ```
     ///
@@ -273,8 +275,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
     ///
@@ -286,14 +289,15 @@ pub trait Device<'a>: Authenticate<'a> + GetPasswordSafe<'a> + GenerateOtp + fmt
     /// let mut manager = nitrokey::take()?;
     /// let device = manager.connect()?;
     /// match device.get_serial_number() {
-    ///     Ok(number) => println!("serial no: {}", number),
+    ///     Ok(number) => println!("serial no: 0x{:08x}", number),
     ///     Err(err) => eprintln!("Could not get serial number: {}", err),
     /// };
     /// #     Ok(())
     /// # }
     /// ```
-    fn get_serial_number(&self) -> Result<String, Error> {
+    fn get_serial_number(&self) -> Result<u32, Error> {
         result_from_string(unsafe { nitrokey_sys::NK_device_serial_number() })
+            .and_then(parse_serial_number)
     }
 
     /// Returns the number of remaining authentication attempts for the user.  The total number of
@@ -625,43 +629,55 @@ pub(crate) fn connect_enum(model: Model) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use super::get_hidapi_serial_number;
+    use super::{get_hidapi_serial_number, parse_serial_number};
+
+    #[test]
+    fn test_parse_serial_number() {
+        fn assert_err(s: &str) {
+            match parse_serial_number(s).unwrap_err() {
+                super::Error::UnexpectedError(_) => {}
+                err => assert!(false, "expected UnexpectedError, got {} (input {})", err, s),
+            }
+        }
+
+        assert_eq!(0x1234, parse_serial_number("1234").unwrap());
+        assert_eq!(0x1234, parse_serial_number("01234").unwrap());
+        assert_eq!(0x1234, parse_serial_number("001234").unwrap());
+        assert_eq!(0x1234, parse_serial_number("0001234").unwrap());
+
+        assert_eq!(0, parse_serial_number("0").unwrap());
+
+        assert_eq!(0xdeadbeef, parse_serial_number("deadbeef").unwrap());
+        assert_err("0xdeadbeef");
+        assert_err("deadpork");
+        assert_err("blubb");
+        assert_err("");
+    }
 
     #[test]
-    fn hidapi_serial_number() {
+    fn test_get_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("blubb"));
         assert_eq!(None, get_hidapi_serial_number("1234"));
+        assert_eq!(Some(0x1234), get_hidapi_serial_number("00001234"));
+        assert_eq!(Some(0x1234), get_hidapi_serial_number("000000001234"));
+        assert_eq!(Some(0x1234), get_hidapi_serial_number("100000001234"));
+        assert_eq!(Some(0x12340000), get_hidapi_serial_number("123400000000"));
         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()),
+            Some(0x5678),
             get_hidapi_serial_number("000000000000000000005678")
         );
         assert_eq!(
-            Some("00001234".to_string()),
+            Some(0x1234),
             get_hidapi_serial_number("000012340000000000000000")
         );
         assert_eq!(
-            Some("0000ffff".to_string()),
+            Some(0xffff),
             get_hidapi_serial_number("00000000000000000000FFFF")
         );
         assert_eq!(
-            Some("0000ffff".to_string()),
+            Some(0xffff),
             get_hidapi_serial_number("00000000000000000000ffff")
         );
     }