From 8b713adcfd7f0b03bf713498d29d2de9967b9b29 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 14 Sep 2016 13:43:02 -0700 Subject: [PATCH] Extended Master Secret Peer Review Changes 1. Checked the returns on the hash functions in the sniffer, return new error if any fail. 2. Removed the SHA-512 hash from the sniffer's collection of hashes. Never used in a cipher suite. 3. Added some logging messages in the EMS support in wolfSSL. --- src/internal.c | 7 ++++++ src/sniffer.c | 53 +++++++++++++++++++++++----------------- wolfssl/sniffer_error.h | 1 + wolfssl/sniffer_error.rc | 1 + 4 files changed, 39 insertions(+), 23 deletions(-) diff --git a/src/internal.c b/src/internal.c index 618c4f6eb..c94b5c7fc 100755 --- a/src/internal.c +++ b/src/internal.c @@ -18313,10 +18313,15 @@ int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, /* RFC 7627, 5.3, server-side */ /* if old sess didn't have EMS, but new does, full handshake */ if (!session->haveEMS && ssl->options.haveEMS) { + WOLFSSL_MSG("Attempting to resume a session that didn't " + "use EMS with a new session with EMS. Do full " + "handshake."); ssl->options.resuming = 0; } /* if old sess used EMS, but new doesn't, MUST abort */ else if (session->haveEMS && !ssl->options.haveEMS) { + WOLFSSL_MSG("Trying to resume a session with EMS without " + "using EMS"); return EXT_MASTER_SECRET_NEEDED_E; } } @@ -18859,6 +18864,8 @@ int DoSessionTicket(WOLFSSL* ssl, const byte* input, word32* inOutIdx, /* get master secret */ if (ret == WOLFSSL_TICKET_RET_OK || ret == WOLFSSL_TICKET_RET_CREATE) { XMEMCPY(ssl->arrays->masterSecret, it->msecret, SECRET_LEN); + /* Copy the haveExtendedMasterSecret property from the ticket to + * the saved session, so the property may be checked later. */ ssl->session.haveEMS = it->haveEMS; } diff --git a/src/sniffer.c b/src/sniffer.c index d2d94e1a9..49627960b 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -254,7 +254,8 @@ static const char* const msgTable[] = "Clear ACK Fault", /* 81 */ - "Bad Decrypt Size" + "Bad Decrypt Size", + "Extended Master Secret Hash Error" }; @@ -360,9 +361,6 @@ typedef struct HsHashes { #ifdef WOLFSSL_SHA384 Sha384 hashSha384; #endif -#ifdef WOLFSSL_SHA512 - Sha512 hashSha512; -#endif } HsHashes; @@ -590,10 +588,6 @@ static int HashInit(HsHashes* hash) if (ret == 0) ret = wc_InitSha384(&hash->hashSha384); #endif -#ifdef WOLFSSL_SHA512 - if (ret == 0) - ret = wc_InitSha512(&hash->hashSha512); -#endif return ret; } @@ -624,10 +618,6 @@ static int HashUpdate(HsHashes* hash, const byte* input, int sz) if (ret == 0) ret = wc_Sha384Update(&hash->hashSha384, input, sz); #endif -#ifdef WOLFSSL_SHA512 - if (ret == 0) - ret = wc_Sha512Update(&hash->hashSha512, input, sz); -#endif return ret; } @@ -650,9 +640,6 @@ static int HashCopy(HS_Hashes* d, HsHashes* s) #ifdef WOLFSSL_SHA384 XMEMCPY(&d->hashSha384, &s->hashSha384, sizeof(Sha384)); #endif -#ifdef WOLFSSL_SHA512 - XMEMCPY(&d->hashSha512, &s->hashSha512, sizeof(Sha512)); -#endif return 0; } @@ -2077,8 +2064,13 @@ static int DoHandShake(const byte* input, int* sslBytes, } #ifdef HAVE_EXTENDED_MASTER - if (session->hash) - HashUpdate(session->hash, input, size); + if (session->hash) { + if (HashUpdate(session->hash, input, size) != 0) { + SetError(EXTENDED_MASTER_HASH_STR, error, + session, FATAL_ERROR_STATE); + return -1; + } + } #endif switch (type) { @@ -2123,10 +2115,20 @@ static int DoHandShake(const byte* input, int* sslBytes, Trace(GOT_CLIENT_KEY_EX_STR); #ifdef HAVE_EXTENDED_MASTER if (session->flags.expectEms && session->hash != NULL) { - HashCopy(session->sslServer->hsHashes, session->hash); - HashCopy(session->sslClient->hsHashes, session->hash); - session->sslServer->options.haveEMS = 1; - session->sslClient->options.haveEMS = 1; + if (HashCopy(session->sslServer->hsHashes, + session->hash) == 0 && + HashCopy(session->sslClient->hsHashes, + session->hash) == 0) { + + session->sslServer->options.haveEMS = 1; + session->sslClient->options.haveEMS = 1; + } + else { + SetError(EXTENDED_MASTER_HASH_STR, error, + session, FATAL_ERROR_STATE); + ret = -1; + } + XMEMSET(session->hash, 0, sizeof(HsHashes)); free(session->hash); session->hash = NULL; } @@ -2135,7 +2137,8 @@ static int DoHandShake(const byte* input, int* sslBytes, session->sslClient->options.haveEMS = 0; } #endif - ret = ProcessClientKeyExchange(input, sslBytes, session, error); + if (ret == 0) + ret = ProcessClientKeyExchange(input, sslBytes, session, error); break; case certificate_verify: Trace(GOT_CERT_VER_STR); @@ -2355,7 +2358,11 @@ static SnifferSession* CreateSession(IpInfo* ipInfo, TcpInfo* tcpInfo, free(session); return 0; } - HashInit(newHash); + if (HashInit(newHash) != 0) { + SetError(EXTENDED_MASTER_HASH_STR, error, NULL, 0); + free(session); + return 0; + } session->hash = newHash; } #endif diff --git a/wolfssl/sniffer_error.h b/wolfssl/sniffer_error.h index 327455ec2..0c04ba878 100644 --- a/wolfssl/sniffer_error.h +++ b/wolfssl/sniffer_error.h @@ -117,6 +117,7 @@ #define CLEAR_ACK_FAULT 80 #define BAD_DECRYPT_SIZE 81 +#define EXTENDED_MASTER_HASH_STR 82 /* !!!! also add to msgTable in sniffer.c and .rc file !!!! */ diff --git a/wolfssl/sniffer_error.rc b/wolfssl/sniffer_error.rc index e7d998059..947be6119 100644 --- a/wolfssl/sniffer_error.rc +++ b/wolfssl/sniffer_error.rc @@ -98,5 +98,6 @@ STRINGTABLE 80, "Clear ACK Fault" 81, "Bad Decrypt Size" + 82, "Extended Master Secret Hash Error" }