From 24eebcdaaa32d55bf49d069d8320be5dbd6fdab9 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 26 Jan 2020 14:26:01 +0100 Subject: 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 --- src/device/mod.rs | 108 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 62 insertions(+), 46 deletions(-) (limited to 'src/device') 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, /// 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, + /// The serial number of the device, or `None` if the device does not expose its serial number. + pub serial_number: Option, } 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: S) -> Result { + 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 { +fn get_hidapi_serial_number(serial_number: &str) -> Option { 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; - /// 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 { + fn get_serial_number(&self) -> Result { 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") ); } -- cgit v1.2.1