SecretService client authorisation for short-lived processes

[nbarrientos]

Hi,

If I understand correctly, since 2.7.0, apps requesting secrets via SecretService have to be authorised to do so, either every time a secret request is issued (Allow Selected) or permanently (Allow All and Future). I understand the value of this feature which strengthens KeepassXC's security.

However, in my case isync uses secret-tool to retrieve the password of my IMAP account, with something along the lines of:

PassCmd "/usr/bin/secret-tool lookup host mail.example.org account joe@example.org"

The issue now is that every run of the mail sync process creates a new secret-tool process so KeypassXC asks interactively for authorisation to deliver the secret. I imagine that the SecretService implementation is designed to save the PID or other fingerprint of a running process and whitelist it and does not have short-lived processes in mind (just a guess, sorry, I haven't looked at the implementation details).

As you can imagine, this is a bit cumbersome. I've set for the time being ConfirmAccessItem=false, however maybe it'd be nice to have more fine grained authorisation rules like: "all requests coming from a process with binary /usr/bin/secret-tool and parent /usr/bin/mbsync are authorised to retrieve secret A". I'm rather happy with ConfirmAccessItem=false as, if I recall correctly, it restores the behaviour of the previous version of KeepassXC however, does my proposal make any sense to you?

Thanks.

[droidmonkey]

Hmmm, I could imagine remembering the process stack instead of, or in addition to, pid. This would let you remember that a specific process was using secret-tool to request credentials. Perhaps could be a special case for secret-tool only.

[nbarrientos]

Yup, in my case it might be tricky because it's systemd spawning mbsync via a service unit and a timer so the only "resident" process to remember/authorise in the tree would be systemd --user which is a bit too generic. Ideally, for me having a way to define a rule to allow any requests from /usr/bin/secret-tool (any PID) invoked by /usr/bin/mbsync (any PID) would be perfect.

Indeed as you're suggesting an alternative would be to handle /usr/bin/secret-tool as a special case and add another config option to always authorise new /usr/bin/secret-tool processes, regardless of the parent or process stack.

[CrfzdPQM6]

I had the same problem using secret-tool to provide login to a caldav server. Another oddity is that every time an access request is triggered, keepass first insists on unlocking all databases that are in tabs within the KeePassXC application, and only then goes to the one (that was in scope at first) that has the Secret Service access available. I only encountered this before when another tab was active, different to the one that had the Secret Service integration.

[droidmonkey]

It is 'dangerous' to trust secret-tool implicitly because any adversary or malicious program would be using it to harvest credentials. So this is a hard problem to solve correctly.

[jonasc]

It is 'dangerous' to trust secret-tool implicitly because any adversary or malicious program would be using it to harvest credentials. So this is a hard problem to solve correctly.

Could a slightly more secure way be to have dedicated scripts which only access a specific entry? Like /usr/bin/local/get-mail-pwd as

#!/usr/bin/bash
exec secret-tool lookup Uuid <my-uuid-here>

This script could, for additional security, check whether it is called by the right script (e.g. by examining /proc/$PPID/exe and /proc/$PPID/cmdline).

#!/usr/bin/bash
[[ $(readlink "/proc/$PPID/exe") == /usr/bin/mbsync ]] || exit 1
exec secret-tool lookup Uuid <my-uuid-here>

Obviously this script should not be writable as that would defeat the security improvement.

[Aetf]

Isn't this #6458?

[droidmonkey]

Somewhat, perhaps a special case for explicit approvals of secret-tool parent process.

[Aetf]

To special handle secret-tool we have to figure out a way to identify secret-tool. Obviously just checking for /usr/bin/secret-tool isn't reliable due to reasons in #6458.

[droidmonkey]

Well you leave it up to the user, just detect the process name (not path):

Warning: It looks like a process is using secret-tool, be sure you trust the parent process calling it before allowing.

[Aetf]

The process name can be even easier to change, e.g. just rename a script to secret-tool. So I think client fingerprinting is still necessary if we ever want to special case secret-tool. And that seems to be doable now. See my other comment there.

We can then decide whether we want to go with the full client fingerprinting and rule matching, or just special case secret-tool, and check the parent process instead, and do not save persistent decisions like what we have now. (This seems to be not enough for @nbarrientos, though. As the parent process is also transient)

[CrfzdPQM6]

Is it only secret-tool that is relevant here? Though, like @nbarrientos , I first came across this issue with secret-tool, I've since seen similar behaviour with pinentry-gnome3, which handles my GPG key passphrase entry from keepassxc.

[nbarrientos]

What about whitelisting the hash of a given binary in the process tree executed by a given user? In my case I'd accept secret retrievals performed by invocations of /usr/bin/mbsync (hash of the binary to be saved when whitelisted for the first time) executed by my username. Then if /usr/bin/mbsync calls /usr/bin/secret-tool or talks via D-Bus directly would be irrelevant. KeepassXC would have to walk back the process tree looking for a binary hash in the whitelist, basically.

In other words, "allow any process of this trusted binary with hash h called by euid u (or any child process it creates) to retrieve secret s". That would cover my case, I think.

Binary upgrades due to a software updates would have to be re-whitelisted but that's acceptable and much less cumbersome than having to approve every request.

(Thanks for looking at this, btw)

[ioogithub]

I am confused with this issue. Is this the same issue as my bug report that was closed here: https://github.com/keepassxreboot/keepassxc/issues/7623?

I have been using the secret service function reliably for over a year. I got this new version of keepass during a dist-upgrade and now I am prompted hundreds of times a day for this Access Request.

I click the "Allow All & Future" button and the blue "Remember" box is checked however the very next email I open Keepass prompts me again for the Access Request. How is this not a bug? Keepass is not remembering the "Remember" checkbox, is simple disregards it.

This worked perfectly in the previous versions. 2.7.0 totally breaks my daily computing experience.

[Aetf]

@ioogithub due to various limitations (mostly discussed in #6458), KPXC only remembers the decision for as long as both the client and the KPXC processes are running.

This works fine for long-running clients, but unfortunately, it turns out many clients simply use a separate short-lived process (likesecret-tool) to request the password, thus this issue.

You can simply disable "Confirm when passwords are retrieved by clients" to revert to 2.6.x behavior for now.

[izbushka]

You can simply disable "Confirm when passwords are retrieved by clients" to revert to 2.6.x behavior for now.

The problem is this option change nothing since 2.7.2. It still asks for access every time regardless of this setting.

[mormegil-cz]

I commented on a wrong bug (closed duplicate), so copying my comment it from there:

I believe the problem with the program ignoring the Confirm when passwords are retrieved by clients setting is in UnlockPrompt::unlockItems where the code tests whether client->itemKnown(uuid) and then if !client->itemAuthorized(uuid). However: while DBusClient::itemAuthorized takes into account the FdoSecrets::settings()->confirmAccessItem() setting, DBusClient::itemKnown does not.

Which means the code in unlockItems ignores the setting and always needs to ask for the confirmation.

I am not sure which of the two places is the correct one to add the check: Either DBusClient::itemKnown should return true always when FdoSecrets::settings()->confirmAccessItem() is unset, or UnlockPrompt::unlockItems should check for the setting as well. I tested the latter variant and this seem to work:

diff --git a/src/fdosecrets/objects/Prompt.cpp b/src/fdosecrets/objects/Prompt.cpp
index bd01de89..e89cd499 100644
--- a/src/fdosecrets/objects/Prompt.cpp
+++ b/src/fdosecrets/objects/Prompt.cpp
@@ -23,6 +23,7 @@
 #include "fdosecrets/objects/Session.h"
 #include "fdosecrets/widgets/AccessControlDialog.h"

+#include "FdoSecretsSettings.h"
 #include "core/Entry.h"
 #include "gui/MessageBox.h"

@@ -298,7 +299,7 @@ namespace FdoSecrets
                 }
                 auto entry = item->backend();
                 auto uuid = entry->uuid();
-                if (client->itemKnown(uuid)) {
+                if (client->itemKnown(uuid) || !FdoSecrets::settings()->confirmAccessItem()) {
                     if (!client->itemAuthorized(uuid)) {
                         m_numRejected += 1;

[siddhpant]

The easiest way would be to have keepassxc have its own secret-tool implementation which uses the existing running process, and symlink that to /usr/bin/secret-tool. It can be as easy as having a script which calls it with appropriate arguments. For example: keepassxc secret-tool $@

[siddhpant]

In other words, "allow any process of this trusted binary with hash h called by euid u (or any child process it creates) to retrieve secret s". That would cover my case, I think.

@nbarrientos IIUC, the issue here is malicious secret-tool, so allowing all child-processes doesn't address it. Though I agree whitelisting parent process is a far cleaner way. If someone is able to replace secret-tool they are anyways inside the system.