From ebd754d88330478981f65e4724cc561ceff4f9e7 Mon Sep 17 00:00:00 2001
From: Robin Krahl <robin.krahl@ireas.org>
Date: Tue, 28 Jan 2020 20:31:56 +0100
Subject: Regenerate temporary passwords with null bytes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously, we silently cut off temporary passwords that contained a
null byte.  With the change to CString, we returned a LibraryError
instead.  With this patch, we change to generate_password function to
continue generating passwords until we have a password without a null
byte.

The chance of generating a password with a null byte is ca. 10 % for our
temporary password with 25 characters.  Therefore the chance of having
to re-generate the password multiple times is low enough that we don’t
bother with re-generating only the null bytes of the password for the
time being.  This should be improved in the future.
---
 CHANGELOG.md |  1 +
 TODO.md      |  2 ++
 src/util.rs  | 12 +++++++++---
 3 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index be65865..5f03db2 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@ SPDX-License-Identifier: CC0-1.0
 
 # Unreleased
 - 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
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/util.rs b/src/util.rs
index b9b1a68..a0d0d1b 100644
--- a/src/util.rs
+++ b/src/util.rs
@@ -76,9 +76,15 @@ pub fn get_last_error() -> Error {
 }
 
 pub fn generate_password(length: usize) -> Result<CString, Error> {
-    let mut data = vec![0u8; length];
-    OsRng.fill_bytes(&mut data[..]);
-    get_cstring(data)
+    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> {
-- 
cgit v1.2.1