Skip to content
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

[FdoSecrets] Only show unlock prompt before search if there is no unlocked & exposed database #7574

Closed
droidmonkey opened this issue Mar 22, 2022 · 16 comments · Fixed by #8028

Comments

@droidmonkey
Copy link
Member

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.

Originally posted by @CrfzdPQM6 in #7571 (comment)

@Aetf
Copy link
Contributor

Aetf commented Mar 22, 2022

Unlock before search was introduced in #6943. Usually the client calls search on the whole service, which will search each collection in the service, thus trying to unlock every one of them.

However, only databases with an exposed group have the corresponding collection. So if you only have one such database, KPXC shouldn't unlock others.

@CrfzdPQM6 could you verify that other databases don't have an exposed group set?

And here's a command to show all collections

dbus-send --session --print-reply --dest=org.freedesktop.secrets /org/freedesktop/secrets org.freedesktop.DBus.Properties.GetAll string:"org.freedesktop.Secret.Service"

@CrfzdPQM6
Copy link

I upgraded from 2.6.6. to 2.7.0 this morning and before then I'd never encountered this behaviour.

I believe, @Aetf , that this is the only database with an exposed group. And, indeed, if I issue the command that you wrote, I get a single dict entry in an array returned.

@Aetf
Copy link
Contributor

Aetf commented Mar 22, 2022

Yes, 2.7.0 is the first version containing the new "unlock before search" .

Could you describe in more details about your setup? You have multiple databases in tabs, but all of them are locked, right? Then when you use secret-tool to search, you see multiple database unlock dialogs?

@niko-lay
Copy link

I've faced same issue.
I have 3 dbs in keepassXC and only one uses as SSI service, one DB is already unlocked before login to skype. Right after skype opening I see couple of unlocking requests to each DBs and window to store credentials access (Skype for Desktop MSA/my-skype-login and Skype for Desktop/my-skype-login), in few seconds skype logged out

@Aetf
Copy link
Contributor

Aetf commented Mar 22, 2022

I now get it.

Locked databases are assumed to be exposed because the exposed settings are saved in the db.
And application usually just searches in all databases, triggering the unlock prompt for all of them.

For now, the behavior can be disabled by unchecking "Application Settings -> Secret Service Integration -> Prompt to unlock
database before searching".

Maybe a more useful setting would be unlocking a specific database before searching?

@droidmonkey
Copy link
Member Author

Or show the database unlock dialog, let the user choose the right database to unlock, then roll with it. If the collection is not found you can either re-issue the unlock dialog or error out.

@Aetf
Copy link
Contributor

Aetf commented Mar 23, 2022

I don't think the database unlock dialog allows selecting database? Without that, in the worst case, the user still has
to unlock all databases before the correct one.

Given that most likely people will have only one database exposed, just letting the user set that database file path in
the settings should be more straightforward.
Something like: "Ensure this database is loaded and unlocked before search"

This should cover most use cases.

@CrfzdPQM6
Copy link

Yes, 2.7.0 is the first version containing the new "unlock before search" .

Could you describe in more details about your setup? You have multiple databases in tabs, but all of them are locked, right? Then when you use secret-tool to search, you see multiple database unlock dialogs?

I have multiple database tabs, all locked except for the one that also contains the SS integration, and that one is unlocked. Before 2.7.0 I learnt that I had to have that tab active/selected/focused in order for the SS query to succeed. But now when I issue a secret-tool query, it insists that I unlock the other tabs before it issues a dialog about access to SS in the original one.

@CrfzdPQM6
Copy link

I now get it.

Locked databases are assumed to be exposed because the exposed settings are saved in the db. And application usually just searches in all databases, triggering the unlock prompt for all of them.

For now, the behavior can be disabled by unchecking "Application Settings -> Secret Service Integration -> Prompt to unlock database before searching".

Maybe a more useful setting would be unlocking a specific database before searching?

Yes, that works for now, as long as I have the db unlocked. Yes, prompting to unlock a specific database would be a really useful feature because I haven't yet figured out how to queue automatically-starting services/applications behind my manual unlocking of keepassxc, and I think this would give me a (short) window to do it before those applications fallback on manual password entry.

@droidmonkey
Copy link
Member Author

@Aetf the database unlock dialog let's you choose now in 2.7.0

@Aetf
Copy link
Contributor

Aetf commented Mar 23, 2022

the database unlock dialog let's you choose now in 2.7.0

Ah good. I didn't notice that.

So with this, the logic could be:

If there's already an unlocked collection, just use that (even though this is not fully confirming to the spec which
requires searching all collections),
otherwise, show the unlock dialog until there's an unlocked and exposed collection.

@Aetf Aetf changed the title Secret service forces unlock of all databases [FdoSecrets] Only show unlock prompt before search if there is no unlocked & exposed database Mar 23, 2022
@Aetf
Copy link
Contributor

Aetf commented Mar 23, 2022

I can't work on this right away so anyone feels free to take a shot. It shouldn't be too complex.
I'll pick this up later in May.

@CrfzdPQM6
Copy link

Thank you, @Aetf . I've no experience with the keepassxc codebase so I probably wouldn't be the right person to attempt this. For now the workaround is OK. Grateful for all your efforts.

@CaseOf
Copy link

CaseOf commented May 7, 2022

Hello,
In my usecase, I’m using a main database, and a database for Secret Service.
I’m now using a FIDO key to login on my computer, and I was using gnome-keyring before for Secret Service with pam module for reusing my user password to unlock.
gnome-keyring does not handle at all security keys and workaround proposed are not really sustainable.
I then did replace gnome-keyring with KeePassXC since it handles security keys and Secret Service.
For a smooth workflow, I have decided to put that on another database secured only with my security and not a password, since I’m logging in with a security key only too.
Actuallly, unlock prompt is showing the last database used and asks more actions to choose the right one if it is not the wanted database. Issue #7851 is also related to that for cases where a database is unlocked and not the other.
What I’m looking for here, is to do less actions for a smoother workflow for coexisting databases.
Unless you have a different idea about my usecase?

@michaelk83
Copy link

Maybe remember the last database that was used for Secret Service, and separately the last database that was used for Auto-Type. Then the unlock prompt can pre-focus the appropriate DB depending on where the request is coming from.

Aetf added a commit to Aetf/keepassxc that referenced this issue May 9, 2022
The current logic is:
* If there're already unlocked collections, just use those,
* otherwise, show the unlock dialog until there's an unlocked and exposed collection.

Fix keepassxreboot#7574
Aetf added a commit to Aetf/keepassxc that referenced this issue May 9, 2022
The current logic is:
* If there're already unlocked collections, just use those,
* otherwise, show the unlock dialog until there's an unlocked and exposed collection.

Fix keepassxreboot#7574
Aetf added a commit to Aetf/keepassxc that referenced this issue May 9, 2022
The current logic is:
* If there're already unlocked collections, just use those,
* otherwise, show the unlock dialog until there's an unlocked and exposed collection.

Fix keepassxreboot#7574
Aetf added a commit to Aetf/keepassxc that referenced this issue May 9, 2022
The current logic is:
* If there're already unlocked collections, just use those,
* otherwise, show the unlock dialog until there's an unlocked and exposed collection.

Fix keepassxreboot#7574
@Aetf
Copy link
Contributor

Aetf commented May 9, 2022

Maybe remember the last database that was used for Secret Service, and separately the last database that was used for Auto-Type. Then the unlock prompt can pre-focus the appropriate DB depending on where the request is coming from.

This sounds reasonable. The info should be saved similarly to the last window position, last file dialog path, etc., as local state data, separated from normal config.

From a quick glance of the code, it seems KPXC doesn't save any of these yet. Adding them would be not trivial (although shouldn't be hard, either), so this bit isn't included in the PR.

droidmonkey pushed a commit to Aetf/keepassxc that referenced this issue Jun 4, 2022
This commit implements the following logic:
* If there're already unlocked collections, just use those,
* otherwise, show the unlock dialog until there's an unlocked and exposed collection.

* Fixes keepassxreboot#7574
droidmonkey pushed a commit that referenced this issue Jun 4, 2022
This commit implements the following logic:
* If there're already unlocked collections, just use those,
* otherwise, show the unlock dialog until there's an unlocked and exposed collection.

* Fixes #7574
droidmonkey pushed a commit that referenced this issue Jun 27, 2022
This commit implements the following logic:
* If there're already unlocked collections, just use those,
* otherwise, show the unlock dialog until there's an unlocked and exposed collection.

* Fixes #7574
t-h-e pushed a commit to t-h-e/keepassxc that referenced this issue Sep 8, 2022
This commit implements the following logic:
* If there're already unlocked collections, just use those,
* otherwise, show the unlock dialog until there's an unlocked and exposed collection.

* Fixes keepassxreboot#7574
droidmonkey pushed a commit that referenced this issue Sep 22, 2022
This commit implements the following logic:
* If there're already unlocked collections, just use those,
* otherwise, show the unlock dialog until there's an unlocked and exposed collection.

* Fixes #7574
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants