From 73e4e58795fd130f0fe9dfb7d26e7946308b477f Mon Sep 17 00:00:00 2001 From: John Safranek Date: Mon, 3 Oct 2016 15:30:41 -0700 Subject: [PATCH] 1. Regroup the session keys into a separate structure. 2. Separate sets of keys for client and server. 3. Store generated keys in handshake info record. 4. Copy session keys over when sending and receiving the New Keys message. --- src/internal.c | 72 +++++++++++++++++++++++++++++----------------- src/ssh.c | 29 ++++++++++--------- wolfssh/internal.h | 27 ++++++++--------- 3 files changed, 76 insertions(+), 52 deletions(-) diff --git a/src/internal.c b/src/internal.c index 5f39717e..b4cdf950 100644 --- a/src/internal.c +++ b/src/internal.c @@ -954,9 +954,13 @@ static int DoKexInit(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) } else { ssh->handshake->encryptId = algoId; - ssh->handshake->blockSz = ssh->ivClientSz = ssh->ivServerSz - = BlockSzForId(algoId); - ssh->encKeyClientSz = ssh->encKeyServerSz = KeySzForId(algoId); + ssh->handshake->blockSz = + ssh->handshake->clientKeys.ivSz = + ssh->handshake->serverKeys.ivSz = + BlockSzForId(algoId); + ssh->handshake->clientKeys.encKeySz = + ssh->handshake->serverKeys.encKeySz = + KeySzForId(algoId); } } @@ -987,7 +991,9 @@ static int DoKexInit(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) else { ssh->handshake->macId = algoId; ssh->handshake->macSz = MacSzForId(algoId); - ssh->macKeyClientSz = ssh->macKeyServerSz = KeySzForId(algoId); + ssh->handshake->clientKeys.macKeySz = + ssh->handshake->serverKeys.macKeySz = + KeySzForId(algoId); } } } @@ -1176,6 +1182,7 @@ static int DoNewKeys(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) ssh->peerMacId = ssh->handshake->macId; ssh->peerBlockSz = ssh->handshake->blockSz; ssh->peerMacSz = ssh->handshake->macSz; + WMEMCPY(&ssh->clientKeys, &ssh->handshake->clientKeys, sizeof(Keys)); switch (ssh->peerEncryptId) { case ID_NONE: @@ -1185,8 +1192,9 @@ static int DoNewKeys(WOLFSSH* ssh, uint8_t* buf, uint32_t len, uint32_t* idx) case ID_AES128_CBC: WLOG(WS_LOG_DEBUG, "DNK: peer using cipher aes128-cbc"); ret = wc_AesSetKey(&ssh->decryptCipher.aes, - ssh->encKeyClient, ssh->encKeyClientSz, - ssh->ivClient, AES_DECRYPTION); + ssh->clientKeys.encKey, + ssh->clientKeys.encKeySz, + ssh->clientKeys.iv, AES_DECRYPTION); break; default: @@ -1289,31 +1297,37 @@ int GenerateKey(uint8_t hashId, uint8_t keyId, static int GenerateKeys(WOLFSSH* ssh) { + Keys* cK; + Keys* sK; + if (ssh == NULL) return WS_BAD_ARGUMENT; + cK = &ssh->handshake->clientKeys; + sK = &ssh->handshake->serverKeys; + GenerateKey(0, 'A', - ssh->ivClient, ssh->ivClientSz, + cK->iv, cK->ivSz, ssh->k, ssh->kSz, ssh->h, ssh->hSz, ssh->sessionId, ssh->sessionIdSz); GenerateKey(0, 'B', - ssh->ivServer, ssh->ivServerSz, + sK->iv, sK->ivSz, ssh->k, ssh->kSz, ssh->h, ssh->hSz, ssh->sessionId, ssh->sessionIdSz); GenerateKey(0, 'C', - ssh->encKeyClient, ssh->encKeyClientSz, + cK->encKey, cK->encKeySz, ssh->k, ssh->kSz, ssh->h, ssh->hSz, ssh->sessionId, ssh->sessionIdSz); GenerateKey(0, 'D', - ssh->encKeyServer, ssh->encKeyServerSz, + sK->encKey, sK->encKeySz, ssh->k, ssh->kSz, ssh->h, ssh->hSz, ssh->sessionId, ssh->sessionIdSz); GenerateKey(0, 'E', - ssh->macKeyClient, ssh->macKeyClientSz, + cK->macKey, cK->macKeySz, ssh->k, ssh->kSz, ssh->h, ssh->hSz, ssh->sessionId, ssh->sessionIdSz); GenerateKey(0, 'F', - ssh->macKeyServer, ssh->macKeyServerSz, + sK->macKey, sK->macKeySz, ssh->k, ssh->kSz, ssh->h, ssh->hSz, ssh->sessionId, ssh->sessionIdSz); @@ -1325,17 +1339,17 @@ static int GenerateKeys(WOLFSSH* ssh) printf("Session ID:\n"); DumpOctetString(ssh->sessionId, ssh->sessionIdSz); printf("A:\n"); - DumpOctetString(ssh->ivClient, ssh->ivClientSz); + DumpOctetString(cK->iv, cK->ivSz); printf("B:\n"); - DumpOctetString(ssh->ivServer, ssh->ivServerSz); + DumpOctetString(sK->iv, sK->ivSz); printf("C:\n"); - DumpOctetString(ssh->encKeyClient, ssh->encKeyClientSz); + DumpOctetString(cK->encKey, cK->encKeySz); printf("D:\n"); - DumpOctetString(ssh->encKeyServer, ssh->encKeyServerSz); + DumpOctetString(sK->encKey, sK->encKeySz); printf("E:\n"); - DumpOctetString(ssh->macKeyClient, ssh->macKeyClientSz); + DumpOctetString(cK->macKey, cK->macKeySz); printf("F:\n"); - DumpOctetString(ssh->macKeyServer, ssh->macKeyServerSz); + DumpOctetString(sK->macKey, sK->macKeySz); printf("\n"); #endif /* SHOW_SECRETS */ @@ -2322,7 +2336,8 @@ static INLINE int CreateMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, uint8_t digest[SHA_DIGEST_SIZE]; wc_HmacSetKey(&hmac, SHA, - ssh->macKeyServer, ssh->macKeyServerSz); + ssh->serverKeys.macKey, + ssh->serverKeys.macKeySz); wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); wc_HmacUpdate(&hmac, in, inSz); wc_HmacFinal(&hmac, digest); @@ -2335,7 +2350,8 @@ static INLINE int CreateMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, Hmac hmac; wc_HmacSetKey(&hmac, SHA, - ssh->macKeyServer, ssh->macKeyServerSz); + ssh->serverKeys.macKey, + ssh->serverKeys.macKeySz); wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); wc_HmacUpdate(&hmac, in, inSz); wc_HmacFinal(&hmac, mac); @@ -2347,7 +2363,8 @@ static INLINE int CreateMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, Hmac hmac; wc_HmacSetKey(&hmac, SHA256, - ssh->macKeyServer, ssh->macKeyServerSz); + ssh->serverKeys.macKey, + ssh->serverKeys.macKeySz); wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); wc_HmacUpdate(&hmac, in, inSz); wc_HmacFinal(&hmac, mac); @@ -2376,7 +2393,7 @@ static INLINE int VerifyMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, WLOG(WS_LOG_DEBUG, "VerifyMac %s", IdToName(ssh->peerMacId)); WLOG(WS_LOG_DEBUG, "VM: inSz = %u", inSz); WLOG(WS_LOG_DEBUG, "VM: seq = %u", ssh->peerSeq); - WLOG(WS_LOG_DEBUG, "VM: keyLen = %u", ssh->macKeyClientSz); + WLOG(WS_LOG_DEBUG, "VM: keyLen = %u", ssh->clientKeys.macKeySz); switch (ssh->peerMacId) { case ID_NONE: @@ -2384,7 +2401,8 @@ static INLINE int VerifyMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, case ID_HMAC_SHA1: case ID_HMAC_SHA1_96: - wc_HmacSetKey(&hmac, SHA, ssh->macKeyClient, ssh->macKeyClientSz); + wc_HmacSetKey(&hmac, SHA, + ssh->clientKeys.macKey, ssh->clientKeys.macKeySz); wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); wc_HmacUpdate(&hmac, in, inSz); wc_HmacFinal(&hmac, checkMac); @@ -2394,7 +2412,7 @@ static INLINE int VerifyMac(WOLFSSH* ssh, const uint8_t* in, uint32_t inSz, case ID_HMAC_SHA2_256: wc_HmacSetKey(&hmac, SHA256, - ssh->macKeyClient, ssh->macKeyClientSz); + ssh->clientKeys.macKey, ssh->clientKeys.macKeySz); wc_HmacUpdate(&hmac, flatSeq, sizeof(flatSeq)); wc_HmacUpdate(&hmac, in, inSz); wc_HmacFinal(&hmac, checkMac); @@ -3002,6 +3020,7 @@ int SendNewKeys(WOLFSSH* ssh) ssh->encryptId = ssh->handshake->encryptId; ssh->macSz = ssh->handshake->macSz; ssh->macId = ssh->handshake->macId; + WMEMCPY(&ssh->serverKeys, &ssh->handshake->serverKeys, sizeof(Keys)); switch (ssh->encryptId) { case ID_NONE: @@ -3011,8 +3030,9 @@ int SendNewKeys(WOLFSSH* ssh) case ID_AES128_CBC: WLOG(WS_LOG_DEBUG, "SNK: using cipher aes128-cbc"); ret = wc_AesSetKey(&ssh->encryptCipher.aes, - ssh->encKeyServer, ssh->encKeyServerSz, - ssh->ivServer, AES_ENCRYPTION); + ssh->serverKeys.encKey, + ssh->serverKeys.encKeySz, + ssh->serverKeys.iv, AES_ENCRYPTION); break; default: diff --git a/src/ssh.c b/src/ssh.c index 1cddbdbd..9a731fde 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -125,23 +125,25 @@ void wolfSSH_CTX_free(WOLFSSH_CTX* ctx) static WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx) { - HandshakeInfo* handshake; - RNG* rng; + HandshakeInfo* handshake = NULL; + RNG* rng = NULL; + void* heap; WLOG(WS_LOG_DEBUG, "Entering SshInit()"); - if (ssh == NULL) + if (ssh == NULL || ctx == NULL) return ssh; + heap = ctx->heap; handshake = (HandshakeInfo*)WMALLOC(sizeof(HandshakeInfo), - ctx->heap, DYNTYPE_HS); - if (handshake == NULL) { - wolfSSH_free(ssh); - return NULL; - } + heap, DYNTYPE_HS); + rng = (RNG*)WMALLOC(sizeof(RNG), heap, DYNTYPE_RNG); - rng = (RNG*)WMALLOC(sizeof(RNG), ctx->heap, DYNTYPE_RNG); - if (rng == NULL || wc_InitRng(rng) != 0) { + if (handshake == NULL || rng == NULL || wc_InitRng(rng) != 0) { + + WLOG(WS_LOG_DEBUG, "SshInit: Cannot allocate memory.\n"); + WFREE(handshake, heap, DYNTYPE_HS); + WFREE(rng, heap, DYNTYPE_RNG); wolfSSH_free(ssh); return NULL; } @@ -220,9 +222,11 @@ static void SshResourceFree(WOLFSSH* ssh, void* heap) ShrinkBuffer(&ssh->outputBuffer, 1); ForceZero(ssh->k, ssh->kSz); if (ssh->handshake) { - WMEMSET(ssh->handshake, 0, sizeof(HandshakeInfo)); + ForceZero(ssh->handshake, sizeof(HandshakeInfo)); WFREE(ssh->handshake, heap, DYNTYPE_HS); } + ForceZero(&ssh->clientKeys, sizeof(Keys)); + ForceZero(&ssh->serverKeys, sizeof(Keys)); if (ssh->rng) { wc_FreeRng(ssh->rng); WFREE(ssh->rng, heap, DYNTYPE_RNG); @@ -244,11 +248,10 @@ static void SshResourceFree(WOLFSSH* ssh, void* heap) void wolfSSH_free(WOLFSSH* ssh) { - void* heap = ssh->ctx ? ssh->ctx->heap : NULL; - WLOG(WS_LOG_DEBUG, "Entering wolfSSH_free()"); if (ssh) { + void* heap = ssh->ctx ? ssh->ctx->heap : NULL; SshResourceFree(ssh, heap); WFREE(ssh, heap, DYNTYPE_SSH); } diff --git a/wolfssh/internal.h b/wolfssh/internal.h index 096c5283..268d5025 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -154,6 +154,16 @@ typedef struct Ciphers { } Ciphers; +typedef struct Keys { + uint8_t iv[AES_BLOCK_SIZE]; + uint8_t ivSz; + uint8_t encKey[AES_BLOCK_SIZE]; + uint8_t encKeySz; + uint8_t macKey[MAX_HMAC_SZ]; + uint8_t macKeySz; +} Keys; + + typedef struct HandshakeInfo { uint8_t kexId; uint8_t pubKeyId; @@ -164,6 +174,8 @@ typedef struct HandshakeInfo { uint8_t blockSz; uint8_t macSz; + Keys clientKeys; + Keys serverKeys; Sha hash; uint8_t e[257]; /* May have a leading zero, for unsigned. */ uint32_t eSz; @@ -223,19 +235,8 @@ struct WOLFSSH { uint8_t sessionId[SHA_DIGEST_SIZE]; uint32_t sessionIdSz; - uint8_t ivClient[AES_BLOCK_SIZE]; - uint8_t ivClientSz; - uint8_t ivServer[AES_BLOCK_SIZE]; - uint8_t ivServerSz; - uint8_t encKeyClient[AES_BLOCK_SIZE]; - uint8_t encKeyClientSz; - uint8_t encKeyServer[AES_BLOCK_SIZE]; - uint8_t encKeyServerSz; - uint8_t macKeyClient[MAX_HMAC_SZ]; - uint8_t macKeyClientSz; - uint8_t macKeyServer[MAX_HMAC_SZ]; - uint8_t macKeyServerSz; - + Keys clientKeys; + Keys serverKeys; HandshakeInfo* handshake; void* userAuthCtx;