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.
This commit is contained in:
Christian Kandeler
2011-04-28 10:57:17 +02:00
parent c5a40e9e63
commit 9d90809e6d
7 changed files with 42 additions and 18 deletions

View File

@@ -44,6 +44,9 @@
#include <botan/pubkey.h> #include <botan/pubkey.h>
#include <botan/rsa.h> #include <botan/rsa.h>
#ifdef CREATOR_SSH_DEBUG
#include <iostream>
#endif
#include <string> #include <string>
using namespace Botan; using namespace Botan;
@@ -65,6 +68,18 @@ namespace {
Q_UNUSED(list); Q_UNUSED(list);
#endif #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 } // anonymous namespace
SshKeyExchange::SshKeyExchange(SshSendFacility &sendFacility) SshKeyExchange::SshKeyExchange(SshSendFacility &sendFacility)
@@ -77,9 +92,7 @@ SshKeyExchange::~SshKeyExchange() {}
void SshKeyExchange::sendKexInitPacket(const QByteArray &serverId) void SshKeyExchange::sendKexInitPacket(const QByteArray &serverId)
{ {
m_serverId = serverId; m_serverId = serverId;
const AbstractSshPacket::Payload &payload m_clientKexInitPayload = m_sendFacility.sendKeyExchangeInitPacket();
= m_sendFacility.sendKeyExchangeInitPacket();
m_clientKexInitPayload = QByteArray(payload.data, payload.size);
} }
bool SshKeyExchange::sendDhInitPacket(const SshIncomingPacket &serverKexInit) bool SshKeyExchange::sendDhInitPacket(const SshIncomingPacket &serverKexInit)
@@ -132,8 +145,7 @@ bool SshKeyExchange::sendDhInitPacket(const SshIncomingPacket &serverKexInit)
m_dhKey.reset(new DH_PrivateKey(rng, m_dhKey.reset(new DH_PrivateKey(rng,
DL_Group(botanKeyExchangeAlgoName(keyAlgo)))); DL_Group(botanKeyExchangeAlgoName(keyAlgo))));
const AbstractSshPacket::Payload &payload = serverKexInit.payLoad(); m_serverKexInitPayload = serverKexInit.payLoad();
m_serverKexInitPayload = QByteArray(payload.data, payload.size);
m_sendFacility.sendKeyDhInitPacket(m_dhKey->get_y()); m_sendFacility.sendKeyDhInitPacket(m_dhKey->get_y());
return kexInitParams.firstKexPacketFollows; return kexInitParams.firstKexPacketFollows;
} }
@@ -165,6 +177,19 @@ void SshKeyExchange::sendNewKeysPacket(const SshIncomingPacket &dhReply,
concatenatedData.size()); concatenatedData.size());
m_h = convertByteArray(hashResult); 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<Public_Key> sigKey;
QScopedPointer<PK_Verifier> verifier; QScopedPointer<PK_Verifier> verifier;
if (m_serverHostKeyAlgo == SshCapabilities::PubKeyDss) { if (m_serverHostKeyAlgo == SshCapabilities::PubKeyDss) {

View File

@@ -55,7 +55,7 @@ quint32 SshOutgoingPacket::macLength() const
return m_encrypter.macLength(); return m_encrypter.macLength();
} }
void SshOutgoingPacket::generateKeyExchangeInitPacket() QByteArray SshOutgoingPacket::generateKeyExchangeInitPacket()
{ {
const QByteArray &supportedkeyExchangeMethods const QByteArray &supportedkeyExchangeMethods
= encodeNameList(SshCapabilities::KeyExchangeMethods); = encodeNameList(SshCapabilities::KeyExchangeMethods);
@@ -81,7 +81,9 @@ void SshOutgoingPacket::generateKeyExchangeInitPacket()
m_data.append(supportedLanguages).append(supportedLanguages); m_data.append(supportedLanguages).append(supportedLanguages);
appendBool(false); // No guessed packet. appendBool(false); // No guessed packet.
m_data.append(QByteArray(4, 0)); // Reserved. m_data.append(QByteArray(4, 0)); // Reserved.
QByteArray payload = m_data.mid(PayloadOffset);
finalize(); finalize();
return payload;
} }
void SshOutgoingPacket::generateKeyDhInitPacket(const Botan::BigInt &e) void SshOutgoingPacket::generateKeyDhInitPacket(const Botan::BigInt &e)

View File

@@ -48,7 +48,7 @@ public:
SshOutgoingPacket(const SshEncryptionFacility &encrypter, SshOutgoingPacket(const SshEncryptionFacility &encrypter,
const quint32 &seqNr); const quint32 &seqNr);
void generateKeyExchangeInitPacket(); QByteArray generateKeyExchangeInitPacket(); // Returns payload.
void generateKeyDhInitPacket(const Botan::BigInt &e); void generateKeyDhInitPacket(const Botan::BigInt &e);
void generateNewKeysPacket(); void generateNewKeysPacket();
void generateDisconnectPacket(SshErrorCode reason, void generateDisconnectPacket(SshErrorCode reason,

View File

@@ -86,12 +86,10 @@ SshPacketType AbstractSshPacket::type() const
return static_cast<SshPacketType>(m_data.at(TypeOffset)); return static_cast<SshPacketType>(m_data.at(TypeOffset));
} }
AbstractSshPacket::Payload AbstractSshPacket::payLoad() const QByteArray AbstractSshPacket::payLoad() const
{ {
Payload p; return QByteArray(m_data.constData() + PayloadOffset,
p.data = m_data.constData() + PayloadOffset; length() - paddingLength() - 1);
p.size = length() - paddingLength() - 1;
return p;
} }
void AbstractSshPacket::printRawBytes() const void AbstractSshPacket::printRawBytes() const

View File

@@ -113,8 +113,7 @@ public:
const QByteArray &rawData() const { return m_data; } const QByteArray &rawData() const { return m_data; }
struct Payload { const char *data; quint32 size; }; QByteArray payLoad() const;
Payload payLoad() const;
protected: protected:
AbstractSshPacket(); AbstractSshPacket();

View File

@@ -74,11 +74,11 @@ void SshSendFacility::createAuthenticationKey(const QByteArray &privKeyFileConte
m_encrypter.createAuthenticationKey(privKeyFileContents); m_encrypter.createAuthenticationKey(privKeyFileContents);
} }
SshOutgoingPacket::Payload SshSendFacility::sendKeyExchangeInitPacket() QByteArray SshSendFacility::sendKeyExchangeInitPacket()
{ {
m_outgoingPacket.generateKeyExchangeInitPacket(); const QByteArray &payLoad = m_outgoingPacket.generateKeyExchangeInitPacket();
sendPacket(); sendPacket();
return m_outgoingPacket.payLoad(); return payLoad;
} }
void SshSendFacility::sendKeyDhInitPacket(const Botan::BigInt &e) void SshSendFacility::sendKeyDhInitPacket(const Botan::BigInt &e)

View File

@@ -55,7 +55,7 @@ public:
void recreateKeys(const SshKeyExchange &keyExchange); void recreateKeys(const SshKeyExchange &keyExchange);
void createAuthenticationKey(const QByteArray &privKeyFileContents); void createAuthenticationKey(const QByteArray &privKeyFileContents);
SshOutgoingPacket::Payload sendKeyExchangeInitPacket(); QByteArray sendKeyExchangeInitPacket();
void sendKeyDhInitPacket(const Botan::BigInt &e); void sendKeyDhInitPacket(const Botan::BigInt &e);
void sendNewKeysPacket(); void sendNewKeysPacket();
void sendDisconnectPacket(SshErrorCode reason, void sendDisconnectPacket(SshErrorCode reason,