fix(policy): require unscoped KMS bundle grants (#5697)

crates/policy/src/policy/policy.rs

@@ -2135,6 +2135,87 @@ mod test {
         Ok(())
     }
 
+    #[tokio::test]
+    async fn test_kms_bundle_actions_require_an_unscoped_statement() -> Result<()> {
+        use crate::policy::action::{Action, KmsAction};
+
+        let scoped = Policy::parse_config(
+            br#"{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Action": ["kms:*"],
+      "Resource": ["arn:aws:kms:::key/key-a"]
+    }
+  ]
+}"#,
+        )?;
+        let unscoped = Policy::parse_config(
+            br#"{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Action": ["kms:Backup", "kms:Restore"]
+    }
+  ]
+}"#,
+        )?;
+        let partially_scoped = Policy::parse_config(
+            br#"{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Action": ["kms:Backup", "kms:Restore"],
+      "NotResource": ["arn:aws:kms:::key/protected"]
+    }
+  ]
+}"#,
+        )?;
+        let scoped_deny = Policy::parse_config(
+            br#"{
+  "Version": "2012-10-17",
+  "Statement": [
+    {
+      "Effect": "Allow",
+      "Action": ["kms:Backup", "kms:Restore"]
+    },
+    {
+      "Effect": "Deny",
+      "Action": ["kms:Backup", "kms:Restore"],
+      "Resource": ["arn:aws:kms:::key/protected"]
+    }
+  ]
+}"#,
+        )?;
+        let conditions = HashMap::new();
+        let claims = HashMap::new();
+
+        for action in [KmsAction::BackupAction, KmsAction::RestoreAction] {
+            let args = kms_args(Action::KmsAction(action), "", &conditions, &claims);
+            assert!(
+                !scoped.is_allowed(&args).await,
+                "a single-key grant must not authorize the all-key {action:?} operation"
+            );
+            assert!(
+                !partially_scoped.is_allowed(&args).await,
+                "a grant excluding one key must not authorize the all-key {action:?} operation"
+            );
+            assert!(
+                unscoped.is_allowed(&args).await,
+                "an action-only grant must continue authorizing {action:?}"
+            );
+            assert!(
+                !scoped_deny.is_allowed(&args).await,
+                "a deny for included key material must block the all-key {action:?} operation"
+            );
+        }
+
+        Ok(())
+    }
+
     #[tokio::test]
     async fn test_kms_statement_without_resource_matches_every_key() -> Result<()> {
         use crate::policy::action::{Action, KmsAction};

crates/policy/src/policy/statement.rs

@@ -14,7 +14,7 @@
 
 use super::{
     ActionSet, Args, BucketPolicyArgs, Effect, Error as IamError, Functions, ID, Principal, ResourceSet, Validator,
-    action::{Action, S3Action},
+    action::{Action, KmsAction, S3Action},
     function::key_name::{KeyName, S3KeyName},
     resource::Resource,
     variables::{VariableContext, VariableResolver},
@@ -182,6 +182,14 @@ impl Statement {
     /// left empty. An empty `args.object` means the caller did not scope the request to a
     /// key, which preserves the legacy match-every-key behaviour.
     async fn kms_key_scope_matches(&self, args: &Args<'_>, resolver: &VariableResolver) -> bool {
+        if matches!(args.action, Action::KmsAction(KmsAction::BackupAction | KmsAction::RestoreAction))
+            && (!self.resources.is_empty() || !self.not_resources.is_empty())
+        {
+            // Global bundle operations require an unscoped Allow, while a Deny
+            // covering any key must still block an operation covering every key.
+            return matches!(self.effect, Effect::Deny);
+        }
+
         let kms_resources: Vec<&Resource> = self.resources.iter().filter(|resource| resource.is_kms()).collect();
         let kms_not_resources: Vec<&Resource> = self.not_resources.iter().filter(|resource| resource.is_kms()).collect();