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

@@ -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;
         }
 

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

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 !!!! */
 
 

wolfssl/sniffer_error.rc

@@ -98,5 +98,6 @@ STRINGTABLE
     80, "Clear ACK Fault"
 
     81, "Bad Decrypt Size"
+    82, "Extended Master Secret Hash Error"
 }