From 571aa90d6a95b9ab8225856bcd1a68269b6a4265 Mon Sep 17 00:00:00 2001 From: Matteo Settenvini Date: Mon, 5 Sep 2022 23:15:29 +0200 Subject: [PATCH] Finish implementation of gethostbyname --- .gitignore | 3 +++ Cargo.toml | 2 +- src/gethostbyname.rs | 2 ++ src/policy_checker/dbus.rs | 34 ++++++++++++++++++++++++++++ src/policy_checker/mod.rs | 27 +--------------------- tests/common/dbus.rs | 34 ++++++++++------------------ tests/common/mod.rs | 39 ++++++++++++++++++++++---------- tests/integration_test.rs | 46 +++++++++++++++++++++----------------- 8 files changed, 105 insertions(+), 82 deletions(-) diff --git a/.gitignore b/.gitignore index 3f04c2d..c516175 100644 --- a/.gitignore +++ b/.gitignore @@ -4,6 +4,9 @@ *.orig *~ +.gdbinit +.lldbinit + /build /target /Cargo.lock diff --git a/Cargo.toml b/Cargo.toml index df07ab8..27abd7c 100644 --- a/Cargo.toml +++ b/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" diff --git a/src/gethostbyname.rs b/src/gethostbyname.rs index 578a76b..59a85b4 100644 --- a/src/gethostbyname.rs +++ b/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), diff --git a/src/policy_checker/dbus.rs b/src/policy_checker/dbus.rs index 09ee099..c379894 100644 --- a/src/policy_checker/dbus.rs +++ b/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; + +pub async fn restrictions_for(user: Uid) -> anyhow::Result, 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?) +} diff --git a/src/policy_checker/mod.rs b/src/policy_checker/mod.rs index e96af2c..1c39d3b 100644 --- a/src/policy_checker/mod.rs +++ b/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?; - - #[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?) + dbus::restrictions_for(user).await } pub async fn resolver(&self, user: Option) -> Result>> { diff --git a/tests/common/dbus.rs b/tests/common/dbus.rs index 1e88816..fee255d 100644 --- a/tests/common/dbus.rs +++ b/tests/common/dbus.rs @@ -6,17 +6,12 @@ include!(concat!( "/src/policy_checker/dbus.rs" )); -use { - event_listener::{Event, EventListener}, - nix::unistd::Uid, - std::collections::HashMap, - zbus::dbus_interface, -}; +use {std::collections::HashMap, zbus::dbus_interface}; +#[derive(Debug)] pub struct MalcontentDBusMock { responses: HashMap>, 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>) -> 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); } } diff --git a/tests/common/mod.rs b/tests/common/mod.rs index ff9f2a8..d70a794 100644 --- a/tests/common/mod.rs +++ b/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 { Ok(IpAddr::from_str(&addr_string)?) } -pub fn resolve_with_module(family: libc::c_int, hostname: &str) -> Result { +pub fn resolve_with_module(family: libc::c_int, hostname: &str) -> Result> { 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 Result { - if let Some(addr) = sa.as_sockaddr_in() { - Ok(IpAddr::V4(Ipv4Addr::from(addr.ip()))) - } else if let Some(addr) = sa.as_sockaddr_in6() { - Ok(IpAddr::V6(Ipv6Addr::from(addr.ip()))) - } else { - bail!("addrinfo is not either an IPv4 or IPv6 address") +unsafe fn convert_addrinfo(addrs: *const libc::addrinfo) -> Result> { + let mut ips = vec![]; + + let mut addr = addrs; + 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) } diff --git a/tests/integration_test.rs b/tests/integration_test.rs index 78c0ef9..65cb585 100644 --- a/tests/integration_test.rs +++ b/tests/integration_test.rs @@ -17,29 +17,34 @@ static CLOUDFLARE_PARENTALCONTROL_ADDRS: Lazy = 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)), - hostname: "cloudflare-dns.com".into(), + ip: IpAddr::V6(Ipv6Addr::new(0x2606, 0x4700, 0x4700, 0, 0, 0, 0, 0x1113)), + hostname: "family.cloudflare-dns.com".into(), }, Restriction { - ip: IpAddr::V6(Ipv6Addr::new(2606, 4700, 4700, 0, 0, 0, 0, 1003)), - hostname: "cloudflare-dns.com".into(), + ip: IpAddr::V6(Ipv6Addr::new(0x2606, 0x4700, 0x4700, 0, 0, 0, 0, 0x1003)), + hostname: "family.cloudflare-dns.com".into(), }, ] }); +static NO_RESTRICTIONS: Lazy = Lazy::new(|| vec![]); + fork_test! { #[test] fn nss_module_is_loaded() -> Result<()> { common::setup()?; - common::resolve_with_module(libc::AF_INET, "gnome.org")?; - Ok(()) + 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)?; - assert_eq!(system_addr, our_addr); + let our_addrs = common::resolve_with_module(family, HOSTNAME)?; + 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)?; - assert_ne!(system_addr, our_addr); - assert_eq!(our_addr, IpAddr::V4(Ipv4Addr::UNSPECIFIED)); - + let our_addrs = common::resolve_with_module(libc::AF_INET, HOSTNAME)?; + assert!(!our_addrs.contains(&system_addr)); + 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)?; - assert_ne!(system_addr, our_addr); - assert_eq!(our_addr, IpAddr::V6(Ipv6Addr::UNSPECIFIED)); + let our_addrs = common::resolve_with_module(libc::AF_INET6, HOSTNAME)?; + assert!(!our_addrs.contains(&system_addr)); + 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)?; - assert_eq!(system_addr, our_addr); + let our_addrs = common::resolve_with_module(family, HOSTNAME)?; + assert!(our_addrs.contains(&system_addr)); } Ok(()) })