aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRobin Krahl <robin.krahl@ireas.org>2018-06-07 00:41:20 +0200
committerRobin Krahl <robin.krahl@ireas.org>2018-06-07 00:41:20 +0200
commitf95e2be7422243bbbb07ae07f6b026bd6d578099 (patch)
tree81f98fffe8c89b19e82eb9e28ff0d6b1a7a7c2a4
parent22e378677d5b00a05c021dc6660651608b384e0d (diff)
downloadnitrokey-rs-f95e2be7422243bbbb07ae07f6b026bd6d578099.tar.gz
nitrokey-rs-f95e2be7422243bbbb07ae07f6b026bd6d578099.tar.bz2
Remove NK_lock_device call from PasswordSafe::drop
When enabled, the password safe can be used without authentication. The lock device can be used to lock the password safe. Currently, PasswordSafe::drop calls this command to make sure that other applications cannot access the password safe without authentication. On the Nitrokey Storage, locking the device may also disable the encrypted or hidden volume. As using the password safe should not have side effects on the storage volumes, this patch removes the call to the lock device command from the Drop implementation. Instead, the user should call this method after making sure that it does not have side effects. A feature request for a command that only locks the password safe without side effects is submitted to the Nitrokey Storage firmware repository: https://github.com/Nitrokey/nitrokey-storage-firmware/issues/65
-rw-r--r--TODO.md5
-rw-r--r--src/pws.rs44
-rw-r--r--src/tests/pws.rs4
3 files changed, 39 insertions, 14 deletions
diff --git a/TODO.md b/TODO.md
index e2fb3fe..b666ce3 100644
--- a/TODO.md
+++ b/TODO.md
@@ -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
diff --git a/src/pws.rs b/src/pws.rs
index 85726c5..903ecef 100644
--- a/src/pws.rs
+++ b/src/pws.rs
@@ -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);
}