diff options
| -rw-r--r-- | TODO.md | 5 | ||||
| -rw-r--r-- | src/pws.rs | 44 | ||||
| -rw-r--r-- | src/tests/pws.rs | 4 |
3 files changed, 39 insertions, 14 deletions
@@ -35,6 +35,9 @@ - Differentiate empty strings and errors (see `result_from_string`). - Check integer conversions. - Consider implementing `Into<CommandError>` for `(Device, CommandError)` -- Check error handling in PasswordSafe::drop(). +- Lock password safe in `PasswordSafe::drop()` (see [nitrokey-storage-firmware + issue 65][]). - Disable creation of multiple password safes at the same time. - Check timing in Storage tests. + +[nitrokey-storage-firmware issue 65]: https://github.com/Nitrokey/nitrokey-storage-firmware/issues/65 @@ -17,27 +17,39 @@ pub const SLOT_COUNT: u8 = 16; /// the [`GetPasswordSafe`][] trait. Note that the device must live at least as long as the /// password safe. /// +/// Once the password safe has been unlocked, it can be accessed without a password. Therefore it +/// is mandatory to call [`lock`][] on the corresponding device after the password store is used. +/// As this command may have side effects on the Nitrokey Storage, it cannot be called +/// automatically once the password safe is destroyed. +/// /// # Examples /// /// Open a password safe and access a password: /// /// ```no_run -/// use nitrokey::GetPasswordSafe; +/// use nitrokey::{Device, GetPasswordSafe, PasswordSafe}; /// # use nitrokey::CommandError; /// +/// fn use_password_safe(pws: &PasswordSafe) -> Result<(), CommandError> { +/// 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(()) +/// } +/// /// # fn try_main() -> Result<(), CommandError> { /// let device = nitrokey::connect()?; /// 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); +/// use_password_safe(&pws); +/// device.lock(); /// # Ok(()) /// # } /// ``` /// /// [`SLOT_COUNT`]: constant.SLOT_COUNT.html /// [`get_password_safe`]: trait.GetPasswordSafe.html#method.get_password_safe +/// [`lock`]: trait.Device.html#method.lock /// [`GetPasswordSafe`]: trait.GetPasswordSafe.html pub struct PasswordSafe<'a> { _device: &'a Device, @@ -50,9 +62,12 @@ pub struct PasswordSafe<'a> { /// /// [`PasswordSafe`]: struct.PasswordSafe.html pub trait GetPasswordSafe { - /// Enables and returns the password safe. This method consumes the device. You can go back - /// to the device using the [`device`][] method of the returned password safe. If the method - /// fails, the current device will be returned as part of the error. + /// Enables and returns the password safe. + /// + /// The underlying device must always live at least as long as a password safe retrieved from + /// it. It is mandatory to lock the underlying device using [`lock`][] after the password safe + /// has been used. Otherwise, other applications can access the password store without + /// authentication. /// /// # Errors /// @@ -62,7 +77,7 @@ pub trait GetPasswordSafe { /// # Example /// /// ```no_run - /// use nitrokey::{GetPasswordSafe, PasswordSafe}; + /// use nitrokey::{Device, GetPasswordSafe, PasswordSafe}; /// # use nitrokey::CommandError; /// /// fn use_password_safe(pws: &PasswordSafe) {} @@ -70,7 +85,10 @@ pub trait GetPasswordSafe { /// # fn try_main() -> Result<(), CommandError> { /// let device = nitrokey::connect()?; /// match device.get_password_safe("123456") { - /// Ok(pws) => use_password_safe(&pws), + /// Ok(pws) => { + /// use_password_safe(&pws); + /// device.lock(); + /// }, /// Err(err) => println!("Could not open the password safe: {:?}", err), /// }; /// # Ok(()) @@ -78,6 +96,7 @@ pub trait GetPasswordSafe { /// ``` /// /// [`device`]: struct.PasswordSafe.html#method.device + /// [`lock`]: trait.Device.html#method.lock /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword fn get_password_safe(&self, user_pin: &str) -> Result<PasswordSafe, CommandError>; @@ -317,9 +336,8 @@ impl<'a> PasswordSafe<'a> { impl<'a> Drop for PasswordSafe<'a> { fn drop(&mut self) { - unsafe { - nitrokey_sys::NK_lock_device(); - } + // TODO: disable the password safe -- NK_lock_device has side effects on the Nitrokey + // Storage, see https://github.com/Nitrokey/nitrokey-storage-firmware/issues/65 } } diff --git a/src/tests/pws.rs b/src/tests/pws.rs index d6125a9..02e33cd 100644 --- a/src/tests/pws.rs +++ b/src/tests/pws.rs @@ -1,3 +1,4 @@ +use device::Device; use nitrokey_sys; use pws::{GetPasswordSafe, PasswordSafe, SLOT_COUNT}; use tests::util::{Target, ADMIN_PASSWORD, USER_PASSWORD}; @@ -36,6 +37,9 @@ fn drop() { assert_eq!(Ok(String::from("name")), result); } let result = result_from_string(unsafe { nitrokey_sys::NK_get_password_safe_slot_name(1) }); + assert_eq!(Ok(String::from("name")), result); + device.lock(); + let result = result_from_string(unsafe { nitrokey_sys::NK_get_password_safe_slot_name(1) }); assert_eq!(Err(CommandError::NotAuthorized), result); } |