Skip to content

Commit 279a5a2

Browse files
committed
Address review: Use more secure fdinfo for parsing senders PID, if possible.
1 parent 7ba3655 commit 279a5a2

1 file changed

Lines changed: 98 additions & 10 deletions

File tree

credentialsd/src/dbus/gateway.rs

Lines changed: 98 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,19 @@ use credentialsd_common::{
1414
},
1515
};
1616
use tokio::sync::Mutex as AsyncMutex;
17-
use zbus::{fdo, interface, message::Header, Connection, DBusError};
17+
use zbus::{
18+
fdo, interface,
19+
message::Header,
20+
names::{BusName, UniqueName},
21+
Connection, DBusError,
22+
};
1823

1924
use crate::dbus::{
2025
create_credential_request_try_into_ctap2, create_credential_response_try_from_ctap2,
2126
get_credential_request_try_into_ctap2, get_credential_response_try_from_ctap2,
2227
CredentialRequestController,
2328
};
29+
use std::os::fd::AsRawFd;
2430

2531
pub const SERVICE_NAME: &str = "xyz.iinuwa.credentialsd.Credentials";
2632
pub const SERVICE_PATH: &str = "/xyz/iinuwa/credentialsd/Credentials";
@@ -47,16 +53,73 @@ struct CredentialGateway<C: CredentialRequestController> {
4753
controller: Arc<AsyncMutex<C>>,
4854
}
4955

50-
async fn query_connection_peer_binary(
51-
header: Header<'_>,
56+
async fn query_peer_pid_via_fdinfo(
5257
connection: &Connection,
53-
) -> Option<RequestingApplication> {
54-
// 1. Get the sender's unique bus name
55-
let sender_unique_name = header.sender()?;
58+
sender_unique_name: &UniqueName<'_>,
59+
) -> Option<u32> {
60+
let dbus_proxy = match zbus::fdo::DBusProxy::new(connection).await {
61+
Ok(p) => p,
62+
Err(e) => {
63+
tracing::error!("Failed to establish DBus proxy to query peer info: {e:?}");
64+
return None;
65+
}
66+
};
5667

57-
tracing::debug!("Received request from sender: {}", sender_unique_name);
68+
let peer_credentials = match dbus_proxy
69+
.get_connection_credentials(BusName::from(sender_unique_name.to_owned()))
70+
.await
71+
{
72+
Ok(c) => c,
73+
Err(e) => {
74+
tracing::error!("Failed to get peer credentials: {e:?}");
75+
return None;
76+
}
77+
};
78+
79+
let pidfd = match peer_credentials.process_fd() {
80+
Some(p) => p.as_raw_fd(),
81+
None => {
82+
tracing::error!("Failed to get process fd from peer credentials");
83+
return None;
84+
}
85+
};
86+
87+
let fdinfo_str = match std::fs::read_to_string(format!("/proc/self/fdinfo/{pidfd}")) {
88+
Ok(fdinfo) => fdinfo,
89+
Err(e) => {
90+
tracing::error!("Failed to read fdinfo from procfs: {e}");
91+
return None;
92+
}
93+
};
5894

59-
// 2. Use the connection to query the D-Bus daemon for more info
95+
// Find the line that starts with "Pid:"
96+
let pid_line = match fdinfo_str.lines().find(|line| line.starts_with("Pid:")) {
97+
Some(line) => line,
98+
None => {
99+
tracing::error!("Failed to read PID from fdinfo");
100+
return None;
101+
}
102+
};
103+
104+
let pid_str = pid_line[4..].trim();
105+
106+
// std::process::id() also returns u32
107+
let pid: u32 = match pid_str.parse() {
108+
Ok(id) => id,
109+
Err(e) => {
110+
tracing::error!("Failed to parse PID from fdinfo entry: {e}");
111+
return None;
112+
}
113+
};
114+
115+
Some(pid)
116+
}
117+
118+
async fn query_peer_pid_via_dbus(
119+
connection: &Connection,
120+
sender_unique_name: &UniqueName<'_>,
121+
) -> Option<u32> {
122+
// Use the connection to query the D-Bus daemon for more info
60123
let proxy = match zbus::Proxy::new(
61124
connection,
62125
"org.freedesktop.DBus",
@@ -72,7 +135,7 @@ async fn query_connection_peer_binary(
72135
}
73136
};
74137

75-
// 3. Get the Process ID (PID) of the peer
138+
// Get the Process ID (PID) of the peer
76139
let pid_result = match proxy
77140
.call_method("GetConnectionUnixProcessID", &(sender_unique_name))
78141
.await
@@ -90,8 +153,33 @@ async fn query_connection_peer_binary(
90153
return None;
91154
}
92155
};
156+
Some(pid)
157+
}
158+
159+
async fn query_connection_peer_binary(
160+
header: Header<'_>,
161+
connection: &Connection,
162+
) -> Option<RequestingApplication> {
163+
// Get the sender's unique bus name
164+
let sender_unique_name = header.sender()?;
165+
166+
tracing::debug!("Received request from sender: {}", sender_unique_name);
167+
168+
// Get the senders PID.
169+
//
170+
// First, try to get the PID via the more secure fdinfo
171+
let mut pid = query_peer_pid_via_fdinfo(connection, sender_unique_name).await;
172+
// If that fails, we fall back to asking dbus directly for the peers PID
173+
if pid.is_none() {
174+
pid = query_peer_pid_via_dbus(connection, sender_unique_name).await;
175+
}
176+
177+
let Some(pid) = pid else {
178+
tracing::error!("Failed to determine peers PID. Skipping application details query.");
179+
return None;
180+
};
93181

94-
// 4. Get binary path via PID from /proc file-system
182+
// Get binary path via PID from /proc file-system
95183
// TODO: To be REALLY sure, we may want to look at /proc/PID/exe instead. It is a symlink to
96184
// the actual binary, giving a full path instead of only the command name.
97185
// This should in theory be "more secure", but also may disconcert novice users with no

0 commit comments

Comments
 (0)