diff options
| -rw-r--r-- | CHANGELOG.md | 4 | ||||
| -rw-r--r-- | Cargo.toml | 2 | ||||
| -rw-r--r-- | TODO.md | 2 | ||||
| -rw-r--r-- | src/auth.rs | 37 | ||||
| -rw-r--r-- | src/util.rs | 14 |
5 files changed, 31 insertions, 28 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c0f6b4..f5bb7cb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ SPDX-License-Identifier: CC0-1.0 - Change the type of the field `DeviceInfo.serial_number` from `Option<String>` to `Option<u32>`. +# v0.5.2 (2020-01-28) +- Use `CString` to store the temporary password instead of `Vec<u8>`. +- Regenerate temporary passwords if they would contain a null byte. + # v0.5.1 (2020-01-15) - Fix serial number formatting for Nitrokey Pro devices with firmware 0.8 or older in the `list_devices` function. @@ -3,7 +3,7 @@ [package] name = "nitrokey" -version = "0.5.1" +version = "0.5.2" authors = ["Robin Krahl <robin.krahl@ireas.org>"] edition = "2018" homepage = "https://code.ireas.org/nitrokey-rs/" @@ -6,5 +6,7 @@ SPDX-License-Identifier: CC0-1.0 - Clear passwords from memory. - Lock password safe in `PasswordSafe::drop()` (see [nitrokey-storage-firmware issue 65][]). +- Consider only regenerating the null bytes instead of the complete password in + `util::generate_password`. [nitrokey-storage-firmware issue 65]: https://github.com/Nitrokey/nitrokey-storage-firmware/issues/65 diff --git a/src/auth.rs b/src/auth.rs index cab1021..6748ca1 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,9 +118,7 @@ pub trait Authenticate<'a> { } trait AuthenticatedDevice<T> { - fn new(device: T, temp_password: Vec<u8>) -> Self; - - fn temp_password_ptr(&self) -> *const c_char; + fn new(device: T, temp_password: CString) -> Self; } /// A Nitrokey device with user authentication. @@ -134,7 +133,7 @@ trait AuthenticatedDevice<T> { #[derive(Debug)] pub struct User<'a, T: Device<'a>> { device: T, - temp_password: Vec<u8>, + temp_password: CString, marker: marker::PhantomData<&'a T>, } @@ -150,7 +149,7 @@ pub struct User<'a, T: Device<'a>> { #[derive(Debug)] pub struct Admin<'a, T: Device<'a>> { device: T, - temp_password: Vec<u8>, + temp_password: CString, marker: marker::PhantomData<&'a T>, } @@ -169,7 +168,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))), @@ -234,29 +233,25 @@ 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<String, Error> { 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<String, Error> { 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()) }) } } impl<'a, T: Device<'a>> AuthenticatedDevice<T> for User<'a, T> { - fn new(device: T, temp_password: Vec<u8>) -> Self { + fn new(device: T, temp_password: CString) -> Self { User { device, temp_password, marker: marker::PhantomData, } } - - fn temp_password_ptr(&self) -> *const c_char { - self.temp_password.as_ptr() as *const c_char - } } impl<'a, T: Device<'a>> ops::Deref for Admin<'a, T> { @@ -318,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(), ) }) } @@ -337,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(), ) }) } @@ -354,36 +349,32 @@ 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()) }) } } impl<'a, T: Device<'a>> AuthenticatedDevice<T> for Admin<'a, T> { - fn new(device: T, temp_password: Vec<u8>) -> Self { + fn new(device: T, temp_password: CString) -> Self { Admin { device, temp_password, marker: marker::PhantomData, } } - - fn temp_password_ptr(&self) -> *const c_char { - self.temp_password.as_ptr() as *const c_char - } } impl<'a> Authenticate<'a> for DeviceWrapper<'a> { diff --git a/src/util.rs b/src/util.rs index 1b52c3d..08946d6 100644 --- a/src/util.rs +++ b/src/util.rs @@ -76,10 +76,16 @@ pub fn get_last_error() -> Error { }) } -pub fn generate_password(length: usize) -> Result<Vec<u8>, Error> { - let mut data = vec![0u8; length]; - OsRng.fill_bytes(&mut data[..]); - Ok(data) +pub fn generate_password(length: usize) -> Result<CString, Error> { + loop { + // Randomly generate a password until we get a string *without* null bytes. Otherwise + // the string would be cut off prematurely due to null-termination in C. + let mut data = vec![0u8; length]; + OsRng.fill_bytes(&mut data[..]); + if let Ok(s) = CString::new(data) { + return Ok(s); + } + } } pub fn get_cstring<T: Into<Vec<u8>>>(s: T) -> Result<CString, Error> { |