From b0a61f437a54d31aed580c66230d43832a44c623 Mon Sep 17 00:00:00 2001 From: Toni Spets Date: Thu, 1 Mar 2018 18:27:53 +0200 Subject: [PATCH] SSH Agent: Fix handling of encrypted RSA keys Also fix multiple UI issues caused by said keys. Fixes #1560 --- src/gui/entry/EditEntryWidget.cpp | 60 +++++++++++++++++++------------ src/gui/entry/EditEntryWidget.h | 2 +- src/sshagent/OpenSSHKey.cpp | 10 +++++- src/sshagent/SSHAgent.cpp | 6 +++- 4 files changed, 52 insertions(+), 26 deletions(-) diff --git a/src/gui/entry/EditEntryWidget.cpp b/src/gui/entry/EditEntryWidget.cpp index 3b70e4453..8d931bcfb 100644 --- a/src/gui/entry/EditEntryWidget.cpp +++ b/src/gui/entry/EditEntryWidget.cpp @@ -346,24 +346,32 @@ void EditEntryWidget::updateSSHAgentKeyInfo() return; } - m_sshAgentUi->fingerprintTextLabel->setText(key.fingerprint()); - - if (key.encrypted()) { - m_sshAgentUi->commentTextLabel->setText(tr("(encrypted)")); - m_sshAgentUi->decryptButton->setEnabled(true); + if (!key.fingerprint().isEmpty()) { + m_sshAgentUi->fingerprintTextLabel->setText(key.fingerprint()); } else { - m_sshAgentUi->commentTextLabel->setText(key.comment()); + m_sshAgentUi->fingerprintTextLabel->setText(tr("(encrypted)")); } - m_sshAgentUi->publicKeyEdit->document()->setPlainText(key.publicKey()); + if (!key.comment().isEmpty() || !key.encrypted()) { + m_sshAgentUi->commentTextLabel->setText(key.comment()); + } else { + m_sshAgentUi->commentTextLabel->setText(tr("(encrypted)")); + m_sshAgentUi->decryptButton->setEnabled(true); + } + + if (!key.publicKey().isEmpty()) { + m_sshAgentUi->publicKeyEdit->document()->setPlainText(key.publicKey()); + m_sshAgentUi->copyToClipboardButton->setEnabled(true); + } else { + m_sshAgentUi->publicKeyEdit->document()->setPlainText(tr("(encrypted)")); + m_sshAgentUi->copyToClipboardButton->setDisabled(true); + } // enable agent buttons only if we have an agent running if (SSHAgent::instance()->isAgentRunning()) { m_sshAgentUi->addToAgentButton->setEnabled(true); m_sshAgentUi->removeFromAgentButton->setEnabled(true); } - - m_sshAgentUi->copyToClipboardButton->setEnabled(true); } void EditEntryWidget::saveSSHAgentConfig() @@ -410,7 +418,7 @@ void EditEntryWidget::browsePrivateKey() } } -bool EditEntryWidget::getOpenSSHKey(OpenSSHKey& key) +bool EditEntryWidget::getOpenSSHKey(OpenSSHKey& key, bool decrypt) { QByteArray privateKeyData; @@ -436,7 +444,7 @@ bool EditEntryWidget::getOpenSSHKey(OpenSSHKey& key) privateKeyData = localFile.readAll(); } - if (privateKeyData.length() == 0) { + if (privateKeyData.isEmpty()) { return false; } @@ -445,6 +453,13 @@ bool EditEntryWidget::getOpenSSHKey(OpenSSHKey& key) return false; } + if (key.encrypted() && (decrypt || key.publicKey().isEmpty())) { + if (!key.openPrivateKey(m_entry->password())) { + showMessage(key.errorString(), MessageWidget::Error); + return false; + } + } + if (key.comment().isEmpty()) { key.setComment(m_entry->username()); } @@ -456,16 +471,12 @@ void EditEntryWidget::addKeyToAgent() { OpenSSHKey key; - if (!getOpenSSHKey(key)) { + if (!getOpenSSHKey(key, true)) { return; } - if (!key.openPrivateKey(m_entry->password())) { - showMessage(key.errorString(), MessageWidget::Error); - } else { - m_sshAgentUi->commentTextLabel->setText(key.comment()); - m_sshAgentUi->publicKeyEdit->document()->setPlainText(key.publicKey()); - } + m_sshAgentUi->commentTextLabel->setText(key.comment()); + m_sshAgentUi->publicKeyEdit->document()->setPlainText(key.publicKey()); quint32 lifetime = 0; bool confirm = m_sshAgentUi->requireUserConfirmationCheckBox->isChecked(); @@ -494,16 +505,19 @@ void EditEntryWidget::decryptPrivateKey() { OpenSSHKey key; - if (!getOpenSSHKey(key)) { + if (!getOpenSSHKey(key, true)) { return; } - if (!key.openPrivateKey(m_entry->password())) { - showMessage(key.errorString(), MessageWidget::Error); - } else { + if (!key.comment().isEmpty()) { m_sshAgentUi->commentTextLabel->setText(key.comment()); - m_sshAgentUi->publicKeyEdit->document()->setPlainText(key.publicKey()); + } else { + m_sshAgentUi->commentTextLabel->setText(tr("n/a")); } + + m_sshAgentUi->fingerprintTextLabel->setText(key.fingerprint()); + m_sshAgentUi->publicKeyEdit->document()->setPlainText(key.publicKey()); + m_sshAgentUi->copyToClipboardButton->setEnabled(true); } void EditEntryWidget::copyPublicKey() diff --git a/src/gui/entry/EditEntryWidget.h b/src/gui/entry/EditEntryWidget.h index 66d89dbfb..a7c8e3271 100644 --- a/src/gui/entry/EditEntryWidget.h +++ b/src/gui/entry/EditEntryWidget.h @@ -128,7 +128,7 @@ private: QMenu* createPresetsMenu(); void updateEntryData(Entry* entry) const; #ifdef WITH_XC_SSHAGENT - bool getOpenSSHKey(OpenSSHKey& key); + bool getOpenSSHKey(OpenSSHKey& key, bool decrypt = false); void saveSSHAgentConfig(); #endif diff --git a/src/sshagent/OpenSSHKey.cpp b/src/sshagent/OpenSSHKey.cpp index ce867a95f..ccc7606f0 100644 --- a/src/sshagent/OpenSSHKey.cpp +++ b/src/sshagent/OpenSSHKey.cpp @@ -94,6 +94,10 @@ int OpenSSHKey::keyLength() const const QString OpenSSHKey::fingerprint() const { + if (m_publicData.isEmpty()) { + return {}; + } + QByteArray publicKey; BinaryStream stream(&publicKey); @@ -115,6 +119,10 @@ const QString OpenSSHKey::comment() const const QString OpenSSHKey::publicKey() const { + if (m_publicData.isEmpty()) { + return {}; + } + QByteArray publicKey; BinaryStream stream(&publicKey); @@ -326,7 +334,7 @@ bool OpenSSHKey::openPrivateKey(const QString& passphrase) return false; } - if (passphrase.length() == 0) { + if (passphrase.isEmpty()) { m_error = tr("Passphrase is required to decrypt this key"); return false; } diff --git a/src/sshagent/SSHAgent.cpp b/src/sshagent/SSHAgent.cpp index a65c6e4f1..32ccf816d 100644 --- a/src/sshagent/SSHAgent.cpp +++ b/src/sshagent/SSHAgent.cpp @@ -260,6 +260,10 @@ void SSHAgent::databaseModeChanged(DatabaseWidget::Mode mode) continue; } + if (!key.openPrivateKey(e->password())) { + continue; + } + if (key.comment().isEmpty()) { key.setComment(e->username()); } @@ -268,7 +272,7 @@ void SSHAgent::databaseModeChanged(DatabaseWidget::Mode mode) removeIdentityAtLock(key, uuid); } - if (settings.addAtDatabaseOpen() && key.openPrivateKey(e->password())) { + if (settings.addAtDatabaseOpen()) { int lifetime = 0; if (settings.useLifetimeConstraintWhenAdding()) {