Keepassxc: Improve error when inputting incorrect password

[mfc]

This is a hold-over from KeePassX and now that we have responsive developers it would be great to improve it.

Expected Behavior

When you input the wrong password, it should tell you you inputted the incorrect password.

Current Behavior

When you input the wrong password, you get: Unable to open the database. Wrong key or database file is corrupt. Noticeably absent from that is the possibility you inputted the wrong password. I have found many new KeePassX users confused by this error message and think that their database is corrupt because it is not immediately evident that they may have inputted the wrong password.

Possible Solution

Change the error text, for instance to: Unable to open the database. Invalid credentials provided.

Debug Info

KeePassXC - 2.2.0
Revision: caa49a8ef3ee28ed478192389b21d61107b3b8e0

[droidmonkey]

I agree with this, but not the wording

[mfc]

updated the example of new wording. obviously feel free to propose your own :)

[louib]

@mfc @droidmonkey what about Unable to open the database. Invalid credentials or the database file is corrupt.. That would cover the password, the key file and even the yubikey.

[droidmonkey]

Invalid credentials provided...

[mfc]

Invalid credentials provided

yeah i think not mentioning the database potentially being corrupted would be a good idea. there are a lot of other potential things that may be wrong, but they are all highly unlikely compared to invalid credentials being provided.

[droidmonkey]

The message might add "database is corrupt" at the end after 3 failed tries.

[phoerious]

I would add it add first try, just not as the first thing.

[macau23]

Telling an end user that their database might be corrupt when they enter the wrong password is terrifying for them.

Can we not distinguish between a corrupt database and an incorrect password?

[phoerious]

No, we can't.

[macau23]

Could we? :)

[TheZ3ro]

@macau23 No, If you try to decrypt a database with a wrong password, the decryption will work just fine but the output will be just garbage, the same thing happen if you use a correct password on a corrupt database, garbage output

[macau23]

So there can't be a checksum on the encrypted database?

[mfc]

I think the simplest solution is just improve the language of the error text. I think @droidmonkey's suggestion works best: Invalid credentials provided.

If the database is indeed corrupt, the user is not going to know what to do with that information anyway, and will need to go online to search for information about it. If you want to add database checksum functionality I see that as an enhancement, and if it is seen as worthwhile it can be implemented at some point. But that should probably be a separate ticket.

For a simple solution to this big UX problem I think changing to error text to Invalid credentials provided is the appropriate step forward.

[lindhe]

Since it is hard or impossible to distinguish between a corrupted password file or a password file which you provide incorrect credentials to, I would suggest the following:

1. The first time KeePassXC opens the file, write something sensible like "Could not open file with supplied credentials." if the user fails. Otherwise, open the file.
2. After the file is opened, make a hash of the _encrypted_ file, so that we know a fingerprint of the latest version of the file which we know is valid. This hash needs to be updated each time we write to it.
3. Cache the file along with the filename (you already save a list of recently opened files; let's keep it at the same place or wherever).
4. Close the file.
5. The next time we try to open the file, if the hash saved matches the hash of the current file we know that the file has not been changed or corrupted. If we can not open it, we know for sure (right?) that the user provided incorrect credentials for the file.
6. If the saved hash does not match the current hash of the encrypted file, it must have been corrupted, or changed by a third party so we are back at step 1.

Since we hash the encrypted file, this should not be a security concern. And since you already keep a list of recently used files, I see no privacy issues either.

[TheZ3ro]

Seems just an overcomplicated solution for a very simple UX problem.
I'm not even sure there is even a problem in the first place.

My 2 cent:

Unable to open the database. Invalid credentials provided, please retry.
If this happen again your database file is likely to be corrupt.

[phoerious]

I was thinking the same. Keep it simple.

[mfc]

hi folks, given there is agreement on the language change and it is one string (maybe the most important string), could this be bumped up in priority? or could you point me to the part of the code where this exists and I will create pull request? would be nice to have it done within 2 years of it being reported, thanks.

[droidmonkey]

I got it covered, this issue got lost in the forest. Thanks for pinging

[mfc]

great, thanks @droidmonkey

[phoerious]

We should also show a warning if capslock is enabled.