From d0f63513bb935d3d931c86a1ab7b68d6ed44bf27 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 01:53:08 +0000 Subject: Move util::CommandError to the new error module This prepares the refactoring of util::CommandError into multiple enums. --- src/auth.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 2d61d4b..e805e54 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -6,10 +6,9 @@ use nitrokey_sys; use crate::config::{Config, RawConfig}; use crate::device::{Device, DeviceWrapper, Pro, Storage}; +use crate::error::CommandError; use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData, RawOtpSlotData}; -use crate::util::{ - generate_password, get_command_result, get_cstring, result_from_string, CommandError, -}; +use crate::util::{generate_password, get_command_result, get_cstring, result_from_string}; static TEMPORARY_PASSWORD_LENGTH: usize = 25; -- cgit v1.2.1 From 94390aadc8a3997d379bf5e4c0bc00c2a9669a34 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 20 Jan 2019 20:58:18 +0000 Subject: Return Error instead of CommandError This patch changes all public functions to return the Error enum instead of the CommandError enum. This breaks the tests which will be fixed with the next patch. This patch also adds a placeholder variant Error::CommandError and a placeholder enum CommandError to make the transition to a new nitrokey-test version easier. --- src/auth.rs | 54 +++++++++++++++++++++++++++--------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index e805e54..509d3aa 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -6,7 +6,7 @@ use nitrokey_sys; use crate::config::{Config, RawConfig}; use crate::device::{Device, DeviceWrapper, Pro, Storage}; -use crate::error::CommandError; +use crate::error::{CommandError, Error}; use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData, RawOtpSlotData}; use crate::util::{generate_password, get_command_result, get_cstring, result_from_string}; @@ -33,12 +33,12 @@ pub trait Authenticate { /// /// ```no_run /// use nitrokey::{Authenticate, DeviceWrapper, User}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// /// fn perform_user_task(device: &User) {} /// fn perform_other_task(device: &DeviceWrapper) {} /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let device = match device.authenticate_user("123456") { /// Ok(user) => { @@ -58,7 +58,7 @@ pub trait Authenticate { /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn authenticate_user(self, password: &str) -> Result, (Self, CommandError)> + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> where Self: Device + Sized; @@ -79,12 +79,12 @@ pub trait Authenticate { /// /// ```no_run /// use nitrokey::{Authenticate, Admin, DeviceWrapper}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// /// fn perform_admin_task(device: &Admin) {} /// fn perform_other_task(device: &DeviceWrapper) {} /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let device = match device.authenticate_admin("123456") { /// Ok(admin) => { @@ -104,7 +104,7 @@ pub trait Authenticate { /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn authenticate_admin(self, password: &str) -> Result, (Self, CommandError)> + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> where Self: Device + Sized; } @@ -143,7 +143,7 @@ pub struct Admin { temp_password: Vec, } -fn authenticate(device: D, password: &str, callback: T) -> Result +fn authenticate(device: D, password: &str, callback: T) -> Result where D: Device, A: AuthenticatedDevice, @@ -161,7 +161,7 @@ where let temp_password_ptr = temp_password.as_ptr() as *const c_char; return match callback(password_ptr, temp_password_ptr) { 0 => Ok(A::new(device, temp_password)), - rv => Err((device, CommandError::from(rv))), + rv => Err((device, CommandError::from(rv).into())), }; } @@ -169,7 +169,7 @@ fn authenticate_user_wrapper( device: T, constructor: C, password: &str, -) -> Result, (DeviceWrapper, CommandError)> +) -> Result, (DeviceWrapper, Error)> where T: Device, C: Fn(T) -> DeviceWrapper, @@ -185,7 +185,7 @@ fn authenticate_admin_wrapper( device: T, constructor: C, password: &str, -) -> Result, (DeviceWrapper, CommandError)> +) -> Result, (DeviceWrapper, Error)> where T: Device, C: Fn(T) -> DeviceWrapper, @@ -215,14 +215,14 @@ impl Deref for User { } impl GenerateOtp for User { - fn get_hotp_code(&self, slot: u8) -> Result { + fn get_hotp_code(&self, slot: u8) -> Result { unsafe { let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; return result_from_string(nitrokey_sys::NK_get_hotp_code_PIN(slot, temp_password_ptr)); } } - fn get_totp_code(&self, slot: u8) -> Result { + fn get_totp_code(&self, slot: u8) -> Result { unsafe { let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; return result_from_string(nitrokey_sys::NK_get_totp_code_PIN( @@ -271,9 +271,9 @@ impl Admin { /// /// ```no_run /// use nitrokey::{Authenticate, Config}; - /// # use nitrokey::CommandError; + /// # use nitrokey::Error; /// - /// # fn try_main() -> Result<(), CommandError> { + /// # fn try_main() -> Result<(), Error> { /// let device = nitrokey::connect()?; /// let config = Config::new(None, None, None, false); /// match device.authenticate_admin("12345678") { @@ -288,7 +288,7 @@ impl Admin { /// ``` /// /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot - pub fn write_config(&self, config: Config) -> Result<(), CommandError> { + pub fn write_config(&self, config: Config) -> Result<(), Error> { let raw_config = RawConfig::try_from(config)?; unsafe { get_command_result(nitrokey_sys::NK_write_config( @@ -302,7 +302,7 @@ impl Admin { } } - fn write_otp_slot(&self, data: OtpSlotData, callback: C) -> Result<(), CommandError> + fn write_otp_slot(&self, data: OtpSlotData, callback: C) -> Result<(), Error> where C: Fn(RawOtpSlotData, *const c_char) -> c_int, { @@ -313,7 +313,7 @@ impl Admin { } impl ConfigureOtp for Admin { - fn write_hotp_slot(&self, data: OtpSlotData, counter: u64) -> Result<(), CommandError> { + fn write_hotp_slot(&self, data: OtpSlotData, counter: u64) -> Result<(), Error> { self.write_otp_slot(data, |raw_data: RawOtpSlotData, temp_password_ptr| unsafe { nitrokey_sys::NK_write_hotp_slot( raw_data.number, @@ -329,7 +329,7 @@ impl ConfigureOtp for Admin { }) } - fn write_totp_slot(&self, data: OtpSlotData, time_window: u16) -> Result<(), CommandError> { + fn write_totp_slot(&self, data: OtpSlotData, time_window: u16) -> Result<(), Error> { self.write_otp_slot(data, |raw_data: RawOtpSlotData, temp_password_ptr| unsafe { nitrokey_sys::NK_write_totp_slot( raw_data.number, @@ -345,12 +345,12 @@ impl ConfigureOtp for Admin { }) } - fn erase_hotp_slot(&self, slot: u8) -> Result<(), CommandError> { + fn erase_hotp_slot(&self, slot: u8) -> Result<(), Error> { let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; unsafe { get_command_result(nitrokey_sys::NK_erase_hotp_slot(slot, temp_password_ptr)) } } - fn erase_totp_slot(&self, slot: u8) -> Result<(), CommandError> { + fn erase_totp_slot(&self, slot: u8) -> Result<(), Error> { let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; unsafe { get_command_result(nitrokey_sys::NK_erase_totp_slot(slot, temp_password_ptr)) } } @@ -366,7 +366,7 @@ impl AuthenticatedDevice for Admin { } impl Authenticate for DeviceWrapper { - fn authenticate_user(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { match self { DeviceWrapper::Storage(storage) => { authenticate_user_wrapper(storage, DeviceWrapper::Storage, password) @@ -375,7 +375,7 @@ impl Authenticate for DeviceWrapper { } } - fn authenticate_admin(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { match self { DeviceWrapper::Storage(storage) => { authenticate_admin_wrapper(storage, DeviceWrapper::Storage, password) @@ -388,13 +388,13 @@ impl Authenticate for DeviceWrapper { } impl Authenticate for Pro { - fn authenticate_user(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) }) } - fn authenticate_admin(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_first_authenticate(password_ptr, temp_password_ptr) }) @@ -402,13 +402,13 @@ impl Authenticate for Pro { } impl Authenticate for Storage { - fn authenticate_user(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) }) } - fn authenticate_admin(self, password: &str) -> Result, (Self, CommandError)> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_first_authenticate(password_ptr, temp_password_ptr) }) -- cgit v1.2.1 From 944e1fa0d51e547dde2a9368d2b8431b109f63c4 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 04:15:31 +0000 Subject: Move the CommandError::Unknown to Error An error code can not only indiciate a command error, but also a library or device communication error. Therefore, the variant for an unknown error code should be placed in the top-level Error enum instead of the CommandError enum. --- src/auth.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 509d3aa..e05f6b3 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -6,7 +6,7 @@ use nitrokey_sys; use crate::config::{Config, RawConfig}; use crate::device::{Device, DeviceWrapper, Pro, Storage}; -use crate::error::{CommandError, Error}; +use crate::error::Error; use crate::otp::{ConfigureOtp, GenerateOtp, OtpMode, OtpSlotData, RawOtpSlotData}; use crate::util::{generate_password, get_command_result, get_cstring, result_from_string}; @@ -161,7 +161,7 @@ where let temp_password_ptr = temp_password.as_ptr() as *const c_char; return match callback(password_ptr, temp_password_ptr) { 0 => Ok(A::new(device, temp_password)), - rv => Err((device, CommandError::from(rv).into())), + rv => Err((device, Error::from(rv))), }; } -- cgit v1.2.1 From 5e258d26b55af6bed7c316b1c7ac12e20946702d Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Thu, 17 Jan 2019 12:47:52 +0000 Subject: Refactor library errors into LibraryError enum Previously, library errors were part of the CommandError enum. As command errors and library errors are two different error types, they should be split into two enums. --- src/auth.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index e05f6b3..d1eb049 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -55,7 +55,7 @@ pub trait Authenticate { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword fn authenticate_user(self, password: &str) -> Result, (Self, Error)> @@ -101,7 +101,7 @@ pub trait Authenticate { /// # } /// ``` /// - /// [`InvalidString`]: enum.CommandError.html#variant.InvalidString + /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> @@ -287,7 +287,7 @@ impl Admin { /// # } /// ``` /// - /// [`InvalidSlot`]: enum.CommandError.html#variant.InvalidSlot + /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot pub fn write_config(&self, config: Config) -> Result<(), Error> { let raw_config = RawConfig::try_from(config)?; unsafe { -- cgit v1.2.1 From e31009064eaaef9153ad5da3911aa0a939a050c2 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Wed, 23 Jan 2019 03:02:31 +0000 Subject: Add temp_password_ptr method to AuthenticatedDevice To reduce the number of casts, we introduce the temp_password_ptr method that casts the pointer received from the Vec to a c_char pointer that can be handled by libnitrokey. --- src/auth.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index d1eb049..541de35 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -111,6 +111,8 @@ pub trait Authenticate { trait AuthenticatedDevice { fn new(device: T, temp_password: Vec) -> Self; + + fn temp_password_ptr(&self) -> *const c_char; } /// A Nitrokey device with user authentication. @@ -217,20 +219,19 @@ impl Deref for User { impl GenerateOtp for User { fn get_hotp_code(&self, slot: u8) -> Result { unsafe { - let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; + let temp_password_ptr = self.temp_password_ptr(); return result_from_string(nitrokey_sys::NK_get_hotp_code_PIN(slot, temp_password_ptr)); } } fn get_totp_code(&self, slot: u8) -> Result { unsafe { - let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; return result_from_string(nitrokey_sys::NK_get_totp_code_PIN( slot, 0, 0, 0, - temp_password_ptr, + self.temp_password_ptr(), )); } } @@ -243,6 +244,10 @@ impl AuthenticatedDevice for User { temp_password, } } + + fn temp_password_ptr(&self) -> *const c_char { + self.temp_password.as_ptr() as *const c_char + } } impl Deref for Admin { @@ -297,7 +302,7 @@ impl Admin { raw_config.scrollock, raw_config.user_password, false, - self.temp_password.as_ptr() as *const c_char, + self.temp_password_ptr(), )) } } @@ -307,8 +312,7 @@ impl Admin { C: Fn(RawOtpSlotData, *const c_char) -> c_int, { let raw_data = RawOtpSlotData::new(data)?; - let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; - get_command_result(callback(raw_data, temp_password_ptr)) + get_command_result(callback(raw_data, self.temp_password_ptr())) } } @@ -346,12 +350,12 @@ impl ConfigureOtp for Admin { } fn erase_hotp_slot(&self, slot: u8) -> Result<(), Error> { - let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; + let temp_password_ptr = self.temp_password_ptr(); unsafe { get_command_result(nitrokey_sys::NK_erase_hotp_slot(slot, temp_password_ptr)) } } fn erase_totp_slot(&self, slot: u8) -> Result<(), Error> { - let temp_password_ptr = self.temp_password.as_ptr() as *const c_char; + let temp_password_ptr = self.temp_password_ptr(); unsafe { get_command_result(nitrokey_sys::NK_erase_totp_slot(slot, temp_password_ptr)) } } } @@ -363,6 +367,10 @@ impl AuthenticatedDevice for Admin { temp_password, } } + + fn temp_password_ptr(&self) -> *const c_char { + self.temp_password.as_ptr() as *const c_char + } } impl Authenticate for DeviceWrapper { -- cgit v1.2.1 From 5540ca5e76ffe5efe27d8819efb9e62066a10219 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Wed, 23 Jan 2019 03:46:09 +0000 Subject: Refactor and clean up all code This includes: - using idiomatic Rust - limiting the scope of unsafe blocks - simplifying code --- src/auth.rs | 59 ++++++++++++++++++++++++----------------------------------- 1 file changed, 24 insertions(+), 35 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 541de35..b97bee6 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -161,10 +161,10 @@ where }; let password_ptr = password.as_ptr(); let temp_password_ptr = temp_password.as_ptr() as *const c_char; - return match callback(password_ptr, temp_password_ptr) { + match callback(password_ptr, temp_password_ptr) { 0 => Ok(A::new(device, temp_password)), rv => Err((device, Error::from(rv))), - }; + } } fn authenticate_user_wrapper( @@ -218,22 +218,15 @@ impl Deref for User { impl GenerateOtp for User { fn get_hotp_code(&self, slot: u8) -> Result { - unsafe { - let temp_password_ptr = self.temp_password_ptr(); - return result_from_string(nitrokey_sys::NK_get_hotp_code_PIN(slot, temp_password_ptr)); - } + result_from_string(unsafe { + nitrokey_sys::NK_get_hotp_code_PIN(slot, self.temp_password_ptr()) + }) } fn get_totp_code(&self, slot: u8) -> Result { - unsafe { - return result_from_string(nitrokey_sys::NK_get_totp_code_PIN( - slot, - 0, - 0, - 0, - self.temp_password_ptr(), - )); - } + result_from_string(unsafe { + nitrokey_sys::NK_get_totp_code_PIN(slot, 0, 0, 0, self.temp_password_ptr()) + }) } } @@ -295,30 +288,23 @@ impl Admin { /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot pub fn write_config(&self, config: Config) -> Result<(), Error> { let raw_config = RawConfig::try_from(config)?; - unsafe { - get_command_result(nitrokey_sys::NK_write_config( + get_command_result(unsafe { + nitrokey_sys::NK_write_config( raw_config.numlock, raw_config.capslock, raw_config.scrollock, raw_config.user_password, false, self.temp_password_ptr(), - )) - } - } - - fn write_otp_slot(&self, data: OtpSlotData, callback: C) -> Result<(), Error> - where - C: Fn(RawOtpSlotData, *const c_char) -> c_int, - { - let raw_data = RawOtpSlotData::new(data)?; - get_command_result(callback(raw_data, self.temp_password_ptr())) + ) + }) } } impl ConfigureOtp for Admin { fn write_hotp_slot(&self, data: OtpSlotData, counter: u64) -> Result<(), Error> { - self.write_otp_slot(data, |raw_data: RawOtpSlotData, temp_password_ptr| unsafe { + let raw_data = RawOtpSlotData::new(data)?; + get_command_result(unsafe { nitrokey_sys::NK_write_hotp_slot( raw_data.number, raw_data.name.as_ptr(), @@ -328,13 +314,14 @@ impl ConfigureOtp for Admin { raw_data.use_enter, raw_data.use_token_id, raw_data.token_id.as_ptr(), - temp_password_ptr, + self.temp_password_ptr(), ) }) } fn write_totp_slot(&self, data: OtpSlotData, time_window: u16) -> Result<(), Error> { - self.write_otp_slot(data, |raw_data: RawOtpSlotData, temp_password_ptr| unsafe { + let raw_data = RawOtpSlotData::new(data)?; + get_command_result(unsafe { nitrokey_sys::NK_write_totp_slot( raw_data.number, raw_data.name.as_ptr(), @@ -344,19 +331,21 @@ impl ConfigureOtp for Admin { raw_data.use_enter, raw_data.use_token_id, raw_data.token_id.as_ptr(), - temp_password_ptr, + self.temp_password_ptr(), ) }) } fn erase_hotp_slot(&self, slot: u8) -> Result<(), Error> { - let temp_password_ptr = self.temp_password_ptr(); - unsafe { get_command_result(nitrokey_sys::NK_erase_hotp_slot(slot, temp_password_ptr)) } + get_command_result(unsafe { + nitrokey_sys::NK_erase_hotp_slot(slot, self.temp_password_ptr()) + }) } fn erase_totp_slot(&self, slot: u8) -> Result<(), Error> { - let temp_password_ptr = self.temp_password_ptr(); - unsafe { get_command_result(nitrokey_sys::NK_erase_totp_slot(slot, temp_password_ptr)) } + get_command_result(unsafe { + nitrokey_sys::NK_erase_totp_slot(slot, self.temp_password_ptr()) + }) } } -- cgit v1.2.1 From fdb7bac3063e62776bfc13f184cf786da19f42d1 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Wed, 23 Jan 2019 16:33:26 +0100 Subject: Add license and copyright information This patch adds license and copyright information to all files to make nitrokey-rs compliant with the REUSE practices [0]. [0] https://reuse.software/practices/2.0/ --- src/auth.rs | 3 +++ 1 file changed, 3 insertions(+) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index b97bee6..18b6572 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,3 +1,6 @@ +// Copyright (C) 2018-2019 Robin Krahl +// SPDX-License-Identifier: MIT + use std::ops::Deref; use std::os::raw::c_char; use std::os::raw::c_int; -- cgit v1.2.1 From 12b6a4d6c56cba4c2a87027d7c5f26ebe42d3315 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Mon, 28 Jan 2019 20:02:18 +0000 Subject: Prefer eprintln over println for error messages --- src/auth.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 18b6572..8978f32 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -49,7 +49,7 @@ pub trait Authenticate { /// user.device() /// }, /// Err((device, err)) => { - /// println!("Could not authenticate as user: {}", err); + /// eprintln!("Could not authenticate as user: {}", err); /// device /// }, /// }; @@ -95,7 +95,7 @@ pub trait Authenticate { /// admin.device() /// }, /// Err((device, err)) => { - /// println!("Could not authenticate as admin: {}", err); + /// eprintln!("Could not authenticate as admin: {}", err); /// device /// }, /// }; @@ -282,7 +282,7 @@ impl Admin { /// admin.write_config(config); /// () /// }, - /// Err((_, err)) => println!("Could not authenticate as admin: {}", err), + /// Err((_, err)) => eprintln!("Could not authenticate as admin: {}", err), /// }; /// # Ok(()) /// # } -- cgit v1.2.1 From e97ccf213eec4e2d056c2f72079e4eeb7ac66f3f Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Mon, 28 Jan 2019 12:05:42 +0000 Subject: Implement DerefMut for User and Admin As we want to change some methods to take a mutable reference to a Device, we implement DerefMut for User and Admin so that users can obtain a mutable reference to the wrapped device. --- src/auth.rs | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 8978f32..8cec49c 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,7 +1,7 @@ // Copyright (C) 2018-2019 Robin Krahl // SPDX-License-Identifier: MIT -use std::ops::Deref; +use std::ops; use std::os::raw::c_char; use std::os::raw::c_int; @@ -211,7 +211,7 @@ impl User { } } -impl Deref for User { +impl ops::Deref for User { type Target = T; fn deref(&self) -> &Self::Target { @@ -219,6 +219,12 @@ impl Deref for User { } } +impl ops::DerefMut for User { + fn deref_mut(&mut self) -> &mut T { + &mut self.device + } +} + impl GenerateOtp for User { fn get_hotp_code(&self, slot: u8) -> Result { result_from_string(unsafe { @@ -246,7 +252,7 @@ impl AuthenticatedDevice for User { } } -impl Deref for Admin { +impl ops::Deref for Admin { type Target = T; fn deref(&self) -> &Self::Target { @@ -254,6 +260,12 @@ impl Deref for Admin { } } +impl ops::DerefMut for Admin { + fn deref_mut(&mut self) -> &mut T { + &mut self.device + } +} + impl Admin { /// Forgets the user authentication and returns an unauthenticated device. This method /// consumes the authenticated device. It does not perform any actual commands on the -- cgit v1.2.1 From f49e61589e32217f97c94aa86d826f6b65170fba Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Mon, 28 Jan 2019 12:27:15 +0000 Subject: Require mutable reference if method changes device state Previously, all methods that access a Nitrokey device took a reference to the device as input. This method changes methods that change the device state to require a mutable reference instead. In most case, this is straightforward as the method writes data to the device (for example write_config or change_user_pin). But there are two edge cases: - Authenticating with a PIN changes the device state as it may decrease the PIN retry counter if the authentication fails. - Generating an HOTP code changes the device state as it increases the HOTP counter. --- src/auth.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 8cec49c..f9f50fa 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -226,7 +226,7 @@ impl ops::DerefMut for User { } impl GenerateOtp for User { - fn get_hotp_code(&self, slot: u8) -> Result { + fn get_hotp_code(&mut self, slot: u8) -> Result { result_from_string(unsafe { nitrokey_sys::NK_get_hotp_code_PIN(slot, self.temp_password_ptr()) }) @@ -290,7 +290,7 @@ impl Admin { /// let device = nitrokey::connect()?; /// let config = Config::new(None, None, None, false); /// match device.authenticate_admin("12345678") { - /// Ok(admin) => { + /// Ok(mut admin) => { /// admin.write_config(config); /// () /// }, @@ -301,7 +301,7 @@ impl Admin { /// ``` /// /// [`InvalidSlot`]: enum.LibraryError.html#variant.InvalidSlot - pub fn write_config(&self, config: Config) -> Result<(), Error> { + pub fn write_config(&mut self, config: Config) -> Result<(), Error> { let raw_config = RawConfig::try_from(config)?; get_command_result(unsafe { nitrokey_sys::NK_write_config( @@ -317,7 +317,7 @@ impl Admin { } impl ConfigureOtp for Admin { - fn write_hotp_slot(&self, data: OtpSlotData, counter: u64) -> Result<(), Error> { + fn write_hotp_slot(&mut self, data: OtpSlotData, counter: u64) -> Result<(), Error> { let raw_data = RawOtpSlotData::new(data)?; get_command_result(unsafe { nitrokey_sys::NK_write_hotp_slot( @@ -334,7 +334,7 @@ impl ConfigureOtp for Admin { }) } - fn write_totp_slot(&self, data: OtpSlotData, time_window: u16) -> Result<(), Error> { + fn write_totp_slot(&mut self, data: OtpSlotData, time_window: u16) -> Result<(), Error> { let raw_data = RawOtpSlotData::new(data)?; get_command_result(unsafe { nitrokey_sys::NK_write_totp_slot( @@ -351,13 +351,13 @@ impl ConfigureOtp for Admin { }) } - fn erase_hotp_slot(&self, slot: u8) -> Result<(), Error> { + fn erase_hotp_slot(&mut self, slot: u8) -> Result<(), Error> { get_command_result(unsafe { nitrokey_sys::NK_erase_hotp_slot(slot, self.temp_password_ptr()) }) } - fn erase_totp_slot(&self, slot: u8) -> Result<(), Error> { + fn erase_totp_slot(&mut self, slot: u8) -> Result<(), Error> { get_command_result(unsafe { nitrokey_sys::NK_erase_totp_slot(slot, self.temp_password_ptr()) }) -- cgit v1.2.1 From 0972bbe82623c3d9649b6023d8f50d304aa0cde6 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Mon, 28 Jan 2019 14:24:12 +0000 Subject: Refactor User and Admin to use a mutable reference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In the initial nitrokey-rs implementation, the Admin and the User struct take the Device by value to make sure that the user cannot initiate a second authentication while this first is still active (which would invalidate the temporary password). Now we realized that this is not necessary – taking a mutable reference has the same effect, but leads to a much cleaner API. This patch refactors the Admin and User structs – and all dependent code – to use a mutable reference instead of a Device value. --- src/auth.rs | 183 ++++++++++++++++++------------------------------------------ 1 file changed, 53 insertions(+), 130 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index f9f50fa..573fed3 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -42,16 +42,10 @@ pub trait Authenticate { /// fn perform_other_task(device: &DeviceWrapper) {} /// /// # fn try_main() -> Result<(), Error> { - /// let device = nitrokey::connect()?; - /// let device = match device.authenticate_user("123456") { - /// Ok(user) => { - /// perform_user_task(&user); - /// user.device() - /// }, - /// Err((device, err)) => { - /// eprintln!("Could not authenticate as user: {}", err); - /// device - /// }, + /// let mut device = nitrokey::connect()?; + /// match device.authenticate_user("123456") { + /// Ok(user) => perform_user_task(&user), + /// Err(err) => eprintln!("Could not authenticate as user: {}", err), /// }; /// perform_other_task(&device); /// # Ok(()) @@ -61,9 +55,9 @@ pub trait Authenticate { /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn authenticate_user(self, password: &str) -> Result, (Self, Error)> + fn authenticate_user(&mut self, password: &str) -> Result, Error> where - Self: Device + Sized; + Self: Device + std::marker::Sized; /// Performs admin authentication. This method consumes the device. If successful, an /// authenticated device is returned. Otherwise, the current unauthenticated device and the @@ -88,16 +82,10 @@ pub trait Authenticate { /// fn perform_other_task(device: &DeviceWrapper) {} /// /// # fn try_main() -> Result<(), Error> { - /// let device = nitrokey::connect()?; - /// let device = match device.authenticate_admin("123456") { - /// Ok(admin) => { - /// perform_admin_task(&admin); - /// admin.device() - /// }, - /// Err((device, err)) => { - /// eprintln!("Could not authenticate as admin: {}", err); - /// device - /// }, + /// let mut device = nitrokey::connect()?; + /// match device.authenticate_admin("123456") { + /// Ok(admin) => perform_admin_task(&admin), + /// Err(err) => eprintln!("Could not authenticate as admin: {}", err), /// }; /// perform_other_task(&device); /// # Ok(()) @@ -107,13 +95,13 @@ pub trait Authenticate { /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> + fn authenticate_admin(&mut self, password: &str) -> Result, Error> where - Self: Device + Sized; + Self: Device + std::marker::Sized; } -trait AuthenticatedDevice { - fn new(device: T, temp_password: Vec) -> Self; +trait AuthenticatedDevice<'a, T> { + fn new(device: &'a mut T, temp_password: Vec) -> Self; fn temp_password_ptr(&self) -> *const c_char; } @@ -128,8 +116,8 @@ trait AuthenticatedDevice { /// [`authenticate_admin`]: trait.Authenticate.html#method.authenticate_admin /// [`device`]: #method.device #[derive(Debug)] -pub struct User { - device: T, +pub struct User<'a, T: Device> { + device: &'a mut T, temp_password: Vec, } @@ -143,89 +131,42 @@ pub struct User { /// [`authenticate_admin`]: trait.Authenticate.html#method.authenticate_admin /// [`device`]: #method.device #[derive(Debug)] -pub struct Admin { - device: T, +pub struct Admin<'a, T: Device> { + device: &'a mut T, temp_password: Vec, } -fn authenticate(device: D, password: &str, callback: T) -> Result +fn authenticate<'a, D, A, T>(device: &'a mut D, password: &str, callback: T) -> Result where D: Device, - A: AuthenticatedDevice, + A: AuthenticatedDevice<'a, D>, T: Fn(*const c_char, *const c_char) -> c_int, { - let temp_password = match generate_password(TEMPORARY_PASSWORD_LENGTH) { - Ok(temp_password) => temp_password, - Err(err) => return Err((device, err)), - }; - let password = match get_cstring(password) { - Ok(password) => password, - Err(err) => return Err((device, err)), - }; + let temp_password = generate_password(TEMPORARY_PASSWORD_LENGTH)?; + let password = get_cstring(password)?; let password_ptr = password.as_ptr(); let temp_password_ptr = temp_password.as_ptr() as *const c_char; match callback(password_ptr, temp_password_ptr) { 0 => Ok(A::new(device, temp_password)), - rv => Err((device, Error::from(rv))), + rv => Err(Error::from(rv)), } } -fn authenticate_user_wrapper( - device: T, - constructor: C, - password: &str, -) -> Result, (DeviceWrapper, Error)> -where - T: Device, - C: Fn(T) -> DeviceWrapper, -{ - let result = device.authenticate_user(password); - match result { - Ok(user) => Ok(User::new(constructor(user.device), user.temp_password)), - Err((device, err)) => Err((constructor(device), err)), - } -} - -fn authenticate_admin_wrapper( - device: T, - constructor: C, - password: &str, -) -> Result, (DeviceWrapper, Error)> -where - T: Device, - C: Fn(T) -> DeviceWrapper, -{ - let result = device.authenticate_admin(password); - match result { - Ok(user) => Ok(Admin::new(constructor(user.device), user.temp_password)), - Err((device, err)) => Err((constructor(device), err)), - } -} - -impl User { - /// Forgets the user authentication and returns an unauthenticated device. This method - /// consumes the authenticated device. It does not perform any actual commands on the - /// Nitrokey. - pub fn device(self) -> T { - self.device - } -} - -impl ops::Deref for User { +impl<'a, T: Device> ops::Deref for User<'a, T> { type Target = T; fn deref(&self) -> &Self::Target { - &self.device + self.device } } -impl ops::DerefMut for User { +impl<'a, T: Device> ops::DerefMut for User<'a, T> { fn deref_mut(&mut self) -> &mut T { - &mut self.device + self.device } } -impl GenerateOtp for User { +impl<'a, T: Device> GenerateOtp for User<'a, T> { fn get_hotp_code(&mut self, slot: u8) -> Result { result_from_string(unsafe { nitrokey_sys::NK_get_hotp_code_PIN(slot, self.temp_password_ptr()) @@ -239,8 +180,8 @@ impl GenerateOtp for User { } } -impl AuthenticatedDevice for User { - fn new(device: T, temp_password: Vec) -> Self { +impl<'a, T: Device> AuthenticatedDevice<'a, T> for User<'a, T> { + fn new(device: &'a mut T, temp_password: Vec) -> Self { User { device, temp_password, @@ -252,28 +193,21 @@ impl AuthenticatedDevice for User { } } -impl ops::Deref for Admin { +impl<'a, T: Device> ops::Deref for Admin<'a, T> { type Target = T; fn deref(&self) -> &Self::Target { - &self.device + self.device } } -impl ops::DerefMut for Admin { +impl<'a, T: Device> ops::DerefMut for Admin<'a, T> { fn deref_mut(&mut self) -> &mut T { - &mut self.device - } -} - -impl Admin { - /// Forgets the user authentication and returns an unauthenticated device. This method - /// consumes the authenticated device. It does not perform any actual commands on the - /// Nitrokey. - pub fn device(self) -> T { self.device } +} +impl<'a, T: Device> Admin<'a, T> { /// Writes the given configuration to the Nitrokey device. /// /// # Errors @@ -287,14 +221,11 @@ impl Admin { /// # use nitrokey::Error; /// /// # fn try_main() -> Result<(), Error> { - /// let device = nitrokey::connect()?; + /// let mut device = nitrokey::connect()?; /// let config = Config::new(None, None, None, false); /// match device.authenticate_admin("12345678") { - /// Ok(mut admin) => { - /// admin.write_config(config); - /// () - /// }, - /// Err((_, err)) => eprintln!("Could not authenticate as admin: {}", err), + /// Ok(mut admin) => admin.write_config(config)?, + /// Err(err) => eprintln!("Could not authenticate as admin: {}", err), /// }; /// # Ok(()) /// # } @@ -316,7 +247,7 @@ impl Admin { } } -impl ConfigureOtp for Admin { +impl<'a, T: Device> ConfigureOtp for Admin<'a, T> { fn write_hotp_slot(&mut self, data: OtpSlotData, counter: u64) -> Result<(), Error> { let raw_data = RawOtpSlotData::new(data)?; get_command_result(unsafe { @@ -364,8 +295,8 @@ impl ConfigureOtp for Admin { } } -impl AuthenticatedDevice for Admin { - fn new(device: T, temp_password: Vec) -> Self { +impl<'a, T: Device> AuthenticatedDevice<'a, T> for Admin<'a, T> { + fn new(device: &'a mut T, temp_password: Vec) -> Self { Admin { device, temp_password, @@ -378,35 +309,27 @@ impl AuthenticatedDevice for Admin { } impl Authenticate for DeviceWrapper { - fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { - match self { - DeviceWrapper::Storage(storage) => { - authenticate_user_wrapper(storage, DeviceWrapper::Storage, password) - } - DeviceWrapper::Pro(pro) => authenticate_user_wrapper(pro, DeviceWrapper::Pro, password), - } + fn authenticate_user(&mut self, password: &str) -> Result, Error> { + authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { + nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) + }) } - fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { - match self { - DeviceWrapper::Storage(storage) => { - authenticate_admin_wrapper(storage, DeviceWrapper::Storage, password) - } - DeviceWrapper::Pro(pro) => { - authenticate_admin_wrapper(pro, DeviceWrapper::Pro, password) - } - } + fn authenticate_admin(&mut self, password: &str) -> Result, Error> { + authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { + nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) + }) } } impl Authenticate for Pro { - fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { + fn authenticate_user(&mut self, password: &str) -> Result, Error> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) }) } - fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { + fn authenticate_admin(&mut self, password: &str) -> Result, Error> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_first_authenticate(password_ptr, temp_password_ptr) }) @@ -414,13 +337,13 @@ impl Authenticate for Pro { } impl Authenticate for Storage { - fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { + fn authenticate_user(&mut self, password: &str) -> Result, Error> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) }) } - fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { + fn authenticate_admin(&mut self, password: &str) -> Result, Error> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_first_authenticate(password_ptr, temp_password_ptr) }) -- cgit v1.2.1 From 83641ca0518e4c766c63e40d0787e4f0b436652a Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Tue, 5 Feb 2019 12:47:24 +0000 Subject: Revert "Refactor User and Admin to use a mutable reference" This reverts commit 0972bbe82623c3d9649b6023d8f50d304aa0cde6. --- src/auth.rs | 183 ++++++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 130 insertions(+), 53 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 573fed3..f9f50fa 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -42,10 +42,16 @@ pub trait Authenticate { /// fn perform_other_task(device: &DeviceWrapper) {} /// /// # fn try_main() -> Result<(), Error> { - /// let mut device = nitrokey::connect()?; - /// match device.authenticate_user("123456") { - /// Ok(user) => perform_user_task(&user), - /// Err(err) => eprintln!("Could not authenticate as user: {}", err), + /// let device = nitrokey::connect()?; + /// let device = match device.authenticate_user("123456") { + /// Ok(user) => { + /// perform_user_task(&user); + /// user.device() + /// }, + /// Err((device, err)) => { + /// eprintln!("Could not authenticate as user: {}", err); + /// device + /// }, /// }; /// perform_other_task(&device); /// # Ok(()) @@ -55,9 +61,9 @@ pub trait Authenticate { /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn authenticate_user(&mut self, password: &str) -> Result, Error> + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> where - Self: Device + std::marker::Sized; + Self: Device + Sized; /// Performs admin authentication. This method consumes the device. If successful, an /// authenticated device is returned. Otherwise, the current unauthenticated device and the @@ -82,10 +88,16 @@ pub trait Authenticate { /// fn perform_other_task(device: &DeviceWrapper) {} /// /// # fn try_main() -> Result<(), Error> { - /// let mut device = nitrokey::connect()?; - /// match device.authenticate_admin("123456") { - /// Ok(admin) => perform_admin_task(&admin), - /// Err(err) => eprintln!("Could not authenticate as admin: {}", err), + /// let device = nitrokey::connect()?; + /// let device = match device.authenticate_admin("123456") { + /// Ok(admin) => { + /// perform_admin_task(&admin); + /// admin.device() + /// }, + /// Err((device, err)) => { + /// eprintln!("Could not authenticate as admin: {}", err); + /// device + /// }, /// }; /// perform_other_task(&device); /// # Ok(()) @@ -95,13 +107,13 @@ pub trait Authenticate { /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn authenticate_admin(&mut self, password: &str) -> Result, Error> + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> where - Self: Device + std::marker::Sized; + Self: Device + Sized; } -trait AuthenticatedDevice<'a, T> { - fn new(device: &'a mut T, temp_password: Vec) -> Self; +trait AuthenticatedDevice { + fn new(device: T, temp_password: Vec) -> Self; fn temp_password_ptr(&self) -> *const c_char; } @@ -116,8 +128,8 @@ trait AuthenticatedDevice<'a, T> { /// [`authenticate_admin`]: trait.Authenticate.html#method.authenticate_admin /// [`device`]: #method.device #[derive(Debug)] -pub struct User<'a, T: Device> { - device: &'a mut T, +pub struct User { + device: T, temp_password: Vec, } @@ -131,42 +143,89 @@ pub struct User<'a, T: Device> { /// [`authenticate_admin`]: trait.Authenticate.html#method.authenticate_admin /// [`device`]: #method.device #[derive(Debug)] -pub struct Admin<'a, T: Device> { - device: &'a mut T, +pub struct Admin { + device: T, temp_password: Vec, } -fn authenticate<'a, D, A, T>(device: &'a mut D, password: &str, callback: T) -> Result +fn authenticate(device: D, password: &str, callback: T) -> Result where D: Device, - A: AuthenticatedDevice<'a, D>, + A: AuthenticatedDevice, T: Fn(*const c_char, *const c_char) -> c_int, { - let temp_password = generate_password(TEMPORARY_PASSWORD_LENGTH)?; - let password = get_cstring(password)?; + let temp_password = match generate_password(TEMPORARY_PASSWORD_LENGTH) { + Ok(temp_password) => temp_password, + Err(err) => return Err((device, err)), + }; + let password = match get_cstring(password) { + Ok(password) => password, + Err(err) => return Err((device, err)), + }; let password_ptr = password.as_ptr(); let temp_password_ptr = temp_password.as_ptr() as *const c_char; match callback(password_ptr, temp_password_ptr) { 0 => Ok(A::new(device, temp_password)), - rv => Err(Error::from(rv)), + rv => Err((device, Error::from(rv))), } } -impl<'a, T: Device> ops::Deref for User<'a, T> { +fn authenticate_user_wrapper( + device: T, + constructor: C, + password: &str, +) -> Result, (DeviceWrapper, Error)> +where + T: Device, + C: Fn(T) -> DeviceWrapper, +{ + let result = device.authenticate_user(password); + match result { + Ok(user) => Ok(User::new(constructor(user.device), user.temp_password)), + Err((device, err)) => Err((constructor(device), err)), + } +} + +fn authenticate_admin_wrapper( + device: T, + constructor: C, + password: &str, +) -> Result, (DeviceWrapper, Error)> +where + T: Device, + C: Fn(T) -> DeviceWrapper, +{ + let result = device.authenticate_admin(password); + match result { + Ok(user) => Ok(Admin::new(constructor(user.device), user.temp_password)), + Err((device, err)) => Err((constructor(device), err)), + } +} + +impl User { + /// Forgets the user authentication and returns an unauthenticated device. This method + /// consumes the authenticated device. It does not perform any actual commands on the + /// Nitrokey. + pub fn device(self) -> T { + self.device + } +} + +impl ops::Deref for User { type Target = T; fn deref(&self) -> &Self::Target { - self.device + &self.device } } -impl<'a, T: Device> ops::DerefMut for User<'a, T> { +impl ops::DerefMut for User { fn deref_mut(&mut self) -> &mut T { - self.device + &mut self.device } } -impl<'a, T: Device> GenerateOtp for User<'a, T> { +impl GenerateOtp for User { fn get_hotp_code(&mut self, slot: u8) -> Result { result_from_string(unsafe { nitrokey_sys::NK_get_hotp_code_PIN(slot, self.temp_password_ptr()) @@ -180,8 +239,8 @@ impl<'a, T: Device> GenerateOtp for User<'a, T> { } } -impl<'a, T: Device> AuthenticatedDevice<'a, T> for User<'a, T> { - fn new(device: &'a mut T, temp_password: Vec) -> Self { +impl AuthenticatedDevice for User { + fn new(device: T, temp_password: Vec) -> Self { User { device, temp_password, @@ -193,21 +252,28 @@ impl<'a, T: Device> AuthenticatedDevice<'a, T> for User<'a, T> { } } -impl<'a, T: Device> ops::Deref for Admin<'a, T> { +impl ops::Deref for Admin { type Target = T; fn deref(&self) -> &Self::Target { - self.device + &self.device } } -impl<'a, T: Device> ops::DerefMut for Admin<'a, T> { +impl ops::DerefMut for Admin { fn deref_mut(&mut self) -> &mut T { - self.device + &mut self.device } } -impl<'a, T: Device> Admin<'a, T> { +impl Admin { + /// Forgets the user authentication and returns an unauthenticated device. This method + /// consumes the authenticated device. It does not perform any actual commands on the + /// Nitrokey. + pub fn device(self) -> T { + self.device + } + /// Writes the given configuration to the Nitrokey device. /// /// # Errors @@ -221,11 +287,14 @@ impl<'a, T: Device> Admin<'a, T> { /// # use nitrokey::Error; /// /// # fn try_main() -> Result<(), Error> { - /// let mut device = nitrokey::connect()?; + /// let device = nitrokey::connect()?; /// let config = Config::new(None, None, None, false); /// match device.authenticate_admin("12345678") { - /// Ok(mut admin) => admin.write_config(config)?, - /// Err(err) => eprintln!("Could not authenticate as admin: {}", err), + /// Ok(mut admin) => { + /// admin.write_config(config); + /// () + /// }, + /// Err((_, err)) => eprintln!("Could not authenticate as admin: {}", err), /// }; /// # Ok(()) /// # } @@ -247,7 +316,7 @@ impl<'a, T: Device> Admin<'a, T> { } } -impl<'a, T: Device> ConfigureOtp for Admin<'a, T> { +impl ConfigureOtp for Admin { fn write_hotp_slot(&mut self, data: OtpSlotData, counter: u64) -> Result<(), Error> { let raw_data = RawOtpSlotData::new(data)?; get_command_result(unsafe { @@ -295,8 +364,8 @@ impl<'a, T: Device> ConfigureOtp for Admin<'a, T> { } } -impl<'a, T: Device> AuthenticatedDevice<'a, T> for Admin<'a, T> { - fn new(device: &'a mut T, temp_password: Vec) -> Self { +impl AuthenticatedDevice for Admin { + fn new(device: T, temp_password: Vec) -> Self { Admin { device, temp_password, @@ -309,27 +378,35 @@ impl<'a, T: Device> AuthenticatedDevice<'a, T> for Admin<'a, T> { } impl Authenticate for DeviceWrapper { - fn authenticate_user(&mut self, password: &str) -> Result, Error> { - authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { - nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) - }) + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { + match self { + DeviceWrapper::Storage(storage) => { + authenticate_user_wrapper(storage, DeviceWrapper::Storage, password) + } + DeviceWrapper::Pro(pro) => authenticate_user_wrapper(pro, DeviceWrapper::Pro, password), + } } - fn authenticate_admin(&mut self, password: &str) -> Result, Error> { - authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { - nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) - }) + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { + match self { + DeviceWrapper::Storage(storage) => { + authenticate_admin_wrapper(storage, DeviceWrapper::Storage, password) + } + DeviceWrapper::Pro(pro) => { + authenticate_admin_wrapper(pro, DeviceWrapper::Pro, password) + } + } } } impl Authenticate for Pro { - fn authenticate_user(&mut self, password: &str) -> Result, Error> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) }) } - fn authenticate_admin(&mut self, password: &str) -> Result, Error> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_first_authenticate(password_ptr, temp_password_ptr) }) @@ -337,13 +414,13 @@ impl Authenticate for Pro { } impl Authenticate for Storage { - fn authenticate_user(&mut self, password: &str) -> Result, Error> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) }) } - fn authenticate_admin(&mut self, password: &str) -> Result, Error> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_first_authenticate(password_ptr, temp_password_ptr) }) -- cgit v1.2.1 From fe2f39826ade5a156945dabb8c8ab725378a15c1 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 27 Jan 2019 18:42:14 +0000 Subject: 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. --- src/auth.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index f9f50fa..2ed7bfc 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -170,14 +170,14 @@ where } } -fn authenticate_user_wrapper( +fn authenticate_user_wrapper<'a, T, C>( device: T, constructor: C, password: &str, -) -> Result, (DeviceWrapper, Error)> +) -> Result>, (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( +fn authenticate_admin_wrapper<'a, T, C>( device: T, constructor: C, password: &str, -) -> Result, (DeviceWrapper, Error)> +) -> Result>, (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 AuthenticatedDevice for Admin { } } -impl Authenticate for DeviceWrapper { +impl<'a> Authenticate for DeviceWrapper<'a> { fn authenticate_user(self, password: &str) -> Result, (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, (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, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) -- cgit v1.2.1 From 12fa62483cf45d868099d5d4020333af492eebde Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Tue, 9 Jul 2019 08:09:02 +0000 Subject: Introduce into_manager for Device MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To enable applications like nitrokey-test to go back to a manager instance from a Device instance, we add the into_manager function to the Device trait. To do that, we have to keep track of the Manager’s lifetime by adding a lifetime to Device (and then to some other traits that use Device). --- src/auth.rs | 69 +++++++++++++++++++++++++++++++++---------------------------- 1 file changed, 37 insertions(+), 32 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 2ed7bfc..829d083 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,6 +1,7 @@ // Copyright (C) 2018-2019 Robin Krahl // SPDX-License-Identifier: MIT +use std::marker; use std::ops; use std::os::raw::c_char; use std::os::raw::c_int; @@ -18,7 +19,7 @@ static TEMPORARY_PASSWORD_LENGTH: usize = 25; /// Provides methods to authenticate as a user or as an admin using a PIN. The authenticated /// methods will consume the current device instance. On success, they return the authenticated /// device. Otherwise, they return the current unauthenticated device and the error code. -pub trait Authenticate { +pub trait Authenticate<'a> { /// Performs user authentication. This method consumes the device. If successful, an /// authenticated device is returned. Otherwise, the current unauthenticated device and the /// error are returned. @@ -61,9 +62,9 @@ pub trait Authenticate { /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn authenticate_user(self, password: &str) -> Result, (Self, Error)> + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> where - Self: Device + Sized; + Self: Device<'a> + Sized; /// Performs admin authentication. This method consumes the device. If successful, an /// authenticated device is returned. Otherwise, the current unauthenticated device and the @@ -107,9 +108,9 @@ pub trait Authenticate { /// [`InvalidString`]: enum.LibraryError.html#variant.InvalidString /// [`RngError`]: enum.CommandError.html#variant.RngError /// [`WrongPassword`]: enum.CommandError.html#variant.WrongPassword - fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> where - Self: Device + Sized; + Self: Device<'a> + Sized; } trait AuthenticatedDevice { @@ -128,9 +129,10 @@ trait AuthenticatedDevice { /// [`authenticate_admin`]: trait.Authenticate.html#method.authenticate_admin /// [`device`]: #method.device #[derive(Debug)] -pub struct User { +pub struct User<'a, T: Device<'a>> { device: T, temp_password: Vec, + marker: marker::PhantomData<&'a T>, } /// A Nitrokey device with admin authentication. @@ -143,14 +145,15 @@ pub struct User { /// [`authenticate_admin`]: trait.Authenticate.html#method.authenticate_admin /// [`device`]: #method.device #[derive(Debug)] -pub struct Admin { +pub struct Admin<'a, T: Device<'a>> { device: T, temp_password: Vec, + marker: marker::PhantomData<&'a T>, } -fn authenticate(device: D, password: &str, callback: T) -> Result +fn authenticate<'a, D, A, T>(device: D, password: &str, callback: T) -> Result where - D: Device, + D: Device<'a>, A: AuthenticatedDevice, T: Fn(*const c_char, *const c_char) -> c_int, { @@ -174,9 +177,9 @@ fn authenticate_user_wrapper<'a, T, C>( device: T, constructor: C, password: &str, -) -> Result>, (DeviceWrapper<'a>, Error)> +) -> Result>, (DeviceWrapper<'a>, Error)> where - T: Device, + T: Device<'a> + 'a, C: Fn(T) -> DeviceWrapper<'a>, { let result = device.authenticate_user(password); @@ -190,9 +193,9 @@ fn authenticate_admin_wrapper<'a, T, C>( device: T, constructor: C, password: &str, -) -> Result>, (DeviceWrapper<'a>, Error)> +) -> Result>, (DeviceWrapper<'a>, Error)> where - T: Device, + T: Device<'a> + 'a, C: Fn(T) -> DeviceWrapper<'a>, { let result = device.authenticate_admin(password); @@ -202,7 +205,7 @@ where } } -impl User { +impl<'a, T: Device<'a>> User<'a, T> { /// Forgets the user authentication and returns an unauthenticated device. This method /// consumes the authenticated device. It does not perform any actual commands on the /// Nitrokey. @@ -211,7 +214,7 @@ impl User { } } -impl ops::Deref for User { +impl<'a, T: Device<'a>> ops::Deref for User<'a, T> { type Target = T; fn deref(&self) -> &Self::Target { @@ -219,13 +222,13 @@ impl ops::Deref for User { } } -impl ops::DerefMut for User { +impl<'a, T: Device<'a>> ops::DerefMut for User<'a, T> { fn deref_mut(&mut self) -> &mut T { &mut self.device } } -impl GenerateOtp for User { +impl<'a, T: Device<'a>> GenerateOtp for User<'a, T> { fn get_hotp_code(&mut self, slot: u8) -> Result { result_from_string(unsafe { nitrokey_sys::NK_get_hotp_code_PIN(slot, self.temp_password_ptr()) @@ -239,11 +242,12 @@ impl GenerateOtp for User { } } -impl AuthenticatedDevice for User { +impl<'a, T: Device<'a>> AuthenticatedDevice for User<'a, T> { fn new(device: T, temp_password: Vec) -> Self { User { device, temp_password, + marker: marker::PhantomData, } } @@ -252,7 +256,7 @@ impl AuthenticatedDevice for User { } } -impl ops::Deref for Admin { +impl<'a, T: Device<'a>> ops::Deref for Admin<'a, T> { type Target = T; fn deref(&self) -> &Self::Target { @@ -260,13 +264,13 @@ impl ops::Deref for Admin { } } -impl ops::DerefMut for Admin { +impl<'a, T: Device<'a>> ops::DerefMut for Admin<'a, T> { fn deref_mut(&mut self) -> &mut T { &mut self.device } } -impl Admin { +impl<'a, T: Device<'a>> Admin<'a, T> { /// Forgets the user authentication and returns an unauthenticated device. This method /// consumes the authenticated device. It does not perform any actual commands on the /// Nitrokey. @@ -316,7 +320,7 @@ impl Admin { } } -impl ConfigureOtp for Admin { +impl<'a, T: Device<'a>> ConfigureOtp for Admin<'a, T> { fn write_hotp_slot(&mut self, data: OtpSlotData, counter: u64) -> Result<(), Error> { let raw_data = RawOtpSlotData::new(data)?; get_command_result(unsafe { @@ -364,11 +368,12 @@ impl ConfigureOtp for Admin { } } -impl AuthenticatedDevice for Admin { +impl<'a, T: Device<'a>> AuthenticatedDevice for Admin<'a, T> { fn new(device: T, temp_password: Vec) -> Self { Admin { device, temp_password, + marker: marker::PhantomData, } } @@ -377,8 +382,8 @@ impl AuthenticatedDevice for Admin { } } -impl<'a> Authenticate for DeviceWrapper<'a> { - fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { +impl<'a> Authenticate<'a> for DeviceWrapper<'a> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { match self { DeviceWrapper::Storage(storage) => { authenticate_user_wrapper(storage, DeviceWrapper::Storage, password) @@ -387,7 +392,7 @@ impl<'a> Authenticate for DeviceWrapper<'a> { } } - fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { match self { DeviceWrapper::Storage(storage) => { authenticate_admin_wrapper(storage, DeviceWrapper::Storage, password) @@ -399,28 +404,28 @@ impl<'a> Authenticate for DeviceWrapper<'a> { } } -impl<'a> Authenticate for Pro<'a> { - fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { +impl<'a> Authenticate<'a> for Pro<'a> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) }) } - fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_first_authenticate(password_ptr, temp_password_ptr) }) } } -impl<'a> Authenticate for Storage<'a> { - fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { +impl<'a> Authenticate<'a> for Storage<'a> { + fn authenticate_user(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_user_authenticate(password_ptr, temp_password_ptr) }) } - fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { + fn authenticate_admin(self, password: &str) -> Result, (Self, Error)> { authenticate(self, password, |password_ptr, temp_password_ptr| unsafe { nitrokey_sys::NK_first_authenticate(password_ptr, temp_password_ptr) }) -- cgit v1.2.1 From 62e8ee8f5d02511d6eb5dc179b087b04e88c1b94 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Sun, 27 Jan 2019 19:52:53 +0000 Subject: Update documentation for Manager refactoring This patch updates the documentation to reflect the latest changes to connection handling. It also updates the doc tests to prefer the new methods over the old ones. --- src/auth.rs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 829d083..0b000f7 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -39,11 +39,12 @@ pub trait Authenticate<'a> { /// use nitrokey::{Authenticate, DeviceWrapper, User}; /// # use nitrokey::Error; /// - /// fn perform_user_task(device: &User) {} + /// fn perform_user_task<'a>(device: &User<'a, DeviceWrapper<'a>>) {} /// fn perform_other_task(device: &DeviceWrapper) {} /// /// # fn try_main() -> Result<(), Error> { - /// let device = nitrokey::connect()?; + /// let mut manager = nitrokey::take()?; + /// let device = manager.connect()?; /// let device = match device.authenticate_user("123456") { /// Ok(user) => { /// perform_user_task(&user); @@ -85,11 +86,12 @@ pub trait Authenticate<'a> { /// use nitrokey::{Authenticate, Admin, DeviceWrapper}; /// # use nitrokey::Error; /// - /// fn perform_admin_task(device: &Admin) {} + /// fn perform_admin_task<'a>(device: &Admin<'a, DeviceWrapper<'a>>) {} /// fn perform_other_task(device: &DeviceWrapper) {} /// /// # fn try_main() -> Result<(), Error> { - /// let device = nitrokey::connect()?; + /// let mut manager = nitrokey::take()?; + /// let device = manager.connect()?; /// let device = match device.authenticate_admin("123456") { /// Ok(admin) => { /// perform_admin_task(&admin); @@ -291,7 +293,8 @@ impl<'a, T: Device<'a>> Admin<'a, T> { /// # use nitrokey::Error; /// /// # fn try_main() -> Result<(), Error> { - /// let device = nitrokey::connect()?; + /// let mut manager = nitrokey::take()?; + /// let device = manager.connect()?; /// let config = Config::new(None, None, None, false); /// match device.authenticate_admin("12345678") { /// Ok(mut admin) => { -- cgit v1.2.1 From c35a20f6c034e4d8aa1eeba3eef85429e09d95dc Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Fri, 27 Dec 2019 23:07:56 +0100 Subject: Implement std::convert::TryFrom for RawConfig Previously, the RawConfig struct had a try_from function. As the TryFrom trait has been stabilized with Rust 1.34.0, we can use it instead. --- src/auth.rs | 1 + 1 file changed, 1 insertion(+) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 0b000f7..cab1021 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -1,6 +1,7 @@ // Copyright (C) 2018-2019 Robin Krahl // SPDX-License-Identifier: MIT +use std::convert::TryFrom as _; use std::marker; use std::ops; use std::os::raw::c_char; -- cgit v1.2.1