Merge branch 'release/2.3.2' into develop

CMakeLists.txt

@@ -245,8 +245,8 @@ if(MINGW)
   set(CMAKE_RC_COMPILE_OBJECT "<CMAKE_RC_COMPILER> <FLAGS> -O coff <DEFINES> -i <SOURCE> -o <OBJECT>")
   if(NOT (CMAKE_BUILD_TYPE STREQUAL "Debug" OR CMAKE_BUILD_TYPE STREQUAL "RelWithDebInfo"))
     # Enable DEP and ASLR
-    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--nxcompat -Wl,--dynamicbase")
-    set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -Wl,--nxcompat -Wl,--dynamicbase")
+    set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -Wl,--nxcompat -Wl,--dynamicbase -Wl,--high-entropy-va")
+    set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -Wl,--nxcompat -Wl,--dynamicbase -Wl,--high-entropy-va")
   endif()
 endif()
 

README.md

@@ -33,7 +33,7 @@ so please check out your distribution's package list to see if KeePassXC is avai
 [Mozilla Firefox](https://addons.mozilla.org/en-US/firefox/addon/keepasshttp-connector/) and
 [Google Chrome or Chromium](https://chrome.google.com/webstore/detail/keepasshttp-connector/dafgdjggglmmknipkhngniifhplpcldb), and
 [passafari](https://github.com/mmichaa/passafari.safariextension/) in Safari. [[See note about KeePassHTTP]](#Note_about_KeePassHTTP)
-- Browser integration with KeePassXC-Browser using [native messaging](https://developer.chrome.com/extensions/nativeMessaging) for [Mozilla Firefox](https://addons.mozilla.org/en-US/firefox/addon/keepassxc-browser/) and [Google Chrome or Chromium](https://chrome.google.com/webstore/detail/keepassxc-browser/iopaggbpplllidnfmcghoonnokmjoicf)
+- Browser integration with KeePassXC-Browser using [native messaging](https://developer.chrome.com/extensions/nativeMessaging) for [Mozilla Firefox](https://addons.mozilla.org/en-US/firefox/addon/keepassxc-browser/) and [Google Chrome or Chromium](https://chrome.google.com/webstore/detail/keepassxc-browser/oboonakemofpalcgghocfoadofidjkkk)
 - Many bug fixes
 
 For a full list of features and changes, read the [CHANGELOG](CHANGELOG) document.

src/autotype/AutoType.cpp

@@ -140,13 +140,6 @@ QStringList AutoType::windowTitles()
     return m_plugin->windowTitles();
 }
 
-void AutoType::resetInAutoType()
-{
-    m_inAutoType.unlock();
-
-    emit autotypeRejected();
-}
-
 void AutoType::raiseWindow()
 {
 #if defined(Q_OS_MAC)
@@ -199,9 +192,14 @@ int AutoType::callEventFilter(void* event)
  */
 void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, const QString& sequence, WId window)
 {
+    if (!m_inAutoType.tryLock()) {
+        return;
+    }
+
     // no edit to the sequence beyond this point
     if (!verifyAutoTypeSyntax(sequence)) {
         emit autotypeRejected();
+        m_inAutoType.unlock();
         return;
     }
 
@@ -210,6 +208,7 @@ void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, c
 
     if (!parseActions(sequence, entry, actions)) {
         emit autotypeRejected();
+        m_inAutoType.unlock();
         return;
     }
 
@@ -233,6 +232,7 @@ void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, c
         if (m_plugin->activeWindow() != window) {
             qWarning("Active window changed, interrupting auto-type.");
             emit autotypeRejected();
+            m_inAutoType.unlock();
             return;
         }
 
@@ -242,6 +242,8 @@ void AutoType::executeAutoTypeActions(const Entry* entry, QWidget* hideWindow, c
 
     // emit signal only if autotype performed correctly
     emit autotypePerformed();
+
+    m_inAutoType.unlock();
 }
 
 /**
@@ -259,13 +261,7 @@ void AutoType::performAutoType(const Entry* entry, QWidget* hideWindow)
         return;
     }
 
-    if (!m_inAutoType.tryLock()) {
-        return;
-    }
-
     executeAutoTypeActions(entry, hideWindow, sequences.first());
-
-    m_inAutoType.unlock();
 }
 
 /**
@@ -278,13 +274,14 @@ void AutoType::performGlobalAutoType(const QList<Database*>& dbList)
         return;
     }
 
-    QString windowTitle = m_plugin->activeWindowTitle();
-
-    if (windowTitle.isEmpty()) {
+    if (!m_inGlobalAutoTypeDialog.tryLock()) {
         return;
     }
 
-    if (!m_inAutoType.tryLock()) {
+    QString windowTitle = m_plugin->activeWindowTitle();
+
+    if (windowTitle.isEmpty()) {
+        m_inGlobalAutoTypeDialog.unlock();
         return;
     }
 
@@ -303,8 +300,6 @@ void AutoType::performGlobalAutoType(const QList<Database*>& dbList)
     }
 
     if (matchList.isEmpty()) {
-        m_inAutoType.unlock();
-
         if (qobject_cast<QApplication*>(QCoreApplication::instance())) {
             auto* msgBox = new QMessageBox();
             msgBox->setAttribute(Qt::WA_DeleteOnClose);
@@ -318,16 +313,20 @@ void AutoType::performGlobalAutoType(const QList<Database*>& dbList)
             msgBox->activateWindow();
         }
 
+        m_inGlobalAutoTypeDialog.unlock();
         emit autotypeRejected();
     } else if ((matchList.size() == 1) && !config()->get("security/autotypeask").toBool()) {
         executeAutoTypeActions(matchList.first().entry, nullptr, matchList.first().sequence);
-        m_inAutoType.unlock();
+        m_inGlobalAutoTypeDialog.unlock();
     } else {
         m_windowFromGlobal = m_plugin->activeWindow();
         auto* selectDialog = new AutoTypeSelectDialog();
+
+        // connect slots, both of which must unlock the m_inGlobalAutoTypeDialog mutex
         connect(selectDialog, SIGNAL(matchActivated(AutoTypeMatch)),
                 SLOT(performAutoTypeFromGlobal(AutoTypeMatch)));
-        connect(selectDialog, SIGNAL(rejected()), SLOT(resetInAutoType()));
+        connect(selectDialog, SIGNAL(rejected()), SLOT(autoTypeRejectedFromGlobal()));
+
         selectDialog->setMatchList(matchList);
 #if defined(Q_OS_MAC)
         m_plugin->raiseOwnWindow();
@@ -341,14 +340,22 @@ void AutoType::performGlobalAutoType(const QList<Database*>& dbList)
 
 void AutoType::performAutoTypeFromGlobal(AutoTypeMatch match)
 {
-    // We don't care about the result here, the mutex should already be locked. Now it's locked for sure
-    m_inAutoType.tryLock();
-
     m_plugin->raiseWindow(m_windowFromGlobal);
-
     executeAutoTypeActions(match.entry, nullptr, match.sequence, m_windowFromGlobal);
 
-    m_inAutoType.unlock();
+    // make sure the mutex is definitely locked before we unlock it
+    Q_UNUSED(m_inGlobalAutoTypeDialog.tryLock());
+    m_inGlobalAutoTypeDialog.unlock();
+}
+
+void AutoType::autoTypeRejectedFromGlobal()
+{
+    // this slot can be called twice when the selection dialog is deleted,
+    // so make sure the mutex is locked before we try unlocking it
+    Q_UNUSED(m_inGlobalAutoTypeDialog.tryLock());
+    m_inGlobalAutoTypeDialog.unlock();
+
+    emit autotypeRejected();
 }
 
 /**

src/autotype/AutoType.h

@@ -69,7 +69,7 @@ signals:
 
 private slots:
     void performAutoTypeFromGlobal(AutoTypeMatch match);
-    void resetInAutoType();
+    void autoTypeRejectedFromGlobal();
     void unloadPlugin();
 
 private:
@@ -88,6 +88,7 @@ private:
     bool windowMatches(const QString& windowTitle, const QString& windowPattern);
 
     QMutex m_inAutoType;
+    QMutex m_inGlobalAutoTypeDialog;
     int m_autoTypeDelay;
     Qt::Key m_currentGlobalKey;
     Qt::KeyboardModifiers m_currentGlobalModifiers;

src/browser/NativeMessagingBase.cpp

@@ -121,7 +121,8 @@ void NativeMessagingBase::sendReply(const QJsonObject& json)
 void NativeMessagingBase::sendReply(const QString& reply)
 {
     if (!reply.isEmpty()) {
-        uint len = reply.length();
+        QByteArray bytes = reply.toUtf8();
+        uint len = bytes.size();
         std::cout << char(((len>>0) & 0xFF)) << char(((len>>8) & 0xFF)) << char(((len>>16) & 0xFF)) << char(((len>>24) & 0xFF));
         std::cout << reply.toStdString() << std::flush;
     }

src/core/FilePath.cpp

@@ -97,6 +97,15 @@ QString FilePath::wordlistPath(const QString& name)
 }
 
 QIcon FilePath::applicationIcon()
+{
+#ifdef KEEPASSXC_DIST_SNAP
+    return icon("apps", "keepassxc", false);
+#else
+    return icon("apps", "keepassxc");
+#endif
+}
+
+QIcon FilePath::trayIcon()
 {
     bool darkIcon = useDarkIcon();
 
@@ -107,7 +116,6 @@ QIcon FilePath::applicationIcon()
 #endif
 }
 
-
 QIcon FilePath::trayIconLocked()
 {
 #ifdef KEEPASSXC_DIST_SNAP

src/core/FilePath.h

@@ -29,6 +29,7 @@ public:
     QString pluginPath(const QString& name);
     QString wordlistPath(const QString& name);
     QIcon applicationIcon();
+    QIcon trayIcon();
     QIcon trayIconLocked();
     QIcon trayIconUnlocked();
     QIcon icon(const QString& category, const QString& name, bool fromTheme = true);

src/gui/DatabaseOpenWidget.cpp

@@ -173,6 +173,9 @@ void DatabaseOpenWidget::openDatabase()
         return;
     }
 
+    m_ui->editPassword->setShowPassword(false);
+    QCoreApplication::processEvents();
+
     QFile file(m_filename);
     if (!file.open(QIODevice::ReadOnly)) {
         m_ui->messageWidget->showMessage(

src/gui/MainWindow.cpp

@@ -899,7 +899,7 @@ void MainWindow::updateTrayIcon()
 
             m_trayIcon->setContextMenu(menu);
             
-            m_trayIcon->setIcon(filePath()->applicationIcon());
+            m_trayIcon->setIcon(filePath()->trayIcon());
             m_trayIcon->show();
         }
         if (m_ui->tabWidget->hasLockableDatabases()) {
@@ -971,7 +971,11 @@ void MainWindow::trayIconTriggered(QSystemTrayIcon::ActivationReason reason)
 void MainWindow::hideWindow()
 {
     saveWindowInformation();
-#ifndef Q_OS_MAC
+#if !defined(Q_OS_LINUX) && !defined(Q_OS_MAC)
+    // On some Linux systems, the window should NOT be minimized and hidden (i.e. not shown), at
+    // the same time (which would happen if both minimize on startup and minimize to tray are set)
+    // since otherwise it causes problems on restore as seen on issue #1595. Hiding it is enough.
+    // TODO: Add an explanation for why this is also not done on Mac (or remove the check)
     setWindowState(windowState() | Qt::WindowMinimized);
 #endif
     QTimer::singleShot(0, this, SLOT(hide()));
@@ -983,7 +987,7 @@ void MainWindow::hideWindow()
 
 void MainWindow::toggleWindow()
 {
-    if ((QApplication::activeWindow() == this) && isVisible() && !isMinimized()) {
+    if (isVisible() && !isMinimized()) {
         hideWindow();
     } else {
         bringToFront();

src/gui/entry/EditEntryWidget.cpp

@@ -281,6 +281,8 @@ void EditEntryWidget::setupSSHAgent()
     connect(m_sshAgentUi->decryptButton, SIGNAL(clicked()), SLOT(decryptPrivateKey()));
     connect(m_sshAgentUi->copyToClipboardButton, SIGNAL(clicked()), SLOT(copyPublicKey()));
 
+    connect(m_advancedUi->attachmentsWidget->entryAttachments(), SIGNAL(modified()), SLOT(updateAttachments()));
+
     addPage(tr("SSH Agent"), FilePath::instance()->icon("apps", "utilities-terminal"), m_sshAgentWidget);
 }
 
@@ -299,6 +301,27 @@ void EditEntryWidget::updateSSHAgent()
     m_sshAgentUi->removeFromAgentButton->setEnabled(false);
     m_sshAgentUi->copyToClipboardButton->setEnabled(false);
 
+    m_sshAgentSettings = settings;
+    updateSSHAgentAttachments();
+
+    if (settings.selectedType() == "attachment") {
+        m_sshAgentUi->attachmentRadioButton->setChecked(true);
+    } else {
+        m_sshAgentUi->externalFileRadioButton->setChecked(true);
+    }
+
+    updateSSHAgentKeyInfo();
+}
+
+void EditEntryWidget::updateSSHAgentAttachment()
+{
+    m_sshAgentUi->attachmentRadioButton->setChecked(true);
+    updateSSHAgentKeyInfo();
+}
+
+void EditEntryWidget::updateSSHAgentAttachments()
+{
+    m_sshAgentUi->attachmentComboBox->clear();
     m_sshAgentUi->attachmentComboBox->addItem("");
 
     auto attachments = m_advancedUi->attachmentsWidget->entryAttachments();
@@ -310,24 +333,8 @@ void EditEntryWidget::updateSSHAgent()
         m_sshAgentUi->attachmentComboBox->addItem(fileName);
     }
 
-    m_sshAgentUi->attachmentComboBox->setCurrentText(settings.attachmentName());
-    m_sshAgentUi->externalFileEdit->setText(settings.fileName());
-
-    if (settings.selectedType() == "attachment") {
-        m_sshAgentUi->attachmentRadioButton->setChecked(true);
-    } else {
-        m_sshAgentUi->externalFileRadioButton->setChecked(true);
-    }
-
-    m_sshAgentSettings = settings;
-
-    updateSSHAgentKeyInfo();
-}
-
-void EditEntryWidget::updateSSHAgentAttachment()
-{
-    m_sshAgentUi->attachmentRadioButton->setChecked(true);
-    updateSSHAgentKeyInfo();
+    m_sshAgentUi->attachmentComboBox->setCurrentText(m_sshAgentSettings.attachmentName());
+    m_sshAgentUi->externalFileEdit->setText(m_sshAgentSettings.fileName());
 }
 
 void EditEntryWidget::updateSSHAgentKeyInfo()
@@ -420,12 +427,16 @@ void EditEntryWidget::browsePrivateKey()
 
 bool EditEntryWidget::getOpenSSHKey(OpenSSHKey& key, bool decrypt)
 {
+    QString fileName;
     QByteArray privateKeyData;
 
     if (m_sshAgentUi->attachmentRadioButton->isChecked()) {
-        privateKeyData = m_advancedUi->attachmentsWidget->getAttachment(m_sshAgentUi->attachmentComboBox->currentText());
+        fileName = m_sshAgentUi->attachmentComboBox->currentText();
+        privateKeyData = m_advancedUi->attachmentsWidget->getAttachment(fileName);
     } else {
         QFile localFile(m_sshAgentUi->externalFileEdit->text());
+        QFileInfo localFileInfo(localFile);
+        fileName = localFileInfo.fileName();
 
         if (localFile.fileName().isEmpty()) {
             return false;
@@ -464,6 +475,10 @@ bool EditEntryWidget::getOpenSSHKey(OpenSSHKey& key, bool decrypt)
         key.setComment(m_entry->username());
     }
 
+    if (key.comment().isEmpty()) {
+        key.setComment(fileName);
+    }
+
     return true;
 }
 

src/gui/entry/EditEntryWidget.h

@@ -103,6 +103,7 @@ private slots:
 #ifdef WITH_XC_SSHAGENT
     void updateSSHAgent();
     void updateSSHAgentAttachment();
+    void updateSSHAgentAttachments();
     void updateSSHAgentKeyInfo();
     void browsePrivateKey();
     void addKeyToAgent();

src/main.cpp

@@ -29,6 +29,8 @@
 #include "gui/MainWindow.h"
 #include "gui/MessageBox.h"
 
+#include "cli/Utils.h"
+
 #if defined(WITH_ASAN) && defined(WITH_LSAN)
 #include <sanitizer/lsan_interface.h>
 #endif
@@ -125,7 +127,14 @@ int main(int argc, char** argv)
     // start minimized if configured
     bool minimizeOnStartup = config()->get("GUI/MinimizeOnStartup").toBool();
     bool minimizeToTray    = config()->get("GUI/MinimizeToTray").toBool();
+#ifndef Q_OS_LINUX
     if (minimizeOnStartup) {
+#else
+    // On some Linux systems, the window should NOT be minimized and hidden (i.e. not shown), at
+    // the same time (which would happen if both minimize on startup and minimize to tray are set)
+    // since otherwise it causes problems on restore as seen on issue #1595. Hiding it is enough.
+    if (minimizeOnStartup && !minimizeToTray) {
+#endif
         mainWindow.setWindowState(Qt::WindowMinimized);
     }
     if (!(minimizeOnStartup && minimizeToTray)) {
@@ -148,7 +157,9 @@ int main(int argc, char** argv)
             // we always need consume a line of STDIN if --pw-stdin is set to clear out the
             // buffer for native messaging, even if the specified file does not exist
             static QTextStream in(stdin, QIODevice::ReadOnly);
-            password = in.readLine();
+            static QTextStream out(stdout, QIODevice::WriteOnly);
+            out << QCoreApplication::translate("Main", "Database password: ") << flush;
+            password = Utils::getPassword();
         }
 
         if (!filename.isEmpty() && QFile::exists(filename) && !filename.endsWith(".json", Qt::CaseInsensitive)) {

src/sshagent/SSHAgent.cpp

@@ -186,7 +186,17 @@ bool SSHAgent::addIdentity(OpenSSHKey& key, quint32 lifetime, bool confirm)
     }
 
     if (responseData.length() < 1 || static_cast<quint8>(responseData[0]) != SSH_AGENT_SUCCESS) {
-        m_error = tr("Agent refused this identity.");
+        m_error = tr("Agent refused this identity. Possible reasons include:")
+            + "\n" + tr("The key has already been added.");
+
+        if (lifetime > 0) {
+            m_error += "\n" + tr("Restricted lifetime is not supported by the agent (check options).");
+        }
+
+        if (confirm) {
+            m_error += "\n" + tr("A confirmation request is not supported by the agent (check options).");
+        }
+
         return false;
     }
 
@@ -268,10 +278,15 @@ void SSHAgent::databaseModeChanged(DatabaseWidget::Mode mode)
             }
 
             QByteArray keyData;
+            QString fileName;
             if (settings.selectedType() == "attachment") {
-                keyData = e->attachments()->value(settings.attachmentName());
+                fileName = settings.attachmentName();
+                keyData = e->attachments()->value(fileName);
             } else if (!settings.fileName().isEmpty()) {
                 QFile file(settings.fileName());
+                QFileInfo fileInfo(file);
+
+                fileName = fileInfo.fileName();
 
                 if (file.size() > 1024 * 1024) {
                     continue;
@@ -302,6 +317,10 @@ void SSHAgent::databaseModeChanged(DatabaseWidget::Mode mode)
                 key.setComment(e->username());
             }
 
+            if (key.comment().isEmpty()) {
+                key.setComment(fileName);
+            }
+
             if (settings.removeAtDatabaseClose()) {
                 removeIdentityAtLock(key, uuid);
             }

src/sshagent/blf.h

@@ -34,17 +34,7 @@
 #ifndef _BLF_H_
 #define _BLF_H_
 
-#ifdef _WIN32
-
-#include <stdint.h>
-
-typedef uint32_t u_int32_t;
-typedef uint16_t u_int16_t;
-typedef uint8_t u_int8_t;
-
-#define bzero(p,s) memset(p, 0, s)
-
-#endif
+#include "includes.h"
 
 #if !defined(HAVE_BCRYPT_PBKDF) && !defined(HAVE_BLH_H)
 

src/sshagent/blowfish.c

@@ -39,7 +39,7 @@
  * Bruce Schneier.
  */
 
-#define HAVE_BLF_H
+#include "includes.h"
 
 #if !defined(HAVE_BCRYPT_PBKDF) && (!defined(HAVE_BLOWFISH_INITSTATE) || \
     !defined(HAVE_BLOWFISH_EXPAND0STATE) || !defined(HAVE_BLF_ENC))
@@ -51,7 +51,7 @@
 
 #include <sys/types.h>
 #ifdef HAVE_BLF_H
-#include "blf.h"
+#include <blf.h>
 #endif
 
 #undef inline

src/sshagent/includes.h

@@ -0,0 +1,20 @@
+// mimic openSSH-portable's includes.h file to be able to use
+// its unmodified blowfish code
+
+#define HAVE_BLF_H
+
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE /* activate extra prototypes for glibc */
+#endif
+#include <sys/types.h>
+
+
+#ifdef _WIN32
+#include <stdint.h>
+
+typedef uint32_t u_int32_t;
+typedef uint16_t u_int16_t;
+typedef uint8_t u_int8_t;
+
+#define bzero(p,s) memset(p, 0, s)
+#endif

tests/gui/TestGui.cpp

@@ -71,6 +71,8 @@ void TestGui::initTestCase()
     Config::createTempFileInstance();
     // Disable autosave so we can test the modified file indicator
     Config::instance()->set("AutoSaveAfterEveryChange", false);
+    // Enable the tray icon so we can test hiding/restoring the window
+    Config::instance()->set("GUI/ShowTrayIcon", true);
 
     m_mainWindow = new MainWindow();
     m_tabWidget = m_mainWindow->findChild<DatabaseTabWidget*>("tabWidget");
@@ -1091,6 +1093,32 @@ void TestGui::testDragAndDropKdbxFiles()
     QCOMPARE(m_tabWidget->count(), openedDatabasesCount);
 }
 
+void TestGui::testTrayRestoreHide()
+{
+    if (!QSystemTrayIcon::isSystemTrayAvailable()) {
+        QSKIP("QSystemTrayIcon::isSystemTrayAvailable() = false, skipping tray restore/hide test...");
+    }
+
+    QSystemTrayIcon* trayIcon = m_mainWindow->findChild<QSystemTrayIcon*>();
+    QVERIFY(m_mainWindow->isVisible());
+
+    trayIcon->activated(QSystemTrayIcon::Trigger);
+    Tools::wait(100);
+    QVERIFY(!m_mainWindow->isVisible());
+
+    trayIcon->activated(QSystemTrayIcon::Trigger);
+    Tools::wait(100);
+    QVERIFY(m_mainWindow->isVisible());
+
+    trayIcon->activated(QSystemTrayIcon::Trigger);
+    Tools::wait(100);
+    QVERIFY(!m_mainWindow->isVisible());
+
+    trayIcon->activated(QSystemTrayIcon::Trigger);
+    Tools::wait(100);
+    QVERIFY(m_mainWindow->isVisible());
+}
+
 void TestGui::cleanupTestCase()
 {
     delete m_mainWindow;

tests/gui/TestGui.h

@@ -63,6 +63,7 @@ private slots:
     void testKeePass1Import();
     void testDatabaseLocking();
     void testDragAndDropKdbxFiles();
+    void testTrayRestoreHide();
 
 private:
     int addCannedEntries();