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.

diff --git a/src/libs/utils/ssh/sshkeyexchange.cpp b/src/libs/utils/ssh/sshkeyexchange.cpp
index 1112cd2..1165569 100644 (file)
--- 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 8983ca4..2604b90 100644 (file)
--- 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 e103b9c..e527d6e 100644 (file)
--- 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 9b5fdfd..0b97cd6 100644 (file)
--- 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 bf70a8d..fab9243 100644 (file)
--- 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 fc816c1..c0b433f 100644 (file)
--- 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 d14b739..993ce01 100644 (file)
--- 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,