From ce1c1fe0a63275410bd1dce938dfb5008177c85a Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 15 Sep 2020 10:01:05 -0700 Subject: [PATCH 1/8] Fix for sniffer using `HAVE_MAX_FRAGMENT` in "certificate" type message. ZD 10903 --- src/sniffer.c | 99 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 86 insertions(+), 13 deletions(-) diff --git a/src/sniffer.c b/src/sniffer.c index 5658de58c..177d93b89 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -452,6 +452,11 @@ typedef struct SnifferSession { word32 srvReassemblyMemory; /* server packet memory used */ struct SnifferSession* next; /* for hash table list */ byte* ticketID; /* mac ID of session ticket */ +#ifdef HAVE_MAX_FRAGMENT + byte* tlsFragBuf; + word32 tlsFragOffset; + word32 tlsFragSize; +#endif #ifdef HAVE_SNI const char* sni; /* server name indication */ #endif @@ -648,6 +653,12 @@ static void FreeSnifferSession(SnifferSession* session) #ifdef WOLFSSL_TLS13 if (session->cliKeyShare) XFREE(session->cliKeyShare, NULL, DYNAMIC_TYPE_TMP_BUFFER); +#endif +#ifdef HAVE_MAX_FRAGMENT + if (session->tlsFragBuf) { + XFREE(session->tlsFragBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + session->tlsFragBuf = NULL; + } #endif } XFREE(session, NULL, DYNAMIC_TYPE_SNIFFER_SESSION); @@ -2743,6 +2754,24 @@ static int ProcessServerHello(int msgSz, const byte* input, int* sslBytes, session->sslClient->session.ticketNonce.len = 1; session->sslClient->session.ticketNonce.data[0] = 0; break; + #endif + #ifdef HAVE_MAX_FRAGMENT + case EXT_MAX_FRAGMENT_LENGTH: + { + word16 max_fragment = MAX_RECORD_SIZE; + switch (input[0]) { + case WOLFSSL_MFL_2_8 : max_fragment = 256; break; + case WOLFSSL_MFL_2_9 : max_fragment = 512; break; + case WOLFSSL_MFL_2_10: max_fragment = 1024; break; + case WOLFSSL_MFL_2_11: max_fragment = 2048; break; + case WOLFSSL_MFL_2_12: max_fragment = 4096; break; + case WOLFSSL_MFL_2_13: max_fragment = 8192; break; + default: break; + } + session->sslServer->max_fragment = max_fragment; + session->sslClient->max_fragment = max_fragment; + break; + } #endif case EXT_SUPPORTED_VERSIONS: session->sslServer->version.major = input[0]; @@ -3407,14 +3436,30 @@ static int ProcessFinished(const byte* input, int size, int* sslBytes, /* Process HandShake input */ static int DoHandShake(const byte* input, int* sslBytes, - SnifferSession* session, char* error) + SnifferSession* session, char* error, word16 rhSize) { byte type; int size; int ret = 0; - int startBytes; WOLFSSL* ssl; +#ifdef HAVE_MAX_FRAGMENT + if (session->tlsFragBuf) { + XMEMCPY(session->tlsFragBuf + session->tlsFragOffset, input, rhSize); + session->tlsFragOffset += rhSize; + *sslBytes -= rhSize; + + if (session->tlsFragOffset < session->tlsFragSize) { + return 0; + } + + /* reassembled complete fragment */ + input = session->tlsFragBuf; + *sslBytes = session->tlsFragSize; + rhSize = session->tlsFragSize; + } +#endif + if (*sslBytes < HANDSHAKE_HEADER_SZ) { SetError(HANDSHAKE_INPUT_STR, error, session, FATAL_ERROR_STATE); return -1; @@ -3424,7 +3469,6 @@ static int DoHandShake(const byte* input, int* sslBytes, input += HANDSHAKE_HEADER_SZ; *sslBytes -= HANDSHAKE_HEADER_SZ; - startBytes = *sslBytes; if (*sslBytes < size) { Trace(SPLIT_HANDSHAKE_MSG_STR); @@ -3449,6 +3493,30 @@ static int DoHandShake(const byte* input, int* sslBytes, } #endif +#ifdef HAVE_MAX_FRAGMENT + if (rhSize < size) { + /* partial fragment, let's reassemble */ + if (session->tlsFragBuf == NULL) { + session->tlsFragOffset = 0; + session->tlsFragSize = size + HANDSHAKE_HEADER_SZ; + session->tlsFragBuf = (byte*)XMALLOC(session->tlsFragSize, NULL, DYNAMIC_TYPE_TMP_BUFFER); + if (session->tlsFragBuf == NULL) { + SetError(MEMORY_STR, error, NULL, 0); + return 0; + } + + /* include the handshake header */ + input -= HANDSHAKE_HEADER_SZ; + *sslBytes += HANDSHAKE_HEADER_SZ; + } + + XMEMCPY(session->tlsFragBuf + session->tlsFragOffset, input, rhSize); + session->tlsFragOffset += rhSize; + *sslBytes -= rhSize; + return 0; + } +#endif + #ifdef WOLFSSL_TLS13 if (type != client_hello) { /* For resumption the hash is before / after client_hello PSK binder */ @@ -3465,7 +3533,8 @@ static int DoHandShake(const byte* input, int* sslBytes, if (HashUpdate(session->hash, input, size) != 0) { SetError(EXTENDED_MASTER_HASH_STR, error, session, FATAL_ERROR_STATE); - return -1; + ret = -1; + goto exit; } } #endif @@ -3560,10 +3629,17 @@ static int DoHandShake(const byte* input, int* sslBytes, break; default: SetError(GOT_UNKNOWN_HANDSHAKE_STR, error, session, 0); - return -1; + ret = -1; + break; } - *sslBytes = startBytes - size; /* actual bytes of full process */ +exit: +#ifdef HAVE_MAX_FRAGMENT + if (session->tlsFragBuf) { + XFREE(session->tlsFragBuf, NULL, DYNAMIC_TYPE_TMP_BUFFER); + session->tlsFragBuf = NULL; + } +#endif return ret; } @@ -4818,11 +4894,9 @@ doPart: switch ((enum ContentType)rh.type) { case handshake: { - int startIdx = sslBytes; - int used; - + int inOutIdx = sslBytes; Trace(GOT_HANDSHAKE_STR); - ret = DoHandShake(sslFrame, &sslBytes, session, error); + ret = DoHandShake(sslFrame, &inOutIdx, session, error, rhSize); if (ret != 0) { if (session->flags.fatalError == 0) SetError(BAD_HANDSHAKE_STR, error, session, @@ -4830,9 +4904,8 @@ doPart: return -1; } - /* DoHandShake now fully decrements sslBytes to remaining */ - used = startIdx - sslBytes; - sslFrame += used; + sslFrame += rhSize; + sslBytes -= rhSize; if (decrypted) sslFrame += ssl->keys.padSz; } From b5163bd1fae5ffcddb94436df8072b331cfb0190 Mon Sep 17 00:00:00 2001 From: David Garske Date: Tue, 15 Sep 2020 14:25:22 -0700 Subject: [PATCH 2/8] Added support for 802.11Q VLAN frames. Fix build error with unused "ret" when building with `WOLFSSL_SNIFFER_WATCH`. Fixed bad characters in sniffer README.md configure example. --- src/sniffer.c | 17 +++++++++++++++-- sslSniffer/README.md | 4 ++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/src/sniffer.c b/src/sniffer.c index 177d93b89..2b1e25f41 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -1906,7 +1906,7 @@ static int CheckIp6Hdr(Ip6Hdr* iphdr, IpInfo* info, int length, char* error) /* returns 0 on success, -1 on error */ static int CheckIpHdr(IpHdr* iphdr, IpInfo* info, int length, char* error) { - int version = IP_V(iphdr); + int version = IP_V(iphdr); if (version == IPV6) return CheckIp6Hdr((Ip6Hdr*)iphdr, info, length, error); @@ -3281,7 +3281,6 @@ static int KeyWatchCall(SnifferSession* session, const byte* data, int dataSz, static int ProcessCertificate(const byte* input, int* sslBytes, SnifferSession* session, char* error) { - int ret; const byte* certChain; word32 certChainSz; word32 certSz; @@ -4058,6 +4057,9 @@ int TcpChecksum(IpInfo* ipInfo, TcpInfo* tcpInfo, int dataLen, static int CheckHeaders(IpInfo* ipInfo, TcpInfo* tcpInfo, const byte* packet, int length, const byte** sslFrame, int* sslBytes, char* error) { + IpHdr* iphdr = (IpHdr*)packet; + int version; + TraceHeader(); TracePacket(); @@ -4066,6 +4068,17 @@ static int CheckHeaders(IpInfo* ipInfo, TcpInfo* tcpInfo, const byte* packet, SetError(PACKET_HDR_SHORT_STR, error, NULL, 0); return -1; } + + version = IP_V(iphdr); + if (version != IPV6 && version != IPV4) { + /* Is this VLAN IEEE 802.1Q Frame? TPID = 0x8100 */ + if (packet[2] == 0x81 && packet[3] == 0x00) { + /* trim VLAN header and try again */ + packet += 8; + length -= 8; + } + } + if (CheckIpHdr((IpHdr*)packet, ipInfo, length, error) != 0) return -1; diff --git a/sslSniffer/README.md b/sslSniffer/README.md index a9c48eecc..34d46c2ab 100644 --- a/sslSniffer/README.md +++ b/sslSniffer/README.md @@ -43,9 +43,9 @@ All options may be enabled with the following configure command line: ```sh ./configure --enable-sniffer \ - CPPFLAGS=”-DWOLFSSL_SNIFFER_STATS -DWOLFSSL_SNIFFER_WATCH \ + CPPFLAGS="-DWOLFSSL_SNIFFER_STATS -DWOLFSSL_SNIFFER_WATCH \ -DWOLFSSL_SNIFFER_STORE_DATA_CB -DWOLFSSL_SNIFFER_CHAIN_INPUT \ - -DSTARTTLS_ALLOWED” + -DSTARTTLS_ALLOWED" ``` To add some other cipher support to the sniffer, you can add options like: From 7e2d44ba9a2085396df9a8322aedc75f42ee5254 Mon Sep 17 00:00:00 2001 From: David Garske Date: Wed, 16 Sep 2020 09:51:09 -0700 Subject: [PATCH 3/8] Fix possible unused `rhSize`. --- src/sniffer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/sniffer.c b/src/sniffer.c index 2b1e25f41..89c744fbd 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -3442,6 +3442,8 @@ static int DoHandShake(const byte* input, int* sslBytes, int ret = 0; WOLFSSL* ssl; + (void)rhSize; + #ifdef HAVE_MAX_FRAGMENT if (session->tlsFragBuf) { XMEMCPY(session->tlsFragBuf + session->tlsFragOffset, input, rhSize); From adedde7d16e05b2ca4f0e378ffeea5c07cc4ed49 Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 21 Sep 2020 10:14:40 -0700 Subject: [PATCH 4/8] Fix to not treat cert/key not found as error in `myWatchCb` and `WOLFSSL_SNIFFER_WATCH`. The key can be pased as argument to `./snifftest` and if built with sniffer watch let's keep trying to parse instead of throwing an error. --- sslSniffer/sslSnifferTest/snifftest.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/sslSniffer/sslSnifferTest/snifftest.c b/sslSniffer/sslSnifferTest/snifftest.c index 2330f693a..ec4fc31a2 100644 --- a/sslSniffer/sslSnifferTest/snifftest.c +++ b/sslSniffer/sslSnifferTest/snifftest.c @@ -267,8 +267,11 @@ static int myWatchCb(void* vSniffer, certName = DEFAULT_SERVER_KEY_ECC; } - if (certName == NULL) - return -1; + if (certName == NULL) { + /* don't return error if key is not loaded */ + printf("Warning: No matching key found for cert hash\n"); + return 0; + } return ssl_SetWatchKey_file(vSniffer, certName, FILETYPE_PEM, NULL, error); } From bc960a9c258105ab0fa6e1ba4fd96e519a58c29e Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 21 Sep 2020 14:33:42 -0700 Subject: [PATCH 5/8] Fix for sniffer with SNI enabled to properly handle WOLFSSL_SUCCESS error code in `ProcessClientHello`. ZD 10926 --- src/sniffer.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/sniffer.c b/src/sniffer.c index 89c744fbd..fa27dba65 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -3000,6 +3000,10 @@ static int ProcessClientHello(const byte* input, int* sslBytes, } wc_UnLockMutex(&session->context->namedKeysMutex); } + if (ret > 0) { + /* make sure WOLFSSL_SUCCESS is converted to zero error code */ + ret = 0; + } } #endif From 7cfbc598ed2c984fb99db5c9f1adf255aaa65704 Mon Sep 17 00:00:00 2001 From: David Garske Date: Mon, 21 Sep 2020 17:53:13 -0700 Subject: [PATCH 6/8] Fix to not assume TLS v1.3 based on extended key share extension. --- src/sniffer.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/sniffer.c b/src/sniffer.c index fa27dba65..19ab310d2 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -2778,6 +2778,10 @@ static int ProcessServerHello(int msgSz, const byte* input, int* sslBytes, session->sslServer->version.minor = input[1]; session->sslClient->version.major = input[0]; session->sslClient->version.minor = input[1]; + if (IsAtLeastTLSv1_3(session->sslServer->version)) { + /* The server side handshake encryption is on for future packets */ + session->flags.serverCipherOn = 1; + } break; case EXT_MASTER_SECRET: #ifdef HAVE_EXTENDED_MASTER @@ -3131,9 +3135,6 @@ static int ProcessClientHello(const byte* input, int* sslBytes, break; } XMEMCPY(session->cliKeyShare, &input[2], ksLen); - - /* The server side handshake encryption is on for future packets */ - session->flags.serverCipherOn = 1; break; } #ifdef HAVE_SESSION_TICKET From bbaf4090b84f39639fca189327f074c94cccdbd9 Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 24 Sep 2020 15:03:26 -0700 Subject: [PATCH 7/8] Fixes for sniffer when using static ECC keys. Adds TLS v1.2 ECC key fallback detection and fixes new ECC RNG requirement for timing resistance. --- src/sniffer.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/src/sniffer.c b/src/sniffer.c index 19ab310d2..8f591fffa 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -2066,6 +2066,11 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, { word32 idx = 0; int ret; +#ifdef HAVE_ECC + int useEccCurveId = ECC_CURVE_DEF; + if (ksInfo && ksInfo->curve_id != 0) + useEccCurveId = ksInfo->curve_id; +#endif #ifndef NO_RSA /* Static RSA */ @@ -2085,6 +2090,11 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, SetError(RSA_DECODE_STR, error, session, 0); #endif } + #ifdef HAVE_ECC + else { + useEccCurveId = -1; /* don't try loading ECC */ + } + #endif } if (ret == 0) { @@ -2135,7 +2145,7 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, #endif /* !NO_RSA */ #if !defined(NO_DH) && defined(WOLFSSL_DH_EXTRA) - /* Static Ephemeral DH Key */ + /* Static DH Key */ if (ksInfo && ksInfo->dh_key_bits != 0) { DhKey dhKey; const DhParams* params; @@ -2223,8 +2233,8 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, #endif /* !NO_DH && WOLFSSL_DH_EXTRA */ #ifdef HAVE_ECC - /* Static Ephemeral ECC Key */ - if (ksInfo && ksInfo->curve_id != 0) { + /* Static ECC Key */ + if (useEccCurveId >= ECC_CURVE_DEF) { ecc_key key; ecc_key pubKey; int length, keyInit = 0, pubKeyInit = 0; @@ -2235,6 +2245,15 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, keyInit = 1; ret = wc_ecc_init(&pubKey); } + + #if defined(ECC_TIMING_RESISTANT) && (!defined(HAVE_FIPS) || \ + (!defined(HAVE_FIPS_VERSION) || (HAVE_FIPS_VERSION != 2))) && \ + !defined(HAVE_SELFTEST) + if (ret == 0) { + ret = wc_ecc_set_rng(&key, session->sslServer->rng); + } + #endif + if (ret == 0) { pubKeyInit = 1; ret = wc_EccPrivateKeyDecode(keyBuf->buffer, &idx, &key, keyBuf->length); @@ -2258,7 +2277,7 @@ static int SetupKeys(const byte* input, int* sslBytes, SnifferSession* session, } if (ret == 0) { - ret = wc_ecc_import_x963_ex(input, length, &pubKey, ksInfo->curve_id); + ret = wc_ecc_import_x963_ex(input, length, &pubKey, useEccCurveId); if (ret != 0) { SetError(ECC_PUB_DECODE_STR, error, session, FATAL_ERROR_STATE); } From 5ef5c279b5226487f1f8f499f882d91fb07ce19f Mon Sep 17 00:00:00 2001 From: David Garske Date: Thu, 24 Sep 2020 15:53:12 -0700 Subject: [PATCH 8/8] Fix for previous max fragment commit to correctly process a TLS packet with multiple handshake messages. Fix to free the wolfSSL objects first then wolfSSL_CTX. --- src/sniffer.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/src/sniffer.c b/src/sniffer.c index 8f591fffa..dfaec0538 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -677,14 +677,7 @@ void ssl_FreeSniffer(void) wc_LockMutex(&ServerListMutex); wc_LockMutex(&SessionMutex); - srv = ServerList; - while (srv) { - removeServer = srv; - srv = srv->next; - FreeSnifferServer(removeServer); - } - ServerList = NULL; - + /* Free sessions (wolfSSL objects) first */ for (i = 0; i < HASH_SIZE; i++) { session = SessionTable[i]; while (session) { @@ -695,6 +688,15 @@ void ssl_FreeSniffer(void) } SessionCount = 0; + /* Then server (wolfSSL_CTX) */ + srv = ServerList; + while (srv) { + removeServer = srv; + srv = srv->next; + FreeSnifferServer(removeServer); + } + ServerList = NULL; + wc_UnLockMutex(&SessionMutex); wc_UnLockMutex(&ServerListMutex); @@ -3465,6 +3467,7 @@ static int DoHandShake(const byte* input, int* sslBytes, int size; int ret = 0; WOLFSSL* ssl; + int startBytes; (void)rhSize; @@ -3494,6 +3497,7 @@ static int DoHandShake(const byte* input, int* sslBytes, input += HANDSHAKE_HEADER_SZ; *sslBytes -= HANDSHAKE_HEADER_SZ; + startBytes = *sslBytes; if (*sslBytes < size) { Trace(SPLIT_HANDSHAKE_MSG_STR); @@ -3666,6 +3670,8 @@ exit: } #endif + *sslBytes = startBytes - size; /* actual bytes of full process */ + return ret; } @@ -4933,18 +4939,21 @@ doPart: switch ((enum ContentType)rh.type) { case handshake: { - int inOutIdx = sslBytes; + int startIdx = sslBytes; + int used; + Trace(GOT_HANDSHAKE_STR); - ret = DoHandShake(sslFrame, &inOutIdx, session, error, rhSize); - if (ret != 0) { + ret = DoHandShake(sslFrame, &sslBytes, session, error, rhSize); + if (ret != 0 || sslBytes > startIdx) { if (session->flags.fatalError == 0) SetError(BAD_HANDSHAKE_STR, error, session, FATAL_ERROR_STATE); return -1; } - sslFrame += rhSize; - sslBytes -= rhSize; + /* DoHandShake now fully decrements sslBytes to remaining */ + used = startIdx - sslBytes; + sslFrame += used; if (decrypted) sslFrame += ssl->keys.padSz; }