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

External DB changes silently overwritten in the absence of YubiKey #5836

Closed
l-mb opened this issue Dec 15, 2020 · 2 comments
Closed

External DB changes silently overwritten in the absence of YubiKey #5836

l-mb opened this issue Dec 15, 2020 · 2 comments

Comments

@l-mb
Copy link

l-mb commented Dec 15, 2020

Overview

Hitting a sync issue that results in loss of changes done by other instances.

Steps to Reproduce / Observed Behaviour

I'm using K2A with a Yubikey with a database backed by a Nextcloud instance. The file is also synced with keepassxc on my desktop.

When I make changes on the desktop, this works as expected; I need the YK plugged in to save, and when I'm back on mobile, it detects the DB has been changed and refreshes, asking for the YK again. As expected.

However, when I make changes on mobile, such as a new folder (new key is identical), with the YK connected, K2A will save the DB back to my Nextcloud instance. This, in turn, gets synced back to my desktop's files almost immediately. (So far, so good.)

HOWEVER, on my keepassxc desktop at this time, I get a transient pop-up that it can't read the key (since the YK isn't plugged in, since I'm not leaving that behind), and that it'd really like me to plug in that key. The popup then disappears (which is one part of this bug: I think this notification should persist until remedied).

Now I come back to my desktop, and even though the pop-up is gone, I reconnect my Yubikey. KeePassXC does not automatically refresh the DB from on-disk. Nor do I see any "Refresh" button that I could choose, nothing indicates the data I see is outdated.

I make some change - create a new entry here, or even if I close the database. What happens is that this will write out the database, overwriting the on-disk file. At this point, the changes from the other clients are gone. (And immediately sync'ed back to the cloud.)

Finally, I go back to mobile. K2A detects that the DB has changed, asks me to resynchronize, and hangs at Loading database ... (Transforming master key ...). I need to restart K2A via a force-close and reopen the DB. (This part is not relevant to KeePassXC, but I'm including it here so I can reference the full report in the K2A report.)

I suspect this behaviour could be even more likely if any of the client changes don't immediately get sync'ed via my Nextcloud instance, e.g., if they were disconnected?

Expected Behavior

I'd expect that

  1. The notification that something happened persists
  2. At minimum, once user returns to the desktop and reconnects the YubiKey, KXC should auto-refresh from the DB
  3. Optimally, if there should be pending in-memory changes, maybe KeePassXC should (auto-)merge on-disk with in-mem before writing it back? (I suspect this could happen if files get desynchronized due to lack of connectivity at some point)

Context

Work-arounds:

  • Have two YubiKeys and leave one plugged in while KXC is running (somewhat undesirable)
  • Manually closing the DB before switching devices (inconvenient)
  • Removing YK protection; in those cases the syncs are fast enough that I don't notice them.

[NOTE]:
KeePassXC - Version 2.6.2
Revision: e9b9582

Qt 5.15.2
Debugging mode is disabled.

Operating system: openSUSE Tumbleweed
CPU architecture: x86_64
Kernel: linux 5.10.0-rc7-2.g9688120-default

Enabled extensions:

  • Auto-Type
  • Browser Integration
  • SSH Agent
  • KeeShare (only unsigned sharing)
  • YubiKey
  • Secret Service Integration

Cryptographic libraries:
libgcrypt 1.8.7

K2A 1.0.8c-r1 on Android 11 (unrelated, I suspect)

@l-mb l-mb added the bug label Dec 15, 2020
@droidmonkey
Copy link
Member

droidmonkey commented Dec 15, 2020

We know, its covered by #5290

@l-mb
Copy link
Author

l-mb commented Dec 16, 2020

We know, its covered by #5290

Oh, my bad. I'd have sworn I searched for a duplicate before opening this! Thanks so much.

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

No branches or pull requests

2 participants