From 61ef5b92bbe15f50d1eb344e23cc9795cb666af5 Mon Sep 17 00:00:00 2001 From: user Date: Tue, 5 Dec 2023 14:50:13 -0500 Subject: [PATCH 01/13] fix: add deprecated keygen command This allows users to use the old keygen command, while being informed about its deprecation. --- rosenpass/src/cli.rs | 59 ++++++++++++++++++++++++++++++++++++++----- rosenpass/src/main.rs | 3 ++- 2 files changed, 55 insertions(+), 7 deletions(-) diff --git a/rosenpass/src/cli.rs b/rosenpass/src/cli.rs index dc6b5964..86cee92b 100644 --- a/rosenpass/src/cli.rs +++ b/rosenpass/src/cli.rs @@ -87,6 +87,15 @@ pub enum Cli { force: bool, }, + /// Deprecated - use gen-keys instead + #[allow(rustdoc::broken_intra_doc_links)] + #[allow(rustdoc::invalid_html_tags)] + Keygen { + // NOTE yes, the legacy keygen argument initially really accepted "privet-key", not "secret-key"! + /// public-key private-key + args: Vec, + }, + /// Validate a configuration Validate { config_files: Vec }, @@ -119,6 +128,40 @@ impl Cli { config::Rosenpass::example_config().store(config_file)?; } + // Deprecated - use gen-keys instead + Keygen { args } => { + log::warn!("The 'keygen' command is deprecated. Please use the 'gen-keys' command instead."); + + let mut public_key: Option = None; + let mut secret_key: Option = None; + + // Manual arg parsing, since clap wants to prefix flags with "--" + let mut args = args.into_iter(); + loop { + match (args.next().as_ref().map(String::as_str), args.next()) { + (Some("private-key"), Some(opt)) | (Some("secret-key"), Some(opt)) => { + secret_key = Some(opt.into()); + } + (Some("public-key"), Some(opt)) => { + public_key = Some(opt.into()); + } + (Some(flag), _) => { + bail!("Unknown option `{}`", flag); + } + (_, _) => break, + }; + } + + if secret_key.is_none() { + bail!("private-key is required"); + } + if public_key.is_none() { + bail!("public-key is required"); + } + + generate_and_save_keypair(secret_key.unwrap(), public_key.unwrap())?; + } + GenKeys { config_file, public_key, @@ -160,12 +203,7 @@ impl Cli { } // generate the keys and store them in files - let mut ssk = crate::protocol::SSk::random(); - let mut spk = crate::protocol::SPk::random(); - StaticKem::keygen(ssk.secret_mut(), spk.secret_mut())?; - - ssk.store_secret(skf)?; - spk.store_secret(pkf)?; + generate_and_save_keypair(skf, pkf)?; } ExchangeConfig { config_file } => { @@ -246,3 +284,12 @@ impl Cli { srv.event_loop() } } + +/// generate secret and public keys, store in files according to the paths passed as arguments +fn generate_and_save_keypair(secret_key: PathBuf, public_key: PathBuf) -> anyhow::Result<()> { + let mut ssk = crate::protocol::SSk::random(); + let mut spk = crate::protocol::SPk::random(); + StaticKem::keygen(ssk.secret_mut(), spk.secret_mut())?; + ssk.store_secret(secret_key)?; + spk.store_secret(public_key) +} diff --git a/rosenpass/src/main.rs b/rosenpass/src/main.rs index c6c9d84d..e4db49eb 100644 --- a/rosenpass/src/main.rs +++ b/rosenpass/src/main.rs @@ -5,7 +5,8 @@ use std::process::exit; /// Catches errors, prints them through the logger, then exits pub fn main() { - env_logger::init(); + // default to displaying warning and error log messages only + env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("warn")).init(); let res = attempt!({ rosenpass_sodium::init()?; From eb76179dc4f6ea5ee3780a538ddf25b6ab6e8183 Mon Sep 17 00:00:00 2001 From: alankritdabral_2 <149957793+AlooDon@users.noreply.github.com> Date: Mon, 20 Nov 2023 18:48:26 +0530 Subject: [PATCH 02/13] feat: add format_rustcode.sh script This script makes it possible to check formatting of rust code found in the various markdown files in the repo. It is also added as a job to the QC CI workflow. --- .github/workflows/qc.yaml | 8 +++ format_rust_code.sh | 115 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+) create mode 100755 format_rust_code.sh diff --git a/.github/workflows/qc.yaml b/.github/workflows/qc.yaml index 173c0d5a..eda617a1 100644 --- a/.github/workflows/qc.yaml +++ b/.github/workflows/qc.yaml @@ -25,6 +25,14 @@ jobs: - name: Run ShellCheck uses: ludeeus/action-shellcheck@master + rustfmt: + name: Rust Format + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - name: Run Rust Formatting Script + run: bash format_rust_code.sh --mode check + cargo-audit: runs-on: ubuntu-latest steps: diff --git a/format_rust_code.sh b/format_rust_code.sh new file mode 100755 index 00000000..b5cc10f1 --- /dev/null +++ b/format_rust_code.sh @@ -0,0 +1,115 @@ +#!/usr/bin/env bash + +# Parse command line options +while [[ $# -gt 0 ]]; do + case "$1" in + --mode) + mode="$2" + shift 2 + ;; + *) + echo "Unknown option: $1" + exit 1 + ;; + esac +done + +# Check if mode is specified +if [ -z "$mode" ]; then + echo "Please specify the mode using --mode option. Valid modes are 'check' and 'fix'." + exit 1 +fi + +# Find all Markdown files in the current directory and its subdirectories +mapfile -t md_files < <(find . -type f -name "*.md") + +count=0 +# Iterate through each Markdown file +for file in "${md_files[@]}"; do + # Use awk to extract Rust code blocks enclosed within triple backticks + rust_code_blocks=$(awk '/```rust/{flag=1; next}/```/{flag=0} flag' "$file") + + # Count the number of Rust code blocks + num_fences=$(awk '/```rust/{f=1} f{if(/```/){f=0; count++}} END{print count}' "$file") + + if [ -n "$rust_code_blocks" ]; then + echo "Processing Rust code in $file" + # Iterate through each Rust code block + for ((i=1; i <= num_fences ; i++)); do + # Extract individual Rust code block using awk + current_rust_block=$(awk -v i="$i" '/```rust/{f=1; if (++count == i) next} f&&/```/{f=0;next} f' "$file") + # Variable to check if we have added the main function + add_main=0 + # Check if the Rust code block is already inside a function + if ! echo "$current_rust_block" | grep -q "fn main()"; then + # If not, wrap it in a main function + current_rust_block=$'fn main() {\n'"$current_rust_block"$'\n}' + add_main=1 + fi + if [ "$mode" == "check" ]; then + # Apply changes to the Rust code block + formatted_rust_code=$(echo "$current_rust_block" | rustfmt) + # Use rustfmt to format the Rust code block, remove first and last lines, and remove the first 4 spaces if added main function + if [ "$add_main" == 1 ]; then + formatted_rust_code=$(echo "$formatted_rust_code" | sed '1d;$d' | sed 's/^ //') + current_rust_block=$(echo "$current_rust_block" | sed '1d;') + current_rust_block=$(echo "$current_rust_block" | sed '$d') + fi + if [ "$formatted_rust_code" == "$current_rust_block" ]; then + echo "No changes needed in Rust code block $i in $file" + else + echo -e "\nChanges needed in Rust code block $i in $file:\n" + echo "$formatted_rust_code" + count=+1 + fi + + elif [ "$mode" == "fix" ]; then + # Replace current_rust_block with formatted_rust_code in the file + formatted_rust_code=$(echo "$current_rust_block" | rustfmt) + # Use rustfmt to format the Rust code block, remove first and last lines, and remove the first 4 spaces if added main function + if [ "$add_main" == 1 ]; then + formatted_rust_code=$(echo "$formatted_rust_code" | sed '1d;$d' | sed 's/^ //') + current_rust_block=$(echo "$current_rust_block" | sed '1d;') + current_rust_block=$(echo "$current_rust_block" | sed '$d') + fi + # Check if the formatted code is the same as the current Rust code block + if [ "$formatted_rust_code" == "$current_rust_block" ]; then + echo "No changes needed in Rust code block $i in $file" + else + echo "Formatting Rust code block $i in $file" + # Replace current_rust_block with formatted_rust_code in the file + # Use awk to find the line number of the pattern + + start_line=$(grep -n "^\`\`\`rust" "$file" | sed -n "${i}p" | cut -d: -f1) + end_line=$(grep -n "^\`\`\`" "$file" | awk -F: -v start_line="$start_line" '$1 > start_line {print $1; exit;}') + + if [ -n "$start_line" ] && [ -n "$end_line" ]; then + # Print lines before the Rust code block + head -n "$((start_line - 1))" "$file" + + # Print the formatted Rust code block + echo "\`\`\`rust" + echo "$formatted_rust_code" + echo "\`\`\`" + + # Print lines after the Rust code block + tail -n +"$((end_line + 1))" "$file" + else + # Rust code block not found or end line not found + cat "$file" + fi > tmpfile && mv tmpfile "$file" + + fi + else + echo "Unknown mode: $mode. Valid modes are 'check' and 'fix'." + exit 1 + fi + done + fi +done + +# CI failure if changes are needed +if [ $count -gt 0 ]; then + echo "CI failed: Changes needed in Rust code blocks." + exit 1 +fi From 7c83e244f988646a96c764b09f16ed3f4e53f32d Mon Sep 17 00:00:00 2001 From: wucke13 Date: Fri, 22 Dec 2023 17:45:39 +0100 Subject: [PATCH 03/13] fix: fix Rust code in markdown files This applies the novel format_rustcode.sh script to the markdown files in the repo, to maintain a consistent style across code examples. --- to/README.md | 109 +++++++++++++++++++++++++++++++++------------------ 1 file changed, 71 insertions(+), 38 deletions(-) diff --git a/to/README.md b/to/README.md index b8e01469..5c34874f 100644 --- a/to/README.md +++ b/to/README.md @@ -12,15 +12,17 @@ The crate provides chained functions to simplify allocating the destination para For now this crate is experimental; patch releases are guaranteed not to contain any breaking changes, but minor releases may. ```rust -use std::ops::BitXorAssign; -use rosenpass_to::{To, to, with_destination}; use rosenpass_to::ops::copy_array; +use rosenpass_to::{to, with_destination, To}; +use std::ops::BitXorAssign; // Destination functions return some value that implements the To trait. // Unfortunately dealing with lifetimes is a bit more finicky than it would# // be without destination parameters -fn xor_slice<'a, T>(src: &'a[T]) -> impl To<[T], ()> + 'a - where T: BitXorAssign + Clone { +fn xor_slice<'a, T>(src: &'a [T]) -> impl To<[T], ()> + 'a +where + T: BitXorAssign + Clone, +{ // Custom implementations of the to trait can be created, but the easiest with_destination(move |dst: &mut [T]| { assert!(src.len() == dst.len()); @@ -65,7 +67,7 @@ assert_eq!(&dst[..], &flip01[..]); // The builtin function copy_array supports to_value() since its // destination parameter is a fixed size array, which can be allocated // using default() -let dst : [u8; 4] = copy_array(flip01).to_value(); +let dst: [u8; 4] = copy_array(flip01).to_value(); assert_eq!(&dst, flip01); ``` @@ -84,7 +86,9 @@ Functions declared like this are more cumbersome to use when the destination par use std::ops::BitXorAssign; fn xor_slice(dst: &mut [T], src: &[T]) - where T: BitXorAssign + Clone { +where + T: BitXorAssign + Clone, +{ assert!(src.len() == dst.len()); for (d, s) in dst.iter_mut().zip(src.iter()) { *d ^= s.clone(); @@ -114,8 +118,8 @@ assert_eq!(&dst[..], &flip01[..]); There are a couple of ways to use a function with destination: ```rust -use rosenpass_to::{to, To}; use rosenpass_to::ops::{copy_array, copy_slice_least}; +use rosenpass_to::{to, To}; let mut dst = b" ".to_vec(); @@ -129,7 +133,8 @@ copy_slice_least(b"This is fin").to(&mut dst[..]); assert_eq!(&dst[..], b"This is fin"); // You can allocate the destination variable on the fly using `.to_this(...)` -let tmp = copy_slice_least(b"This is new---").to_this(|| b"This will be overwritten".to_owned()); +let tmp = + copy_slice_least(b"This is new---").to_this(|| b"This will be overwritten".to_owned()); assert_eq!(&tmp[..], b"This is new---verwritten"); // You can allocate the destination variable on the fly `.collect(..)` if it implements default @@ -147,8 +152,11 @@ assert_eq!(&tmp[..], b"Fixed"); The to crate provides basic functions with destination for copying data between slices and arrays. ```rust +use rosenpass_to::ops::{ + copy_array, copy_slice, copy_slice_least, copy_slice_least_src, try_copy_slice, + try_copy_slice_least_src, +}; use rosenpass_to::{to, To}; -use rosenpass_to::ops::{copy_array, copy_slice, copy_slice_least, copy_slice_least_src, try_copy_slice, try_copy_slice_least_src}; let mut dst = b" ".to_vec(); @@ -161,18 +169,33 @@ to(&mut dst[4..], copy_slice_least_src(b"!!!")); assert_eq!(&dst[..], b"Hell!!!orld"); // Copy a slice, copying as many bytes as possible -to(&mut dst[6..], copy_slice_least(b"xxxxxxxxxxxxxxxxxxxxxxxxxxxxx")); +to( + &mut dst[6..], + copy_slice_least(b"xxxxxxxxxxxxxxxxxxxxxxxxxxxxx"), +); assert_eq!(&dst[..], b"Hell!!xxxxx"); // Copy a slice, will return None and abort if the sizes do not much assert_eq!(Some(()), to(&mut dst[..], try_copy_slice(b"Hello World"))); assert_eq!(None, to(&mut dst[..], try_copy_slice(b"---"))); -assert_eq!(None, to(&mut dst[..], try_copy_slice(b"---------------------"))); +assert_eq!( + None, + to(&mut dst[..], try_copy_slice(b"---------------------")) +); assert_eq!(&dst[..], b"Hello World"); // Copy a slice, will return None and abort if source is longer than destination -assert_eq!(Some(()), to(&mut dst[4..], try_copy_slice_least_src(b"!!!"))); -assert_eq!(None, to(&mut dst[4..], try_copy_slice_least_src(b"-------------------------"))); +assert_eq!( + Some(()), + to(&mut dst[4..], try_copy_slice_least_src(b"!!!")) +); +assert_eq!( + None, + to( + &mut dst[4..], + try_copy_slice_least_src(b"-------------------------") + ) +); assert_eq!(&dst[..], b"Hell!!!orld"); // Copy fixed size arrays all at once @@ -186,12 +209,14 @@ assert_eq!(&dst, b"Hello"); The easiest way to declare a function with destination is to use the with_destination function. ```rust -use rosenpass_to::{To, to, with_destination}; use rosenpass_to::ops::copy_array; +use rosenpass_to::{to, with_destination, To}; /// Copy the given slice to the start of a vector, reusing its memory if possible fn copy_to_vec<'a, T>(src: &'a [T]) -> impl To, ()> + 'a - where T: Clone { +where + T: Clone, +{ with_destination(move |dst: &mut Vec| { dst.clear(); dst.extend_from_slice(src); @@ -217,7 +242,9 @@ The same pattern can be implemented without `to`, at the cost of being slightly ```rust /// Copy the given slice to the start of a vector, reusing its memory if possible fn copy_to_vec(dst: &mut Vec, src: &[T]) - where T: Clone { +where + T: Clone, +{ dst.clear(); dst.extend_from_slice(src); } @@ -240,11 +267,11 @@ Alternative functions are returned, that return a `to::Beside` value, containing destination variable and the return value. ```rust -use std::cmp::{min, max}; -use rosenpass_to::{To, to, with_destination, Beside}; +use rosenpass_to::{to, with_destination, Beside, To}; +use std::cmp::{max, min}; /// Copy an array of floats and calculate the average -pub fn copy_and_average<'a>(src: &'a[f64]) -> impl To<[f64], f64> + 'a { +pub fn copy_and_average<'a>(src: &'a [f64]) -> impl To<[f64], f64> + 'a { with_destination(move |dst: &mut [f64]| { assert!(src.len() == dst.len()); let mut sum = 0f64; @@ -300,8 +327,8 @@ assert_eq!(tmp, Beside([42f64; 3], 42f64)); When Beside values contain a `()`, `Option<()>`, or `Result<(), Error>` return value, they expose a special method called `.condense()`; this method consumes the Beside value and condenses destination and return value into one value. ```rust +use rosenpass_to::Beside; use std::result::Result; -use rosenpass_to::{Beside}; assert_eq!((), Beside((), ()).condense()); @@ -318,8 +345,8 @@ assert_eq!(Err(()), Beside(42, err_unit).condense()); When condense is implemented for a type, `.to_this(|| ...)`, `.to_value()`, and `.collect::<...>()` on the `To` trait can be used even with a return value: ```rust +use rosenpass_to::ops::try_copy_slice; use rosenpass_to::To; -use rosenpass_to::ops::try_copy_slice;; let tmp = try_copy_slice(b"Hello World").collect::<[u8; 11]>(); assert_eq!(tmp, Some(*b"Hello World")); @@ -337,8 +364,8 @@ assert_eq!(tmp, None); The same naturally also works for Results, but the example is a bit harder to motivate: ```rust +use rosenpass_to::{to, with_destination, To}; use std::result::Result; -use rosenpass_to::{to, To, with_destination}; #[derive(PartialEq, Eq, Debug, Default)] struct InvalidFloat; @@ -380,8 +407,8 @@ Condensation is implemented through a trait called CondenseBeside ([local](Conde If you can not implement this trait because its for an external type (see [orphan rule](https://doc.rust-lang.org/book/ch10-02-traits.html#implementing-a-trait-on-a-type)), this crate welcomes contributions of new Condensation rules. ```rust -use rosenpass_to::{To, with_destination, Beside, CondenseBeside}; use rosenpass_to::ops::copy_slice; +use rosenpass_to::{with_destination, Beside, CondenseBeside, To}; #[derive(PartialEq, Eq, Debug, Default)] struct MyTuple(Left, Right); @@ -396,7 +423,10 @@ impl CondenseBeside for MyTuple<(), Right> { } fn copy_slice_and_return_something<'a, T, U>(src: &'a [T], something: U) -> impl To<[T], U> + 'a - where T: Copy, U: 'a { +where + T: Copy, + U: 'a, +{ with_destination(move |dst: &mut [T]| { copy_slice(src).to(dst); something @@ -417,7 +447,7 @@ Using `with_destination(...)` is convenient, but since it uses closures it resul Implementing the ToTrait manual is the right choice for library use cases. ```rust -use rosenpass_to::{to, To, with_destination}; +use rosenpass_to::{to, with_destination, To}; struct TryCopySliceSource<'a, T: Copy> { src: &'a [T], @@ -425,17 +455,20 @@ struct TryCopySliceSource<'a, T: Copy> { impl<'a, T: Copy> To<[T], Option<()>> for TryCopySliceSource<'a, T> { fn to(self, dst: &mut [T]) -> Option<()> { - (self.src.len() == dst.len()) - .then(|| dst.copy_from_slice(self.src)) + (self.src.len() == dst.len()).then(|| dst.copy_from_slice(self.src)) } } fn try_copy_slice<'a, T>(src: &'a [T]) -> TryCopySliceSource<'a, T> - where T: Copy { +where + T: Copy, +{ TryCopySliceSource { src } } -let mut dst = try_copy_slice(b"Hello World").collect::<[u8; 11]>().unwrap(); +let mut dst = try_copy_slice(b"Hello World") + .collect::<[u8; 11]>() + .unwrap(); assert_eq!(&dst[..], b"Hello World"); assert_eq!(None, to(&mut dst[..], try_copy_slice(b"---"))); ``` @@ -445,8 +478,8 @@ assert_eq!(None, to(&mut dst[..], try_copy_slice(b"---"))); Destinations can also be used with methods. This example demonstrates using destinations in an extension trait for everything that implements `Borrow<[T]>` for any `T` and a concrete `To` trait implementation. ```rust +use rosenpass_to::{to, with_destination, To}; use std::borrow::Borrow; -use rosenpass_to::{to, To, with_destination}; struct TryCopySliceSource<'a, T: Copy> { src: &'a [T], @@ -454,24 +487,24 @@ struct TryCopySliceSource<'a, T: Copy> { impl<'a, T: Copy> To<[T], Option<()>> for TryCopySliceSource<'a, T> { fn to(self, dst: &mut [T]) -> Option<()> { - (self.src.len() == dst.len()) - .then(|| dst.copy_from_slice(self.src)) + (self.src.len() == dst.len()).then(|| dst.copy_from_slice(self.src)) } } trait TryCopySliceExt<'a, T: Copy> { - fn try_copy_slice(&'a self) -> TryCopySliceSource<'a, T>; + fn try_copy_slice(&'a self) -> TryCopySliceSource<'a, T>; } impl<'a, T: 'a + Copy, Ref: 'a + Borrow<[T]>> TryCopySliceExt<'a, T> for Ref { - fn try_copy_slice(&'a self) -> TryCopySliceSource<'a, T> { - TryCopySliceSource { - src: self.borrow() - } + fn try_copy_slice(&'a self) -> TryCopySliceSource<'a, T> { + TryCopySliceSource { src: self.borrow() } } } -let mut dst = b"Hello World".try_copy_slice().collect::<[u8; 11]>().unwrap(); +let mut dst = b"Hello World" + .try_copy_slice() + .collect::<[u8; 11]>() + .unwrap(); assert_eq!(&dst[..], b"Hello World"); assert_eq!(None, to(&mut dst[..], b"---".try_copy_slice())); ``` From 4a170b19830c2df80c6031b73adce876df5b9947 Mon Sep 17 00:00:00 2001 From: James Brownlee Date: Wed, 22 Nov 2023 20:11:04 -0500 Subject: [PATCH 04/13] feat: add inital identity hiding code to proverif --- analysis/03_identity_hiding.entry.mpv | 31 +++++++++++ analysis/crypto/kem.mpv | 5 +- analysis/rosenpass/oracles.mpv | 75 ++++++++++++++++++++++++--- analysis/rosenpass/protocol.mpv | 18 +++++++ 4 files changed, 122 insertions(+), 7 deletions(-) create mode 100644 analysis/03_identity_hiding.entry.mpv diff --git a/analysis/03_identity_hiding.entry.mpv b/analysis/03_identity_hiding.entry.mpv new file mode 100644 index 00000000..d18d4250 --- /dev/null +++ b/analysis/03_identity_hiding.entry.mpv @@ -0,0 +1,31 @@ +#include "config.mpv" + +#define CHAINING_KEY_EVENTS 1 +#define MESSAGE_TRANSMISSION_EVENTS 1 +#define SESSION_START_EVENTS 0 +#define RANDOMIZED_CALL_IDS 0 +#define CONSTANT_KEYS 1 +#define SECURE_RNG 1 +#undef FULL_MODEL +#undef SIMPLE_MODEL +#define SIMPLE_MODEL 1 + +#include "prelude/basic.mpv" +#include "crypto/key.mpv" +#include "crypto/kem.mpv" + +//free initiator_sk1, initiator_sk2, responder_sk: kem_sk [private]. +free initiator_pk, responder_pk: kem_pk[private]. +// noninterf initiator_pk among(kem_pub(initiator_sk1), kem_pub(initiator_sk2)). + +#include "rosenpass/oracles.mpv" + +let identity_hiding_main() = 0 + | REP(INITIATOR_BOUND, Oinitiator) + | REP(RESPONDER_BOUND, Oinit_hello) + | REP(RESPONDER_BOUND, Oinit_conf). + +let main = identity_hiding_main. + +weaksecret initiator_pk. +weaksecret responder_pk. diff --git a/analysis/crypto/kem.mpv b/analysis/crypto/kem.mpv index edfbe1d4..168263a6 100644 --- a/analysis/crypto/kem.mpv +++ b/analysis/crypto/kem.mpv @@ -9,10 +9,13 @@ type kem_sk. type kem_pk. fun kem_pub(kem_sk) : kem_pk. +fun kem_private(kem_pk) : kem_sk[private]. fun kem_enc(kem_pk, key) : bits. fun kem_dec(kem_sk, bits) : key reduc forall sk:kem_sk, shk:key; - kem_dec(sk, kem_enc(kem_pub(sk), shk)) = shk. + kem_dec(sk, kem_enc(kem_pub(sk), shk)) = shk + otherwise forall pk:kem_pk, shk:key; + kem_dec(kem_private(pk), kem_enc(pk, shk)) = shk. fun kem_pk2b(kem_pk) : bits [typeConverter]. diff --git a/analysis/rosenpass/oracles.mpv b/analysis/rosenpass/oracles.mpv index d21e6b24..0825f7a0 100644 --- a/analysis/rosenpass/oracles.mpv +++ b/analysis/rosenpass/oracles.mpv @@ -54,7 +54,17 @@ let Oinit_conf() = #else call <- Cinit_conf(Ssskm, Spsk, Sspkt, ic); #endif + +#ifdef CONSTANT_KEYS + spkt <- initiator_pk; + spkm <- responder_pk; + sskm <- kem_private(spkm); + psk <- setup_key(Spsk); + biscuit_key <- biscuit_key(sskm); +#else SETUP_HANDSHAKE_STATE() +#endif + eski <- kem_sk0; epki <- kem_pk0; let try_ = ( @@ -98,7 +108,7 @@ let Oresp_hello(HS_DECL_ARGS) = SES_EV( event InitiatorSession(rh, osk); ) ic /* success */ ) in ( - out(C, ic) + out(C, EnvelopeInitConf(create_mac(spkt, IC2b(ic)), ic)) /* fail */ ) else ( #if MESSAGE_TRANSMISSION_EVENTS event RHRjct(rh, psk, sski, spkr) @@ -125,14 +135,40 @@ let Oinit_hello() = #endif // TODO: This is ugly let InitHello(sidi, epki, sctr, pidiC, auth) = ih in + +#ifdef CONSTANT_KEYS + spkt <- initiator_pk; + spkm <- responder_pk; + sskm <- kem_private(spkm); + psk <- setup_key(Spsk); + biscuit_key <- biscuit_key(sskm); +#else SETUP_HANDSHAKE_STATE() +#endif + eski <- kem_sk0; - epti <- rng_key(setup_seed(Septi)); // RHR4 - spti <- rng_key(setup_seed(Sspti)); // RHR5 + event ConsumeBn(biscuit_no, sskm, spkt, call); event ConsumeSidr(sidr, call); + +#ifdef SECURE_RNG + new septi_trusted_prec: seed_prec; + new sspti_trusted_prec: seed_prec; + + septi_trusted_seed <- make_trusted_seed(septi_trusted_prec); + sspti_trusted_seed <- make_trusted_seed(sspti_trusted_prec); + + epti <- rng_key(setup_seed(septi_trusted_seed)); // RHR4 + spti <- rng_key(setup_seed(sspti_trusted_seed)); // RHR4 + event ConsumeSeed(Epti, setup_seed(septi_trusted_seed), call); + event ConsumeSeed(Spti, setup_seed(sspti_trusted_seed), call); +#else + epti <- rng_key(setup_seed(Septi)); // RHR4 + spti <- rng_key(setup_seed(Sspti)); // RHR5 event ConsumeSeed(Epti, setup_seed(Septi), call); event ConsumeSeed(Spti, setup_seed(Sspti), call); +#endif + let rh = ( INITHELLO_CONSUME() ck_ini <- ck; @@ -141,7 +177,7 @@ let Oinit_hello() = MTX_EV( event RHSent(ih, rh, psk, sskr, spki); ) rh /* success */ ) in ( - out(C, rh) + out(C, EnvelopeRespHello(create_mac(spkt, RH2b(rh)), rh)) /* fail */ ) else ( #if MESSAGE_TRANSMISSION_EVENTS event IHRjct(ih, psk, sskr, spki) @@ -167,6 +203,7 @@ CK_EV( event OskOinitiator_ck(key). ) CK_EV( event OskOinitiator(key, key, kem_sk, kem_pk, key). ) MTX_EV( event IHSent(InitHello_t, key, kem_sk, kem_pk). ) event ConsumeSidi(SessionId, Atom). + let Oinitiator() = in(C, Cinitiator(sidi, Ssskm, Spsk, Sspkt, Seski, Ssptr)); #if RANDOMIZED_CALL_IDS @@ -174,18 +211,44 @@ let Oinitiator() = #else call <- Cinitiator(sidi, Ssskm, Spsk, Sspkt, Seski, Ssptr); #endif + +#ifdef CONSTANT_KEYS + spkt <- responder_pk; + spkm <- initiator_pk; + sskm <- kem_private(spkm); + psk <- setup_key(Spsk); + biscuit_key <- biscuit_key(sskm); +#else SETUP_HANDSHAKE_STATE() - RNG_KEM_PAIR(eski, epki, Seski) // IHI3 +#endif + sidr <- sid0; + +#ifdef SECURE_RNG + new ssptr_trusted_prec: seed_prec; + new seski_trusted_prec: seed_prec; + + ssptr_trusted_seed <- make_trusted_seed(ssptr_trusted_prec); + seski_trusted_seed <- make_trusted_seed(seski_trusted_prec); + + RNG_KEM_PAIR(eski, epki, seski_trusted_seed) // IHI3 + sptr <- rng_key(setup_seed(ssptr_trusted_seed)); // IHI5 + event ConsumeSidi(sidi, call); + event ConsumeSeed(Sptr, setup_seed(ssptr_trusted_seed), call); + event ConsumeSeed(Eski, setup_seed(seski_trusted_seed), call); +#else + RNG_KEM_PAIR(eski, epki, Seski) // IHI3 sptr <- rng_key(setup_seed(Ssptr)); // IHI5 event ConsumeSidi(sidi, call); event ConsumeSeed(Sptr, setup_seed(Ssptr), call); event ConsumeSeed(Eski, setup_seed(Seski), call); +#endif + INITHELLO_PRODUCE() CK_EV( event OskOinitiator_ck(ck); ) CK_EV( event OskOinitiator(ck, psk, sski, spkr, sptr); ) MTX_EV( event IHSent(ih, psk, sski, spkr); ) - out(C, ih); + out(C, EnvelopeInitHello(create_mac(spkt, IH2b(ih) ), ih)); Oresp_hello(HS_PASS_ARGS). restriction sid:SessionId, ad1:Atom, ad2:Atom; diff --git a/analysis/rosenpass/protocol.mpv b/analysis/rosenpass/protocol.mpv index 658b05fc..9874cf4e 100644 --- a/analysis/rosenpass/protocol.mpv +++ b/analysis/rosenpass/protocol.mpv @@ -2,6 +2,15 @@ #include "crypto/kem.mpv" #include "rosenpass/handshake_state.mpv" +#define ENVELOPE(TYPE) \ + type MCAT(MCAT(Envelope, TYPE), _t). \ + fun CAT(Envelope, TYPE) ( \ + key, \ + MCAT(TYPE, _t) \ + ) : MCAT(MCAT(Envelope, TYPE), _t) [data]. + +letfun create_mac(pk:kem_pk, payload:bits) = lprf2(MAC, kem_pk2b(pk), payload). + type InitHello_t. fun InitHello( SessionId, // sidi @@ -11,6 +20,9 @@ fun InitHello( bits // auth ) : InitHello_t [data]. +ENVELOPE(InitHello) +fun IH2b(InitHello_t) : bitstring [typeConverter]. + #define INITHELLO_PRODUCE() \ ck <- lprf1(CK_INIT, kem_pk2b(spkr)); /* IHI1 */ \ /* not handled here */ /* IHI2 */ \ @@ -41,6 +53,9 @@ fun RespHello( bits // auth ) : RespHello_t [data]. +ENVELOPE(RespHello) +fun RH2b(RespHello_t) : bitstring [typeConverter]. + #define RESPHELLO_PRODUCE() \ /* not handled here */ /* RHR1 */ \ MIX2(sid2b(sidr), sid2b(sidi)) /* RHR3 */ \ @@ -67,6 +82,9 @@ fun InitConf( bits // auth ) : InitConf_t [data]. +ENVELOPE(InitConf) +fun IC2b(InitConf_t) : bitstring [typeConverter]. + #define INITCONF_PRODUCE() \ MIX2(sid2b(sidi), sid2b(sidr)) /* ICI3 */ \ ENCRYPT_AND_MIX(auth, empty) /* ICI4 */ \ From 91da0dfd2da54c3b3fa960475e90e9a36de03561 Mon Sep 17 00:00:00 2001 From: James Brownlee Date: Sat, 25 Nov 2023 20:49:32 -0500 Subject: [PATCH 05/13] feat: identity hiding in two stage process Changed identity hiding test to work as a two stage process where participants with fresh secure secret keys communicate with each other and other compromised participants. Then the attacker is asked to identify the difference between two of the secure participants as on of them acts as a responder. --- analysis/03_identity_hiding.entry.mpv | 105 +++++++++++++++++++--- analysis/crypto/kem.mpv | 5 +- analysis/rosenpass/oracles.mpv | 123 +++++++++----------------- analysis/rosenpass/protocol.mpv | 16 ++-- 4 files changed, 142 insertions(+), 107 deletions(-) diff --git a/analysis/03_identity_hiding.entry.mpv b/analysis/03_identity_hiding.entry.mpv index d18d4250..7533ae87 100644 --- a/analysis/03_identity_hiding.entry.mpv +++ b/analysis/03_identity_hiding.entry.mpv @@ -1,31 +1,114 @@ +/* + This identity hiding process tests whether the rosenpass protocol is able to protect the identity of an initiator or responder. + The participants in the test are trusted initiators, trusted responders and compromised initiators and responders. + The test consists of two phases. In the first phase all of the participants can communicate with each other using the rosenpass protocol. + An attacker observes the first phase and is able to intercept and modify messages and choose participants to communicate with each other + + In the second phase if the anonymity of an initiator is being tested then one of two trusted initiators is chosen. + The chosen initiator communicates directly with a trusted responder. + If an attacker can determine which initiator was chosen then the anonymity of the initiator has been compromised. + Otherwise the protocol has successfully protected the initiators’ identity. + + If the anonymity of a responder is being tested then one of two trusted responders is chosen instead. + Then an initiator communicates directly with the chosen responder. + If an attacker can determine which responder was chosen then the anonymity of the responder is compromised. + Otherwise the protocol successfully protects the identity of a responder. + + The Proverif code treats the public key as synonymous with identity. + In the above test when a responder or initiator is chosen what is actually chosen is the public/private key pair to use for communication. + Traditionally when a responder or initiator is chosen they would be chosen randomly. + The way Proverif makes a "choice" is by simulating multiple processes, one process per choice + Then the processes are compared and if an association between a public key and a process can be made the test fails. + As the choice is at least as bad as choosing the worst possible option the credibility of the test is maintained. + The drawback is that Proverif is only able to tell if the identity can be brute forced but misses any probabilistic associations. + As usual Proverif also assumes perfect encryption and in particular assumes encryption cannot be linked to identity. + + One of the tradeoffs made here is that the choice function in Proverif is slow but this is in favour of being able to write more precise tests. + Another issue is the choice function does not work with queries so a test needs to be run for each set of assumptions. + In this case the test uses secure rng and a fresh secure biscuit key. +*/ + + #include "config.mpv" #define CHAINING_KEY_EVENTS 1 #define MESSAGE_TRANSMISSION_EVENTS 1 #define SESSION_START_EVENTS 0 #define RANDOMIZED_CALL_IDS 0 -#define CONSTANT_KEYS 1 -#define SECURE_RNG 1 #undef FULL_MODEL #undef SIMPLE_MODEL #define SIMPLE_MODEL 1 #include "prelude/basic.mpv" #include "crypto/key.mpv" +#include "rosenpass/oracles.mpv" #include "crypto/kem.mpv" -//free initiator_sk1, initiator_sk2, responder_sk: kem_sk [private]. -free initiator_pk, responder_pk: kem_pk[private]. -// noninterf initiator_pk among(kem_pub(initiator_sk1), kem_pub(initiator_sk2)). +#define NEW_TRUSTED_SEED(name) \ + new MCAT(name, _secret_seed):seed_prec; \ + name <- make_trusted_seed(MCAT(name, _secret_seed)); \ -#include "rosenpass/oracles.mpv" +free D:channel [private]. +free secure_biscuit_no:Atom [private]. +free secure_sidi,secure_sidr:SessionId [private]. +free secure_psk:key [private]. +free initiator1, initiator2:kem_sk_prec. +free responder1, responder2:kem_sk_prec. -let identity_hiding_main() = 0 - | REP(INITIATOR_BOUND, Oinitiator) +let secure_init_hello(initiator: kem_sk_tmpl, sidi : SessionId, psk: key_tmpl, responder: kem_sk_tmpl) = + NEW_TRUSTED_SEED(seski_trusted_seed) + NEW_TRUSTED_SEED(ssptr_trusted_seed) + Oinitiator_inner(sidi, initiator, psk, responder, seski_trusted_seed, ssptr_trusted_seed, D). + +let secure_resp_hello(initiator: kem_sk_tmpl, responder: kem_sk_tmpl, sidr:SessionId, sidi:SessionId, biscuit_no:Atom, psk:key_tmpl) = + in(D, Envelope(k, IH2b(InitHello(=sidi, epki, sctr, pidiC, auth)))); + ih <- InitHello(sidi, epki, sctr, pidiC, auth); + NEW_TRUSTED_SEED(septi_trusted_seed) + NEW_TRUSTED_SEED(sspti_trusted_seed) + Oinit_hello_inner(sidr, biscuit_no, responder, psk, initiator, septi_trusted_seed, sspti_trusted_seed, ih, D). + +let secure_init_conf(initiator: kem_sk_tmpl, responder: kem_sk_tmpl, psk:key_tmpl, sidi:SessionId, sidr:SessionId) = + in(D, Envelope(k3, IC2b(InitConf(=sidi, =sidr, biscuit, auth3)))); + ic <- InitConf(sidi,sidr,biscuit, auth3); + NEW_TRUSTED_SEED(seski_trusted_seed) + NEW_TRUSTED_SEED(ssptr_trusted_seed) + Oinit_conf_inner(initiator, psk, responder, ic). + +let secure_communication(initiator: kem_sk_tmpl, responder:kem_sk_tmpl) = + secure_key <- prepare_key(secure_psk); + (!secure_init_hello(initiator, secure_sidi, secure_key, responder)) + | !secure_resp_hello(initiator, responder, secure_sidr, secure_sidi, secure_biscuit_no, secure_key) + | !(secure_init_conf(initiator, responder, secure_key, secure_sidi, secure_sidr)). + +let pipeChannel(D:channel, C:channel) = + in(D, b:bits); + out(C, b). + +fun kem_private(kem_pk): kem_sk + reduc forall sk_tmpl:kem_sk; + kem_private(kem_pub(sk_tmpl)) = sk_tmpl[private]. + +let secretCommunication() = + initiator_pk <- choice[setup_kem_pk(make_trusted_kem_sk(initiator1)), setup_kem_pk(make_trusted_kem_sk(initiator2))]; + initiator_seed <- prepare_kem_sk(kem_private(initiator_pk)); + responder_seed <- prepare_kem_sk(trusted_kem_sk(responder1)); + // initiator_seed <- prepare_kem_sk(trusted_kem_sk(initiator1)); + // responder_pk <- choice[setup_kem_pk(make_trusted_kem_sk(responder1)), setup_kem_pk(make_trusted_kem_sk(responder2))]; + // responder_seed <- prepare_kem_sk(kem_private(responder_pk)); + secure_communication(initiator_seed, responder_seed) | !pipeChannel(D, C). + +let reveal_pks() = + out(C, setup_kem_pk(make_trusted_kem_sk(responder1))); + out(C, setup_kem_pk(make_trusted_kem_sk(responder2))); + out(C, setup_kem_pk(make_trusted_kem_sk(initiator1))); + out(C, setup_kem_pk(make_trusted_kem_sk(initiator2))). + +let rosenpass_main2() = + REP(INITIATOR_BOUND, Oinitiator) | REP(RESPONDER_BOUND, Oinit_hello) | REP(RESPONDER_BOUND, Oinit_conf). -let main = identity_hiding_main. +let identity_hiding_main() = + 0 | reveal_pks() | rosenpass_main2() | phase 1; secretCommunication(). -weaksecret initiator_pk. -weaksecret responder_pk. +let main = identity_hiding_main. diff --git a/analysis/crypto/kem.mpv b/analysis/crypto/kem.mpv index 168263a6..edfbe1d4 100644 --- a/analysis/crypto/kem.mpv +++ b/analysis/crypto/kem.mpv @@ -9,13 +9,10 @@ type kem_sk. type kem_pk. fun kem_pub(kem_sk) : kem_pk. -fun kem_private(kem_pk) : kem_sk[private]. fun kem_enc(kem_pk, key) : bits. fun kem_dec(kem_sk, bits) : key reduc forall sk:kem_sk, shk:key; - kem_dec(sk, kem_enc(kem_pub(sk), shk)) = shk - otherwise forall pk:kem_pk, shk:key; - kem_dec(kem_private(pk), kem_enc(pk, shk)) = shk. + kem_dec(sk, kem_enc(kem_pub(sk), shk)) = shk. fun kem_pk2b(kem_pk) : bits [typeConverter]. diff --git a/analysis/rosenpass/oracles.mpv b/analysis/rosenpass/oracles.mpv index 0825f7a0..9212ac79 100644 --- a/analysis/rosenpass/oracles.mpv +++ b/analysis/rosenpass/oracles.mpv @@ -47,23 +47,15 @@ CK_EV( event OskOinit_conf(key, key). ) MTX_EV( event ICRjct(InitConf_t, key, kem_sk, kem_pk). ) SES_EV( event ResponderSession(InitConf_t, key). ) event ConsumeBiscuit(Atom, kem_sk, kem_pk, Atom). -let Oinit_conf() = - in(C, Cinit_conf(Ssskm, Spsk, Sspkt, ic)); + +let Oinit_conf_inner(Ssskm:kem_sk_tmpl, Spsk:key_tmpl, Sspkt:kem_sk_tmpl, ic:InitConf_t) = #if RANDOMIZED_CALL_IDS new call:Atom; #else call <- Cinit_conf(Ssskm, Spsk, Sspkt, ic); #endif -#ifdef CONSTANT_KEYS - spkt <- initiator_pk; - spkm <- responder_pk; - sskm <- kem_private(spkm); - psk <- setup_key(Spsk); - biscuit_key <- biscuit_key(sskm); -#else SETUP_HANDSHAKE_STATE() -#endif eski <- kem_sk0; epki <- kem_pk0; @@ -82,6 +74,10 @@ let Oinit_conf() = 0 #endif ). + +let Oinit_conf() = + in(C, Cinit_conf(Ssskm, Spsk, Sspkt, ic)); + Oinit_conf_inner(Ssskm, Spsk, Sspkt, ic). restriction biscuit_no:Atom, sskm:kem_sk, spkr:kem_pk, ad1:Atom, ad2:Atom; event(ConsumeBiscuit(biscuit_no, sskm, spkr, ad1)) && event(ConsumeBiscuit(biscuit_no, sskm, spkr, ad2)) @@ -95,8 +91,8 @@ CK_EV( event OskOresp_hello(key, key, key). ) MTX_EV( event RHRjct(RespHello_t, key, kem_sk, kem_pk). ) MTX_EV( event ICSent(RespHello_t, InitConf_t, key, kem_sk, kem_pk). ) SES_EV( event InitiatorSession(RespHello_t, key). ) -let Oresp_hello(HS_DECL_ARGS) = - in(C, Cresp_hello(RespHello(sidr, =sidi, ecti, scti, biscuit, auth))); +let Oresp_hello(HS_DECL_ARGS, C_in:channel) = + in(C_in, Cresp_hello(RespHello(sidr, =sidi, ecti, scti, biscuit, auth))); rh <- RespHello(sidr, sidi, ecti, scti, biscuit, auth); /* try */ let ic = ( ck_ini <- ck; @@ -108,7 +104,7 @@ let Oresp_hello(HS_DECL_ARGS) = SES_EV( event InitiatorSession(rh, osk); ) ic /* success */ ) in ( - out(C, EnvelopeInitConf(create_mac(spkt, IC2b(ic)), ic)) + out(C_in, Envelope(create_mac(spkt, IC2b(ic)), IC2b(ic))) /* fail */ ) else ( #if MESSAGE_TRANSMISSION_EVENTS event RHRjct(rh, psk, sski, spkr) @@ -126,8 +122,8 @@ MTX_EV( event IHRjct(InitHello_t, key, kem_sk, kem_pk). ) MTX_EV( event RHSent(InitHello_t, RespHello_t, key, kem_sk, kem_pk). ) event ConsumeSidr(SessionId, Atom). event ConsumeBn(Atom, kem_sk, kem_pk, Atom). -let Oinit_hello() = - in(C, Cinit_hello(sidr, biscuit_no, Ssskm, Spsk, Sspkt, Septi, Sspti, ih)); + +let Oinit_hello_inner(sidm:SessionId, biscuit_no:Atom, Ssskm:kem_sk_tmpl, Spsk:key_tmpl, Sspkt: kem_sk_tmpl, Septi: seed_tmpl, Sspti: seed_tmpl, ih: InitHello_t, C_out:channel) = #if RANDOMIZED_CALL_IDS new call:Atom; #else @@ -136,38 +132,17 @@ let Oinit_hello() = // TODO: This is ugly let InitHello(sidi, epki, sctr, pidiC, auth) = ih in -#ifdef CONSTANT_KEYS - spkt <- initiator_pk; - spkm <- responder_pk; - sskm <- kem_private(spkm); - psk <- setup_key(Spsk); - biscuit_key <- biscuit_key(sskm); -#else SETUP_HANDSHAKE_STATE() -#endif eski <- kem_sk0; event ConsumeBn(biscuit_no, sskm, spkt, call); event ConsumeSidr(sidr, call); -#ifdef SECURE_RNG - new septi_trusted_prec: seed_prec; - new sspti_trusted_prec: seed_prec; - - septi_trusted_seed <- make_trusted_seed(septi_trusted_prec); - sspti_trusted_seed <- make_trusted_seed(sspti_trusted_prec); - - epti <- rng_key(setup_seed(septi_trusted_seed)); // RHR4 - spti <- rng_key(setup_seed(sspti_trusted_seed)); // RHR4 - event ConsumeSeed(Epti, setup_seed(septi_trusted_seed), call); - event ConsumeSeed(Spti, setup_seed(sspti_trusted_seed), call); -#else epti <- rng_key(setup_seed(Septi)); // RHR4 spti <- rng_key(setup_seed(Sspti)); // RHR5 event ConsumeSeed(Epti, setup_seed(Septi), call); event ConsumeSeed(Spti, setup_seed(Sspti), call); -#endif let rh = ( INITHELLO_CONSUME() @@ -177,7 +152,8 @@ let Oinit_hello() = MTX_EV( event RHSent(ih, rh, psk, sskr, spki); ) rh /* success */ ) in ( - out(C, EnvelopeRespHello(create_mac(spkt, RH2b(rh)), rh)) + out(C_out, Envelope(create_mac(spkt, RH2b(rh)), RH2b(rh))) + /* fail */ ) else ( #if MESSAGE_TRANSMISSION_EVENTS event IHRjct(ih, psk, sskr, spki) @@ -186,6 +162,10 @@ let Oinit_hello() = #endif ). +let Oinit_hello() = + in(C, Cinit_hello(sidr, biscuit_no, Ssskm, Spsk, Sspkt, Septi, Sspti, ih)); + Oinit_hello_inner(sidr, biscuit_no, Ssskm, Spsk, Sspkt, Septi, Sspti, ih, C). + restriction sid:SessionId, ad1:Atom, ad2:Atom; event(ConsumeSidr(sid, ad1)) && event(ConsumeSidr(sid, ad2)) ==> ad1 = ad2. @@ -204,52 +184,33 @@ CK_EV( event OskOinitiator(key, key, kem_sk, kem_pk, key). ) MTX_EV( event IHSent(InitHello_t, key, kem_sk, kem_pk). ) event ConsumeSidi(SessionId, Atom). +let Oinitiator_inner(sidi: SessionId, Ssskm: kem_sk_tmpl, Spsk: key_tmpl, Sspkt: kem_sk_tmpl, Seski: seed_tmpl, Ssptr: seed_tmpl, C_out:channel) = + #if RANDOMIZED_CALL_IDS + new call:Atom; + #else + call <- Cinitiator(sidi, Ssskm, Spsk, Sspkt, Seski, Ssptr); + #endif + + SETUP_HANDSHAKE_STATE() + + sidr <- sid0; + + RNG_KEM_PAIR(eski, epki, Seski) // IHI3 + sptr <- rng_key(setup_seed(Ssptr)); // IHI5 + event ConsumeSidi(sidi, call); + event ConsumeSeed(Sptr, setup_seed(Ssptr), call); + event ConsumeSeed(Eski, setup_seed(Seski), call); + + INITHELLO_PRODUCE() + CK_EV( event OskOinitiator_ck(ck); ) + CK_EV( event OskOinitiator(ck, psk, sski, spkr, sptr); ) + MTX_EV( event IHSent(ih, psk, sski, spkr); ) + out(C_out, Envelope(create_mac(spkt, IH2b(ih)), IH2b(ih))); + Oresp_hello(HS_PASS_ARGS, C_out). + let Oinitiator() = in(C, Cinitiator(sidi, Ssskm, Spsk, Sspkt, Seski, Ssptr)); -#if RANDOMIZED_CALL_IDS - new call:Atom; -#else - call <- Cinitiator(sidi, Ssskm, Spsk, Sspkt, Seski, Ssptr); -#endif - -#ifdef CONSTANT_KEYS - spkt <- responder_pk; - spkm <- initiator_pk; - sskm <- kem_private(spkm); - psk <- setup_key(Spsk); - biscuit_key <- biscuit_key(sskm); -#else - SETUP_HANDSHAKE_STATE() -#endif - - sidr <- sid0; - -#ifdef SECURE_RNG - new ssptr_trusted_prec: seed_prec; - new seski_trusted_prec: seed_prec; - - ssptr_trusted_seed <- make_trusted_seed(ssptr_trusted_prec); - seski_trusted_seed <- make_trusted_seed(seski_trusted_prec); - - RNG_KEM_PAIR(eski, epki, seski_trusted_seed) // IHI3 - sptr <- rng_key(setup_seed(ssptr_trusted_seed)); // IHI5 - event ConsumeSidi(sidi, call); - event ConsumeSeed(Sptr, setup_seed(ssptr_trusted_seed), call); - event ConsumeSeed(Eski, setup_seed(seski_trusted_seed), call); -#else - RNG_KEM_PAIR(eski, epki, Seski) // IHI3 - sptr <- rng_key(setup_seed(Ssptr)); // IHI5 - event ConsumeSidi(sidi, call); - event ConsumeSeed(Sptr, setup_seed(Ssptr), call); - event ConsumeSeed(Eski, setup_seed(Seski), call); -#endif - - INITHELLO_PRODUCE() - CK_EV( event OskOinitiator_ck(ck); ) - CK_EV( event OskOinitiator(ck, psk, sski, spkr, sptr); ) - MTX_EV( event IHSent(ih, psk, sski, spkr); ) - out(C, EnvelopeInitHello(create_mac(spkt, IH2b(ih) ), ih)); - Oresp_hello(HS_PASS_ARGS). + Oinitiator_inner(sidi, Ssskm, Spsk, Sspkt, Seski, Ssptr, C). restriction sid:SessionId, ad1:Atom, ad2:Atom; event(ConsumeSidi(sid, ad1)) && event(ConsumeSidi(sid, ad2)) diff --git a/analysis/rosenpass/protocol.mpv b/analysis/rosenpass/protocol.mpv index 9874cf4e..a9009819 100644 --- a/analysis/rosenpass/protocol.mpv +++ b/analysis/rosenpass/protocol.mpv @@ -2,13 +2,10 @@ #include "crypto/kem.mpv" #include "rosenpass/handshake_state.mpv" -#define ENVELOPE(TYPE) \ - type MCAT(MCAT(Envelope, TYPE), _t). \ - fun CAT(Envelope, TYPE) ( \ - key, \ - MCAT(TYPE, _t) \ - ) : MCAT(MCAT(Envelope, TYPE), _t) [data]. - +fun Envelope( + key, + bits +): bits [data]. letfun create_mac(pk:kem_pk, payload:bits) = lprf2(MAC, kem_pk2b(pk), payload). type InitHello_t. @@ -20,7 +17,6 @@ fun InitHello( bits // auth ) : InitHello_t [data]. -ENVELOPE(InitHello) fun IH2b(InitHello_t) : bitstring [typeConverter]. #define INITHELLO_PRODUCE() \ @@ -53,10 +49,9 @@ fun RespHello( bits // auth ) : RespHello_t [data]. -ENVELOPE(RespHello) fun RH2b(RespHello_t) : bitstring [typeConverter]. -#define RESPHELLO_PRODUCE() \ +#define RESPHELLO_PRODUCE() \ /* not handled here */ /* RHR1 */ \ MIX2(sid2b(sidr), sid2b(sidi)) /* RHR3 */ \ ENCAPS_AND_MIX(ecti, epki, epti) /* RHR4 */ \ @@ -82,7 +77,6 @@ fun InitConf( bits // auth ) : InitConf_t [data]. -ENVELOPE(InitConf) fun IC2b(InitConf_t) : bitstring [typeConverter]. #define INITCONF_PRODUCE() \ From b2a64ed17a30d835b1bb9bd9bc1a9ac21fe72780 Mon Sep 17 00:00:00 2001 From: James Brownlee Date: Fri, 15 Dec 2023 10:55:16 -0500 Subject: [PATCH 06/13] feat: add INITIATOR_TEST and RESPONDER_TEST macros Added INITIATOR_TEST and RESPONDER_TEST macros to the identity hiding mpv file that can be used to selectively test the anonymity of the initiator or the responder. --- analysis/03_identity_hiding.entry.mpv | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/analysis/03_identity_hiding.entry.mpv b/analysis/03_identity_hiding.entry.mpv index 7533ae87..985fda0c 100644 --- a/analysis/03_identity_hiding.entry.mpv +++ b/analysis/03_identity_hiding.entry.mpv @@ -44,6 +44,7 @@ #include "rosenpass/oracles.mpv" #include "crypto/kem.mpv" +#define INITIATOR_TEST #define NEW_TRUSTED_SEED(name) \ new MCAT(name, _secret_seed):seed_prec; \ name <- make_trusted_seed(MCAT(name, _secret_seed)); \ @@ -89,12 +90,18 @@ fun kem_private(kem_pk): kem_sk kem_private(kem_pub(sk_tmpl)) = sk_tmpl[private]. let secretCommunication() = +#ifdef INITIATOR_TEST initiator_pk <- choice[setup_kem_pk(make_trusted_kem_sk(initiator1)), setup_kem_pk(make_trusted_kem_sk(initiator2))]; initiator_seed <- prepare_kem_sk(kem_private(initiator_pk)); +#else + initiator_seed <- prepare_kem_sk(trusted_kem_sk(initiator1)); +#endif +#ifdef RESPONDER_TEST + responder_pk <- choice[setup_kem_pk(make_trusted_kem_sk(responder1)), setup_kem_pk(make_trusted_kem_sk(responder2))]; + responder_seed <- prepare_kem_sk(kem_private(responder_pk)); +#else responder_seed <- prepare_kem_sk(trusted_kem_sk(responder1)); - // initiator_seed <- prepare_kem_sk(trusted_kem_sk(initiator1)); - // responder_pk <- choice[setup_kem_pk(make_trusted_kem_sk(responder1)), setup_kem_pk(make_trusted_kem_sk(responder2))]; - // responder_seed <- prepare_kem_sk(kem_private(responder_pk)); +#endif secure_communication(initiator_seed, responder_seed) | !pipeChannel(D, C). let reveal_pks() = From 85c447052e5bd7e3845815685b1ce45318690025 Mon Sep 17 00:00:00 2001 From: Karolin Varner Date: Wed, 27 Dec 2023 16:57:30 +0100 Subject: [PATCH 07/13] feat: Migrate to memsec --- .github/workflows/qc.yaml | 4 +- Cargo.lock | 94 +++++++++++++++++++++- Cargo.toml | 4 +- fuzz/Cargo.toml | 8 +- fuzz/fuzz_targets/box_secret_alloc.rs | 8 ++ fuzz/fuzz_targets/box_sodium_alloc.rs | 12 --- fuzz/fuzz_targets/vec_secret_alloc.rs | 9 +++ fuzz/fuzz_targets/vec_sodium_alloc.rs | 13 ---- secret-memory/Cargo.toml | 6 ++ secret-memory/src/alloc/memsec.rs | 108 ++++++++++++++++++++++++++ secret-memory/src/alloc/mod.rs | 6 ++ secret-memory/src/lib.rs | 2 + secret-memory/src/secret.rs | 33 ++++---- sodium/src/alloc/allocator.rs | 95 ---------------------- sodium/src/alloc/mod.rs | 10 --- sodium/src/lib.rs | 1 - 16 files changed, 258 insertions(+), 155 deletions(-) create mode 100644 fuzz/fuzz_targets/box_secret_alloc.rs delete mode 100644 fuzz/fuzz_targets/box_sodium_alloc.rs create mode 100644 fuzz/fuzz_targets/vec_secret_alloc.rs delete mode 100644 fuzz/fuzz_targets/vec_sodium_alloc.rs create mode 100644 secret-memory/src/alloc/memsec.rs create mode 100644 secret-memory/src/alloc/mod.rs delete mode 100644 sodium/src/alloc/allocator.rs delete mode 100644 sodium/src/alloc/mod.rs diff --git a/.github/workflows/qc.yaml b/.github/workflows/qc.yaml index eda617a1..903282e6 100644 --- a/.github/workflows/qc.yaml +++ b/.github/workflows/qc.yaml @@ -154,5 +154,5 @@ jobs: cargo fuzz run fuzz_handle_msg -- -max_total_time=5 ulimit -s 8192000 && RUST_MIN_STACK=33554432000 && cargo fuzz run fuzz_kyber_encaps -- -max_total_time=5 cargo fuzz run fuzz_mceliece_encaps -- -max_total_time=5 - cargo fuzz run fuzz_box_sodium_alloc -- -max_total_time=5 - cargo fuzz run fuzz_vec_sodium_alloc -- -max_total_time=5 + cargo fuzz run fuzz_box_secret_alloc -- -max_total_time=5 + cargo fuzz run fuzz_vec_secret_alloc -- -max_total_time=5 diff --git a/Cargo.lock b/Cargo.lock index b4f59a0a..918c4cde 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -46,9 +46,18 @@ dependencies = [ [[package]] name = "allocator-api2" -version = "0.2.16" +version = "0.2.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0942ffc6dcaadf03badf6e6a2d0228460359d5e34b57ccdc720b7382dfbd5ec5" +checksum = "c4f263788a35611fba42eb41ff811c5d0360c58b97402570312a350736e2542e" + +[[package]] +name = "allocator-api2-tests" +version = "0.2.14" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "82e6d832cc75b9841b21c847420f1334645387f088324f34eac923a98efa3d89" +dependencies = [ + "allocator-api2", +] [[package]] name = "anes" @@ -804,6 +813,17 @@ dependencies = [ "autocfg", ] +[[package]] +name = "memsec" +version = "0.6.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0fa0916b001582d253822171bd23f4a0229d32b9507fae236f5da8cad515ba7c" +dependencies = [ + "getrandom", + "libc", + "windows-sys 0.45.0", +] + [[package]] name = "minimal-lexical" version = "0.2.1" @@ -1190,9 +1210,13 @@ dependencies = [ name = "rosenpass-secret-memory" version = "0.1.0" dependencies = [ + "allocator-api2", + "allocator-api2-tests", "anyhow", "lazy_static", "libsodium-sys-stable", + "log", + "memsec", "rand", "rosenpass-sodium", "rosenpass-to", @@ -1704,6 +1728,15 @@ version = "0.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" +[[package]] +name = "windows-sys" +version = "0.45.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "75283be5efb2831d37ea142365f009c02ec203cd29a3ebecbc093d52315b66d0" +dependencies = [ + "windows-targets 0.42.2", +] + [[package]] name = "windows-sys" version = "0.48.0" @@ -1722,6 +1755,21 @@ dependencies = [ "windows-targets 0.52.0", ] +[[package]] +name = "windows-targets" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8e5180c00cd44c9b1c88adb3693291f1cd93605ded80c250a75d472756b4d071" +dependencies = [ + "windows_aarch64_gnullvm 0.42.2", + "windows_aarch64_msvc 0.42.2", + "windows_i686_gnu 0.42.2", + "windows_i686_msvc 0.42.2", + "windows_x86_64_gnu 0.42.2", + "windows_x86_64_gnullvm 0.42.2", + "windows_x86_64_msvc 0.42.2", +] + [[package]] name = "windows-targets" version = "0.48.5" @@ -1752,6 +1800,12 @@ dependencies = [ "windows_x86_64_msvc 0.52.0", ] +[[package]] +name = "windows_aarch64_gnullvm" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "597a5118570b68bc08d8d59125332c54f1ba9d9adeedeef5b99b02ba2b0698f8" + [[package]] name = "windows_aarch64_gnullvm" version = "0.48.5" @@ -1764,6 +1818,12 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cb7764e35d4db8a7921e09562a0304bf2f93e0a51bfccee0bd0bb0b666b015ea" +[[package]] +name = "windows_aarch64_msvc" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e08e8864a60f06ef0d0ff4ba04124db8b0fb3be5776a5cd47641e942e58c4d43" + [[package]] name = "windows_aarch64_msvc" version = "0.48.5" @@ -1776,6 +1836,12 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bbaa0368d4f1d2aaefc55b6fcfee13f41544ddf36801e793edbbfd7d7df075ef" +[[package]] +name = "windows_i686_gnu" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c61d927d8da41da96a81f029489353e68739737d3beca43145c8afec9a31a84f" + [[package]] name = "windows_i686_gnu" version = "0.48.5" @@ -1788,6 +1854,12 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a28637cb1fa3560a16915793afb20081aba2c92ee8af57b4d5f28e4b3e7df313" +[[package]] +name = "windows_i686_msvc" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "44d840b6ec649f480a41c8d80f9c65108b92d89345dd94027bfe06ac444d1060" + [[package]] name = "windows_i686_msvc" version = "0.48.5" @@ -1800,6 +1872,12 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ffe5e8e31046ce6230cc7215707b816e339ff4d4d67c65dffa206fd0f7aa7b9a" +[[package]] +name = "windows_x86_64_gnu" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8de912b8b8feb55c064867cf047dda097f92d51efad5b491dfb98f6bbb70cb36" + [[package]] name = "windows_x86_64_gnu" version = "0.48.5" @@ -1812,6 +1890,12 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3d6fa32db2bc4a2f5abeacf2b69f7992cd09dca97498da74a151a3132c26befd" +[[package]] +name = "windows_x86_64_gnullvm" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "26d41b46a36d453748aedef1486d5c7a85db22e56aff34643984ea85514e94a3" + [[package]] name = "windows_x86_64_gnullvm" version = "0.48.5" @@ -1824,6 +1908,12 @@ version = "0.52.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1a657e1e9d3f514745a572a6846d3c7aa7dbe1658c056ed9c3344c4109a6949e" +[[package]] +name = "windows_x86_64_msvc" +version = "0.42.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9aec5da331524158c6d1a4ac0ab1541149c0b9505fde06423b02f5ef0106b9f0" + [[package]] name = "windows_x86_64_msvc" version = "0.48.5" diff --git a/Cargo.toml b/Cargo.toml index 3ab316fd..96337f8c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,7 +48,9 @@ paste = "1.0.14" env_logger = "0.10.1" toml = "0.7.8" static_assertions = "1.1.0" -allocator-api2 = "0.2.16" +allocator-api2 = "0.2.14" +allocator-api2-tests = "0.2.14" +memsec = "0.6.3" rand = "0.8.5" log = { version = "0.4.20" } clap = { version = "4.4.10", features = ["derive"] } diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 89000db4..0e26f236 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -49,13 +49,13 @@ test = false doc = false [[bin]] -name = "fuzz_box_sodium_alloc" -path = "fuzz_targets/box_sodium_alloc.rs" +name = "fuzz_box_secret_alloc" +path = "fuzz_targets/box_secret_alloc.rs" test = false doc = false [[bin]] -name = "fuzz_vec_sodium_alloc" -path = "fuzz_targets/vec_sodium_alloc.rs" +name = "fuzz_vec_secret_alloc" +path = "fuzz_targets/vec_secret_alloc.rs" test = false doc = false diff --git a/fuzz/fuzz_targets/box_secret_alloc.rs b/fuzz/fuzz_targets/box_secret_alloc.rs new file mode 100644 index 00000000..872ef8fb --- /dev/null +++ b/fuzz/fuzz_targets/box_secret_alloc.rs @@ -0,0 +1,8 @@ +#![no_main] + +use libfuzzer_sys::fuzz_target; +use rosenpass_secret_memory::alloc::secret_box; + +fuzz_target!(|data: &[u8]| { + let _ = secret_box(data); +}); diff --git a/fuzz/fuzz_targets/box_sodium_alloc.rs b/fuzz/fuzz_targets/box_sodium_alloc.rs deleted file mode 100644 index e1b1799f..00000000 --- a/fuzz/fuzz_targets/box_sodium_alloc.rs +++ /dev/null @@ -1,12 +0,0 @@ -#![no_main] - -use libfuzzer_sys::fuzz_target; -use rosenpass_sodium::{ - alloc::{Alloc as SodiumAlloc, Box as SodiumBox}, - init, -}; - -fuzz_target!(|data: &[u8]| { - let _ = init(); - let _ = SodiumBox::new_in(data, SodiumAlloc::new()); -}); diff --git a/fuzz/fuzz_targets/vec_secret_alloc.rs b/fuzz/fuzz_targets/vec_secret_alloc.rs new file mode 100644 index 00000000..66b4ae2a --- /dev/null +++ b/fuzz/fuzz_targets/vec_secret_alloc.rs @@ -0,0 +1,9 @@ +#![no_main] + +use libfuzzer_sys::fuzz_target; +use rosenpass_secret_memory::alloc::secret_vec; + +fuzz_target!(|data: &[u8]| { + let mut vec = secret_vec(); + vec.extend_from_slice(data); +}); diff --git a/fuzz/fuzz_targets/vec_sodium_alloc.rs b/fuzz/fuzz_targets/vec_sodium_alloc.rs deleted file mode 100644 index a37368dc..00000000 --- a/fuzz/fuzz_targets/vec_sodium_alloc.rs +++ /dev/null @@ -1,13 +0,0 @@ -#![no_main] - -use libfuzzer_sys::fuzz_target; -use rosenpass_sodium::{ - alloc::{Alloc as SodiumAlloc, Vec as SodiumVec}, - init, -}; - -fuzz_target!(|data: &[u8]| { - let _ = init(); - let mut vec = SodiumVec::new_in(SodiumAlloc::new()); - vec.extend_from_slice(data); -}); diff --git a/secret-memory/Cargo.toml b/secret-memory/Cargo.toml index 45b5169a..ebb4d192 100644 --- a/secret-memory/Cargo.toml +++ b/secret-memory/Cargo.toml @@ -18,3 +18,9 @@ libsodium-sys-stable = { workspace = true } lazy_static = { workspace = true } zeroize = { workspace = true } rand = { workspace = true } +memsec = { workspace = true } +allocator-api2 = { workspace = true } +log = { workspace = true } + +[dev-dependencies] +allocator-api2-tests = { workspace = true } diff --git a/secret-memory/src/alloc/memsec.rs b/secret-memory/src/alloc/memsec.rs new file mode 100644 index 00000000..cfbf99a2 --- /dev/null +++ b/secret-memory/src/alloc/memsec.rs @@ -0,0 +1,108 @@ +use std::fmt; +use std::ptr::NonNull; + +use allocator_api2::alloc::{AllocError, Allocator, Layout}; + +#[derive(Copy, Clone, Default)] +struct MemsecAllocatorContents; + +/// Memory allocation using sodium_malloc/sodium_free +#[derive(Copy, Clone, Default)] +pub struct MemsecAllocator { + _dummy_private_data: MemsecAllocatorContents, +} + +/// A box backed by the memsec allocator +pub type MemsecBox = allocator_api2::boxed::Box; + +/// A vector backed by the memsec allocator +pub type MemsecVec = allocator_api2::vec::Vec; + +pub fn memsec_box(x: T) -> MemsecBox { + MemsecBox::::new_in(x, MemsecAllocator::new()) +} + +pub fn memsec_vec() -> MemsecVec { + MemsecVec::::new_in(MemsecAllocator::new()) +} + +impl MemsecAllocator { + pub fn new() -> Self { + Self { + _dummy_private_data: MemsecAllocatorContents, + } + } +} + +unsafe impl Allocator for MemsecAllocator { + fn allocate(&self, layout: Layout) -> Result, AllocError> { + // Call memsec allocator + let mem: Option> = unsafe { memsec::malloc_sized(layout.size()) }; + + // Unwrap the option + let Some(mem) = mem else { + log::error!("Allocation {layout:?} was requested but memsec returned a null pointer"); + return Err(AllocError); + }; + + // Ensure the right alignment is used + let off = (mem.as_ptr() as *const u8).align_offset(layout.align()); + if off != 0 { + log::error!("Allocation {layout:?} was requested but memsec returned allocation \ + with offset {off} from the requested alignment. Memsec always allocates values \ + at the end of a memory page for security reasons, custom alignments are not supported. \ + You could try allocating an oversized value."); + unsafe { memsec::free(mem) }; + return Err(AllocError); + }; + + Ok(mem) + } + + unsafe fn deallocate(&self, ptr: NonNull, _layout: Layout) { + unsafe { + memsec::free(ptr); + } + } +} + +impl fmt::Debug for MemsecAllocator { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.write_str("") + } +} + +#[cfg(test)] +mod test { + use allocator_api2_tests::make_test; + + use super::*; + + make_test! { test_sizes(MemsecAllocator::new()) } + make_test! { test_vec(MemsecAllocator::new()) } + make_test! { test_many_boxes(MemsecAllocator::new()) } + + #[test] + fn memsec_allocation() { + let alloc = MemsecAllocator::new(); + memsec_allocation_impl::<0>(&alloc); + memsec_allocation_impl::<7>(&alloc); + memsec_allocation_impl::<8>(&alloc); + memsec_allocation_impl::<64>(&alloc); + memsec_allocation_impl::<999>(&alloc); + } + + fn memsec_allocation_impl(alloc: &MemsecAllocator) { + let layout = Layout::new::<[u8; N]>(); + let mem = alloc.allocate(layout).unwrap(); + + // https://libsodium.gitbook.io/doc/memory_management#guarded-heap-allocations + // promises us that allocated memory is initialized with the magic byte 0xDB + // and memsec promises to provide a reimplementation of the libsodium mechanism; + // it uses the magic value 0xD0 though + assert_eq!(unsafe { mem.as_ref() }, &[0xD0u8; N]); + + let mem = NonNull::new(mem.as_ptr() as *mut u8).unwrap(); + unsafe { alloc.deallocate(mem, layout) }; + } +} diff --git a/secret-memory/src/alloc/mod.rs b/secret-memory/src/alloc/mod.rs new file mode 100644 index 00000000..e97fd8d5 --- /dev/null +++ b/secret-memory/src/alloc/mod.rs @@ -0,0 +1,6 @@ +pub mod memsec; + +pub use crate::alloc::memsec::{ + memsec_box as secret_box, memsec_vec as secret_vec, MemsecAllocator as SecretAllocator, + MemsecBox as SecretBox, MemsecVec as SecretVec, +}; diff --git a/secret-memory/src/lib.rs b/secret-memory/src/lib.rs index 75a6e723..b38ed756 100644 --- a/secret-memory/src/lib.rs +++ b/secret-memory/src/lib.rs @@ -2,6 +2,8 @@ pub mod debug; pub mod file; pub mod rand; +pub mod alloc; + mod public; pub use crate::public::Public; diff --git a/secret-memory/src/secret.rs b/secret-memory/src/secret.rs index 54cc5a4a..74764fed 100644 --- a/secret-memory/src/secret.rs +++ b/secret-memory/src/secret.rs @@ -1,15 +1,18 @@ -use crate::file::StoreSecret; +use std::{collections::HashMap, convert::TryInto, fmt, path::Path, sync::Mutex}; + 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 zeroize::{Zeroize, ZeroizeOnDrop}; + use rosenpass_util::{ b64::b64_reader, file::{fopen_r, LoadValue, LoadValueB64, ReadExactToEnd}, functional::mutating, }; -use std::{collections::HashMap, convert::TryInto, fmt, path::Path, sync::Mutex}; -use zeroize::{Zeroize, ZeroizeOnDrop}; + +use crate::alloc::{secret_box, SecretBox, SecretVec}; +use crate::file::StoreSecret; // 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 @@ -28,7 +31,7 @@ lazy_static! { /// [libsodium documentation](https://libsodium.gitbook.io/doc/memory_management#guarded-heap-allocations) #[derive(Debug)] // TODO check on Debug derive, is that clever struct SecretMemoryPool { - pool: HashMap>>, + pool: HashMap>>, } impl SecretMemoryPool { @@ -41,13 +44,13 @@ impl SecretMemoryPool { } /// Return secret back to the pool for future re-use - pub fn release(&mut self, mut sec: SodiumBox<[u8; N]>) { + pub fn release(&mut self, mut sec: SecretBox<[u8; N]>) { sec.zeroize(); // This conversion sequence is weird but at least it guarantees // that the heap allocation is preserved according to the docs - let sec: SodiumVec = sec.into(); - let sec: SodiumBox<[u8]> = sec.into(); + let sec: SecretVec = sec.into(); + let sec: SecretBox<[u8]> = sec.into(); self.pool.entry(N).or_default().push(sec); } @@ -56,10 +59,10 @@ impl SecretMemoryPool { /// chunk is found in the inventory. /// /// The secret is guaranteed to be full of nullbytes - pub fn take(&mut self) -> SodiumBox<[u8; N]> { + pub fn take(&mut self) -> SecretBox<[u8; N]> { let entry = self.pool.entry(N).or_default(); match entry.pop() { - None => SodiumBox::new_in([0u8; N], SodiumAlloc::default()), + None => secret_box([0u8; N]), Some(sec) => sec.try_into().unwrap(), } } @@ -67,7 +70,7 @@ impl SecretMemoryPool { /// Storeage for a secret backed by [rosenpass_sodium::alloc::Alloc] pub struct Secret { - storage: Option>, + storage: Option>, } impl Secret { @@ -200,7 +203,7 @@ mod test { rosenpass_sodium::init().unwrap(); const N: usize = 0x100; let mut pool = SecretMemoryPool::new(); - let secret: SodiumBox<[u8; N]> = pool.take(); + let secret: SecretBox<[u8; N]> = pool.take(); assert_eq!(secret.as_ref(), &[0; N]); } @@ -210,7 +213,7 @@ mod test { rosenpass_sodium::init().unwrap(); const N: usize = 0x100; let mut pool = SecretMemoryPool::new(); - let secret: SodiumBox<[u8; N]> = pool.take(); + let secret: SecretBox<[u8; N]> = pool.take(); std::mem::drop(pool); assert_eq!(secret.as_ref(), &[0; N]); } @@ -221,14 +224,14 @@ mod test { rosenpass_sodium::init().unwrap(); const N: usize = 1; let mut pool = SecretMemoryPool::new(); - let mut secret: SodiumBox<[u8; N]> = pool.take(); + let mut secret: SecretBox<[u8; N]> = pool.take(); let old_secret_ptr = secret.as_ref().as_ptr(); secret.as_mut()[0] = 0x13; pool.release(secret); // now check that we get the same ptr - let new_secret: SodiumBox<[u8; N]> = pool.take(); + let new_secret: SecretBox<[u8; N]> = pool.take(); assert_eq!(old_secret_ptr, new_secret.as_ref().as_ptr()); // and that the secret was zeroized diff --git a/sodium/src/alloc/allocator.rs b/sodium/src/alloc/allocator.rs deleted file mode 100644 index ad1314c9..00000000 --- a/sodium/src/alloc/allocator.rs +++ /dev/null @@ -1,95 +0,0 @@ -use allocator_api2::alloc::{AllocError, Allocator, Layout}; -use libsodium_sys as libsodium; -use std::fmt; -use std::os::raw::c_void; -use std::ptr::NonNull; - -#[derive(Clone, Default)] -struct AllocatorContents; - -/// Memory allocation using sodium_malloc/sodium_free -#[derive(Clone, Default)] -pub struct Alloc { - _dummy_private_data: AllocatorContents, -} - -impl Alloc { - pub fn new() -> Self { - Alloc { - _dummy_private_data: AllocatorContents, - } - } -} - -unsafe impl Allocator for Alloc { - fn allocate(&self, layout: Layout) -> Result, AllocError> { - // Call sodium allocator - let ptr = unsafe { libsodium::sodium_malloc(layout.size()) }; - - // Ensure the right allocation is used - let off = ptr.align_offset(layout.align()); - if off != 0 { - log::error!("Allocation {layout:?} was requested but libsodium returned allocation \ - with offset {off} from the requested alignment. Libsodium always allocates values \ - at the end of a memory page for security reasons, custom alignments are not supported. \ - You could try allocating an oversized value."); - return Err(AllocError); - } - - // Convert to a pointer size - let ptr = core::ptr::slice_from_raw_parts_mut(ptr as *mut u8, layout.size()); - - // Conversion to a *const u8, then to a &[u8] - match NonNull::new(ptr) { - None => { - log::error!( - "Allocation {layout:?} was requested but libsodium returned a null pointer" - ); - Err(AllocError) - } - Some(ret) => Ok(ret), - } - } - - unsafe fn deallocate(&self, ptr: NonNull, _layout: Layout) { - unsafe { - libsodium::sodium_free(ptr.as_ptr() as *mut c_void); - } - } -} - -impl fmt::Debug for Alloc { - fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - fmt.write_str("") - } -} - -#[cfg(test)] -mod test { - use super::*; - - /// checks that the can malloc with libsodium - #[test] - fn sodium_allocation() { - crate::init().unwrap(); - let alloc = Alloc::new(); - sodium_allocation_impl::<0>(&alloc); - sodium_allocation_impl::<7>(&alloc); - sodium_allocation_impl::<8>(&alloc); - sodium_allocation_impl::<64>(&alloc); - sodium_allocation_impl::<999>(&alloc); - } - - fn sodium_allocation_impl(alloc: &Alloc) { - crate::init().unwrap(); - let layout = Layout::new::<[u8; N]>(); - let mem = alloc.allocate(layout).unwrap(); - - // https://libsodium.gitbook.io/doc/memory_management#guarded-heap-allocations - // promises us that allocated memory is initialized with the magic byte 0xDB - assert_eq!(unsafe { mem.as_ref() }, &[0xDBu8; N]); - - let mem = NonNull::new(mem.as_ptr() as *mut u8).unwrap(); - unsafe { alloc.deallocate(mem, layout) }; - } -} diff --git a/sodium/src/alloc/mod.rs b/sodium/src/alloc/mod.rs deleted file mode 100644 index 0dceb01c..00000000 --- a/sodium/src/alloc/mod.rs +++ /dev/null @@ -1,10 +0,0 @@ -//! Access to sodium_malloc/sodium_free - -mod allocator; -pub use allocator::Alloc; - -/// A box backed by sodium_malloc -pub type Box = allocator_api2::boxed::Box; - -/// A vector backed by sodium_malloc -pub type Vec = allocator_api2::vec::Vec; diff --git a/sodium/src/lib.rs b/sodium/src/lib.rs index 752e1a7f..606bbd0c 100644 --- a/sodium/src/lib.rs +++ b/sodium/src/lib.rs @@ -16,6 +16,5 @@ pub fn init() -> anyhow::Result<()> { } pub mod aead; -pub mod alloc; pub mod hash; pub mod helpers; From e3b72487db6f9299dee1d4e0b11e77658fd867b1 Mon Sep 17 00:00:00 2001 From: Karolin Varner Date: Tue, 2 Jan 2024 19:51:20 +0100 Subject: [PATCH 08/13] fix: Make sure all tests are run during CI runs Had to fix the tests in util/src/result.rs. --- .github/workflows/qc.yaml | 4 ++-- util/src/result.rs | 32 +++++++++++++++++++------------- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/.github/workflows/qc.yaml b/.github/workflows/qc.yaml index 903282e6..aa16d381 100644 --- a/.github/workflows/qc.yaml +++ b/.github/workflows/qc.yaml @@ -101,7 +101,7 @@ jobs: # liboqs requires quite a lot of stack memory, thus we adjust # the default stack size picked for new threads (which is used # by `cargo test`) to be _big enough_. Setting it to 8 MiB - - run: RUST_MIN_STACK=8388608 cargo test + - run: RUST_MIN_STACK=8388608 cargo test --workspace cargo-test-nix-devshell-x86_64-linux: runs-on: @@ -124,7 +124,7 @@ jobs: with: name: rosenpass authToken: ${{ secrets.CACHIX_AUTH_TOKEN }} - - run: nix develop --command cargo test + - run: nix develop --command cargo test --workspace cargo-fuzz: runs-on: ubuntu-latest diff --git a/util/src/result.rs b/util/src/result.rs index 47d1a5ca..8f0f258f 100644 --- a/util/src/result.rs +++ b/util/src/result.rs @@ -35,25 +35,30 @@ pub trait GuaranteedValue { /// ``` /// use std::num::Wrapping; /// use std::result::Result; -/// use std::convert::Infallible +/// use std::convert::Infallible; +/// use std::ops::Add; /// -/// trait FailableAddition { +/// use rosenpass_util::result::{Guaranteed, GuaranteedValue}; +/// +/// trait FailableAddition: Sized { /// type Error; /// fn failable_addition(&self, other: &Self) -> Result; /// } /// +/// #[derive(Copy, Clone, Debug, Eq, PartialEq)] /// struct OverflowError; /// -/// impl FailableAddition for Wrapping { +/// impl FailableAddition for Wrapping +/// where for <'a> &'a Wrapping: Add> { /// type Error = Infallible; /// fn failable_addition(&self, other: &Self) -> Guaranteed { -/// self + other +/// Ok(self + other) /// } /// } /// -/// impl FailableAddition for u32 { -/// type Error = Infallible; -/// fn failable_addition(&self, other: &Self) -> Guaranteed { +/// impl FailableAddition for u32 { +/// type Error = OverflowError; +/// fn failable_addition(&self, other: &Self) -> Result { /// match self.checked_add(*other) { /// Some(v) => Ok(v), /// None => Err(OverflowError), @@ -64,10 +69,11 @@ pub trait GuaranteedValue { /// fn failable_multiply(a: &T, b: u32) /// -> Result /// where -/// T: FailableAddition { +/// T: FailableAddition { +/// assert!(b >= 2); // Acceptable only because this is for demonstration purposes /// let mut accu = a.failable_addition(a)?; -/// for _ in ..(b-1) { -/// accu.failable_addition(a)?; +/// for _ in 2..b { +/// accu = accu.failable_addition(a)?; /// } /// Ok(accu) /// } @@ -75,12 +81,12 @@ pub trait GuaranteedValue { /// // We can use .guaranteed() with Wrapping, since the operation uses /// // the Infallible error type. /// // We can also use unwrap which just happens to not raise an error. -/// assert_eq!(failable_multiply(&Wrapping::new(42u32), 3).guaranteed(), 126); -/// assert_eq!(failable_multiply(&Wrapping::new(42u32), 3).unwrap(), 126); +/// assert_eq!(failable_multiply(&Wrapping(42u32), 3).guaranteed(), Wrapping(126)); +/// assert_eq!(failable_multiply(&Wrapping(42u32), 3).unwrap(), Wrapping(126)); /// /// // We can not use .guaranteed() with u32, since there can be an error. /// // We can however use unwrap(), which may panic -/// assert_eq!(failable_multiply(&42u32, 3).guaranteed(), 126); // COMPILER ERROR +/// //assert_eq!(failable_multiply(&42u32, 3).guaranteed(), 126); // COMPILER ERROR /// assert_eq!(failable_multiply(&42u32, 3).unwrap(), 126); /// ``` pub type Guaranteed = Result; From 9824db4f0917a3a88b4e1c742a5b09a7dd0116e0 Mon Sep 17 00:00:00 2001 From: Karolin Varner Date: Tue, 2 Jan 2024 19:57:51 +0100 Subject: [PATCH 09/13] fix: Migrate away from lazy_static in favor of thread_local MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The new secret memory pool was causing CI failures in the fuzzing code, due to the fuzzer compiling its binaries with memory sanitizer support. https://doc.rust-lang.org/beta/unstable-book/compiler-flags/sanitizer.html Using lazy_static was – intentionally – introducing a memory leak, but the LeakSanitizer detected this and raised an error. Now by using thread_local we are calling the destructors and so – while still being a memory leak in practice – the LeakSanitizer no longer detects this behaviour as an error. Alternatively we could have used a known-leaks list with the leak-sanitizer, but this would have increased the complexity of the build setup. Finally, this was likely triggered with the migration to memsec, because libsodium circumvents the malloc/free calls, relying on direct calls to MMAP. --- Cargo.lock | 140 ++++++++++++++++++++++++++++++--- Cargo.toml | 1 - secret-memory/Cargo.toml | 1 - secret-memory/src/secret.rs | 151 ++++++++++++++++++++++++++++-------- 4 files changed, 248 insertions(+), 45 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 918c4cde..28b7eaba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -188,7 +188,7 @@ dependencies = [ "regex", "rustc-hash", "shlex", - "syn", + "syn 2.0.39", "which", ] @@ -256,6 +256,12 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" +[[package]] +name = "cfg_aliases" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" + [[package]] name = "ciborium" version = "0.2.1" @@ -337,7 +343,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn", + "syn 2.0.39", ] [[package]] @@ -471,7 +477,7 @@ checksum = "67e77553c4162a157adbf834ebae5b415acbecbeafc7a74b0e886657506a7611" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.39", ] [[package]] @@ -523,7 +529,7 @@ checksum = "d4029edd3e734da6fe05b6cd7bd2960760a616bd2ddd0d59a0124746d6272af0" dependencies = [ "cfg-if", "libc", - "redox_syscall", + "redox_syscall 0.3.5", "windows-sys 0.48.0", ] @@ -662,6 +668,15 @@ dependencies = [ "hashbrown 0.14.3", ] +[[package]] +name = "instant" +version = "0.1.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7a5bbe824c507c5da5956355e86a746d82e0e1464f65d862cc5e71da70e94b2c" +dependencies = [ + "cfg-if", +] + [[package]] name = "is-terminal" version = "0.4.9" @@ -792,6 +807,16 @@ version = "0.4.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4cd1a83af159aa67994778be9070f0ae1bd732942279cabb14f86f986a21456" +[[package]] +name = "lock_api" +version = "0.4.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3c168f8615b12bc01f9c17e2eb0cc07dcae1940121185446edc3744920e8ef45" +dependencies = [ + "autocfg", + "scopeguard", +] + [[package]] name = "log" version = "0.4.20" @@ -915,6 +940,31 @@ version = "6.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e2355d85b9a3786f481747ced0e0ff2ba35213a1f9bd406ed906554d7af805a1" +[[package]] +name = "parking_lot" +version = "0.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7d17b78036a60663b797adeaee46f5c9dfebb86948d1255007a1d6be0271ff99" +dependencies = [ + "instant", + "lock_api", + "parking_lot_core", +] + +[[package]] +name = "parking_lot_core" +version = "0.8.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "60a2cfe6f0ad2bfc16aefa463b497d5c7a5ecd44a23efa72aa342d90177356dc" +dependencies = [ + "cfg-if", + "instant", + "libc", + "redox_syscall 0.2.16", + "smallvec", + "winapi", +] + [[package]] name = "paste" version = "1.0.14" @@ -980,7 +1030,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ae005bd773ab59b4725093fd7df83fd7892f7d8eafb48dbd7de6e024e4215f9d" dependencies = [ "proc-macro2", - "syn", + "syn 2.0.39", ] [[package]] @@ -1060,6 +1110,15 @@ dependencies = [ "crossbeam-utils", ] +[[package]] +name = "redox_syscall" +version = "0.2.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fb5a58c1855b4b6819d59012155603f0b22ad30cad752600aadfcb695265519a" +dependencies = [ + "bitflags 1.3.2", +] + [[package]] name = "redox_syscall" version = "0.3.5" @@ -1213,7 +1272,6 @@ dependencies = [ "allocator-api2", "allocator-api2-tests", "anyhow", - "lazy_static", "libsodium-sys-stable", "log", "memsec", @@ -1221,6 +1279,7 @@ dependencies = [ "rosenpass-sodium", "rosenpass-to", "rosenpass-util", + "static_init", "zeroize", ] @@ -1346,7 +1405,7 @@ checksum = "43576ca501357b9b071ac53cdc7da8ef0cbd9493d8df094cd821777ea6e894d3" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.39", ] [[package]] @@ -1375,6 +1434,12 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a7cee0529a6d40f580e7a5e6c495c8fbfe21b7b52795ed4bb5e62cdf92bc6380" +[[package]] +name = "smallvec" +version = "1.11.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4dccd0940a2dcdf68d092b8cbab7dc0ad8fa938bf95787e1b916b0e3d0e8e970" + [[package]] name = "spin" version = "0.9.8" @@ -1400,12 +1465,51 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" +[[package]] +name = "static_init" +version = "1.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8a2a1c578e98c1c16fc3b8ec1328f7659a500737d7a0c6d625e73e830ff9c1f6" +dependencies = [ + "bitflags 1.3.2", + "cfg_aliases", + "libc", + "parking_lot", + "parking_lot_core", + "static_init_macro", + "winapi", +] + +[[package]] +name = "static_init_macro" +version = "1.0.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "70a2595fc3aa78f2d0e45dd425b22282dd863273761cc77780914b2cf3003acf" +dependencies = [ + "cfg_aliases", + "memchr", + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "strsim" version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "syn" +version = "1.0.109" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237" +dependencies = [ + "proc-macro2", + "quote", + "unicode-ident", +] + [[package]] name = "syn" version = "2.0.39" @@ -1466,7 +1570,7 @@ checksum = "266b2e40bc00e5a6c09c3584011e08b06f123c00362c92b975ba9843aaaa14b8" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.39", ] [[package]] @@ -1636,7 +1740,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn", + "syn 2.0.39", "wasm-bindgen-shared", ] @@ -1658,7 +1762,7 @@ checksum = "c5353b8dab669f5e10f5bd76df26a9360c748f054f862ff5f3f8aae0c7fb3907" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.39", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -1961,7 +2065,7 @@ checksum = "c2f140bda219a26ccc0cdb03dba58af72590c53b22642577d88a927bc5c87d6b" dependencies = [ "proc-macro2", "quote", - "syn", + "syn 2.0.39", ] [[package]] @@ -1969,6 +2073,20 @@ name = "zeroize" version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" +dependencies = [ + "zeroize_derive", +] + +[[package]] +name = "zeroize_derive" +version = "1.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69" +dependencies = [ + "proc-macro2", + "quote", + "syn 2.0.39", +] [[package]] name = "zip" diff --git a/Cargo.toml b/Cargo.toml index 96337f8c..43a401bb 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,7 +42,6 @@ doc-comment = "0.3.3" base64 = "0.21.5" zeroize = "1.7.0" memoffset = "0.9.0" -lazy_static = "1.4.0" thiserror = "1.0.50" paste = "1.0.14" env_logger = "0.10.1" diff --git a/secret-memory/Cargo.toml b/secret-memory/Cargo.toml index ebb4d192..3f56f3d1 100644 --- a/secret-memory/Cargo.toml +++ b/secret-memory/Cargo.toml @@ -15,7 +15,6 @@ rosenpass-to = { workspace = true } rosenpass-sodium = { workspace = true } rosenpass-util = { workspace = true } libsodium-sys-stable = { workspace = true } -lazy_static = { workspace = true } zeroize = { workspace = true } rand = { workspace = true } memsec = { workspace = true } diff --git a/secret-memory/src/secret.rs b/secret-memory/src/secret.rs index 74764fed..ab859c33 100644 --- a/secret-memory/src/secret.rs +++ b/secret-memory/src/secret.rs @@ -1,15 +1,17 @@ -use std::{collections::HashMap, convert::TryInto, fmt, path::Path, sync::Mutex}; +use std::cell::RefCell; +use std::collections::HashMap; +use std::convert::TryInto; +use std::fmt; +use std::ops::{Deref, DerefMut}; +use std::path::Path; use anyhow::Context; -use lazy_static::lazy_static; use rand::{Fill as Randomize, Rng}; use zeroize::{Zeroize, ZeroizeOnDrop}; -use rosenpass_util::{ - b64::b64_reader, - file::{fopen_r, LoadValue, LoadValueB64, ReadExactToEnd}, - functional::mutating, -}; +use rosenpass_util::b64::b64_reader; +use rosenpass_util::file::{fopen_r, LoadValue, LoadValueB64, ReadExactToEnd}; +use rosenpass_util::functional::mutating; use crate::alloc::{secret_box, SecretBox, SecretVec}; use crate::file::StoreSecret; @@ -17,8 +19,78 @@ use crate::file::StoreSecret; // 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 // be reused… -lazy_static! { - static ref SECRET_CACHE: Mutex = Mutex::new(SecretMemoryPool::new()); +thread_local! { + static SECRET_CACHE: RefCell = RefCell::new(SecretMemoryPool::new()); +} + +fn with_secret_memory_pool(mut f: Fn) -> R +where + Fn: FnMut(Option<&mut SecretMemoryPool>) -> R, +{ + // This acquires the SECRET_CACHE + SECRET_CACHE + .try_with(|cell| { + // And acquires the inner reference + cell.try_borrow_mut() + .as_deref_mut() + // To call the given function + .map(|pool| f(Some(pool))) + .ok() + }) + .ok() + .flatten() + // Failing that, the given function is called with None + .unwrap_or_else(|| f(None)) +} + +// Wrapper around SecretBox that applies automatic zeroization +#[derive(Debug)] +struct ZeroizingSecretBox(Option>); + +impl ZeroizingSecretBox { + fn new(boxed: T) -> Self { + ZeroizingSecretBox(Some(secret_box(boxed))) + } +} + +impl ZeroizingSecretBox { + fn from_secret_box(inner: SecretBox) -> Self { + Self(Some(inner)) + } + + fn take(mut self) -> SecretBox { + self.0.take().unwrap() + } +} + +impl ZeroizeOnDrop for ZeroizingSecretBox {} +impl Zeroize for ZeroizingSecretBox { + fn zeroize(&mut self) { + if let Some(inner) = &mut self.0 { + let inner: &mut SecretBox = inner; // type annotation + inner.zeroize() + } + } +} + +impl Drop for ZeroizingSecretBox { + fn drop(&mut self) { + self.zeroize() + } +} + +impl Deref for ZeroizingSecretBox { + type Target = T; + + fn deref(&self) -> &T { + &self.0.as_ref().unwrap() + } +} + +impl DerefMut for ZeroizingSecretBox { + fn deref_mut(&mut self) -> &mut T { + self.0.as_mut().unwrap() + } } /// Pool that stores secret memory allocations @@ -31,7 +103,7 @@ lazy_static! { /// [libsodium documentation](https://libsodium.gitbook.io/doc/memory_management#guarded-heap-allocations) #[derive(Debug)] // TODO check on Debug derive, is that clever struct SecretMemoryPool { - pool: HashMap>>, + pool: HashMap>>, } impl SecretMemoryPool { @@ -44,33 +116,37 @@ impl SecretMemoryPool { } /// Return secret back to the pool for future re-use - pub fn release(&mut self, mut sec: SecretBox<[u8; N]>) { + pub fn release(&mut self, mut sec: ZeroizingSecretBox<[u8; N]>) { sec.zeroize(); // This conversion sequence is weird but at least it guarantees // that the heap allocation is preserved according to the docs - let sec: SecretVec = sec.into(); + let sec: SecretVec = sec.take().into(); let sec: SecretBox<[u8]> = sec.into(); - self.pool.entry(N).or_default().push(sec); + self.pool + .entry(N) + .or_default() + .push(ZeroizingSecretBox::from_secret_box(sec)); } /// Take protected memory from the pool, allocating new one if no suitable /// chunk is found in the inventory. /// /// The secret is guaranteed to be full of nullbytes - pub fn take(&mut self) -> SecretBox<[u8; N]> { + pub fn take(&mut self) -> ZeroizingSecretBox<[u8; N]> { let entry = self.pool.entry(N).or_default(); - match entry.pop() { + let inner = match entry.pop() { None => secret_box([0u8; N]), - Some(sec) => sec.try_into().unwrap(), - } + Some(sec) => sec.take().try_into().unwrap(), + }; + ZeroizingSecretBox::from_secret_box(inner) } } /// Storeage for a secret backed by [rosenpass_sodium::alloc::Alloc] pub struct Secret { - storage: Option>, + storage: Option>, } impl Secret { @@ -84,9 +160,12 @@ impl Secret { pub fn zero() -> Self { // Using [SecretMemoryPool] here because this operation is expensive, // yet it is used in hot loops - Self { - storage: Some(SECRET_CACHE.lock().unwrap().take()), - } + let buf = with_secret_memory_pool(|pool| { + pool.map(|p| p.take()) + .unwrap_or_else(|| ZeroizingSecretBox::new([0u8; N])) + }); + + Self { storage: Some(buf) } } /// Returns a new [Secret] that is randomized @@ -101,7 +180,7 @@ impl Secret { /// Borrows the data pub fn secret(&self) -> &[u8; N] { - self.storage.as_ref().unwrap() + &self.storage.as_ref().unwrap() } /// Borrows the data mutably @@ -110,13 +189,6 @@ impl Secret { } } -impl ZeroizeOnDrop for Secret {} -impl Zeroize for Secret { - fn zeroize(&mut self) { - self.secret_mut().zeroize(); - } -} - 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 @@ -127,11 +199,26 @@ impl Randomize for Secret { } } +impl ZeroizeOnDrop for Secret {} +impl Zeroize for Secret { + fn zeroize(&mut self) { + if let Some(inner) = &mut self.storage { + inner.zeroize() + } + } +} + impl Drop for Secret { fn drop(&mut self) { - self.storage - .take() - .map(|sec| SECRET_CACHE.lock().unwrap().release(sec)); + with_secret_memory_pool(|pool| { + if let Some((pool, secret)) = pool.zip(self.storage.take()) { + pool.release(secret); + } + }); + + // This should be unnecessary: The pool has one item – the inner secret – which + // zeroizes itself on drop. Calling it should not do any harm though… + self.zeroize() } } From 30cb0e98018d0febb7921ecd58e197f0ada96004 Mon Sep 17 00:00:00 2001 From: Karolin Varner Date: Tue, 2 Jan 2024 20:02:54 +0100 Subject: [PATCH 10/13] chore: Remove references to libsodium from secret-memory --- Cargo.lock | 139 +++--------------------------- secret-memory/src/alloc/memsec.rs | 2 +- secret-memory/src/secret.rs | 16 ++-- 3 files changed, 16 insertions(+), 141 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 28b7eaba..d2818082 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -188,7 +188,7 @@ dependencies = [ "regex", "rustc-hash", "shlex", - "syn 2.0.39", + "syn", "which", ] @@ -256,12 +256,6 @@ version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "baf1de4339761588bc0619e3cbc0120ee582ebb74b53b4efbf79117bd2da40fd" -[[package]] -name = "cfg_aliases" -version = "0.1.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fd16c4719339c4530435d38e511904438d07cce7950afa3718a84ac36c10e89e" - [[package]] name = "ciborium" version = "0.2.1" @@ -343,7 +337,7 @@ dependencies = [ "heck", "proc-macro2", "quote", - "syn 2.0.39", + "syn", ] [[package]] @@ -477,7 +471,7 @@ checksum = "67e77553c4162a157adbf834ebae5b415acbecbeafc7a74b0e886657506a7611" dependencies = [ "proc-macro2", "quote", - "syn 2.0.39", + "syn", ] [[package]] @@ -529,7 +523,7 @@ checksum = "d4029edd3e734da6fe05b6cd7bd2960760a616bd2ddd0d59a0124746d6272af0" dependencies = [ "cfg-if", "libc", - "redox_syscall 0.3.5", + "redox_syscall", "windows-sys 0.48.0", ] @@ -668,15 +662,6 @@ dependencies = [ "hashbrown 0.14.3", ] -[[package]] -name = "instant" -version = "0.1.12" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7a5bbe824c507c5da5956355e86a746d82e0e1464f65d862cc5e71da70e94b2c" -dependencies = [ - "cfg-if", -] - [[package]] name = "is-terminal" version = "0.4.9" @@ -807,16 +792,6 @@ version = "0.4.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c4cd1a83af159aa67994778be9070f0ae1bd732942279cabb14f86f986a21456" -[[package]] -name = "lock_api" -version = "0.4.11" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3c168f8615b12bc01f9c17e2eb0cc07dcae1940121185446edc3744920e8ef45" -dependencies = [ - "autocfg", - "scopeguard", -] - [[package]] name = "log" version = "0.4.20" @@ -940,31 +915,6 @@ version = "6.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e2355d85b9a3786f481747ced0e0ff2ba35213a1f9bd406ed906554d7af805a1" -[[package]] -name = "parking_lot" -version = "0.11.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7d17b78036a60663b797adeaee46f5c9dfebb86948d1255007a1d6be0271ff99" -dependencies = [ - "instant", - "lock_api", - "parking_lot_core", -] - -[[package]] -name = "parking_lot_core" -version = "0.8.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "60a2cfe6f0ad2bfc16aefa463b497d5c7a5ecd44a23efa72aa342d90177356dc" -dependencies = [ - "cfg-if", - "instant", - "libc", - "redox_syscall 0.2.16", - "smallvec", - "winapi", -] - [[package]] name = "paste" version = "1.0.14" @@ -1030,7 +980,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "ae005bd773ab59b4725093fd7df83fd7892f7d8eafb48dbd7de6e024e4215f9d" dependencies = [ "proc-macro2", - "syn 2.0.39", + "syn", ] [[package]] @@ -1110,15 +1060,6 @@ dependencies = [ "crossbeam-utils", ] -[[package]] -name = "redox_syscall" -version = "0.2.16" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "fb5a58c1855b4b6819d59012155603f0b22ad30cad752600aadfcb695265519a" -dependencies = [ - "bitflags 1.3.2", -] - [[package]] name = "redox_syscall" version = "0.3.5" @@ -1279,7 +1220,6 @@ dependencies = [ "rosenpass-sodium", "rosenpass-to", "rosenpass-util", - "static_init", "zeroize", ] @@ -1405,7 +1345,7 @@ checksum = "43576ca501357b9b071ac53cdc7da8ef0cbd9493d8df094cd821777ea6e894d3" dependencies = [ "proc-macro2", "quote", - "syn 2.0.39", + "syn", ] [[package]] @@ -1434,12 +1374,6 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a7cee0529a6d40f580e7a5e6c495c8fbfe21b7b52795ed4bb5e62cdf92bc6380" -[[package]] -name = "smallvec" -version = "1.11.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4dccd0940a2dcdf68d092b8cbab7dc0ad8fa938bf95787e1b916b0e3d0e8e970" - [[package]] name = "spin" version = "0.9.8" @@ -1465,51 +1399,12 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a2eb9349b6444b326872e140eb1cf5e7c522154d69e7a0ffb0fb81c06b37543f" -[[package]] -name = "static_init" -version = "1.0.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "8a2a1c578e98c1c16fc3b8ec1328f7659a500737d7a0c6d625e73e830ff9c1f6" -dependencies = [ - "bitflags 1.3.2", - "cfg_aliases", - "libc", - "parking_lot", - "parking_lot_core", - "static_init_macro", - "winapi", -] - -[[package]] -name = "static_init_macro" -version = "1.0.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "70a2595fc3aa78f2d0e45dd425b22282dd863273761cc77780914b2cf3003acf" -dependencies = [ - "cfg_aliases", - "memchr", - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "strsim" version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" -[[package]] -name = "syn" -version = "1.0.109" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72b64191b275b66ffe2469e8af2c1cfe3bafa67b529ead792a6d0160888b4237" -dependencies = [ - "proc-macro2", - "quote", - "unicode-ident", -] - [[package]] name = "syn" version = "2.0.39" @@ -1570,7 +1465,7 @@ checksum = "266b2e40bc00e5a6c09c3584011e08b06f123c00362c92b975ba9843aaaa14b8" dependencies = [ "proc-macro2", "quote", - "syn 2.0.39", + "syn", ] [[package]] @@ -1740,7 +1635,7 @@ dependencies = [ "once_cell", "proc-macro2", "quote", - "syn 2.0.39", + "syn", "wasm-bindgen-shared", ] @@ -1762,7 +1657,7 @@ checksum = "c5353b8dab669f5e10f5bd76df26a9360c748f054f862ff5f3f8aae0c7fb3907" dependencies = [ "proc-macro2", "quote", - "syn 2.0.39", + "syn", "wasm-bindgen-backend", "wasm-bindgen-shared", ] @@ -2065,7 +1960,7 @@ checksum = "c2f140bda219a26ccc0cdb03dba58af72590c53b22642577d88a927bc5c87d6b" dependencies = [ "proc-macro2", "quote", - "syn 2.0.39", + "syn", ] [[package]] @@ -2073,20 +1968,6 @@ name = "zeroize" version = "1.7.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "525b4ec142c6b68a2d10f01f7bbf6755599ca3f81ea53b8431b7dd348f5fdb2d" -dependencies = [ - "zeroize_derive", -] - -[[package]] -name = "zeroize_derive" -version = "1.4.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ce36e65b0d2999d2aafac989fb249189a141aee1f53c612c1f37d72631959f69" -dependencies = [ - "proc-macro2", - "quote", - "syn 2.0.39", -] [[package]] name = "zip" diff --git a/secret-memory/src/alloc/memsec.rs b/secret-memory/src/alloc/memsec.rs index cfbf99a2..eec48971 100644 --- a/secret-memory/src/alloc/memsec.rs +++ b/secret-memory/src/alloc/memsec.rs @@ -6,7 +6,7 @@ use allocator_api2::alloc::{AllocError, Allocator, Layout}; #[derive(Copy, Clone, Default)] struct MemsecAllocatorContents; -/// Memory allocation using sodium_malloc/sodium_free +/// Memory allocation using using the memsec crate #[derive(Copy, Clone, Default)] pub struct MemsecAllocator { _dummy_private_data: MemsecAllocatorContents, diff --git a/secret-memory/src/secret.rs b/secret-memory/src/secret.rs index ab859c33..bbaf090c 100644 --- a/secret-memory/src/secret.rs +++ b/secret-memory/src/secret.rs @@ -98,9 +98,6 @@ impl DerefMut for ZeroizingSecretBox { /// Allocation of secret memory is expensive. Thus, this struct provides a /// pool of secret memory, readily available to yield protected, slices of /// memory. -/// -/// Further information about the protection in place can be found in in the -/// [libsodium documentation](https://libsodium.gitbook.io/doc/memory_management#guarded-heap-allocations) #[derive(Debug)] // TODO check on Debug derive, is that clever struct SecretMemoryPool { pool: HashMap>>, @@ -144,7 +141,7 @@ impl SecretMemoryPool { } } -/// Storeage for a secret backed by [rosenpass_sodium::alloc::Alloc] +/// Storage for secret data pub struct Secret { storage: Option>, } @@ -287,20 +284,18 @@ mod test { /// check that we can alloc using the magic pool #[test] fn secret_memory_pool_take() { - rosenpass_sodium::init().unwrap(); const N: usize = 0x100; let mut pool = SecretMemoryPool::new(); - let secret: SecretBox<[u8; N]> = pool.take(); + let secret: ZeroizingSecretBox<[u8; N]> = pool.take(); assert_eq!(secret.as_ref(), &[0; N]); } /// check that a secrete lives, even if its [SecretMemoryPool] is deleted #[test] fn secret_memory_pool_drop() { - rosenpass_sodium::init().unwrap(); const N: usize = 0x100; let mut pool = SecretMemoryPool::new(); - let secret: SecretBox<[u8; N]> = pool.take(); + let secret: ZeroizingSecretBox<[u8; N]> = pool.take(); std::mem::drop(pool); assert_eq!(secret.as_ref(), &[0; N]); } @@ -308,17 +303,16 @@ mod test { /// check that a secrete can be reborn, freshly initialized with zero #[test] fn secret_memory_pool_release() { - rosenpass_sodium::init().unwrap(); const N: usize = 1; let mut pool = SecretMemoryPool::new(); - let mut secret: SecretBox<[u8; N]> = pool.take(); + let mut secret: ZeroizingSecretBox<[u8; N]> = pool.take(); let old_secret_ptr = secret.as_ref().as_ptr(); secret.as_mut()[0] = 0x13; pool.release(secret); // now check that we get the same ptr - let new_secret: SecretBox<[u8; N]> = pool.take(); + let new_secret: ZeroizingSecretBox<[u8; N]> = pool.take(); assert_eq!(old_secret_ptr, new_secret.as_ref().as_ptr()); // and that the secret was zeroized From 1c14be38ddd71cde9dcfcd9b52ba7cee060d6b58 Mon Sep 17 00:00:00 2001 From: wucke13 Date: Wed, 3 Jan 2024 18:20:33 +0100 Subject: [PATCH 11/13] fix: make benches work again Somehow in the past while splitting into many crates, we broke the bench setup. This commit both fixes it, and adds a CI job that ensures it is still working to avoid such silent failure in the future. The benchmarks are not actually run, they would take forever on the slow GitHub Actions runners, but they are at least compiled. --- .github/workflows/qc.yaml | 20 ++++++++++++++++++++ rosenpass/benches/handshake.rs | 14 ++++++-------- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/.github/workflows/qc.yaml b/.github/workflows/qc.yaml index aa16d381..2f9a0db3 100644 --- a/.github/workflows/qc.yaml +++ b/.github/workflows/qc.yaml @@ -33,6 +33,26 @@ jobs: - name: Run Rust Formatting Script run: bash format_rust_code.sh --mode check + cargo-bench: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v3 + - uses: actions/cache@v3 + with: + path: | + ~/.cargo/bin/ + ~/.cargo/registry/index/ + ~/.cargo/registry/cache/ + ~/.cargo/git/db/ + target/ + key: ${{ runner.os }}-cargo-${{ hashFiles('**/Cargo.lock') }} + - name: Install libsodium + run: sudo apt-get install -y libsodium-dev + # liboqs requires quite a lot of stack memory, thus we adjust + # the default stack size picked for new threads (which is used + # by `cargo test`) to be _big enough_. Setting it to 8 MiB + - run: RUST_MIN_STACK=8388608 cargo bench --no-run --workspace + cargo-audit: runs-on: ubuntu-latest steps: diff --git a/rosenpass/benches/handshake.rs b/rosenpass/benches/handshake.rs index 1a46df53..e276a9d5 100644 --- a/rosenpass/benches/handshake.rs +++ b/rosenpass/benches/handshake.rs @@ -1,10 +1,8 @@ use anyhow::Result; -use rosenpass::pqkem::KEM; -use rosenpass::{ - pqkem::StaticKEM, - protocol::{CryptoServer, HandleMsgResult, MsgBuf, PeerPtr, SPk, SSk, SymKey}, - sodium::sodium_init, -}; +use rosenpass::protocol::{CryptoServer, HandleMsgResult, MsgBuf, PeerPtr, SPk, SSk, SymKey}; + +use rosenpass_cipher_traits::Kem; +use rosenpass_ciphers::kem::StaticKem; use criterion::{black_box, criterion_group, criterion_main, Criterion}; @@ -41,7 +39,7 @@ fn hs(ini: &mut CryptoServer, res: &mut CryptoServer) -> Result<()> { fn keygen() -> Result<(SSk, SPk)> { let (mut sk, mut pk) = (SSk::zero(), SPk::zero()); - StaticKEM::keygen(sk.secret_mut(), pk.secret_mut())?; + StaticKem::keygen(sk.secret_mut(), pk.secret_mut())?; Ok((sk, pk)) } @@ -58,7 +56,7 @@ fn make_server_pair() -> Result<(CryptoServer, CryptoServer)> { } fn criterion_benchmark(c: &mut Criterion) { - sodium_init().unwrap(); + rosenpass_sodium::init().unwrap(); let (mut a, mut b) = make_server_pair().unwrap(); c.bench_function("cca_secret_alloc", |bench| { bench.iter(|| { From 26cb4a587f243c5c8e32bb9da70c09d2a8088e0a Mon Sep 17 00:00:00 2001 From: wucke13 Date: Wed, 3 Jan 2024 18:26:52 +0100 Subject: [PATCH 12/13] fix: apply clippy lints --- rosenpass/src/cli.rs | 2 +- rosenpass/src/config.rs | 2 +- rosenpass/src/protocol.rs | 16 ++++++---------- rosenpass/tests/integration_test.rs | 11 ++++------- 4 files changed, 12 insertions(+), 19 deletions(-) diff --git a/rosenpass/src/cli.rs b/rosenpass/src/cli.rs index 86cee92b..7b1a7d42 100644 --- a/rosenpass/src/cli.rs +++ b/rosenpass/src/cli.rs @@ -138,7 +138,7 @@ impl Cli { // Manual arg parsing, since clap wants to prefix flags with "--" let mut args = args.into_iter(); loop { - match (args.next().as_ref().map(String::as_str), args.next()) { + match (args.next().as_deref(), args.next()) { (Some("private-key"), Some(opt)) | (Some("secret-key"), Some(opt)) => { secret_key = Some(opt.into()); } diff --git a/rosenpass/src/config.rs b/rosenpass/src/config.rs index 8eee0493..c61c62e0 100644 --- a/rosenpass/src/config.rs +++ b/rosenpass/src/config.rs @@ -374,7 +374,7 @@ mod test { use super::*; fn split_str(s: &str) -> Vec { - s.split(" ").map(|s| s.to_string()).collect() + s.split(' ').map(|s| s.to_string()).collect() } #[test] diff --git a/rosenpass/src/protocol.rs b/rosenpass/src/protocol.rs index c541d607..9605d002 100644 --- a/rosenpass/src/protocol.rs +++ b/rosenpass/src/protocol.rs @@ -1770,14 +1770,10 @@ mod test { // Process the entire handshake let mut msglen = Some(me.initiate_handshake(PEER0, &mut *resbuf).unwrap()); - loop { - if let Some(l) = msglen { - std::mem::swap(&mut me, &mut they); - std::mem::swap(&mut msgbuf, &mut resbuf); - msglen = test_incorrect_sizes_for_msg(&mut me, &*msgbuf, l, &mut *resbuf); - } else { - break; - } + while let Some(l) = msglen { + std::mem::swap(&mut me, &mut they); + std::mem::swap(&mut msgbuf, &mut resbuf); + msglen = test_incorrect_sizes_for_msg(&mut me, &*msgbuf, l, &mut *resbuf); } assert_eq!( @@ -1804,8 +1800,8 @@ mod test { } let res = srv.handle_msg(&msgbuf[..l], resbuf); - assert!(matches!(res, Err(_))); // handle_msg should raise an error - assert!(!resbuf.iter().find(|x| **x != 0).is_some()); // resbuf should not have been changed + assert!(res.is_err()); // handle_msg should raise an error + assert!(!resbuf.iter().any(|x| *x != 0)); // resbuf should not have been changed } // Apply the proper handle_msg operation diff --git a/rosenpass/tests/integration_test.rs b/rosenpass/tests/integration_test.rs index a953d661..e786df3f 100644 --- a/rosenpass/tests/integration_test.rs +++ b/rosenpass/tests/integration_test.rs @@ -30,11 +30,8 @@ fn generate_keys() { fn find_udp_socket() -> u16 { for port in 1025..=u16::MAX { - match UdpSocket::bind(("127.0.0.1", port)) { - Ok(_) => { - return port; - } - _ => {} + if UdpSocket::bind(("127.0.0.1", port)).is_ok() { + return port; } } panic!("no free UDP port found"); @@ -54,9 +51,9 @@ fn check_exchange() { for (secret_key_path, pub_key_path) in secret_key_paths.iter().zip(public_key_paths.iter()) { let output = test_bin::get_test_bin(BIN) .args(["gen-keys", "--secret-key"]) - .arg(&secret_key_path) + .arg(secret_key_path) .arg("--public-key") - .arg(&pub_key_path) + .arg(pub_key_path) .output() .expect("Failed to start {BIN}"); From 62aa9b4351536a0bd0eeaae14878e0c3b691b3c2 Mon Sep 17 00:00:00 2001 From: wucke13 Date: Wed, 3 Jan 2024 18:32:09 +0100 Subject: [PATCH 13/13] fix: second round of clippy lints Clippy would not automatically apply these fixes, so they were applied by hand. --- constant-time/src/lib.rs | 2 +- secret-memory/src/public.rs | 2 +- secret-memory/src/secret.rs | 4 ++-- to/src/ops.rs | 12 ++++++------ util/src/file.rs | 8 ++++---- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/constant-time/src/lib.rs b/constant-time/src/lib.rs index f1a8c549..33908ba5 100644 --- a/constant-time/src/lib.rs +++ b/constant-time/src/lib.rs @@ -16,7 +16,7 @@ use rosenpass_to::{with_destination, To}; /// /// If source and destination are of different sizes. #[inline] -pub fn xor<'a>(src: &'a [u8]) -> impl To<[u8], ()> + 'a { +pub fn xor(src: &[u8]) -> impl To<[u8], ()> + '_ { with_destination(|dst: &mut [u8]| { assert!(src.len() == dst.len()); for (dv, sv) in dst.iter_mut().zip(src.iter()) { diff --git a/secret-memory/src/public.rs b/secret-memory/src/public.rs index 39a43b84..342b367f 100644 --- a/secret-memory/src/public.rs +++ b/secret-memory/src/public.rs @@ -20,7 +20,7 @@ pub struct Public { impl Public { /// Create a new [Public] from a byte slice pub fn from_slice(value: &[u8]) -> Self { - copy_slice(value).to_this(|| Self::zero()) + copy_slice(value).to_this(Self::zero) } /// Create a new [Public] from a byte array diff --git a/secret-memory/src/secret.rs b/secret-memory/src/secret.rs index bbaf090c..0b2d993d 100644 --- a/secret-memory/src/secret.rs +++ b/secret-memory/src/secret.rs @@ -83,7 +83,7 @@ impl Deref for ZeroizingSecretBox { type Target = T; fn deref(&self) -> &T { - &self.0.as_ref().unwrap() + self.0.as_ref().unwrap() } } @@ -177,7 +177,7 @@ impl Secret { /// Borrows the data pub fn secret(&self) -> &[u8; N] { - &self.storage.as_ref().unwrap() + self.storage.as_ref().unwrap() } /// Borrows the data mutably diff --git a/to/src/ops.rs b/to/src/ops.rs index cb4aad8b..385c477f 100644 --- a/to/src/ops.rs +++ b/to/src/ops.rs @@ -8,7 +8,7 @@ use crate::{with_destination, To}; /// # Panics /// /// This function will panic if the two slices have different lengths. -pub fn copy_slice<'a, T>(origin: &'a [T]) -> impl To<[T], ()> + 'a +pub fn copy_slice(origin: &[T]) -> impl To<[T], ()> + '_ where T: Copy, { @@ -23,7 +23,7 @@ where /// # Panics /// /// This function will panic if destination is shorter than origin. -pub fn copy_slice_least_src<'a, T>(origin: &'a [T]) -> impl To<[T], ()> + 'a +pub fn copy_slice_least_src(origin: &[T]) -> impl To<[T], ()> + '_ where T: Copy, { @@ -34,7 +34,7 @@ where /// destination. /// /// Copies as much data as is present in the shorter slice. -pub fn copy_slice_least<'a, T>(origin: &'a [T]) -> impl To<[T], ()> + 'a +pub fn copy_slice_least(origin: &[T]) -> impl To<[T], ()> + '_ where T: Copy, { @@ -47,7 +47,7 @@ where /// Function with destination that attempts to copy data from origin into the destination. /// /// Will return None if the slices are of different lengths. -pub fn try_copy_slice<'a, T>(origin: &'a [T]) -> impl To<[T], Option<()>> + 'a +pub fn try_copy_slice(origin: &[T]) -> impl To<[T], Option<()>> + '_ where T: Copy, { @@ -62,7 +62,7 @@ where /// Destination may be longer than origin. /// /// Will return None if the destination is shorter than origin. -pub fn try_copy_slice_least_src<'a, T>(origin: &'a [T]) -> impl To<[T], Option<()>> + 'a +pub fn try_copy_slice_least_src(origin: &[T]) -> impl To<[T], Option<()>> + '_ where T: Copy, { @@ -72,7 +72,7 @@ where } /// Function with destination that copies all data between two array references. -pub fn copy_array<'a, T, const N: usize>(origin: &'a [T; N]) -> impl To<[T; N], ()> + 'a +pub fn copy_array(origin: &[T; N]) -> impl To<[T; N], ()> + '_ where T: Copy, { diff --git a/util/src/file.rs b/util/src/file.rs index 48433820..48f8eb50 100644 --- a/util/src/file.rs +++ b/util/src/file.rs @@ -6,21 +6,21 @@ use std::{fs::OpenOptions, path::Path}; /// Open a file writable pub fn fopen_w>(path: P) -> std::io::Result { - Ok(OpenOptions::new() + OpenOptions::new() .read(false) .write(true) .create(true) .truncate(true) - .open(path)?) + .open(path) } /// Open a file readable pub fn fopen_r>(path: P) -> std::io::Result { - Ok(OpenOptions::new() + OpenOptions::new() .read(true) .write(false) .create(false) .truncate(false) - .open(path)?) + .open(path) } pub trait ReadExactToEnd {