From 603c4313cc5d326055a2ae7f499a088da04516e0 Mon Sep 17 00:00:00 2001 From: nemo Date: Wed, 1 Jul 2020 15:12:09 -0400 Subject: [PATCH 1/2] fix: apply security audit results In particular, this PR consider FPS-14/15/16/17/18/19/20. Changes are applied for FPS-15/16/20. --- README.md | 18 ++++++------------ storage-proofs/core/src/crypto/feistel.rs | 2 +- storage-proofs/core/src/sector.rs | 4 ++-- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 527e5d228..557837435 100644 --- a/README.md +++ b/README.md @@ -17,7 +17,7 @@ There are currently four different crates: - [**Filecoin Proofs (`filecoin-proofs`)**](./filecoin-proofs) - A wrapper around `storage-proofs`, providing an FFI-exported API callable from C (and in practice called by [go-filecoin](https://github.com/filecoin-project/go-filecoin') via cgo). Filecoin-specific values of setup parameters are included here, and circuit parameters generated by Filecoin’s (future) trusted setup will also live here. + A wrapper around `storage-proofs`, providing an FFI-exported API callable from C (and in practice called by [go-filecoin](https://github.com/filecoin-project/go-filecoin) via cgo). Filecoin-specific values of setup parameters are included here, and circuit parameters generated by Filecoin’s (future) trusted setup will also live here. ![FPS crate dependencies](/img/fps-dependencies.png?raw=true) @@ -81,23 +81,19 @@ Running them > cargo bench --all ``` -To benchmark the examples you can [bencher](src/bin/bencher.rs). +To benchmark the examples you can use [benchy](src/bin/benchy/main.rs), [stacked](src/bin/benchy/stacked.rs), [prodbench](src/bin/benchy/prodbench.rs), [merkleproofs](src/bin/benchy/merkleproofs.rs], [winning_post](src/bin/benchy/winning_post.rs), or [window_post](src/bin/benchy/window_post.rs). ``` # build the script > cargo build # run the benchmarks -> ./target/debug/bencher +> ./target/release/benchy ``` -The results are written into the `.bencher` directory, as JSON files. The benchmarks are controlled through the [bench.config.toml](bench.config.toml) file. +The results are displyed at the command line, or alternatively written as JSON files. Note: On macOS you need `gtime` (`brew install gnu-time`), as the built in `time` command is not enough. -## Profiling - -For development purposes we have an (experimental) support for CPU and memory profiling in Rust through a [`gperftools`](https://github.com/dignifiedquire/rust-gperftools) binding library. These can be enabled though the `cpu-profile` and `heap-profile` features in `filecoin-proofs`. An example setup can be found in this [`Dockerfile`](./Dockerfile-profile) to profile CPU usage for the [`stacked`](https://github.com/filecoin-project/rust-fil-proofs/blob/master/filecoin-proofs/examples/stacked.rs#L40-L61) example. - ## Logging For better logging with backtraces on errors, developers should use `expects` rather than `expect` on `Result` and `Option`. @@ -277,15 +273,13 @@ The **FPS** is accessed from [**go-filecoin**](https://github.com/filecoin-proje The Rust source code serves as the source of truth defining the **FPS** APIs. View the source directly: - [**filecoin-proofs**](https://github.com/filecoin-project/rust-fil-proofs/blob/master/filecoin-proofs/src/api/mod.rs) -- [**sector-base**](https://github.com/filecoin-project/rust-fil-proofs/blob/master/sector-base/README.md#api-reference). +- [**filecoin-proofs-api**](https://github.com/filecoin-project/rust-filecoin-proofs-api). Or better, generate the documentation locally (until repository is public). Follow the instructions to generate documentation above. Then navigate to: -- **Sector Base API:** `…/rust-fil-proofs/target/doc/sector_base/api/index.html` - **Filecoin Proofs API:** `…/rust-fil-proofs/target/doc/filecoin_proofs/api/index.html` -- [Go implementation of filecoin-proofs sectorbuilder API](https://github.com/filecoin-project/go-filecoin/blob/master/proofs/sectorbuilder/rustsectorbuilder.go) and [associated interface structures](https://github.com/filecoin-project/go-filecoin/blob/master/proofs/sectorbuilder/interface.go). -- [Go implementation of filecoin-proofs verifier API](https://github.com/filecoin-project/go-filecoin/blob/master/proofs/rustverifier.go) and [associated interface structures](https://github.com/filecoin-project/go-filecoin/blob/master/proofs/interface.go). +- [Go implementation of filecoin-proofs sectorbuilder API](https://github.com/filecoin-project/go-sectorbuilder/blob/master/sectorbuilder.go) and [associated interface structures](https://github.com/filecoin-project/go-sectorbuilder/blob/master/interface.go). ## Contributing diff --git a/storage-proofs/core/src/crypto/feistel.rs b/storage-proofs/core/src/crypto/feistel.rs index c064f0106..9ae3f7e04 100644 --- a/storage-proofs/core/src/crypto/feistel.rs +++ b/storage-proofs/core/src/crypto/feistel.rs @@ -222,7 +222,7 @@ mod tests { // Since every element in the set is reversibly mapped to another element also in the set, // this is indeed a permutation. assert_eq!(i, v, "failed to permute"); - assert!(p <= *n, "output number is too big"); + assert!(p < *n, "output number is too big"); } } } diff --git a/storage-proofs/core/src/sector.rs b/storage-proofs/core/src/sector.rs index b4595705f..2ec8e32ba 100644 --- a/storage-proofs/core/src/sector.rs +++ b/storage-proofs/core/src/sector.rs @@ -40,8 +40,8 @@ impl fmt::Display for SectorId { } impl SectorId { - pub fn as_fr_safe(self) -> [u8; 31] { - let mut buf: [u8; 31] = [0; 31]; + pub fn as_fr_safe(self) -> [u8; 32] { + let mut buf: [u8; 32] = [0; 32]; byteorder::LittleEndian::write_u64(&mut buf[0..8], self.0); buf } From 276bfdf06f3173486c4d75caabad6d84318e3e85 Mon Sep 17 00:00:00 2001 From: nemo Date: Wed, 8 Jul 2020 17:41:32 -0400 Subject: [PATCH 2/2] fix: apply snark audit updates --- fil-proofs-tooling/Cargo.toml | 1 - .../src/bin/update_tree_r_cache/main.rs | 3 +-- storage-proofs/core/src/compound_proof.rs | 6 +++--- storage-proofs/porep/Cargo.toml | 1 - .../porep/src/stacked/circuit/column.rs | 4 ++++ .../porep/src/stacked/circuit/create_label.rs | 1 + .../porep/src/stacked/circuit/params.rs | 7 ++++++- .../porep/src/stacked/vanilla/proof_scheme.rs | 4 ++++ storage-proofs/post/Cargo.toml | 1 - storage-proofs/post/src/election/circuit.rs | 2 +- storage-proofs/post/src/election/compound.rs | 2 +- storage-proofs/post/src/election/vanilla.rs | 2 +- storage-proofs/post/src/fallback/circuit.rs | 2 +- storage-proofs/post/src/fallback/compound.rs | 2 +- storage-proofs/post/src/fallback/vanilla.rs | 16 +++++++++++++--- storage-proofs/post/src/rational/circuit.rs | 2 +- storage-proofs/post/src/rational/compound.rs | 2 +- storage-proofs/post/src/rational/vanilla.rs | 4 ++-- 18 files changed, 41 insertions(+), 21 deletions(-) diff --git a/fil-proofs-tooling/Cargo.toml b/fil-proofs-tooling/Cargo.toml index 8d5858adc..f63fa32bd 100644 --- a/fil-proofs-tooling/Cargo.toml +++ b/fil-proofs-tooling/Cargo.toml @@ -49,7 +49,6 @@ rayon = "1.3.0" flexi_logger = "0.14.7" typenum = "1.11.2" generic-array = "0.13.2" -tempdir = "0.3.7" [features] default = ["gpu", "measurements"] diff --git a/fil-proofs-tooling/src/bin/update_tree_r_cache/main.rs b/fil-proofs-tooling/src/bin/update_tree_r_cache/main.rs index e99f96cc5..9959c54a3 100644 --- a/fil-proofs-tooling/src/bin/update_tree_r_cache/main.rs +++ b/fil-proofs-tooling/src/bin/update_tree_r_cache/main.rs @@ -8,7 +8,6 @@ use generic_array::typenum::Unsigned; use memmap::MmapOptions; use merkletree::merkle::get_merkle_tree_len; use merkletree::store::{ExternalReader, ReplicaConfig, Store, StoreConfig}; -use tempdir::TempDir; use filecoin_proofs::constants::*; use filecoin_proofs::types::*; @@ -244,7 +243,7 @@ fn run_verify(sector_size: usize, cache: PathBuf, replica_path: PathBuf) -> Resu // Rebuild each of the tree_r_last base trees (in a new temp dir so as not to interfere // with any existing ones on disk) and check if the roots match what's cached on disk - let tmp_dir = TempDir::new("tree-r-last-verify")?; + let tmp_dir = tempfile::tempdir().unwrap(); let tmp_path = tmp_dir.path(); create_dir_all(&tmp_path)?; diff --git a/storage-proofs/core/src/compound_proof.rs b/storage-proofs/core/src/compound_proof.rs index 63e9950e6..293b213bf 100644 --- a/storage-proofs/core/src/compound_proof.rs +++ b/storage-proofs/core/src/compound_proof.rs @@ -202,18 +202,18 @@ where /// default trait methods. fn circuit_proofs( pub_in: &S::PublicInputs, - vanilla_proof: Vec, + vanilla_proofs: Vec, pub_params: &S::PublicParams, groth_params: &groth16::MappedParameters, priority: bool, ) -> Result>> { let mut rng = OsRng; ensure!( - !vanilla_proof.is_empty(), + !vanilla_proofs.is_empty(), "cannot create a circuit proof over missing vanilla proofs" ); - let circuits = vanilla_proof + let circuits = vanilla_proofs .into_par_iter() .enumerate() .map(|(k, vanilla_proof)| { diff --git a/storage-proofs/porep/Cargo.toml b/storage-proofs/porep/Cargo.toml index 59e8e7988..a3241a442 100644 --- a/storage-proofs/porep/Cargo.toml +++ b/storage-proofs/porep/Cargo.toml @@ -34,7 +34,6 @@ hex = "0.4.2" byteorder = "1.3.4" [dev-dependencies] -tempdir = "0.3.7" tempfile = "3" rand_xorshift = "0.2.0" criterion = "0.3.2" diff --git a/storage-proofs/porep/src/stacked/circuit/column.rs b/storage-proofs/porep/src/stacked/circuit/column.rs index 5980f8e0e..6b5dabe0a 100644 --- a/storage-proofs/porep/src/stacked/circuit/column.rs +++ b/storage-proofs/porep/src/stacked/circuit/column.rs @@ -56,6 +56,10 @@ impl Column { } impl AllocatedColumn { + pub fn len(&self) -> usize { + self.rows.len() + } + /// Creates the column hash of this column. pub fn hash>( &self, diff --git a/storage-proofs/porep/src/stacked/circuit/create_label.rs b/storage-proofs/porep/src/stacked/circuit/create_label.rs index 78140d0ce..8a882a197 100644 --- a/storage-proofs/porep/src/stacked/circuit/create_label.rs +++ b/storage-proofs/porep/src/stacked/circuit/create_label.rs @@ -18,6 +18,7 @@ where E: JubjubEngine, CS: ConstraintSystem, { + assert!(replica_id.len() >= 32, "replica id is too small"); assert!(replica_id.len() <= 256, "replica id is too large"); assert_eq!(parents.len(), TOTAL_PARENTS, "invalid sized parents"); diff --git a/storage-proofs/porep/src/stacked/circuit/params.rs b/storage-proofs/porep/src/stacked/circuit/params.rs index 6ba10bbac..bd5f5de65 100644 --- a/storage-proofs/porep/src/stacked/circuit/params.rs +++ b/storage-proofs/porep/src/stacked/circuit/params.rs @@ -94,6 +94,9 @@ impl Proof { .. } = self; + assert!(!drg_parents_proofs.is_empty()); + assert!(!exp_parents_proofs.is_empty()); + // -- verify initial data layer // PrivateInput: data_leaf @@ -112,11 +115,12 @@ impl Proof { // -- verify replica column openings // Private Inputs for the DRG parent nodes. - let mut drg_parents = Vec::new(); + let mut drg_parents = Vec::with_capacity(layers); for (i, parent) in drg_parents_proofs.into_iter().enumerate() { let (parent_col, inclusion_path) = parent.alloc(cs.namespace(|| format!("drg_parent_{}_num", i)))?; + assert_eq!(layers, parent_col.len()); // calculate column hash let val = parent_col.hash(cs.namespace(|| format!("drg_parent_{}_constraint", i)))?; @@ -136,6 +140,7 @@ impl Proof { for (i, parent) in exp_parents_proofs.into_iter().enumerate() { let (parent_col, inclusion_path) = parent.alloc(cs.namespace(|| format!("exp_parent_{}_num", i)))?; + assert_eq!(layers, parent_col.len()); // calculate column hash let val = parent_col.hash(cs.namespace(|| format!("exp_parent_{}_constraint", i)))?; diff --git a/storage-proofs/porep/src/stacked/vanilla/proof_scheme.rs b/storage-proofs/porep/src/stacked/vanilla/proof_scheme.rs index c56e26a84..f12e09c17 100644 --- a/storage-proofs/porep/src/stacked/vanilla/proof_scheme.rs +++ b/storage-proofs/porep/src/stacked/vanilla/proof_scheme.rs @@ -154,6 +154,10 @@ impl<'a, 'c, Tree: 'static + MerkleTreeTrait, G: 'static + Hasher> ProofScheme<' ) -> bool { let partition_challenges = public_params.layer_challenges.challenges_count_all(); + assert_eq!( + partition_challenges.checked_mul(partitions), + Some(partition_challenges * partitions) + ); partition_challenges * partitions >= requirements.minimum_challenges } } diff --git a/storage-proofs/post/Cargo.toml b/storage-proofs/post/Cargo.toml index d4a0d2c21..6dcd56d3e 100644 --- a/storage-proofs/post/Cargo.toml +++ b/storage-proofs/post/Cargo.toml @@ -31,7 +31,6 @@ neptune = { version = "1.0.1", features = ["gpu"] } num_cpus = "1.10.1" [dev-dependencies] -tempdir = "0.3.7" tempfile = "3" pretty_assertions = "0.6.1" rand_xorshift = "0.2.0" diff --git a/storage-proofs/post/src/election/circuit.rs b/storage-proofs/post/src/election/circuit.rs index 20ebf774a..5f0f66b5a 100644 --- a/storage-proofs/post/src/election/circuit.rs +++ b/storage-proofs/post/src/election/circuit.rs @@ -224,7 +224,7 @@ mod tests { let mut trees = BTreeMap::new(); // Construct and store an MT using a named store. - let temp_dir = tempdir::TempDir::new("tree").unwrap(); + let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); for i in 0..5 { diff --git a/storage-proofs/post/src/election/compound.rs b/storage-proofs/post/src/election/compound.rs index 921c3938e..9dcc3ad8f 100644 --- a/storage-proofs/post/src/election/compound.rs +++ b/storage-proofs/post/src/election/compound.rs @@ -214,7 +214,7 @@ mod tests { let mut trees = BTreeMap::new(); // Construct and store an MT using a named store. - let temp_dir = tempdir::TempDir::new("tree").unwrap(); + let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); for i in 0..5 { diff --git a/storage-proofs/post/src/election/vanilla.rs b/storage-proofs/post/src/election/vanilla.rs index 30cb47c40..f5ae4d974 100644 --- a/storage-proofs/post/src/election/vanilla.rs +++ b/storage-proofs/post/src/election/vanilla.rs @@ -478,7 +478,7 @@ mod tests { let mut trees = BTreeMap::new(); // Construct and store an MT using a named store. - let temp_dir = tempdir::TempDir::new("tree").unwrap(); + let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); for i in 0..5 { diff --git a/storage-proofs/post/src/fallback/circuit.rs b/storage-proofs/post/src/fallback/circuit.rs index 0a174611b..d39fbe909 100644 --- a/storage-proofs/post/src/fallback/circuit.rs +++ b/storage-proofs/post/src/fallback/circuit.rs @@ -315,7 +315,7 @@ mod tests { }; // Construct and store an MT using a named DiskStore. - let temp_dir = tempdir::TempDir::new("level_cache_tree_v1").unwrap(); + let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); let mut pub_sectors = Vec::new(); diff --git a/storage-proofs/post/src/fallback/compound.rs b/storage-proofs/post/src/fallback/compound.rs index 981bbedfb..0ac150102 100644 --- a/storage-proofs/post/src/fallback/compound.rs +++ b/storage-proofs/post/src/fallback/compound.rs @@ -244,7 +244,7 @@ mod tests { }; // Construct and store an MT using a named DiskStore. - let temp_dir = tempdir::TempDir::new("level_cache_tree_v1").unwrap(); + let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); let mut pub_sectors = Vec::new(); diff --git a/storage-proofs/post/src/fallback/vanilla.rs b/storage-proofs/post/src/fallback/vanilla.rs index 0e4b2e041..cb6662d6e 100644 --- a/storage-proofs/post/src/fallback/vanilla.rs +++ b/storage-proofs/post/src/fallback/vanilla.rs @@ -457,8 +457,18 @@ impl<'a, Tree: 'a + MerkleTreeTrait> ProofScheme<'a> for FallbackPoSt<'a, Tree> requirements: &Self::Requirements, partitions: usize, ) -> bool { - partitions * public_params.sector_count * public_params.challenge_count - >= requirements.minimum_challenge_count + let checked = partitions * public_params.sector_count; + + assert_eq!( + partitions.checked_mul(public_params.sector_count), + Some(checked) + ); + assert_eq!( + checked.checked_mul(public_params.challenge_count), + Some(checked * public_params.challenge_count) + ); + + checked * public_params.challenge_count >= requirements.minimum_challenge_count } } @@ -497,7 +507,7 @@ mod tests { let prover_id = ::Domain::random(rng); // Construct and store an MT using a named DiskStore. - let temp_dir = tempdir::TempDir::new("level_cache_tree").unwrap(); + let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); let mut pub_sectors = Vec::new(); diff --git a/storage-proofs/post/src/rational/circuit.rs b/storage-proofs/post/src/rational/circuit.rs index 4b1f5c4cf..40d22b351 100644 --- a/storage-proofs/post/src/rational/circuit.rs +++ b/storage-proofs/post/src/rational/circuit.rs @@ -151,7 +151,7 @@ mod tests { }; // Construct and store an MT using a named DiskStore. - let temp_dir = tempdir::TempDir::new("tree").unwrap(); + let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); let (_data1, tree1) = generate_tree::(rng, leaves, Some(temp_path.to_path_buf())); diff --git a/storage-proofs/post/src/rational/compound.rs b/storage-proofs/post/src/rational/compound.rs index d892b6627..b8324efd6 100644 --- a/storage-proofs/post/src/rational/compound.rs +++ b/storage-proofs/post/src/rational/compound.rs @@ -223,7 +223,7 @@ mod tests { let pub_params = RationalPoStCompound::::setup(&setup_params).expect("setup failed"); // Construct and store an MT using a named DiskStore. - let temp_dir = tempdir::TempDir::new("tree").unwrap(); + let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); let (_data1, tree1) = generate_tree::(rng, leaves, Some(temp_path.to_path_buf())); diff --git a/storage-proofs/post/src/rational/vanilla.rs b/storage-proofs/post/src/rational/vanilla.rs index d2fdf32b0..3a5bfe1ae 100644 --- a/storage-proofs/post/src/rational/vanilla.rs +++ b/storage-proofs/post/src/rational/vanilla.rs @@ -324,7 +324,7 @@ mod tests { }; // Construct and store an MT using a named store. - let temp_dir = tempdir::TempDir::new("tree").unwrap(); + let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); let (_data1, tree1) = generate_tree::(rng, leaves, Some(temp_path.to_path_buf())); @@ -438,7 +438,7 @@ mod tests { }; // Construct and store an MT using a named store. - let temp_dir = tempdir::TempDir::new("tree").unwrap(); + let temp_dir = tempfile::tempdir().unwrap(); let temp_path = temp_dir.path(); let (_data, tree) = generate_tree::(rng, leaves, Some(temp_path.to_path_buf()));