From a4a78605ba2f3c645272694403850dfd7bbc655b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 30 Oct 2019 12:05:03 -0700 Subject: [PATCH] KEX fixes 1. Changed the KEX buffers to use a macro to set their value based on a 3072-bit DH key. 2. Fixed the selection of the KEX type string sent to the peer in the KEX Init message. --- src/internal.c | 34 +++++++++++++++++++--------------- wolfssh/internal.h | 11 ++++++++--- 2 files changed, 27 insertions(+), 18 deletions(-) diff --git a/src/internal.c b/src/internal.c index cb5370667..08eae5e97 100644 --- a/src/internal.c +++ b/src/internal.c @@ -5483,30 +5483,30 @@ static const char cannedKexAlgoNames[] = #if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) "ecdh-sha2-nistp256" #endif -#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_ECDH_GEX_SHA256) +#if !defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && !defined(WOLFSSH_NO_DH_GEX_SHA256) "," #endif -#if !defined(WOLFSSH_NO_ECDH_GEX_SHA256) +#if !defined(WOLFSSH_NO_DH_GEX_SHA256) "diffie-hellman-group-exchange-sha256" #endif -#if (!defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) || !defined(WOLFSSH_NO_ECDH_GEX_SHA256))\ - && !defined(WOLFSSH_NO_ECDH_GROUP14_SHA1) +#if (!defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) || !defined(WOLFSSH_NO_DH_GEX_SHA256))\ + && !defined(WOLFSSH_NO_DH_GROUP14_SHA1) "," #endif -#if !defined(WOLFSSH_NO_ECDH_GROUP14_SHA1) +#if !defined(WOLFSSH_NO_DH_GROUP14_SHA1) "diffie-hellman-group14-sha1" #endif -#if (!defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) || !defined(WOLFSSH_NO_ECDH_GEX_SHA256) \ - || !defined(WOLFSSH_NO_ECDH_GROUP14_SHA1)) && !defined(WOLFSSH_NO_ECDH_GROUP1_SHA1) +#if (!defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) || !defined(WOLFSSH_NO_DH_GEX_SHA256) \ + || !defined(WOLFSSH_NO_DH_GROUP14_SHA1)) && !defined(WOLFSSH_NO_DH_GROUP1_SHA1) "," #endif -#if !defined(WOLFSSH_NO_ECDH_GROUP1_SHA1) +#if !defined(WOLFSSH_NO_DH_GROUP1_SHA1) "diffie-hellman-group1-sha1"; #endif -#if defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && defined(WOLFSSH_NO_ECDH_GEX_SHA256)\ - && defined(WOLFSSH_NO_ECDH_GROUP14_SHA1) && defined(WOLFSSH_NO_ECDH_GROUP1_SHA1) - #warning "You need at least one of ECDH-SHA2-NISTP256, ECDH-GEX-SHA256, " - "ECDH_GROUP14-SHA1 or ECDH-GROUP1-SHA1" +#if defined(WOLFSSH_NO_ECDH_SHA2_NISTP256) && defined(WOLFSSH_NO_DH_GEX_SHA256)\ + && defined(WOLFSSH_NO_DH_GROUP14_SHA1) && defined(WOLFSSH_NO_DH_GROUP1_SHA1) + #warning "You need at least one of ECDH-SHA2-NISTP256, DH-GEX-SHA256, " + "DH-GROUP14-SHA1 or DH-GROUP1-SHA1" #endif static const char cannedNoneNames[] = "none"; @@ -5993,7 +5993,7 @@ int SendKexDhReply(WOLFSSH* ssh) if (ret == 0) { if (!useEcc) { DhKey privKey; - byte y[256]; + byte y[MAX_KEX_KEY_SZ]; word32 ySz = sizeof(y); ret = wc_InitDhKey(&privKey); @@ -6364,6 +6364,10 @@ int SendKexDhGexRequest(WOLFSSH* ssh) output[idx++] = MSGID_KEXDH_GEX_REQUEST; + WLOG(WS_LOG_INFO, " min = %u, preferred = %u, max = %u", + ssh->handshake->dhGexMinSz, + ssh->handshake->dhGexPreferredSz, + ssh->handshake->dhGexMaxSz); c32toa(ssh->handshake->dhGexMinSz, output + idx); idx += UINT32_SZ; c32toa(ssh->handshake->dhGexPreferredSz, output + idx); @@ -6466,7 +6470,7 @@ int SendKexDhInit(WOLFSSH* ssh) word32 generatorSz = dhGeneratorSz; int ret = WS_SUCCESS; byte msgId = MSGID_KEXDH_INIT; - byte e[256]; + byte e[MAX_KEX_KEY_SZ+1]; /* plus 1 in case of padding. */ word32 eSz = sizeof(e); byte ePad = 0; @@ -6563,7 +6567,7 @@ int SendKexDhInit(WOLFSSH* ssh) if (ePad) { output[idx] = 0; - idx += 1; + idx++; } WMEMCPY(output + idx, e, eSz); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index c1e941118..24b3e50de 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -143,6 +143,10 @@ enum { /* This is from RFC 4253 section 6.1. */ #define MAX_PACKET_SZ 35000 #endif +#ifndef MAX_KEX_KEY_SZ + /* This is based on the 3072-bit DH key that is the preferred size. */ + #define MAX_KEX_KEY_SZ (3072 / 8) +#endif WOLFSSH_LOCAL byte NameToId(const char*, word32); WOLFSSH_LOCAL const char* IdToName(byte); @@ -231,9 +235,10 @@ typedef struct HandshakeInfo { Keys keys; Keys peerKeys; wc_HashAlg hash; - byte e[257]; /* May have a leading zero for unsigned or is a Q_S value. */ + byte e[MAX_KEX_KEY_SZ+1]; /* May have a leading zero for unsigned + or is a Q_S value. */ word32 eSz; - byte x[257]; /* May have a leading zero, for unsigned. */ + byte x[MAX_KEX_KEY_SZ+1]; /* May have a leading zero, for unsigned. */ word32 xSz; byte* kexInit; word32 kexInitSz; @@ -387,7 +392,7 @@ struct WOLFSSH { byte h[WC_MAX_DIGEST_SIZE]; word32 hSz; - byte k[257]; /* May have a leading zero, for unsigned. */ + byte k[MAX_KEX_KEY_SZ+1]; /* May have a leading zero, for unsigned. */ word32 kSz; byte sessionId[WC_MAX_DIGEST_SIZE]; word32 sessionIdSz;