From 26ad3d3b9a5d983a7990f932e4acd99861b165da Mon Sep 17 00:00:00 2001 From: "Raymond E. Pasco" Date: Mon, 26 Jun 2023 12:14:57 -0400 Subject: [PATCH] masp: check size and hash of params in node --- shared/Cargo.toml | 2 +- shared/src/ledger/masp.rs | 194 ++++++++++++++++++++++++++------------ 2 files changed, 134 insertions(+), 62 deletions(-) diff --git a/shared/Cargo.toml b/shared/Cargo.toml index ad2eb6f973e..75fda1782ae 100644 --- a/shared/Cargo.toml +++ b/shared/Cargo.toml @@ -128,7 +128,7 @@ wasmer-engine-universal = {version = "=2.2.0", optional = true} wasmer-vm = {version = "2.2.0", optional = true} wasmparser.workspace = true masp_primitives.workspace = true -masp_proofs.workspace = true +masp_proofs = { git = "https://github.com/anoma/masp", rev = "cfea8c95d3f73077ca3e25380fd27e5b46e828fd", default-features = false, features = ["local-prover", "download-params"] } rand = {version = "0.8", default-features = false, optional = true} rand_core = {version = "0.6", default-features = false, optional = true} zeroize.workspace = true diff --git a/shared/src/ledger/masp.rs b/shared/src/ledger/masp.rs index 2b1a9a0184a..35756954541 100644 --- a/shared/src/ledger/masp.rs +++ b/shared/src/ledger/masp.rs @@ -4,7 +4,6 @@ use std::collections::hash_map::Entry; use std::collections::{BTreeMap, HashMap, HashSet}; use std::env; use std::fmt::Debug; -use std::fs::File; #[cfg(feature = "masp-tx-gen")] use std::ops::Deref; use std::path::PathBuf; @@ -44,9 +43,7 @@ use masp_primitives::transaction::{ TransparentAddress, Unauthorized, }; use masp_primitives::zip32::{ExtendedFullViewingKey, ExtendedSpendingKey}; -use masp_proofs::bellman::groth16::{ - prepare_verifying_key, PreparedVerifyingKey, -}; +use masp_proofs::bellman::groth16::PreparedVerifyingKey; use masp_proofs::bls12_381::Bls12; use masp_proofs::prover::LocalTxProver; use masp_proofs::sapling::SaplingVerificationContext; @@ -90,62 +87,37 @@ pub const OUTPUT_NAME: &str = "masp-output.params"; /// Convert circuit name pub const CONVERT_NAME: &str = "masp-convert.params"; -enum MaspParams { - Spend, - Convert, - Output, -} - -/// Generic helper to load Sapling params. -fn load_params(name: MaspParams) -> ( - masp_proofs::bellman::groth16::Parameters, - masp_proofs::bellman::groth16::PreparedVerifyingKey, +fn load_pvks() -> ( + PreparedVerifyingKey, + PreparedVerifyingKey, + PreparedVerifyingKey, ) { - let params_name = match name { - MaspParams::Spend => SPEND_NAME, - MaspParams::Convert => CONVERT_NAME, - MaspParams::Output => OUTPUT_NAME, - }; - let params_dir = get_params_dir(); - let params_path = params_dir.join(params_name); - if !params_path.exists() { - #[cfg(feature = "masp_proofs/download-params")] - masp_proofs::download_parameters() - .expect("MASP parameters not present or downloadable"); - #[cfg(not(feature = "masp_proofs/download-params"))] - panic!("MASP parameters not present or downloadable"); - } - let param_f = File::open(params_path).unwrap(); - let params = - masp_proofs::bellman::groth16::Parameters::read(¶m_f, false) - .unwrap(); - let vk = prepare_verifying_key(¶ms.vk); - (params, vk) -} - -/// Load Sapling spend params. -pub fn load_spend_params() -> ( - masp_proofs::bellman::groth16::Parameters, - masp_proofs::bellman::groth16::PreparedVerifyingKey, -) { - load_params(MaspParams::Spend) -} + let [spend_path, convert_path, output_path] = + [SPEND_NAME, CONVERT_NAME, OUTPUT_NAME].map(|p| params_dir.join(p)); -/// Load Sapling convert params. -pub fn load_convert_params() -> ( - masp_proofs::bellman::groth16::Parameters, - masp_proofs::bellman::groth16::PreparedVerifyingKey, -) { - load_params(MaspParams::Convert) -} - -/// Load Sapling output params. -pub fn load_output_params() -> ( - masp_proofs::bellman::groth16::Parameters, - masp_proofs::bellman::groth16::PreparedVerifyingKey, -) { - load_params(MaspParams::Output) + if !spend_path.exists() || !convert_path.exists() || !output_path.exists() { + let paths = masp_proofs::download_masp_parameters(None).expect( + "MASP parameters were not present, expected the download to \ + succeed", + ); + if paths.spend != spend_path + || paths.convert != convert_path + || paths.output != output_path + { + panic!( + "unrecoverable: downloaded missing masp params, but to an \ + unfamiliar path" + ) + } + } + // size and blake2b checked here + let params = masp_proofs::load_parameters( + spend_path.as_path(), + output_path.as_path(), + convert_path.as_path(), + ); + (params.spend_vk, params.convert_vk, params.output_vk) } /// check_spend wrapper @@ -277,9 +249,7 @@ pub fn verify_shielded_tx(transaction: &Transaction) -> bool { tracing::info!("sighash computed"); - let (_, spend_pvk) = load_spend_params(); - let (_, convert_pvk) = load_convert_params(); - let (_, output_pvk) = load_output_params(); + let (spend_pvk, convert_pvk, output_pvk) = load_pvks(); let mut ctx = SaplingVerificationContext::new(true); let spends_valid = sapling_bundle.shielded_spends.iter().all(|spend| { @@ -365,7 +335,7 @@ impl /// Abstracts platform specific details away from the logic of shielded pool /// operations. -#[async_trait(?Send)] +#[async_trait(? Send)] pub trait ShieldedUtils: Sized + BorshDeserialize + BorshSerialize + Default + Clone { @@ -1546,3 +1516,105 @@ fn convert_amount( .expect("invalid value for amount"); (asset_type, amount) } + +mod tests { + /// quick and dirty test. will fail on size check + #[test] + #[should_panic(expected = "parameter file size is not correct")] + fn test_wrong_masp_params() { + use std::io::Write; + + use super::{CONVERT_NAME, OUTPUT_NAME, SPEND_NAME}; + + let tempdir = tempfile::tempdir() + .expect("expected a temp dir") + .into_path(); + let fake_params_paths = + [SPEND_NAME, CONVERT_NAME, OUTPUT_NAME].map(|p| tempdir.join(p)); + for path in fake_params_paths { + let mut f = + std::fs::File::create(path).expect("expected a temp file"); + f.write_all(b"fake params") + .expect("expected a writable temp file"); + f.sync_all() + .expect("expected a writable temp file (on sync)"); + } + + std::env::set_var(super::ENV_VAR_MASP_PARAMS_DIR, tempdir.as_os_str()); + // should panic here + super::load_pvks(); + } + + /// a more involved test, using dummy parameters with the right + /// size but the wrong hash. + #[test] + #[should_panic(expected = "parameter file is not correct")] + fn test_wrong_masp_params_hash() { + use masp_primitives::ff::PrimeField; + use masp_proofs::bellman::groth16::{ + generate_random_parameters, Parameters, + }; + use masp_proofs::bellman::{Circuit, ConstraintSystem, SynthesisError}; + use masp_proofs::bls12_381::{Bls12, Scalar}; + + use super::{CONVERT_NAME, OUTPUT_NAME, SPEND_NAME}; + + struct FakeCircuit { + x: E, + } + + impl Circuit for FakeCircuit { + fn synthesize>( + self, + cs: &mut CS, + ) -> Result<(), SynthesisError> { + let x = cs.alloc(|| "x", || Ok(self.x)).unwrap(); + cs.enforce( + || { + "this is an extra long constraint name so that rustfmt \ + is ok with wrapping the params of enforce()" + }, + |lc| lc + x, + |lc| lc + x, + |lc| lc + x, + ); + Ok(()) + } + } + + let dummy_circuit = FakeCircuit { x: Scalar::zero() }; + let mut rng = rand::thread_rng(); + let fake_params: Parameters = + generate_random_parameters(dummy_circuit, &mut rng) + .expect("expected to generate fake params"); + + let tempdir = tempfile::tempdir() + .expect("expected a temp dir") + .into_path(); + // TODO: get masp to export these consts + let fake_params_paths = [ + (SPEND_NAME, 49848572u64), + (CONVERT_NAME, 22570940u64), + (OUTPUT_NAME, 16398620u64), + ] + .map(|(p, s)| (tempdir.join(p), s)); + for (path, size) in fake_params_paths { + let mut f = + std::fs::File::create(path).expect("expected a temp file"); + fake_params + .write(&mut f) + .expect("expected a writable temp file"); + // the dummy circuit has one constraint, and therefore its + // params should always be smaller than the large masp + // circuit params. so this truncate extends the file, and + // extra bytes at the end do not make it invalid. + f.set_len(size).expect("expected to truncate the temp file"); + f.sync_all() + .expect("expected a writable temp file (on sync)"); + } + + std::env::set_var(super::ENV_VAR_MASP_PARAMS_DIR, tempdir.as_os_str()); + // should panic here + super::load_pvks(); + } +}