Add Verifier::verify_misbehaviour_header for verifying headers comi…

…ng from a misbehaviour evidence (#1300)

* Add `Verifier::verify_misbehaviour_header` for verifying headers coming from a misbehaviour evidence

* Add changelog entry

* Address review comments

* Call `validate_against_trusted` directly to better see correspondance with `verify`

* Rename `verify` to `verify_update_header`

* Add another changelog entry

.changelog/unreleased/breaking-changes/1294-verify-update-header.md

@@ -0,0 +1,4 @@
+- [`tendermint-light-client-verifier`] Rename `Verifier::verify`
+  to `Verifier::verify_update_header` to better describe
+  its purpose versus `Verifier::verify_misbehaviour_header`
+  ([\#1294](https://github.com/informalsystems/tendermint-rs/issues/1294))

.changelog/unreleased/features/1294-verify-misbehavior-header.md

@@ -0,0 +1,5 @@
+- [`tendermint-light-client-verifier`] Add `Verifier::verify_misbehaviour_header`
+  for verifying headers coming from a misbehaviour evidence.
+  The verification for these headers is a bit more relaxed in order to catch FLA attacks.
+  In particular the "header in the future" check for the header should be skipped.
+  ([\#1294](https://github.com/informalsystems/tendermint-rs/issues/1294))

light-client-js/src/lib.rs

@@ -32,7 +32,7 @@ pub fn verify(untrusted: JsValue, trusted: JsValue, options: JsValue, now: JsVal
     let result = deserialize_params(untrusted, trusted, options, now).map(
         |(untrusted, trusted, options, now)| {
             let verifier = ProdVerifier::default();
-            verifier.verify(
+            verifier.verify_update_header(
                 untrusted.as_untrusted_state(),
                 trusted.as_trusted_state(),
                 &options,

light-client-verifier/src/verifier.rs

@@ -51,8 +51,20 @@ impl From<Result<(), VerificationError>> for Verdict {
 /// - [TMBC-VAL-CONTAINS-CORR.1]
 /// - [TMBC-VAL-COMMIT.1]
 pub trait Verifier: Send + Sync {
-    /// Perform the verification.
-    fn verify(
+    /// Verify a header received in a `MsgUpdateClient`.
+    fn verify_update_header(
+        &self,
+        untrusted: UntrustedBlockState<'_>,
+        trusted: TrustedBlockState<'_>,
+        options: &Options,
+        now: Time,
+    ) -> Verdict;
+
+    /// Verify a header received in `MsgSubmitMisbehaviour`.
+    /// The verification for these headers is a bit more relaxed in order to catch FLA attacks.
+    /// In particular the "header in the future" check for the header should be skipped
+    /// from `validate_against_trusted`.
+    fn verify_misbehaviour_header(
         &self,
         untrusted: UntrustedBlockState<'_>,
         trusted: TrustedBlockState<'_>,
@@ -62,21 +74,21 @@ pub trait Verifier: Send + Sync {
 }
 
 macro_rules! verdict {
-    ($e:expr) => {
+    ($e:expr) => {{
         let result = $e;
         if result.is_err() {
             return result.into();
         }
-    };
+    }};
 }
 
 macro_rules! ensure_verdict_success {
-    ($e:expr) => {
+    ($e:expr) => {{
         let verdict = $e;
         if !matches!(verdict, Verdict::Success) {
             return verdict;
         }
-    };
+    }};
 }
 
 /// Predicate verifier encapsulating components necessary to facilitate
@@ -146,8 +158,8 @@ where
         Verdict::Success
     }
 
-    /// Validate an `UntrustedBlockState`, based on the given `TrustedBlockState`, `Options` and
-    /// current time.
+    /// Validate an `UntrustedBlockState` coming from a client update,
+    /// based on the given `TrustedBlockState`, `Options` and current time.
     pub fn validate_against_trusted(
         &self,
         untrusted: &UntrustedBlockState<'_>,
@@ -162,13 +174,6 @@ where
             now,
         ));
 
-        // Ensure the header isn't from a future time
-        verdict!(self.predicates.is_header_from_past(
-            untrusted.signed_header.header.time,
-            options.clock_drift,
-            now,
-        ));
-
         // Check that the untrusted block is more recent than the trusted state
         verdict!(self
             .predicates
@@ -199,6 +204,22 @@ where
         Verdict::Success
     }
 
+    /// Ensure the header isn't from a future time
+    pub fn check_header_is_from_past(
+        &self,
+        untrusted: &UntrustedBlockState<'_>,
+        options: &Options,
+        now: Time,
+    ) -> Verdict {
+        verdict!(self.predicates.is_header_from_past(
+            untrusted.signed_header.header.time,
+            options.clock_drift,
+            now,
+        ));
+
+        Verdict::Success
+    }
+
     /// Check there is enough overlap between the validator sets of the trusted and untrusted
     /// blocks.
     pub fn verify_commit_against_trusted(
@@ -257,7 +278,26 @@ where
     /// `trusted.next_validators.hash() == trusted.next_validators_hash`,
     /// as typically the `trusted.next_validators` validator set comes from the relayer,
     /// and `trusted.next_validators_hash` is the hash stored on chain.
-    fn verify(
+    fn verify_update_header(
+        &self,
+        untrusted: UntrustedBlockState<'_>,
+        trusted: TrustedBlockState<'_>,
+        options: &Options,
+        now: Time,
+    ) -> Verdict {
+        ensure_verdict_success!(self.verify_validator_sets(&untrusted));
+        ensure_verdict_success!(self.validate_against_trusted(&untrusted, &trusted, options, now));
+        ensure_verdict_success!(self.check_header_is_from_past(&untrusted, options, now));
+        ensure_verdict_success!(self.verify_commit_against_trusted(&untrusted, &trusted, options));
+        ensure_verdict_success!(self.verify_commit(&untrusted));
+
+        Verdict::Success
+    }
+
+    /// Verify a header received in `MsgSubmitMisbehaviour`.
+    /// The verification for these headers is a bit more relaxed in order to catch FLA attacks.
+    /// In particular the "header in the future" check for the header should be skipped.
+    fn verify_misbehaviour_header(
         &self,
         untrusted: UntrustedBlockState<'_>,
         trusted: TrustedBlockState<'_>,
@@ -329,7 +369,7 @@ mod tests {
             clock_drift: Default::default(),
         };
 
-        let verdict = vp.verify(
+        let verdict = vp.verify_update_header(
             light_block_2.as_untrusted_state(),
             light_block_1.as_trusted_state(),
             &opt,

light-client/src/light_client.rs

@@ -211,7 +211,7 @@ impl LightClient {
             let (current_block, status) = self.get_or_fetch_block(current_height, state)?;
 
             // Validate and verify the current block
-            let verdict = self.verifier.verify(
+            let verdict = self.verifier.verify_update_header(
                 current_block.as_untrusted_state(),
                 trusted_block.as_trusted_state(),
                 &self.options,

light-client/src/tests.rs

@@ -169,7 +169,7 @@ pub fn verify_single(
         clock_drift,
     };
 
-    let result = verifier.verify(
+    let result = verifier.verify_update_header(
         input.as_untrusted_state(),
         trusted_block.as_trusted_state(),
         &options,