SSH: Fix key re-exchange bug.

We used the encrypted client payload by mistake. The bug only manifests
itself from the second key exchange on, because during the first one,
no encryption is active, so the "encrypted" payload is the same as
the non-encrypted one.

7 files changed, 42 insertions, 18 deletions

diff --git a/src/libs/utils/ssh/sshkeyexchange.cpp b/src/libs/utils/ssh/sshkeyexchange.cpp
index 1112cd2b97..1165569ea0 100644
--- a/src/libs/utils/ssh/sshkeyexchange.cpp
+++ b/src/libs/utils/ssh/sshkeyexchange.cpp
@@ -44,6 +44,9 @@
 #include <botan/pubkey.h>
 #include <botan/rsa.h>
 
+#ifdef CREATOR_SSH_DEBUG
+#include <iostream>
+#endif
 #include <string>
 
 using namespace Botan;
@@ -65,6 +68,18 @@ namespace {
         Q_UNUSED(list);
 #endif
     }
+
+#ifdef CREATOR_SSH_DEBUG
+    void printData(const char *name, const QByteArray &data)
+    {
+        std::cerr << std::hex;
+        qDebug("The client thinks the %s has length %d and is:", name, data.count());
+        for (int i = 0; i < data.count(); ++i)
+            std::cerr << (static_cast<unsigned int>(data.at(i)) & 0xff) << ' ';
+        std::cerr << std::endl;
+    }
+#endif
+
 } // anonymous namespace
 
 SshKeyExchange::SshKeyExchange(SshSendFacility &sendFacility)
@@ -77,9 +92,7 @@ SshKeyExchange::~SshKeyExchange() {}
 void SshKeyExchange::sendKexInitPacket(const QByteArray &serverId)
 {
     m_serverId = serverId;
-    const AbstractSshPacket::Payload &payload
-        = m_sendFacility.sendKeyExchangeInitPacket();
-    m_clientKexInitPayload = QByteArray(payload.data, payload.size);
+    m_clientKexInitPayload = m_sendFacility.sendKeyExchangeInitPacket();
 }
 
 bool SshKeyExchange::sendDhInitPacket(const SshIncomingPacket &serverKexInit)
@@ -132,8 +145,7 @@ bool SshKeyExchange::sendDhInitPacket(const SshIncomingPacket &serverKexInit)
     m_dhKey.reset(new DH_PrivateKey(rng,
         DL_Group(botanKeyExchangeAlgoName(keyAlgo))));
 
-    const AbstractSshPacket::Payload &payload = serverKexInit.payLoad();
-    m_serverKexInitPayload = QByteArray(payload.data, payload.size);
+    m_serverKexInitPayload = serverKexInit.payLoad();
     m_sendFacility.sendKeyDhInitPacket(m_dhKey->get_y());
     return kexInitParams.firstKexPacketFollows;
 }
@@ -165,6 +177,19 @@ void SshKeyExchange::sendNewKeysPacket(const SshIncomingPacket &dhReply,
                         concatenatedData.size());
     m_h = convertByteArray(hashResult);
 
+#ifdef CREATOR_SSH_DEBUG
+    printData("Client Id", AbstractSshPacket::encodeString(clientId));
+    printData("Server Id", AbstractSshPacket::encodeString(m_serverId));
+    printData("Client Payload", AbstractSshPacket::encodeString(m_clientKexInitPayload));
+    printData("Server payload", AbstractSshPacket::encodeString(m_serverKexInitPayload));
+    printData("K_S", reply.k_s);
+    printData("y", AbstractSshPacket::encodeMpInt(m_dhKey->get_y()));
+    printData("f", AbstractSshPacket::encodeMpInt(reply.f));
+    printData("K", m_k);
+    printData("Concatenated data", concatenatedData);
+    printData("H", m_h);
+#endif // CREATOR_SSH_DEBUG
+
     QScopedPointer<Public_Key> sigKey;
     QScopedPointer<PK_Verifier> verifier;
     if (m_serverHostKeyAlgo == SshCapabilities::PubKeyDss) {
diff --git a/src/libs/utils/ssh/sshoutgoingpacket.cpp b/src/libs/utils/ssh/sshoutgoingpacket.cpp
index 8983ca4c70..2604b90072 100644
--- a/src/libs/utils/ssh/sshoutgoingpacket.cpp
+++ b/src/libs/utils/ssh/sshoutgoingpacket.cpp
@@ -55,7 +55,7 @@ quint32 SshOutgoingPacket::macLength() const
     return m_encrypter.macLength();
 }
 
-void SshOutgoingPacket::generateKeyExchangeInitPacket()
+QByteArray SshOutgoingPacket::generateKeyExchangeInitPacket()
 {
     const QByteArray &supportedkeyExchangeMethods
         = encodeNameList(SshCapabilities::KeyExchangeMethods);
@@ -81,7 +81,9 @@ void SshOutgoingPacket::generateKeyExchangeInitPacket()
     m_data.append(supportedLanguages).append(supportedLanguages);
     appendBool(false); // No guessed packet.
     m_data.append(QByteArray(4, 0)); // Reserved.
+    QByteArray payload = m_data.mid(PayloadOffset);
     finalize();
+    return payload;
 }
 
 void SshOutgoingPacket::generateKeyDhInitPacket(const Botan::BigInt &e)
diff --git a/src/libs/utils/ssh/sshoutgoingpacket_p.h b/src/libs/utils/ssh/sshoutgoingpacket_p.h
index e103b9c7c3..e527d6e02a 100644
--- a/src/libs/utils/ssh/sshoutgoingpacket_p.h
+++ b/src/libs/utils/ssh/sshoutgoingpacket_p.h
@@ -48,7 +48,7 @@ public:
     SshOutgoingPacket(const SshEncryptionFacility &encrypter,
         const quint32 &seqNr);
 
-    void generateKeyExchangeInitPacket();
+    QByteArray generateKeyExchangeInitPacket(); // Returns payload.
     void generateKeyDhInitPacket(const Botan::BigInt &e);
     void generateNewKeysPacket();
     void generateDisconnectPacket(SshErrorCode reason,
diff --git a/src/libs/utils/ssh/sshpacket.cpp b/src/libs/utils/ssh/sshpacket.cpp
index 9b5fdfdbaf..0b97cd6c9b 100644
--- a/src/libs/utils/ssh/sshpacket.cpp
+++ b/src/libs/utils/ssh/sshpacket.cpp
@@ -86,12 +86,10 @@ SshPacketType AbstractSshPacket::type() const
     return static_cast<SshPacketType>(m_data.at(TypeOffset));
 }
 
-AbstractSshPacket::Payload AbstractSshPacket::payLoad() const
+QByteArray AbstractSshPacket::payLoad() const
 {
-    Payload p;
-    p.data = m_data.constData() + PayloadOffset;
-    p.size = length() - paddingLength() - 1;
-    return p;
+    return QByteArray(m_data.constData() + PayloadOffset,
+        length() - paddingLength() - 1);
 }
 
 void AbstractSshPacket::printRawBytes() const
diff --git a/src/libs/utils/ssh/sshpacket_p.h b/src/libs/utils/ssh/sshpacket_p.h
index bf70a8d340..fab9243b9c 100644
--- a/src/libs/utils/ssh/sshpacket_p.h
+++ b/src/libs/utils/ssh/sshpacket_p.h
@@ -113,8 +113,7 @@ public:
 
     const QByteArray &rawData() const { return m_data; }
 
-    struct Payload { const char *data; quint32 size; };
-    Payload payLoad() const;
+    QByteArray payLoad() const;
 
 protected:
     AbstractSshPacket();
diff --git a/src/libs/utils/ssh/sshsendfacility.cpp b/src/libs/utils/ssh/sshsendfacility.cpp
index fc816c1033..c0b433fbdf 100644
--- a/src/libs/utils/ssh/sshsendfacility.cpp
+++ b/src/libs/utils/ssh/sshsendfacility.cpp
@@ -74,11 +74,11 @@ void SshSendFacility::createAuthenticationKey(const QByteArray &privKeyFileConte
     m_encrypter.createAuthenticationKey(privKeyFileContents);
 }
 
-SshOutgoingPacket::Payload SshSendFacility::sendKeyExchangeInitPacket()
+QByteArray SshSendFacility::sendKeyExchangeInitPacket()
 {
-    m_outgoingPacket.generateKeyExchangeInitPacket();
+    const QByteArray &payLoad = m_outgoingPacket.generateKeyExchangeInitPacket();
     sendPacket();
-    return m_outgoingPacket.payLoad();
+    return payLoad;
 }
 
 void SshSendFacility::sendKeyDhInitPacket(const Botan::BigInt &e)
diff --git a/src/libs/utils/ssh/sshsendfacility_p.h b/src/libs/utils/ssh/sshsendfacility_p.h
index d14b739102..993ce017d1 100644
--- a/src/libs/utils/ssh/sshsendfacility_p.h
+++ b/src/libs/utils/ssh/sshsendfacility_p.h
@@ -55,7 +55,7 @@ public:
     void recreateKeys(const SshKeyExchange &keyExchange);
     void createAuthenticationKey(const QByteArray &privKeyFileContents);
 
-    SshOutgoingPacket::Payload sendKeyExchangeInitPacket();
+    QByteArray sendKeyExchangeInitPacket();
     void sendKeyDhInitPacket(const Botan::BigInt &e);
     void sendNewKeysPacket();
     void sendDisconnectPacket(SshErrorCode reason,