Skip to content

Commit

Permalink
SSH Agent: Show error messages if something fails
Browse files Browse the repository at this point in the history
  • Loading branch information
@hifi @phoerious
hifi authored and phoerious committed Mar 6, 2018
1 parent 4ce0005 commit 0847589
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 13 deletions.
6 changes: 6 additions & 0 deletions src/gui/MainWindow.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions src/gui/MainWindow.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
14 changes: 11 additions & 3 deletions src/gui/entry/EditEntryWidget.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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;
}
}

Expand Down
50 changes: 43 additions & 7 deletions src/sshagent/SSHAgent.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -67,21 +72,23 @@ 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;
BinaryStream stream(&socket);

socket.connectToServer(m_socketPath);
if (!socket.waitForConnected(500)) {
m_error = tr("Agent connection failed.");
return false;
}

stream.writeString(in);
stream.flush();

if (!stream.readString(out)) {
m_error = tr("Agent protocol error.");
return false;
}

Expand All @@ -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<quint32>(in.length()) > AGENT_MAX_MSGLEN - 4) {
m_error = tr("Agent connection failed.");
return false;
}

Expand All @@ -104,13 +113,15 @@ 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;
}

LPVOID ptr = MapViewOfFile(handle, FILE_MAP_WRITE, 0, 0, 0);

if (!ptr) {
CloseHandle(handle);
m_error = tr("Agent connection failed.");
return false;
}

Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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<quint8>(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);

Expand All @@ -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<quint8>(responseData[0]) != SSH_AGENT_SUCCESS) {
m_error = tr("Agent does not have this identity.");
return false;
}

Expand All @@ -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<OpenSSHKey> 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()) {
Expand Down Expand Up @@ -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);
}
}
}
}
Expand Down
11 changes: 8 additions & 3 deletions src/sshagent/SSHAgent.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,15 @@ class SSHAgent : public QObject
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);

Expand All @@ -56,7 +60,7 @@ public slots:
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;

Expand All @@ -68,6 +72,7 @@ public slots:
#endif

QMap<QString, QSet<OpenSSHKey>> m_keys;
QString m_error;
};

#endif // AGENTCLIENT_H

0 comments on commit 0847589

Please sign in to comment.