Merge pull request zcash#80 from filecoin-project/bugfix/verify-chall…

…enge-identity

Verify identify of challenged node when validating merkle proof (non-circuit).

examples/drgporep.rs

@@ -167,7 +167,10 @@ impl Example<Bls12> for DrgPoRepApp {
         let data_root = Some(proof_nc.node.root().into());
         let prover_id = Some(prover_id_bytes.as_slice());
 
-        assert!(proof_nc.node.validate(), "failed to verify data commitment");
+        assert!(
+            proof_nc.node.validate(challenge),
+            "failed to verify data commitment"
+        );
         assert!(
             proof_nc.node.validate_data(&data_node),
             "failed to verify data commitment with data"

src/circuit/drgporep.rs

@@ -246,7 +246,10 @@ mod tests {
         let data_root = Some(proof_nc.node.root().into());
         let prover_id = Some(prover_id.as_slice());
 
-        assert!(proof_nc.node.validate(), "failed to verify data commitment");
+        assert!(
+            proof_nc.node.validate(challenge),
+            "failed to verify data commitment"
+        );
         assert!(
             proof_nc.node.validate_data(&data_node.unwrap()),
             "failed to verify data commitment with data"

src/drgporep.rs

@@ -143,16 +143,25 @@ impl<'a> ProofScheme<'a> for DrgPoRep {
         let challenge = pub_inputs.challenge % pub_params.graph.size();
         assert_ne!(challenge, 0, "can not prove the first node");
 
-        // We should return false, not Err here -- though having the failure information is
-        // useful when debugging. What to do…
-
-        if !proof.replica_node.proof.validate() {
+        if !proof.replica_node.proof.validate(challenge) {
             println!("invalid replica node");
             return Ok(false);
         }
 
-        for (_, p) in &proof.replica_parents {
-            if !p.proof.validate() {
+        let expected_parents = pub_params.graph.parents(challenge);
+        let actual_parents = proof
+            .replica_parents
+            .iter()
+            .map(|x| x.0)
+            .collect::<Vec<_>>();
+
+        if expected_parents != actual_parents {
+            println!("proof parents were not those provided in public parameters");
+            return Ok(false);
+        }
+
+        for (parent_node, p) in &proof.replica_parents {
+            if !p.proof.validate(*parent_node) {
                 println!("invalid replica parent: {:?}", p);
                 return Ok(false);
             }
@@ -306,7 +315,13 @@ mod tests {
         }
     }
 
-    fn prove_verify(lambda: usize, n: usize, i: usize) {
+    fn prove_verify_aux(
+        lambda: usize,
+        n: usize,
+        i: usize,
+        use_wrong_challenge: bool,
+        use_wrong_parents: bool,
+    ) {
         let rng = &mut XorShiftRng::from_seed([0x3dbe6259, 0x8d313d76, 0x3237db17, 0xe5bc0654]);
 
         let m = i * 10;
@@ -344,11 +359,63 @@ mod tests {
             aux: &aux,
         };
 
-        let proof = DrgPoRep::prove(&pp, &pub_inputs, &priv_inputs).unwrap();
-        assert!(
-            DrgPoRep::verify(&pp, &pub_inputs, &proof).unwrap(),
-            "failed to verify"
-        );
+        let real_proof = DrgPoRep::prove(&pp, &pub_inputs, &priv_inputs).unwrap();
+
+        if use_wrong_parents {
+            // Only one 'wrong' option will be tested at a time.
+            assert!(!use_wrong_challenge);
+            let real_parents = real_proof.replica_parents;
+            // A real node will never have all parents equal to 1.
+            let fake_parents = real_parents
+                .iter()
+                .map(|(p, data_proof)| (1, data_proof.clone()))
+                .collect::<Vec<_>>();
+
+            let proof = Proof::new(
+                real_proof.replica_node,
+                fake_parents,
+                real_proof.node.into(),
+            );
+
+            assert!(
+                !DrgPoRep::verify(&pp, &pub_inputs, &proof).unwrap(),
+                "verified in error -- with wrong parents"
+            );
+            return;
+        }
+
+        let proof = real_proof;
+
+        if use_wrong_challenge {
+            let pub_inputs_with_wrong_challenge_for_proof = PublicInputs {
+                prover_id: &bytes_into_fr::<Bls12>(prover_id.as_slice()).unwrap(),
+                challenge: if challenge == 1 { 2 } else { 1 },
+                tau: &tau,
+            };
+            let verified =
+                DrgPoRep::verify(&pp, &pub_inputs_with_wrong_challenge_for_proof, &proof).unwrap();
+            assert!(
+                !verified,
+                "wrongly verified proof which does not match challenge in public input"
+            );
+        } else {
+            assert!(
+                DrgPoRep::verify(&pp, &pub_inputs, &proof).unwrap(),
+                "failed to verify"
+            );
+        }
+    }
+
+    fn prove_verify(lambda: usize, n: usize, i: usize) {
+        prove_verify_aux(lambda, n, i, false, false)
+    }
+
+    fn prove_verify_wrong_challenge(lambda: usize, n: usize, i: usize) {
+        prove_verify_aux(lambda, n, i, true, false)
+    }
+
+    fn prove_verify_wrong_parents(lambda: usize, n: usize, i: usize) {
+        prove_verify_aux(lambda, n, i, false, true)
     }
 
     table_tests!{
@@ -372,4 +439,15 @@ mod tests {
             prove_verify_32_10_5(32, 10, 5);
         }
     }
+
+    #[test]
+    fn test_drgporep_verifies_using_challenge() {
+        prove_verify_wrong_challenge(32, 5, 1);
+    }
+
+    #[test]
+    fn test_drgporep_verifies_parents() {
+        prove_verify_wrong_parents(32, 5, 1);
+    }
+
 }

src/drgraph.rs

@@ -31,6 +31,41 @@ pub struct MerkleProof {
     leaf: TreeHash,
 }
 
+fn path_index(path: &[(TreeHash, bool)]) -> usize {
+    path.iter().rev().fold(0, |acc, (_, is_right)| {
+        (acc << 1) + if *is_right { 1 } else { 0 }
+    })
+}
+
+pub fn hash_leaf(data: &Hashable<TreeAlgorithm>) -> TreeHash {
+    let mut a = TreeAlgorithm::default();
+    data.hash(&mut a);
+    let item_hash = a.hash();
+    let leaf_hash = a.leaf(item_hash);
+
+    leaf_hash
+}
+
+pub fn hash_node(data: &Hashable<TreeAlgorithm>) -> TreeHash {
+    let mut a = TreeAlgorithm::default();
+    data.hash(&mut a);
+    let item_hash = a.hash();
+
+    item_hash
+}
+
+pub fn make_proof_for_test(
+    root: TreeHash,
+    leaf: TreeHash,
+    path: Vec<(TreeHash, bool)>,
+) -> MerkleProof {
+    MerkleProof {
+        path: path,
+        root: root,
+        leaf: leaf,
+    }
+}
+
 impl MerkleProof {
     /// Convert the merkle path into the format expected by the circuits, which is a vector of options of the tuples.
     /// This does __not__ include the root and the leaf.
@@ -48,10 +83,14 @@ impl MerkleProof {
             .collect::<Vec<_>>()
     }
 
-    /// Validates the MerkleProof
-    pub fn validate(&self) -> bool {
+    /// Validates the MerkleProof and that it corresponds to the supplied node.
+    pub fn validate(&self, node: usize) -> bool {
         let mut a = TreeAlgorithm::default();
 
+        if path_index(&self.path) != node {
+            return false;
+        }
+
         self.root() == (0..self.path.len()).fold(self.leaf, |h, i| {
             a.reset();
             let is_right = self.path[i].1;
@@ -66,7 +105,7 @@ impl MerkleProof {
         })
     }
 
-    /// Validates that the data, hashes to the leave of the merkel path.
+    /// Validates that the data hashes to the leaf of the merkle path.
     pub fn validate_data(&self, data: &Hashable<TreeAlgorithm>) -> bool {
         let mut a = TreeAlgorithm::default();
         data.hash(&mut a);
@@ -365,7 +404,7 @@ mod tests {
 
             assert_eq!(mp.len(), len);
 
-            assert!(mp.validate(), "failed to validate valid merkle path");
+            assert!(mp.validate(i), "failed to validate valid merkle path");
             let data_slice = &data[i * 16..(i + 1) * 16].to_vec();
             assert!(
                 mp.validate_data(&data_slice),

src/merklepor.rs

@@ -85,14 +85,17 @@ impl<'a> ProofScheme<'a> for MerklePoR {
             return Err(format_err!("invalid root"));
         }
 
-        Ok(proof.proof.validate_data(&proof.data))
+        let data_valid = proof.proof.validate_data(&proof.data);
+        let path_valid = proof.proof.validate(pub_inputs.challenge);
+
+        Ok(data_valid && path_valid)
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use drgraph::{BucketGraph, Graph};
+    use drgraph::{hash_leaf, make_proof_for_test, BucketGraph, Graph};
     use fr32::{bytes_into_fr, fr_into_bytes};
     use pairing::bls12_381::Bls12;
     use rand::{Rng, SeedableRng, XorShiftRng};
@@ -129,4 +132,92 @@ mod tests {
 
         assert!(MerklePoR::verify(&pub_params, &pub_inputs, &proof).unwrap());
     }
+
+    // Construct a proof that satisfies a cursory validation:
+    // Data and proof are minimally consistent.
+    // Proof root matches that requested in public inputs.
+    // However, note that data has no relationship to anything,
+    // and proof path does not actually prove that data was in the tree corresponding to expected root.
+    fn make_bogus_proof(pub_inputs: &PublicInputs, rng: &mut XorShiftRng) -> DataProof {
+        let bogus_leaf = bytes_into_fr::<Bls12>(&fr_into_bytes::<Bls12>(&rng.gen())).unwrap();
+        let hashed_leaf = hash_leaf(&bogus_leaf);
+
+        DataProof {
+            data: bogus_leaf,
+            proof: make_proof_for_test(
+                pub_inputs.commitment,
+                hashed_leaf,
+                vec![(hashed_leaf, true)],
+            ),
+        }
+    }
+
+    #[test]
+    fn test_merklepor_actually_validates() {
+        let rng = &mut XorShiftRng::from_seed([0x3dbe6259, 0x8d313d76, 0x3237db17, 0xe5bc0654]);
+
+        let pub_params = PublicParams {
+            lambda: 32,
+            leaves: 32,
+        };
+
+        let data: Vec<u8> = (0..32)
+            .flat_map(|_| fr_into_bytes::<Bls12>(&rng.gen()))
+            .collect();
+
+        let graph = BucketGraph::new(32, 16);
+        let tree = graph.merkle_tree(data.as_slice(), 32).unwrap();
+
+        let pub_inputs = PublicInputs {
+            challenge: 3,
+            commitment: tree.root(),
+        };
+
+        let bad_proof = make_bogus_proof(&pub_inputs, rng);
+
+        let verified = MerklePoR::verify(&pub_params, &pub_inputs, &bad_proof).unwrap();
+
+        // A bad proof should not be verified!
+        assert!(!verified);
+    }
+
+    #[test]
+    fn test_merklepor_validates_challenge_identity() {
+        let rng = &mut XorShiftRng::from_seed([0x3dbe6259, 0x8d313d76, 0x3237db17, 0xe5bc0654]);
+
+        let pub_params = PublicParams {
+            lambda: 32,
+            leaves: 32,
+        };
+
+        let data: Vec<u8> = (0..32)
+            .flat_map(|_| fr_into_bytes::<Bls12>(&rng.gen()))
+            .collect();
+
+        let graph = BucketGraph::new(32, 16);
+        let tree = graph.merkle_tree(data.as_slice(), 32).unwrap();
+
+        let pub_inputs = PublicInputs {
+            challenge: 3,
+            commitment: tree.root(),
+        };
+
+        let leaf = bytes_into_fr::<Bls12>(
+            data_at_node(data.as_slice(), pub_inputs.challenge, pub_params.lambda).unwrap(),
+        ).unwrap();
+
+        let priv_inputs = PrivateInputs { tree: &tree, leaf };
+
+        let proof = MerklePoR::prove(&pub_params, &pub_inputs, &priv_inputs).unwrap();
+
+        let different_pub_inputs = PublicInputs {
+            challenge: 999,
+            commitment: tree.root(),
+        };
+
+        let verified = MerklePoR::verify(&pub_params, &different_pub_inputs, &proof).unwrap();
+
+        // A proof created with a the wrong challenge not be verified!
+        assert!(!verified);
+    }
 }