From 237990ccbe6afdfdb6c1e265018a3dd8ff16a30f Mon Sep 17 00:00:00 2001 From: Franz-Stefan Preiss Date: Tue, 29 Oct 2024 00:12:24 +0100 Subject: [PATCH] feat(crypto): CRP-2593 skip ingress_expiry check for anonymous queries 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 https://github.com/dfinity/interface-spec/pull/343. --- .../http_request_test_utils/src/lib.rs | 11 ++++++ .../ingress_message/tests/validate_request.rs | 38 +++++++++++++++++-- rs/validator/src/ingress_validation.rs | 8 +++- 3 files changed, 52 insertions(+), 5 deletions(-) diff --git a/rs/validator/http_request_test_utils/src/lib.rs b/rs/validator/http_request_test_utils/src/lib.rs index 6ef12cf9d66..49de404996d 100644 --- 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(rng: &mut R) -> Vec( + exception: AuthenticationScheme, + rng: &mut R, +) -> Vec { + let all_schemes = all_authentication_schemes(rng); + all_schemes + .into_iter() + .filter(|scheme| scheme != &exception) + .collect() +} + pub fn random_user_key_pair(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 index 60c6460066e..6ef8597e7fc 100644 --- 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 index 0b5db63cf26..cd760bcd5e7 100644 --- 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 { + 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 { + 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 { 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(),