From 95d18c5b06399fa4109de808f70b1048d4494d39 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Tue, 5 Jun 2018 22:19:53 +0200 Subject: Use a Device reference in PasswordSafe Instead of wrapping an owned Device instance, PasswordSafe now only requires a reference to a Device. The lifetime parameter makes sure that the device lives at least as long as the password safe. Using a reference instead of an owned device allows us to implement Drop on PasswordSafe to make sure that the password safe is disabled once it is destructed. --- src/pws.rs | 189 ++++++++++++++++++------------------------------------- src/tests/pws.rs | 44 +++++-------- 2 files changed, 75 insertions(+), 158 deletions(-) diff --git a/src/pws.rs b/src/pws.rs index fc4b516..87a71dd 100644 --- a/src/pws.rs +++ b/src/pws.rs @@ -12,55 +12,41 @@ pub const SLOT_COUNT: u8 = 16; /// A password safe on a Nitrokey device. /// /// The password safe stores a tuple consisting of a name, a login and a password on a slot. The -/// number of available slots is [`SLOT_COUNT`][]. The slots are addressed starting with zero. -/// -/// The password safe struct wraps a device. To get back to the original device, use the -/// [`device`][] method. To retrieve a password safe from a Nitrokey device, use the -/// [`get_password_safe`][] method from the [`GetPasswordSafe`][] trait. +/// number of available slots is [`SLOT_COUNT`][]. The slots are addressed starting with zero. To +/// retrieve a password safe from a Nitrokey device, use the [`get_password_safe`][] method from +/// the [`GetPasswordSafe`][] trait. Note that the device must live at least as long as the +/// password safe. /// /// # Examples /// /// Open a password safe and access a password: /// /// ```no_run -/// use nitrokey::{Device, GetPasswordSafe}; +/// use nitrokey::GetPasswordSafe; /// # use nitrokey::CommandError; /// -/// fn use_device(device: &T) {} -/// /// # fn try_main() -> Result<(), CommandError> { /// let device = nitrokey::connect()?; -/// let device = match device.get_password_safe("123456") { -/// Ok(pws) => { -/// let name = pws.get_slot_name(0)?; -/// let login = pws.get_slot_login(0)?; -/// let password = pws.get_slot_login(0)?; -/// println!("Credentials for {}: login {}, password {}", name, login, password); -/// pws.device() -/// }, -/// Err((device, err)) => { -/// println!("Could not open the password safe: {:?}", err); -/// device -/// }, -/// }; -/// use_device(&device); +/// let pws = device.get_password_safe("123456")?; +/// let name = pws.get_slot_name(0)?; +/// let login = pws.get_slot_login(0)?; +/// let password = pws.get_slot_login(0)?; +/// println!("Credentials for {}: login {}, password {}", name, login, password); /// # Ok(()) /// # } /// ``` /// /// [`SLOT_COUNT`]: constant.SLOT_COUNT.html -/// [`device`]: #method.device /// [`get_password_safe`]: trait.GetPasswordSafe.html#method.get_password_safe /// [`GetPasswordSafe`]: trait.GetPasswordSafe.html -pub struct PasswordSafe { - device: T, +pub struct PasswordSafe<'a> { + _device: &'a Device, } /// Provides access to a [`PasswordSafe`][]. /// -/// When retrieving a password safe, the underlying device is consumed. On success, the returned -/// password safe also containes the device. On error, the device is returned as part of the -/// error. +/// The device that implements this trait must always live at least as long as a password safe +/// retrieved from it. /// /// [`PasswordSafe`]: struct.PasswordSafe.html pub trait GetPasswordSafe { @@ -76,25 +62,17 @@ pub trait GetPasswordSafe { /// # Example /// /// ```no_run - /// use nitrokey::{Device, GetPasswordSafe, PasswordSafe}; + /// use nitrokey::{GetPasswordSafe, PasswordSafe}; /// # use nitrokey::CommandError; /// - /// fn use_password_safe(pws: &PasswordSafe) {} - /// fn use_device(device: &T) {} + /// fn use_password_safe(pws: &PasswordSafe) {} /// /// # fn try_main() -> Result<(), CommandError> { /// let device = nitrokey::connect()?; - /// let device = match device.get_password_safe("123456") { - /// Ok(pws) => { - /// use_password_safe(&pws); - /// pws.device() - /// }, - /// Err((device, err)) => { - /// println!("Could not open the password safe: {:?}", err); - /// device - /// }, + /// match device.get_password_safe("123456") { + /// Ok(pws) => use_password_safe(&pws), + /// Err(err) => println!("Could not open the password safe: {:?}", err), /// }; - /// use_device(&device); /// # Ok(()) /// # } /// ``` @@ -102,18 +80,16 @@ pub trait GetPasswordSafe { /// [`device`]: struct.PasswordSafe.html#method.device /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn get_password_safe(self, user_pin: &str) -> Result, (Self, CommandError)> - where - Self: Device + Sized; + fn get_password_safe(&self, user_pin: &str) -> Result; } -fn get_password_safe( - device: T, +fn get_password_safe<'a>( + device: &'a Device, user_pin: &str, -) -> Result, (T, CommandError)> { +) -> Result, CommandError> { let user_pin_string = CString::new(user_pin); if user_pin_string.is_err() { - return Err((device, CommandError::InvalidString)); + return Err(CommandError::InvalidString); } let user_pin_string = user_pin_string.unwrap(); let status = unsafe { @@ -122,36 +98,12 @@ fn get_password_safe( )) }; match status { - CommandStatus::Success => Ok(PasswordSafe { device }), - CommandStatus::Error(err) => Err((device, err)), + CommandStatus::Success => Ok(PasswordSafe { _device: device }), + CommandStatus::Error(err) => Err(err), } } -fn get_password_safe_wrapper( - device: T, - constructor: C, - user_pin: &str, -) -> Result, (DeviceWrapper, CommandError)> -where - T: Device, - C: Fn(T) -> DeviceWrapper, -{ - let result = device.get_password_safe(user_pin); - match result { - Ok(pws) => Ok(PasswordSafe { - device: constructor(pws.device), - }), - Err((device, err)) => Err((constructor(device), err)), - } -} - -impl PasswordSafe { - /// Forgets the password safe access and returns an unauthenticated device. This method - /// consumes the password safe. It does not perform any actual commands on the Nitrokey. - pub fn device(self) -> T { - self.device - } - +impl<'a> PasswordSafe<'a> { /// Returns the status of all password slots. /// /// The status indicates whether a slot is programmed or not. @@ -164,18 +116,14 @@ impl PasswordSafe { /// /// # fn try_main() -> Result<(), CommandError> { /// let device = nitrokey::connect()?; - /// match device.get_password_safe("123456") { - /// Ok(pws) => { - /// pws.get_slot_status()?.iter().enumerate().for_each(|(slot, programmed)| { - /// let status = match *programmed { - /// true => "programmed", - /// false => "not programmed", - /// }; - /// println!("Slot {}: {}", slot, status); - /// }); - /// }, - /// Err((_, err)) => println!("Could not open the password safe: {:?}", err), - /// }; + /// let pws = device.get_password_safe("123456")?; + /// pws.get_slot_status()?.iter().enumerate().for_each(|(slot, programmed)| { + /// let status = match *programmed { + /// true => "programmed", + /// false => "not programmed", + /// }; + /// println!("Slot {}: {}", slot, status); + /// }); /// # Ok(()) /// # } /// ``` @@ -218,7 +166,7 @@ impl PasswordSafe { /// let password = pws.get_slot_login(0)?; /// println!("Credentials for {}: login {}, password {}", name, login, password); /// }, - /// Err((_, err)) => println!("Could not open the password safe: {:?}", err), + /// Err(err) => println!("Could not open the password safe: {:?}", err), /// }; /// # Ok(()) /// # } @@ -245,15 +193,11 @@ impl PasswordSafe { /// /// # fn try_main() -> Result<(), CommandError> { /// let device = nitrokey::connect()?; - /// match device.get_password_safe("123456") { - /// Ok(pws) => { - /// let name = pws.get_slot_name(0)?; - /// let login = pws.get_slot_login(0)?; - /// let password = pws.get_slot_login(0)?; - /// println!("Credentials for {}: login {}, password {}", name, login, password); - /// }, - /// Err((_, err)) => println!("Could not open the password safe: {:?}", err), - /// }; + /// let pws = device.get_password_safe("123456")?; + /// let name = pws.get_slot_name(0)?; + /// let login = pws.get_slot_login(0)?; + /// let password = pws.get_slot_login(0)?; + /// println!("Credentials for {}: login {}, password {}", name, login, password); /// # Ok(()) /// # } /// ``` @@ -279,15 +223,11 @@ impl PasswordSafe { /// /// # fn try_main() -> Result<(), CommandError> { /// let device = nitrokey::connect()?; - /// match device.get_password_safe("123456") { - /// Ok(pws) => { - /// let name = pws.get_slot_name(0)?; - /// let login = pws.get_slot_login(0)?; - /// let password = pws.get_slot_login(0)?; - /// println!("Credentials for {}: login {}, password {}", name, login, password); - /// }, - /// Err((_, err)) => println!("Could not open the password safe: {:?}", err), - /// }; + /// let pws = device.get_password_safe("123456")?; + /// let name = pws.get_slot_name(0)?; + /// let login = pws.get_slot_login(0)?; + /// let password = pws.get_slot_login(0)?; + /// println!("Credentials for {}: login {}, password {}", name, login, password); /// # Ok(()) /// # } /// ``` @@ -308,18 +248,16 @@ impl PasswordSafe { /// # Example /// /// ```no_run - /// use nitrokey::{CommandStatus, GetPasswordSafe}; + /// use nitrokey::GetPasswordSafe; /// # use nitrokey::CommandError; /// /// # fn try_main() -> Result<(), CommandError> { /// let device = nitrokey::connect()?; - /// match device.get_password_safe("123456") { - /// Ok(pws) => match pws.write_slot(0, "GitHub", "johndoe", "passw0rd") { - /// CommandStatus::Success => println!("Successfully wrote slot 0."), - /// CommandStatus::Error(err) => println!("Could not write slot 0: {:?}", err), - /// }, - /// Err((_, err)) => println!("Could not open the password safe: {:?}", err), - /// }; + /// let pws = device.get_password_safe("123456")?; + /// let name = pws.get_slot_name(0)?; + /// let login = pws.get_slot_login(0)?; + /// let password = pws.get_slot_login(0)?; + /// println!("Credentials for {}: login {}, password {}", name, login, password); /// # Ok(()) /// # } /// ``` @@ -362,12 +300,10 @@ impl PasswordSafe { /// /// # fn try_main() -> Result<(), CommandError> { /// let device = nitrokey::connect()?; - /// match device.get_password_safe("123456") { - /// Ok(pws) => match pws.erase_slot(0) { - /// CommandStatus::Success => println!("Successfully erased slot 0."), - /// CommandStatus::Error(err) => println!("Could not erase slot 0: {:?}", err), - /// }, - /// Err((_, err)) => println!("Could not open the password safe: {:?}", err), + /// let pws = device.get_password_safe("123456")?; + /// match pws.erase_slot(0) { + /// CommandStatus::Success => println!("Erased slot 0."), + /// CommandStatus::Error(err) => println!("Could not erase slot 0: {:?}", err), /// }; /// # Ok(()) /// # } @@ -380,24 +316,19 @@ impl PasswordSafe { } impl GetPasswordSafe for Pro { - fn get_password_safe(self, user_pin: &str) -> Result, (Self, CommandError)> { + fn get_password_safe(&self, user_pin: &str) -> Result { get_password_safe(self, user_pin) } } impl GetPasswordSafe for Storage { - fn get_password_safe(self, user_pin: &str) -> Result, (Self, CommandError)> { + fn get_password_safe(&self, user_pin: &str) -> Result { get_password_safe(self, user_pin) } } impl GetPasswordSafe for DeviceWrapper { - fn get_password_safe(self, user_pin: &str) -> Result, (Self, CommandError)> { - match self { - DeviceWrapper::Storage(storage) => { - get_password_safe_wrapper(storage, DeviceWrapper::Storage, user_pin) - } - DeviceWrapper::Pro(pro) => get_password_safe_wrapper(pro, DeviceWrapper::Pro, user_pin), - } + fn get_password_safe(&self, user_pin: &str) -> Result { + get_password_safe(self, user_pin) } } diff --git a/src/tests/pws.rs b/src/tests/pws.rs index f45a81a..30d6853 100644 --- a/src/tests/pws.rs +++ b/src/tests/pws.rs @@ -2,46 +2,29 @@ use pws::{GetPasswordSafe, PasswordSafe, SLOT_COUNT}; use tests::util::{Target, ADMIN_PASSWORD, USER_PASSWORD}; use util::{CommandError, CommandStatus}; -fn get_pws() -> PasswordSafe { - Target::connect() - .unwrap() - .get_password_safe(USER_PASSWORD) - .unwrap() +fn get_pws(device: &Target) -> PasswordSafe { + device.get_password_safe(USER_PASSWORD).unwrap() } #[test] #[cfg_attr(not(any(feature = "test-pro", feature = "test-storage")), ignore)] fn enable() { + let device = Target::connect().unwrap(); assert!( - Target::connect() - .unwrap() + device .get_password_safe(&(USER_PASSWORD.to_owned() + "123")) .is_err() ); - assert!( - Target::connect() - .unwrap() - .get_password_safe(USER_PASSWORD) - .is_ok() - ); - assert!( - Target::connect() - .unwrap() - .get_password_safe(ADMIN_PASSWORD) - .is_err() - ); - assert!( - Target::connect() - .unwrap() - .get_password_safe(USER_PASSWORD) - .is_ok() - ); + assert!(device.get_password_safe(USER_PASSWORD).is_ok()); + assert!(device.get_password_safe(ADMIN_PASSWORD).is_err()); + assert!(device.get_password_safe(USER_PASSWORD).is_ok()); } #[test] #[cfg_attr(not(any(feature = "test-pro", feature = "test-storage")), ignore)] fn get_status() { - let pws = get_pws(); + let device = Target::connect().unwrap(); + let pws = get_pws(&device); for i in 0..SLOT_COUNT { assert_eq!( CommandStatus::Success, @@ -75,7 +58,8 @@ fn get_status() { #[test] #[cfg_attr(not(any(feature = "test-pro", feature = "test-storage")), ignore)] fn get_data() { - let pws = get_pws(); + let device = Target::connect().unwrap(); + let pws = get_pws(&device); assert_eq!( CommandStatus::Success, pws.write_slot(1, "name", "login", "password") @@ -118,7 +102,8 @@ fn get_data() { #[test] #[cfg_attr(not(any(feature = "test-pro", feature = "test-storage")), ignore)] fn write() { - let pws = get_pws(); + let device = Target::connect().unwrap(); + let pws = get_pws(&device); assert_eq!( CommandStatus::Error(CommandError::InvalidSlot), @@ -153,7 +138,8 @@ fn write() { #[test] #[cfg_attr(not(any(feature = "test-pro", feature = "test-storage")), ignore)] fn erase() { - let pws = get_pws(); + let device = Target::connect().unwrap(); + let pws = get_pws(&device); assert_eq!( CommandStatus::Error(CommandError::InvalidSlot), pws.erase_slot(SLOT_COUNT) -- cgit v1.2.3