From 40ad211f3e70b715f3960097563730926a07fe57 Mon Sep 17 00:00:00 2001 From: Toni Spets Date: Tue, 28 Jan 2020 20:46:23 +0200 Subject: [PATCH] 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 | 6 +- src/gui/EditWidget.cpp | 2 +- src/gui/MainWindow.cpp | 3 +- src/gui/entry/EditEntryWidget.cpp | 22 +++--- src/gui/entry/EditEntryWidget.h | 1 - src/sshagent/AgentSettingsWidget.cpp | 27 ++++--- src/sshagent/AgentSettingsWidget.ui | 2 +- src/sshagent/SSHAgent.cpp | 113 ++++++++++++++++++--------- src/sshagent/SSHAgent.h | 26 +++--- 9 files changed, 127 insertions(+), 75 deletions(-) diff --git a/src/gui/DatabaseWidget.cpp b/src/gui/DatabaseWidget.cpp index fd579b04a..d7521f3b2 100644 --- a/src/gui/DatabaseWidget.cpp +++ b/src/gui/DatabaseWidget.cpp @@ -217,9 +217,9 @@ DatabaseWidget::DatabaseWidget(QSharedPointer db, QWidget* parent) m_searchLimitGroup = config()->get("SearchLimitGroup", false).toBool(); #ifdef WITH_XC_SSHAGENT - if (config()->get("SSHAgent", false).toBool()) { - connect(this, SIGNAL(databaseLocked()), SSHAgent::instance(), SLOT(databaseModeChanged())); - connect(this, SIGNAL(databaseUnlocked()), SSHAgent::instance(), SLOT(databaseModeChanged())); + if (sshAgent()->isEnabled()) { + connect(this, SIGNAL(databaseLocked()), sshAgent(), SLOT(databaseModeChanged())); + connect(this, SIGNAL(databaseUnlocked()), sshAgent(), SLOT(databaseModeChanged())); } #endif diff --git a/src/gui/EditWidget.cpp b/src/gui/EditWidget.cpp index cfae5d7e6..f9bcbb5af 100644 --- a/src/gui/EditWidget.cpp +++ b/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(m_ui->stackedWidget->widget(i)); - if (scrollArea != nullptr && scrollArea->widget() == widget) { + if (scrollArea && scrollArea->widget() == widget) { index = i; break; } diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index b0dec4bf5..b225165a6 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -189,8 +189,7 @@ MainWindow::MainWindow() #endif #ifdef WITH_XC_SSHAGENT - SSHAgent::init(this); - connect(SSHAgent::instance(), SIGNAL(error(QString)), this, SLOT(showErrorMessage(QString))); + connect(sshAgent(), SIGNAL(error(QString)), this, SLOT(showErrorMessage(QString))); m_ui->settingsWidget->addSettingsPage(new AgentSettingsPage(m_ui->tabWidget)); #endif diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index 2880e8ba0..7b1e85472 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/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)) { - showMessage(SSHAgent::instance()->errorString(), MessageWidget::Error); + if (!sshAgent()->addIdentity(key, settings)) { + showMessage(sshAgent()->errorString(), MessageWidget::Error); return; } } @@ -714,8 +713,8 @@ void EditEntryWidget::removeKeyFromAgent() return; } - if (!SSHAgent::instance()->removeIdentity(key)) { - showMessage(SSHAgent::instance()->errorString(), MessageWidget::Error); + if (!sshAgent()->removeIdentity(key)) { + 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 diff --git a/src/gui/entry/EditEntryWidget.h b/src/gui/entry/EditEntryWidget.h index 783bb3cb9..dafc4a283 100644 --- a/src/gui/entry/EditEntryWidget.h +++ b/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 m_mainUi; diff --git a/src/sshagent/AgentSettingsWidget.cpp b/src/sshagent/AgentSettingsWidget.cpp index f95a19845..e06929195 100644 --- a/src/sshagent/AgentSettingsWidget.cpp +++ b/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(sshAgentEnabled); + m_ui->sshAuthSockMessageWidget->setVisible(sshAgent()->isEnabled()); 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 sshAuthSockOverride = config()->get("SSHAuthSockOverride", "").toString(); + auto sshAuthSock = sshAgent()->socketPath(false); + 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(true); + m_ui->sshAuthSockMessageWidget->setVisible(sshAgentEnabled); + 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()); - config()->set("SSHAuthSockOverride", m_ui->sshAuthSockOverrideEdit->text()); + auto 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()); } diff --git a/src/sshagent/AgentSettingsWidget.ui b/src/sshagent/AgentSettingsWidget.ui index 20142f1c9..3b8c70fad 100644 --- a/src/sshagent/AgentSettingsWidget.ui +++ b/src/sshagent/AgentSettingsWidget.ui @@ -26,7 +26,7 @@ - Enable SSH Agent (requires restart) + Enable SSH Agent integration diff --git a/src/sshagent/SSHAgent.cpp b/src/sshagent/SSHAgent.cpp index 571f7b99f..a6aedca37 100644 --- a/src/sshagent/SSHAgent.cpp +++ b/src/sshagent/SSHAgent.cpp @@ -29,46 +29,72 @@ #include #endif -SSHAgent* SSHAgent::m_instance; - -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 -} +Q_GLOBAL_STATIC(SSHAgent, s_sshAgent); SSHAgent::~SSHAgent() { - 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); - } + removeAllIdentities(); } SSHAgent* SSHAgent::instance() { - if (!m_instance) { - qFatal("Race condition: instance wanted before it was initialized, this is a bug."); - } - - return m_instance; + return s_sshAgent; } -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. diff --git a/src/sshagent/SSHAgent.h b/src/sshagent/SSHAgent.h index 92389112f..9c76e8e6a 100644 --- a/src/sshagent/SSHAgent.h +++ b/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