fix(attestation): verify sig during validation (#1037)

Author: themighty1

crates/attestation/src/fixtures.rs

@@ -1,5 +1,4 @@
 //! Attestation fixtures.
-
 use tlsn_core::{
     connection::{CertBinding, CertBindingV1_2},
     fixtures::ConnectionFixture,
@@ -13,7 +12,10 @@ use tlsn_core::{
 use crate::{
     Attestation, AttestationConfig, CryptoProvider, Extension,
     request::{Request, RequestConfig},
-    signing::SignatureAlgId,
+    signing::{
+        KeyAlgId, SignatureAlgId, SignatureVerifier, SignatureVerifierProvider, Signer,
+        SignerProvider,
+    },
 };
 
 /// A Request fixture used for testing.
@@ -102,7 +104,8 @@ pub fn attestation_fixture(
     let mut provider = CryptoProvider::default();
     match signature_alg {
         SignatureAlgId::SECP256K1 => provider.signer.set_secp256k1(&[42u8; 32]).unwrap(),
-        SignatureAlgId::SECP256R1 => provider.signer.set_secp256r1(&[42u8; 32]).unwrap(),
+        SignatureAlgId::SECP256K1ETH => provider.signer.set_secp256k1eth(&[43u8; 32]).unwrap(),
+        SignatureAlgId::SECP256R1 => provider.signer.set_secp256r1(&[44u8; 32]).unwrap(),
         _ => unimplemented!(),
     };
 
@@ -122,3 +125,68 @@ pub fn attestation_fixture(
 
     attestation_builder.build(&provider).unwrap()
 }
+
+/// Returns a crypto provider which supports only a custom signature alg.
+pub fn custom_provider_fixture() -> CryptoProvider {
+    const CUSTOM_SIG_ALG_ID: SignatureAlgId = SignatureAlgId::new(128);
+
+    // A dummy signer.
+    struct DummySigner {}
+    impl Signer for DummySigner {
+        fn alg_id(&self) -> SignatureAlgId {
+            CUSTOM_SIG_ALG_ID
+        }
+
+        fn sign(
+            &self,
+            msg: &[u8],
+        ) -> Result<crate::signing::Signature, crate::signing::SignatureError> {
+            Ok(crate::signing::Signature {
+                alg: CUSTOM_SIG_ALG_ID,
+                data: msg.to_vec(),
+            })
+        }
+
+        fn verifying_key(&self) -> crate::signing::VerifyingKey {
+            crate::signing::VerifyingKey {
+                alg: KeyAlgId::new(128),
+                data: vec![1, 2, 3, 4],
+            }
+        }
+    }
+
+    // A dummy verifier.
+    struct DummyVerifier {}
+    impl SignatureVerifier for DummyVerifier {
+        fn alg_id(&self) -> SignatureAlgId {
+            CUSTOM_SIG_ALG_ID
+        }
+
+        fn verify(
+            &self,
+            _key: &crate::signing::VerifyingKey,
+            msg: &[u8],
+            sig: &[u8],
+        ) -> Result<(), crate::signing::SignatureError> {
+            if msg == sig {
+                Ok(())
+            } else {
+                Err(crate::signing::SignatureError::from_str(
+                    "invalid signature",
+                ))
+            }
+        }
+    }
+
+    let mut provider = CryptoProvider::default();
+
+    let mut signer_provider = SignerProvider::default();
+    signer_provider.set_signer(Box::new(DummySigner {}));
+    provider.signer = signer_provider;
+
+    let mut verifier_provider = SignatureVerifierProvider::empty();
+    verifier_provider.set_verifier(Box::new(DummyVerifier {}));
+    provider.signature = verifier_provider;
+
+    provider
+}

crates/attestation/src/request.rs

@@ -20,7 +20,10 @@ use serde::{Deserialize, Serialize};
 
 use tlsn_core::hash::HashAlgId;
 
-use crate::{Attestation, Extension, connection::ServerCertCommitment, signing::SignatureAlgId};
+use crate::{
+    Attestation, CryptoProvider, Extension, connection::ServerCertCommitment,
+    serialize::CanonicalSerialize, signing::SignatureAlgId,
+};
 
 pub use builder::{RequestBuilder, RequestBuilderError};
 pub use config::{RequestConfig, RequestConfigBuilder, RequestConfigBuilderError};
@@ -41,44 +44,102 @@ impl Request {
     }
 
     /// Validates the content of the attestation against this request.
-    pub fn validate(&self, attestation: &Attestation) -> Result<(), InconsistentAttestation> {
+    pub fn validate(
+        &self,
+        attestation: &Attestation,
+        provider: &CryptoProvider,
+    ) -> Result<(), AttestationValidationError> {
         if attestation.signature.alg != self.signature_alg {
-            return Err(InconsistentAttestation(format!(
+            return Err(AttestationValidationError::inconsistent(format!(
                 "signature algorithm: expected {:?}, got {:?}",
                 self.signature_alg, attestation.signature.alg
             )));
         }
 
         if attestation.header.root.alg != self.hash_alg {
-            return Err(InconsistentAttestation(format!(
+            return Err(AttestationValidationError::inconsistent(format!(
                 "hash algorithm: expected {:?}, got {:?}",
                 self.hash_alg, attestation.header.root.alg
             )));
         }
 
         if attestation.body.cert_commitment() != &self.server_cert_commitment {
-            return Err(InconsistentAttestation(
-                "server certificate commitment does not match".to_string(),
+            return Err(AttestationValidationError::inconsistent(
+                "server certificate commitment does not match",
             ));
         }
 
         // TODO: improve the O(M*N) complexity of this check.
         for extension in &self.extensions {
             if !attestation.body.extensions().any(|e| e == extension) {
-                return Err(InconsistentAttestation(
-                    "extension is missing from the attestation".to_string(),
+                return Err(AttestationValidationError::inconsistent(
+                    "extension is missing from the attestation",
                 ));
             }
         }
 
+        let verifier = provider
+            .signature
+            .get(&attestation.signature.alg)
+            .map_err(|_| {
+                AttestationValidationError::provider(format!(
+                    "provider not configured for signature algorithm id {:?}",
+                    attestation.signature.alg,
+                ))
+            })?;
+
+        verifier
+            .verify(
+                &attestation.body.verifying_key.data,
+                &CanonicalSerialize::serialize(&attestation.header),
+                &attestation.signature.data,
+            )
+            .map_err(|_| {
+                AttestationValidationError::inconsistent("failed to verify the signature")
+            })?;
+
         Ok(())
     }
 }
 
 /// Error for [`Request::validate`].
 #[derive(Debug, thiserror::Error)]
-#[error("inconsistent attestation: {0}")]
-pub struct InconsistentAttestation(String);
+#[error("attestation validation error: {kind}: {message}")]
+pub struct AttestationValidationError {
+    kind: ErrorKind,
+    message: String,
+}
+
+impl AttestationValidationError {
+    fn inconsistent(msg: impl Into<String>) -> Self {
+        Self {
+            kind: ErrorKind::Inconsistent,
+            message: msg.into(),
+        }
+    }
+
+    fn provider(msg: impl Into<String>) -> Self {
+        Self {
+            kind: ErrorKind::Provider,
+            message: msg.into(),
+        }
+    }
+}
+
+#[derive(Debug)]
+enum ErrorKind {
+    Inconsistent,
+    Provider,
+}
+
+impl std::fmt::Display for ErrorKind {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        match self {
+            ErrorKind::Inconsistent => write!(f, "inconsistent"),
+            ErrorKind::Provider => write!(f, "provider"),
+        }
+    }
+}
 
 #[cfg(test)]
 mod test {
@@ -93,7 +154,8 @@ mod test {
     use crate::{
         CryptoProvider,
         connection::ServerCertOpening,
-        fixtures::{RequestFixture, attestation_fixture, request_fixture},
+        fixtures::{RequestFixture, attestation_fixture, custom_provider_fixture, request_fixture},
+        request::{AttestationValidationError, ErrorKind},
         signing::SignatureAlgId,
     };
 
@@ -113,7 +175,9 @@ mod test {
         let attestation =
             attestation_fixture(request.clone(), connection, SignatureAlgId::SECP256K1, &[]);
 
-        assert!(request.validate(&attestation).is_ok())
+        let provider = CryptoProvider::default();
+
+        assert!(request.validate(&attestation, &provider).is_ok())
     }
 
     #[test]
@@ -134,7 +198,9 @@ mod test {
 
         request.signature_alg = SignatureAlgId::SECP256R1;
 
-        let res = request.validate(&attestation);
+        let provider = CryptoProvider::default();
+
+        let res = request.validate(&attestation, &provider);
         assert!(res.is_err());
     }
 
@@ -156,7 +222,9 @@ mod test {
 
         request.hash_alg = HashAlgId::SHA256;
 
-        let res = request.validate(&attestation);
+        let provider = CryptoProvider::default();
+
+        let res = request.validate(&attestation, &provider);
         assert!(res.is_err())
     }
 
@@ -184,11 +252,62 @@ mod test {
         });
         let opening = ServerCertOpening::new(server_cert_data);
 
-        let crypto_provider = CryptoProvider::default();
+        let provider = CryptoProvider::default();
         request.server_cert_commitment =
-            opening.commit(crypto_provider.hash.get(&HashAlgId::BLAKE3).unwrap());
+            opening.commit(provider.hash.get(&HashAlgId::BLAKE3).unwrap());
 
-        let res = request.validate(&attestation);
+        let res = request.validate(&attestation, &provider);
         assert!(res.is_err())
     }
+
+    #[test]
+    fn test_wrong_sig() {
+        let transcript = Transcript::new(GET_WITH_HEADER, OK_JSON);
+        let connection = ConnectionFixture::tlsnotary(transcript.length());
+
+        let RequestFixture { request, .. } = request_fixture(
+            transcript,
+            encoding_provider(GET_WITH_HEADER, OK_JSON),
+            connection.clone(),
+            Blake3::default(),
+            Vec::new(),
+        );
+
+        let mut attestation =
+            attestation_fixture(request.clone(), connection, SignatureAlgId::SECP256K1, &[]);
+
+        // Corrupt the signature.
+        attestation.signature.data[1] = attestation.signature.data[1].wrapping_add(1);
+
+        let provider = CryptoProvider::default();
+
+        assert!(request.validate(&attestation, &provider).is_err())
+    }
+
+    #[test]
+    fn test_wrong_provider() {
+        let transcript = Transcript::new(GET_WITH_HEADER, OK_JSON);
+        let connection = ConnectionFixture::tlsnotary(transcript.length());
+
+        let RequestFixture { request, .. } = request_fixture(
+            transcript,
+            encoding_provider(GET_WITH_HEADER, OK_JSON),
+            connection.clone(),
+            Blake3::default(),
+            Vec::new(),
+        );
+
+        let attestation =
+            attestation_fixture(request.clone(), connection, SignatureAlgId::SECP256K1, &[]);
+
+        let provider = custom_provider_fixture();
+
+        assert!(matches!(
+            request.validate(&attestation, &provider),
+            Err(AttestationValidationError {
+                kind: ErrorKind::Provider,
+                ..
+            })
+        ))
+    }
 }

crates/attestation/src/signing.rs

@@ -202,6 +202,14 @@ impl SignatureVerifierProvider {
             .map(|s| &**s)
             .ok_or(UnknownSignatureAlgId(*alg))
     }
+
+    /// Returns am empty provider.
+    #[cfg(any(test, feature = "fixtures"))]
+    pub fn empty() -> Self {
+        Self {
+            verifiers: HashMap::default(),
+        }
+    }
 }
 
 /// Signature verifier.
@@ -229,6 +237,14 @@ impl_domain_separator!(VerifyingKey);
 #[error("signature verification failed: {0}")]
 pub struct SignatureError(String);
 
+impl SignatureError {
+    /// Creates a new error with the given message.
+    #[allow(clippy::should_implement_trait)]
+    pub fn from_str(msg: &str) -> Self {
+        Self(msg.to_string())
+    }
+}
+
 /// A signature.
 #[derive(Debug, Clone, Deserialize, Serialize)]
 pub struct Signature {

crates/attestation/tests/api.rs

@@ -101,7 +101,7 @@ fn test_api() {
     let attestation = attestation_builder.build(&provider).unwrap();
 
     // Prover validates the attestation is consistent with its request.
-    request.validate(&attestation).unwrap();
+    request.validate(&attestation, &provider).unwrap();
 
     let mut transcript_proof_builder = secrets.transcript_proof_builder();
 

crates/examples/attestation/prove.rs

@@ -297,8 +297,11 @@ async fn notarize(
         .await
         .map_err(|err| anyhow!("notary did not respond with attestation: {err}"))?;
 
+    // Signature verifier for the signature algorithm in the request.
+    let provider = CryptoProvider::default();
+
     // Check the attestation is consistent with the Prover's view.
-    request.validate(&attestation)?;
+    request.validate(&attestation, &provider)?;
 
     Ok((attestation, secrets))
 }