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) => {
-        #[derive(Debug, Type, Serialize, Clone, ZVariantValue, Default)]
+        #[derive(Debug, Type, Serialize, Clone, ZVariantValue)]
         pub struct $record_type {
             pub verdict: bool,
             pub priority: u64,
@@ -85,6 +87,23 @@ macro_rules! impl_record {
             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,
+                }
+            }
+        }
     };
 }
 
@@ -98,19 +117,16 @@ impl TryFrom<&Rule> for SendRecord {
                 allow,
                 priority,
                 send_broadcast,
-                send_destination,
-                send_interface,
-                send_member,
-                send_path,
                 send_type,
+                ..
             } => Ok(Self {
                 verdict: *allow,
                 priority: *priority,
                 broadcast: *send_broadcast,
-                name: send_destination.clone(),
-                interface: send_interface.clone(),
-                member: send_member.clone(),
-                path: send_path.clone(),
+                name: rule.name()?,
+                interface: rule.interface()?,
+                member: rule.member()?,
+                path: rule.path()?,
                 record_type: *send_type,
                 ..Self::default()
             }),
@@ -133,19 +149,16 @@ impl TryFrom<&Rule> for ReceiveRecord {
                 allow,
                 priority,
                 receive_broadcast,
-                receive_interface,
-                receive_member,
-                receive_path,
-                receive_sender,
                 receive_type,
+                ..
             } => Ok(Self {
                 verdict: *allow,
                 priority: *priority,
                 broadcast: *receive_broadcast,
-                interface: receive_interface.clone(),
-                member: receive_member.clone(),
-                path: receive_path.clone(),
-                name: receive_sender.clone(),
+                interface: rule.interface()?,
+                member: rule.member()?,
+                path: rule.path()?,
+                name: rule.name()?,
                 record_type: *receive_type,
                 ..Self::default()
             }),
@@ -455,4 +468,24 @@ 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();
+        policy.optimize();
+
+        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, "");
+    }
 }

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

@@ -466,6 +466,118 @@ fn find_priority_threshold(rules: &[Rule]) -> u64 {
 }
 
 impl Rule {
+    /// Returns the "interface" value used in the final policy, replacing wildcards with empty
+    /// strings
+    /// See: https://github.com/bus1/dbus-broker/blob/e3324b3736fd40d95e7943fca6e485013d15d643/src/launch/policy.c#L97
+    pub(crate) fn interface(&self) -> Result<String> {
+        match self {
+            Self::Send { send_interface, .. } => {
+                if send_interface == "*" {
+                    Ok("".to_string())
+                } else {
+                    Ok(send_interface.to_owned())
+                }
+            }
+            Self::Receive {
+                receive_interface, ..
+            } => {
+                if receive_interface == "*" {
+                    Ok("".to_string())
+                } else {
+                    Ok(receive_interface.to_owned())
+                }
+            }
+            _ => error::InvalidPropertyForRuleSnafu {
+                property: "interface".to_string(),
+                rule_type: format!("{self:?}"),
+            }
+            .fail(),
+        }
+    }
+
+    /// Returns the "name" value used in the final policy, replacing wildcards with empty
+    /// strings
+    /// See: https://github.com/bus1/dbus-broker/blob/e3324b3736fd40d95e7943fca6e485013d15d643/src/launch/policy.c#L97
+    pub(crate) fn name(&self) -> Result<String> {
+        match self {
+            Self::Send {
+                send_destination, ..
+            } => {
+                if send_destination == "*" {
+                    Ok("".to_string())
+                } else {
+                    Ok(send_destination.to_owned())
+                }
+            }
+            Self::Receive { receive_sender, .. } => {
+                if receive_sender == "*" {
+                    Ok("".to_string())
+                } else {
+                    Ok(receive_sender.to_owned())
+                }
+            }
+            _ => error::InvalidPropertyForRuleSnafu {
+                property: "name".to_string(),
+                rule_type: format!("{self:?}"),
+            }
+            .fail(),
+        }
+    }
+
+    /// Returns the "member" value used in the final policy, replacing wildcards with empty
+    /// strings
+    /// See: https://github.com/bus1/dbus-broker/blob/e3324b3736fd40d95e7943fca6e485013d15d643/src/launch/policy.c#L97
+    pub(crate) fn member(&self) -> Result<String> {
+        match self {
+            Self::Send { send_member, .. } => {
+                if send_member == "*" {
+                    Ok("".to_string())
+                } else {
+                    Ok(send_member.to_owned())
+                }
+            }
+            Self::Receive { receive_member, .. } => {
+                if receive_member == "*" {
+                    Ok("".to_string())
+                } else {
+                    Ok(receive_member.to_owned())
+                }
+            }
+            _ => error::InvalidPropertyForRuleSnafu {
+                property: "member".to_string(),
+                rule_type: format!("{self:?}"),
+            }
+            .fail(),
+        }
+    }
+
+    /// Returns the "path" value used in the final policy, replacing wildcards with empty
+    /// strings
+    /// See: https://github.com/bus1/dbus-broker/blob/e3324b3736fd40d95e7943fca6e485013d15d643/src/launch/policy.c#L97
+    pub(crate) fn path(&self) -> Result<String> {
+        match self {
+            Self::Send { send_path, .. } => {
+                if send_path == "*" {
+                    Ok("".to_string())
+                } else {
+                    Ok(send_path.to_owned())
+                }
+            }
+            Self::Receive { receive_path, .. } => {
+                if receive_path == "*" {
+                    Ok("".to_string())
+                } else {
+                    Ok(receive_path.to_owned())
+                }
+            }
+            _ => error::InvalidPropertyForRuleSnafu {
+                property: "path".to_string(),
+                rule_type: format!("{self:?}"),
+            }
+            .fail(),
+        }
+    }
+
     /// Sets the priority of the rule
     fn set_priority(&mut self, new_priority: u64) {
         match self {
@@ -551,6 +663,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 +1033,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(),
+        }; "witt 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");
+    }
 }