From 669fbb40d894460e9603dcf6e953373e53a19347 Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Tue, 28 Jan 2020 19:42:41 +0100 Subject: Use CString to store temporary passwords This patch changes the generate_password function and the User and Admin structs to use a CString instead of a Vec when storing temporary passwords. This makes sure that the strings that are passed to the C API are properly null-terminated. --- src/auth.rs | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index cab1021..571e198 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -2,6 +2,7 @@ // SPDX-License-Identifier: MIT use std::convert::TryFrom as _; +use std::ffi::CString; use std::marker; use std::ops; use std::os::raw::c_char; @@ -117,7 +118,7 @@ pub trait Authenticate<'a> { } trait AuthenticatedDevice { - fn new(device: T, temp_password: Vec) -> Self; + fn new(device: T, temp_password: CString) -> Self; fn temp_password_ptr(&self) -> *const c_char; } @@ -134,7 +135,7 @@ trait AuthenticatedDevice { #[derive(Debug)] pub struct User<'a, T: Device<'a>> { device: T, - temp_password: Vec, + temp_password: CString, marker: marker::PhantomData<&'a T>, } @@ -150,7 +151,7 @@ pub struct User<'a, T: Device<'a>> { #[derive(Debug)] pub struct Admin<'a, T: Device<'a>> { device: T, - temp_password: Vec, + temp_password: CString, marker: marker::PhantomData<&'a T>, } @@ -169,7 +170,7 @@ where Err(err) => return Err((device, err)), }; let password_ptr = password.as_ptr(); - let temp_password_ptr = temp_password.as_ptr() as *const c_char; + let temp_password_ptr = temp_password.as_ptr(); match callback(password_ptr, temp_password_ptr) { 0 => Ok(A::new(device, temp_password)), rv => Err((device, Error::from(rv))), @@ -246,7 +247,7 @@ impl<'a, T: Device<'a>> GenerateOtp for User<'a, T> { } impl<'a, T: Device<'a>> AuthenticatedDevice for User<'a, T> { - fn new(device: T, temp_password: Vec) -> Self { + fn new(device: T, temp_password: CString) -> Self { User { device, temp_password, @@ -255,7 +256,7 @@ impl<'a, T: Device<'a>> AuthenticatedDevice for User<'a, T> { } fn temp_password_ptr(&self) -> *const c_char { - self.temp_password.as_ptr() as *const c_char + self.temp_password.as_ptr() } } @@ -373,7 +374,7 @@ impl<'a, T: Device<'a>> ConfigureOtp for Admin<'a, T> { } impl<'a, T: Device<'a>> AuthenticatedDevice for Admin<'a, T> { - fn new(device: T, temp_password: Vec) -> Self { + fn new(device: T, temp_password: CString) -> Self { Admin { device, temp_password, @@ -382,7 +383,7 @@ impl<'a, T: Device<'a>> AuthenticatedDevice for Admin<'a, T> { } fn temp_password_ptr(&self) -> *const c_char { - self.temp_password.as_ptr() as *const c_char + self.temp_password.as_ptr() } } -- cgit v1.2.1 From 777cbd0fee8187325b0272d3264b535828d4b4ea Mon Sep 17 00:00:00 2001 From: Robin Krahl Date: Tue, 28 Jan 2020 19:47:39 +0100 Subject: Remove AuthenticatedDevice::temp_password_ptr We introduced the AuthenticatedDevice::temp_password_ptr function to reduce the number of casts needed in our code base. Since we switched from Vec to CString, we no longer have to cast the return value of as_ptr. Therefore we can remove the temp_password_ptr function to reduce code complexity. --- src/auth.rs | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) (limited to 'src/auth.rs') diff --git a/src/auth.rs b/src/auth.rs index 571e198..6748ca1 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -119,8 +119,6 @@ pub trait Authenticate<'a> { trait AuthenticatedDevice { fn new(device: T, temp_password: CString) -> Self; - - fn temp_password_ptr(&self) -> *const c_char; } /// A Nitrokey device with user authentication. @@ -235,13 +233,13 @@ impl<'a, T: Device<'a>> ops::DerefMut for User<'a, T> { 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()) + nitrokey_sys::NK_get_hotp_code_PIN(slot, self.temp_password.as_ptr()) }) } fn get_totp_code(&self, slot: u8) -> Result { result_from_string(unsafe { - nitrokey_sys::NK_get_totp_code_PIN(slot, 0, 0, 0, self.temp_password_ptr()) + nitrokey_sys::NK_get_totp_code_PIN(slot, 0, 0, 0, self.temp_password.as_ptr()) }) } } @@ -254,10 +252,6 @@ impl<'a, T: Device<'a>> AuthenticatedDevice for User<'a, T> { marker: marker::PhantomData, } } - - fn temp_password_ptr(&self) -> *const c_char { - self.temp_password.as_ptr() - } } impl<'a, T: Device<'a>> ops::Deref for Admin<'a, T> { @@ -319,7 +313,7 @@ impl<'a, T: Device<'a>> Admin<'a, T> { raw_config.scrollock, raw_config.user_password, false, - self.temp_password_ptr(), + self.temp_password.as_ptr(), ) }) } @@ -338,7 +332,7 @@ impl<'a, T: Device<'a>> ConfigureOtp for Admin<'a, T> { raw_data.use_enter, raw_data.use_token_id, raw_data.token_id.as_ptr(), - self.temp_password_ptr(), + self.temp_password.as_ptr(), ) }) } @@ -355,20 +349,20 @@ impl<'a, T: Device<'a>> ConfigureOtp for Admin<'a, T> { raw_data.use_enter, raw_data.use_token_id, raw_data.token_id.as_ptr(), - self.temp_password_ptr(), + self.temp_password.as_ptr(), ) }) } 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()) + nitrokey_sys::NK_erase_hotp_slot(slot, self.temp_password.as_ptr()) }) } 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()) + nitrokey_sys::NK_erase_totp_slot(slot, self.temp_password.as_ptr()) }) } } @@ -381,10 +375,6 @@ impl<'a, T: Device<'a>> AuthenticatedDevice for Admin<'a, T> { marker: marker::PhantomData, } } - - fn temp_password_ptr(&self) -> *const c_char { - self.temp_password.as_ptr() - } } impl<'a> Authenticate<'a> for DeviceWrapper<'a> { -- cgit v1.2.1