Skip to content

Commit f31ed0d

Browse files
LinuxJediLinuxJedi
committed
Fix logic bug in TLSX_TCA_Find causing incorrect Trusted CA matching
The while loop conditions in TLSX_TCA_Find were inverted, causing two bugs: the loop short-circuited on type match alone without checking the id content, and the XMEMCMP sense was reversed (continuing on match, stopping on mismatch). This meant any TCA entry with a matching type would be returned as a match regardless of whether the identifier actually matched. Restructure the loop to correctly require both type and id (size + content) to match before returning an entry, and to match any entry immediately for PRE_AGREED type. Add test_TLSX_TCA_Find unit test exercising exact match, mismatched id, and PRE_AGREED cases via memio handshake.
1 parent d81bb72 commit f31ed0d

4 files changed

Lines changed: 137 additions & 2 deletions

File tree

‎src/tls.c‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2849,9 +2849,14 @@ static TCA* TLSX_TCA_Find(TCA *list, byte type, const byte* id, word16 idSz)
28492849
{
28502850
TCA* tca = list;
28512851

2852-
while (tca && tca->type != type && type != WOLFSSL_TRUSTED_CA_PRE_AGREED &&
2853-
idSz != tca->idSz && !XMEMCMP(id, tca->id, idSz))
2852+
while (tca) {
2853+
if (type == WOLFSSL_TRUSTED_CA_PRE_AGREED)
2854+
break;
2855+
if (tca->type == type && idSz == tca->idSz &&
2856+
XMEMCMP(id, tca->id, idSz) == 0)
2857+
break;
28542858
tca = tca->next;
2859+
}
28552860

28562861
return tca;
28572862
}

‎tests/api.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32829,6 +32829,7 @@ TEST_CASE testCases[] = {
3282932829
TEST_DECL(test_wolfSSL_DisableExtendedMasterSecret),
3283032830
TEST_DECL(test_certificate_authorities_certificate_request),
3283132831
TEST_DECL(test_certificate_authorities_client_hello),
32832+
TEST_DECL(test_TLSX_TCA_Find),
3283232833
TEST_DECL(test_wolfSSL_wolfSSL_UseSecureRenegotiation),
3283332834
TEST_DECL(test_wolfSSL_SCR_Reconnect),
3283432835
TEST_DECL(test_wolfSSL_SCR_check_enabled),

‎tests/api/test_tls_ext.c‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -323,6 +323,134 @@ static int certificate_authorities_server_cb(WOLFSSL *ssl, void *_arg) {
323323
}
324324
#endif
325325

326+
/* Walk the TLSX list to find an extension by type. Avoids calling the
327+
* WOLFSSL_LOCAL TLSX_Find which is not available in shared library builds. */
328+
static TLSX* test_TLSX_find_ext(TLSX* list, TLSX_Type type)
329+
{
330+
while (list) {
331+
if (list->type == type)
332+
return list;
333+
list = list->next;
334+
}
335+
return NULL;
336+
}
337+
338+
int test_TLSX_TCA_Find(void)
339+
{
340+
EXPECT_DECLS;
341+
#if defined(HAVE_TRUSTED_CA) && !defined(NO_SHA) && \
342+
defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) && \
343+
!defined(NO_WOLFSSL_SERVER) && !defined(NO_WOLFSSL_CLIENT)
344+
/* Two different 20-byte SHA1 ids */
345+
byte id_A[WC_SHA_DIGEST_SIZE];
346+
byte id_B[WC_SHA_DIGEST_SIZE];
347+
TLSX* ext;
348+
349+
XMEMSET(id_A, 0xAA, sizeof(id_A));
350+
XMEMSET(id_B, 0xBB, sizeof(id_B));
351+
352+
/* Test 1: Exact match - same type and same id should match */
353+
{
354+
struct test_memio_ctx test_ctx;
355+
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
356+
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
357+
358+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
359+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c,
360+
&ssl_s, wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0);
361+
362+
/* Server has KEY_SHA1 with id_A */
363+
ExpectIntEQ(wolfSSL_UseTrustedCA(ssl_s, WOLFSSL_TRUSTED_CA_KEY_SHA1,
364+
id_A, sizeof(id_A)), WOLFSSL_SUCCESS);
365+
/* Client sends KEY_SHA1 with id_A (same) */
366+
ExpectIntEQ(wolfSSL_UseTrustedCA(ssl_c, WOLFSSL_TRUSTED_CA_KEY_SHA1,
367+
id_A, sizeof(id_A)), WOLFSSL_SUCCESS);
368+
369+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
370+
371+
/* Server should have found a match and responded */
372+
ext = test_TLSX_find_ext(ssl_c ? ssl_c->extensions : NULL,
373+
TLSX_TRUSTED_CA_KEYS);
374+
ExpectNotNull(ext);
375+
if (EXPECT_SUCCESS())
376+
ExpectIntEQ(ext->resp, 1);
377+
378+
wolfSSL_free(ssl_c);
379+
wolfSSL_free(ssl_s);
380+
wolfSSL_CTX_free(ctx_c);
381+
wolfSSL_CTX_free(ctx_s);
382+
}
383+
384+
/* Test 2: Same type, different id - should NOT match.
385+
* This is the key test that exposes the logic bug in TLSX_TCA_Find
386+
* where matching on type alone (without checking id content) causes
387+
* a false positive. */
388+
{
389+
struct test_memio_ctx test_ctx;
390+
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
391+
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
392+
393+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
394+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c,
395+
&ssl_s, wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0);
396+
397+
/* Server has KEY_SHA1 with id_A */
398+
ExpectIntEQ(wolfSSL_UseTrustedCA(ssl_s, WOLFSSL_TRUSTED_CA_KEY_SHA1,
399+
id_A, sizeof(id_A)), WOLFSSL_SUCCESS);
400+
/* Client sends KEY_SHA1 with id_B (different!) */
401+
ExpectIntEQ(wolfSSL_UseTrustedCA(ssl_c, WOLFSSL_TRUSTED_CA_KEY_SHA1,
402+
id_B, sizeof(id_B)), WOLFSSL_SUCCESS);
403+
404+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
405+
406+
/* Server should NOT have found a match - ids differ */
407+
ext = test_TLSX_find_ext(ssl_c ? ssl_c->extensions : NULL,
408+
TLSX_TRUSTED_CA_KEYS);
409+
ExpectNotNull(ext);
410+
if (EXPECT_SUCCESS())
411+
ExpectIntEQ(ext->resp, 0);
412+
413+
wolfSSL_free(ssl_c);
414+
wolfSSL_free(ssl_s);
415+
wolfSSL_CTX_free(ctx_c);
416+
wolfSSL_CTX_free(ctx_s);
417+
}
418+
419+
/* Test 3: PRE_AGREED should match any server entry */
420+
{
421+
struct test_memio_ctx test_ctx;
422+
WOLFSSL_CTX *ctx_c = NULL, *ctx_s = NULL;
423+
WOLFSSL *ssl_c = NULL, *ssl_s = NULL;
424+
425+
XMEMSET(&test_ctx, 0, sizeof(test_ctx));
426+
ExpectIntEQ(test_memio_setup(&test_ctx, &ctx_c, &ctx_s, &ssl_c,
427+
&ssl_s, wolfTLSv1_2_client_method, wolfTLSv1_2_server_method), 0);
428+
429+
/* Server has KEY_SHA1 with id_A */
430+
ExpectIntEQ(wolfSSL_UseTrustedCA(ssl_s, WOLFSSL_TRUSTED_CA_KEY_SHA1,
431+
id_A, sizeof(id_A)), WOLFSSL_SUCCESS);
432+
/* Client sends PRE_AGREED (no id needed) */
433+
ExpectIntEQ(wolfSSL_UseTrustedCA(ssl_c, WOLFSSL_TRUSTED_CA_PRE_AGREED,
434+
NULL, 0), WOLFSSL_SUCCESS);
435+
436+
ExpectIntEQ(test_memio_do_handshake(ssl_c, ssl_s, 10, NULL), 0);
437+
438+
/* Server should have matched (PRE_AGREED matches anything) */
439+
ext = test_TLSX_find_ext(ssl_c ? ssl_c->extensions : NULL,
440+
TLSX_TRUSTED_CA_KEYS);
441+
ExpectNotNull(ext);
442+
if (EXPECT_SUCCESS())
443+
ExpectIntEQ(ext->resp, 1);
444+
445+
wolfSSL_free(ssl_c);
446+
wolfSSL_free(ssl_s);
447+
wolfSSL_CTX_free(ctx_c);
448+
wolfSSL_CTX_free(ctx_s);
449+
}
450+
#endif
451+
return EXPECT_RESULT();
452+
}
453+
326454
int test_certificate_authorities_client_hello(void) {
327455
EXPECT_DECLS;
328456
#if !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \

‎tests/api/test_tls_ext.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,5 +26,6 @@ int test_tls_ems_downgrade(void);
2626
int test_wolfSSL_DisableExtendedMasterSecret(void);
2727
int test_certificate_authorities_certificate_request(void);
2828
int test_certificate_authorities_client_hello(void);
29+
int test_TLSX_TCA_Find(void);
2930

3031
#endif /* TESTS_API_TEST_TLS_EMS_H */

0 commit comments

Comments
 (0)