Merge pull-request #441

Author: r-n-o

src/Cargo.toml

@@ -11,8 +11,16 @@ members = [
   "qos_p256",
   "qos_nsm",
 ]
-exclude = ["init", "qos_aws", "qos_system", "qos_enclave", "eif_build"]
-# We need this to avoid issues with the mock feature unintentionally being
+exclude = [
+  "init",
+  "qos_aws",
+  "qos_system",
+  "qos_enclave",
+  "eif_build",
+  "qos_p256/fuzz",
+  "qos_crypto/fuzz",
+]
+# We need this to avoid issues with the mock feature uinintentionally being
 # enabled just because some tests need it.
 # https://nickb.dev/blog/cargo-workspace-and-the-feature-unification-pitfall/
 resolver = "2"

src/qos_crypto/fuzz/Cargo.toml

@@ -0,0 +1,42 @@
+[package]
+name = "qos_crypto_fuzz"
+version = "0.0.0"
+publish = false
+edition = "2021"
+
+[package.metadata]
+cargo-fuzz = true
+
+[dependencies]
+libfuzzer-sys = "0.4"
+arbitrary = { version = "1", features = ["derive"] }
+
+[dependencies.qos_crypto]
+path = ".."
+
+# Prevent this from interfering with workspaces
+[workspace]
+members = ["."]
+
+[profile.release]
+# enable arithmetic checks at runtime
+overflow-check = 1
+
+[[bin]]
+name = "1_shamir_generate_reconstruct"
+path = "fuzz_targets/1_shamir_generate_reconstruct.rs"
+test = false
+doc = false
+
+
+[[bin]]
+name = "2_shamir_input_reconstruct_two_shares"
+path = "fuzz_targets/2_shamir_input_reconstruct_two_shares.rs"
+test = false
+doc = false
+
+[[bin]]
+name = "3_shamir_input_reconstruct_three_shares"
+path = "fuzz_targets/3_shamir_input_reconstruct_three_shares.rs"
+test = false
+doc = false

src/qos_crypto/fuzz/fuzz_targets/1_shamir_generate_reconstruct.rs

@@ -0,0 +1,72 @@
+#![no_main]
+
+use libfuzzer_sys::fuzz_target;
+use qos_crypto::shamir::*;
+
+#[derive(Clone, Debug, arbitrary::Arbitrary)]
+pub struct FuzzShamirStruct {
+	pub n: usize,
+	pub k: usize,
+	secret: Box<[u8]>,
+}
+
+use std::{convert::TryFrom, iter};
+
+// let the fuzzer control the number of shares, share threshold number, and secret
+fuzz_target!(|fuzzerdata: FuzzShamirStruct| {
+	let n = fuzzerdata.n;
+	let k = fuzzerdata.k;
+	let secret = fuzzerdata.secret;
+
+	// FUZZER NOTE the effort to reconstruct shares is O(n²) so inputs with a large n
+	// are particularly slow
+
+	// FUZZER TODO artificial limit n to avoid slow inputs, reconsider
+	if n > 64 {
+		return;
+	}
+
+	// FUZZER NOTE the shares_generate() function uses RNG internally and is
+	// therefore non-deterministic, which may limit the reproducibility and effectiveness of this harness
+	let all_shares_res = shares_generate(&secret, n, k);
+
+	match all_shares_res {
+		Err(_) => {}
+		Ok(all_shares) => {
+			// Reconstruct with all the shares
+			let shares = all_shares.clone();
+			let reconstructed =
+				shares_reconstruct(&shares).expect("should succeed");
+			// expect the reconstruction to work
+			assert_eq!(secret.to_vec(), reconstructed);
+
+			// Reconstruct with enough shares
+			let shares = &all_shares[..k];
+			let reconstructed =
+				shares_reconstruct(shares).expect("should succeed");
+
+			// expect the reconstruction to work
+			assert_eq!(secret.to_vec(), reconstructed);
+
+			// Reconstruct with not enough shares
+			let shares = &all_shares[..(k - 1)];
+
+			// although this function returns a Result<>, it does not automatically detect that is has received
+			// an insufficent number of shares and Err() out - instead, it returns Ok() with an incorrect result
+			let reconstructed_res = shares_reconstruct(shares);
+
+			match reconstructed_res {
+				// error case is not interesting
+				Err(_) => {}
+				// OK case is common
+				Ok(reconstructed) => {
+					// if we managed to reconstruct the secret with less than the minimum number of shares
+					// the something is wrong, or we have a random collision
+					if reconstructed == secret.to_vec() {
+						panic!("reconstructed the secret with less than k shares, this should not happen")
+					}
+				}
+			}
+		}
+	}
+});

src/qos_crypto/fuzz/fuzz_targets/2_shamir_input_reconstruct_two_shares.rs

@@ -0,0 +1,41 @@
+#![no_main]
+
+use libfuzzer_sys::fuzz_target;
+use qos_crypto::shamir::*;
+
+/// let the fuzzer come up with two different shares of arbitrary length
+#[derive(Clone, Debug, arbitrary::Arbitrary)]
+pub struct FuzzShareReconstruct {
+	share_one: Box<[u8]>,
+	share_two: Box<[u8]>,
+}
+
+// let the fuzzer control the share data in a two share reconstruction scenario
+fuzz_target!(|fuzzerdata: FuzzShareReconstruct| {
+	let mut shares: Vec<Vec<u8>> = Vec::new();
+
+	// FUZZER NOTE the effort to reconstruct shares is O(n²) so inputs with a large n
+	// are particularly slow
+
+	// this construction with three shares covers more edge cases than the two share variant
+	let mut share_one: Vec<u8> = Vec::new();
+	let mut share_two: Vec<u8> = Vec::new();
+	let mut share_three: Vec<u8> = Vec::new();
+
+	share_one.extend_from_slice(&fuzzerdata.share_one);
+	share_two.extend_from_slice(&fuzzerdata.share_two);
+
+	shares.push(share_one);
+	shares.push(share_two);
+
+	// Reconstruct with the shares, we expect this to error out often
+	let reconstructed_res = shares_reconstruct(&shares);
+	if !reconstructed_res.is_err() {
+		let _reconstructed = reconstructed_res.unwrap();
+
+		// debug print is useful for manual evaluation
+		// println!("reconstructed {:?}", _reconstructed);
+		// println!("from shares: {:?}", shares);
+		// println!("");
+	}
+});

src/qos_crypto/fuzz/fuzz_targets/3_shamir_input_reconstruct_three_shares.rs

@@ -0,0 +1,51 @@
+#![no_main]
+
+use libfuzzer_sys::fuzz_target;
+use qos_crypto::shamir::*;
+
+/// let the fuzzer come up with three different shares of arbitrary length
+#[derive(Clone, Debug, arbitrary::Arbitrary)]
+pub struct FuzzShareReconstruct {
+	share_one: Box<[u8]>,
+	share_two: Box<[u8]>,
+	share_three: Box<[u8]>,
+}
+
+// let the fuzzer control the share data in a three share reconstruction scenario
+fuzz_target!(|fuzzerdata: FuzzShareReconstruct| {
+	let mut shares: Vec<Vec<u8>> = Vec::new();
+	
+	// note that the effort to reconstruct shares is O(n²) so inputs with a large n
+	// are particularly slow
+	// here we have n == 3, so this is not a problem
+
+	// this construction with three shares covers more edge cases than the two share variant
+	let mut share_one: Vec<u8> = Vec::new();
+	let mut share_two: Vec<u8> = Vec::new();
+	let mut share_three: Vec<u8> = Vec::new();
+
+	share_one.extend_from_slice(&fuzzerdata.share_one);
+	share_two.extend_from_slice(&fuzzerdata.share_two);
+	share_three.extend_from_slice(&fuzzerdata.share_three);
+
+	// Fuzz workaround for issue in vsss-rs <= 4.3.5
+	// the bug is fixed in vsss-rs 4.3.6
+	// if(share_one.len() != share_two.len() ) || (share_one.len() != share_three.len() )  {
+	//     return;
+	// }
+
+	shares.push(share_one);
+	shares.push(share_two);
+	shares.push(share_three);
+
+	// Reconstruct with the shares, we expect this to error out often
+	let reconstructed_res = shares_reconstruct(&shares);
+	if !reconstructed_res.is_err() {
+		// let _reconstructed = reconstructed_res.unwrap();
+
+		// debug print is useful for manual evaluation
+		// println!("reconstructed {:?}", _reconstructed);
+		// println!("from shares: {:?}", shares);
+		// println!("");
+	}
+});