From 5448cdc565f5f5f0fc964dbfc2696e3184323c82 Mon Sep 17 00:00:00 2001 From: Karolin Varner Date: Thu, 30 Nov 2023 18:23:59 +0100 Subject: [PATCH] feat: Use the rand crate for random values instead of sodium --- Cargo.lock | 38 +++++++++++++++++++++++++++++++++++++ Cargo.toml | 1 + rosenpass/Cargo.toml | 1 + rosenpass/src/protocol.rs | 2 +- secret-memory/Cargo.toml | 1 + secret-memory/src/lib.rs | 1 + secret-memory/src/public.rs | 9 ++++++++- secret-memory/src/rand.rs | 5 +++++ secret-memory/src/secret.rs | 19 ++++++++++++++----- sodium/src/helpers.rs | 19 ------------------- 10 files changed, 70 insertions(+), 26 deletions(-) create mode 100644 secret-memory/src/rand.rs diff --git a/Cargo.lock b/Cargo.lock index 9b3cab1..cc5f63d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -947,6 +947,12 @@ dependencies = [ "plotters-backend", ] +[[package]] +name = "ppv-lite86" +version = "0.2.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" + [[package]] name = "prettyplease" version = "0.2.15" @@ -984,6 +990,36 @@ dependencies = [ "proc-macro2", ] +[[package]] +name = "rand" +version = "0.8.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "34af8d1a0e25924bc5b7c43c079c942339d8f0a8b57c39049bef581b46327404" +dependencies = [ + "libc", + "rand_chacha", + "rand_core", +] + +[[package]] +name = "rand_chacha" +version = "0.3.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e6c10a63a0fa32252be49d21e7709d4d4baf8d231c2dbce1eaa8141b9b127d88" +dependencies = [ + "ppv-lite86", + "rand_core", +] + +[[package]] +name = "rand_core" +version = "0.6.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ec0be4795e2f6a28069bec0b5ff3e2ac9bafc99e6a9a7dc3547996c5c816922c" +dependencies = [ + "getrandom", +] + [[package]] name = "rayon" version = "1.8.0" @@ -1076,6 +1112,7 @@ dependencies = [ "mio", "oqs-sys", "paste", + "rand", "rosenpass-ciphers", "rosenpass-constant-time", "rosenpass-secret-memory", @@ -1130,6 +1167,7 @@ dependencies = [ "anyhow", "lazy_static", "libsodium-sys-stable", + "rand", "rosenpass-sodium", "rosenpass-to", "rosenpass-util", diff --git a/Cargo.toml b/Cargo.toml index e336960..485cbbf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,6 +43,7 @@ env_logger = "0.10.0" toml = "0.7.4" static_assertions = "1.1.0" allocator-api2 = "0.2.16" +rand = "0.8.5" log = { version = "0.4.17" } clap = { version = "4.3.0", features = ["derive"] } serde = { version = "1.0.163", features = ["derive"] } diff --git a/rosenpass/Cargo.toml b/rosenpass/Cargo.toml index f8a0591..65f43c5 100644 --- a/rosenpass/Cargo.toml +++ b/rosenpass/Cargo.toml @@ -33,6 +33,7 @@ serde = { workspace = true } toml = { workspace = true } clap = { workspace = true } mio = { workspace = true } +rand = { workspace = true } [build-dependencies] anyhow = { workspace = true } diff --git a/rosenpass/src/protocol.rs b/rosenpass/src/protocol.rs index 5f50b01..ce86b1c 100644 --- a/rosenpass/src/protocol.rs +++ b/rosenpass/src/protocol.rs @@ -1150,7 +1150,7 @@ impl IniHsPtr { .min(ih.tx_count as f64), ) * RETRANSMIT_DELAY_JITTER - * (rosenpass_sodium::helpers::rand_f64() + 1.0); // TODO: Replace with the rand crate + * (rand::random::() + 1.0); // TODO: Replace with the rand crate ih.tx_count += 1; Ok(()) } diff --git a/secret-memory/Cargo.toml b/secret-memory/Cargo.toml index c102599..45b5169 100644 --- a/secret-memory/Cargo.toml +++ b/secret-memory/Cargo.toml @@ -17,3 +17,4 @@ rosenpass-util = { workspace = true } libsodium-sys-stable = { workspace = true } lazy_static = { workspace = true } zeroize = { workspace = true } +rand = { workspace = true } diff --git a/secret-memory/src/lib.rs b/secret-memory/src/lib.rs index 3e57f54..75a6e72 100644 --- a/secret-memory/src/lib.rs +++ b/secret-memory/src/lib.rs @@ -1,5 +1,6 @@ pub mod debug; pub mod file; +pub mod rand; mod public; pub use crate::public::Public; diff --git a/secret-memory/src/public.rs b/secret-memory/src/public.rs index a27b4e2..39a43b8 100644 --- a/secret-memory/src/public.rs +++ b/secret-memory/src/public.rs @@ -1,4 +1,5 @@ use crate::debug::debug_crypto_array; +use rand::{Fill as Randomize, Rng}; use rosenpass_to::{ops::copy_slice, To}; use rosenpass_util::file::{fopen_r, LoadValue, ReadExactToEnd, StoreValue}; use rosenpass_util::functional::mutating; @@ -39,7 +40,13 @@ impl Public { /// Randomize all bytes in an existing [Public] pub fn randomize(&mut self) { - rosenpass_sodium::helpers::randombytes_buf(&mut self.value); + self.try_fill(&mut crate::rand::rng()).unwrap() + } +} + +impl Randomize for Public { + fn try_fill(&mut self, rng: &mut R) -> Result<(), rand::Error> { + self.value.try_fill(rng) } } diff --git a/secret-memory/src/rand.rs b/secret-memory/src/rand.rs new file mode 100644 index 0000000..a6daa6d --- /dev/null +++ b/secret-memory/src/rand.rs @@ -0,0 +1,5 @@ +pub type Rng = rand::rngs::ThreadRng; + +pub fn rng() -> Rng { + rand::thread_rng() +} diff --git a/secret-memory/src/secret.rs b/secret-memory/src/secret.rs index 1bbeea5..54cc5a4 100644 --- a/secret-memory/src/secret.rs +++ b/secret-memory/src/secret.rs @@ -1,16 +1,15 @@ +use crate::file::StoreSecret; use anyhow::Context; use lazy_static::lazy_static; +use rand::{Fill as Randomize, Rng}; use rosenpass_sodium::alloc::{Alloc as SodiumAlloc, Box as SodiumBox, Vec as SodiumVec}; use rosenpass_util::{ b64::b64_reader, file::{fopen_r, LoadValue, LoadValueB64, ReadExactToEnd}, functional::mutating, }; -use zeroize::{Zeroize, ZeroizeOnDrop}; - use std::{collections::HashMap, convert::TryInto, fmt, path::Path, sync::Mutex}; - -use crate::file::StoreSecret; +use zeroize::{Zeroize, ZeroizeOnDrop}; // This might become a problem in library usage; it's effectively a memory // leak which probably isn't a problem right now because most memory will @@ -94,7 +93,7 @@ impl Secret { /// Sets all data an existing secret to random bytes pub fn randomize(&mut self) { - rosenpass_sodium::helpers::randombytes_buf(self.secret_mut()); + self.try_fill(&mut crate::rand::rng()).unwrap() } /// Borrows the data @@ -115,6 +114,16 @@ impl Zeroize for Secret { } } +impl Randomize for Secret { + fn try_fill(&mut self, rng: &mut R) -> Result<(), rand::Error> { + // Zeroize self first just to make sure the barriers from the zeroize create take + // effect to prevent the compiler from optimizing this away. + // We should at some point replace this with our own barriers. + self.zeroize(); + self.secret_mut().try_fill(rng) + } +} + impl Drop for Secret { fn drop(&mut self) { self.storage diff --git a/sodium/src/helpers.rs b/sodium/src/helpers.rs index 26e0bf5..a64f95b 100644 --- a/sodium/src/helpers.rs +++ b/sodium/src/helpers.rs @@ -26,22 +26,3 @@ pub fn increment(v: &mut [u8]) { libsodium::sodium_increment(v.as_mut_ptr(), v.len()); } } - -#[inline] -pub fn randombytes_buf(buf: &mut [u8]) { - unsafe { libsodium::randombytes_buf(buf.as_mut_ptr() as *mut c_void, buf.len()) }; -} - -// Choose a fully random u64 -// TODO: Replace with ::rand::random -pub fn rand_u64() -> u64 { - let mut buf = [0u8; 8]; - randombytes_buf(&mut buf); - u64::from_le_bytes(buf) -} - -// Choose a random f64 in [0; 1] inclusive; quick and dirty -// TODO: Replace with ::rand::random -pub fn rand_f64() -> f64 { - (rand_u64() as f64) / (u64::MAX as f64) -}