feat: reject update calls for delegations with queries-only permissions

rs/tests/consensus/request_auth_malicious_replica_test.rs

@@ -549,9 +549,11 @@ async fn test_request_with_delegation<T: Identity + 'static>(
     // A delegation from identity to identity2 for the specific canister ID.
     let delegation = match delegation_targets {
         Some(targets) => {
-            Delegation::new_with_targets(
+            Delegation::new(
                 signature.public_key.clone().unwrap(), // public key of identity2
                 Time::from_nanos_since_unix_epoch(delegation_expiry),
+            )
+            .with_targets(
                 targets
                     .into_iter()
                     .map(|principal| CanisterId::try_from(principal.as_slice()).unwrap())

rs/tests/crypto/ingress_verification_test.rs

@@ -19,9 +19,10 @@ use ic_system_test_driver::util::{
 };
 use ic_types::crypto::Signable;
 use ic_types::messages::{
-    Blob, Certificate, Delegation, HttpCallContent, HttpCanisterUpdate, HttpQueryContent,
-    HttpReadState, HttpReadStateContent, HttpReadStateResponse, HttpRequestEnvelope, HttpUserQuery,
-    MessageId, RawSignedSenderInfo, SenderInfoContent, SignedDelegation,
+    Blob, Certificate, Delegation, DelegationPermissions, HttpCallContent, HttpCanisterUpdate,
+    HttpQueryContent, HttpReadState, HttpReadStateContent, HttpReadStateResponse,
+    HttpRequestEnvelope, HttpUserQuery, MessageId, RawSignedSenderInfo, SenderInfoContent,
+    SignedDelegation,
 };
 use ic_types::{CanisterId, PrincipalId, Time};
 use ic_universal_canister::wasm;
@@ -43,6 +44,7 @@ fn main() -> Result<()> {
             SystemTestSubGroup::new()
                 .add_test(systest!(requests_with_delegations))
                 .add_test(systest!(requests_with_delegations_with_targets))
+                .add_test(systest!(requests_with_delegation_permissions))
                 .add_test(systest!(requests_with_delegation_loop))
                 .add_test(systest!(requests_to_mgmt_canister_with_delegations))
                 .add_test(systest!(requests_with_sender_info))
@@ -677,6 +679,146 @@ pub fn requests_with_delegations_with_targets(env: TestEnv) {
     });
 }
 
+// Tests for the optional `permissions` field on delegations: `"queries"`
+// restricts the sender to query calls and read_state requests (update calls
+// are rejected), `"all"` (and an absent field) permits everything, and any
+// other value invalidates the delegation for all request kinds.
+pub fn requests_with_delegation_permissions(env: TestEnv) {
+    let logger = env.logger();
+    let node = env.get_first_healthy_node_snapshot();
+    let agent = node.build_default_agent();
+    let rng = &mut reproducible_rng();
+    block_on({
+        async move {
+            let node_url = node.get_public_url();
+            debug!(logger, "Selected replica"; "url" => format!("{}", node_url));
+
+            let canister =
+                UniversalCanister::new_with_retries(&agent, node.effective_canister_id(), &logger)
+                    .await;
+            let test_info = TestInformation {
+                url: node_url,
+                canister_id: canister_id_from_principal(&canister.canister_id()),
+            };
+
+            // Expected outcome for a delegation chain carrying some
+            // `permissions` values.
+            enum Outcome {
+                /// Query and read_state succeed; update calls are rejected
+                /// with an error whose text contains the given substring.
+                QueriesOnly { err: &'static str },
+                /// All request kinds succeed.
+                All,
+            }
+
+            struct PermissionsTest {
+                note: &'static str,
+                // The `permissions` value of each delegation in the chain
+                // (`None` means the field is omitted for that delegation).
+                permissions: Vec<Option<DelegationPermissions>>,
+                outcome: Outcome,
+            }
+
+            let update_not_permitted = "Update calls are not permitted";
+
+            let scenarios = [
+                PermissionsTest {
+                    note: "single delegation restricted to queries",
+                    permissions: vec![Some(DelegationPermissions::Queries)],
+                    outcome: Outcome::QueriesOnly {
+                        err: update_not_permitted,
+                    },
+                },
+                PermissionsTest {
+                    note: "single delegation permitting all",
+                    permissions: vec![Some(DelegationPermissions::All)],
+                    outcome: Outcome::All,
+                },
+                PermissionsTest {
+                    note: "queries restriction in the first of two delegations",
+                    permissions: vec![Some(DelegationPermissions::Queries), None],
+                    outcome: Outcome::QueriesOnly {
+                        err: update_not_permitted,
+                    },
+                },
+                PermissionsTest {
+                    note: "queries restriction in the second of two delegations",
+                    permissions: vec![
+                        Some(DelegationPermissions::All),
+                        Some(DelegationPermissions::Queries),
+                    ],
+                    outcome: Outcome::QueriesOnly {
+                        err: update_not_permitted,
+                    },
+                },
+            ];
+
+            for scenario in &scenarios {
+                let delegation_count = scenario.permissions.len();
+                let mut identities = Vec::with_capacity(delegation_count + 1);
+                for _ in 0..=delegation_count {
+                    let id_type = GenericIdentityType::random_incl_canister(&canister, rng);
+                    identities.push(GenericIdentity::new(id_type, rng));
+                }
+
+                let delegations =
+                    create_delegations_with_permissions(&identities, &scenario.permissions);
+                let sender = &identities[0];
+                let signer = &identities[identities.len() - 1];
+
+                for &api_ver in ALL_QUERY_API_VERSIONS {
+                    let response = perform_query_call_with_delegations(
+                        api_ver,
+                        &test_info,
+                        sender,
+                        signer,
+                        &delegations,
+                    )
+                    .await;
+                    // Query calls are permitted regardless of the outcome.
+                    response.expect_query_ok(api_ver);
+                }
+
+                for &api_ver in ALL_READ_STATE_API_VERSIONS {
+                    let response = perform_read_state_call_with_delegations(
+                        api_ver,
+                        &test_info,
+                        sender,
+                        signer,
+                        &delegations,
+                    )
+                    .await;
+                    // read_state requests are permitted regardless of the outcome.
+                    response.expect_read_state_ok(api_ver);
+                }
+
+                for &api_ver in ALL_UPDATE_API_VERSIONS {
+                    let response = perform_update_call_with_delegations(
+                        api_ver,
+                        &test_info,
+                        sender,
+                        signer,
+                        &delegations,
+                    )
+                    .await;
+                    match scenario.outcome {
+                        Outcome::All => response.expect_update_ok(api_ver),
+                        Outcome::QueriesOnly { err } => {
+                            assert_eq!(
+                                response.status(),
+                                400,
+                                "Scenario {} (update) using v{api_ver} unexpectedly succeeded",
+                                scenario.note
+                            );
+                            response.expect_text_error(err);
+                        }
+                    }
+                }
+            }
+        }
+    });
+}
+
 // Tests for handling of delegation loops
 pub fn requests_with_delegation_loop(env: TestEnv) {
     let logger = env.logger();
@@ -1563,11 +1705,8 @@ fn create_delegations_with_targets(
             let delegation = if targets[i - 1].is_empty() {
                 Delegation::new(identities[i].public_key_der(), delegation_expiry)
             } else {
-                Delegation::new_with_targets(
-                    identities[i].public_key_der(),
-                    delegation_expiry,
-                    targets[i - 1].clone(),
-                )
+                Delegation::new(identities[i].public_key_der(), delegation_expiry)
+                    .with_targets(targets[i - 1].clone())
             };
 
             let signed_delegation = sign_delegation(delegation, &identities[i - 1]);
@@ -1579,6 +1718,30 @@ fn create_delegations_with_targets(
     delegations
 }
 
+fn create_delegations_with_permissions(
+    identities: &[GenericIdentity],
+    permissions: &[Option<DelegationPermissions>],
+) -> Vec<SignedDelegation> {
+    let delegation_expiry = Time::from_nanos_since_unix_epoch(expiry_time().as_nanos() as u64);
+
+    let delegation_count = identities.len() - 1;
+    let mut delegations = Vec::with_capacity(delegation_count);
+
+    for i in 1..=delegation_count {
+        let delegation = match permissions[i - 1] {
+            Some(permissions) => Delegation::new(identities[i].public_key_der(), delegation_expiry)
+                .with_permissions(permissions),
+            None => Delegation::new(identities[i].public_key_der(), delegation_expiry),
+        };
+
+        let signed_delegation = sign_delegation(delegation, &identities[i - 1]);
+
+        delegations.push(signed_delegation);
+    }
+
+    delegations
+}
+
 fn random_canister_id<R: Rng + CryptoRng>(rng: &mut R) -> CanisterId {
     CanisterId::from_u64(rng.r#gen::<u64>())
 }

rs/types/types/src/messages.rs

@@ -10,12 +10,13 @@ mod webauthn;
 
 pub use self::http::{
     Authentication, Certificate, CertificateDelegation, CertificateDelegationFormat,
-    CertificateDelegationMetadata, Delegation, HasCanisterId, HttpCallContent, HttpCanisterUpdate,
-    HttpQueryContent, HttpQueryResponse, HttpQueryResponseReply, HttpReadState,
-    HttpReadStateContent, HttpReadStateResponse, HttpReply, HttpRequest, HttpRequestContent,
-    HttpRequestEnvelope, HttpRequestError, HttpSignedQueryResponse, HttpStatusResponse,
-    HttpUserQuery, NodeSignature, QueryResponseHash, RawHttpRequestVal, RawSignedSenderInfo,
-    ReplicaHealthStatus, SenderInfoContent, SignedDelegation, SignedSenderInfo,
+    CertificateDelegationMetadata, Delegation, DelegationPermissions, HasCanisterId,
+    HttpCallContent, HttpCanisterUpdate, HttpQueryContent, HttpQueryResponse,
+    HttpQueryResponseReply, HttpReadState, HttpReadStateContent, HttpReadStateResponse, HttpReply,
+    HttpRequest, HttpRequestContent, HttpRequestEnvelope, HttpRequestError,
+    HttpSignedQueryResponse, HttpStatusResponse, HttpUserQuery, NodeSignature, QueryResponseHash,
+    RawHttpRequestVal, RawSignedSenderInfo, ReplicaHealthStatus, SenderInfoContent,
+    SignedDelegation, SignedSenderInfo,
 };
 use crate::methods::Callback;
 pub use crate::methods::SystemMethod;

rs/types/types/src/messages/http.rs

@@ -536,6 +536,34 @@ impl From<CanisterIdError> for HttpRequestError {
     }
 }
 
+/// The kinds of calls a delegation permits, as defined in
+/// `<https://docs.internetcomputer.org/references/ic-interface-spec/https-interface/#authentication>`.
+/// An absent field on the [`Delegation`] is unrestricted, equivalent to
+/// [`DelegationPermissions::All`].
+#[derive(Clone, Copy, Eq, PartialEq, Hash, Debug, Deserialize, Serialize)]
+#[cfg_attr(test, derive(Arbitrary))]
+pub enum DelegationPermissions {
+    /// The sender may only perform query calls and `read_state` requests;
+    /// requests to `/call` endpoints are rejected.
+    #[serde(rename = "queries")]
+    Queries,
+    /// The sender may perform all kinds of calls (queries, replicated
+    /// queries, and updates). Same as omitting the field.
+    #[serde(rename = "all")]
+    All,
+}
+
+impl DelegationPermissions {
+    /// The textual value as it appears on the wire and in the
+    /// representation-independent hash.
+    fn as_str(&self) -> &'static str {
+        match self {
+            DelegationPermissions::Queries => "queries",
+            DelegationPermissions::All => "all",
+        }
+    }
+}
+
 /// Describes a delegation map as defined in
 /// `<https://internetcomputer.org/docs/current/references/ic-interface-spec#certification-delegation>`.
 #[derive(Clone, Eq, PartialEq, Hash, Debug, Deserialize, Serialize)]
@@ -544,6 +572,7 @@ pub struct Delegation {
     pubkey: Blob,
     expiration: Time,
     targets: Option<Vec<Blob>>,
+    permissions: Option<DelegationPermissions>,
 }
 
 impl Delegation {
@@ -552,15 +581,20 @@ impl Delegation {
             pubkey: Blob(pubkey),
             expiration,
             targets: None,
+            permissions: None,
         }
     }
 
-    pub fn new_with_targets(pubkey: Vec<u8>, expiration: Time, targets: Vec<CanisterId>) -> Self {
-        Self {
-            pubkey: Blob(pubkey),
-            expiration,
-            targets: Some(targets.iter().map(|c| Blob(c.get().to_vec())).collect()),
-        }
+    /// Restricts the delegation to the given canister targets.
+    pub fn with_targets(mut self, targets: Vec<CanisterId>) -> Self {
+        self.targets = Some(targets.iter().map(|c| Blob(c.get().to_vec())).collect());
+        self
+    }
+
+    /// Restricts the kinds of calls the delegation permits.
+    pub fn with_permissions(mut self, permissions: DelegationPermissions) -> Self {
+        self.permissions = Some(permissions);
+        self
     }
 
     pub fn pubkey(&self) -> &Vec<u8> {
@@ -590,6 +624,12 @@ impl Delegation {
     pub fn number_of_targets(&self) -> Option<usize> {
         self.targets.as_ref().map(Vec::len)
     }
+
+    /// The kinds of calls the delegation permits, if restricted.
+    /// `None` means the delegation is unrestricted.
+    pub fn permissions(&self) -> Option<DelegationPermissions> {
+        self.permissions
+    }
 }
 
 impl SignedBytesWithoutDomainSeparator for Delegation {
@@ -606,6 +646,9 @@ impl SignedBytesWithoutDomainSeparator for Delegation {
                 Array(targets.iter().map(|t| Bytes(t.0.as_slice())).collect()),
             );
         }
+        if let Some(permissions) = self.permissions {
+            map.insert("permissions", String(permissions.as_str()));
+        }
 
         bytes.extend_from_slice(&hash_of_map(&map, |key, value| hash_key_val(key, value)));
     }