Fix serial number for older Nitrokey Pro in list_devices

Previously, we assumed that the serial number returned by hidapi
contains the Nitrokey serial number as the least significant bytes.  As
disussed here [0], this is not true for Nitrokey Pro devices with
firmware version 0.8 or older:  They write the serial number to the most
significant bytes instead.

This patch update the get_hidapi_serial_number function so that
list_devices now returns the correctly formatted and truncated serial
number for all Nitrokey Pro devices.  It also makes sure that the serial
number is lowercase to be consistent with libnitrokey’s formatting.

2 files changed, 53 insertions, 20 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 54ee7d3..a52e043 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,10 @@ Copyright (C) 2019-2020 Robin Krahl <robin.krahl@ireas.org>
 SPDX-License-Identifier: CC0-1.0
 -->
 
+# Unreleased
+- Fix serial number formatting for Nitrokey Pro devices with firmware 0.8 or
+  older in the `list_devices` function.
+
 # v0.5.0 (2020-01-14)
 - List these libnitrokey functions as unsupported:
   - `NK_change_firmware_password_pro`
diff --git a/src/device/mod.rs b/src/device/mod.rs
index 16d6b11..0234bf0 100644
--- a/src/device/mod.rs
+++ b/src/device/mod.rs
@@ -5,7 +5,6 @@ mod pro;
 mod storage;
 mod wrapper;
 
-use std::cmp;
 use std::convert::{TryFrom, TryInto};
 use std::ffi;
 use std::fmt;
@@ -120,21 +119,38 @@ impl fmt::Display for DeviceInfo {
 
 /// Parses a serial number returned by hidapi and transforms it to the Nitrokey format.
 ///
-/// If the serial number is all zero, this function returns `None`.  Otherwise, all leading zeros
-/// (except the last eight characters) are stripped and `Some(_)` is returned.  If the serial
-/// number has less than eight characters, leading zeros are added.
+/// 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.
+///
+/// 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> {
-    let mut iter = serial_number.char_indices().skip_while(|(_, c)| *c == '0');
-    let first_non_null = iter.next();
+    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
+        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 {
-        // keep at least the last 8 characters
-        let len = serial_number.len();
-        let cut = cmp::min(len.saturating_sub(8), i);
-        let (_, suffix) = serial_number.split_at(cut);
-        // if necessary, add leading zeros to reach 8 characters
-        let fill = 8usize.saturating_sub(len);
-        Some("0".repeat(fill) + suffix)
+        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)
+        } 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)
+        }
     } else {
+        // The serial number is all zero
         None
     }
 }
@@ -615,10 +631,7 @@ mod tests {
     fn hidapi_serial_number() {
         assert_eq!(None, get_hidapi_serial_number(""));
         assert_eq!(None, get_hidapi_serial_number("00000000000000000"));
-        assert_eq!(
-            Some("00001234".to_string()),
-            get_hidapi_serial_number("1234")
-        );
+        assert_eq!(None, get_hidapi_serial_number("1234"));
         assert_eq!(
             Some("00001234".to_string()),
             get_hidapi_serial_number("00001234")
@@ -628,12 +641,28 @@ mod tests {
             get_hidapi_serial_number("000000001234")
         );
         assert_eq!(
-            Some("100000001234".to_string()),
+            Some("00001234".to_string()),
             get_hidapi_serial_number("100000001234")
         );
         assert_eq!(
-            Some("10000001234".to_string()),
-            get_hidapi_serial_number("010000001234")
+            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")
         );
     }
 }