whippet: fix default values and wildcard usage

Use the correct default value for the `max_fds` fields of send and
receive records.

Replace wildcard values for send and receive rules with an empty string
as this is what the broker uses for fallback rules.

Signed-off-by: Arnaldo Garcia Rincon <agarrcia@amazon.com>

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

@@ -26,6 +26,8 @@ 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.
 ///
@@ -72,7 +74,7 @@ pub struct OwnRecord {
 /// https://github.com/bus1/dbus-broker/blob/b0db0890d1254477cf832e5f9f0a798360c80fd9/src/launch/policy.c#L877
 macro_rules! impl_record {
     ($record_type:ident, $rule_variant:path, [$(($struct_field:ident, $rule_field:ident)),*]) => {
-        #[derive(Debug, Type, Serialize, Clone, ZVariantValue, Default)]
+        #[derive(Debug, Type, Serialize, Clone, ZVariantValue)]
         pub struct $record_type {
             pub verdict: bool,
             pub priority: u64,
@@ -86,6 +88,23 @@ macro_rules! impl_record {
             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;
 
@@ -99,6 +118,10 @@ macro_rules! impl_record {
                     } => 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()
                     }),
@@ -113,23 +136,17 @@ macro_rules! impl_record {
     };
 }
 
-impl_record!(SendRecord, Rule::Send, [
-    (broadcast, send_broadcast),
-    (name, send_destination),
-    (interface, send_interface),
-    (member, send_member),
-    (path, send_path),
-    (record_type, send_type)
-]);
-
-impl_record!(ReceiveRecord, Rule::Receive, [
-    (broadcast, receive_broadcast),
-    (interface, receive_interface),
-    (member, receive_member),
-    (path, receive_path),
-    (name, receive_sender),
-    (record_type, receive_type)
-]);
+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
@@ -428,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");
+    }
 }