From d2989d9f434918a1e0fdbe4e27a3efc364f86c0b Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 16 Nov 2018 10:53:01 -0800 Subject: [PATCH 1/4] Sniffer Fix Drop a handshake message if it is split across TLS records. The likely messages dropped are certificate and certificate request, which are ignored by the sniffer. --- src/sniffer.c | 8 +++++--- wolfssl/sniffer_error.h | 1 + wolfssl/sniffer_error.rc | 1 + 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/sniffer.c b/src/sniffer.c index 96c633fe5..462946030 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -245,7 +245,8 @@ static const char* const msgTable[] = /* 81 */ "Bad Decrypt Size", - "Extended Master Secret Hash Error" + "Extended Master Secret Hash Error", + "Handshake Message Split Across TLS Records" }; @@ -2060,8 +2061,9 @@ static int DoHandShake(const byte* input, int* sslBytes, startBytes = *sslBytes; if (*sslBytes < size) { - SetError(HANDSHAKE_INPUT_STR, error, session, FATAL_ERROR_STATE); - return -1; + Trace(SPLIT_HANDSHAKE_MSG_STR); + *sslBytes = 0; + return ret; } /* A session's arrays are released when the handshake is completed. */ diff --git a/wolfssl/sniffer_error.h b/wolfssl/sniffer_error.h index 8c813b198..0af7079a1 100644 --- a/wolfssl/sniffer_error.h +++ b/wolfssl/sniffer_error.h @@ -118,6 +118,7 @@ #define BAD_DECRYPT_SIZE 81 #define EXTENDED_MASTER_HASH_STR 82 +#define SPLIT_HANDSHAKE_MSG_STR 83 /* !!!! also add to msgTable in sniffer.c and .rc file !!!! */ diff --git a/wolfssl/sniffer_error.rc b/wolfssl/sniffer_error.rc index 947be6119..735e3184a 100644 --- a/wolfssl/sniffer_error.rc +++ b/wolfssl/sniffer_error.rc @@ -99,5 +99,6 @@ STRINGTABLE 81, "Bad Decrypt Size" 82, "Extended Master Secret Hash Error" + 83, "Handshake Message Split Across TLS Records" } From 6ee60bbb49dd2507d1153a0dc8926a299109e03d Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 16 Nov 2018 14:21:36 -0800 Subject: [PATCH 2/4] Sniffer Update 1. Adds a new function ssl_DecodePacketWithSessionInfo() that returns a copy of the TLS session info (version and suite ID) for the packet that is decoded. 2. Adds a new function DecodePacketInternal() that does the same work as the old DecodePacket() with the additional Session Info behavior. 3. Both DecodePacket public functions call the internal version. --- src/sniffer.c | 38 +++++++++++++++++++++++++++++++++++++- wolfssl/sniffer.h | 31 +++++++++++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/src/sniffer.c b/src/sniffer.c index 462946030..7e3803475 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -3467,7 +3467,8 @@ static int RemoveFatalSession(IpInfo* ipInfo, TcpInfo* tcpInfo, /* Passes in an IP/TCP packet for decoding (ethernet/localhost frame) removed */ /* returns Number of bytes on success, 0 for no data yet, and -1 on error */ -int ssl_DecodePacket(const byte* packet, int length, byte** data, char* error) +static int ssl_DecodePacketInternal(const byte* packet, int length, + byte** data, SSLInfo* sslInfo, char* error) { TcpInfo tcpInfo; IpInfo ipInfo; @@ -3477,6 +3478,9 @@ int ssl_DecodePacket(const byte* packet, int length, byte** data, char* error) int ret; SnifferSession* session = 0; + if (NULL != sslInfo) + XMEMSET(sslInfo, 0, sizeof(SSLInfo)); + if (CheckHeaders(&ipInfo, &tcpInfo, packet, length, &sslFrame, &sslBytes, error) != 0) return -1; @@ -3500,10 +3504,42 @@ int ssl_DecodePacket(const byte* packet, int length, byte** data, char* error) ret = ProcessMessage(sslFrame, session, sslBytes, data, end, error); if (RemoveFatalSession(&ipInfo, &tcpInfo, session, error)) return -1; CheckFinCapture(&ipInfo, &tcpInfo, session); + + /* Pass back Session Info after we have processed the Server Hello. */ + if ((NULL != sslInfo) && (0 != session->sslServer->options.cipherSuite)) { + sslInfo->isValid = 1; + sslInfo->protocolVersionMajor = session->sslServer->version.major; + sslInfo->protocolVersionMinor = session->sslServer->version.minor; + sslInfo->serverCipherSuite0 = session->sslServer->options.cipherSuite0; + sslInfo->serverCipherSuite = session->sslServer->options.cipherSuite; + + const char* pCipher = wolfSSL_get_cipher(session->sslServer); + if (pCipher) + XMEMCPY(sslInfo->serverCipherSuiteName, pCipher, + sizeof(sslInfo->serverCipherSuiteName) - 1); + } return ret; } +/* Passes in an IP/TCP packet for decoding (ethernet/localhost frame) removed */ +/* returns Number of bytes on success, 0 for no data yet, and -1 on error */ +/* Also returns Session Info if available */ +int ssl_DecodePacketWithSessionInfo(const unsigned char* packet, int length, + unsigned char** data, SSLInfo* sslInfo, char* error) +{ + return ssl_DecodePacketInternal(packet, length, data, sslInfo, error); +} + + +/* Passes in an IP/TCP packet for decoding (ethernet/localhost frame) removed */ +/* returns Number of bytes on success, 0 for no data yet, and -1 on error */ +int ssl_DecodePacket(const byte* packet, int length, byte** data, char* error) +{ + return ssl_DecodePacketInternal(packet, length, data, NULL, error); +} + + /* Deallocator for the decoded data buffer. */ /* returns 0 on success, -1 on error */ int ssl_FreeDecodeBuffer(byte** data, char* error) diff --git a/wolfssl/sniffer.h b/wolfssl/sniffer.h index 4bd9f42d7..2595d23be 100644 --- a/wolfssl/sniffer.h +++ b/wolfssl/sniffer.h @@ -93,6 +93,37 @@ enum { }; +/* + * New Sniffer API that provides read-only access to the TLS and cipher + * information associated with the SSL session. + */ + +#if defined(__GNUC__) + #define WOLFSSL_PACK __attribute__ ((packed)) +#else + #define WOLFSSL_PACK +#endif + + +typedef struct SSLInfo +{ + unsigned char isValid; + /* indicates if the info in this struct is valid: 0 = no, 1 = yes */ + unsigned char protocolVersionMajor; /* SSL Version: major */ + unsigned char protocolVersionMinor; /* SSL Version: minor */ + unsigned char serverCipherSuite0; /* first byte, normally 0 */ + unsigned char serverCipherSuite; /* second byte, actual suite */ + unsigned char serverCipherSuiteName[256]; + /* cipher name, e.g., "TLS_RSA_..." */ +} WOLFSSL_PACK SSLInfo; + + +WOLFSSL_API +SSL_SNIFFER_API int ssl_DecodePacketWithSessionInfo( + const unsigned char* packet, int length, + unsigned char** data, SSLInfo* sslInfo, char* error); + + #ifdef __cplusplus } /* extern "C" */ #endif From 3599798aac0b89ca8128970253a3489ee0406a6d Mon Sep 17 00:00:00 2001 From: John Safranek Date: Fri, 16 Nov 2018 15:54:19 -0800 Subject: [PATCH 3/4] Move a variable declaration to the start of a block instead of in the middle. --- src/sniffer.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/sniffer.c b/src/sniffer.c index 7e3803475..9aeb42e91 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -3507,14 +3507,16 @@ static int ssl_DecodePacketInternal(const byte* packet, int length, /* Pass back Session Info after we have processed the Server Hello. */ if ((NULL != sslInfo) && (0 != session->sslServer->options.cipherSuite)) { + const char* pCipher; + sslInfo->isValid = 1; sslInfo->protocolVersionMajor = session->sslServer->version.major; sslInfo->protocolVersionMinor = session->sslServer->version.minor; sslInfo->serverCipherSuite0 = session->sslServer->options.cipherSuite0; sslInfo->serverCipherSuite = session->sslServer->options.cipherSuite; - const char* pCipher = wolfSSL_get_cipher(session->sslServer); - if (pCipher) + pCipher = wolfSSL_get_cipher(session->sslServer); + if (NULL != pCipher) XMEMCPY(sslInfo->serverCipherSuiteName, pCipher, sizeof(sslInfo->serverCipherSuiteName) - 1); } From 96b4ddad82969cecabd1c20df52b88a16235103d Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 21 Nov 2018 11:29:28 -0800 Subject: [PATCH 4/4] Sniffer Update 1. Collect the SSL Info capture into its own function. 2. Add a Trace function for the SSL Info. 3. When copying the IANA name for the cipher suite, use a strncpy instead of a memcpy and cap the copy at the length of the destination. Force a null terminator at the end of the destination, just in case. 4. Modify the snifftest to collect the SSL Info. --- src/sniffer.c | 67 ++++++++++++++++++++------- sslSniffer/sslSnifferTest/snifftest.c | 4 +- 2 files changed, 53 insertions(+), 18 deletions(-) diff --git a/src/sniffer.c b/src/sniffer.c index 9aeb42e91..5db8f423c 100644 --- a/src/sniffer.c +++ b/src/sniffer.c @@ -1017,6 +1017,23 @@ static void TraceRemovedSession(void) } +/* Show SSLInfo if provided and is valid. */ +static void TraceSessionInfo(SSLInfo* sslInfo) +{ + if (TraceOn) { + if (sslInfo != NULL && sslInfo->isValid) { + fprintf(TraceFile, + "\tver:(%u %u) suiteId:(%02x %02x) suiteName:(%s)\n", + sslInfo->protocolVersionMajor, + sslInfo->protocolVersionMinor, + sslInfo->serverCipherSuite0, + sslInfo->serverCipherSuite, + sslInfo->serverCipherSuiteName); + } + } +} + + /* Set user error string */ static void SetError(int idx, char* error, SnifferSession* session, int fatal) { @@ -3465,6 +3482,38 @@ static int RemoveFatalSession(IpInfo* ipInfo, TcpInfo* tcpInfo, } +/* Copies the session's infomation to the provided sslInfo. Skip copy if + * SSLInfo is not provided. */ +static void CopySessionInfo(SnifferSession* session, SSLInfo* sslInfo) +{ + if (NULL != sslInfo) { + XMEMSET(sslInfo, 0, sizeof(SSLInfo)); + + /* Pass back Session Info after we have processed the Server Hello. */ + if (0 != session->sslServer->options.cipherSuite) { + const char* pCipher; + + sslInfo->isValid = 1; + sslInfo->protocolVersionMajor = session->sslServer->version.major; + sslInfo->protocolVersionMinor = session->sslServer->version.minor; + sslInfo->serverCipherSuite0 = + session->sslServer->options.cipherSuite0; + sslInfo->serverCipherSuite = + session->sslServer->options.cipherSuite; + + pCipher = wolfSSL_get_cipher(session->sslServer); + if (NULL != pCipher) { + XSTRNCPY((char*)sslInfo->serverCipherSuiteName, pCipher, + sizeof(sslInfo->serverCipherSuiteName)); + sslInfo->serverCipherSuiteName + [sizeof(sslInfo->serverCipherSuiteName) - 1] = '\0'; + } + TraceSessionInfo(sslInfo); + } + } +} + + /* Passes in an IP/TCP packet for decoding (ethernet/localhost frame) removed */ /* returns Number of bytes on success, 0 for no data yet, and -1 on error */ static int ssl_DecodePacketInternal(const byte* packet, int length, @@ -3478,9 +3527,6 @@ static int ssl_DecodePacketInternal(const byte* packet, int length, int ret; SnifferSession* session = 0; - if (NULL != sslInfo) - XMEMSET(sslInfo, 0, sizeof(SSLInfo)); - if (CheckHeaders(&ipInfo, &tcpInfo, packet, length, &sslFrame, &sslBytes, error) != 0) return -1; @@ -3505,21 +3551,8 @@ static int ssl_DecodePacketInternal(const byte* packet, int length, if (RemoveFatalSession(&ipInfo, &tcpInfo, session, error)) return -1; CheckFinCapture(&ipInfo, &tcpInfo, session); - /* Pass back Session Info after we have processed the Server Hello. */ - if ((NULL != sslInfo) && (0 != session->sslServer->options.cipherSuite)) { - const char* pCipher; + CopySessionInfo(session, sslInfo); - sslInfo->isValid = 1; - sslInfo->protocolVersionMajor = session->sslServer->version.major; - sslInfo->protocolVersionMinor = session->sslServer->version.minor; - sslInfo->serverCipherSuite0 = session->sslServer->options.cipherSuite0; - sslInfo->serverCipherSuite = session->sslServer->options.cipherSuite; - - pCipher = wolfSSL_get_cipher(session->sslServer); - if (NULL != pCipher) - XMEMCPY(sslInfo->serverCipherSuiteName, pCipher, - sizeof(sslInfo->serverCipherSuiteName) - 1); - } return ret; } diff --git a/sslSniffer/sslSnifferTest/snifftest.c b/sslSniffer/sslSnifferTest/snifftest.c index a2aec78ef..998d1d7b8 100644 --- a/sslSniffer/sslSnifferTest/snifftest.c +++ b/sslSniffer/sslSnifferTest/snifftest.c @@ -295,6 +295,7 @@ int main(int argc, char** argv) static int packetNumber = 0; struct pcap_pkthdr header; const unsigned char* packet = pcap_next(pcap, &header); + SSLInfo sslInfo; packetNumber++; if (packet) { @@ -307,7 +308,8 @@ int main(int argc, char** argv) else continue; - ret = ssl_DecodePacket(packet, header.caplen, &data, err); + ret = ssl_DecodePacketWithSessionInfo(packet, header.caplen, &data, + &sslInfo, err); if (ret < 0) { printf("ssl_Decode ret = %d, %s\n", ret, err); hadBadPacket = 1;