From d0baa23f9a15152a370092e955c2cf87891c26e7 Mon Sep 17 00:00:00 2001
From: Samuel Tardieu <sam@rfc1149.net>
Date: Fri, 11 Nov 2022 10:55:04 +0100
Subject: [PATCH] Use constant size generic parameter for random bytes
 generation

All uses of `get_random()` were in the form of:

  `&get_random(vec![0u8; SIZE])`

with `SIZE` being a constant.

Building a `Vec` is unnecessary for two reasons. First, it uses a
very short-lived dynamic memory allocation. Second, a `Vec` is a
resizable object, which is useless in those context when random
data have a fixed size and will only be read.

`get_random_bytes()` takes a constant as a generic parameter and
returns an array with the requested number of random bytes.

Stack safety analysis: the random bytes will be allocated on the
caller stack for a very short time (until the encoding function has
been called on the data). In some cases, the random bytes take
less room than the `Vec` did (a `Vec` is 24 bytes on a 64 bit
computer). The maximum used size is 180 bytes, which makes it
for 0.008% of the default stack size for a Rust thread (2MiB),
so this is a non-issue.

Also, most of the uses of those random bytes are to encode them
using an `Encoding`. The function `crypto::encode_random_bytes()`
generates random bytes and encode them with the provided
`Encoding`, leading to code deduplication.

`generate_id()` has also been converted to use a constant generic
parameter as well since the length of the requested String is always
a constant.
---
 src/api/core/two_factor/authenticator.rs |  2 +-
 src/api/core/two_factor/mod.rs           |  2 +-
 src/api/notifications.rs                 |  2 +-
 src/crypto.rs                            | 21 ++++++++++++++-------
 src/db/models/device.rs                  |  4 ++--
 5 files changed, 19 insertions(+), 12 deletions(-)

diff --git a/src/api/core/two_factor/authenticator.rs b/src/api/core/two_factor/authenticator.rs
index 7373b581..68342e95 100644
--- a/src/api/core/two_factor/authenticator.rs
+++ b/src/api/core/two_factor/authenticator.rs
@@ -34,7 +34,7 @@ async fn generate_authenticator(data: JsonUpcase<PasswordData>, headers: Headers
 
     let (enabled, key) = match twofactor {
         Some(tf) => (true, tf.data),
-        _ => (false, BASE32.encode(&crypto::get_random(vec![0u8; 20]))),
+        _ => (false, crypto::encode_random_bytes::<20>(BASE32)),
     };
 
     Ok(Json(json!({
diff --git a/src/api/core/two_factor/mod.rs b/src/api/core/two_factor/mod.rs
index 78f1318e..3d5eee83 100644
--- a/src/api/core/two_factor/mod.rs
+++ b/src/api/core/two_factor/mod.rs
@@ -105,7 +105,7 @@ async fn recover(data: JsonUpcase<RecoverTwoFactor>, mut conn: DbConn) -> JsonRe
 
 async fn _generate_recover_code(user: &mut User, conn: &mut DbConn) {
     if user.totp_recover.is_none() {
-        let totp_recover = BASE32.encode(&crypto::get_random(vec![0u8; 20]));
+        let totp_recover = crypto::encode_random_bytes::<20>(BASE32);
         user.totp_recover = Some(totp_recover);
         user.save(conn).await.ok();
     }
diff --git a/src/api/notifications.rs b/src/api/notifications.rs
index 2657d312..cd53c96d 100644
--- a/src/api/notifications.rs
+++ b/src/api/notifications.rs
@@ -56,7 +56,7 @@ fn negotiate(_headers: Headers) -> Json<JsonValue> {
     use crate::crypto;
     use data_encoding::BASE64URL;
 
-    let conn_id = BASE64URL.encode(&crypto::get_random(vec![0u8; 16]));
+    let conn_id = crypto::encode_random_bytes::<16>(BASE64URL);
     let mut available_transports: Vec<JsonValue> = Vec::new();
 
     if CONFIG.websocket_enabled() {
diff --git a/src/crypto.rs b/src/crypto.rs
index be9680cb..daf5124d 100644
--- a/src/crypto.rs
+++ b/src/crypto.rs
@@ -3,7 +3,7 @@
 //
 use std::num::NonZeroU32;
 
-use data_encoding::HEXLOWER;
+use data_encoding::{Encoding, HEXLOWER};
 use ring::{digest, hmac, pbkdf2};
 
 static DIGEST_ALG: pbkdf2::Algorithm = pbkdf2::PBKDF2_HMAC_SHA256;
@@ -38,17 +38,24 @@ pub fn hmac_sign(key: &str, data: &str) -> String {
 //
 
 pub fn get_random_64() -> Vec<u8> {
-    get_random(vec![0u8; 64])
+    get_random_bytes::<64>().to_vec()
 }
 
-pub fn get_random(mut array: Vec<u8>) -> Vec<u8> {
+/// Return an array holding `N` random bytes.
+pub fn get_random_bytes<const N: usize>() -> [u8; N] {
     use ring::rand::{SecureRandom, SystemRandom};
 
+    let mut array = [0; N];
     SystemRandom::new().fill(&mut array).expect("Error generating random values");
 
     array
 }
 
+/// Encode random bytes using the provided function.
+pub fn encode_random_bytes<const N: usize>(e: Encoding) -> String {
+    e.encode(&get_random_bytes::<N>())
+}
+
 /// Generates a random string over a specified alphabet.
 pub fn get_random_string(alphabet: &[u8], num_chars: usize) -> String {
     // Ref: https://rust-lang-nursery.github.io/rust-cookbook/algorithms/randomness.html
@@ -77,18 +84,18 @@ pub fn get_random_string_alphanum(num_chars: usize) -> String {
     get_random_string(ALPHABET, num_chars)
 }
 
-pub fn generate_id(num_bytes: usize) -> String {
-    HEXLOWER.encode(&get_random(vec![0; num_bytes]))
+pub fn generate_id<const N: usize>() -> String {
+    encode_random_bytes::<N>(HEXLOWER)
 }
 
 pub fn generate_send_id() -> String {
     // Send IDs are globally scoped, so make them longer to avoid collisions.
-    generate_id(32) // 256 bits
+    generate_id::<32>() // 256 bits
 }
 
 pub fn generate_attachment_id() -> String {
     // Attachment IDs are scoped to a cipher, so they can be smaller.
-    generate_id(10) // 80 bits
+    generate_id::<10>() // 80 bits
 }
 
 /// Generates a numeric token for email-based verifications.
diff --git a/src/db/models/device.rs b/src/db/models/device.rs
index e8a933cb..e47ccadc 100644
--- a/src/db/models/device.rs
+++ b/src/db/models/device.rs
@@ -48,7 +48,7 @@ impl Device {
         use crate::crypto;
         use data_encoding::BASE64;
 
-        let twofactor_remember = BASE64.encode(&crypto::get_random(vec![0u8; 180]));
+        let twofactor_remember = crypto::encode_random_bytes::<180>(BASE64);
         self.twofactor_remember = Some(twofactor_remember.clone());
 
         twofactor_remember
@@ -69,7 +69,7 @@ impl Device {
             use crate::crypto;
             use data_encoding::BASE64URL;
 
-            self.refresh_token = BASE64URL.encode(&crypto::get_random_64());
+            self.refresh_token = crypto::encode_random_bytes::<64>(BASE64URL);
         }
 
         // Update the expiration of the device and the last update date