From 139fc09f1028f0f2d073748dfca8529dbe98807b Mon Sep 17 00:00:00 2001 From: Henry de Valence Date: Wed, 5 Aug 2020 03:26:31 -0700 Subject: [PATCH] bellman: add VerificationError (#254) * bellman: add VerificationError This adds a distinct VerificationError type to the crate and changes `verify_proof` to return `Result<(), VerificationError>` rather than `Result`. This is significantly safer, because it avoids the need to mix pattern-matching logic with boolean logic (the cause of RUSTSEC-2019-0004). * Rename VerificationError variants per review comments. * Add missing Clone impl to VerificationError. --- bellman/src/groth16/mod.rs | 4 ++-- bellman/src/groth16/tests/mod.rs | 2 +- bellman/src/groth16/verifier.rs | 25 +++++++++++++--------- bellman/src/lib.rs | 31 +++++++++++++++++++++++----- bellman/tests/mimc.rs | 2 +- zcash_proofs/src/sapling/prover.rs | 10 +-------- zcash_proofs/src/sapling/verifier.rs | 16 ++------------ zcash_proofs/src/sprout.rs | 8 +------ 8 files changed, 49 insertions(+), 49 deletions(-) diff --git a/bellman/src/groth16/mod.rs b/bellman/src/groth16/mod.rs index 7c97197aa..624324f2a 100644 --- a/bellman/src/groth16/mod.rs +++ b/bellman/src/groth16/mod.rs @@ -560,8 +560,8 @@ mod test_with_bls12_381 { let de_proof = Proof::read(&v[..]).unwrap(); assert!(proof == de_proof); - assert!(verify_proof(&pvk, &proof, &[c]).unwrap()); - assert!(!verify_proof(&pvk, &proof, &[a]).unwrap()); + assert!(verify_proof(&pvk, &proof, &[c]).is_ok()); + assert!(verify_proof(&pvk, &proof, &[a]).is_err()); } } } diff --git a/bellman/src/groth16/tests/mod.rs b/bellman/src/groth16/tests/mod.rs index 8038e1721..371f7c16f 100644 --- a/bellman/src/groth16/tests/mod.rs +++ b/bellman/src/groth16/tests/mod.rs @@ -377,5 +377,5 @@ fn test_xordemo() { assert_eq!(expected_c, proof.c); } - assert!(verify_proof(&pvk, &proof, &[Fr::one()]).unwrap()); + assert!(verify_proof(&pvk, &proof, &[Fr::one()]).is_ok()); } diff --git a/bellman/src/groth16/verifier.rs b/bellman/src/groth16/verifier.rs index 0fe8c9460..18a376db6 100644 --- a/bellman/src/groth16/verifier.rs +++ b/bellman/src/groth16/verifier.rs @@ -4,7 +4,7 @@ use std::ops::{AddAssign, Neg}; use super::{PreparedVerifyingKey, Proof, VerifyingKey}; -use crate::SynthesisError; +use crate::VerificationError; pub fn prepare_verifying_key(vk: &VerifyingKey) -> PreparedVerifyingKey { let gamma = vk.gamma_g2.neg(); @@ -22,9 +22,9 @@ pub fn verify_proof<'a, E: MultiMillerLoop>( pvk: &'a PreparedVerifyingKey, proof: &Proof, public_inputs: &[E::Fr], -) -> Result { +) -> Result<(), VerificationError> { if (public_inputs.len() + 1) != pvk.ic.len() { - return Err(SynthesisError::MalformedVerifyingKey); + return Err(VerificationError::InvalidVerifyingKey); } let mut acc = pvk.ic[0].to_curve(); @@ -41,11 +41,16 @@ pub fn verify_proof<'a, E: MultiMillerLoop>( // A * B + inputs * (-gamma) + C * (-delta) = alpha * beta // which allows us to do a single final exponentiation. - Ok(E::multi_miller_loop(&[ - (&proof.a, &proof.b.into()), - (&acc.to_affine(), &pvk.neg_gamma_g2), - (&proof.c, &pvk.neg_delta_g2), - ]) - .final_exponentiation() - == pvk.alpha_g1_beta_g2) + if pvk.alpha_g1_beta_g2 + == E::multi_miller_loop(&[ + (&proof.a, &proof.b.into()), + (&acc.to_affine(), &pvk.neg_gamma_g2), + (&proof.c, &pvk.neg_delta_g2), + ]) + .final_exponentiation() + { + Ok(()) + } else { + Err(VerificationError::InvalidProof) + } } diff --git a/bellman/src/lib.rs b/bellman/src/lib.rs index 6412a928b..89e7f8bd9 100644 --- a/bellman/src/lib.rs +++ b/bellman/src/lib.rs @@ -122,7 +122,7 @@ //! let inputs = multipack::compute_multipacking(&hash_bits); //! //! // Check the proof! -//! assert!(groth16::verify_proof(&pvk, &proof, &inputs).unwrap()); +//! assert!(groth16::verify_proof(&pvk, &proof, &inputs).is_ok()); //! ``` //! //! # Roadmap @@ -301,7 +301,7 @@ impl<'a, Scalar: PrimeField> Sub<(Scalar, &'a LinearCombination)> } /// This is an error that could occur during circuit synthesis contexts, -/// such as CRS generation, proving or verification. +/// such as CRS generation or proving. #[derive(Debug)] pub enum SynthesisError { /// During synthesis, we lacked knowledge of a variable assignment. @@ -316,8 +316,6 @@ pub enum SynthesisError { UnexpectedIdentity, /// During proof generation, we encountered an I/O error with the CRS IoError(io::Error), - /// During verification, our verifying key was malformed. - MalformedVerifyingKey, /// During CRS generation, we observed an unconstrained auxiliary variable UnconstrainedVariable, } @@ -339,7 +337,6 @@ impl Error for SynthesisError { SynthesisError::PolynomialDegreeTooLarge => "polynomial degree is too large", SynthesisError::UnexpectedIdentity => "encountered an identity element in the CRS", SynthesisError::IoError(_) => "encountered an I/O error", - SynthesisError::MalformedVerifyingKey => "malformed verifying key", SynthesisError::UnconstrainedVariable => "auxiliary variable was unconstrained", } } @@ -356,6 +353,30 @@ impl fmt::Display for SynthesisError { } } +/// An error during verification. +#[derive(Debug, Clone)] +pub enum VerificationError { + /// Verification was attempted with a malformed verifying key. + InvalidVerifyingKey, + /// Proof verification failed. + InvalidProof, +} + +impl Error for VerificationError { + fn description(&self) -> &str { + match *self { + VerificationError::InvalidVerifyingKey => "malformed verifying key", + VerificationError::InvalidProof => "proof verification failed", + } + } +} + +impl fmt::Display for VerificationError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> { + write!(f, "{}", self) + } +} + /// Represents a constraint system which can have new variables /// allocated and constrains between them formed. pub trait ConstraintSystem: Sized { diff --git a/bellman/tests/mimc.rs b/bellman/tests/mimc.rs index b708a18a2..269690840 100644 --- a/bellman/tests/mimc.rs +++ b/bellman/tests/mimc.rs @@ -210,7 +210,7 @@ fn test_mimc() { let start = Instant::now(); let proof = Proof::read(&proof_vec[..]).unwrap(); // Check the proof - assert!(verify_proof(&pvk, &proof, &[image]).unwrap()); + assert!(verify_proof(&pvk, &proof, &[image]).is_ok()); total_verifying += start.elapsed(); } let proving_avg = total_proving / SAMPLES; diff --git a/zcash_proofs/src/sapling/prover.rs b/zcash_proofs/src/sapling/prover.rs index cc3898e06..06f9d3ac8 100644 --- a/zcash_proofs/src/sapling/prover.rs +++ b/zcash_proofs/src/sapling/prover.rs @@ -154,15 +154,7 @@ impl SaplingProvingContext { } // Verify the proof - match verify_proof(verifying_key, &proof, &public_input[..]) { - // No error, and proof verification successful - Ok(true) => {} - - // Any other case - _ => { - return Err(()); - } - } + verify_proof(verifying_key, &proof, &public_input[..]).map_err(|_| ())?; // Compute value commitment let value_commitment: edwards::Point = value_commitment.cm(params).into(); diff --git a/zcash_proofs/src/sapling/verifier.rs b/zcash_proofs/src/sapling/verifier.rs index 32e4d8ca8..2a1fffaa1 100644 --- a/zcash_proofs/src/sapling/verifier.rs +++ b/zcash_proofs/src/sapling/verifier.rs @@ -106,13 +106,7 @@ impl SaplingVerificationContext { } // Verify the proof - match verify_proof(verifying_key, &zkproof, &public_input[..]) { - // No error, and proof verification successful - Ok(true) => true, - - // Any other case - _ => false, - } + verify_proof(verifying_key, &zkproof, &public_input[..]).is_ok() } /// Perform consensus checks on a Sapling OutputDescription, while @@ -159,13 +153,7 @@ impl SaplingVerificationContext { public_input[4] = cm; // Verify the proof - match verify_proof(verifying_key, &zkproof, &public_input[..]) { - // No error, and proof verification successful - Ok(true) => true, - - // Any other case - _ => false, - } + verify_proof(verifying_key, &zkproof, &public_input[..]).is_ok() } /// Perform consensus checks on the valueBalance and bindingSig parts of a diff --git a/zcash_proofs/src/sprout.rs b/zcash_proofs/src/sprout.rs index efd0508ec..d6646b5ce 100644 --- a/zcash_proofs/src/sprout.rs +++ b/zcash_proofs/src/sprout.rs @@ -169,11 +169,5 @@ pub fn verify_proof( }; // Verify the proof - match groth16::verify_proof(verifying_key, &proof, &public_input[..]) { - // No error, and proof verification successful - Ok(true) => true, - - // Any other case - _ => false, - } + groth16::verify_proof(verifying_key, &proof, &public_input[..]).is_ok() }