Allow toggling SSH Agent integration without restart

- use Q_GLOBAL_STATIC for singleton
- move all configuration to SSHAgent class
- various cleanups to agent code

Fixes #1196

src/gui/DatabaseWidget.cpp

@@ -217,9 +217,9 @@ DatabaseWidget::DatabaseWidget(QSharedPointer<Database> db, QWidget* parent)
     m_searchLimitGroup = config()->get("SearchLimitGroup", false).toBool();
 
 #ifdef WITH_XC_SSHAGENT
-    if (config()->get("SSHAgent", false).toBool()) {
+    if (sshAgent()->isEnabled()) {
-        connect(this, SIGNAL(databaseLocked()), SSHAgent::instance(), SLOT(databaseModeChanged()));
+        connect(this, SIGNAL(databaseLocked()), sshAgent(), SLOT(databaseModeChanged()));
-        connect(this, SIGNAL(databaseUnlocked()), SSHAgent::instance(), SLOT(databaseModeChanged()));
+        connect(this, SIGNAL(databaseUnlocked()), sshAgent(), SLOT(databaseModeChanged()));
     }
 #endif
 

src/gui/EditWidget.cpp

@@ -74,7 +74,7 @@ void EditWidget::setPageHidden(QWidget* widget, bool hidden)
 
     for (int i = 0; i < m_ui->stackedWidget->count(); i++) {
         auto* scrollArea = qobject_cast<QScrollArea*>(m_ui->stackedWidget->widget(i));
-        if (scrollArea != nullptr && scrollArea->widget() == widget) {
+        if (scrollArea && scrollArea->widget() == widget) {
             index = i;
             break;
         }

src/gui/MainWindow.cpp

@@ -189,8 +189,7 @@ MainWindow::MainWindow()
 #endif
 
 #ifdef WITH_XC_SSHAGENT
-    SSHAgent::init(this);
+    connect(sshAgent(), SIGNAL(error(QString)), this, SLOT(showErrorMessage(QString)));
-    connect(SSHAgent::instance(), SIGNAL(error(QString)), this, SLOT(showErrorMessage(QString)));
     m_ui->settingsWidget->addSettingsPage(new AgentSettingsPage(m_ui->tabWidget));
 #endif
 

src/gui/entry/EditEntryWidget.cpp

@@ -107,7 +107,6 @@ EditEntryWidget::EditEntryWidget(QWidget* parent)
 
 #ifdef WITH_XC_SSHAGENT
     setupSSHAgent();
-    m_sshAgentEnabled = config()->get("SSHAgent", false).toBool();
 #endif
 
 #ifdef WITH_XC_BROWSER
@@ -451,7 +450,7 @@ void EditEntryWidget::setupEntryUpdate()
 
 #ifdef WITH_XC_SSHAGENT
     // SSH Agent tab
-    if (config()->get("SSHAgent", false).toBool()) {
+    if (sshAgent()->isEnabled()) {
         connect(m_sshAgentUi->attachmentRadioButton, SIGNAL(toggled(bool)), this, SLOT(setModified()));
         connect(m_sshAgentUi->externalFileRadioButton, SIGNAL(toggled(bool)), this, SLOT(setModified()));
         connect(m_sshAgentUi->attachmentComboBox, SIGNAL(currentIndexChanged(int)), this, SLOT(setModified()));
@@ -628,11 +627,11 @@ void EditEntryWidget::updateSSHAgentKeyInfo()
     }
 
     // enable agent buttons only if we have an agent running
-    if (SSHAgent::instance()->isAgentRunning()) {
+    if (sshAgent()->isAgentRunning()) {
         m_sshAgentUi->addToAgentButton->setEnabled(true);
         m_sshAgentUi->removeFromAgentButton->setEnabled(true);
 
-        SSHAgent::instance()->setAutoRemoveOnLock(key, m_sshAgentUi->removeKeyFromAgentCheckBox->isChecked());
+        sshAgent()->setAutoRemoveOnLock(key, m_sshAgentUi->removeKeyFromAgentCheckBox->isChecked());
     }
 }
 
@@ -700,8 +699,8 @@ void EditEntryWidget::addKeyToAgent()
     KeeAgentSettings settings;
     toKeeAgentSettings(settings);
 
-    if (!SSHAgent::instance()->addIdentity(key, settings)) {
+    if (!sshAgent()->addIdentity(key, settings)) {
-        showMessage(SSHAgent::instance()->errorString(), MessageWidget::Error);
+        showMessage(sshAgent()->errorString(), MessageWidget::Error);
         return;
     }
 }
@@ -714,8 +713,8 @@ void EditEntryWidget::removeKeyFromAgent()
         return;
     }
 
-    if (!SSHAgent::instance()->removeIdentity(key)) {
+    if (!sshAgent()->removeIdentity(key)) {
-        showMessage(SSHAgent::instance()->errorString(), MessageWidget::Error);
+        showMessage(sshAgent()->errorString(), MessageWidget::Error);
         return;
     }
 }
@@ -792,6 +791,9 @@ void EditEntryWidget::loadEntry(Entry* entry,
 
     setCurrentPage(0);
     setPageHidden(m_historyWidget, m_history || m_entry->historyItems().count() < 1);
+#ifdef WITH_XC_SSHAGENT
+    setPageHidden(m_sshAgentWidget, !sshAgent()->isEnabled());
+#endif
 
     // Force the user to Save/Discard new entries
     showApplyButton(!m_create);
@@ -903,7 +905,7 @@ void EditEntryWidget::setForms(Entry* entry, bool restore)
     updateAutoTypeEnabled();
 
 #ifdef WITH_XC_SSHAGENT
-    if (m_sshAgentEnabled) {
+    if (sshAgent()->isEnabled()) {
         updateSSHAgent();
     }
 #endif
@@ -1090,7 +1092,7 @@ void EditEntryWidget::updateEntryData(Entry* entry) const
     entry->autoTypeAssociations()->copyDataFrom(m_autoTypeAssoc);
 
 #ifdef WITH_XC_SSHAGENT
-    if (m_sshAgentEnabled) {
+    if (sshAgent()->isEnabled()) {
         m_sshAgentSettings.toEntry(entry);
     }
 #endif

src/gui/entry/EditEntryWidget.h

@@ -164,7 +164,6 @@ private:
     bool m_create;
     bool m_history;
 #ifdef WITH_XC_SSHAGENT
-    bool m_sshAgentEnabled;
     KeeAgentSettings m_sshAgentSettings;
 #endif
     const QScopedPointer<Ui::EditEntryWidgetMain> m_mainUi;

src/sshagent/AgentSettingsWidget.cpp

@@ -33,8 +33,7 @@ AgentSettingsWidget::AgentSettingsWidget(QWidget* parent)
 #else
     m_ui->sshAuthSockWidget->setVisible(false);
 #endif
-    auto sshAgentEnabled = config()->get("SSHAgent", false).toBool();
+    m_ui->sshAuthSockMessageWidget->setVisible(sshAgent()->isEnabled());
-    m_ui->sshAuthSockMessageWidget->setVisible(sshAgentEnabled);
     m_ui->sshAuthSockMessageWidget->setCloseButtonVisible(false);
     m_ui->sshAuthSockMessageWidget->setAutoHideTimeout(-1);
 }
@@ -45,20 +44,21 @@ AgentSettingsWidget::~AgentSettingsWidget()
 
 void AgentSettingsWidget::loadSettings()
 {
-    auto sshAgentEnabled = config()->get("SSHAgent", false).toBool();
+    auto sshAgentEnabled = sshAgent()->isEnabled();
+
     m_ui->enableSSHAgentCheckBox->setChecked(sshAgentEnabled);
 #ifdef Q_OS_WIN
-    m_ui->useOpenSSHCheckBox->setChecked(config()->get("SSHAgentOpenSSH", false).toBool());
+    m_ui->useOpenSSHCheckBox->setChecked(sshAgent()->useOpenSSH());
 #else
-    auto sshAuthSock = QProcessEnvironment::systemEnvironment().value("SSH_AUTH_SOCK");
+    auto sshAuthSock = sshAgent()->socketPath(false);
-    auto sshAuthSockOverride = config()->get("SSHAuthSockOverride", "").toString();
+    auto sshAuthSockOverride = sshAgent()->authSockOverride();
     m_ui->sshAuthSockLabel->setText(sshAuthSock.isEmpty() ? tr("(empty)") : sshAuthSock);
     m_ui->sshAuthSockOverrideEdit->setText(sshAuthSockOverride);
 #endif
 
-    if (sshAgentEnabled) {
+    m_ui->sshAuthSockMessageWidget->setVisible(sshAgentEnabled);
-        m_ui->sshAuthSockMessageWidget->setVisible(true);
 
+    if (sshAgentEnabled) {
 #ifndef Q_OS_WIN
         if (sshAuthSock.isEmpty() && sshAuthSockOverride.isEmpty()) {
             m_ui->sshAuthSockMessageWidget->showMessage(
@@ -68,20 +68,21 @@ void AgentSettingsWidget::loadSettings()
             return;
         }
 #endif
-        if (SSHAgent::instance()->testConnection()) {
+        if (sshAgent()->testConnection()) {
             m_ui->sshAuthSockMessageWidget->showMessage(tr("SSH Agent connection is working!"),
                                                         MessageWidget::Positive);
         } else {
-            m_ui->sshAuthSockMessageWidget->showMessage(SSHAgent::instance()->errorString(), MessageWidget::Error);
+            m_ui->sshAuthSockMessageWidget->showMessage(sshAgent()->errorString(), MessageWidget::Error);
         }
     }
 }
 
 void AgentSettingsWidget::saveSettings()
 {
-    config()->set("SSHAgent", m_ui->enableSSHAgentCheckBox->isChecked());
+    auto sshAuthSockOverride = m_ui->sshAuthSockOverrideEdit->text();
-    config()->set("SSHAuthSockOverride", m_ui->sshAuthSockOverrideEdit->text());
+    sshAgent()->setAuthSockOverride(sshAuthSockOverride);
 #ifdef Q_OS_WIN
-    config()->set("SSHAgentOpenSSH", m_ui->useOpenSSHCheckBox->isChecked());
+    sshAgent()->setUseOpenSSH(m_ui->useOpenSSHCheckBox->isChecked());
 #endif
+    sshAgent()->setEnabled(m_ui->enableSSHAgentCheckBox->isChecked());
 }

src/sshagent/AgentSettingsWidget.ui

@@ -26,7 +26,7 @@
    <item>
     <widget class="QCheckBox" name="enableSSHAgentCheckBox">
      <property name="text">
-      <string>Enable SSH Agent (requires restart)</string>
+      <string>Enable SSH Agent integration</string>
      </property>
     </widget>
    </item>

src/sshagent/SSHAgent.cpp

@@ -29,46 +29,72 @@
 #include <windows.h>
 #endif
 
-SSHAgent* SSHAgent::m_instance;
+Q_GLOBAL_STATIC(SSHAgent, s_sshAgent);
-
-SSHAgent::SSHAgent(QObject* parent)
-    : QObject(parent)
-{
-#ifndef Q_OS_WIN
-    m_socketPath = config()->get("SSHAuthSockOverride", "").toString();
-    if (m_socketPath.isEmpty()) {
-        m_socketPath = QProcessEnvironment::systemEnvironment().value("SSH_AUTH_SOCK");
-    }
-#else
-    m_socketPath = "\\\\.\\pipe\\openssh-ssh-agent";
-#endif
-}
 
 SSHAgent::~SSHAgent()
 {
-    auto it = m_addedKeys.begin();
+    removeAllIdentities();
-    while (it != m_addedKeys.end()) {
-        // Remove key if requested to remove on lock
-        if (it.value()) {
-            OpenSSHKey key = it.key();
-            removeIdentity(key);
-        }
-        it = m_addedKeys.erase(it);
-    }
 }
 
 SSHAgent* SSHAgent::instance()
 {
-    if (!m_instance) {
+    return s_sshAgent;
-        qFatal("Race condition: instance wanted before it was initialized, this is a bug.");
-    }
-
-    return m_instance;
 }
 
-void SSHAgent::init(QObject* parent)
+bool SSHAgent::isEnabled() const
 {
-    m_instance = new SSHAgent(parent);
+    return config()->get("SSHAgent").toBool();
+}
+
+void SSHAgent::setEnabled(bool enabled)
+{
+    if (isEnabled() && !enabled) {
+        removeAllIdentities();
+    }
+
+    config()->set("SSHAgent", enabled);
+}
+
+QString SSHAgent::authSockOverride() const
+{
+    return config()->get("SSHAuthSockOverride").toString();
+}
+
+void SSHAgent::setAuthSockOverride(QString& authSockOverride)
+{
+    config()->set("SSHAuthSockOverride", authSockOverride);
+}
+
+#ifdef Q_OS_WIN
+bool SSHAgent::useOpenSSH() const
+{
+    return config()->get("SSHAgentOpenSSH").toBool();
+}
+
+void SSHAgent::setUseOpenSSH(bool useOpenSSH)
+{
+    config()->set("SSHAgentOpenSSH", useOpenSSH);
+}
+#endif
+
+QString SSHAgent::socketPath(bool allowOverride = true) const
+{
+    QString socketPath;
+
+#ifndef Q_OS_WIN
+    if (allowOverride) {
+        socketPath = authSockOverride();
+    }
+
+    // if the overridden path is empty (no override set), default to environment
+    if (socketPath.isEmpty()) {
+        socketPath = QProcessEnvironment::systemEnvironment().value("SSH_AUTH_SOCK");
+    }
+#else
+    socketPath = "\\\\.\\pipe\\openssh-ssh-agent";
+#endif
+
+    return socketPath;
 }
 
 const QString SSHAgent::errorString() const
@@ -79,12 +105,13 @@ const QString SSHAgent::errorString() const
 bool SSHAgent::isAgentRunning() const
 {
 #ifndef Q_OS_WIN
-    return !m_socketPath.isEmpty();
+    QFileInfo socketFileInfo(socketPath());
+    return !socketFileInfo.path().isEmpty() && socketFileInfo.exists();
 #else
-    if (!config()->get("SSHAgentOpenSSH").toBool()) {
+    if (!useOpenSSH()) {
         return (FindWindowA("Pageant", "Pageant") != nullptr);
     } else {
-        return WaitNamedPipe(m_socketPath.toLatin1().data(), 100);
+        return WaitNamedPipe(socketPath().toLatin1().data(), 100);
     }
 #endif
 }
@@ -92,7 +119,7 @@ bool SSHAgent::isAgentRunning() const
 bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out)
 {
 #ifdef Q_OS_WIN
-    if (!config()->get("SSHAgentOpenSSH").toBool()) {
+    if (!useOpenSSH()) {
         return sendMessagePageant(in, out);
     }
 #endif
@@ -100,7 +127,7 @@ bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out)
     QLocalSocket socket;
     BinaryStream stream(&socket);
 
-    socket.connectToServer(m_socketPath);
+    socket.connectToServer(socketPath());
     if (!socket.waitForConnected(500)) {
         m_error = tr("Agent connection failed.");
         return false;
@@ -300,6 +327,22 @@ bool SSHAgent::removeIdentity(OpenSSHKey& key)
     return sendMessage(requestData, responseData);
 }
 
+/**
+ * Remove all identities known to this instance
+ */
+void SSHAgent::removeAllIdentities()
+{
+    auto it = m_addedKeys.begin();
+    while (it != m_addedKeys.end()) {
+        // Remove key if requested to remove on lock
+        if (it.value()) {
+            OpenSSHKey key = it.key();
+            removeIdentity(key);
+        }
+        it = m_addedKeys.erase(it);
+    }
+}
+
 /**
  * Change "remove identity on lock" setting for a key already added to the agent.
  * Will to nothing if the key has not been added to the agent.

src/sshagent/SSHAgent.h

@@ -32,14 +32,25 @@ class SSHAgent : public QObject
     Q_OBJECT
 
 public:
+    ~SSHAgent() override;
     static SSHAgent* instance();
-    static void init(QObject* parent);
+
+    bool isEnabled() const;
+    void setEnabled(bool enabled);
+    QString socketPath(bool allowOverride) const;
+    QString authSockOverride() const;
+    void setAuthSockOverride(QString& authSockOverride);
+#ifdef Q_OS_WIN
+    bool useOpenSSH() const;
+    void setUseOpenSSH(bool useOpenSSH);
+#endif
 
     const QString errorString() const;
     bool isAgentRunning() const;
     bool testConnection();
     bool addIdentity(OpenSSHKey& key, KeeAgentSettings& settings);
     bool removeIdentity(OpenSSHKey& key);
+    void removeAllIdentities();
     void setAutoRemoveOnLock(const OpenSSHKey& key, bool autoRemove);
 
 signals:
@@ -60,18 +71,10 @@ private:
     const quint8 SSH_AGENT_CONSTRAIN_LIFETIME = 1;
     const quint8 SSH_AGENT_CONSTRAIN_CONFIRM = 2;
 
-    explicit SSHAgent(QObject* parent = nullptr);
-    ~SSHAgent();
-
     bool sendMessage(const QByteArray& in, QByteArray& out);
 #ifdef Q_OS_WIN
     bool sendMessagePageant(const QByteArray& in, QByteArray& out);
-#endif
 
-    static SSHAgent* m_instance;
-
-    QString m_socketPath;
-#ifdef Q_OS_WIN
     const quint32 AGENT_MAX_MSGLEN = 8192;
     const quint32 AGENT_COPYDATA_ID = 0x804e50ba;
 #endif
@@ -80,4 +83,9 @@ private:
     QString m_error;
 };
 
+static inline SSHAgent* sshAgent()
+{
+    return SSHAgent::instance();
+}
+
 #endif // KEEPASSXC_SSHAGENT_H