SSH Agent: Integration tests against ssh-agent

Windows testing is currently explicitly disabled due to too many different scenarios to run an agent and MSYS2 having its own.

src/sshagent/AgentSettingsWidget.cpp

@@ -68,7 +68,8 @@ void AgentSettingsWidget::loadSettings()
             return;
         }
 #endif
-        if (sshAgent()->testConnection()) {
+        QList<QSharedPointer<OpenSSHKey>> keys;
+        if (sshAgent()->listIdentities(keys)) {
             m_ui->sshAuthSockMessageWidget->showMessage(tr("SSH Agent connection is working!"),
                                                         MessageWidget::Positive);
         } else {

src/sshagent/SSHAgent.cpp

@@ -212,36 +212,6 @@ bool SSHAgent::sendMessagePageant(const QByteArray& in, QByteArray& out)
 }
 #endif
 
-/**
- * Test if connection to SSH agent is working.
- *
- * @return true on success
- */
-bool SSHAgent::testConnection()
-{
-    if (!isAgentRunning()) {
-        m_error = tr("No agent running, cannot test connection.");
-        return false;
-    }
-
-    QByteArray requestData;
-    BinaryStream request(&requestData);
-
-    request.write(SSH_AGENTC_REQUEST_IDENTITIES);
-
-    QByteArray responseData;
-    if (!sendMessage(requestData, responseData)) {
-        return false;
-    }
-
-    if (responseData.length() < 1 || static_cast<quint8>(responseData[0]) != SSH_AGENT_IDENTITIES_ANSWER) {
-        m_error = tr("Agent protocol error.");
-        return false;
-    }
-
-    return true;
-}
-
 /**
  * Add the identity to the SSH agent.
  *
@@ -328,6 +298,99 @@ bool SSHAgent::removeIdentity(OpenSSHKey& key)
     return sendMessage(requestData, responseData);
 }
 
+/**
+ * Get a list of identities from the SSH agent.
+ *
+ * @param list list of keys to append
+ * @return true on success
+ */
+bool SSHAgent::listIdentities(QList<QSharedPointer<OpenSSHKey>>& list)
+{
+    if (!isAgentRunning()) {
+        m_error = tr("No agent running, cannot list identities.");
+        return false;
+    }
+
+    QByteArray requestData;
+    BinaryStream request(&requestData);
+
+    request.write(SSH_AGENTC_REQUEST_IDENTITIES);
+
+    QByteArray responseData;
+    if (!sendMessage(requestData, responseData)) {
+        return false;
+    }
+
+    BinaryStream response(&responseData);
+
+    quint8 responseType;
+    if (!response.read(responseType) || responseType != SSH_AGENT_IDENTITIES_ANSWER) {
+        m_error = tr("Agent protocol error.");
+        return false;
+    }
+
+    quint32 nKeys;
+    if (!response.read(nKeys)) {
+        m_error = tr("Agent protocol error.");
+        return false;
+    }
+
+    for (quint32 i = 0; i < nKeys; i++) {
+        QByteArray publicData;
+        QString comment;
+
+        if (!response.readString(publicData)) {
+            m_error = tr("Agent protocol error.");
+            return false;
+        }
+
+        if (!response.readString(comment)) {
+            m_error = tr("Agent protocol error.");
+            return false;
+        }
+
+        OpenSSHKey* key = new OpenSSHKey();
+        key->setComment(comment);
+
+        list.append(QSharedPointer<OpenSSHKey>(key));
+
+        BinaryStream publicDataStream(&publicData);
+        if (!key->readPublic(publicDataStream)) {
+            m_error = key->errorString();
+            return false;
+        }
+    }
+
+    return true;
+}
+
+/**
+ * Check if this identity is loaded in the SSH Agent.
+ *
+ * @param key identity to remove
+ * @param loaded is the key laoded
+ * @return true on success
+ */
+bool SSHAgent::checkIdentity(OpenSSHKey& key, bool& loaded)
+{
+    QList<QSharedPointer<OpenSSHKey>> list;
+
+    if (!listIdentities(list)) {
+        return false;
+    }
+
+    loaded = false;
+
+    for (const auto it : list) {
+        if (*it == key) {
+            loaded = true;
+            break;
+        }
+    }
+
+    return true;
+}
+
 /**
  * Remove all identities known to this instance
  */

src/sshagent/SSHAgent.h

@@ -47,8 +47,9 @@ public:
 
     const QString errorString() const;
     bool isAgentRunning() const;
-    bool testConnection();
     bool addIdentity(OpenSSHKey& key, KeeAgentSettings& settings);
+    bool listIdentities(QList<QSharedPointer<OpenSSHKey>>& list);
+    bool checkIdentity(OpenSSHKey& key, bool& loaded);
     bool removeIdentity(OpenSSHKey& key);
     void removeAllIdentities();
     void setAutoRemoveOnLock(const OpenSSHKey& key, bool autoRemove);

tests/CMakeLists.txt

@@ -165,6 +165,10 @@ endif()
 if(WITH_XC_CRYPTO_SSH)
     add_unit_test(NAME testopensshkey SOURCES TestOpenSSHKey.cpp
             LIBS ${TEST_LIBRARIES})
+    if(NOT WIN32)
+        add_unit_test(NAME testsshagent SOURCES TestSSHAgent.cpp
+                LIBS ${TEST_LIBRARIES})
+    endif()
 endif()
 
 add_unit_test(NAME testentry SOURCES TestEntry.cpp

tests/TestSSHAgent.cpp

@@ -0,0 +1,214 @@
+/*
+ *  Copyright (C) 2020 KeePassXC Team <team@keepassxc.org>
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 2 or (at your option)
+ *  version 3 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "TestSSHAgent.h"
+#include "TestGlobal.h"
+#include "core/Config.h"
+#include "crypto/Crypto.h"
+#include "sshagent/SSHAgent.h"
+
+QTEST_GUILESS_MAIN(TestSSHAgent)
+
+void TestSSHAgent::initTestCase()
+{
+    QVERIFY(Crypto::init());
+    Config::createTempFileInstance();
+
+    m_agentSocketFile.setAutoRemove(true);
+    QVERIFY(m_agentSocketFile.open());
+
+    m_agentSocketFileName = m_agentSocketFile.fileName();
+    QVERIFY(!m_agentSocketFileName.isEmpty());
+
+    // let ssh-agent re-create it as a socket
+    QVERIFY(m_agentSocketFile.remove());
+
+    QStringList arguments;
+    arguments << "-D"
+              << "-a" << m_agentSocketFileName;
+
+    QElapsedTimer timer;
+    timer.start();
+
+    qDebug() << "ssh-agent starting with arguments" << arguments;
+    m_agentProcess.setProcessChannelMode(QProcess::ForwardedChannels);
+    m_agentProcess.start("ssh-agent", arguments);
+    m_agentProcess.closeWriteChannel();
+
+    if (!m_agentProcess.waitForStarted()) {
+        QSKIP("ssh-agent could not be started");
+    }
+
+    qDebug() << "ssh-agent started as pid" << m_agentProcess.pid();
+
+    // we need to wait for the agent to open the socket before going into real tests
+    QFileInfo socketFileInfo(m_agentSocketFileName);
+    while (!timer.hasExpired(2000)) {
+        if (socketFileInfo.exists()) {
+            break;
+        }
+        QTest::qWait(10);
+    }
+
+    QVERIFY(socketFileInfo.exists());
+    qDebug() << "ssh-agent initialized in" << timer.elapsed() << "ms";
+
+    // initialize test key
+    const QString keyString = QString("-----BEGIN OPENSSH PRIVATE KEY-----\n"
+                                      "b3BlbnNzaC1rZXktdjEAAAAABG5vbmUAAAAEbm9uZQAAAAAAAAABAAAAMwAAAAtzc2gtZW\n"
+                                      "QyNTUxOQAAACDdlO5F2kF2WzedrBAHBi9wBHeISzXZ0IuIqrp0EzeazAAAAKjgCfj94An4\n"
+                                      "/QAAAAtzc2gtZWQyNTUxOQAAACDdlO5F2kF2WzedrBAHBi9wBHeISzXZ0IuIqrp0EzeazA\n"
+                                      "AAAEBe1iilZFho8ZGAliiSj5URvFtGrgvmnEKdiLZow5hOR92U7kXaQXZbN52sEAcGL3AE\n"
+                                      "d4hLNdnQi4iqunQTN5rMAAAAH29wZW5zc2hrZXktdGVzdC1wYXJzZUBrZWVwYXNzeGMBAg\n"
+                                      "MEBQY=\n"
+                                      "-----END OPENSSH PRIVATE KEY-----\n");
+
+    const QByteArray keyData = keyString.toLatin1();
+
+    QVERIFY(m_key.parsePKCS1PEM(keyData));
+}
+
+void TestSSHAgent::testConfiguration()
+{
+    SSHAgent agent;
+
+    // default config must not enable agent
+    QVERIFY(!agent.isEnabled());
+
+    agent.setEnabled(true);
+    QVERIFY(agent.isEnabled());
+
+    // this will either be an empty string or the real ssh-agent socket path, doesn't matter
+    QString defaultSocketPath = agent.socketPath(false);
+
+    // overridden path must match default before setting an override
+    QCOMPARE(agent.socketPath(true), defaultSocketPath);
+
+    agent.setAuthSockOverride(m_agentSocketFileName);
+
+    // overridden path must match what we set
+    QCOMPARE(agent.socketPath(true), m_agentSocketFileName);
+
+    // non-overridden path must match the default
+    QCOMPARE(agent.socketPath(false), defaultSocketPath);
+}
+
+void TestSSHAgent::testIdentity()
+{
+    SSHAgent agent;
+    agent.setEnabled(true);
+    agent.setAuthSockOverride(m_agentSocketFileName);
+
+    QVERIFY(agent.isAgentRunning());
+
+    KeeAgentSettings settings;
+    bool keyInAgent;
+
+    // test adding a key works
+    QVERIFY(agent.addIdentity(m_key, settings));
+    QVERIFY(agent.checkIdentity(m_key, keyInAgent) && keyInAgent);
+
+    // test removing a key works
+    QVERIFY(agent.removeIdentity(m_key));
+    QVERIFY(agent.checkIdentity(m_key, keyInAgent) && !keyInAgent);
+}
+
+void TestSSHAgent::testRemoveOnClose()
+{
+    SSHAgent agent;
+    agent.setEnabled(true);
+    agent.setAuthSockOverride(m_agentSocketFileName);
+
+    QVERIFY(agent.isAgentRunning());
+
+    KeeAgentSettings settings;
+    bool keyInAgent;
+
+    settings.setRemoveAtDatabaseClose(true);
+    QVERIFY(agent.addIdentity(m_key, settings));
+    QVERIFY(agent.checkIdentity(m_key, keyInAgent) && keyInAgent);
+    agent.setEnabled(false);
+    QVERIFY(agent.checkIdentity(m_key, keyInAgent) && !keyInAgent);
+}
+
+void TestSSHAgent::testLifetimeConstraint()
+{
+    SSHAgent agent;
+    agent.setEnabled(true);
+    agent.setAuthSockOverride(m_agentSocketFileName);
+
+    QVERIFY(agent.isAgentRunning());
+
+    KeeAgentSettings settings;
+    bool keyInAgent;
+
+    settings.setUseLifetimeConstraintWhenAdding(true);
+    settings.setLifetimeConstraintDuration(2); // two seconds
+
+    // identity should be in agent immediately after adding
+    QVERIFY(agent.addIdentity(m_key, settings));
+    QVERIFY(agent.checkIdentity(m_key, keyInAgent) && keyInAgent);
+
+    QElapsedTimer timer;
+    timer.start();
+
+    // wait for the identity to time out
+    while (!timer.hasExpired(5000)) {
+        QVERIFY(agent.checkIdentity(m_key, keyInAgent));
+
+        if (!keyInAgent) {
+            break;
+        }
+
+        QTest::qWait(100);
+    }
+
+    QVERIFY(!keyInAgent);
+}
+
+void TestSSHAgent::testConfirmConstraint()
+{
+    SSHAgent agent;
+    agent.setEnabled(true);
+    agent.setAuthSockOverride(m_agentSocketFileName);
+
+    QVERIFY(agent.isAgentRunning());
+
+    KeeAgentSettings settings;
+    bool keyInAgent;
+
+    settings.setUseConfirmConstraintWhenAdding(true);
+
+    QVERIFY(agent.addIdentity(m_key, settings));
+
+    // we can't test confirmation itself is working but we can test the agent accepts the key
+    QVERIFY(agent.checkIdentity(m_key, keyInAgent) && keyInAgent);
+
+    QVERIFY(agent.removeIdentity(m_key));
+    QVERIFY(agent.checkIdentity(m_key, keyInAgent) && !keyInAgent);
+}
+
+void TestSSHAgent::cleanupTestCase()
+{
+    if (m_agentProcess.state() != QProcess::NotRunning) {
+        qDebug() << "Killing ssh-agent pid" << m_agentProcess.pid();
+        m_agentProcess.terminate();
+        m_agentProcess.waitForFinished();
+    }
+
+    m_agentSocketFile.remove();
+}

tests/TestSSHAgent.h

@@ -0,0 +1,46 @@
+/*
+ *  Copyright (C) 2020 KeePassXC Team <team@keepassxc.org>
+ *
+ *  This program is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 2 or (at your option)
+ *  version 3 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef TESTSSHAGENT_H
+#define TESTSSHAGENT_H
+
+#include "crypto/ssh/OpenSSHKey.h"
+#include <QObject>
+#include <QProcess>
+#include <QTemporaryFile>
+
+class TestSSHAgent : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void initTestCase();
+    void testConfiguration();
+    void testIdentity();
+    void testRemoveOnClose();
+    void testLifetimeConstraint();
+    void testConfirmConstraint();
+    void cleanupTestCase();
+
+private:
+    QTemporaryFile m_agentSocketFile;
+    QString m_agentSocketFileName;
+    QProcess m_agentProcess;
+    OpenSSHKey m_key;
+};
+
+#endif // TESTSSHAGENT_H