From 084758908aa4864a956c6cf4e1638ad42211d067 Mon Sep 17 00:00:00 2001 From: Toni Spets Date: Sat, 3 Mar 2018 17:49:00 +0200 Subject: [PATCH] SSH Agent: Show error messages if something fails --- src/gui/MainWindow.cpp | 6 ++++ src/gui/MainWindow.h | 1 + src/gui/entry/EditEntryWidget.cpp | 14 +++++++-- src/sshagent/SSHAgent.cpp | 50 ++++++++++++++++++++++++++----- src/sshagent/SSHAgent.h | 11 +++++-- 5 files changed, 69 insertions(+), 13 deletions(-) diff --git a/src/gui/MainWindow.cpp b/src/gui/MainWindow.cpp index 7f732faca..ba69c0173 100644 --- a/src/gui/MainWindow.cpp +++ b/src/gui/MainWindow.cpp @@ -198,6 +198,7 @@ MainWindow::MainWindow() #endif #ifdef WITH_XC_SSHAGENT SSHAgent::init(this); + connect(SSHAgent::instance(), SIGNAL(error(QString)), this, SLOT(showErrorMessage(QString))); m_ui->settingsWidget->addSettingsPage(new AgentSettingsPage(m_ui->tabWidget)); #endif @@ -454,6 +455,11 @@ void MainWindow::showKeePassHTTPDeprecationNotice() disconnect(m_ui->globalMessageWidget, SIGNAL(hideAnimationFinished()), this, SLOT(showKeePassHTTPDeprecationNotice())); } +void MainWindow::showErrorMessage(const QString& message) +{ + m_ui->globalMessageWidget->showMessage(message, MessageWidget::Error); +} + void MainWindow::appExit() { m_appExitCalled = true; diff --git a/src/gui/MainWindow.h b/src/gui/MainWindow.h index fd31fab0b..20e548910 100644 --- a/src/gui/MainWindow.h +++ b/src/gui/MainWindow.h @@ -104,6 +104,7 @@ private slots: void hideTabMessage(); void handleScreenLock(); void showKeePassHTTPDeprecationNotice(); + void showErrorMessage(const QString& message); private: static void setShortcut(QAction* action, QKeySequence::StandardKey standard, int fallback = 0); diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index 8d931bcfb..4d12dd16b 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -485,7 +485,10 @@ void EditEntryWidget::addKeyToAgent() lifetime = m_sshAgentUi->lifetimeSpinBox->value(); } - SSHAgent::instance()->addIdentity(key, lifetime, confirm); + if (!SSHAgent::instance()->addIdentity(key, lifetime, confirm)) { + showMessage(SSHAgent::instance()->errorString(), MessageWidget::Error); + return; + } if (m_sshAgentUi->removeKeyFromAgentCheckBox->isChecked()) { SSHAgent::instance()->removeIdentityAtLock(key, m_entry->uuid()); @@ -496,8 +499,13 @@ void EditEntryWidget::removeKeyFromAgent() { OpenSSHKey key; - if (getOpenSSHKey(key)) { - SSHAgent::instance()->removeIdentity(key); + if (!getOpenSSHKey(key)) { + return; + } + + if (!SSHAgent::instance()->removeIdentity(key)) { + showMessage(SSHAgent::instance()->errorString(), MessageWidget::Error); + return; } } diff --git a/src/sshagent/SSHAgent.cpp b/src/sshagent/SSHAgent.cpp index 32ccf816d..45d774aab 100644 --- a/src/sshagent/SSHAgent.cpp +++ b/src/sshagent/SSHAgent.cpp @@ -58,6 +58,11 @@ void SSHAgent::init(QObject* parent) m_instance = new SSHAgent(parent); } +const QString SSHAgent::errorString() const +{ + return m_error; +} + bool SSHAgent::isAgentRunning() const { #ifndef Q_OS_WIN @@ -67,7 +72,7 @@ bool SSHAgent::isAgentRunning() const #endif } -bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out) const +bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out) { #ifndef Q_OS_WIN QLocalSocket socket; @@ -75,6 +80,7 @@ bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out) const socket.connectToServer(m_socketPath); if (!socket.waitForConnected(500)) { + m_error = tr("Agent connection failed."); return false; } @@ -82,6 +88,7 @@ bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out) const stream.flush(); if (!stream.readString(out)) { + m_error = tr("Agent protocol error."); return false; } @@ -92,10 +99,12 @@ bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out) const HWND hWnd = FindWindowA("Pageant", "Pageant"); if (!hWnd) { + m_error = tr("Agent connection failed."); return false; } if (static_cast(in.length()) > AGENT_MAX_MSGLEN - 4) { + m_error = tr("Agent connection failed."); return false; } @@ -104,6 +113,7 @@ bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out) const HANDLE handle = CreateFileMappingA(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, AGENT_MAX_MSGLEN, mapName.data()); if (!handle) { + m_error = tr("Agent connection failed."); return false; } @@ -111,6 +121,7 @@ bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out) const if (!ptr) { CloseHandle(handle); + m_error = tr("Agent connection failed."); return false; } @@ -132,7 +143,11 @@ bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out) const if (responseLength <= AGENT_MAX_MSGLEN) { out.resize(responseLength); memcpy(out.data(), requestData, responseLength); + } else { + m_error = tr("Agent protocol error."); } + } else { + m_error = tr("Agent protocol error."); } UnmapViewOfFile(ptr); @@ -143,8 +158,13 @@ bool SSHAgent::sendMessage(const QByteArray& in, QByteArray& out) const } -bool SSHAgent::addIdentity(OpenSSHKey& key, quint32 lifetime, bool confirm) const +bool SSHAgent::addIdentity(OpenSSHKey& key, quint32 lifetime, bool confirm) { + if (!isAgentRunning()) { + m_error = tr("No agent running, cannot add identity."); + return false; + } + QByteArray requestData; BinaryStream request(&requestData); @@ -161,17 +181,25 @@ bool SSHAgent::addIdentity(OpenSSHKey& key, quint32 lifetime, bool confirm) cons } QByteArray responseData; - sendMessage(requestData, responseData); + if (!sendMessage(requestData, responseData)) { + return false; + } if (responseData.length() < 1 || static_cast(responseData[0]) != SSH_AGENT_SUCCESS) { + m_error = tr("Agent refused this identity."); return false; } return true; } -bool SSHAgent::removeIdentity(OpenSSHKey& key) const +bool SSHAgent::removeIdentity(OpenSSHKey& key) { + if (!isAgentRunning()) { + m_error = tr("No agent running, cannot remove identity."); + return false; + } + QByteArray requestData; BinaryStream request(&requestData); @@ -183,9 +211,12 @@ bool SSHAgent::removeIdentity(OpenSSHKey& key) const request.writeString(keyData); QByteArray responseData; - sendMessage(requestData, responseData); + if (!sendMessage(requestData, responseData)) { + return false; + } if (responseData.length() < 1 || static_cast(responseData[0]) != SSH_AGENT_SUCCESS) { + m_error = tr("Agent does not have this identity."); return false; } @@ -210,9 +241,12 @@ void SSHAgent::databaseModeChanged(DatabaseWidget::Mode mode) Uuid uuid = widget->database()->uuid(); if (mode == DatabaseWidget::LockedMode && m_keys.contains(uuid.toHex())) { + QSet keys = m_keys.take(uuid.toHex()); for (OpenSSHKey key : keys) { - removeIdentity(key); + if (!removeIdentity(key)) { + emit error(m_error); + } } } else if (mode == DatabaseWidget::ViewMode && !m_keys.contains(uuid.toHex())) { for (Entry* e : widget->database()->rootGroup()->entriesRecursive()) { @@ -279,7 +313,9 @@ void SSHAgent::databaseModeChanged(DatabaseWidget::Mode mode) lifetime = settings.lifetimeConstraintDuration(); } - addIdentity(key, lifetime, settings.useConfirmConstraintWhenAdding()); + if (!addIdentity(key, lifetime, settings.useConfirmConstraintWhenAdding())) { + emit error(m_error); + } } } } diff --git a/src/sshagent/SSHAgent.h b/src/sshagent/SSHAgent.h index 078ff7b0d..adb08cad6 100644 --- a/src/sshagent/SSHAgent.h +++ b/src/sshagent/SSHAgent.h @@ -33,11 +33,15 @@ public: static SSHAgent* instance(); static void init(QObject* parent); + const QString errorString() const; bool isAgentRunning() const; - bool addIdentity(OpenSSHKey& key, quint32 lifetime = 0, bool confirm = false) const; - bool removeIdentity(OpenSSHKey& key) const; + bool addIdentity(OpenSSHKey& key, quint32 lifetime = 0, bool confirm = false); + bool removeIdentity(OpenSSHKey& key); void removeIdentityAtLock(const OpenSSHKey& key, const Uuid& uuid); +signals: + void error(const QString& message); + public slots: void databaseModeChanged(DatabaseWidget::Mode mode = DatabaseWidget::LockedMode); @@ -56,7 +60,7 @@ private: explicit SSHAgent(QObject* parent = nullptr); ~SSHAgent(); - bool sendMessage(const QByteArray& in, QByteArray& out) const; + bool sendMessage(const QByteArray& in, QByteArray& out); static SSHAgent* m_instance; @@ -68,6 +72,7 @@ private: #endif QMap> m_keys; + QString m_error; }; #endif // AGENTCLIENT_H