Skip to content

Commit a6861d9

Browse files
committed
Validate the host key signature algorithm name in DoKexDhReply().
The client-side KEXDH_REPLY path was parsing the signature blob name and skipping over it without checking that it matched the negotiated host key algorithm. That allowed an RSA server to negotiate rsa-sha2-256 or rsa-sha2-512 but send a signature blob labeled ssh-rsa instead. Fix this by comparing the signature blob name against the expected signature type derived from handshake->pubKeyId before verifying the signature bytes. Add regress coverage that drives an in-memory client/server handshake, rewrites the server's first KEXDH_REPLY on the wire, and verifies the client rejects rsa-sha2-256 and rsa-sha2-512 replies whose signature blob name is downgraded to ssh-rsa. F-2077
1 parent 98e3b63 commit a6861d9

2 files changed

Lines changed: 613 additions & 4 deletions

File tree

src/internal.c

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5761,6 +5761,9 @@ static int KeyAgree_client(WOLFSSH* ssh, byte hashId, const byte* f, word32 fSz)
57615761
}
57625762

57635763

5764+
static INLINE byte SigTypeForId(byte id);
5765+
5766+
57645767
static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
57655768
{
57665769
struct wolfSSH_sigKeyBlock *sigKeyBlock_ptr = NULL;
@@ -6010,9 +6013,10 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
60106013
#ifndef WOLFSSH_NO_RSA
60116014
int tmpIdx = begin - sigSz;
60126015
#endif
6013-
/* Skip past the sig name. Check it, though. Other SSH
6014-
* implementations do the verify based on the name, despite what
6015-
* was agreed upon. XXX*/
6016+
const char* expectedSigName =
6017+
IdToName(SigTypeForId(ssh->handshake->pubKeyId));
6018+
word32 expectedSigNameSz = (word32)WSTRLEN(expectedSigName);
6019+
60166020
begin = 0;
60176021
ret = GetUint32(&scratch, sig, sigSz, &begin);
60186022
if (ret == WS_SUCCESS) {
@@ -6023,6 +6027,15 @@ static int DoKexDhReply(WOLFSSH* ssh, byte* buf, word32 len, word32* idx)
60236027
ret = WS_PARSE_E;
60246028
}
60256029
}
6030+
if (ret == WS_SUCCESS) {
6031+
if (scratch != expectedSigNameSz ||
6032+
WMEMCMP(sig + begin, expectedSigName, scratch) != 0) {
6033+
WLOG(WS_LOG_DEBUG,
6034+
"signature name %.*s did not match negotiated %s",
6035+
(int)scratch, sig + begin, expectedSigName);
6036+
ret = WS_PARSE_E;
6037+
}
6038+
}
60266039
if (ret == WS_SUCCESS) {
60276040
begin += scratch;
60286041
ret = GetUint32(&scratch, sig, sigSz, &begin);
@@ -10569,7 +10582,6 @@ static int PreparePacket(WOLFSSH* ssh, word32 payloadSz)
1056910582
return ret;
1057010583
}
1057110584

10572-
1057310585
static int BundlePacket(WOLFSSH* ssh)
1057410586
{
1057510587
byte* output = NULL;

0 commit comments

Comments
 (0)