Merge branch 'hotfix-0.5.2'

5 files changed, 31 insertions, 28 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cba0e83..b75904c 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,10 @@ Copyright (C) 2019-2020 Robin Krahl <robin.krahl@ireas.org>
 SPDX-License-Identifier: CC0-1.0
 -->
 
+# 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.
diff --git a/Cargo.toml b/Cargo.toml
index 59af067..e6e8ec7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -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/"
diff --git a/TODO.md b/TODO.md
index 92d4b04..e50d354 100644
--- a/TODO.md
+++ b/TODO.md
@@ -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 5a56c55..a0d0d1b 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -75,10 +75,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> {