Add Polkit Quick Unlock mechanism

[HexF]

This pull request closes 2 issues relating to the Quick Unlock feature (commonly referred to as fingerprint support) - #5991 and #3337.
The changes made implement quick unlocking via Polkit on Linux, which provides support for fingerprint authentication, giving Linux feature parity with macOS with its TouchID and Windows with Windows Hello.
Additionally, polkit will fall back to a password if no fingerprint sensor is found, as shown in the screenshots below.

The Polkit mechanism works similar to the existing TouchID implementation, by where the database key is encrypted using a random key which is stored in secure storage, in this case the kernel-space keyring.
As far as I am aware, Linux does not provide a kernel-level mechanism for releasing keys when appropriate credentials such as a password or fingerprint have been provided, so the check is done entirely in the KeepassXC process.

This pull request also makes changes to both the TouchID and Windows Hello implementations of Quick Unlocking, bringing them on to a common interface which should allow for easy development of further Quick Unlock mechanisms - one that comes to mind is using TOTP/one time passwords to unlock the database (#798). The polkit implementation also uses this interface.

Screenshots

(please excuse the polkit action having the wrong wording in the above screenshot - it is surprisingly hard to add polkit actions on nixos without installing anything!)

Testing strategy

Local testing has been carried out through the following steps:

1. Open and unlock a database
2. Lock the database
3. Observe the Quick Unlock "Unlock Database" button being present
4. Click the button
5. Observe a polkit authentication request
6. Satisfy this request through entering a password
7. Observe the Keepass database unlocked

Type of change

• ✅ New feature (change that adds functionality)
• ✅ Refactor (significant modification to existing code)

[HexF]

Not sure how to go about adding the libkeyutils-dev dependency to all the CI which seems to be whats failing on Linux.

[droidmonkey]

That's on our end we'll take care of that

This pull request also makes changes to both the TouchID and Windows Hello implementations of Quick Unlocking, bringing them on to a common interface which should allow for easy development of further Quick Unlock mechanisms

You are my hero thank you

[HexF]

Haven't touched this in a bit, but now that the vulnerability in 1Password that I discovered has been fixed I feel it's appropriate to discuss it here as it has direct implications on the use of PolKit.

The vulnerability boils down to being able to redirect D-Bus calls from the password manager to polkit into your own service. This happens by adjusting the DBUS_SYSTEM_BUS_ADDRESS environment variable when the application is started.

I didn't investigate the exact way 1Password fixed this, but there are two ways which I foresee fixing this for KeePassXC

1. Run a separate process as root which manages the releasing of partial key data from PolKit when authorization is completed
2. Clear environment variables, ignoring the changes to the system bus address (pkexec does this)

I'm leaning more towards a separate process, but there is a large inconvenience of having to install a system service for this.

Any opinions @droidmonkey

[droidmonkey]

We won't install a system service on our end. This boils down to a Linux environment vulnerability not an application vulnerability. Choice 1 is something either polkit or the desktop env should implement for the benefit of everyone. Choice 2 is a workaround because of a lack of choice 1.

Let's just capture the value of the environment variable on startup and/or ignore the actual value.

[Gimli05]

Hi!

Can we get a status update on this one?

Is polkit excluded due to the dbus redirect issue? Saving the env var value on startup and ignoring its actual value during runtime seems to be a valid option to me. However I'm not sure whether storing it for just this purpose would be a nice solution, rather than a hacky one. On the other hand I cannot see a valid purpose why the dbus address would need to be changed during runtime.

[HexF]

Hi, Sorry been really busy.

I am thinking as a way to combat the environment variable making it easy to set, we compile in a list of allowed paths for the dbus system bus.
If it does not match any of the approved paths, QuickUnlock will be disabled and/or display a nag bar indicating the presence of an untrustworthy path.

[michaelk83]

A hardcoded path whitelist sounds good to me. If the path doesn't match, I'm in favor of both disabling QuickUnlock and showing a warning similar to what the Secret Service component does when there's another Secret Service provider (such as Gnome keyring) already running.

keepassxc/src/fdosecrets/dbus/DBusMgr.cpp

Line 412 in bb37cf3

emit error(tr("Failed to register DBus service at %1.<br/>").arg(DBUS_SERVICE_SECRET) + existing);

[HexF]

New changes implement a single fixed path to access DBus from, which should contain polkit.
The only vulnerability I can see here is if polkit is not installed on the system, and then a malicious actor simply claims the service on the dbus.
I can't see any way around this, and the chances of it occurring are very slim, as most modern Linux distros run Polkit.

Changes also implement a publicUuid, which is stored in the pubilc headers headers as KPXC_PUBLIC_UUID.
This is used instead of the file path to reference the database for quick unlocking.
uuid was insufficient as it was randomized every time the database was locked/unlocked.

I'll try get to these merge conflicts tomorrow, but in the mean time any feedback is much appreciated.
I assume these CI failures are due to a fairly outdated repo.

[droidmonkey]

It's basically ready we have a queue of merges to make

[spikespaz]

Package maintainers eagerly await.

[droidmonkey]

And done

[anantakrishna]

This issue is to be expected when keepassxc is not installed in the system and hence the polkit action isn't registered with polkit.

Appreciation for your efforts. I can't manage it to work. I've reinstalled it through apt instead of flatpak. Still there is no Quick Unlock functionality. Is there anything else one need to configure?

KeePassXC - Version 2.7.6
Revision: dd21def

Qt 5.15.3
Debugging mode is disabled.

Operating system: Ubuntu 22.04.3 LTS
CPU architecture: x86_64
Kernel: linux 6.2.0-37-generic

Enabled extensions:
- Auto-Type
- Browser Integration
- SSH Agent
- KeeShare
- YubiKey
- Secret Service Integration

Cryptographic libraries:
- Botan 2.19.1

[droidmonkey]

Polkit quick unlock won't be available until 2.8.0 is released

[HexF]

Do we have a rough date on when 2.8.0 will be released?

[droidmonkey]

I'm hoping for shortly after the new year, we have some important changes in the pipeline right now

[RobieWoods]

Hello @HexF and @droidmonkey,

I have a question regarding this PR, @HexF, you wrote the following:

The Polkit mechanism works similar to the existing TouchID implementation, by where the database key is encrypted using a
random key which is stored in secure storage, in this case the kernel-space keyring.

I would like to consider an Evil maid attack scenario. Let's suppose I have KeepassXC on my computer, my computer is locked but I'm not around. Is the Polkit Quick Unlock is as secure as typing the full password on Linux? If an attacker manage to dump the entire memory while I have left my device unattended, will he be capable to decipher the KeepassXC database if Quick Unlock is enabled even if my database is locked?

I have installed KeepassXC Git with Polkit integration, and it works great 🎉. I had the feeling at first that using this PR could increase both the security (since I can increase the password size and lock my database very quickly) and facilitate the ease of use. However, now I'm unsure about the security side. While reading this PR I had the feeling that the Quick Unlock using Polkit could allow an attacker to access the database even if it's locked by "simply" dumping the RAM of the device.

In any case, I'm glad this PR has been merged. I love using my fingerprint to decipher my Keepass database 😅. So thank you!

Kind regards,
Rob

[droidmonkey]

If you lose physical access to the machine such that a RAM dump can be accomplished, then not much can protect you. Also known as "its my computer now".

[goetzc]

If you lose physical access to the machine such that a RAM dump can be accomplished, then not much can protect you. Also known as "its my computer now".

What about encrypting that key (using the fingerprint data as the key for this) in user-space before adding it to the keyring, would this be possible?

[BlackHoleFox]

Fingerprint sensors, by design, don't release their template data (the scan and representation of your fingerprint) outside of the scanner for security. OSes only get back a yes/no with some replay protection (depending what protocol its using).

[droidmonkey]

Also fingerprints are FAR from random and would make terrible keys for encryption. The way polkit integration is implemented now is as secure as possible given the lack of kernel owned keyspace on Linux (i.e., the Linux kernel offers no way to handle sensitive data for user land). Other operating systems like macOS handle encryption keys on the kernel side (and via a hardware key store) to protect data from user land.

[RobieWoods]

Okay, thanks for the reply!

In my setup I have a bash command that automatically lock all my databases when I press Meta+L before starting my locker. Could a command be designed to remove the temporary key in Polkit to avoid such attack?

I'm refering to this key:

keepassxc/src/quickunlock/Polkit.cpp

Line 190 in a8cfefe

long keychainDataSize = keyctl_read_alloc(keySerial, &keychainBuffer);

[HexF]

The key which is stored with the kernel is only one part in decoding the database master key, which is derived from more data stored in user-space.
For an evil-maid attack to be successful in this case, both kernel memory and user-space KeepassXC memory would need to be dumped, which may be possible using techniques such as ThunderSpy, but is unlikely to occur.
If you're that concerned about a memory dumping evil-maid attack, the best course of action would be to turn off the computer when left unattended.
If your just concerned about KeepassXC, you could try closing the application which would both remove the entry from the Linux keyring, and remove the other half of the key stored in user-space.

[RobieWoods]

Actually, I was thinking that a dbus function could be created. Like the ones described in the wiki. I would love to be able to do a:

$ qdbus org.keepassxc.KeePassXC.MainWindow /keepassxc org.keepassxc.KeePassXC.MainWindow.resetQuickUnlock

But even if KeepassXC doesn't have a Dbus function for that, I'm worried about the "kill the app" solution. The encryptedMasterKeys stored in the user space are only scrubbed once here in Polkit::getKey:

keepassxc/src/quickunlock/Polkit.cpp

Line 225 in a8cfefe

Botan::secure_scrub_memory(encryptedMasterKey.data(), encryptedMasterKey.size());

I really think the encryptedMasterKeys should be scrubed in the two Polkit::reset(...) methods as well. hashmap.remove() and hashmap.clear are not enough in my opinion.

Click to see the links of the `Polkit::reset(...)` methods I'm refering to

https://github.com/keepassxreboot/keepassxc/blob/a8cfefe6c8a4c10f4a215a003a98148b56840474/src/quickunlock/Polkit.cpp#L99 https://github.com/keepassxreboot/keepassxc/blob/a8cfefe6c8a4c10f4a215a003a98148b56840474/src/quickunlock/Polkit.cpp#L84

And finally, I think the destructor Polkit::~Polkit() should call the global Polkit::reset() to have everything scrubbed properly when the app is closed for good.

[droidmonkey]

We scrub all memory upon free.

[RobieWoods]

We scrub all memory upon free.

That's great, sorry, I didn't know that. I just read the operator overload of delete, it's very neat :)
I have two remaining questions regarding that subject:

1. This one is probably a noob one, since all data is scrubbed when being freed, is scrubbing the data on QByteArray really necessary here and here? Is it redundant? Or I forget something?
2. Would you guys be open to idea if I open a PR to add org.keepassxc.KeePassXC.MainWindow.resetQuickUnlock dbus option?

[droidmonkey]

The reset quick unlock dbus command is super specific to your use case / desires. I personally don't see purpose in something like that and truly defeats the purpose of quick unlock in the first place. It seems like you want to, but not really, use quick unlock.

Those memory scrubs are overkill, but sometimes qt can be a little clingy to containers like QString and QByteArray so it doesn't hurt to explicitly clear the buffer while you have hold of it.

[RobieWoods]

Okay thanks! I think you are right. My request is too specific and can be handled by a simple script (at least for my case since KeepassXC always start in background). I just needed to call a small script before calling my Locker.

If that script interest some people, here it is:

#!/usr/bin/env bash

function back() {
  nohup $@ > /dev/null 2>&1 &
}

KEEPASSXC_DBUS_STR="org.keepassxc.KeePassXC.MainWindow"

if timeout 5 qdbus ${KEEPASSXC_DBUS_STR} /keepassxc lockAllDatabases ; then
  # exit keepassxc and restart it to reset the QuickUnlock keys
  # see: https://github.com/keepassxreboot/keepassxc/pull/8983#issuecomment-1876248571
  qdbus ${KEEPASSXC_DBUS_STR} /keepassxc appExit && back keepassxc
else
  echo "Warning: Cannot Lock KeepassXC database"
fi

[rgarcia89]

Someone from the maintainer team knows if an early release including this feature is possible? I am seeing a lot of still open backlog items for 2.8.0.

This feature seems to be very awaited by the Linux users.

PS: I am aware of the possibility to build it by myself 😉

[Arbel-arad]

i tried using the polkit authentication on arch, but it's giving me some errors:
nixutils: failed to parse "/proc/3/stat"

KeePassXC - Version 2.8.0-snapshot
Build Type: Snapshot
Revision: a472ef8

Qt 5.15.12
Debugging mode is disabled.

Operating system: Arch Linux
CPU architecture: x86_64
Kernel: linux 6.7.6-zen1-1-zen

Enabled extensions:
- Auto-Type
- Browser Integration
- Passkeys
- SSH Agent
- KeeShare
- YubiKey
- Secret Service Integration

Cryptographic libraries:
- Botan 3.3.0

[droidmonkey]

Ahh yes, this is important: nixutils: failed to parse "/proc/3/stat"

Can you share the contents of that file? It's the process information for keepassxc.

[rgarcia89]

I am seeing the same on manjaro linux.

nixutils: failed to parse  "/proc/351389/stat"

content of the file

351389 (keepassxc) S 351236 344368 344368 0 -1 4194304 20684 691 339 0 283 37 0 0 20 0 7 0 20803161 1217449984 55042 18446744073709551615 1 1 0 0 0 0 0 16781312 16391 0 0 0 17 3 0 0 0 0 0 0 0 0 0 0 0 0 0

[droidmonkey]

I found the error! #10350

[rgarcia89]

Awesome I look forward to the snapshot build

[rgarcia89]

@droidmonkey still not working with the latest snapshot build.

Getting the same error

 nixutils: failed to parse  "/proc/494940/stat"

content of the file

494940 (keepassxc) S 494754 1945 1945 0 -1 4194304 17338 685 336 0 77 8 0 0 20 0 8 0 55832569 1272360960 38561 18446744073709551615 1 1 0 0 0 0 0 16781312 16391 0 0 0 17 4 0 0 0 0 0 0 0 0 0 0 0 0 0

[droidmonkey]

It's not merged yet.....

[rgarcia89]

true story - totally missed that