feat(crypto): CRP-2593 skip ingress_expiry check for anonymous querie…

…s and read_state requests (#1768) Adapts ingress message validation such that the ingress_expiry check is skipped for anonymous queries and anonymous read_state requests. Implements the interface specification changes from dfinity/interface-spec#343.
dfinity · Oct 28, 2024 · 237990c · 237990c
1 parent dd5ce4f
commit 237990c
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 5 deletions.
diff --git a/rs/validator/http_request_test_utils/src/lib.rs b/rs/validator/http_request_test_utils/src/lib.rs
@@ -730,6 +730,17 @@ pub fn all_authentication_schemes<R: Rng + CryptoRng>(rng: &mut R) -> Vec<Authen
     schemes
 }
 
+pub fn all_authentication_schemes_except<R: Rng + CryptoRng>(
+    exception: AuthenticationScheme,
+    rng: &mut R,
+) -> Vec<AuthenticationScheme> {
+    let all_schemes = all_authentication_schemes(rng);
+    all_schemes
+        .into_iter()
+        .filter(|scheme| scheme != &exception)
+        .collect()
+}
+
 pub fn random_user_key_pair<R: Rng + CryptoRng>(rng: &mut R) -> DirectAuthenticationScheme {
     UserKeyPair(Ed25519KeyPair::generate(rng))
 }

diff --git a/rs/validator/ingress_message/tests/validate_request.rs b/rs/validator/ingress_message/tests/validate_request.rs
@@ -9,10 +9,10 @@ use ic_validator_http_request_test_utils::DirectAuthenticationScheme::{
     CanisterSignature, UserKeyPair,
 };
 use ic_validator_http_request_test_utils::{
-    all_authentication_schemes, canister_signature, hard_coded_root_of_trust, random_user_key_pair,
-    AuthenticationScheme, CanisterSigner, DirectAuthenticationScheme, HttpRequestBuilder,
-    HttpRequestEnvelopeContent, RootOfTrust, CANISTER_ID_SIGNER, CANISTER_SIGNATURE_SEED,
-    CURRENT_TIME,
+    all_authentication_schemes, all_authentication_schemes_except, canister_signature,
+    hard_coded_root_of_trust, random_user_key_pair, AuthenticationScheme, CanisterSigner,
+    DirectAuthenticationScheme, HttpRequestBuilder, HttpRequestEnvelopeContent, RootOfTrust,
+    CANISTER_ID_SIGNER, CANISTER_SIGNATURE_SEED, CURRENT_TIME,
 };
 use ic_validator_ingress_message::AuthenticationError;
 use ic_validator_ingress_message::AuthenticationError::DelegationContainsCyclesError;
@@ -150,6 +150,8 @@ mod ingress_expiry {
                 HttpRequestBuilder::new_update_call(),
                 scheme.clone(),
             );
+        }
+        for scheme in all_authentication_schemes_except(AuthenticationScheme::Anonymous, rng) {
             test(&verifier, HttpRequestBuilder::new_query(), scheme.clone());
             test(&verifier, HttpRequestBuilder::new_read_state(), scheme);
         }
@@ -189,6 +191,8 @@ mod ingress_expiry {
                 HttpRequestBuilder::new_update_call(),
                 scheme.clone(),
             );
+        }
+        for scheme in all_authentication_schemes_except(AuthenticationScheme::Anonymous, rng) {
             test(&verifier, HttpRequestBuilder::new_query(), scheme.clone());
             test(&verifier, HttpRequestBuilder::new_read_state(), scheme);
         }
@@ -273,6 +277,32 @@ mod ingress_expiry {
             assert_matches!(result, Ok(()), "Test with {builder_info} failed");
         }
     }
+
+    #[test]
+    fn should_not_error_when_anonymous_read_state_request_expired() {
+        let verifier = verifier_at_time(CURRENT_TIME).build();
+        let request = HttpRequestBuilder::new_read_state()
+            .with_authentication(AuthenticationScheme::Anonymous)
+            .with_ingress_expiry_at(CURRENT_TIME.saturating_sub(Duration::from_nanos(1)))
+            .build();
+
+        let result = verifier.validate_request(&request);
+
+        assert_matches!(result, Ok(()));
+    }
+
+    #[test]
+    fn should_not_error_when_anonymous_query_expired() {
+        let verifier = verifier_at_time(CURRENT_TIME).build();
+        let request = HttpRequestBuilder::new_query()
+            .with_authentication(AuthenticationScheme::Anonymous)
+            .with_ingress_expiry_at(CURRENT_TIME.saturating_sub(Duration::from_nanos(1)))
+            .build();
+
+        let result = verifier.validate_request(&request);
+
+        assert_matches!(result, Ok(()));
+    }
 }
 
 mod read_state_request {

diff --git a/rs/validator/src/ingress_validation.rs b/rs/validator/src/ingress_validation.rs
@@ -112,6 +112,7 @@ where
         current_time: Time,
         root_of_trust_provider: &R,
     ) -> Result<CanisterIdSet, RequestValidationError> {
+        validate_ingress_expiry(request, current_time)?;
         let delegation_targets = validate_request_content(
             request,
             self.validator.as_ref(),
@@ -134,6 +135,9 @@ where
         current_time: Time,
         root_of_trust_provider: &R,
     ) -> Result<CanisterIdSet, RequestValidationError> {
+        if !request.sender().get().is_anonymous() {
+            validate_ingress_expiry(request, current_time)?;
+        }
         let delegation_targets = validate_request_content(
             request,
             self.validator.as_ref(),
@@ -157,6 +161,9 @@ where
         root_of_trust_provider: &R,
     ) -> Result<CanisterIdSet, RequestValidationError> {
         validate_paths_width_and_depth(&request.content().paths)?;
+        if !request.sender().get().is_anonymous() {
+            validate_ingress_expiry(request, current_time)?;
+        }
         validate_request_content(
             request,
             self.validator.as_ref(),
@@ -194,7 +201,6 @@ where
     R::Error: std::error::Error,
 {
     validate_nonce(request)?;
-    validate_ingress_expiry(request, current_time)?;
     validate_user_id_and_signature(
         ingress_signature_verifier,
         &request.sender(),