Finish implementation of gethostbyname

.gitignore

@@ -4,6 +4,9 @@
 *.orig
 *~
 
+.gdbinit
+.lldbinit
+
 /build
 /target
 /Cargo.lock

Cargo.toml

@@ -26,7 +26,7 @@ bindgen = "0.60"
 
 [dev-dependencies]
 malcontent-nss = { path = ".", features = ["integration_test"] }
-event-listener = "2.5"
+env_logger = "0.9"
 futures-util = "0.3"
 rusty-hook = "0.11"
 rusty-forkfork = "0.4"

src/gethostbyname.rs

@@ -21,6 +21,7 @@ use {
 // See https://support.mozilla.org/en-US/kb/configuring-networks-disable-dns-over-https
 const CANARY_HOSTNAME: &str = "use-application-dns.net";
 
+#[derive(Debug)]
 pub struct Args {
     pub name: *const c_char,
     pub family: c_int,
@@ -32,6 +33,7 @@ pub struct Args {
     pub result: Result,
 }
 
+#[derive(Debug)]
 pub enum Result {
     V3(*mut hostent),
     V4(*mut *mut gaih_addrtuple),

src/policy_checker/dbus.rs

@@ -2,6 +2,7 @@
 // SPDX-License-Identifier: GPL-3.0-or-later
 
 use {
+    nix::unistd::Uid,
     serde::{Deserialize, Serialize},
     std::net::IpAddr,
     zbus::{dbus_proxy, Result},
@@ -24,3 +25,36 @@ pub struct Restriction {
 }
 
 pub type Restrictions = Vec<Restriction>;
+
+pub async fn restrictions_for(user: Uid) -> anyhow::Result<Vec<Restriction>, anyhow::Error> {
+    #[cfg(not(feature = "integration_test"))]
+    let proxy = {
+        // This is the normal behavior
+        let connection = zbus::Connection::system().await?;
+        MalcontentDnsProxy::new(&connection).await?
+    };
+
+    #[cfg(feature = "integration_test")]
+    let proxy = {
+        // During integration testing, we want to connect to a private
+        // bus name to avoid clashes with existing system services.
+        let connection = zbus::Connection::session().await?;
+        let dbus_name = std::env::var("TEST_DBUS_SERVICE_NAME")
+            .expect("The test has not set the TEST_DBUS_SERVICE_NAME environment variable to the private bus name prior to attempting name resolution");
+        MalcontentDnsProxy::builder(&connection)
+            .destination(zbus_names::UniqueName::try_from(dbus_name).unwrap())
+            .unwrap()
+            .build()
+            .await
+            .expect("Unable to build DBus proxy object")
+    };
+
+    let restrictions = proxy.get_restrictions(user.as_raw()).await;
+    log::trace!(
+        "malcontent-nss: user {} restrictions are {:?}",
+        user,
+        &restrictions
+    );
+
+    Ok(restrictions?)
+}

src/policy_checker/mod.rs

@@ -4,7 +4,6 @@
 mod dbus;
 
 use {
-    self::dbus::*,
     anyhow::Result,
     nix::unistd::{getuid, Uid},
     once_cell::sync::Lazy,
@@ -41,31 +40,7 @@ impl PolicyChecker {
             return Ok(vec![]);
         };
 
-        let connection = zbus::Connection::session().await?;
+        dbus::restrictions_for(user).await
-
-        #[cfg(not(feature = "integration_test"))]
-        let proxy = MalcontentDnsProxy::new(&connection).await?;
-
-        #[cfg(feature = "integration_test")]
-        let proxy = {
-            let dbus_name = std::env::var("TEST_DBUS_SERVICE_NAME").map_err(|_| {
-                anyhow::anyhow!("The test hasn't set the TEST_DBUS_SERVICE_NAME environment var")
-            })?;
-            MalcontentDnsProxy::builder(&connection)
-                .destination(zbus_names::UniqueName::try_from(dbus_name).unwrap())
-                .unwrap()
-                .build()
-                .await
-                .expect("Unable to build DBus proxy object")
-        };
-
-        let restrictions = proxy.get_restrictions(user.as_raw()).await;
-        log::trace!(
-            "malcontent-nss: user {} restrictions are {:?}",
-            user,
-            &restrictions
-        );
-        Ok(restrictions?)
     }
 
     pub async fn resolver(&self, user: Option<Uid>) -> Result<Option<Arc<TokioAsyncResolver>>> {

tests/common/dbus.rs

@@ -6,17 +6,12 @@ include!(concat!(
     "/src/policy_checker/dbus.rs"
 ));
 
-use {
+use {std::collections::HashMap, zbus::dbus_interface};
-    event_listener::{Event, EventListener},
-    nix::unistd::Uid,
-    std::collections::HashMap,
-    zbus::dbus_interface,
-};
 
+#[derive(Debug)]
 pub struct MalcontentDBusMock {
     responses: HashMap<Uid, Vec<Restrictions>>,
     invocations_left: usize,
-    finished: Event,
 }
 
 #[dbus_interface(name = "com.endlessm.ParentalControls.Dns")]
@@ -33,17 +28,23 @@ impl MalcontentDBusMock {
             "MockError: DBus mock is saturated for user with id {}",
             user_id
         ));
+
         self.invocations_left -= 1;
-        if self.invocations_left == 0 {
-            self.finished.notify(1);
-        }
         restrictions
     }
 }
 
 impl MalcontentDBusMock {
     pub fn new(mut responses: HashMap<Uid, Vec<Restrictions>>) -> Self {
-        let responses_size: usize = responses.values().map(|v| v.len()).sum();
+        let responses_size: usize = responses
+            .values()
+            .map(|v| {
+                std::cmp::max(
+                    v.len(),
+                    1, /* 'No restrictions' still counts as one message */
+                )
+            })
+            .sum();
         for r in responses.values_mut() {
             r.reverse(); // we pop responses from the back, so...
         }
@@ -51,19 +52,10 @@ impl MalcontentDBusMock {
         let ret = Self {
             responses,
             invocations_left: responses_size,
-            finished: Event::new(),
         };
 
-        if ret.invocations_left == 0 {
-            ret.finished.notify(1);
-        }
-
         ret
     }
-
-    pub fn waiter(&self) -> EventListener {
-        self.finished.listen()
-    }
 }
 
 impl Drop for MalcontentDBusMock {
@@ -73,8 +65,6 @@ impl Drop for MalcontentDBusMock {
             "MockError: During teardown, {} invocations are still left on the mock object",
             self.invocations_left
         );
-
-        self.finished.notify(1);
     }
 }
 

tests/common/mod.rs

@@ -91,6 +91,8 @@ static SETUP: Once = Once::new();
 
 pub fn setup() -> Result<()> {
     SETUP.call_once(|| {
+        env_logger::init();
+
         let nss_config_status = unsafe {
             let db = CString::new("hosts").unwrap();
             let resolvers = CString::new("malcontent dns").unwrap();
@@ -130,7 +132,7 @@ pub fn resolve_with_system(family: libc::c_int, host: &str) -> Result<IpAddr> {
     Ok(IpAddr::from_str(&addr_string)?)
 }
 
-pub fn resolve_with_module(family: libc::c_int, hostname: &str) -> Result<IpAddr> {
+pub fn resolve_with_module(family: libc::c_int, hostname: &str) -> Result<Vec<IpAddr>> {
     assert!(
         SETUP.is_completed(),
         "Forgot to call common::setup() at beginning of test?"
@@ -159,20 +161,33 @@ pub fn resolve_with_module(family: libc::c_int, hostname: &str) -> Result<IpAddr
             hostname,
             error.to_str().unwrap()
         );
-        let addr_storage = SockaddrStorage::from_raw((*addr).ai_addr, Some((*addr).ai_addrlen))
+
-            .ok_or(anyhow!("Garbled addrinfo from getaddrinfo()"))?;
+        let ips = convert_addrinfo(addr)?;
-        let ip = convert_addrinfo(&addr_storage)?;
         freeaddrinfo(addr);
-        Ok(ip)
+        Ok(ips)
     }
 }
 
-fn convert_addrinfo(sa: &SockaddrStorage) -> Result<IpAddr> {
+unsafe fn convert_addrinfo(addrs: *const libc::addrinfo) -> Result<Vec<IpAddr>> {
-    if let Some(addr) = sa.as_sockaddr_in() {
+    let mut ips = vec![];
-        Ok(IpAddr::V4(Ipv4Addr::from(addr.ip())))
+
-    } else if let Some(addr) = sa.as_sockaddr_in6() {
+    let mut addr = addrs;
-        Ok(IpAddr::V6(Ipv6Addr::from(addr.ip())))
+    while addr != std::ptr::null() {
+        let addr_storage = SockaddrStorage::from_raw((*addr).ai_addr, Some((*addr).ai_addrlen))
+            .ok_or(anyhow!("Garbled addrinfo from getaddrinfo()"))?;
+
+        if let Some(addr) = addr_storage.as_sockaddr_in() {
+            ips.push(IpAddr::V4(Ipv4Addr::from(addr.ip())));
+        } else if let Some(addr) = addr_storage.as_sockaddr_in6() {
+            ips.push(IpAddr::V6(Ipv6Addr::from(addr.ip())));
         } else {
             bail!("addrinfo is not either an IPv4 or IPv6 address")
         }
+
+        addr = (*addr).ai_next;
+    }
+
+    ips.sort();
+    ips.dedup();
+    Ok(ips)
 }

tests/integration_test.rs

@@ -17,29 +17,34 @@ static CLOUDFLARE_PARENTALCONTROL_ADDRS: Lazy<Restrictions> = Lazy::new(|| {
     vec![
         Restriction {
             ip: IpAddr::V4(Ipv4Addr::new(1, 1, 1, 3)),
-            hostname: "cloudflare-dns.com".into(),
+            hostname: "family.cloudflare-dns.com".into(),
         },
         Restriction {
             ip: IpAddr::V4(Ipv4Addr::new(1, 0, 0, 3)),
-            hostname: "cloudflare-dns.com".into(),
+            hostname: "family.cloudflare-dns.com".into(),
         },
         Restriction {
-            ip: IpAddr::V6(Ipv6Addr::new(2606, 4700, 4700, 0, 0, 0, 0, 1113)),
+            ip: IpAddr::V6(Ipv6Addr::new(0x2606, 0x4700, 0x4700, 0, 0, 0, 0, 0x1113)),
-            hostname: "cloudflare-dns.com".into(),
+            hostname: "family.cloudflare-dns.com".into(),
         },
         Restriction {
-            ip: IpAddr::V6(Ipv6Addr::new(2606, 4700, 4700, 0, 0, 0, 0, 1003)),
+            ip: IpAddr::V6(Ipv6Addr::new(0x2606, 0x4700, 0x4700, 0, 0, 0, 0, 0x1003)),
-            hostname: "cloudflare-dns.com".into(),
+            hostname: "family.cloudflare-dns.com".into(),
         },
     ]
 });
 
+static NO_RESTRICTIONS: Lazy<Restrictions> = Lazy::new(|| vec![]);
+
 fork_test! {
     #[test]
     fn nss_module_is_loaded() -> Result<()> {
         common::setup()?;
+        tokio::runtime::Runtime::new().unwrap().block_on(async {
+            let _dbus = common::mock_dbus(HashMap::from([(getuid(), vec![NO_RESTRICTIONS.clone()])])).await?;
             common::resolve_with_module(libc::AF_INET, "gnome.org")?;
             Ok(())
+        })
     }
 
     #[test]
@@ -124,8 +129,8 @@ fork_test! {
                 )])).await?;
 
                 let system_addr = common::resolve_with_system(family, HOSTNAME)?;
-                let our_addr = common::resolve_with_module(family, HOSTNAME)?;
+                let our_addrs = common::resolve_with_module(family, HOSTNAME)?;
-                assert_eq!(system_addr, our_addr);
+                assert!(our_addrs.contains(&system_addr));
             }
             Ok(())
         })
@@ -144,10 +149,9 @@ fork_test! {
             )])).await?;
 
             let system_addr = common::resolve_with_system(libc::AF_INET, HOSTNAME)?;
-            let our_addr = common::resolve_with_module(libc::AF_INET, HOSTNAME)?;
+            let our_addrs = common::resolve_with_module(libc::AF_INET, HOSTNAME)?;
-            assert_ne!(system_addr, our_addr);
+            assert!(!our_addrs.contains(&system_addr));
-            assert_eq!(our_addr, IpAddr::V4(Ipv4Addr::UNSPECIFIED));
+            assert_eq!(our_addrs, [IpAddr::V4(Ipv4Addr::UNSPECIFIED)]);
-
             Ok(())
         })
     }
@@ -165,9 +169,9 @@ fork_test! {
             )])).await;
 
             let system_addr = common::resolve_with_system(libc::AF_INET6, HOSTNAME)?;
-            let our_addr = common::resolve_with_module(libc::AF_INET6, HOSTNAME)?;
+            let our_addrs = common::resolve_with_module(libc::AF_INET6, HOSTNAME)?;
-            assert_ne!(system_addr, our_addr);
+            assert!(!our_addrs.contains(&system_addr));
-            assert_eq!(our_addr, IpAddr::V6(Ipv6Addr::UNSPECIFIED));
+            assert_eq!(our_addrs, [IpAddr::V6(Ipv6Addr::UNSPECIFIED)]);
             Ok(())
         })
     }
@@ -176,14 +180,14 @@ fork_test! {
     fn privileged_user_bypasses_restrictions() -> Result<()> {
         common::setup()?;
 
-        const HOSTNAME: &str = "nudity.testcategory.com";
+        const HOSTNAME: &str = "malware.testcategory.com";
 
         tokio::runtime::Runtime::new().unwrap().block_on(async {
             for family in [libc::AF_INET, libc::AF_INET6] {
-                let _dbus = common::mock_dbus(HashMap::from([(getuid(), vec![ /* no restriction */])])).await?;
+                let _dbus = common::mock_dbus(HashMap::from([(getuid(), vec![NO_RESTRICTIONS.clone()])])).await?;
                 let system_addr = common::resolve_with_system(family, HOSTNAME)?;
-                let our_addr = common::resolve_with_module(family, HOSTNAME)?;
+                let our_addrs = common::resolve_with_module(family, HOSTNAME)?;
-                assert_eq!(system_addr, our_addr);
+                assert!(our_addrs.contains(&system_addr));
             }
             Ok(())
         })