fix: Code review bug fixes and cleanup

deny.toml

@@ -14,31 +14,6 @@ targets = [
 
 [advisories]
 yanked = "deny"
-ignore = [
-    # https://rustsec.org/advisories/RUSTSEC-2023-0071
-    # "rsa" crate: Marvin Attack: potential key recovery through timing sidechannel
-    #
-    # No patch is yet available, however work is underway to migrate to a fully constant-time implementation
-    # So we need to accept this, as of SDP 25.3 we are not using the rsa crate to create certificates used in production
-    # setups.
-    #
-    # https://github.com/RustCrypto/RSA/issues/19 is the tracking issue
-    "RUSTSEC-2023-0071",
-
-    # https://rustsec.org/advisories/RUSTSEC-2024-0436
-    # The "paste" crate is no longer maintained because the owner states that the implementation is
-    # finished. There are at least two (forked) alternatives which state to be maintained. They'd
-    # need to be vetted before a potential switch. Additionally, they'd need to be in a maintained
-    # state for a couple of years to provide any benefit over using "paste".
-    #
-    # This crate is only used in a single place in the xtask package inside the declarative
-    # "write_crd" macro. The impact of vulnerabilities, if any, should be fairly minimal.
-    #
-    # See thread: https://users.rust-lang.org/t/paste-alternatives/126787/4
-    #
-    # This can only be removed again if we decide to use a different crate.
-    "RUSTSEC-2024-0436",
-]
 
 [bans]
 multiple-versions = "allow"

src/error.rs

@@ -31,7 +31,6 @@ impl<T> ComponentResult<T> {
                     error = &err as &dyn std::error::Error,
                     "error reported by {component}, ignoring...",
                 );
-                err.source();
                 ComponentResult::Err {
                     inner: ComponentError {
                         message: err.to_string(),

src/main.rs

@@ -68,13 +68,28 @@ async fn main() {
         if !next_run_sleep.is_zero() {
             tracing::info!(?next_run, "scheduling next run...");
         }
-        std::thread::sleep(next_run_sleep);
+        tokio::time::sleep(next_run_sleep).await;
 
         let system_information = SystemInformation::collect(&mut collect_ctx).await;
 
-        let serialized = serde_json::to_string_pretty(&system_information).unwrap();
-        if let Some(output_path) = &opts.output {
-            std::fs::write(output_path, &serialized).unwrap();
+        match serde_json::to_string_pretty(&system_information) {
+            Ok(serialized) => {
+                if let Some(output_path) = &opts.output
+                    && let Err(err) = std::fs::write(output_path, &serialized)
+                {
+                    tracing::error!(
+                        path = %output_path.display(),
+                        error = &err as &dyn std::error::Error,
+                        "failed to write JSON output file"
+                    );
+                }
+            }
+            Err(err) => {
+                tracing::error!(
+                    error = &err as &dyn std::error::Error,
+                    "failed to serialize system information"
+                );
+            }
         }
 
         match opts.loop_interval {

src/system_information/disk.rs

@@ -12,10 +12,10 @@ impl Disk {
     #[tracing::instrument(name = "Disk::collect_all")]
     pub fn collect_all() -> Vec<Self> {
         let disks = sysinfo::Disks::new_with_refreshed_list();
-        if disks.into_iter().next().is_none() {
+        if disks.list().is_empty() {
             tracing::info!("no disks found");
         }
-        disks.into_iter().map(Self::from).collect()
+        disks.list().iter().map(Self::from).collect()
     }
 }
 

src/system_information/mod.rs

@@ -15,7 +15,7 @@ pub struct SystemInformation {
     pub os: Option<os::OperatingSystem>,
     pub current_user: Option<ComponentResult<user::User>>,
     pub disks: Option<Vec<disk::Disk>>,
-    pub network: Option<network::SystemNetworkInfo>,
+    pub network: Option<ComponentResult<network::SystemNetworkInfo>>,
     // TODO:
     //  Current time
     //  SElinux/AppArmor
@@ -70,7 +70,10 @@ impl SystemInformation {
                 user::User::collect_current(&ctx.system),
             )),
             disks: Some(disk::Disk::collect_all()),
-            network: Some(network::SystemNetworkInfo::collect().await),
+            network: Some(ComponentResult::report_from_result(
+                "SystemNetworkInfo::collect",
+                network::SystemNetworkInfo::collect().await,
+            )),
             // ..Default::default()
         };
 

src/system_information/network.rs

@@ -3,73 +3,86 @@ use hickory_resolver::{
 };
 use local_ip_address::list_afinet_netifas;
 use serde::Serialize;
+use snafu::{ResultExt, Snafu};
 use std::{
-    collections::{BTreeSet, HashMap},
+    collections::{BTreeMap, BTreeSet},
     net::IpAddr,
     sync::LazyLock,
     time::Duration,
 };
 use tokio::task::JoinSet;
 
-static GLOBAL_DNS_RESOLVER: LazyLock<TokioResolver> = LazyLock::new(|| {
-    let (resolver_config, mut resolver_opts) =
-        read_system_conf().expect("failed to read system resolv config");
+#[derive(Debug, Snafu)]
+pub enum Error {
+    #[snafu(display("failed to list network interfaces"))]
+    ListInterfaces { source: local_ip_address::Error },
+}
+
+static GLOBAL_DNS_RESOLVER: LazyLock<Option<TokioResolver>> = LazyLock::new(|| {
+    let (resolver_config, mut resolver_opts) = match read_system_conf() {
+        Ok(conf) => conf,
+        Err(err) => {
+            tracing::error!(
+                error = &err as &dyn std::error::Error,
+                "failed to read system DNS config, DNS lookups will be skipped"
+            );
+            return None;
+        }
+    };
     resolver_opts.timeout = Duration::from_secs(5);
 
-    TokioResolver::builder_with_config(resolver_config, TokioConnectionProvider::default())
-        .with_options(resolver_opts)
-        .build()
+    Some(
+        TokioResolver::builder_with_config(resolver_config, TokioConnectionProvider::default())
+            .with_options(resolver_opts)
+            .build(),
+    )
 });
 
 /// Captures all system network information, including network interfaces,
 /// and the results of reverse and forward DNS lookups.
 #[derive(Debug, Serialize)]
 pub struct SystemNetworkInfo {
-    pub interfaces: HashMap<String, Vec<IpAddr>>,
-    pub reverse_lookups: HashMap<IpAddr, Vec<String>>,
-    pub forward_lookups: HashMap<String, Vec<IpAddr>>,
+    pub interfaces: BTreeMap<String, Vec<IpAddr>>,
+    pub reverse_lookups: BTreeMap<IpAddr, Vec<String>>,
+    pub forward_lookups: BTreeMap<String, Vec<IpAddr>>,
 }
 
 impl SystemNetworkInfo {
     #[tracing::instrument(name = "SystemNetworkInfo::collect")]
-    pub async fn collect() -> SystemNetworkInfo {
-        let interfaces = match list_afinet_netifas() {
-            Ok(netifs) => {
-                let mut interface_map = std::collections::HashMap::new();
+    pub async fn collect() -> Result<SystemNetworkInfo, Error> {
+        let netifs = list_afinet_netifas().context(ListInterfacesSnafu)?;
+        let mut interfaces = BTreeMap::new();
 
-                // Iterate over the network interfaces and group them by name
-                // An interface may appear multiple times if it has multiple IP addresses (e.g. IPv4 and IPv6)
-                for (name, ip_addr) in netifs {
-                    tracing::info!(
-                        network.interface.name = name,
-                        network.interface.address = %ip_addr,
-                        "found network interface"
-                    );
-                    interface_map
-                        .entry(name)
-                        .or_insert_with(Vec::new)
-                        .push(ip_addr);
-                }
-                interface_map
-            }
-            Err(error) => {
-                tracing::error!(
-                    error = &error as &dyn std::error::Error,
-                    "failed to list network interfaces"
-                );
-                HashMap::new()
-            }
-        };
+        // Iterate over the network interfaces and group them by name
+        // An interface may appear multiple times if it has multiple IP addresses (e.g. IPv4 and IPv6)
+        for (name, ip_addr) in netifs {
+            tracing::info!(
+                network.interface.name = name,
+                network.interface.address = %ip_addr,
+                "found network interface"
+            );
+            interfaces
+                .entry(name)
+                .or_insert_with(Vec::new)
+                .push(ip_addr);
+        }
 
         let ips: BTreeSet<IpAddr> = interfaces.values().flatten().copied().collect();
         tracing::info!(network.addresses.ip = ?ips, "ip addresses");
 
-        let mut reverse_lookups = JoinSet::new();
+        let Some(resolver) = GLOBAL_DNS_RESOLVER.as_ref() else {
+            return Ok(SystemNetworkInfo {
+                interfaces,
+                reverse_lookups: BTreeMap::new(),
+                forward_lookups: BTreeMap::new(),
+            });
+        };
+
+        let mut reverse_lookup_tasks = JoinSet::new();
         for ip in ips {
-            reverse_lookups
-                .spawn(async move { (ip, GLOBAL_DNS_RESOLVER.reverse_lookup(ip).await) });
+            reverse_lookup_tasks.spawn(async move { (ip, resolver.reverse_lookup(ip).await) });
         }
-        let reverse_lookups: HashMap<IpAddr, Vec<String>> = reverse_lookups
+        let reverse_lookups: BTreeMap<IpAddr, Vec<String>> = reverse_lookup_tasks
             .join_all()
             .await
             .into_iter()
@@ -96,16 +109,12 @@ impl SystemNetworkInfo {
         let hostname_set: BTreeSet<String> = reverse_lookups.values().flatten().cloned().collect();
         tracing::info!(network.addresses.hostname = ?hostname_set, "hostnames");
 
-        let mut forward_lookups = JoinSet::new();
+        let mut forward_lookup_tasks = JoinSet::new();
         for hostname in hostname_set {
-            forward_lookups.spawn(async move {
-                (
-                    hostname.clone(),
-                    GLOBAL_DNS_RESOLVER.lookup_ip(hostname).await,
-                )
-            });
+            forward_lookup_tasks
+                .spawn(async move { (hostname.clone(), resolver.lookup_ip(hostname).await) });
         }
-        let forward_lookups: HashMap<String, Vec<IpAddr>> = forward_lookups
+        let forward_lookups: BTreeMap<String, Vec<IpAddr>> = forward_lookup_tasks
             .join_all()
             .await
             .into_iter()
@@ -126,10 +135,10 @@ impl SystemNetworkInfo {
             })
             .collect();
 
-        SystemNetworkInfo {
+        Ok(SystemNetworkInfo {
             interfaces,
             reverse_lookups,
             forward_lookups,
-        }
+        })
     }
 }

src/system_information/user.rs

@@ -8,7 +8,7 @@ use crate::error::SysinfoError;
 pub enum Error {
     #[snafu(display("failed to get pid of the current process"))]
     GetCurrentPid { source: SysinfoError },
-    #[snafu(display("current pid {pid} could not be resolved to a proess"))]
+    #[snafu(display("current pid {pid} could not be resolved to a process"))]
     ResolveCurrentProcess { pid: Pid },
 }
 type Result<T, E = Error> = std::result::Result<T, E>;
@@ -53,7 +53,7 @@ impl User {
         tracing::info!(
             user.name,
             user.uid = user.uid.as_ref().map(|uid| format!("{uid:?}")),
-            user.gid = user.uid.as_ref().map(|gid| format!("{gid:?}")),
+            user.gid = user.gid.as_ref().map(|gid| format!("{gid:?}")),
             "current user"
         );
         Ok(user)