From 5a195e8da014b9cb0b9277e86494d2cc108f7c8b Mon Sep 17 00:00:00 2001 From: overtrue Date: Sat, 1 Aug 2026 10:41:29 +0800 Subject: [PATCH] feat(s3-types): append KMS management event names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KMS key management has no event vocabulary, so an audit consumer cannot name what happened to a key. Add the eight management-plane events at the tail of `EventName`, keeping every existing discriminant — and therefore every `mask()` bit — unchanged. The events live in their own `kms:` namespace and no compound `s3:` selector expands to them, so a bucket notification rule can never start matching key management activity. Regressions cover both directions of that isolation, plus the mask bit budget that a future tail append would otherwise overflow silently. Refs rustfs/backlog#1583 (part of rustfs/backlog#1562) --- crates/notify/src/rules/mod.rs | 2 + crates/notify/src/rules/rules_map_test.rs | 104 +++++++++++++++++++ crates/s3-types/src/event_name.rs | 120 ++++++++++++++++++++++ 3 files changed, 226 insertions(+) create mode 100644 crates/notify/src/rules/rules_map_test.rs diff --git a/crates/notify/src/rules/mod.rs b/crates/notify/src/rules/mod.rs index 28608618a..fe2c87e25 100644 --- a/crates/notify/src/rules/mod.rs +++ b/crates/notify/src/rules/mod.rs @@ -22,6 +22,8 @@ mod pattern_rules_test; #[cfg(test)] mod pattern_test; mod rules_map; +#[cfg(test)] +mod rules_map_test; mod subscriber_index; mod subscriber_snapshot; mod target_id_set; diff --git a/crates/notify/src/rules/rules_map_test.rs b/crates/notify/src/rules/rules_map_test.rs new file mode 100644 index 000000000..93ff68cf3 --- /dev/null +++ b/crates/notify/src/rules/rules_map_test.rs @@ -0,0 +1,104 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Regressions for rule matching against the KMS event namespace. +//! +//! KMS events (backlog#1583) are appended to `EventName` for the audit sink, +//! but they must stay invisible to bucket notification rules: a subscriber +//! asking for `s3:ObjectCreated:*` — or for everything — must never receive +//! key management activity. + +use super::RulesMap; +use rustfs_s3_types::EventName; +use rustfs_targets::arn::TargetID; + +const KMS_EVENTS: &[EventName] = &[ + EventName::KmsKeyCreated, + EventName::KmsKeyRotated, + EventName::KmsKeyEnabled, + EventName::KmsKeyDisabled, + EventName::KmsKeyDeletionScheduled, + EventName::KmsKeyDeletionCancelled, + EventName::KmsKeyDeleted, + EventName::KmsKeyAccessed, +]; + +fn test_target() -> TargetID { + TargetID::new("primary".to_string(), "webhook".to_string()) +} + +fn rules_for(events: &[EventName]) -> RulesMap { + let mut rules = RulesMap::new(); + rules.add_rule_config(events, String::new(), test_target()); + rules +} + +#[test] +fn s3_wildcard_subscriptions_do_not_match_kms_events() { + let rules = rules_for(&[ + EventName::ObjectCreatedAll, + EventName::ObjectAccessedAll, + EventName::ObjectRemovedAll, + EventName::ObjectTaggingAll, + EventName::ObjectReplicationAll, + EventName::ObjectRestoreAll, + EventName::ObjectTransitionAll, + EventName::LifecycleExpirationAll, + EventName::ObjectScannerAll, + ]); + + for event in KMS_EVENTS { + assert!(!rules.has_subscriber(event), "{event} must not have an S3 wildcard subscriber"); + assert!( + rules.match_rules(*event, "any/object").is_empty(), + "{event} must not match any S3 wildcard rule" + ); + } +} + +#[test] +fn everything_subscription_does_not_match_kms_events() { + let rules = rules_for(&[EventName::Everything]); + + // Sanity: the catch-all really is subscribed to the S3 surface. + assert!(rules.has_subscriber(&EventName::ObjectCreatedPut)); + + for event in KMS_EVENTS { + assert!(!rules.has_subscriber(event), "{event} must stay outside the s3 catch-all"); + assert!( + rules.match_rules(*event, "any/object").is_empty(), + "{event} must not match the s3 catch-all rule" + ); + } +} + +#[test] +fn kms_subscription_does_not_leak_into_s3_matching() { + // The inverse direction: even an explicit KMS subscription must not widen + // the mask so that S3 events start matching a KMS-only rule. + let rules = rules_for(KMS_EVENTS); + + for event in [ + EventName::ObjectCreatedPut, + EventName::ObjectRemovedDelete, + EventName::ObjectAccessedGet, + EventName::BucketCreated, + ] { + assert!(!rules.has_subscriber(&event), "{event} must not match a KMS-only rule"); + assert!( + rules.match_rules(event, "any/object").is_empty(), + "{event} must not match a KMS-only rule" + ); + } +} diff --git a/crates/s3-types/src/event_name.rs b/crates/s3-types/src/event_name.rs index fe0ed12f4..663201905 100644 --- a/crates/s3-types/src/event_name.rs +++ b/crates/s3-types/src/event_name.rs @@ -86,6 +86,20 @@ pub enum EventName { ObjectRemovedAbortMultipartUpload, ObjectCreatedCreateMultipartUpload, ObjectRemovedDeleteObjects, + + // KMS management-plane events. They travel to the audit sink only and are + // never produced by the bucket notification path, so no compound `s3:` + // event expands to them. New variants must keep being appended here: the + // discriminant of every preceding variant is a `mask()` bit position, and + // inserting in the middle would silently renumber existing bits. + KmsKeyCreated, + KmsKeyRotated, + KmsKeyEnabled, + KmsKeyDisabled, + KmsKeyDeletionScheduled, + KmsKeyDeletionCancelled, + KmsKeyDeleted, + KmsKeyAccessed, } // Single event type sequential array for Everything.expand() @@ -186,6 +200,17 @@ impl EventName { "s3:Scanner:LargeVersions" => Ok(EventName::ScannerLargeVersions), "s3:Scanner:BigPrefix" => Ok(EventName::ScannerBigPrefix), "s3:Scanner:*" => Ok(EventName::ObjectScannerAll), + // KMS events use their own namespace so a `s3:` wildcard in a bucket + // notification config can never select them. They still round-trip + // because audit entries are persisted and replayed by the store targets. + "kms:Key:Created" => Ok(EventName::KmsKeyCreated), + "kms:Key:Rotated" => Ok(EventName::KmsKeyRotated), + "kms:Key:Enabled" => Ok(EventName::KmsKeyEnabled), + "kms:Key:Disabled" => Ok(EventName::KmsKeyDisabled), + "kms:Key:DeletionScheduled" => Ok(EventName::KmsKeyDeletionScheduled), + "kms:Key:DeletionCancelled" => Ok(EventName::KmsKeyDeletionCancelled), + "kms:Key:Deleted" => Ok(EventName::KmsKeyDeleted), + "kms:Key:Accessed" => Ok(EventName::KmsKeyAccessed), // `Everything` has no string representation (`as_str` yields ""), so it // cannot be parsed back from a string. Every other variant round-trips. _ => Err(ParseEventNameError(s.to_string())), @@ -251,6 +276,14 @@ impl EventName { EventName::ObjectRemovedAbortMultipartUpload => "s3:ObjectRemoved:AbortMultipartUpload", EventName::ObjectCreatedCreateMultipartUpload => "s3:ObjectCreated:CreateMultipartUpload", EventName::ObjectRemovedDeleteObjects => "s3:ObjectRemoved:DeleteObjects", + EventName::KmsKeyCreated => "kms:Key:Created", + EventName::KmsKeyRotated => "kms:Key:Rotated", + EventName::KmsKeyEnabled => "kms:Key:Enabled", + EventName::KmsKeyDisabled => "kms:Key:Disabled", + EventName::KmsKeyDeletionScheduled => "kms:Key:DeletionScheduled", + EventName::KmsKeyDeletionCancelled => "kms:Key:DeletionCancelled", + EventName::KmsKeyDeleted => "kms:Key:Deleted", + EventName::KmsKeyAccessed => "kms:Key:Accessed", } } @@ -357,6 +390,25 @@ impl EventName { | EventName::ObjectRemovedDeleteObjects ) } + + /// Returns `true` for KMS management-plane events. + /// + /// These are audit-only: they are never emitted through the bucket + /// notification pipeline, and no `s3:` event selector expands to them. + #[inline] + pub fn is_kms(&self) -> bool { + matches!( + self, + EventName::KmsKeyCreated + | EventName::KmsKeyRotated + | EventName::KmsKeyEnabled + | EventName::KmsKeyDisabled + | EventName::KmsKeyDeletionScheduled + | EventName::KmsKeyDeletionCancelled + | EventName::KmsKeyDeleted + | EventName::KmsKeyAccessed + ) + } } /// Returns the S3 notification event schema version for a given event. @@ -570,6 +622,26 @@ mod tests { EventName::ObjectRemovedAbortMultipartUpload, EventName::ObjectCreatedCreateMultipartUpload, EventName::ObjectRemovedDeleteObjects, + EventName::KmsKeyCreated, + EventName::KmsKeyRotated, + EventName::KmsKeyEnabled, + EventName::KmsKeyDisabled, + EventName::KmsKeyDeletionScheduled, + EventName::KmsKeyDeletionCancelled, + EventName::KmsKeyDeleted, + EventName::KmsKeyAccessed, + ]; + + /// Every KMS management-plane event. + const KMS_EVENT_NAMES: &[EventName] = &[ + EventName::KmsKeyCreated, + EventName::KmsKeyRotated, + EventName::KmsKeyEnabled, + EventName::KmsKeyDisabled, + EventName::KmsKeyDeletionScheduled, + EventName::KmsKeyDeletionCancelled, + EventName::KmsKeyDeleted, + EventName::KmsKeyAccessed, ]; /// Regression for backlog#965: `mask()` used to recurse forever for the @@ -682,6 +754,54 @@ mod tests { } } + /// KMS events are audit-plane only. No `s3:` selector — including the + /// catch-all `Everything` and every compound "All" type — may share a bit + /// with them, otherwise a bucket notification rule would silently start + /// matching KMS activity. + #[test] + fn test_kms_event_masks_are_disjoint_from_every_s3_selector() { + let s3_selectors: Vec = ALL_EVENT_NAMES.iter().copied().filter(|ev| !ev.is_kms()).collect(); + + let mut seen = 0u64; + for kms in KMS_EVENT_NAMES { + let mask = kms.mask(); + assert_ne!(mask, 0, "KMS event {kms} must have a non-zero mask"); + assert_eq!(seen & mask, 0, "KMS event {kms} mask overlaps another KMS event"); + seen |= mask; + + for s3 in &s3_selectors { + assert_eq!(s3.mask() & mask, 0, "KMS event {kms} mask collides with S3 selector {s3}"); + } + } + } + + /// KMS event names must live in their own namespace so that neither a + /// `s3:` prefix filter nor an `s3:...:*` wildcard can select them. + #[test] + fn test_kms_event_names_are_outside_the_s3_namespace() { + for ev in KMS_EVENT_NAMES { + assert!(ev.is_kms(), "{ev} should be classified as a KMS event"); + assert!(ev.as_str().starts_with("kms:"), "unexpected KMS event name {:?}", ev.as_str()); + assert_eq!(EventName::parse(ev.as_str()).as_ref(), Ok(ev), "KMS event {ev} must round-trip"); + assert_eq!(ev.expand(), vec![*ev], "KMS event {ev} must expand to itself only"); + } + + for ev in ALL_EVENT_NAMES.iter().filter(|ev| !ev.is_kms()) { + assert!(!ev.as_str().starts_with("kms:"), "{ev} must not claim the KMS namespace"); + } + } + + /// `mask()` shifts by `discriminant - 1`, so the enum may hold at most 64 + /// mask-bearing variants. Appending past that overflows the shift instead + /// of failing loudly, so keep the budget check next to the variants. + #[test] + fn test_mask_bit_budget_is_not_exhausted() { + for ev in ALL_EVENT_NAMES { + let value = *ev as u32; + assert!(value <= 64, "{ev} has discriminant {value}; mask() only has 64 bits to hand out"); + } + } + /// `Everything` must cover every sequential single-type bit. #[test] fn test_everything_mask_covers_all_single_types() {