kylemanna / keepassx

🔑 This fork adds YubiKey support
https://github.com/keepassx/keepassx/pull/52
Other
38 stars 5 forks source link

Lock database doesn't work reliably #1

Closed kylemanna closed 10 years ago

kylemanna commented 10 years ago

As mentioned by @fabian-z

I noticed that database locking does not work as expected - it only checks the password (and/or keyfile) but not the yubi key (I can even remove it unnoticed by keepassx). Also, DatabaseOpenWidget does not enable Challenge Response asynchronously for me; the yubikey has to be plugged in when starting keepassx.

https://github.com/keepassx/keepassx/pull/52#issuecomment-46694026

Steps to reproduce:

  1. Create kdbx with password + yk
  2. Add entry / test data and save
  3. Lock database
  4. Remove yk
  5. Unlock database with just password

Possibly other bugs.

Expected behavior:

fabian-z commented 10 years ago

Confirmed fixed consistent with expected behaviour as of b7fd29b.

kylemanna commented 10 years ago

Awesome thanks!

kylemanna commented 10 years ago

By the way, it's worth noting that the database isn't "protected" when locked. Only the UI is made unusable. If a user were to dump the process memory space, the user would gain access the entire database and all keys resident in memory.

Locking the database merely prevents the UI from allowing access to the windows that display the passwords. The challenge process operates consistently with the existing password and key file operations.

fabian-z commented 10 years ago

:+1: This can't be pointed out too often. I was actually expecting it to close & protect the database too until I read the code myself. For consistency of this fork with the current keepassx master it is nonetheless great to have this fixed.