From 4fb865af37093d9b0ee039d8ae48fb2a820f3760 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Tue, 28 Jan 2020 17:38:47 +0100 Subject: Represent serial numbers using SerialNumber struct In a previous commit, we changed the serial number representation from a string to an integer. This made it easier to compare serial numbers, but also introduced new problems: - Serial numbers should be formatted consistently, for example as "{:#010x}". It is hard to ensure this for an integer value. - The format of the serial number may be subject to change. Users should not rely too much on the u32 representation. Therefore we introduce a new SerialNumber struct that represents a serial number. Currently it only stores a u32 value. The following traits and functions can be used to access its value: - FromStr for string parsing - ToString/Display for string formatting - as_u32 to access the underlying integer value --- CHANGELOG.md | 7 +- examples/list-devices.rs | 2 +- src/device/mod.rs | 216 +++++++++++++++++++++++++++++++++++------------ src/device/storage.rs | 4 +- src/lib.rs | 4 +- tests/device.rs | 6 +- 6 files changed, 174 insertions(+), 65 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c94e170..9e675f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,11 +5,12 @@ SPDX-License-Identifier: CC0-1.0 # Unreleased - Add `String` value to the `Error::UnexpectedError` variant. -- Always store serial numbers as integers: +- Always store serial numbers as structs: + - Introduce the `SerialNumber` struct. - Change the return type of `Device::get_serial_number` from `Result` to `Result`. + _>` to `Result`. - Change the type of the field `DeviceInfo.serial_number` from - `Option` to `Option`. + `Option` to `Option`. - Use `NK_get_status` instead of `NK_read_config` to implement the `Device::get_config` function. diff --git a/examples/list-devices.rs b/examples/list-devices.rs index 47fa054..0066f8c 100644 --- a/examples/list-devices.rs +++ b/examples/list-devices.rs @@ -17,7 +17,7 @@ fn main() -> Result<(), nitrokey::Error> { let model = device.get_model(); let status = device.get_status()?; println!( - "{}\t{}\t{}\t\t\t{:08x}", + "{}\t{}\t{}\t\t\t{}", device_info.path, model, status.firmware_version, status.serial_number ); } diff --git a/src/device/mod.rs b/src/device/mod.rs index e9ca0dc..83ab90d 100644 --- a/src/device/mod.rs +++ b/src/device/mod.rs @@ -8,12 +8,13 @@ mod wrapper; use std::convert::{TryFrom, TryInto}; use std::ffi; use std::fmt; +use std::str; 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::{ @@ -69,6 +70,98 @@ impl TryFrom 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, 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 { + // 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 { @@ -77,7 +170,7 @@ pub struct DeviceInfo { /// The USB device path. pub path: String, /// The serial number of the device, or `None` if the device does not expose its serial number. - pub serial_number: Option, + pub serial_number: Option, } impl TryFrom<&nitrokey_sys::NK_device_info> for DeviceInfo { @@ -108,19 +201,13 @@ impl fmt::Display for DeviceInfo { } write!(f, " at {} with ", self.path)?; match self.serial_number { - Some(serial_number) => write!(f, "serial no. 0x{:08x}", serial_number), + Some(serial_number) => write!(f, "serial no. {}", serial_number), None => write!(f, "an unknown serial number"), } } } -/// 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. +/// 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. The @@ -131,7 +218,7 @@ fn parse_serial_number>(s: S) -> Result { /// 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 @@ -148,7 +235,7 @@ fn get_hidapi_serial_number(serial_number: &str) -> Option { // The last eight characters are all zero --> use the first eight serial_number.split_at(8).0 }; - parse_serial_number(substr).ok() + 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,7 +275,7 @@ impl From for Status { major: status.firmware_version_major, minor: status.firmware_version_minor, }, - serial_number: status.serial_number_smart_card, + serial_number: SerialNumber::new(status.serial_number_smart_card), config: RawConfig::from(&status).into(), } } @@ -257,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: 0x{:08x}", status.serial_number); + /// println!("Serial number: {}", status.serial_number); /// # Ok::<(), nitrokey::Error>(()) /// ``` /// @@ -281,15 +368,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: 0x{:08x}", number), + /// Ok(number) => println!("serial no: {}", 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) + .and_then(|s| s.parse()) } /// Returns the number of remaining authentication attempts for the user. The total number of @@ -624,26 +711,50 @@ pub(crate) fn connect_enum(model: Model) -> bool { #[cfg(test)] mod tests { - use super::{get_hidapi_serial_number, parse_serial_number}; + use std::str::FromStr; + + use super::{get_hidapi_serial_number, LibraryError, SerialNumber}; + + #[test] + 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_parse_serial_number() { + 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 parse_serial_number(s).unwrap_err() { - super::Error::UnexpectedError(_) => {} - err => assert!(false, "expected UnexpectedError, got {} (input {})", err, s), + match SerialNumber::from_str(s).unwrap_err() { + super::Error::LibraryError(LibraryError::InvalidHexString) => {} + err => assert!( + false, + "expected InvalidHexString error, 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_ok(0x1234, "1234"); + assert_ok(0x1234, "01234"); + assert_ok(0x1234, "001234"); + assert_ok(0x1234, "0001234"); - assert_eq!(0, parse_serial_number("0").unwrap()); + assert_ok(0, "0"); + assert_ok(0xdeadbeef, "deadbeef"); - assert_eq!(0xdeadbeef, parse_serial_number("deadbeef").unwrap()); - assert_err("0xdeadbeef"); assert_err("deadpork"); assert_err("blubb"); assert_err(""); @@ -651,29 +762,26 @@ mod tests { #[test] 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(0x5678), - get_hidapi_serial_number("000000000000000000005678") - ); - assert_eq!( - Some(0x1234), - get_hidapi_serial_number("000012340000000000000000") - ); - assert_eq!( - Some(0xffff), - get_hidapi_serial_number("00000000000000000000FFFF") - ); - assert_eq!( - Some(0xffff), - get_hidapi_serial_number("00000000000000000000ffff") - ); + 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 1e2c46d..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}; @@ -827,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/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/tests/device.rs b/tests/device.rs index 2989941..1669d9d 100644 --- a/tests/device.rs +++ b/tests/device.rs @@ -10,7 +10,7 @@ use std::{thread, time}; use nitrokey::{ Authenticate, CommandError, CommunicationError, Config, ConfigureOtp, Device, DeviceInfo, Error, GenerateOtp, GetPasswordSafe, LibraryError, OperationStatus, OtpMode, OtpSlotData, - Storage, VolumeMode, DEFAULT_ADMIN_PIN, DEFAULT_USER_PIN, + SerialNumber, Storage, VolumeMode, DEFAULT_ADMIN_PIN, DEFAULT_USER_PIN, }; use nitrokey_test::test as test_device; @@ -47,7 +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 != 0); + assert!(serial_number != SerialNumber::empty()); } nitrokey::Model::Storage => { assert_eq!(None, device.serial_number); @@ -163,7 +163,7 @@ fn get_status(device: DeviceWrapper) { #[test_device] fn get_serial_number(device: DeviceWrapper) { let serial_number = unwrap_ok!(device.get_serial_number()); - assert!(serial_number != 0); + assert!(serial_number != SerialNumber::empty()); } #[test_device] -- cgit v1.2.1