-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[PM-7486] Detect Libsecret Service #8776
Changes from 6 commits
e09c528
6e7fbc4
d7c3016
c241d0f
ff3e839
026b211
fd3027c
a0bffc2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,6 @@ | ||
use anyhow::{anyhow, Result}; | ||
use libsecret::{password_clear_sync, password_lookup_sync, password_store_sync, Schema}; | ||
use std::collections::HashMap; | ||
use std::{collections::HashMap, time::Duration}; | ||
|
||
pub fn get_password(service: &str, account: &str) -> Result<String> { | ||
let res = password_lookup_sync( | ||
|
@@ -15,6 +15,33 @@ pub fn get_password(service: &str, account: &str) -> Result<String> { | |
} | ||
} | ||
|
||
#[zbus::proxy( | ||
interface = "org.freedesktop.DBus", | ||
default_service = "org.freedesktop.DBus", | ||
default_path = "/org/freedesktop/DBus" | ||
)] | ||
trait ListNames { | ||
fn list_names(&self) -> Result<Vec<String>>; | ||
} | ||
|
||
// Query dbus names looking for the org.freedesktop.secrets service, which indicates an active libsecret provider | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I do not see any risk by simply querying dbus names and searching for org.freedesktop.secrets which is similar to performing the following:
Result However, what we do after the query matters, so I reviewed the I would note that the And since we're using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd note too that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with what you're saying except for
We actually have no expectation that the DBus service has anything to do with what is providing the secrets service. What we're trusting is that the machine we're running on does not have a bad actor running a service as a libsecret provider. At that point, though, we're just taking issue with the way an OS's secure storage works -- which is fine and we've done before -- but the alternative in this case is to store the access tokens and refresh tokens in plaintext to a well-known location on disk. Without handling some form of PAM listening service ourselves I don't currently know a better way to do it. Even then, validating the receiving end of IPC is roughly impossible without a trusted intermediary hosting/validating both services. |
||
pub fn has_password_management() -> bool { | ||
let conn = match zbus::blocking::Connection::session() { | ||
Ok(conn) => conn, | ||
Err(_) => return false, | ||
}; | ||
let proxy = match ListNamesProxyBlocking::new(&conn) { | ||
Ok(proxy) => proxy, | ||
Err(_) => return false, | ||
}; | ||
let names = match proxy.list_names() { | ||
Ok(names) => names, | ||
Err(_) => return false, | ||
}; | ||
|
||
names.contains(&"org.freedesktop.secrets".to_string()) | ||
} | ||
|
||
pub fn get_password_keytar(service: &str, account: &str) -> Result<String> { | ||
get_password(service, account) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could probably shorten this and just call it
osSupport
orenabled
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
went with
osSupport
. Everything is namespaced topasswords
, so I quite liked that readability