daemon: Use systemd to validate xdg-desktop-portal process

CHANGELOG.md

@@ -6,6 +6,7 @@
 - daemon: Allow Firefox to contact daemon on more distros with different app IDs (Thank you, @michaelbeaumont!)
 - daemon: Fix all subsequent requests being rejected after the UI failed to launch.
 - daemon: Don't busy loop polling for NFC devices when NFC is enabled.
+- daemon: Use systemd unit name to determine whether a caller is trusted. (Thank you, @NyCodeGHG!)
 - ui: Add Bulgarian translation (Thank you, @salif!)
 - webext: Try to read app ID from associated desktop entry (Thank you, @michaelbeaumont!)
 - ui: Basic recovery from failed attempts to use devices (e.g. flaky bluetooth)

credentialsd/src/gateway/dbus.rs

@@ -1,4 +1,9 @@
-use std::{collections::HashMap, fmt::Display, os::fd::AsRawFd, sync::Arc};
+use std::{
+    collections::HashMap,
+    fmt::Display,
+    os::fd::{AsFd, OwnedFd},
+    sync::Arc,
+};
 
 use serde::{Deserialize, Serialize, ser::SerializeTuple};
 use tokio::sync::Mutex as AsyncMutex;
@@ -336,22 +341,32 @@ async fn validate_app_details(
     let Some(unique_name) = header.sender() else {
         return Err(Error::SecurityError);
     };
-
-    let Some(pid) = query_peer_pid_via_fdinfo(connection, unique_name).await else {
+    let claimed_app_id = if !claimed_app_id.is_empty() {
+        claimed_app_id
+    } else {
+        tracing::warn!("App ID not specified on request. Rejecting request.");
         return Err(Error::SecurityError);
     };
 
-    if claimed_app_id.is_empty() || !super::should_trust_app_id(pid).await {
-        tracing::warn!(
-            ?claimed_app_id,
-            "App ID could not be verified. Rejecting request."
-        );
-        return Err(Error::SecurityError);
-    }
-    // Now we can trust these app detail parameters.
-    let Ok(app_id) = claimed_app_id.parse::<AppId>() else {
-        tracing::warn!("Invalid app ID passed: {claimed_app_id}");
-        return Err(Error::SecurityError);
+    let app_id = {
+        let Some(pidfd) = query_peer_pidfd(connection, unique_name).await else {
+            return Err(Error::SecurityError);
+        };
+
+        if !super::should_trust_app_id(connection, pidfd.as_fd()).await {
+            tracing::warn!(
+                ?claimed_app_id,
+                "App ID could not be verified. Rejecting request."
+            );
+            return Err(Error::SecurityError);
+        }
+
+        // Now we can trust these app detail parameters.
+        let Ok(app_id) = claimed_app_id.parse::<AppId>() else {
+            tracing::warn!("Invalid app ID passed: {claimed_app_id}");
+            return Err(Error::SecurityError);
+        };
+        app_id
     };
 
     // Verify that the origin is valid for the given app ID.
@@ -371,15 +386,17 @@ async fn validate_app_details(
 
     Ok(RequestContext {
         app_id,
-        pid,
+        // TODO: we're no longer using the PID in the UI, but we need to update
+        // the spec before removing it completely. For now, send invalid PID.
+        pid: 0,
         request_kind,
     })
 }
 
-async fn query_peer_pid_via_fdinfo(
+async fn query_peer_pidfd(
     connection: &Connection,
     sender_unique_name: &UniqueName<'_>,
-) -> Option<u32> {
+) -> Option<OwnedFd> {
     let dbus_proxy = match zbus::fdo::DBusProxy::new(connection).await {
         Ok(p) => p,
         Err(e) => {
@@ -399,41 +416,16 @@ async fn query_peer_pid_via_fdinfo(
         }
     };
 
-    let pidfd = match peer_credentials.process_fd() {
-        Some(p) => p.as_raw_fd(),
-        None => {
-            tracing::error!("Failed to get process fd from peer credentials");
-            return None;
-        }
+    let Some(pidfd) = peer_credentials.process_fd() else {
+        tracing::error!("Failed to get process fd from peer credentials");
+        return None;
     };
 
-    let fdinfo_str = match std::fs::read_to_string(format!("/proc/self/fdinfo/{pidfd}")) {
-        Ok(fdinfo) => fdinfo,
-        Err(e) => {
-            tracing::error!("Failed to read fdinfo from procfs: {e}");
-            return None;
-        }
-    };
-
-    // Find the line that starts with "Pid:"
-    let pid_line = match fdinfo_str.lines().find(|line| line.starts_with("Pid:")) {
-        Some(line) => line,
-        None => {
-            tracing::error!("Failed to read PID from fdinfo");
-            return None;
-        }
-    };
-
-    let pid_str = pid_line[4..].trim();
-
-    // std::process::id() also returns u32
-    let pid: u32 = match pid_str.parse() {
-        Ok(id) => id,
-        Err(e) => {
-            tracing::error!("Failed to parse PID from fdinfo entry: {e}");
-            return None;
-        }
-    };
-
-    Some(pid)
+    pidfd
+        .as_fd()
+        .try_clone_to_owned()
+        .inspect_err(
+            |err| tracing::error!(%err, "Failed to duplicate peer process file descriptor"),
+        )
+        .ok()
 }

credentialsd/src/gateway/mod.rs

@@ -7,6 +7,7 @@ mod util;
 use std::{
     collections::HashMap,
     fmt::Display,
+    os::fd::{AsRawFd, BorrowedFd},
     path::{Path, PathBuf},
     sync::Arc,
 };
@@ -229,20 +230,24 @@ fn validate_request(context: &RequestContext) -> Result<NavigationContext, WebAu
     Ok(request_environment)
 }
 
-async fn should_trust_app_id(pid: u32) -> bool {
+async fn should_trust_app_id(connection: &Connection, pidfd: BorrowedFd<'_>) -> bool {
+    // Verify by systemd unit name
+    if let Ok(true) = check_by_systemd_unit_name(connection, pidfd).await {
+        return true;
+    }
+
     // Verify if we should trust the peer based on the file name. We verify that
     // we're in the same mount namespace before using the exe path.
-
+    let Some(pid) = read_pid_from_pidfd(pidfd).await else {
+        return false;
+    };
     // TODO: If the portal is running in a separate mount namespace for security
     // reasons, then this check will fail with a false negative.
-    // In the future, we should retrieve this information from another trusted
-    // source, e.g. check if the PID is in a cgroup managed by systemd and
-    // corresponds to the org.freedesktop.portal.Desktop D-Bus service unit.
     let Ok(my_mnt_ns) = tokio::fs::read_link("/proc/self/ns/mnt").await else {
         tracing::debug!("Could not read peer mount namespace");
         return false;
     };
-    let Ok(peer_mnt_ns) = tokio::fs::read_link(format!("/proc/{pid}/ns/mnt")).await else {
+    let Ok(peer_mnt_ns) = tokio::fs::read_link(format!("/proc/{}/ns/mnt", pid)).await else {
         tracing::debug!("Could not determine our mount namespace");
         return false;
     };
@@ -256,12 +261,12 @@ async fn should_trust_app_id(pid: u32) -> bool {
         return false;
     }
 
-    let Ok(exe_path) = tokio::fs::read_link(format!("/proc/{pid}/exe")).await else {
+    let Ok(exe_path) = tokio::fs::read_link(format!("/proc/{}/exe", pid)).await else {
         tracing::warn!("Cannot read executable name from procfs");
         return false;
     };
 
-    tracing::debug!(?exe_path, %pid, "Found executable path:");
+    tracing::debug!(?exe_path, pidfd = %pidfd.as_raw_fd(), "Found executable path:");
 
     let mut trusted_callers: Vec<PathBuf> = vec![
         PathBuf::from("/usr/lib/xdg-desktop-portal"),
@@ -289,6 +294,50 @@ async fn should_trust_app_id(pid: u32) -> bool {
     }
 }
 
+async fn check_by_systemd_unit_name(
+    connection: &Connection,
+    pidfd: BorrowedFd<'_>,
+) -> zbus::Result<bool> {
+    let systemd_manager = crate::systemd::Manager::create(connection).await?;
+
+    systemd_manager
+        .match_unit_name_by_pidfd(pidfd, "xdg-desktop-portal.service")
+        .await
+}
+
+async fn read_pid_from_pidfd<'a>(pidfd: BorrowedFd<'a>) -> Option<u32> {
+    let pidfd_raw = pidfd.as_raw_fd();
+    let fdinfo_str = match std::fs::read_to_string(format!("/proc/self/fdinfo/{pidfd_raw}")) {
+        Ok(fdinfo) => fdinfo,
+        Err(e) => {
+            tracing::error!("Failed to read fdinfo from procfs: {e}");
+            return None;
+        }
+    };
+
+    // Find the line that starts with "Pid:"
+    let pid_line = match fdinfo_str.lines().find(|line| line.starts_with("Pid:")) {
+        Some(line) => line,
+        None => {
+            tracing::error!("Failed to read PID from fdinfo");
+            return None;
+        }
+    };
+
+    let pid_str = pid_line[4..].trim();
+
+    // std::process::id() also returns u32
+    let pid: u32 = match pid_str.parse() {
+        Ok(id) => id,
+        Err(e) => {
+            tracing::error!("Failed to parse PID from fdinfo entry: {e}");
+            return None;
+        }
+    };
+
+    Some(pid)
+}
+
 fn check_origin_from_app(
     app_id: &AppId,
     origin: Origin,

credentialsd/src/main.rs

@@ -2,6 +2,7 @@ mod credential_service;
 mod dbus;
 mod gateway;
 mod model;
+pub mod systemd;
 mod webauthn;
 
 use std::error::Error;

credentialsd/src/systemd.rs

@@ -0,0 +1,61 @@
+use std::os::fd::BorrowedFd;
+
+use zbus::{
+    Connection, proxy,
+    zvariant::{Fd, OwnedObjectPath},
+};
+
+pub struct Manager<'a> {
+    proxy: Systemd1ManagerProxy<'a>,
+}
+
+impl<'a> Manager<'a> {
+    pub async fn create(connection: &'a Connection) -> zbus::Result<Self> {
+        let proxy = Systemd1ManagerProxy::new(connection).await?;
+        Ok(Self { proxy })
+    }
+
+    /// Returns `true` if the process referred to by `pidfd` is part of the
+    /// systemd unit identified by `name`.
+    pub async fn match_unit_name_by_pidfd(
+        &self,
+        pidfd: BorrowedFd<'_>,
+        name: &str,
+    ) -> zbus::Result<bool> {
+        let (path, _, _) = self
+            .proxy
+            .get_unit_by_pidfd(pidfd.into())
+            .await
+            .inspect_err(|err| tracing::error!(%err, "Failed to lookup unit by pidfd"))?;
+        let conn = self.proxy.inner().connection();
+        let systemd_unit = Systemd1UnitProxy::new(conn, path).await?;
+        let unit_names = systemd_unit
+            .names()
+            .await
+            .inspect_err(|err| tracing::error!(%err, "Failed to look up systemd unit names"))?;
+        let is_match = unit_names.iter().any(|unit_name| unit_name == name);
+        Ok(is_match)
+    }
+}
+
+#[proxy(
+    interface = "org.freedesktop.systemd1.Manager",
+    default_service = "org.freedesktop.systemd1",
+    default_path = "/org/freedesktop/systemd1"
+)]
+trait Systemd1Manager {
+    #[zbus(name = "GetUnitByPIDFD")]
+    fn get_unit_by_pidfd(&self, pidfd: Fd<'_>) -> zbus::Result<(OwnedObjectPath, String, Vec<u8>)>;
+}
+
+#[proxy(
+    interface = "org.freedesktop.systemd1.Unit",
+    default_service = "org.freedesktop.systemd1"
+)]
+trait Systemd1Unit {
+    #[zbus(property)]
+    fn id(&self) -> zbus::Result<String>;
+
+    #[zbus(property)]
+    fn names(&self) -> zbus::Result<Vec<String>>;
+}