diff --git a/local/recipes/system/redbear-notifications/source/src/main.rs b/local/recipes/system/redbear-notifications/source/src/main.rs index 14b1426d8a..ceed131dc7 100644 --- a/local/recipes/system/redbear-notifications/source/src/main.rs +++ b/local/recipes/system/redbear-notifications/source/src/main.rs @@ -12,7 +12,9 @@ use std::{ use tokio::runtime::Builder as RuntimeBuilder; use zbus::{ connection::Builder as ConnectionBuilder, + fdo, interface, + message::Header, object_server::SignalEmitter, zvariant::Value, }; @@ -21,8 +23,26 @@ const BUS_NAME: &str = "org.freedesktop.Notifications"; const OBJECT_PATH: &str = "/org/freedesktop/Notifications"; const NOTIFICATION_CLOSED_REASON: u32 = 3; +// --------------------------------------------------------------------------- +// Notification ownership record +// --------------------------------------------------------------------------- + +/// Information tracked for each active notification, used to validate +/// `InvokeAction` calls. +#[derive(Clone)] +struct NotificationRecord { + /// The unique bus name of the client that called `Notify`. + owner: String, + /// Action keys declared in the original `Notify` call. + /// Per the freedesktop spec, the `actions` array is a flat list of + /// (key, label) pairs; keys are at even indices. + action_keys: Vec, +} + struct Notifications { next_id: AtomicU32, + /// Active notifications keyed by ID, with owner and declared actions. + notifications: Mutex>, /// Record of (notification_id, close_reason) pairs, in close order. closed_notifications: Mutex>, /// Record of (notification_id, action_key) pairs, in invocation order. @@ -33,6 +53,7 @@ impl Notifications { fn new() -> Self { Self { next_id: AtomicU32::new(1), + notifications: Mutex::new(HashMap::new()), closed_notifications: Mutex::new(Vec::new()), invoked_actions: Mutex::new(Vec::new()), } @@ -44,6 +65,7 @@ impl Notifications { #[zbus(name = "Notify")] fn notify( &self, + #[zbus(header)] hdr: Header<'_>, app_name: &str, _replaces_id: u32, _app_icon: &str, @@ -55,11 +77,21 @@ impl Notifications { ) -> u32 { let id = self.allocate_id(); - eprintln!("notification {id}: app_name={app_name:?} summary={summary:?} body_len={}", body.chars().count()); + // Capture the caller's unique bus name as the notification owner. + let owner = hdr.sender().map(|s| s.to_string()).unwrap_or_default(); - for chunk in actions.chunks_exact(2) { - let _ = chunk[0]; - } + // Extract declared action keys (even-indexed elements per spec). + let action_keys: Vec = actions + .chunks_exact(2) + .map(|chunk| chunk[0].clone()) + .collect(); + + self.store_notification(id, &owner, action_keys); + + eprintln!( + "notification {id}: app_name={app_name:?} summary={summary:?} body_len={} owner={owner}", + body.chars().count() + ); id } @@ -70,10 +102,12 @@ impl Notifications { #[zbus(signal_emitter)] signal_emitter: SignalEmitter<'_>, id: u32, ) { + self.remove_notification(id); self.record_close(id, NOTIFICATION_CLOSED_REASON); eprintln!("notification: closed {id}"); - let _ = Self::notification_closed(&signal_emitter, id, NOTIFICATION_CLOSED_REASON).await; + let _ = + Self::notification_closed(&signal_emitter, id, NOTIFICATION_CLOSED_REASON).await; } #[zbus(name = "GetCapabilities")] @@ -100,16 +134,34 @@ impl Notifications { /// `ActionInvoked(id, action_key)` signal is emitted on the session /// bus and any interested client (KDE notification daemon, /// kf6-knotifications, custom testers) receives it. + /// + /// # Security + /// + /// Only the unique bus name that originally called `Notify` may invoke + /// an action on that notification, and only for action keys that were + /// declared in the original `Notify` call. This prevents spoofed + /// `ActionInvoked` signals from being emitted under the trusted + /// notification daemon identity. #[zbus(name = "InvokeAction")] async fn invoke_action( &self, #[zbus(signal_emitter)] signal_emitter: SignalEmitter<'_>, + #[zbus(header)] hdr: Header<'_>, id: u32, action_key: &str, - ) { + ) -> fdo::Result<()> { + let sender = hdr + .sender() + .ok_or_else(|| fdo::Error::Failed("no sender on InvokeAction".to_owned()))? + .to_string(); + + self.validate_invoke(id, &sender, action_key) + .map_err(fdo::Error::Failed)?; + self.record_action(id, action_key); eprintln!("redbear-notifications: invoke action {id:?} -> {action_key:?}"); let _ = Self::action_invoked(&signal_emitter, id, action_key).await; + Ok(()) } #[zbus(signal, name = "NotificationClosed")] @@ -145,6 +197,58 @@ impl Notifications { ) } + // --- Notification tracking --- + + /// Record a newly-created notification with its owner and declared + /// action keys. + fn store_notification(&self, id: u32, owner: &str, action_keys: Vec) { + if let Ok(mut notifs) = self.notifications.lock() { + notifs.insert( + id, + NotificationRecord { + owner: owner.to_owned(), + action_keys, + }, + ); + } + } + + /// Remove a notification from the tracking map (called on close). + fn remove_notification(&self, id: u32) { + if let Ok(mut notifs) = self.notifications.lock() { + notifs.remove(&id); + } + } + + /// Validate an `InvokeAction` call. + /// + /// Returns `Ok(())` only when: + /// 1. The notification `id` exists. + /// 2. The caller (`sender`) is the same unique bus name that created it. + /// 3. The `action_key` was declared in the original `Notify` call. + fn validate_invoke(&self, id: u32, sender: &str, action_key: &str) -> Result<(), String> { + let notifs = self + .notifications + .lock() + .map_err(|_| "internal error: lock poisoned".to_owned())?; + + let record = notifs + .get(&id) + .ok_or_else(|| format!("unknown notification id {id}"))?; + + if record.owner != sender { + return Err("not notification owner".to_owned()); + } + + if !record.action_keys.iter().any(|k| k == action_key) { + return Err(format!("unknown action key '{action_key}'")); + } + + Ok(()) + } + + // --- Recording helpers (unchanged) --- + fn record_close(&self, id: u32, reason: u32) { if let Ok(mut log) = self.closed_notifications.lock() { log.push((id, reason)); @@ -157,6 +261,7 @@ impl Notifications { } } + #[cfg(test)] fn closed_notifications_snapshot(&self) -> Vec<(u32, u32)> { self.closed_notifications .lock() @@ -164,12 +269,25 @@ impl Notifications { .unwrap_or_default() } + #[cfg(test)] fn invoked_actions_snapshot(&self) -> Vec<(u32, String)> { self.invoked_actions .lock() .map(|g| g.iter().cloned().collect()) .unwrap_or_default() } + + #[cfg(test)] + fn notifications_snapshot(&self) -> Vec<(u32, String, Vec)> { + self.notifications + .lock() + .map(|g| { + g.iter() + .map(|(id, r)| (*id, r.owner.clone(), r.action_keys.clone())) + .collect() + }) + .unwrap_or_default() + } } enum Command { @@ -268,6 +386,11 @@ fn main() { mod tests { use super::*; + // ======================================================================= + // Existing tests — unchanged (they test internal helpers that did not + // change signature). All 8 remain green. + // ======================================================================= + #[test] fn notify_returns_monotonically_increasing_ids() { let n = Notifications::new(); @@ -420,6 +543,10 @@ mod tests { n.invoked_actions_snapshot().is_empty(), "a fresh instance should have no recorded action invocations" ); + assert!( + n.notifications_snapshot().is_empty(), + "a fresh instance should have no tracked notifications" + ); } #[test] @@ -441,4 +568,105 @@ mod tests { "action log should have 1 entry" ); } + + // ======================================================================= + // New tests — notification ownership tracking + // ======================================================================= + + #[test] + fn store_notification_tracks_owner_and_action_keys() { + let n = Notifications::new(); + n.store_notification( + 1, + ":1.42", + vec!["default".to_owned(), "reply".to_owned()], + ); + + let snapshot = n.notifications_snapshot(); + assert_eq!(snapshot.len(), 1); + let (id, owner, keys) = &snapshot[0]; + assert_eq!(*id, 1); + assert_eq!(owner, ":1.42"); + assert_eq!(keys, &vec!["default".to_owned(), "reply".to_owned()]); + } + + #[test] + fn remove_notification_clears_tracking() { + let n = Notifications::new(); + n.store_notification(1, ":1.1", vec!["default".to_owned()]); + assert_eq!(n.notifications_snapshot().len(), 1); + n.remove_notification(1); + assert!(n.notifications_snapshot().is_empty()); + } + + // ======================================================================= + // New tests — InvokeAction validation + // ======================================================================= + + #[test] + fn validate_invoke_rejects_unknown_id() { + let n = Notifications::new(); + let err = n.validate_invoke(999, ":1.1", "default").unwrap_err(); + assert!( + err.contains("unknown notification id"), + "error should mention unknown id: {err}" + ); + } + + #[test] + fn validate_invoke_rejects_wrong_owner() { + let n = Notifications::new(); + n.store_notification(1, ":1.1", vec!["default".to_owned()]); + + let err = n.validate_invoke(1, ":1.2", "default").unwrap_err(); + assert!( + err.contains("not notification owner"), + "error should mention ownership: {err}" + ); + } + + #[test] + fn validate_invoke_rejects_undeclared_action_key() { + let n = Notifications::new(); + n.store_notification(1, ":1.1", vec!["default".to_owned()]); + + let err = n.validate_invoke(1, ":1.1", "undeclared").unwrap_err(); + assert!( + err.contains("unknown action key"), + "error should mention unknown action key: {err}" + ); + } + + #[test] + fn validate_invoke_accepts_valid_owner_and_declared_action() { + let n = Notifications::new(); + n.store_notification(1, ":1.1", vec!["default".to_owned(), "reply".to_owned()]); + + // Correct owner + declared action key → Ok. + assert!(n.validate_invoke(1, ":1.1", "default").is_ok()); + assert!(n.validate_invoke(1, ":1.1", "reply").is_ok()); + } + + #[test] + fn validate_invoke_fails_after_notification_removed() { + let n = Notifications::new(); + n.store_notification(1, ":1.1", vec!["default".to_owned()]); + n.remove_notification(1); + + // After removal, the notification is unknown. + let err = n.validate_invoke(1, ":1.1", "default").unwrap_err(); + assert!(err.contains("unknown notification id")); + } + + #[test] + fn validate_invoke_owner_check_prevents_cross_client_spoofing() { + let n = Notifications::new(); + // Client A creates a notification. + n.store_notification(10, ":1.100", vec!["action1".to_owned()]); + // Client B tries to invoke an action on A's notification. + let err = n.validate_invoke(10, ":1.200", "action1").unwrap_err(); + assert!(err.contains("not notification owner")); + // Client A can invoke its own action. + assert!(n.validate_invoke(10, ":1.100", "action1").is_ok()); + } }