From 7123b080eda7310be132872349ee7ae154f27a28 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 27 Apr 2016 12:03:37 -0700 Subject: [PATCH 1/4] fix issue with missing client key exchange and duplicate change cipher spec messages. --- src/internal.c | 66 +++++++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 28 deletions(-) diff --git a/src/internal.c b/src/internal.c index b0d0b7f66..49fc511ff 100755 --- a/src/internal.c +++ b/src/internal.c @@ -6211,7 +6211,10 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type) WOLFSSL_MSG("Duplicate ChangeCipher received"); return DUPLICATE_MSG_E; } - ssl->msgsReceived.got_change_cipher = 1; + /* DTLS is going to ignore the CCS message if the client key + * exchange message wasn't received yet. */ + if (!ssl->options.dtls) + ssl->msgsReceived.got_change_cipher = 1; #ifndef NO_WOLFSSL_CLIENT if (ssl->options.side == WOLFSSL_CLIENT_END) { @@ -6231,7 +6234,8 @@ static int SanityCheckMsgReceived(WOLFSSL* ssl, byte type) } } #endif - + if (ssl->options.dtls) + ssl->msgsReceived.got_change_cipher = 1; break; default: @@ -8028,8 +8032,12 @@ int ProcessReply(WOLFSSL* ssl) if (ssl->options.dtls && ret == SEQUENCE_ERROR) { WOLFSSL_MSG("Silently dropping out of order DTLS message"); ssl->options.processReply = doProcessInit; - ssl->buffers.inputBuffer.length = 0; - ssl->buffers.inputBuffer.idx = 0; + ssl->buffers.inputBuffer.idx += ssl->curSize; + + ret = DtlsPoolSend(ssl); + if (ret != 0) + return ret; + continue; } #endif @@ -8161,31 +8169,33 @@ int ProcessReply(WOLFSSL* ssl) } #endif -#ifdef WOLFSSL_DTLS - /* Check for duplicate CCS message in DTLS mode. - * DTLS allows for duplicate messages, and it should be - * skipped. */ - if (ssl->options.dtls && - ssl->msgsReceived.got_change_cipher) { - - WOLFSSL_MSG("Duplicate ChangeCipher msg"); - ret = DtlsPoolSend(ssl); - if (ret != 0) - return ret; - - if (ssl->curSize != 1) { - WOLFSSL_MSG("Malicious or corrupted" - " duplicate ChangeCipher msg"); - return LENGTH_ERROR; - } - ssl->buffers.inputBuffer.idx++; - break; - } -#endif - ret = SanityCheckMsgReceived(ssl, change_cipher_hs); - if (ret != 0) - return ret; + if (ret != 0) { + if (!ssl->options.dtls) { + return ret; + } +#ifdef WOLFSSL_DTLS + else { + /* Check for duplicate CCS message in DTLS mode. + * DTLS allows for duplicate messages, and it should be + * skipped. Also skip if out of order. */ + if (ret != DUPLICATE_MSG_E && ret != OUT_OF_ORDER_E) + return ret; + + ret = DtlsPoolSend(ssl); + if (ret != 0) + return ret; + + if (ssl->curSize != 1) { + WOLFSSL_MSG("Malicious or corrupted" + " duplicate ChangeCipher msg"); + return LENGTH_ERROR; + } + ssl->buffers.inputBuffer.idx++; + break; + } +#endif /* WOLFSSL_DTLS */ + } #ifdef HAVE_SESSION_TICKET if (ssl->options.side == WOLFSSL_CLIENT_END && From 0511c8cac8eebdda10607b585a563d3a9310ac0d Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 27 Apr 2016 14:04:47 -0700 Subject: [PATCH 2/4] delay check of DTLS handshake message's RH version until the handshake header check --- src/internal.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/internal.c b/src/internal.c index 49fc511ff..31ff1879e 100755 --- a/src/internal.c +++ b/src/internal.c @@ -3982,11 +3982,9 @@ static int GetRecordHeader(WOLFSSL* ssl, const byte* input, word32* inOutIdx, ssl->options.downgrade && ssl->options.connectState < FIRST_REPLY_DONE) WOLFSSL_MSG("Server attempting to accept with different version"); - else if (ssl->options.dtls - && (ssl->options.acceptState == ACCEPT_BEGIN - || ssl->options.acceptState == CLIENT_HELLO_SENT)) - /* Do not check version until Server Hello or Hello Again (2) */ - WOLFSSL_MSG("Use version for formatting only in DTLS till "); + else if (ssl->options.dtls && rh->type == handshake) + /* Check the DTLS handshake message RH version later. */ + WOLFSSL_MSG("DTLS handshake, skip RH version number check"); else { WOLFSSL_MSG("SSL version error"); return VERSION_ERROR; /* only use requested version */ @@ -4064,6 +4062,15 @@ static int GetDtlsHandShakeHeader(WOLFSSL* ssl, const byte* input, idx += DTLS_HANDSHAKE_FRAG_SZ; c24to32(input + idx, fragSz); + if (ssl->curRL.pvMajor != ssl->version.major || + ssl->curRL.pvMinor != ssl->version.minor) { + + if (*type != client_hello && *type != hello_verify_request) + return VERSION_ERROR; + else + WOLFSSL_MSG("DTLS Handshake ignoring hello or " + "hello verify version"); + } return 0; } #endif From e0c7739fd60500bc8d36819bd4d7c5a4f4cc9e2c Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 28 Apr 2016 10:28:57 -0700 Subject: [PATCH 3/4] fix bug with non-blocking DTLS where the stored peer messages were deleted after a timeout --- src/ssl.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/ssl.c b/src/ssl.c index b433bc034..e83b06401 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -6163,8 +6163,6 @@ int wolfSSL_dtls_got_timeout(WOLFSSL* ssl) { int result = SSL_SUCCESS; - DtlsMsgListDelete(ssl->dtls_msg_list, ssl->heap); - ssl->dtls_msg_list = NULL; if (DtlsPoolTimeout(ssl) < 0 || DtlsPoolSend(ssl) < 0) { result = SSL_FATAL_ERROR; } From 2f05c96004f14b735b9930922257035929974885 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Thu, 28 Apr 2016 11:33:29 -0700 Subject: [PATCH 4/4] added braces to else clause for compiler warning differences --- src/internal.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/internal.c b/src/internal.c index 31ff1879e..125ea02e6 100755 --- a/src/internal.c +++ b/src/internal.c @@ -4067,9 +4067,9 @@ static int GetDtlsHandShakeHeader(WOLFSSL* ssl, const byte* input, if (*type != client_hello && *type != hello_verify_request) return VERSION_ERROR; - else - WOLFSSL_MSG("DTLS Handshake ignoring hello or " - "hello verify version"); + else { + WOLFSSL_MSG("DTLS Handshake ignoring hello or verify version"); + } } return 0; }