From 6711756b03b0f462035f33fcd83cc4bbe4377cc9 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Tue, 19 Jul 2022 14:56:52 +0200 Subject: [PATCH 1/6] dtls13: support stream-based medium Don't assume that the underlying medium of DTLS provides the full message in a single operation. This is usually true for message-based socket (eg. using UDP) and false for stream-based socket (eg. using TCP). Commit changes: - Do not error out if we don't have the full message while parsing the header. - Do not assume that the record header is still in the buffer when decrypting the message. - Try to get more data if we didn't read the full DTLS header. --- src/dtls13.c | 26 ++++++++++++++++++++------ src/internal.c | 35 +++++++++++++++++++++++++++-------- wolfssl/internal.h | 5 +++-- 3 files changed, 50 insertions(+), 16 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index bd76ffe52..aeaaa3285 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -1190,6 +1190,26 @@ int Dtls13ReconstructEpochNumber(WOLFSSL* ssl, byte epochBits, return SEQUENCE_ERROR; } +int Dtls13GetUnifiedHeaderSize(const byte input, word16* size) +{ + if (size == NULL) + return BAD_FUNC_ARG; + + if (input & DTLS13_CID_BIT) { + WOLFSSL_MSG("DTLS1.3 header with connection ID. Not supported"); + return WOLFSSL_NOT_IMPLEMENTED; + } + + /* flags (1) + seq 8bit (1) */ + *size = OPAQUE8_LEN + OPAQUE8_LEN; + if (input & DTLS13_SEQ_LEN_BIT) + *size += OPAQUE8_LEN; + if (input & DTLS13_LEN_BIT) + *size += OPAQUE16_LEN; + + return 0; +} + /** * Dtls13ParseUnifiedRecordLayer() - parse DTLS unified header * @ssl: [in] ssl object @@ -1236,10 +1256,6 @@ int Dtls13ParseUnifiedRecordLayer(WOLFSSL* ssl, const byte* input, ato16(input + idx, &hdrInfo->recordLength); idx += DTLS13_LEN_SIZE; - - /* DTLS message must fit inside a datagram */ - if (inputSize < idx + hdrInfo->recordLength) - return LENGTH_ERROR; } else { /* length not present. The size of the record is the all the remaining @@ -1259,8 +1275,6 @@ int Dtls13ParseUnifiedRecordLayer(WOLFSSL* ssl, const byte* input, if (ret != 0) return ret; - hdrInfo->headerLength = idx; - if (seqLen == DTLS13_SEQ_16_LEN) { hdrInfo->seqHiPresent = 1; hdrInfo->seqHi = seqNum[0]; diff --git a/src/internal.c b/src/internal.c index 57c4fffc9..dd2af5d13 100644 --- a/src/internal.c +++ b/src/internal.c @@ -9675,6 +9675,7 @@ int CheckAvailableSize(WOLFSSL *ssl, int size) } #ifdef WOLFSSL_DTLS13 +static int GetInputData(WOLFSSL *ssl, word32 size); static int GetDtls13RecordHeader(WOLFSSL* ssl, const byte* input, word32* inOutIdx, RecordLayerHeader* rh, word16* size) { @@ -9687,6 +9688,9 @@ static int GetDtls13RecordHeader(WOLFSSL* ssl, const byte* input, readSize = ssl->buffers.inputBuffer.length - *inOutIdx; + if (readSize < DTLS_UNIFIED_HEADER_MIN_SZ) + return BUFFER_ERROR; + epochBits = *input & EE_MASK; ret = Dtls13ReconstructEpochNumber(ssl, epochBits, &epochNumber); if (ret != 0) @@ -9718,6 +9722,20 @@ static int GetDtls13RecordHeader(WOLFSSL* ssl, const byte* input, return SEQUENCE_ERROR; } + ret = Dtls13GetUnifiedHeaderSize( + *(input+*inOutIdx), &ssl->dtls13CurRlLength); + if (ret != 0) + return ret; + + if (readSize < ssl->dtls13CurRlLength) { + /* when using DTLS over a medium that does not guarantee that a full + * message is received in a single read, we may end up without the full + * header */ + ret = GetInputData(ssl, ssl->dtls13CurRlLength - readSize); + if (ret != 0) + return ret; + } + ret = Dtls13ParseUnifiedRecordLayer(ssl, input + *inOutIdx, readSize, &hdrInfo); @@ -9745,8 +9763,8 @@ static int GetDtls13RecordHeader(WOLFSSL* ssl, const byte* input, ssl->keys.curSeq); #endif /* WOLFSSL_DEBUG_TLS */ - *inOutIdx += hdrInfo.headerLength; - ssl->dtls13CurRlLength = hdrInfo.headerLength; + XMEMCPY(ssl->dtls13CurRL, input + *inOutIdx, ssl->dtls13CurRlLength); + *inOutIdx += ssl->dtls13CurRlLength; return 0; } @@ -9793,10 +9811,12 @@ static int GetDtlsRecordHeader(WOLFSSL* ssl, const byte* input, } /* not a unified header, check that we have at least - DTLS_RECORD_HEADER_SZ */ - if (read_size < DTLS_RECORD_HEADER_SZ) - return LENGTH_ERROR; - + * DTLS_RECORD_HEADER_SZ */ + if (read_size < DTLS_RECORD_HEADER_SZ) { + ret = GetInputData(ssl, DTLS_RECORD_HEADER_SZ - read_size); + if (ret != 0) + return LENGTH_ERROR; + } #endif /* WOLFSSL_DTLS13 */ /* type and version in same spot */ @@ -18466,8 +18486,7 @@ int ProcessReplyEx(WOLFSSL* ssl, int allowSocketErr) #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { /* aad now points to the record header */ - aad = in->buffer + - in->idx - ssl->dtls13CurRlLength; + aad = ssl->dtls13CurRL; aad_size = ssl->dtls13CurRlLength; } #endif /* WOLFSSL_DTLS13 */ diff --git a/wolfssl/internal.h b/wolfssl/internal.h index 2a1a5f811..800352fd5 100644 --- a/wolfssl/internal.h +++ b/wolfssl/internal.h @@ -1314,6 +1314,7 @@ enum Misc { DTLS_HANDSHAKE_HEADER_SZ = 12, /* normal + seq(2) + offset(3) + length(3) */ DTLS_RECORD_HEADER_SZ = 13, /* normal + epoch(2) + seq_num(6) */ DTLS_UNIFIED_HEADER_MIN_SZ = 2, + DTLS_RECVD_RL_HEADER_MAX_SZ = 5, /* flags + seq_number(2) + length(20) */ DTLS_RECORD_HEADER_MAX_SZ = 13, DTLS_HANDSHAKE_EXTRA = 8, /* diff from normal */ DTLS_RECORD_EXTRA = 8, /* diff from normal */ @@ -4368,7 +4369,6 @@ typedef enum EarlyDataState { typedef struct Dtls13UnifiedHdrInfo { word16 recordLength; - word16 headerLength; byte seqLo; byte seqHi; byte seqHiPresent:1; @@ -4658,7 +4658,7 @@ struct WOLFSSL { Dtls13Epoch *dtls13DecryptEpoch; w64wrapper dtls13Epoch; w64wrapper dtls13PeerEpoch; - + byte dtls13CurRL[DTLS_RECVD_RL_HEADER_MAX_SZ]; word16 dtls13CurRlLength; /* used to store the message if it needs to be fragmented */ @@ -5453,6 +5453,7 @@ WOLFSSL_LOCAL int Dtls13RlAddPlaintextHeader(WOLFSSL* ssl, byte* out, WOLFSSL_LOCAL int Dtls13EncryptRecordNumber(WOLFSSL* ssl, byte* hdr, word16 recordLength); WOLFSSL_LOCAL int Dtls13IsUnifiedHeader(byte header_flags); +WOLFSSL_LOCAL int Dtls13GetUnifiedHeaderSize(const byte input, word16* size); WOLFSSL_LOCAL int Dtls13ParseUnifiedRecordLayer(WOLFSSL* ssl, const byte* input, word16 input_size, Dtls13UnifiedHdrInfo* hdrInfo); WOLFSSL_LOCAL int Dtls13HandshakeSend(WOLFSSL* ssl, byte* output, From 11dfb713e9ebb50821e428f5d0b487a5a4ca7b80 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Tue, 19 Jul 2022 16:32:38 +0200 Subject: [PATCH 2/6] openssl_compatible_default: use DTLSv1.0 as minDowngrade in DTLS --- src/ssl.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ssl.c b/src/ssl.c index 91a452132..c481726db 100644 --- a/src/ssl.c +++ b/src/ssl.c @@ -522,7 +522,8 @@ WOLFSSL_CTX* wolfSSL_CTX_new_ex(WOLFSSL_METHOD* method, void* heap) wolfSSL_CTX_set_verify(ctx, SSL_VERIFY_NONE, NULL); wolfSSL_CTX_set_mode(ctx, SSL_MODE_AUTO_RETRY); if (wolfSSL_CTX_set_min_proto_version(ctx, - SSL3_VERSION) != WOLFSSL_SUCCESS || + (method->version.major == DTLS_MAJOR) ? + DTLS1_VERSION : SSL3_VERSION) != WOLFSSL_SUCCESS || #ifdef HAVE_ANON wolfSSL_CTX_allow_anon_cipher(ctx) != WOLFSSL_SUCCESS || #endif From c0fc87342c3248880f683bb37a62037a0891d054 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Tue, 19 Jul 2022 22:19:40 +0200 Subject: [PATCH 3/6] tls13: avoid spurious state advances in connect/accept state machine --- src/dtls13.c | 2 +- src/tls13.c | 27 +++++++++++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index aeaaa3285..36d329e50 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -426,7 +426,7 @@ static int Dtls13SendFragFromBuffer(WOLFSSL* ssl, byte* output, word16 length) static int Dtls13SendNow(WOLFSSL* ssl, enum HandShakeType handshakeType) { - if (!ssl->options.groupMessages) + if (!ssl->options.groupMessages || ssl->dtls13SendingFragments) return 1; if (handshakeType == client_hello || handshakeType == hello_retry_request || diff --git a/src/tls13.c b/src/tls13.c index dec6d88c4..1f4819c51 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -3339,6 +3339,7 @@ int SendTls13ClientHello(WOLFSSL* ssl) WOLFSSL_START(WC_FUNC_CLIENT_HELLO_SEND); WOLFSSL_ENTER("SendTls13ClientHello"); + ssl->options.buildingMsg = 1; major = SSLv3_MAJOR; tls12minor = TLSv1_2_MINOR; @@ -3613,6 +3614,7 @@ int SendTls13ClientHello(WOLFSSL* ssl) } #endif + ssl->options.buildingMsg = 0; #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { ret = Dtls13HandshakeSend(ssl, args->output, args->sendSz, @@ -5615,6 +5617,7 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType) return ret; } + ssl->options.buildingMsg = 1; #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) idx = DTLS_RECORD_HEADER_SZ + DTLS_HANDSHAKE_HEADER_SZ; @@ -5724,6 +5727,7 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType) if (extMsgType == server_hello) ssl->options.serverState = SERVER_HELLO_COMPLETE; + ssl->options.buildingMsg = 0; #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { ret = Dtls13HandshakeSend(ssl, output, sendSz, sendSz, @@ -5765,6 +5769,7 @@ static int SendTls13EncryptedExtensions(WOLFSSL* ssl) WOLFSSL_START(WC_FUNC_ENCRYPTED_EXTENSIONS_SEND); WOLFSSL_ENTER("SendTls13EncryptedExtensions"); + ssl->options.buildingMsg = 1; ssl->keys.encryptionOn = 1; #ifdef WOLFSSL_DTLS13 @@ -5858,6 +5863,7 @@ static int SendTls13EncryptedExtensions(WOLFSSL* ssl) #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { + ssl->options.buildingMsg = 0; ret = Dtls13HandshakeSend(ssl, output, sendSz, idx, encrypted_extensions, 1); @@ -5878,7 +5884,7 @@ static int SendTls13EncryptedExtensions(WOLFSSL* ssl) return sendSz; ssl->buffers.outputBuffer.length += sendSz; - + ssl->options.buildingMsg = 0; ssl->options.serverState = SERVER_ENCRYPTED_EXTENSIONS_COMPLETE; if (!ssl->options.groupMessages) @@ -5915,6 +5921,8 @@ static int SendTls13CertificateRequest(WOLFSSL* ssl, byte* reqCtx, WOLFSSL_START(WC_FUNC_CERTIFICATE_REQUEST_SEND); WOLFSSL_ENTER("SendTls13CertificateRequest"); + ssl->options.buildingMsg = 1; + if (ssl->options.side == WOLFSSL_SERVER_END) InitSuitesHashSigAlgo(ssl->suites, 1, 1, 1, 0, 1, ssl->buffers.keySz); @@ -5966,6 +5974,7 @@ static int SendTls13CertificateRequest(WOLFSSL* ssl, byte* reqCtx, #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { + ssl->options.buildingMsg = 0; ret = Dtls13HandshakeSend(ssl, output, sendSz, i, certificate_request, 1); @@ -5993,6 +6002,7 @@ static int SendTls13CertificateRequest(WOLFSSL* ssl, byte* reqCtx, #endif ssl->buffers.outputBuffer.length += sendSz; + ssl->options.buildingMsg = 0; if (!ssl->options.groupMessages) ret = SendBuffered(ssl); @@ -6498,6 +6508,8 @@ static int SendTls13Certificate(WOLFSSL* ssl) WOLFSSL_START(WC_FUNC_CERTIFICATE_SEND); WOLFSSL_ENTER("SendTls13Certificate"); + ssl->options.buildingMsg = 1; + #ifdef WOLFSSL_POST_HANDSHAKE_AUTH if (ssl->options.side == WOLFSSL_CLIENT_END && ssl->certReqCtx != NULL) { certReqCtxLen = ssl->certReqCtx->len; @@ -6599,8 +6611,6 @@ static int SendTls13Certificate(WOLFSSL* ssl) } #endif /* WOLFSSL_DTLS13 */ - ssl->options.buildingMsg = 1; - if (ssl->fragOffset == 0) { if (headerSz + certSz + extSz + certChainSz <= maxFragment - HANDSHAKE_HEADER_SZ) { @@ -6716,6 +6726,7 @@ static int SendTls13Certificate(WOLFSSL* ssl) #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { /* DTLS1.3 uses a separate variable and logic for fragments */ + ssl->options.buildingMsg = 0; ssl->fragOffset = 0; ret = Dtls13HandshakeSend(ssl, output, sendSz, i, certificate, 1); } @@ -6739,6 +6750,7 @@ static int SendTls13Certificate(WOLFSSL* ssl) #endif ssl->buffers.outputBuffer.length += sendSz; + ssl->options.buildingMsg = 0; if (!ssl->options.groupMessages) ret = SendBuffered(ssl); } @@ -6822,6 +6834,8 @@ static int SendTls13CertificateVerify(WOLFSSL* ssl) WOLFSSL_START(WC_FUNC_CERTIFICATE_VERIFY_SEND); WOLFSSL_ENTER("SendTls13CertificateVerify"); + ssl->options.buildingMsg = 1; + #if defined(WOLFSSL_RENESAS_TSIP_TLS) && (WOLFSSL_RENESAS_TSIP_VER >= 115) ret = tsip_Tls13SendCertVerify(ssl); if (ret != CRYPTOCB_UNAVAILABLE) { @@ -7184,6 +7198,7 @@ static int SendTls13CertificateVerify(WOLFSSL* ssl) { #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { + ssl->options.buildingMsg = 0; ret = Dtls13HandshakeSend(ssl, args->output, MAX_CERT_VERIFY_SZ + MAX_MSG_EXTRA + MAX_MSG_EXTRA, args->sendSz, certificate_verify, 1); @@ -7219,7 +7234,7 @@ static int SendTls13CertificateVerify(WOLFSSL* ssl) #endif ssl->buffers.outputBuffer.length += args->sendSz; - + ssl->options.buildingMsg = 0; if (!ssl->options.groupMessages) ret = SendBuffered(ssl); break; @@ -7955,6 +7970,7 @@ static int SendTls13Finished(WOLFSSL* ssl) WOLFSSL_START(WC_FUNC_FINISHED_SEND); WOLFSSL_ENTER("SendTls13Finished"); + ssl->options.buildingMsg = 1; #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { headerSz = DTLS_HANDSHAKE_HEADER_SZ; @@ -8057,6 +8073,7 @@ static int SendTls13Finished(WOLFSSL* ssl) #endif ssl->buffers.outputBuffer.length += sendSz; + ssl->options.buildingMsg = 0; } if (ssl->options.side == WOLFSSL_SERVER_END) { @@ -8402,6 +8419,7 @@ static int SendTls13EndOfEarlyData(WOLFSSL* ssl) length = 0; sendSz = idx + length + MAX_MSG_EXTRA; + ssl->options.buildingMsg = 1; /* Check buffers are big enough and grow if needed. */ if ((ret = CheckAvailableSize(ssl, sendSz)) != 0) @@ -8425,6 +8443,7 @@ static int SendTls13EndOfEarlyData(WOLFSSL* ssl) if ((ret = SetKeysSide(ssl, ENCRYPT_SIDE_ONLY)) != 0) return ret; + ssl->options.buildingMsg = 0; if (!ssl->options.groupMessages) ret = SendBuffered(ssl); From 066f17faadb5ce12bb3fa95ae072c2b75f8c9a44 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 20 Jul 2022 14:51:10 +0200 Subject: [PATCH 4/6] fix: dtls13: hello_retry_request type isn't an encrypted message --- src/dtls13.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/dtls13.c b/src/dtls13.c index 36d329e50..ee45034cb 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -234,12 +234,12 @@ static byte Dtls13TypeIsEncrypted(enum HandShakeType hs_type) case hello_request: case hello_verify_request: case client_hello: + case hello_retry_request: case server_hello: break; case encrypted_extensions: case session_ticket: case end_of_early_data: - case hello_retry_request: case certificate: case server_key_exchange: case certificate_request: From 3850e6b554d223f83acf499f830a1b64d8c665b6 Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 20 Jul 2022 14:51:49 +0200 Subject: [PATCH 5/6] fix: dtls13: use aes for record numbers encryption if using aes-ccm --- src/dtls13.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/dtls13.c b/src/dtls13.c index ee45034cb..e31d2c699 100644 --- a/src/dtls13.c +++ b/src/dtls13.c @@ -269,14 +269,15 @@ static int Dtls13GetRnMask(WOLFSSL* ssl, const byte* ciphertext, byte* mask, else c = &ssl->dtlsRecordNumberDecrypt; -#ifdef HAVE_AESGCM - if (ssl->specs.bulk_cipher_algorithm == wolfssl_aes_gcm) { +#if defined(HAVE_AESGCM) || defined(HAVE_AESCCM) + if (ssl->specs.bulk_cipher_algorithm == wolfssl_aes_gcm || + ssl->specs.bulk_cipher_algorithm == wolfssl_aes_ccm) { if (c->aes == NULL) return BAD_STATE_E; return wc_AesEncryptDirect(c->aes, mask, ciphertext); } -#endif /* HAVE_AESGCM */ +#endif /* HAVE_AESGCM || HAVE_AESCCM */ #ifdef HAVE_CHACHA if (ssl->specs.bulk_cipher_algorithm == wolfssl_chacha) { From 2e0d53a07d68fa93bd351714128779820cc25b4a Mon Sep 17 00:00:00 2001 From: Marco Oliverio Date: Wed, 20 Jul 2022 14:52:29 +0200 Subject: [PATCH 6/6] fix: dtls13: use correct handshaketype on hello retry request --- src/tls13.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tls13.c b/src/tls13.c index 1f4819c51..5731f01b1 100644 --- a/src/tls13.c +++ b/src/tls13.c @@ -5731,7 +5731,7 @@ int SendTls13ServerHello(WOLFSSL* ssl, byte extMsgType) #ifdef WOLFSSL_DTLS13 if (ssl->options.dtls) { ret = Dtls13HandshakeSend(ssl, output, sendSz, sendSz, - server_hello, 0); + extMsgType, 0); WOLFSSL_LEAVE("SendTls13ServerHello", ret); WOLFSSL_END(WC_FUNC_SERVER_HELLO_SEND);