Merge pull request #720 from arnaldo2792/fix-whippet-defaults

Fix whippet defaults and wildcard replacements

Author: arnaldo2792

sources/Cargo.lock

@@ -24,3 +24,6 @@ zvariant = { workspace = true }
 
 [build-dependencies]
 generate-readme.workspace = true
+
+[dev-dependencies]
+test-case.workspace = true

sources/whippet/Cargo.toml

@@ -21,11 +21,13 @@
 //! expected by dbus-broker, ensuring compatibility with the broker's policy engine.
 
 use crate::error::{self, Result};
-use crate::policy::{Context, MessageType, Policy, Rule};
+use crate::policy::{Context, Policy, Rule};
 use serde::Serialize;
 use snafu::ResultExt;
 use zvariant::{Type, Value as ZVariantValue};
 
+const DEFAULT_MAX_FDS: u64 = u64::MAX;
+
 /// Top-level policy structure that matches launcher's Dbus format. It is crucial that
 /// the order of the fields remains like it is, otherwise the broker rejects the policy.
 ///
@@ -52,8 +54,8 @@ pub struct PolicyBatch {
     pub(crate) connect_verdict: bool,
     pub(crate) connect_priority: u64,
     pub(crate) own_rules: Vec<OwnRecord>,
-    pub(crate) send_rules: Vec<SendReceiveRecord>,
-    pub(crate) recv_rules: Vec<SendReceiveRecord>,
+    pub(crate) send_rules: Vec<SendRecord>,
+    pub(crate) recv_rules: Vec<ReceiveRecord>,
 }
 
 /// Represents an Own Record in the actual dbus policy
@@ -67,23 +69,85 @@ pub struct OwnRecord {
     pub name: String,
 }
 
-/// Represents a Send Record in the actual dbus policy
+/// Represents a Send/Receive Record in the actual dbus policy
 /// See:
 /// https://github.com/bus1/dbus-broker/blob/b0db0890d1254477cf832e5f9f0a798360c80fd9/src/launch/policy.c#L877
-#[derive(Debug, Type, Serialize, Clone, ZVariantValue, Default)]
-pub struct SendReceiveRecord {
-    pub verdict: bool,
-    pub priority: u64,
-    pub name: String,
-    pub path: String,
-    pub interface: String,
-    pub member: String,
-    pub record_type: MessageType,
-    pub broadcast: u32,
-    pub min_fds: u64,
-    pub max_fds: u64,
+macro_rules! impl_record {
+    ($record_type:ident, $rule_variant:path, [$(($struct_field:ident, $rule_field:ident)),*]) => {
+        #[derive(Debug, Type, Serialize, Clone, ZVariantValue)]
+        pub struct $record_type {
+            pub verdict: bool,
+            pub priority: u64,
+            pub name: String,
+            pub path: String,
+            pub interface: String,
+            pub member: String,
+            pub record_type: crate::policy::MessageType,
+            pub broadcast: u32,
+            pub min_fds: u64,
+            pub max_fds: u64,
+        }
+
+        impl Default for $record_type {
+            fn default() -> Self {
+                Self {
+                    verdict: Default::default(),
+                    priority: Default::default(),
+                    name: Default::default(),
+                    path: Default::default(),
+                    interface: Default::default(),
+                    member: Default::default(),
+                    record_type: Default::default(),
+                    broadcast: Default::default(),
+                    min_fds: Default::default(),
+                    max_fds: crate::dbus_policy::DEFAULT_MAX_FDS,
+                }
+            }
+        }
+
+        impl TryFrom<&Rule> for $record_type {
+            type Error = crate::error::Error;
+
+            fn try_from(rule: &Rule) -> Result<Self> {
+                match rule {
+                    $rule_variant {
+                        allow,
+                        priority,
+                        $($rule_field,)*
+                        ..
+                    } => Ok(Self {
+                        verdict: *allow,
+                        priority: *priority,
+                        name: rule.name()?,
+                        interface: rule.interface()?,
+                        member: rule.member()?,
+                        path: rule.path()?,
+                        $($struct_field: $rule_field.clone(),)*
+                        ..Self::default()
+                    }),
+                    _ => error::RuleToRecordSnafu {
+                        rule_type: format!("{rule:?}"),
+                        record_type: stringify!($record_type).to_string(),
+                    }
+                    .fail(),
+                }
+            }
+        }
+    };
 }
 
+impl_record!(
+    SendRecord,
+    Rule::Send,
+    [(broadcast, send_broadcast), (record_type, send_type)]
+);
+
+impl_record!(
+    ReceiveRecord,
+    Rule::Receive,
+    [(broadcast, receive_broadcast), (record_type, receive_type)]
+);
+
 impl DbusPolicy {
     /// Builds a new DbusPolicy object using "system" as the only supported bus_type
     fn new() -> Self {
@@ -221,63 +285,6 @@ impl TryFrom<&Rule> for OwnRecord {
     }
 }
 
-impl TryFrom<&Rule> for SendReceiveRecord {
-    type Error = crate::error::Error;
-
-    fn try_from(rule: &Rule) -> Result<Self> {
-        match rule {
-            Rule::Send {
-                allow,
-                send_destination,
-                send_path,
-                send_interface,
-                send_member,
-                send_type,
-                send_broadcast,
-                priority,
-                ..
-            } => Ok(SendReceiveRecord {
-                verdict: *allow,
-                name: send_destination.clone(),
-                path: send_path.clone(),
-                interface: send_interface.clone(),
-                member: send_member.clone(),
-                record_type: *send_type,
-                broadcast: *send_broadcast,
-                priority: *priority,
-                ..SendReceiveRecord::default()
-            }),
-
-            Rule::Receive {
-                receive_sender,
-                receive_path,
-                receive_interface,
-                receive_member,
-                receive_type,
-                receive_broadcast,
-                allow,
-                priority,
-                ..
-            } => Ok(SendReceiveRecord {
-                verdict: *allow,
-                name: receive_sender.clone(),
-                path: receive_path.clone(),
-                interface: receive_interface.clone(),
-                member: receive_member.clone(),
-                record_type: *receive_type,
-                broadcast: *receive_broadcast,
-                priority: *priority,
-                ..SendReceiveRecord::default()
-            }),
-            _ => error::RuleToRecordSnafu {
-                rule_type: format!("{rule:?}"),
-                record_type: "SendRecord".to_string(),
-            }
-            .fail(),
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -438,4 +445,42 @@ mod tests {
         let dbus_policy: DbusPolicy = policy.try_into().unwrap();
         assert_eq!(dbus_policy.uid_entries[1].1.send_rules.len(), 2);
     }
+
+    #[test]
+    fn test_wildcards_replaced_with_empty_string() {
+        let config_str = r#"
+            [default]
+            rules = [
+                { allow = true, send_interface = "*", send_destination = "*", send_path = "*", send_member = "*" },
+            ]
+        "#;
+        let mut policy: Policy = toml::from_str(config_str).unwrap();
+        policy.set_rule_priorities(&mut 0u64);
+        policy.prepare().unwrap();
+
+        let dbus_policy: DbusPolicy = policy.try_into().unwrap();
+        assert_eq!(dbus_policy.uid_entries[0].1.send_rules[0].interface, "");
+        assert_eq!(dbus_policy.uid_entries[0].1.send_rules[0].name, "");
+        assert_eq!(dbus_policy.uid_entries[0].1.send_rules[0].member, "");
+        assert_eq!(dbus_policy.uid_entries[0].1.send_rules[0].path, "");
+    }
+
+    #[test]
+    fn test_max_fds_are_always_the_default() {
+        let config_str = r#"
+            [default]
+            rules = [
+                { allow = true, send_interface = "*", send_destination = "*", send_path = "*", send_member = "*" },
+            ]
+        "#;
+        let mut policy: Policy = toml::from_str(config_str).unwrap();
+        policy.set_rule_priorities(&mut 0u64);
+        policy.prepare().unwrap();
+
+        let dbus_policy: DbusPolicy = policy.try_into().unwrap();
+        assert_eq!(
+            dbus_policy.uid_entries[0].1.send_rules[0].max_fds,
+            DEFAULT_MAX_FDS
+        );
+    }
 }

sources/whippet/src/dbus_policy.rs

@@ -133,4 +133,7 @@ pub enum Error {
         uid: String,
         source: std::num::ParseIntError,
     },
+
+    #[snafu(display("Can't get property '{property}' from rule '{rule_type}'"))]
+    InvalidPropertyForRule { property: String, rule_type: String },
 }

sources/whippet/src/error.rs

@@ -465,6 +465,31 @@ fn find_priority_threshold(rules: &[Rule]) -> u64 {
         .unwrap_or_default()
 }
 
+/// Macro to generate rule methods that extract fields and replace wildcards with empty strings
+/// See: https://github.com/bus1/dbus-broker/blob/e3324b3736fd40d95e7943fca6e485013d15d643/src/launch/policy.c#L97
+macro_rules! impl_wildcard_getter {
+    ($method_name:ident, ($($rule_type:ident => $field:ident),*)) => {
+        pub(crate) fn $method_name(&self) -> Result<String> {
+            match self {
+                $(
+                    Self::$rule_type { $field, .. } => {
+                        if $field == "*" {
+                            Ok("".to_string())
+                        } else {
+                            Ok($field.to_owned())
+                        }
+                    }
+                )*
+                _ => error::InvalidPropertyForRuleSnafu {
+                    property: stringify!($method_name).to_string(),
+                    rule_type: format!("{self:?}"),
+                }
+                .fail(),
+            }
+        }
+    };
+}
+
 impl Rule {
     /// Sets the priority of the rule
     fn set_priority(&mut self, new_priority: u64) {
@@ -489,6 +514,11 @@ impl Rule {
     fn is_connect(&self) -> bool {
         matches!(self, Self::ConnectUser { .. })
     }
+
+    impl_wildcard_getter!(interface, (Send => send_interface, Receive => receive_interface));
+    impl_wildcard_getter!(name, (Send => send_destination, Receive => receive_sender));
+    impl_wildcard_getter!(member, (Send => send_member, Receive => receive_member));
+    impl_wildcard_getter!(path, (Send => send_path, Receive => receive_path));
 }
 
 impl Context {
@@ -551,6 +581,7 @@ impl UsernameResolver for PasswdUsernameResolver {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use test_case::test_case;
 
     const ALICE_USER: u32 = 1;
     const BOB_USER: u32 = 2;
@@ -920,4 +951,58 @@ mod tests {
         clean_connect_rules(4, &mut rules);
         assert!(rules.is_empty());
     }
+
+    #[test_case(Rule::Send {
+            allow: false,
+            priority: u64::default(),
+            send_broadcast: u32::default(),
+            send_destination: "*".to_string(),
+            send_interface: "*".to_string(),
+            send_member: "*".to_string(),
+            send_path: "*".to_string(),
+            send_type: MessageType::default(),
+        }; "with a send rule wildcards are replaced with empty strings")]
+    #[test_case(Rule::Receive {
+            allow: false,
+            priority: u64::default(),
+            receive_broadcast: u32::default(),
+            receive_interface: "*".to_string(),
+            receive_member: "*".to_string(),
+            receive_path: "*".to_string(),
+            receive_sender: "*".to_string(),
+            receive_type: MessageType::default(),
+        }; "with a receive rule wildcards are replaced with empty strings")]
+    fn rules_with_wildcards(rule: Rule) {
+        assert_eq!(rule.name().unwrap(), "");
+        assert_eq!(rule.interface().unwrap(), "");
+        assert_eq!(rule.member().unwrap(), "");
+        assert_eq!(rule.path().unwrap(), "");
+    }
+
+    #[test_case(Rule::Send {
+            allow: false,
+            priority: u64::default(),
+            send_broadcast: u32::default(),
+            send_destination: "name".to_string(),
+            send_interface: "interface".to_string(),
+            send_member: "member".to_string(),
+            send_path: "path".to_string(),
+            send_type: MessageType::default(),
+        }; "with a send rule the original value is returned")]
+    #[test_case(Rule::Receive {
+            allow: false,
+            priority: u64::default(),
+            receive_broadcast: u32::default(),
+            receive_interface: "interface".to_string(),
+            receive_member: "member".to_string(),
+            receive_path: "path".to_string(),
+            receive_sender: "name".to_string(),
+            receive_type: MessageType::default(),
+        }; "with a receive rule the original value is returned")]
+    fn rules_without_wildcards(rule: Rule) {
+        assert_eq!(rule.name().unwrap(), "name");
+        assert_eq!(rule.interface().unwrap(), "interface");
+        assert_eq!(rule.member().unwrap(), "member");
+        assert_eq!(rule.path().unwrap(), "path");
+    }
 }