diff options
| -rw-r--r-- | CHANGELOG.md | 5 | ||||
| -rw-r--r-- | src/device/mod.rs | 108 | ||||
| -rw-r--r-- | tests/device.rs | 13 |
3 files changed, 70 insertions, 56 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index da9b2f3..4c0f6b4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,11 @@ SPDX-License-Identifier: CC0-1.0 # Unreleased - Add `String` value to the `Error::UnexpectedError` variant. +- Always store serial numbers as integers: + - Change the return type of `Device::get_serial_number` from `Result<String, + _>` to `Result<u32, _>`. + - Change the type of the field `DeviceInfo.serial_number` from + `Option<String>` to `Option<u32>`. # v0.5.1 (2020-01-15) - Fix serial number formatting for Nitrokey Pro devices with firmware 0.8 or 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") ); } diff --git a/tests/device.rs b/tests/device.rs index a88c956..2989941 100644 --- a/tests/device.rs +++ b/tests/device.rs @@ -47,8 +47,7 @@ fn list_devices(_device: DeviceWrapper) { nitrokey::Model::Pro => { assert!(device.serial_number.is_some()); let serial_number = device.serial_number.unwrap(); - assert!(!serial_number.is_empty()); - assert_valid_serial_number(&serial_number); + assert!(serial_number != 0); } nitrokey::Model::Storage => { assert_eq!(None, device.serial_number); @@ -157,20 +156,14 @@ fn disconnect(device: DeviceWrapper) { fn get_status(device: DeviceWrapper) { let status = unwrap_ok!(device.get_status()); assert_ok!(status.firmware_version, device.get_firmware_version()); - let serial_number = format!("{:08x}", status.serial_number); - assert_ok!(serial_number, device.get_serial_number()); + assert_ok!(status.serial_number, device.get_serial_number()); assert_ok!(status.config, device.get_config()); } -fn assert_valid_serial_number(serial_number: &str) { - assert!(serial_number.is_ascii()); - assert!(serial_number.chars().all(|c| c.is_ascii_hexdigit())); -} - #[test_device] fn get_serial_number(device: DeviceWrapper) { let serial_number = unwrap_ok!(device.get_serial_number()); - assert_valid_serial_number(&serial_number); + assert!(serial_number != 0); } #[test_device] |