Skip to content

Commit d3ce5b8

Browse files
gasbytesgasbytes
committed
DTLS 1.3 and TLS 1.3 focused fixes
dtls13.c: - Fix wrong return value in Dtls13SendFragmentedInternal error path (return outputSz instead of recordLength) - Fix incomplete bounds check in Dtls13SendFragmented to account for DTLS_HANDSHAKE_HEADER_SZ - Fix wrong WOLFSSL_ENTER trace string in Dtls13EpochCopyKeys tls13.c: - Remove wrong (byte) cast on cookie->len passed to TlsCheckCookie - Add missing bounds check on PSK identityLen in SetupPskKey before copying to client_identity - Fix data race on static header array in ExpectedResumptionSecret - Add defensive underflow check in EncryptTls13 for consistency with DecryptTls13 - Fix wrong return variable in DTLS 1.3 Finished send error path (return dtlsRet instead of ret) - Add missing SM3 case and default in Tls13_Exporter hash switch to prevent NULL dereference - Initialize *outSz to 0 in wolfSSL_write_early_data to match wolfSSL_read_early_data - Add bounds check for bindersLen against helloSz in CheckPreSharedKeys - Fix resource leak and hash state corruption in ExpectedResumptionSecret error paths - Fix memory leak of rsaSigBuf in dual-alg RSA+RSA CertificateVerify - Guard against word32 underflow in inputLength - HANDSHAKE_HEADER_SZ in DoTls13HandShakeMsg - Fix swapped side parameter in DeriveFinishedSecret for server-side Finished processing - Fix no_mac fall-through in ssl_handshake_md to return NULL instead of wrong digest - Fix strict aliasing violation in FindPsk PSK key size check - Remove duplicate !ssl->options.dtls check in TLS 1.3 middlebox compat condition tests: - Add regression tests for wolfSSL_write_early_data outSz initialization and DTLS 1.3 Finished send error propagation
1 parent 6fc93ac commit d3ce5b8

4 files changed

Lines changed: 93 additions & 16 deletions

File tree

‎src/dtls13.c‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,7 +1040,7 @@ static int Dtls13SendFragmentedInternal(WOLFSSL* ssl)
10401040
fragLength + DTLS_HANDSHAKE_HEADER_SZ, isEncrypted);
10411041
if (outputSz < 0) {
10421042
Dtls13FreeFragmentsBuffer(ssl);
1043-
return recordLength;
1043+
return outputSz;
10441044
}
10451045

10461046
ret = CheckAvailableSize(ssl, outputSz);
@@ -1102,7 +1102,7 @@ static int Dtls13SendFragmented(WOLFSSL* ssl, byte* message, word16 length,
11021102
isEncrypted = Dtls13TypeIsEncrypted(handshake_type);
11031103
rlHeaderLength = Dtls13GetRlHeaderLength(ssl, isEncrypted);
11041104

1105-
if (length < rlHeaderLength)
1105+
if (length < rlHeaderLength + DTLS_HANDSHAKE_HEADER_SZ)
11061106
return INCOMPLETE_DATA;
11071107

11081108
/* DTLSv1.3 do not consider fragmentation for hash transcript. Build the
@@ -2212,7 +2212,7 @@ static void Dtls13EpochCopyKeys(WOLFSSL* ssl, Dtls13Epoch* e, Keys* k, int side)
22122212
byte clientWrite, serverWrite;
22132213
byte enc, dec;
22142214

2215-
WOLFSSL_ENTER("Dtls13SetEpochKeys");
2215+
WOLFSSL_ENTER("Dtls13EpochCopyKeys");
22162216

22172217
clientWrite = serverWrite = 0;
22182218
enc = dec = 0;

‎src/tls13.c‎

Lines changed: 44 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -948,6 +948,13 @@ static const byte emptySHA512Hash[] = {
948948
0xF9, 0x27, 0xDA, 0x3E
949949
};
950950
#endif
951+
#ifdef WOLFSSL_SM3
952+
static const byte emptySM3Hash[] = {
953+
0x1A, 0xB2, 0x1D, 0x83, 0x55, 0xCF, 0xA1, 0x7F, 0x8E, 0x61, 0x19, 0x48,
954+
0x31, 0xE8, 0x1A, 0x8F, 0x22, 0xBE, 0xC8, 0xC7, 0x28, 0xFE, 0xFB, 0x74,
955+
0x7E, 0xD0, 0x35, 0xEB, 0x50, 0x82, 0xAA, 0x2B
956+
};
957+
#endif
951958
/**
952959
* Implement section 7.5 of RFC 8446
953960
* @return 0 on success
@@ -1003,6 +1010,17 @@ int Tls13_Exporter(WOLFSSL* ssl, unsigned char *out, size_t outLen,
10031010
emptyHash = emptySHA512Hash;
10041011
break;
10051012
#endif
1013+
1014+
#ifdef WOLFSSL_SM3
1015+
case sm3_mac:
1016+
hashType = WC_HASH_TYPE_SM3;
1017+
hashLen = WC_SM3_DIGEST_SIZE;
1018+
emptyHash = emptySM3Hash;
1019+
break;
1020+
#endif
1021+
1022+
default:
1023+
return BAD_FUNC_ARG;
10061024
}
10071025

10081026
/* Derive-Secret(Secret, label, "") */
@@ -2572,7 +2590,7 @@ static int EncryptTls13(WOLFSSL* ssl, byte* output, const byte* input,
25722590
word16 sz, const byte* aad, word16 aadSz, int asyncOkay)
25732591
{
25742592
int ret = 0;
2575-
word16 dataSz = sz - ssl->specs.aead_mac_size;
2593+
word16 dataSz;
25762594
word16 macSz = ssl->specs.aead_mac_size;
25772595
word32 nonceSz = 0;
25782596
#ifdef WOLFSSL_ASYNC_CRYPT
@@ -2581,6 +2599,9 @@ static int EncryptTls13(WOLFSSL* ssl, byte* output, const byte* input,
25812599
#endif
25822600

25832601
WOLFSSL_ENTER("EncryptTls13");
2602+
if (sz < ssl->specs.aead_mac_size)
2603+
return BUFFER_E;
2604+
dataSz = sz - ssl->specs.aead_mac_size;
25842605

25852606
(void)output;
25862607
(void)input;
@@ -4054,6 +4075,7 @@ static const WOLFSSL_EVP_MD* ssl_handshake_md(const byte mac_alg)
40544075
{
40554076
switch(mac_alg) {
40564077
case no_mac:
4078+
return NULL;
40574079
#ifndef NO_MD5
40584080
case md5_mac:
40594081
return wolfSSL_EVP_md5();
@@ -4161,6 +4183,8 @@ static int SetupPskKey(WOLFSSL* ssl, PreSharedKey* psk, int clientHello)
41614183
#endif
41624184

41634185
/* Set the client identity to use. */
4186+
if (psk->identityLen > MAX_PSK_ID_LEN)
4187+
return PSK_KEY_ERROR;
41644188
XMEMSET(ssl->arrays->client_identity, 0,
41654189
sizeof(ssl->arrays->client_identity));
41664190
XMEMCPY(ssl->arrays->client_identity, psk->identity, psk->identityLen);
@@ -5982,7 +6006,7 @@ int FindPskSuite(const WOLFSSL* ssl, PreSharedKey* psk, byte* psk_key,
59826006
}
59836007
if (*found) {
59846008
if (*psk_keySz > MAX_PSK_KEY_LEN &&
5985-
*((int*)psk_keySz) != WC_NO_ERR_TRACE(USE_HW_PSK)) {
6009+
(int)*psk_keySz != WC_NO_ERR_TRACE(USE_HW_PSK)) {
59866010
WOLFSSL_MSG("Key len too long in FindPsk()");
59876011
ret = PSK_KEY_ERROR;
59886012
WOLFSSL_ERROR_VERBOSE(ret);
@@ -6322,6 +6346,8 @@ static int CheckPreSharedKeys(WOLFSSL* ssl, const byte* input, word32 helloSz,
63226346
client_hello, &bindersLen);
63236347
if (ret < 0)
63246348
return ret;
6349+
if (bindersLen > helloSz)
6350+
return BUFFER_ERROR;
63256351

63266352
/* Refine list for PSK processing. */
63276353
sslRefineSuites(ssl, clSuites);
@@ -6564,7 +6590,7 @@ static int RestartHandshakeHashWithCookie(WOLFSSL* ssl, Cookie* cookie)
65646590
int keyShareExt = 0;
65656591
int ret;
65666592

6567-
ret = TlsCheckCookie(ssl, cookie->data, (byte)cookie->len);
6593+
ret = TlsCheckCookie(ssl, cookie->data, cookie->len);
65686594
if (ret < 0)
65696595
return ret;
65706596
cookieDataSz = (word16)ret;
@@ -9741,6 +9767,7 @@ static int SendTls13CertificateVerify(WOLFSSL* ssl)
97419767
#ifndef NO_RSA
97429768
if (ssl->hsAltType == DYNAMIC_TYPE_RSA) {
97439769
/* build encoded signature buffer */
9770+
XFREE(rsaSigBuf->buffer, ssl->heap, DYNAMIC_TYPE_SIGNATURE);
97449771
rsaSigBuf->length = WC_MAX_DIGEST_SIZE;
97459772
rsaSigBuf->buffer = (byte*)XMALLOC(rsaSigBuf->length,
97469773
ssl->heap,
@@ -11488,13 +11515,13 @@ static int SendTls13Finished(WOLFSSL* ssl)
1148811515
*/
1148911516
ret = DeriveFinishedSecret(ssl, ssl->clientSecret,
1149011517
ssl->keys.client_write_MAC_secret,
11491-
WOLFSSL_SERVER_END);
11518+
WOLFSSL_CLIENT_END);
1149211519
if (ret != 0)
1149311520
return ret;
1149411521

1149511522
ret = DeriveFinishedSecret(ssl, ssl->serverSecret,
1149611523
ssl->keys.server_write_MAC_secret,
11497-
WOLFSSL_CLIENT_END);
11524+
WOLFSSL_SERVER_END);
1149811525
if (ret != 0)
1149911526
return ret;
1150011527

@@ -11520,7 +11547,7 @@ static int SendTls13Finished(WOLFSSL* ssl)
1152011547
(word16)(Dtls13GetRlHeaderLength(ssl, 1) + headerSz + finishedSz), finished,
1152111548
1);
1152211549
if (dtlsRet != 0 && dtlsRet != WC_NO_ERR_TRACE(WANT_WRITE))
11523-
return ret;
11550+
return dtlsRet;
1152411551

1152511552
} else
1152611553
#endif /* WOLFSSL_DTLS13 */
@@ -12166,7 +12193,7 @@ static int ExpectedResumptionSecret(WOLFSSL* ssl)
1216612193
word32 finishedSz = 0;
1216712194
byte mac[WC_MAX_DIGEST_SIZE];
1216812195
Digest digest;
12169-
static byte header[] = { 0x14, 0x00, 0x00, 0x00 };
12196+
byte header[] = { 0x14, 0x00, 0x00, 0x00 };
1217012197

1217112198
XMEMSET(&digest, 0, sizeof(Digest));
1217212199

@@ -12206,25 +12233,26 @@ static int ExpectedResumptionSecret(WOLFSSL* ssl)
1220612233
ret = BuildTls13HandshakeHmac(ssl, ssl->keys.client_write_MAC_secret, mac,
1220712234
&finishedSz);
1220812235
if (ret != 0)
12209-
return ret;
12236+
goto restore;
1221012237
header[FINISHED_MSG_SIZE_OFFSET] = finishedSz;
1221112238
#ifdef WOLFSSL_EARLY_DATA
1221212239
if (ssl->earlyData != no_early_data) {
1221312240
static byte endOfEarlyData[] = { 0x05, 0x00, 0x00, 0x00 };
1221412241
ret = HashRaw(ssl, endOfEarlyData, sizeof(endOfEarlyData));
1221512242
if (ret != 0)
12216-
return ret;
12243+
goto restore;
1221712244
}
1221812245
#endif
1221912246
if ((ret = HashRaw(ssl, header, sizeof(header))) != 0)
12220-
return ret;
12247+
goto restore;
1222112248
if ((ret = HashRaw(ssl, mac, finishedSz)) != 0)
12222-
return ret;
12249+
goto restore;
1222312250

1222412251
if ((ret = DeriveResumptionSecret(ssl, ssl->session->masterSecret)) != 0)
12225-
return ret;
12252+
goto restore;
1222612253

1222712254
/* Restore the hash inline with currently seen messages. */
12255+
restore:
1222812256
switch (ssl->specs.mac_algorithm) {
1222912257
#ifndef NO_SHA256
1223012258
case sha256_mac:
@@ -13460,7 +13488,8 @@ int DoTls13HandShakeMsg(WOLFSSL* ssl, byte* input, word32* inOutIdx,
1346013488
}
1346113489

1346213490
ret = EarlySanityCheckMsgReceived(ssl, type,
13463-
min(inputLength - HANDSHAKE_HEADER_SZ, size));
13491+
(inputLength > HANDSHAKE_HEADER_SZ) ?
13492+
min(inputLength - HANDSHAKE_HEADER_SZ, size) : 0);
1346413493
if (ret != 0) {
1346513494
WOLFSSL_ERROR(ret);
1346613495
return ret;
@@ -15308,6 +15337,8 @@ int wolfSSL_write_early_data(WOLFSSL* ssl, const void* data, int sz, int* outSz)
1530815337
if (!IsAtLeastTLSv1_3(ssl->version))
1530915338
return BAD_FUNC_ARG;
1531015339

15340+
*outSz = 0;
15341+
1531115342
#ifndef NO_WOLFSSL_CLIENT
1531215343
if (ssl->options.side == WOLFSSL_SERVER_END)
1531315344
return SIDE_ERROR;

‎tests/api.c‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33050,6 +33050,46 @@ static int test_dtls13_missing_finished_server(void)
3305033050
return EXPECT_RESULT();
3305133051
}
3305233052

33053+
static int test_dtls13_finished_send_error_propagation(void)
33054+
{
33055+
EXPECT_DECLS;
33056+
#if defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && defined(WOLFSSL_DTLS13)
33057+
WOLFSSL_CTX *ctx_c = NULL;
33058+
WOLFSSL_CTX *ctx_s = NULL;
33059+
WOLFSSL *ssl_c = NULL;
33060+
WOLFSSL *ssl_s = NULL;
33061+
struct test_memio_ctx test_ctx;
33062+
33063+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
33064+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c, &ssl_s,
33065+
wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method), 0);
33066+
33067+
/* CH1 */
33068+
ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
33069+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
33070+
/* HRR */
33071+
ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
33072+
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
33073+
/* CH2 */
33074+
ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
33075+
ExpectIntEQ(wolfSSL_get_error(ssl_c, -1), WOLFSSL_ERROR_WANT_READ);
33076+
/* Server first flight with finished */
33077+
ExpectIntEQ(wolfSSL_negotiate(ssl_s), -1);
33078+
ExpectIntEQ(wolfSSL_get_error(ssl_s, -1), WOLFSSL_ERROR_WANT_READ);
33079+
/* Client second flight with finished — block sends to force error */
33080+
test_ctx.s_len = TEST_MEMIO_BUF_SZ;
33081+
ExpectIntEQ(wolfSSL_negotiate(ssl_c), -1);
33082+
/* Verify the error is propagated, not silently swallowed as success */
33083+
ExpectIntNE(wolfSSL_get_error(ssl_c, -1), 0);
33084+
33085+
wolfSSL_free(ssl_c);
33086+
wolfSSL_free(ssl_s);
33087+
wolfSSL_CTX_free(ctx_c);
33088+
wolfSSL_CTX_free(ctx_s);
33089+
#endif
33090+
return EXPECT_RESULT();
33091+
}
33092+
3305333093

3305433094
#if !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER)
3305533095
#ifdef HAVE_CERTIFICATE_STATUS_REQUEST
@@ -35393,6 +35433,7 @@ TEST_CASE testCases[] = {
3539335433
TEST_DECL(test_dtls12_missing_finished),
3539435434
TEST_DECL(test_dtls13_missing_finished_client),
3539535435
TEST_DECL(test_dtls13_missing_finished_server),
35436+
TEST_DECL(test_dtls13_finished_send_error_propagation),
3539635437
TEST_DTLS_DECLS,
3539735438
TEST_DECL(test_tls_multi_handshakes_one_record),
3539835439
TEST_DECL(test_write_dup),

‎tests/api/test_tls13.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,11 @@ int test_tls13_apis(void)
808808
/* invoking without session or psk cbs */
809809
ExpectIntEQ(wolfSSL_write_early_data(clientSsl, earlyData,
810810
sizeof(earlyData), &outSz), WC_NO_ERR_TRACE(BAD_STATE_E));
811+
/* verify *outSz is initialized to 0 even on non-success paths */
812+
outSz = 42;
813+
ExpectIntEQ(wolfSSL_write_early_data(clientSsl, earlyData,
814+
sizeof(earlyData), &outSz), WC_NO_ERR_TRACE(BAD_STATE_E));
815+
ExpectIntEQ(outSz, 0);
811816
#endif
812817

813818
ExpectIntEQ(wolfSSL_read_early_data(NULL, earlyDataBuffer,

0 commit comments

Comments
 (0)