Merge branch 'develop' into feature/autoopen

CMakeLists.txt

@@ -52,6 +52,9 @@ set(KEEPASSXC_VERSION_MINOR "2")
 set(KEEPASSXC_VERSION_PATCH "1")
 set(KEEPASSXC_VERSION "${KEEPASSXC_VERSION_MAJOR}.${KEEPASSXC_VERSION_MINOR}.${KEEPASSXC_VERSION_PATCH}")
 
+# Special flag for snap builds
+set(KEEPASSXC_SNAP_BUILD OFF CACHE BOOL "Set whether this is a build for snap or not")
+
 if("${CMAKE_C_COMPILER}" MATCHES "clang$" OR "${CMAKE_C_COMPILER_ID}" STREQUAL "Clang")
   set(CMAKE_COMPILER_IS_CLANG 1)
 endif()

snapcraft.yaml

@@ -12,7 +12,7 @@ apps:
   keepassxc:
     command: desktop-launch keepassxc
     plugs: [unity7, x11, opengl, gsettings, home, network, network-bind, removable-media, raw-usb]
-    desktop: share/applications/keepassxc.desktop
+    desktop: usr/share/applications/keepassxc.desktop
   cli:
     command: keepassxc-cli
     plugs: [gsettings, home, removable-media, raw-usb]
@@ -23,6 +23,8 @@ parts:
     plugin: cmake
     configflags:
       - -DCMAKE_BUILD_TYPE=Release
+      - -DCMAKE_INSTALL_PREFIX=/usr
+      - -DKEEPASSXC_SNAP_BUILD=ON
       - -DWITH_TESTS=OFF
       - -DWITH_XC_AUTOTYPE=ON
       - -DWITH_XC_HTTP=ON
@@ -39,6 +41,8 @@ parts:
       - libxtst-dev
       - libyubikey-dev
       - libykpers-1-dev
+    install: |
+      sed -i 's|Icon=keepassxc|Icon=${SNAP}/usr/share/icons/hicolor/256x256/apps/keepassxc.png|g' $SNAPCRAFT_PART_INSTALL/usr/share/applications/keepassxc.desktop
     after: [desktop-qt5]
 
   # Redefine desktop-qt5 stage packages to work with Ubuntu 17.04
@@ -55,10 +59,12 @@ parts:
       - locales-all
 
   # Overcome limitation in snapd to support URL loading (CTRL+U)
+  # client needs to install "snapd-xdg-open" on their system
   snapd-xdg-open:
     source: https://github.com/ubuntu-core/snapd-xdg-open.git
-    plugin: dump
-    organize:
-      data/xdg-open: bin/xdg-open
+    source-depth: 1
+    plugin: nil
+    install: |
+      install -D -t $SNAPCRAFT_PART_INSTALL/usr/bin/ data/xdg-open
     stage-packages:
       - dbus

src/config-keepassx.h.cmake

@@ -16,6 +16,8 @@
 #cmakedefine WITH_XC_AUTOTYPE
 #cmakedefine WITH_XC_YUBIKEY
 
+#cmakedefine KEEPASSXC_SNAP_BUILD
+
 #cmakedefine HAVE_PR_SET_DUMPABLE 1
 #cmakedefine HAVE_RLIMIT_CORE 1
 #cmakedefine HAVE_PT_DENY_ATTACH 1

src/core/Entry.cpp

@@ -801,28 +801,31 @@ QString Entry::resolvePlaceholder(const QString& str) const
 
 QString Entry::resolveUrl(const QString& url) const
 {
-#ifdef WITH_XC_HTTP
     QString newUrl = url;
-    if (!url.contains("://")) {
+    if (!url.isEmpty() && !url.contains("://")) {
         // URL doesn't have a protocol, add https by default
         newUrl.prepend("https://");
     }
-    QUrl tempUrl = QUrl(newUrl);
 
-    if (tempUrl.isValid()) {
-        if (tempUrl.scheme() == "cmd") {
-            // URL is a cmd, hopefully the second argument is an URL
-            QStringList cmd = newUrl.split(" ");
-            if (cmd.size() > 1) {
-                return resolveUrl(cmd[1].remove("'").remove("\""));
-            }
-        } else if (tempUrl.scheme() == "http" || tempUrl.scheme() == "https") {
-            // URL is nice
-            return tempUrl.url();
+    if (newUrl.startsWith("cmd://")) {
+        QStringList cmdList = newUrl.split(" ");
+        for (int i=1; i < cmdList.size(); ++i) {
+            // Don't pass arguments to the resolveUrl function (they look like URL's)
+            if (!cmdList[i].startsWith("-") && !cmdList[i].startsWith("/")) {
+                return resolveUrl(cmdList[i].remove(QRegExp("'|\"")));
             }
         }
-#else
-    Q_UNUSED(url);
-#endif
+
+        // No URL in this command
+        return QString("");
+    }
+
+    // Validate the URL
+    QUrl tempUrl = QUrl(newUrl);
+    if (tempUrl.isValid() && (tempUrl.scheme() == "http" || tempUrl.scheme() == "https")) {
+        return tempUrl.url();
+    }
+
+    // No valid http URL's found
     return QString("");
 }

src/core/FilePath.cpp

@@ -91,17 +91,29 @@ QString FilePath::pluginPath(const QString& name)
 
 QIcon FilePath::applicationIcon()
 {
+#ifdef KEEPASSXC_SNAP_BUILD
+    return icon("apps", "keepassxc", false);
+#else
     return icon("apps", "keepassxc");
+#endif
 }
 
 QIcon FilePath::trayIconLocked()
 {
+#ifdef KEEPASSXC_SNAP_BUILD
+    return icon("apps", "keepassxc-locked", false);
+#else
     return icon("apps", "keepassxc-locked");
+#endif
 }
 
 QIcon FilePath::trayIconUnlocked()
 {
+#ifdef KEEPASSXC_SNAP_BUILD
+    return icon("apps", "keepassxc-unlocked", false);
+#else
     return icon("apps", "keepassxc-unlocked");
+#endif
 }
 
 QIcon FilePath::icon(const QString& category, const QString& name, bool fromTheme)

src/core/Metadata.cpp

@@ -443,7 +443,7 @@ void Metadata::copyCustomIcons(const QSet<Uuid>& iconList, const Metadata* other
 
 QByteArray Metadata::hashImage(const QImage& image)
 {
-    auto data = QByteArray((char*)image.bits(), image.byteCount());
+    auto data = QByteArray(reinterpret_cast<const char*>(image.bits()), image.byteCount());
     return QCryptographicHash::hash(data, QCryptographicHash::Md5);
 }
 

src/gui/DatabaseOpenWidget.cpp

@@ -70,9 +70,6 @@ DatabaseOpenWidget::DatabaseOpenWidget(QWidget* parent)
 
     connect(m_ui->buttonRedetectYubikey, SIGNAL(clicked()), SLOT(pollYubikey()));
     connect(m_ui->comboChallengeResponse, SIGNAL(activated(int)), SLOT(activateChallengeResponse()));
-
-    connect(YubiKey::instance(), SIGNAL(detected(int,bool)), SLOT(yubikeyDetected(int,bool)), Qt::QueuedConnection);
-    connect(YubiKey::instance(), SIGNAL(notFound()), SLOT(noYubikeyFound()), Qt::QueuedConnection);
 #else
     m_ui->checkChallengeResponse->setVisible(false);
     m_ui->buttonRedetectYubikey->setVisible(false);
@@ -98,10 +95,24 @@ void DatabaseOpenWidget::showEvent(QShowEvent* event)
     m_ui->editPassword->setFocus();
 
 #ifdef WITH_XC_YUBIKEY
+    connect(YubiKey::instance(), SIGNAL(detected(int,bool)), SLOT(yubikeyDetected(int,bool)), Qt::QueuedConnection);
+    connect(YubiKey::instance(), SIGNAL(detectComplete()), SLOT(yubikeyDetectComplete()), Qt::QueuedConnection);
+    connect(YubiKey::instance(), SIGNAL(notFound()), SLOT(noYubikeyFound()), Qt::QueuedConnection);
+
     pollYubikey();
 #endif
 }
 
+void DatabaseOpenWidget::hideEvent(QHideEvent* event)
+{
+    DialogyWidget::hideEvent(event);
+
+#ifdef WITH_XC_YUBIKEY
+    // Don't listen to any Yubikey events if we are hidden
+    disconnect(YubiKey::instance(), 0, this, 0);
+#endif
+}
+
 void DatabaseOpenWidget::load(const QString& filename)
 {
     m_filename = filename;
@@ -283,10 +294,6 @@ void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking)
     YkChallengeResponseKey yk(slot, blocking);
     // add detected YubiKey to combo box and encode blocking mode in LSB, slot number in second LSB
     m_ui->comboChallengeResponse->addItem(yk.getName(), QVariant((slot << 1) | blocking));
-    m_ui->comboChallengeResponse->setEnabled(true);
-    m_ui->checkChallengeResponse->setEnabled(true);
-    m_ui->buttonRedetectYubikey->setEnabled(true);
-    m_ui->yubikeyProgress->setVisible(false);
 
     if (config()->get("RememberLastKeyFiles").toBool()) {
         QHash<QString, QVariant> lastChallengeResponse = config()->get("LastChallengeResponse").toHash();
@@ -296,6 +303,14 @@ void DatabaseOpenWidget::yubikeyDetected(int slot, bool blocking)
     }
 }
 
+void DatabaseOpenWidget::yubikeyDetectComplete()
+{
+    m_ui->comboChallengeResponse->setEnabled(true);
+    m_ui->checkChallengeResponse->setEnabled(true);
+    m_ui->buttonRedetectYubikey->setEnabled(true);
+    m_ui->yubikeyProgress->setVisible(false);
+}
+
 void DatabaseOpenWidget::noYubikeyFound()
 {
     m_ui->buttonRedetectYubikey->setEnabled(true);

src/gui/DatabaseOpenWidget.h

@@ -51,6 +51,7 @@ signals:
 
 protected:
     void showEvent(QShowEvent* event) override;
+    void hideEvent(QHideEvent* event) override;
     CompositeKey databaseKey();
 
 protected slots:
@@ -63,6 +64,7 @@ private slots:
     void activateChallengeResponse();
     void browseKeyFile();
     void yubikeyDetected(int slot, bool blocking);
+    void yubikeyDetectComplete();
     void noYubikeyFound();
 
 protected:

src/gui/EditWidgetIcons.cpp

@@ -302,6 +302,7 @@ void EditWidgetIcons::addCustomIcon(const QImage &icon)
         }
 
         // Select the new or existing icon
+        updateRadioButtonCustomIcons();
         QModelIndex index = m_customIconModel->indexFromUuid(uuid);
         m_ui->customIconsView->setCurrentIndex(index);
     }

src/gui/MainWindow.cpp

@@ -197,6 +197,7 @@ MainWindow::MainWindow()
     m_ui->actionEntryAutoType->setIcon(filePath()->icon("actions", "auto-type", false));
     m_ui->actionEntryCopyUsername->setIcon(filePath()->icon("actions", "username-copy", false));
     m_ui->actionEntryCopyPassword->setIcon(filePath()->icon("actions", "password-copy", false));
+    m_ui->actionEntryCopyURL->setIcon(filePath()->icon("actions", "url-copy", false));
 
     m_ui->actionGroupNew->setIcon(filePath()->icon("actions", "group-new", false));
     m_ui->actionGroupEdit->setIcon(filePath()->icon("actions", "group-edit", false));

src/gui/MainWindow.ui

@@ -292,6 +292,8 @@
    <addaction name="separator"/>
    <addaction name="actionEntryCopyUsername"/>
    <addaction name="actionEntryCopyPassword"/>
+   <addaction name="actionEntryCopyURL"/>
+   <addaction name="actionEntryAutoType"/>
    <addaction name="separator"/>
    <addaction name="actionPasswordGenerator"/>
    <addaction name="actionLockDatabases"/>
@@ -495,6 +497,9 @@
    <property name="text">
     <string>&amp;Title</string>
    </property>
+   <property name="toolTip">
+    <string>Copy title to clipboard</string>
+   </property>
   </action>
   <action name="actionEntryCopyURL">
    <property name="enabled">
@@ -503,6 +508,9 @@
    <property name="text">
     <string>&amp;URL</string>
    </property>
+   <property name="toolTip">
+    <string>Copy URL to clipboard</string>
+   </property>
   </action>
   <action name="actionEntryCopyNotes">
    <property name="enabled">
@@ -511,6 +519,9 @@
    <property name="text">
     <string>&amp;Notes</string>
    </property>
+   <property name="toolTip">
+    <string>Copy notes to clipboard</string>
+   </property>
   </action>
   <action name="actionExportCsv">
    <property name="enabled">

src/gui/SettingsWidget.cpp

@@ -157,7 +157,7 @@ void SettingsWidget::loadSettings()
     m_secUi->lockDatabaseIdleSpinBox->setValue(config()->get("security/lockdatabaseidlesec").toInt());
     m_secUi->lockDatabaseMinimizeCheckBox->setChecked(config()->get("security/lockdatabaseminimize").toBool());
     m_secUi->lockDatabaseOnScreenLockCheckBox->setChecked(config()->get("security/lockdatabasescreenlock").toBool());
-    m_secUi->lockDatabaseOnScreenLockCheckBox->setChecked(config()->get("security/IconDownloadFallbackToGoogle").toBool());
+    m_secUi->fallbackToGoogle->setChecked(config()->get("security/IconDownloadFallbackToGoogle").toBool());
 
     m_secUi->passwordCleartextCheckBox->setChecked(config()->get("security/passwordscleartext").toBool());
     m_secUi->passwordRepeatCheckBox->setChecked(config()->get("security/passwordsrepeat").toBool());

src/keys/drivers/YubiKey.cpp

@@ -25,6 +25,7 @@
 #include <ykdef.h>
 #include <ykstatus.h>
 
+#include "core/Tools.h"
 #include "core/Global.h"
 #include "crypto/Random.h"
 
@@ -112,23 +113,36 @@ bool YubiKey::deinit()
 
 void YubiKey::detect()
 {
+    bool found = false;
+
     if (init()) {
-        for (int i = 1; i < 3; i++) {
         YubiKey::ChallengeResult result;
         QByteArray rand = randomGen()->randomArray(1);
         QByteArray resp;
 
+        // Check slot 1 and 2 for Challenge-Response HMAC capability
+        for (int i = 1; i <= 2; ++i) {
             result = challenge(i, false, rand, resp);
-            if (result == YubiKey::ALREADY_RUNNING) {
-                emit alreadyRunning();
-                return;
-            } else if (result != YubiKey::ERROR) {
-                emit detected(i, result == YubiKey::WOULDBLOCK);
-                return;
-            }
+            if (result == ALREADY_RUNNING) {
+                // Try this slot again after waiting
+                Tools::sleep(300);
+                result = challenge(i, false, rand, resp);
+            }
+
+            if (result != ALREADY_RUNNING && result != ERROR) {
+                emit detected(i, result == WOULDBLOCK);
+                found = true;
+            }
+            // Wait between slots to let the yubikey settle
+            Tools::sleep(150);
         }
     }
+
+    if (!found) {
         emit notFound();
+    } else {
+        emit detectComplete();
+    }
 }
 
 bool YubiKey::getSerial(unsigned int& serial)
@@ -160,6 +174,7 @@ YubiKey::ChallengeResult YubiKey::challenge(int slot, bool mayBlock, const QByte
     }
 
     // yk_challenge_response() insists on 64 byte response buffer */
+    response.clear();
     response.resize(64);
 
     /* The challenge sent to the yubikey should always be 64 bytes for

src/keys/drivers/YubiKey.h

@@ -87,6 +87,11 @@ signals:
      */
     void detected(int slot, bool blocking);
 
+    /**
+     * Emitted when detection is complete
+     */
+    void detectComplete();
+
     /**
      * Emitted when the YubiKey was challenged and has returned a response.
      */

tests/TestEntry.cpp

@@ -16,6 +16,7 @@
  */
 
 #include "TestEntry.h"
+#include "config-keepassx-tests.h"
 
 #include <QTest>
 
@@ -130,3 +131,30 @@ void TestEntry::testClone()
 
     delete entryOrg;
 }
+
+void TestEntry::testResolveUrl()
+{
+    Entry* entry = new Entry();
+    QString testUrl("www.google.com");
+    QString testCmd("cmd://firefox " + testUrl);
+    QString testComplexCmd("cmd://firefox --start-now --url 'http://" + testUrl + "' --quit");
+    QString nonHttpUrl("ftp://google.com");
+    QString noUrl("random text inserted here");
+
+    // Test standard URL's
+    QCOMPARE(entry->resolveUrl(""), QString(""));
+    QCOMPARE(entry->resolveUrl(testUrl), "https://" + testUrl);
+    QCOMPARE(entry->resolveUrl("http://" + testUrl), "http://" + testUrl);
+    // Test cmd:// with no URL
+    QCOMPARE(entry->resolveUrl("cmd://firefox"), QString(""));
+    QCOMPARE(entry->resolveUrl("cmd://firefox --no-url"), QString(""));
+    // Test cmd:// with URL's
+    QCOMPARE(entry->resolveUrl(testCmd), "https://" + testUrl);
+    QCOMPARE(entry->resolveUrl(testComplexCmd), "http://" + testUrl);
+    // Test non-http URL
+    QCOMPARE(entry->resolveUrl(nonHttpUrl), QString(""));
+    // Test no URL
+    QCOMPARE(entry->resolveUrl(noUrl), QString(""));
+
+    delete entry;
+}

tests/TestEntry.h

@@ -31,6 +31,7 @@ private slots:
     void testHistoryItemDeletion();
     void testCopyDataFrom();
     void testClone();
+    void testResolveUrl();
 };
 
 #endif // KEEPASSX_TESTENTRY_H