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

File not found warning immediately replaced when key file not found #741

Closed
kortschak opened this issue Jul 4, 2017 · 2 comments
Closed
Milestone

Comments

@kortschak
Copy link

When a key file is not found at the path specified, a less than helpful dialogue is shown:

Unable to open the database.
Wrong key or database file is corrupt.

Expected Behavior

A dialogue showing that the key file was not found.

Can't open key file:
No such file or directory

Current Behavior

Currently the dialogue that should be shown is flashed for about a quarter of a second and then replaced with:

Unable to open the database.
Wrong key or database file is corrupt.

It is very easy to miss the actual cause of the failure because of this.

Possible Solution

The first error (file not found) should trump the second. This suggests to me that an ENOENT error condition is being ignored and keepassxc is forging ahead and failing again.

Steps to Reproduce (for bugs)

  1. Create a database with a password and key file.
  2. Attempt to open the database with a bad path to the key file.

Context

Was looking into replacing keepassx for the reasons that this fork exists.

Debug Info

KeePassXC - 2.2.0 / 7580c38
Revision: caa49a8 / 7580c38

Libraries:

  • Qt 5.5.1
  • libgcrypt 1.6.5

Operating system: Ubuntu 16.04.2 LTS
CPU architecture: x86_64
Kernel: linux 4.4.0-81-generic

Enabled extensions:

  • Auto-Type
@kortschak
Copy link
Author

kortschak commented Jul 4, 2017

Looking at gui/DatabaseOpenWidget.cpp it looks like DatabaseOpenWidget::openDatabase accepts the invalid CompositeKey returned by DatabaseWidget::databaseKey on failure. This probably shouldn't happen.

This fixes the problem:

diff --git a/src/gui/DatabaseOpenWidget.cpp b/src/gui/DatabaseOpenWidget.cpp
index 18b4b2b..60c450c 100644
--- a/src/gui/DatabaseOpenWidget.cpp
+++ b/src/gui/DatabaseOpenWidget.cpp
@@ -140,6 +140,8 @@ void DatabaseOpenWidget::openDatabase()
 {
     KeePass2Reader reader;
     CompositeKey masterKey = databaseKey();
+    if (masterKey.isEmpty()) {
+        return;
+    }
 
     QFile file(m_filename);
     if (!file.open(QIODevice::ReadOnly)) {

If this is an acceptable change I am happy to send a PR.

@louib
Copy link
Member

louib commented Oct 21, 2017

@kortschak this should be fixed in 2.2.2

@louib louib closed this as completed Oct 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants