refactor(core): DoS mitigation and additional validation (#648)

* add encoding proof validation

* check that merkle tree indices are not out of bounds

* limit opened plaintext hash data

* add test

* formatting

* bump commitment tree size cap to 30 bits

* remove unnecessary test

* fix stray lines

Author: themighty1

crates/core/src/connection.rs

@@ -314,11 +314,11 @@ impl ServerCertData {
             return Err(CertificateVerificationError::InvalidServerEphemeralKey);
         }
 
-        // Verify server name
+        // Verify server name.
         let server_name = tls_core::dns::ServerName::try_from(server_name.as_ref())
             .map_err(|_| CertificateVerificationError::InvalidIdentity(server_name.clone()))?;
 
-        // Verify server certificate
+        // Verify server certificate.
         let cert_chain = self
             .certs
             .clone()

crates/core/src/merkle.rs

@@ -19,7 +19,7 @@ impl MerkleError {
 #[derive(Clone, Serialize, Deserialize)]
 pub(crate) struct MerkleProof {
     alg: HashAlgId,
-    tree_len: usize,
+    leaf_count: usize,
     proof: rs_merkle::MerkleProof<Hash>,
 }
 
@@ -55,13 +55,18 @@ impl MerkleProof {
             root.value,
             &indices,
             &leaves,
-            self.tree_len,
+            self.leaf_count,
         ) {
             return Err(MerkleError::new("invalid merkle proof"));
         }
 
         Ok(())
     }
+
+    /// Returns the leaf count of the Merkle tree associated with the proof.
+    pub(crate) fn leaf_count(&self) -> usize {
+        self.leaf_count
+    }
 }
 
 #[derive(Clone)]
@@ -118,15 +123,21 @@ impl MerkleTree {
     /// # Panics
     ///
     /// - If the provided indices are not unique and sorted.
+    /// - If the provided indices are out of bounds.
     pub(crate) fn proof(&self, indices: &[usize]) -> MerkleProof {
         assert!(
             indices.windows(2).all(|w| w[0] < w[1]),
             "indices must be unique and sorted"
         );
 
+        assert!(
+            *indices.last().unwrap() < self.tree.leaves_len(),
+            "one or more provided indices are out of bounds"
+        );
+
         MerkleProof {
             alg: self.alg,
-            tree_len: self.tree.leaves_len(),
+            leaf_count: self.tree.leaves_len(),
             proof: self.tree.proof(indices),
         }
     }
@@ -213,6 +224,21 @@ mod test {
         _ = tree.proof(&[2, 4, 3]);
     }
 
+    #[rstest]
+    #[case::sha2(Sha256::default())]
+    #[case::blake3(Blake3::default())]
+    #[case::keccak(Keccak256::default())]
+    #[should_panic]
+    fn test_proof_fail_index_out_of_bounds<H: HashAlgorithm>(#[case] hasher: H) {
+        let mut tree = MerkleTree::new(hasher.id());
+
+        let leaves = leaves(&hasher, [T(0), T(1), T(2), T(3), T(4)]);
+
+        tree.insert(&hasher, leaves.clone());
+
+        _ = tree.proof(&[2, 3, 4, 6]);
+    }
+
     #[rstest]
     #[case::sha2(Sha256::default())]
     #[case::blake3(Blake3::default())]
@@ -257,12 +283,20 @@ mod test {
 
         tree.insert(&hasher, leaves.clone());
 
-        let mut proof = tree.proof(&[2, 3, 4]);
+        let mut proof1 = tree.proof(&[2, 3, 4]);
+        let mut proof2 = proof1.clone();
 
-        proof.tree_len += 1;
+        // Increment the `leaf_count` field.
+        proof1.leaf_count += 1;
+        // Decrement the `leaf_count` field.
+        proof2.leaf_count -= 1;
 
         // Fail because leaf count is wrong.
-        assert!(proof
+        assert!(proof1
+            .verify(&hasher, &tree.root(), choose_leaves([2, 3, 4], &leaves))
+            .is_err());
+
+        assert!(proof2
             .verify(&hasher, &tree.root(), choose_leaves([2, 3, 4], &leaves))
             .is_err());
     }

crates/core/src/transcript/commit.rs

@@ -10,6 +10,15 @@ use crate::{
     transcript::{Direction, Idx, Transcript},
 };
 
+/// The maximum allowed total bytelength of committed data for a single
+/// commitment kind. Used to prevent DoS during verification. (May cause the
+/// verifier to hash up to a max of 1GB * 128 = 128GB of data for certain kinds
+/// of encoding commitments.)
+///
+/// This value must not exceed bcs's MAX_SEQUENCE_LENGTH limit (which is (1 <<
+/// 31) - 1 by default)
+pub(crate) const MAX_TOTAL_COMMITTED_DATA: usize = 1_000_000_000;
+
 /// Kind of transcript commitment.
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)]
 pub enum TranscriptCommitmentKind {

crates/core/src/transcript/encoding.rs

@@ -17,15 +17,6 @@ use serde::{Deserialize, Serialize};
 
 use crate::hash::{impl_domain_separator, TypedHash};
 
-/// The maximum allowed total bytelength of committed data for a single
-/// commitment kind. Used to prevent DoS during verification. (May cause the
-/// verifier to hash up to a max of 1GB * 128 = 128GB of data for certain kinds
-/// of encoding commitments.)
-///
-/// This value must not exceed bcs's MAX_SEQUENCE_LENGTH limit (which is (1 <<
-/// 31) - 1 by default)
-const MAX_TOTAL_COMMITTED_DATA: usize = 1_000_000_000;
-
 /// Transcript encoding commitment.
 #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
 pub struct EncodingCommitment {

crates/core/src/transcript/encoding/proof.rs

@@ -7,9 +7,8 @@ use crate::{
     hash::{Blinded, Blinder, HashAlgorithmExt, HashProviderError},
     merkle::{MerkleError, MerkleProof},
     transcript::{
-        encoding::{
-            new_encoder, tree::EncodingLeaf, Encoder, EncodingCommitment, MAX_TOTAL_COMMITTED_DATA,
-        },
+        commit::MAX_TOTAL_COMMITTED_DATA,
+        encoding::{new_encoder, tree::EncodingLeaf, Encoder, EncodingCommitment},
         Direction, PartialTranscript, Subsequence,
     },
     CryptoProvider,
@@ -27,6 +26,7 @@ opaque_debug::implement!(Opening);
 
 /// An encoding commitment proof.
 #[derive(Debug, Clone, Serialize, Deserialize)]
+#[serde(try_from = "validation::EncodingProofUnchecked")]
 pub struct EncodingProof {
     /// The proof of inclusion of the commitment(s) in the Merkle tree of
     /// commitments.
@@ -177,6 +177,52 @@ impl From<MerkleError> for EncodingProofError {
     }
 }
 
+/// Invalid encoding proof error.
+#[derive(Debug, thiserror::Error)]
+#[error("invalid encoding proof: {0}")]
+pub struct InvalidEncodingProof(&'static str);
+
+mod validation {
+    use super::*;
+
+    /// The maximum allowed height of the Merkle tree of encoding commitments.
+    ///
+    /// The statistical security parameter (SSP) of the encoding commitment
+    /// protocol is calculated as "the number of uniformly random bits in a
+    /// single bit's encoding minus `MAX_HEIGHT`".
+    ///
+    /// For example, a bit encoding used in garbled circuits typically has 127
+    /// uniformly random bits, hence when using it in the encoding
+    /// commitment protocol, the SSP is 127 - 30 = 97 bits.
+    ///
+    /// Leaving this validation here as a fail-safe in case we ever start
+    /// using shorter encodings.
+    const MAX_HEIGHT: usize = 30;
+
+    #[derive(Debug, Deserialize)]
+    pub(super) struct EncodingProofUnchecked {
+        inclusion_proof: MerkleProof,
+        openings: HashMap<usize, Opening>,
+    }
+
+    impl TryFrom<EncodingProofUnchecked> for EncodingProof {
+        type Error = InvalidEncodingProof;
+
+        fn try_from(unchecked: EncodingProofUnchecked) -> Result<Self, Self::Error> {
+            if unchecked.inclusion_proof.leaf_count() > 1 << MAX_HEIGHT {
+                return Err(InvalidEncodingProof(
+                    "the height of the tree exceeds the maximum allowed",
+                ));
+            }
+
+            Ok(Self {
+                inclusion_proof: unchecked.inclusion_proof,
+                openings: unchecked.openings,
+            })
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use tlsn_data_fixtures::http::{request::POST_JSON, response::OK_JSON};

crates/core/src/transcript/proof.rs

@@ -8,7 +8,7 @@ use crate::{
     attestation::Body,
     index::Index,
     transcript::{
-        commit::TranscriptCommitmentKind,
+        commit::{TranscriptCommitmentKind, MAX_TOTAL_COMMITTED_DATA},
         encoding::{EncodingProof, EncodingProofError, EncodingTree},
         hash::{PlaintextHashProof, PlaintextHashProofError, PlaintextHashSecret},
         Direction, Idx, PartialTranscript, Transcript,
@@ -59,6 +59,8 @@ impl TranscriptProof {
         }
 
         // Verify hash openings.
+        let mut total_opened = 0u128;
+
         for proof in self.hash_proofs {
             let commitment = attestation_body
                 .plaintext_hashes()
@@ -71,6 +73,15 @@ impl TranscriptProof {
                     )
                 })?;
 
+            // Make sure the amount of data being proved is bounded.
+            total_opened += commitment.idx.len() as u128;
+            if total_opened > MAX_TOTAL_COMMITTED_DATA as u128 {
+                return Err(TranscriptProofError::new(
+                    ErrorKind::Hash,
+                    "exceeded maximum allowed data",
+                ))?;
+            }
+
             let (direction, seq) = proof.verify(&provider.hash, commitment)?;
             transcript.union_subsequence(direction, &seq);
         }