From f8a8c6cac3f7e1955025e72dde451e00c10a1077 Mon Sep 17 00:00:00 2001
From: Christian Kandeler <christian.kandeler@theqtcompany.com>
Date: Tue, 23 Jun 2015 12:53:30 +0200
Subject: [PATCH] SSH: Do not tie key exchange methods to specific host key
algorithms.
Contrary to what I believed, all key exchange methods are compatible
with all types of host key (at least for everything we support).
Task-number: QTCREATORBUG-14621
Change-Id: Iacae3d36170b9841cceb3705780a64aeb15e10b9
Reviewed-by: Benjamin Zeller <benjamin.zeller@canonical.com>
Reviewed-by: Christian Kandeler <christian.kandeler@theqtcompany.com>
---
src/libs/ssh/sshbotanconversions_p.h | 11 ++++--
src/libs/ssh/sshincomingpacket.cpp | 54 +++++++++++++++++-----------
src/libs/ssh/sshincomingpacket_p.h | 3 +-
src/libs/ssh/sshkeyexchange.cpp | 44 +++++++----------------
4 files changed, 57 insertions(+), 55 deletions(-)
diff --git a/src/libs/ssh/sshbotanconversions_p.h b/src/libs/ssh/sshbotanconversions_p.h
index 31dc4899a72..75182881b91 100644
--- a/src/libs/ssh/sshbotanconversions_p.h
+++ b/src/libs/ssh/sshbotanconversions_p.h
@@ -118,11 +118,16 @@ inline const char *botanHMacAlgoName(const QByteArray &rfcAlgoName)
inline quint32 botanHMacKeyLen(const QByteArray &rfcAlgoName)
{
- Q_ASSERT(rfcAlgoName == SshCapabilities::HMacSha1
- || rfcAlgoName == SshCapabilities::HMacSha256);
if (rfcAlgoName == SshCapabilities::HMacSha1)
return 20;
- return 32;
+ if (rfcAlgoName == SshCapabilities::HMacSha256)
+ return 32;
+ if (rfcAlgoName == SshCapabilities::HMacSha384)
+ return 48;
+ if (rfcAlgoName == SshCapabilities::HMacSha512)
+ return 64;
+ throw SshClientException(SshInternalError, SSH_TR("Unexpected hashing algorithm \"%1\"")
+ .arg(QString::fromLatin1(rfcAlgoName)));
}
} // namespace Internal
diff --git a/src/libs/ssh/sshincomingpacket.cpp b/src/libs/ssh/sshincomingpacket.cpp
index f2101e54dea..1742c335a4a 100644
--- a/src/libs/ssh/sshincomingpacket.cpp
+++ b/src/libs/ssh/sshincomingpacket.cpp
@@ -30,6 +30,7 @@
#include "sshincomingpacket_p.h"
+#include "ssh_global.h"
#include "sshbotanconversions_p.h"
#include "sshcapabilities_p.h"
@@ -169,7 +170,32 @@ SshKeyExchangeInit SshIncomingPacket::extractKeyExchangeInitData() const
return exchangeData;
}
-SshKeyExchangeReply SshIncomingPacket::extractKeyExchangeReply(const QByteArray &pubKeyAlgo) const
+static void getHostKeySpecificReplyData(SshKeyExchangeReply &replyData,
+ const QByteArray &hostKeyAlgo, const QByteArray &input)
+{
+ quint32 offset = 0;
+ if (hostKeyAlgo == SshCapabilities::PubKeyDss || hostKeyAlgo == SshCapabilities::PubKeyRsa) {
+ // DSS: p and q, RSA: e and n
+ replyData.parameters << SshPacketParser::asBigInt(input, &offset);
+ replyData.parameters << SshPacketParser::asBigInt(input, &offset);
+
+ // g and y
+ if (hostKeyAlgo == SshCapabilities::PubKeyDss) {
+ replyData.parameters << SshPacketParser::asBigInt(input, &offset);
+ replyData.parameters << SshPacketParser::asBigInt(input, &offset);
+ }
+ } else {
+ QSSH_ASSERT_AND_RETURN(hostKeyAlgo == SshCapabilities::PubKeyEcdsa);
+ if (SshPacketParser::asString(input, &offset)
+ != hostKeyAlgo.mid(11)) { // Without "ecdsa-sha2-" prefix.
+ throw SshPacketParseException();
+ }
+ replyData.q = SshPacketParser::asString(input, &offset);
+ }
+}
+
+SshKeyExchangeReply SshIncomingPacket::extractKeyExchangeReply(const QByteArray &kexAlgo,
+ const QByteArray &hostKeyAlgo) const
{
Q_ASSERT(isComplete());
Q_ASSERT(type() == SSH_MSG_KEXDH_REPLY);
@@ -179,35 +205,23 @@ SshKeyExchangeReply SshIncomingPacket::extractKeyExchangeReply(const QByteArray
quint32 topLevelOffset = TypeOffset + 1;
replyData.k_s = SshPacketParser::asString(m_data, &topLevelOffset);
quint32 k_sOffset = 0;
- if (SshPacketParser::asString(replyData.k_s, &k_sOffset) != pubKeyAlgo)
+ if (SshPacketParser::asString(replyData.k_s, &k_sOffset) != hostKeyAlgo)
throw SshPacketParseException();
+ getHostKeySpecificReplyData(replyData, hostKeyAlgo, replyData.k_s.mid(k_sOffset));
- if (pubKeyAlgo == SshCapabilities::PubKeyDss || pubKeyAlgo == SshCapabilities::PubKeyRsa) {
-
- // DSS: p and q, RSA: e and n
- replyData.parameters << SshPacketParser::asBigInt(replyData.k_s, &k_sOffset);
- replyData.parameters << SshPacketParser::asBigInt(replyData.k_s, &k_sOffset);
-
- // g and y
- if (pubKeyAlgo == SshCapabilities::PubKeyDss) {
- replyData.parameters << SshPacketParser::asBigInt(replyData.k_s, &k_sOffset);
- replyData.parameters << SshPacketParser::asBigInt(replyData.k_s, &k_sOffset);
- }
-
+ if (kexAlgo == SshCapabilities::DiffieHellmanGroup1Sha1) {
replyData.f = SshPacketParser::asBigInt(m_data, &topLevelOffset);
} else {
- Q_ASSERT(pubKeyAlgo == SshCapabilities::PubKeyEcdsa);
- if (SshPacketParser::asString(replyData.k_s, &k_sOffset) != pubKeyAlgo.mid(11)) // Without "ecdsa-sha2-" prefix.
- throw SshPacketParseException();
- replyData.q = SshPacketParser::asString(replyData.k_s, &k_sOffset);
+ QSSH_ASSERT_AND_RETURN_VALUE(kexAlgo.startsWith(SshCapabilities::EcdhKexNamePrefix),
+ SshKeyExchangeReply());
replyData.q_s = SshPacketParser::asString(m_data, &topLevelOffset);
}
const QByteArray fullSignature = SshPacketParser::asString(m_data, &topLevelOffset);
quint32 sigOffset = 0;
- if (SshPacketParser::asString(fullSignature, &sigOffset) != pubKeyAlgo)
+ if (SshPacketParser::asString(fullSignature, &sigOffset) != hostKeyAlgo)
throw SshPacketParseException();
replyData.signatureBlob = SshPacketParser::asString(fullSignature, &sigOffset);
- if (pubKeyAlgo == SshCapabilities::PubKeyEcdsa) {
+ if (hostKeyAlgo == SshCapabilities::PubKeyEcdsa) {
// Botan's PK_Verifier wants the signature in this format.
quint32 blobOffset = 0;
const Botan::BigInt r = SshPacketParser::asBigInt(replyData.signatureBlob, &blobOffset);
diff --git a/src/libs/ssh/sshincomingpacket_p.h b/src/libs/ssh/sshincomingpacket_p.h
index 9ea9d80278a..d2d17eed6ed 100644
--- a/src/libs/ssh/sshincomingpacket_p.h
+++ b/src/libs/ssh/sshincomingpacket_p.h
@@ -164,7 +164,8 @@ public:
void reset();
SshKeyExchangeInit extractKeyExchangeInitData() const;
- SshKeyExchangeReply extractKeyExchangeReply(const QByteArray &pubKeyAlgo) const;
+ SshKeyExchangeReply extractKeyExchangeReply(const QByteArray &kexAlgo,
+ const QByteArray &hostKeyAlgo) const;
SshDisconnect extractDisconnect() const;
SshUserAuthBanner extractUserAuthBanner() const;
SshUserAuthInfoRequestPacket extractUserAuthInfoRequest() const;
diff --git a/src/libs/ssh/sshkeyexchange.cpp b/src/libs/ssh/sshkeyexchange.cpp
index c553ecc8b22..62071e4fc70 100644
--- a/src/libs/ssh/sshkeyexchange.cpp
+++ b/src/libs/ssh/sshkeyexchange.cpp
@@ -30,6 +30,7 @@
#include "sshkeyexchange_p.h"
+#include "ssh_global.h"
#include "sshbotanconversions_p.h"
#include "sshcapabilities_p.h"
#include "sshsendfacility_p.h"
@@ -115,28 +116,8 @@ bool SshKeyExchange::sendDhInitPacket(const SshIncomingPacket &serverKexInit)
m_kexAlgoName = SshCapabilities::findBestMatch(SshCapabilities::KeyExchangeMethods,
kexInitParams.keyAlgorithms.names);
- const QList<QByteArray> &commonHostKeyAlgos
- = SshCapabilities::commonCapabilities(SshCapabilities::PublicKeyAlgorithms,
- kexInitParams.serverHostKeyAlgorithms.names);
- const bool ecdh = m_kexAlgoName.startsWith(SshCapabilities::EcdhKexNamePrefix);
- foreach (const QByteArray &possibleHostKeyAlgo, commonHostKeyAlgos) {
- if (ecdh && possibleHostKeyAlgo == SshCapabilities::PubKeyEcdsa) {
- m_serverHostKeyAlgo = possibleHostKeyAlgo;
- break;
- }
- if (!ecdh && (possibleHostKeyAlgo == SshCapabilities::PubKeyDss
- || possibleHostKeyAlgo == SshCapabilities::PubKeyRsa)) {
- m_serverHostKeyAlgo = possibleHostKeyAlgo;
- break;
- }
- }
- if (m_serverHostKeyAlgo.isEmpty()) {
- throw SshServerException(SSH_DISCONNECT_KEY_EXCHANGE_FAILED,
- "Invalid combination of key exchange and host key algorithms.",
- QCoreApplication::translate("SshConnection",
- "No matching host key algorithm available for key exchange algorithm \"%1\".")
- .arg(QString::fromLatin1(m_kexAlgoName)));
- }
+ m_serverHostKeyAlgo = SshCapabilities::findBestMatch(SshCapabilities::PublicKeyAlgorithms,
+ kexInitParams.serverHostKeyAlgorithms.names);
determineHashingAlgorithm(kexInitParams, true);
determineHashingAlgorithm(kexInitParams, false);
@@ -152,7 +133,7 @@ bool SshKeyExchange::sendDhInitPacket(const SshIncomingPacket &serverKexInit)
kexInitParams.compressionAlgorithmsServerToClient.names);
AutoSeeded_RNG rng;
- if (ecdh) {
+ if (m_kexAlgoName.startsWith(SshCapabilities::EcdhKexNamePrefix)) {
m_ecdhKey.reset(new ECDH_PrivateKey(rng, EC_Group(botanKeyExchangeAlgoName(m_kexAlgoName))));
m_sendFacility.sendKeyEcdhInitPacket(convertByteArray(m_ecdhKey->public_value()));
} else {
@@ -169,7 +150,7 @@ void SshKeyExchange::sendNewKeysPacket(const SshIncomingPacket &dhReply,
{
const SshKeyExchangeReply &reply
- = dhReply.extractKeyExchangeReply(m_serverHostKeyAlgo);
+ = dhReply.extractKeyExchangeReply(m_kexAlgoName, m_serverHostKeyAlgo);
if (m_dhKey && (reply.f <= 0 || reply.f >= m_dhKey->group_p())) {
throw SSH_SERVER_EXCEPTION(SSH_DISCONNECT_KEY_EXCHANGE_FAILED,
"Server sent invalid f.");
@@ -187,6 +168,7 @@ void SshKeyExchange::sendNewKeysPacket(const SshIncomingPacket &dhReply,
DH_KA_Operation dhOp(*m_dhKey);
SecureVector<byte> encodedF = BigInt::encode(reply.f);
encodedK = dhOp.agree(encodedF, encodedF.size());
+ m_dhKey.reset(nullptr);
} else {
Q_ASSERT(m_ecdhKey);
concatenatedData // Q_C.
@@ -194,7 +176,9 @@ void SshKeyExchange::sendNewKeysPacket(const SshIncomingPacket &dhReply,
concatenatedData += AbstractSshPacket::encodeString(reply.q_s);
ECDH_KA_Operation ecdhOp(*m_ecdhKey);
encodedK = ecdhOp.agree(convertByteArray(reply.q_s), reply.q_s.count());
+ m_ecdhKey.reset(nullptr);
}
+
const BigInt k = BigInt::decode(encodedK);
m_k = AbstractSshPacket::encodeMpInt(k); // Roundtrip, as Botan encodes BigInts somewhat differently.
concatenatedData += m_k;
@@ -228,13 +212,13 @@ void SshKeyExchange::sendNewKeysPacket(const SshIncomingPacket &dhReply,
RSA_PublicKey * const rsaKey
= new RSA_PublicKey(reply.parameters.at(1), reply.parameters.at(0));
sigKey.reset(rsaKey);
- } else if (m_serverHostKeyAlgo == SshCapabilities::PubKeyEcdsa) {
+ } else {
+ QSSH_ASSERT_AND_RETURN(m_serverHostKeyAlgo == SshCapabilities::PubKeyEcdsa);
+ const EC_Group domain("secp256r1");
const PointGFp point = OS2ECP(convertByteArray(reply.q), reply.q.count(),
- m_ecdhKey->domain().get_curve());
- ECDSA_PublicKey * const ecdsaKey = new ECDSA_PublicKey(m_ecdhKey->domain(), point);
+ domain.get_curve());
+ ECDSA_PublicKey * const ecdsaKey = new ECDSA_PublicKey(domain, point);
sigKey.reset(ecdsaKey);
- } else {
- Q_ASSERT(!"Impossible: Neither DSS nor RSA nor ECDSA!");
}
const byte * const botanH = convertByteArray(m_h);
@@ -248,8 +232,6 @@ void SshKeyExchange::sendNewKeysPacket(const SshIncomingPacket &dhReply,
checkHostKey(reply.k_s);
m_sendFacility.sendNewKeysPacket();
- m_dhKey.reset(nullptr);
- m_ecdhKey.reset(nullptr);
}
QByteArray SshKeyExchange::hashAlgoForKexAlgo() const
--
GitLab