~ireas/nitrokey-rs

fe2f39826ade5a156945dabb8c8ab725378a15c1 — Robin Krahl 2 years ago 379bc79
Store mutable reference to Manager in Device

In the last patches, we ensured that devices can only be obtained using
the Manager struct.  But we did not ensure that there is only one device
at a time.  This patch adds a mutable reference to the Manager instance
to the Device implementations.  The borrow checker makes sure that there
is only one mutable reference at a time.

In this patch, we have to remove the old connect, Pro::connect and
Storage::connect functions as they do no longer compile.  (They discard
the MutexGuard which invalidates the reference to the Manager.)
Therefore the tests do no longer compile.
5 files changed, 63 insertions(+), 147 deletions(-)

M CHANGELOG.md
M src/auth.rs
M src/device.rs
M src/lib.rs
M src/pws.rs
M CHANGELOG.md => CHANGELOG.md +1 -2
@@ 44,8 44,7 @@ SPDX-License-Identifier: MIT
- Refactor connection management:
  - Add `ConcurrentAccessError` and `PoisonError` `Error` variants.
  - Add the `Manager` struct that manages connections to Nitrokey devices.
  - Deprecate `connect`, `Pro::connect` and `Storage::connect`.
  - Remove the `connect_model` function.
  - Remove `connect`, `connect_model`, `Pro::connect` and `Storage::connect`.

# v0.3.4 (2019-01-20)
- Fix authentication methods that assumed that `char` is signed.

M src/auth.rs => src/auth.rs +9 -9
@@ 170,14 170,14 @@ where
    }
}

fn authenticate_user_wrapper<T, C>(
fn authenticate_user_wrapper<'a, T, C>(
    device: T,
    constructor: C,
    password: &str,
) -> Result<User<DeviceWrapper>, (DeviceWrapper, Error)>
) -> Result<User<DeviceWrapper<'a>>, (DeviceWrapper<'a>, Error)>
where
    T: Device,
    C: Fn(T) -> DeviceWrapper,
    C: Fn(T) -> DeviceWrapper<'a>,
{
    let result = device.authenticate_user(password);
    match result {


@@ 186,14 186,14 @@ where
    }
}

fn authenticate_admin_wrapper<T, C>(
fn authenticate_admin_wrapper<'a, T, C>(
    device: T,
    constructor: C,
    password: &str,
) -> Result<Admin<DeviceWrapper>, (DeviceWrapper, Error)>
) -> Result<Admin<DeviceWrapper<'a>>, (DeviceWrapper<'a>, Error)>
where
    T: Device,
    C: Fn(T) -> DeviceWrapper,
    C: Fn(T) -> DeviceWrapper<'a>,
{
    let result = device.authenticate_admin(password);
    match result {


@@ 377,7 377,7 @@ impl<T: Device> AuthenticatedDevice<T> for Admin<T> {
    }
}

impl Authenticate for DeviceWrapper {
impl<'a> Authenticate for DeviceWrapper<'a> {
    fn authenticate_user(self, password: &str) -> Result<User<Self>, (Self, Error)> {
        match self {
            DeviceWrapper::Storage(storage) => {


@@ 399,7 399,7 @@ impl Authenticate for DeviceWrapper {
    }
}

impl Authenticate for Pro {
impl<'a> Authenticate for Pro<'a> {
    fn authenticate_user(self, password: &str) -> Result<User<Self>, (Self, Error)> {
        authenticate(self, password, |password_ptr, temp_password_ptr| unsafe {
            nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr)


@@ 413,7 413,7 @@ impl Authenticate for Pro {
    }
}

impl Authenticate for Storage {
impl<'a> Authenticate for Storage<'a> {
    fn authenticate_user(self, password: &str) -> Result<User<Self>, (Self, Error)> {
        authenticate(self, password, |password_ptr, temp_password_ptr| unsafe {
            nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr)

M src/device.rs => src/device.rs +40 -120
@@ 2,14 2,13 @@
// SPDX-License-Identifier: MIT

use std::fmt;
use std::marker;

use libc;
use nitrokey_sys;

use crate::auth::Authenticate;
use crate::config::{Config, RawConfig};
use crate::error::Error;
use crate::error::{CommunicationError, Error};
use crate::otp::GenerateOtp;
use crate::pws::GetPasswordSafe;
use crate::util::{


@@ 109,11 108,11 @@ impl fmt::Display for VolumeMode {
///
/// [`connect`]: fn.connect.html
#[derive(Debug)]
pub enum DeviceWrapper {
pub enum DeviceWrapper<'a> {
    /// A Nitrokey Storage device.
    Storage(Storage),
    Storage(Storage<'a>),
    /// A Nitrokey Pro device.
    Pro(Pro),
    Pro(Pro<'a>),
}

/// A Nitrokey Pro device without user or admin authentication.


@@ 156,10 155,8 @@ pub enum DeviceWrapper {
/// [`connect`]: fn.connect.html
/// [`Pro::connect`]: #method.connect
#[derive(Debug)]
pub struct Pro {
    // make sure that users cannot directly instantiate this type
    #[doc(hidden)]
    marker: marker::PhantomData<()>,
pub struct Pro<'a> {
    manager: &'a mut crate::Manager,
}

/// A Nitrokey Storage device without user or admin authentication.


@@ 202,10 199,8 @@ pub struct Pro {
/// [`connect`]: fn.connect.html
/// [`Storage::connect`]: #method.connect
#[derive(Debug)]
pub struct Storage {
    // make sure that users cannot directly instantiate this type
    #[doc(hidden)]
    marker: marker::PhantomData<()>,
pub struct Storage<'a> {
    manager: &'a mut crate::Manager,
}

/// The status of a volume on a Nitrokey Storage device.


@@ 626,32 621,6 @@ pub trait Device: Authenticate + GetPasswordSafe + GenerateOtp + fmt::Debug {
    }
}

/// Connects to a Nitrokey device.  This method can be used to connect to any connected device,
/// both a Nitrokey Pro and a Nitrokey Storage.
///
/// # Errors
///
/// - [`NotConnected`][] if no Nitrokey device is connected
///
/// # Example
///
/// ```
/// use nitrokey::DeviceWrapper;
///
/// fn do_something(device: DeviceWrapper) {}
///
/// match nitrokey::connect() {
///     Ok(device) => do_something(device),
///     Err(err) => eprintln!("Could not connect to a Nitrokey: {}", err),
/// }
/// ```
///
/// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected
#[deprecated(since = "0.4.0", note = "use `nitrokey::Manager::connect` instead")]
pub fn connect() -> Result<DeviceWrapper, Error> {
    crate::take()?.connect().map_err(Into::into)
}

fn get_connected_model() -> Option<Model> {
    match unsafe { nitrokey_sys::NK_get_device_model() } {
        nitrokey_sys::NK_device_model_NK_PRO => Some(Model::Pro),


@@ 660,15 629,23 @@ fn get_connected_model() -> Option<Model> {
    }
}

pub(crate) fn create_device_wrapper(model: Model) -> DeviceWrapper {
pub(crate) fn create_device_wrapper(
    manager: &mut crate::Manager,
    model: Model,
) -> DeviceWrapper<'_> {
    match model {
        Model::Pro => Pro::new().into(),
        Model::Storage => Storage::new().into(),
        Model::Pro => Pro::new(manager).into(),
        Model::Storage => Storage::new(manager).into(),
    }
}

pub(crate) fn get_connected_device() -> Option<DeviceWrapper> {
    get_connected_model().map(create_device_wrapper)
pub(crate) fn get_connected_device(
    manager: &mut crate::Manager,
) -> Result<DeviceWrapper<'_>, Error> {
    match get_connected_model() {
        Some(model) => Ok(create_device_wrapper(manager, model)),
        None => Err(CommunicationError::NotConnected.into()),
    }
}

pub(crate) fn connect_enum(model: Model) -> bool {


@@ 679,7 656,7 @@ pub(crate) fn connect_enum(model: Model) -> bool {
    unsafe { nitrokey_sys::NK_login_enum(model) == 1 }
}

impl DeviceWrapper {
impl<'a> DeviceWrapper<'a> {
    fn device(&self) -> &dyn Device {
        match *self {
            DeviceWrapper::Storage(ref storage) => storage,


@@ 695,19 672,19 @@ impl DeviceWrapper {
    }
}

impl From<Pro> for DeviceWrapper {
    fn from(device: Pro) -> Self {
impl<'a> From<Pro<'a>> for DeviceWrapper<'a> {
    fn from(device: Pro<'a>) -> Self {
        DeviceWrapper::Pro(device)
    }
}

impl From<Storage> for DeviceWrapper {
    fn from(device: Storage) -> Self {
impl<'a> From<Storage<'a>> for DeviceWrapper<'a> {
    fn from(device: Storage<'a>) -> Self {
        DeviceWrapper::Storage(device)
    }
}

impl GenerateOtp for DeviceWrapper {
impl<'a> GenerateOtp for DeviceWrapper<'a> {
    fn get_hotp_slot_name(&self, slot: u8) -> Result<String, Error> {
        self.device().get_hotp_slot_name(slot)
    }


@@ 725,7 702,7 @@ impl GenerateOtp for DeviceWrapper {
    }
}

impl Device for DeviceWrapper {
impl<'a> Device for DeviceWrapper<'a> {
    fn get_model(&self) -> Model {
        match *self {
            DeviceWrapper::Pro(_) => Model::Pro,


@@ 734,40 711,13 @@ impl Device for DeviceWrapper {
    }
}

impl Pro {
    /// Connects to a Nitrokey Pro.
    ///
    /// # Errors
    ///
    /// - [`NotConnected`][] if no Nitrokey device of the given model is connected
    ///
    /// # Example
    ///
    /// ```
    /// use nitrokey::Pro;
    ///
    /// fn use_pro(device: Pro) {}
    ///
    /// match nitrokey::Pro::connect() {
    ///     Ok(device) => use_pro(device),
    ///     Err(err) => eprintln!("Could not connect to the Nitrokey Pro: {}", err),
    /// }
    /// ```
    ///
    /// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected
    #[deprecated(since = "0.4.0", note = "use `nitrokey::Manager::connect_pro` instead")]
    pub fn connect() -> Result<Pro, Error> {
        crate::take()?.connect_pro().map_err(Into::into)
    }

    pub(crate) fn new() -> Pro {
        Pro {
            marker: marker::PhantomData,
        }
impl<'a> Pro<'a> {
    pub(crate) fn new(manager: &'a mut crate::Manager) -> Pro<'a> {
        Pro { manager }
    }
}

impl Drop for Pro {
impl<'a> Drop for Pro<'a> {
    fn drop(&mut self) {
        unsafe {
            nitrokey_sys::NK_logout();


@@ 775,47 725,17 @@ impl Drop for Pro {
    }
}

impl Device for Pro {
impl<'a> Device for Pro<'a> {
    fn get_model(&self) -> Model {
        Model::Pro
    }
}

impl GenerateOtp for Pro {}
impl<'a> GenerateOtp for Pro<'a> {}

impl Storage {
    /// Connects to a Nitrokey Storage.
    ///
    /// # Errors
    ///
    /// - [`NotConnected`][] if no Nitrokey device of the given model is connected
    ///
    /// # Example
    ///
    /// ```
    /// use nitrokey::Storage;
    ///
    /// fn use_storage(device: Storage) {}
    ///
    /// match nitrokey::Storage::connect() {
    ///     Ok(device) => use_storage(device),
    ///     Err(err) => eprintln!("Could not connect to the Nitrokey Storage: {}", err),
    /// }
    /// ```
    ///
    /// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected
    #[deprecated(
        since = "0.4.0",
        note = "use `nitrokey::Manager::connect_storage` instead"
    )]
    pub fn connect() -> Result<Storage, Error> {
        crate::take()?.connect_storage().map_err(Into::into)
    }

    pub(crate) fn new() -> Storage {
        Storage {
            marker: marker::PhantomData,
        }
impl<'a> Storage<'a> {
    pub(crate) fn new(manager: &'a mut crate::Manager) -> Storage<'a> {
        Storage { manager }
    }

    /// Changes the update PIN.


@@ 1320,7 1240,7 @@ impl Storage {
    }
}

impl Drop for Storage {
impl<'a> Drop for Storage<'a> {
    fn drop(&mut self) {
        unsafe {
            nitrokey_sys::NK_logout();


@@ 1328,13 1248,13 @@ impl Drop for Storage {
    }
}

impl Device for Storage {
impl<'a> Device for Storage<'a> {
    fn get_model(&self) -> Model {
        Model::Storage
    }
}

impl GenerateOtp for Storage {}
impl<'a> GenerateOtp for Storage<'a> {}

impl From<nitrokey_sys::NK_storage_ProductionTest> for StorageProductionInfo {
    fn from(data: nitrokey_sys::NK_storage_ProductionTest) -> Self {

M src/lib.rs => src/lib.rs +10 -13
@@ 110,8 110,8 @@ pub use crate::auth::{Admin, Authenticate, User};
pub use crate::config::Config;
#[allow(deprecated)]
pub use crate::device::{
    connect, Device, DeviceWrapper, Model, Pro, SdCardData, Storage, StorageProductionInfo,
    StorageStatus, VolumeMode, VolumeStatus,
    Device, DeviceWrapper, Model, Pro, SdCardData, Storage, StorageProductionInfo, StorageStatus,
    VolumeMode, VolumeStatus,
};
pub use crate::error::{CommandError, CommunicationError, Error, LibraryError};
pub use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData};


@@ 204,12 204,9 @@ impl Manager {
    /// ```
    ///
    /// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected
    pub fn connect(&mut self) -> Result<DeviceWrapper, Error> {
    pub fn connect(&mut self) -> Result<DeviceWrapper<'_>, Error> {
        if unsafe { nitrokey_sys::NK_login_auto() } == 1 {
            match device::get_connected_device() {
                Some(wrapper) => Ok(wrapper),
                None => Err(CommunicationError::NotConnected.into()),
            }
            device::get_connected_device(self)
        } else {
            Err(CommunicationError::NotConnected.into())
        }


@@ 239,9 236,9 @@ impl Manager {
    /// ```
    ///
    /// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected
    pub fn connect_model(&mut self, model: Model) -> Result<DeviceWrapper, Error> {
    pub fn connect_model(&mut self, model: Model) -> Result<DeviceWrapper<'_>, Error> {
        if device::connect_enum(model) {
            Ok(device::create_device_wrapper(model))
            Ok(device::create_device_wrapper(self, model))
        } else {
            Err(CommunicationError::NotConnected.into())
        }


@@ 270,9 267,9 @@ impl Manager {
    /// ```
    ///
    /// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected
    pub fn connect_pro(&mut self) -> Result<Pro, Error> {
    pub fn connect_pro(&mut self) -> Result<Pro<'_>, Error> {
        if device::connect_enum(device::Model::Pro) {
            Ok(device::Pro::new())
            Ok(device::Pro::new(self))
        } else {
            Err(CommunicationError::NotConnected.into())
        }


@@ 301,9 298,9 @@ impl Manager {
    /// ```
    ///
    /// [`NotConnected`]: enum.CommunicationError.html#variant.NotConnected
    pub fn connect_storage(&mut self) -> Result<Storage, Error> {
    pub fn connect_storage(&mut self) -> Result<Storage<'_>, Error> {
        if device::connect_enum(Model::Storage) {
            Ok(Storage::new())
            Ok(Storage::new(self))
        } else {
            Err(CommunicationError::NotConnected.into())
        }

M src/pws.rs => src/pws.rs +3 -3
@@ 364,19 364,19 @@ impl<'a> Drop for PasswordSafe<'a> {
    }
}

impl GetPasswordSafe for Pro {
impl<'a> GetPasswordSafe for Pro<'a> {
    fn get_password_safe(&mut self, user_pin: &str) -> Result<PasswordSafe<'_>, Error> {
        get_password_safe(self, user_pin)
    }
}

impl GetPasswordSafe for Storage {
impl<'a> GetPasswordSafe for Storage<'a> {
    fn get_password_safe(&mut self, user_pin: &str) -> Result<PasswordSafe<'_>, Error> {
        get_password_safe(self, user_pin)
    }
}

impl GetPasswordSafe for DeviceWrapper {
impl<'a> GetPasswordSafe for DeviceWrapper<'a> {
    fn get_password_safe(&mut self, user_pin: &str) -> Result<PasswordSafe<'_>, Error> {
        get_password_safe(self, user_pin)
    }