diff --git a/src/tls.c b/src/tls.c index be1533996b..27bf3620b2 100644 --- a/src/tls.c +++ b/src/tls.c @@ -7695,6 +7695,12 @@ int TLSX_Cookie_Use(const WOLFSSL* ssl, const byte* data, word16 len, byte* mac, /* Certificate Authorities */ /******************************************************************************/ +/* Smallest legal authorities list from RFC 8446 section 4.2.4: a 2 byte + * length plus at least 1 byte of name. */ +#ifndef WC_CA_NAMES_MIN_SZ + #define WC_CA_NAMES_MIN_SZ 3 +#endif + static word16 TLSX_CA_Names_GetSize(void* data) { WOLFSSL* ssl = (WOLFSSL*)data; @@ -7746,6 +7752,25 @@ static word16 TLSX_CA_Names_Write(void* data, byte* output) return (word16)(output - len); } +/* Count the CA names TLSX_CA_Names_Write() would write. RFC 8446 section + * 4.2.4 needs at least one, so send the extension only when this is non-zero. + * An empty list is one node with a NULL name, which counts as zero. */ +static int TLSX_CA_Names_Count(WOLFSSL* ssl) +{ + WOLF_STACK_OF(WOLFSSL_X509_NAME)* names; + int cnt = 0; + + if (ssl == NULL) + return 0; + + for (names = SSL_PRIORITY_CA_NAMES(ssl); names != NULL; + names = names->next) { + if (names->data.name != NULL) + cnt++; + } + return cnt; +} + static int TLSX_CA_Names_Parse(WOLFSSL *ssl, const byte* input, word16 length, byte isRequest) { @@ -7767,6 +7792,14 @@ static int TLSX_CA_Names_Parse(WOLFSSL *ssl, const byte* input, if (extLen != length) return BUFFER_ERROR; + /* RFC 8446 section 4.2.4 says authorities<3..2^16-1>, and the size table + * in TLSX_Parse skips certificate_request. Set WC_CA_NAMES_MIN_SZ to 0 + * to accept short lists the way older versions did. */ +#if WC_CA_NAMES_MIN_SZ > 0 + if (extLen < WC_CA_NAMES_MIN_SZ) + return BUFFER_ERROR; +#endif + while (length) { word16 idx = 0; WOLFSSL_X509_NAME* name = NULL; @@ -16461,7 +16494,7 @@ int TLSX_PopulateExtensions(WOLFSSL* ssl, byte isServer) #ifdef WOLFSSL_TLS13 #if !defined(NO_CERTS) && !defined(WOLFSSL_NO_CA_NAMES) if (IsAtLeastTLSv1_3(ssl->version) && - SSL_PRIORITY_CA_NAMES(ssl) != NULL) { + TLSX_CA_Names_Count(ssl) > 0) { WOLFSSL_MSG("Adding certificate authorities extension"); if ((ret = TLSX_Push(&ssl->extensions, TLSX_CERTIFICATE_AUTHORITIES, ssl, ssl->heap)) != 0) { @@ -17506,7 +17539,7 @@ int TLSX_GetRequestSize(WOLFSSL* ssl, byte msgType, word32* pLength) #endif #if !defined(NO_CERTS) && !defined(WOLFSSL_NO_CA_NAMES) if (!IsAtLeastTLSv1_3(ssl->version) || - SSL_CA_NAMES(ssl) == NULL) { + TLSX_CA_Names_Count(ssl) == 0) { TURN_ON(semaphore, TLSX_ToSemaphore(TLSX_CERTIFICATE_AUTHORITIES)); } @@ -17531,7 +17564,7 @@ int TLSX_GetRequestSize(WOLFSSL* ssl, byte msgType, word32* pLength) TURN_OFF(semaphore, TLSX_ToSemaphore(TLSX_SIGNATURE_ALGORITHMS)); #endif #if !defined(NO_CERTS) && !defined(WOLFSSL_NO_CA_NAMES) - if (SSL_PRIORITY_CA_NAMES(ssl) != NULL) { + if (TLSX_CA_Names_Count(ssl) > 0) { TURN_OFF(semaphore, TLSX_ToSemaphore(TLSX_CERTIFICATE_AUTHORITIES)); } @@ -17745,7 +17778,7 @@ int TLSX_WriteRequest(WOLFSSL* ssl, byte* output, byte msgType, word32* pOffset) } #endif #if !defined(NO_CERTS) && !defined(WOLFSSL_NO_CA_NAMES) - if (!IsAtLeastTLSv1_3(ssl->version) || SSL_CA_NAMES(ssl) == NULL) { + if (!IsAtLeastTLSv1_3(ssl->version) || TLSX_CA_Names_Count(ssl) == 0) { TURN_ON(semaphore, TLSX_ToSemaphore(TLSX_CERTIFICATE_AUTHORITIES)); } @@ -17776,7 +17809,7 @@ int TLSX_WriteRequest(WOLFSSL* ssl, byte* output, byte msgType, word32* pOffset) TURN_OFF(semaphore, TLSX_ToSemaphore(TLSX_SIGNATURE_ALGORITHMS)); #endif #if !defined(NO_CERTS) && !defined(WOLFSSL_NO_CA_NAMES) - if (SSL_PRIORITY_CA_NAMES(ssl) != NULL) { + if (TLSX_CA_Names_Count(ssl) > 0) { TURN_OFF(semaphore, TLSX_ToSemaphore(TLSX_CERTIFICATE_AUTHORITIES)); } diff --git a/tests/api.c b/tests/api.c index 69e8a0cb82..9f102c5ffd 100644 --- a/tests/api.c +++ b/tests/api.c @@ -39446,6 +39446,9 @@ TEST_CASE testCases[] = { TEST_DECL(test_wolfSSL_DisableExtendedMasterSecret), TEST_DECL(test_certificate_authorities_certificate_request), TEST_DECL(test_certificate_authorities_client_hello), + TEST_DECL(test_certificate_authorities_empty_client_hello), + TEST_DECL(test_certificate_authorities_empty_cert_request), + TEST_DECL(test_certificate_authorities_short_parse), TEST_DECL(test_TLSX_TCA_Find), TEST_DECL(test_TLSX_SNI_GetSize_overflow), TEST_DECL(test_TLSX_ECH_msg_type_validation), diff --git a/tests/api/test_tls_ext.c b/tests/api/test_tls_ext.c index 6091ffc89e..237d4c5ad7 100644 --- a/tests/api/test_tls_ext.c +++ b/tests/api/test_tls_ext.c @@ -1295,6 +1295,161 @@ int test_certificate_authorities_client_hello(void) { return EXPECT_RESULT(); } +/* Same rule as above, but for the CertificateRequest a server sends. This + * used to let the handshake succeed and leave the client holding a + * present-but-empty CA name list. */ +int test_certificate_authorities_empty_cert_request(void) { + EXPECT_DECLS; +#if !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(WOLFSSL_NO_CA_NAMES) && !defined(NO_BIO) && \ + !defined(NO_CERTS) && !defined(NO_TLS) && (defined(OPENSSL_EXTRA) || \ + defined(OPENSSL_EXTRA_X509_SMALL)) && (defined(OPENSSL_ALL) || \ + defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY)) && \ + defined(WOLFSSL_TLS13) && defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) + struct test_memio_ctx test_ctx; + WOLFSSL_CTX *ctx_srv = NULL, *ctx_cli = NULL; + WOLFSSL *ssl_srv = NULL, *ssl_cli = NULL; + WOLF_STACK_OF(X509_NAME) *empty = NULL; + struct client_cb_arg cb_arg = { NULL, NULL }; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + ExpectIntEQ(0, test_memio_setup(&test_ctx, &ctx_cli, &ctx_srv, + &ssl_cli, NULL, wolfTLSv1_3_client_method, + wolfTLSv1_3_server_method)); + + /* Ask the client for a certificate so the server sends a + * CertificateRequest. */ + wolfSSL_CTX_set_verify(ctx_srv, + SSL_VERIFY_PEER | SSL_VERIFY_FAIL_IF_NO_PEER_CERT, NULL); + ExpectIntEQ(WOLFSSL_SUCCESS, + wolfSSL_CTX_load_verify_locations(ctx_srv, cliCertFile, NULL)); + + ExpectNotNull(ssl_srv = wolfSSL_new(ctx_srv)); + wolfSSL_SetIOReadCtx(ssl_srv, &test_ctx); + wolfSSL_SetIOWriteCtx(ssl_srv, &test_ctx); +#if !defined(NO_DH) + SetDH(ssl_srv); +#endif + + /* ssl_srv takes ownership of the empty stack. */ + ExpectNotNull(empty = wolfSSL_sk_X509_NAME_new(NULL)); + wolfSSL_set0_CA_list(ssl_srv, empty); + + wolfSSL_CTX_set_cert_cb(ctx_cli, certificate_authorities_client_cb, + &cb_arg); + + ExpectIntEQ(0, test_memio_do_handshake(ssl_cli, ssl_srv, 10, NULL)); + + /* NULL means the server sent no extension. Before the fix this was a + * present-but-empty list. */ + ExpectNull(cb_arg.names2); + + wolfSSL_free(ssl_cli); + wolfSSL_CTX_free(ctx_cli); + wolfSSL_free(ssl_srv); + wolfSSL_CTX_free(ctx_srv); +#endif + return EXPECT_RESULT(); +} + +/* A certificate_authorities list shorter than 3 bytes must be rejected on + * receive. TLSX_Parse only size-checks ClientHello and ServerHello, so a + * CertificateRequest used to slip an empty list through. */ +int test_certificate_authorities_short_parse(void) { + EXPECT_DECLS; +#if defined(WOLFSSL_TLS13) && !defined(NO_CERTS) && \ + !defined(WOLFSSL_NO_CA_NAMES) && defined(OPENSSL_EXTRA) && \ + !defined(NO_WOLFSSL_CLIENT) && !defined(NO_TLS) + WOLFSSL_CTX* ctx = NULL; + WOLFSSL* ssl = NULL; + Suites suites; + /* type = 0x002f, size = 2, authorities list length = 0 */ + const byte emptyList[] = { 0x00, 0x2f, 0x00, 0x02, 0x00, 0x00 }; + + XMEMSET(&suites, 0, sizeof(suites)); + + ExpectNotNull(ctx = wolfSSL_CTX_new(wolfTLSv1_3_client_method())); + ExpectNotNull(ssl = wolfSSL_new(ctx)); + + ExpectIntEQ(TLSX_Parse(ssl, emptyList, (word16)sizeof(emptyList), + certificate_request, &suites), + WC_NO_ERR_TRACE(BUFFER_ERROR)); + + wolfSSL_free(ssl); + wolfSSL_CTX_free(ctx); +#endif + return EXPECT_RESULT(); +} + +/* An empty CA list must send no certificate_authorities extension at all. + * We used to send an empty one, which RFC 8446 section 4.2.4 says is too + * short, so the peer sent a decode_error alert and the handshake failed. */ +int test_certificate_authorities_empty_client_hello(void) { + EXPECT_DECLS; +#if !defined(NO_WOLFSSL_CLIENT) && !defined(NO_WOLFSSL_SERVER) && \ + !defined(WOLFSSL_NO_CA_NAMES) && !defined(NO_BIO) && \ + !defined(NO_CERTS) && (defined(OPENSSL_EXTRA) || \ + defined(OPENSSL_EXTRA_X509_SMALL)) && (defined(OPENSSL_ALL) || \ + defined(WOLFSSL_NGINX) || defined(HAVE_LIGHTY)) && \ + defined(WOLFSSL_TLS13) && defined(HAVE_MANUAL_MEMIO_TESTS_DEPENDENCIES) + + struct test_params { + method_provider client_meth; + method_provider server_meth; + int doUdp; + } params[] = { + /* TLS >= 1.3 only */ +#ifdef WOLFSSL_TLS13 + {wolfTLSv1_3_client_method, wolfTLSv1_3_server_method, 0}, +#endif +#ifdef WOLFSSL_DTLS13 + {wolfDTLSv1_3_client_method, wolfDTLSv1_3_server_method, 1}, +#endif + }; + size_t i; + + for (i = 0; i < sizeof(params) / sizeof(*params); i++) { + struct test_memio_ctx test_ctx; + WOLFSSL_CTX *ctx_srv = NULL; + WOLFSSL *ssl_srv = NULL; + WOLFSSL_CTX *ctx_cli = NULL; + WOLFSSL *ssl_cli = NULL; + WOLF_STACK_OF(X509_NAME) *cb_arg = NULL; + WOLF_STACK_OF(X509_NAME) *empty = NULL; + + if (EXPECT_FAIL()) + break; + + XMEMSET(&test_ctx, 0, sizeof(test_ctx)); + + ExpectIntEQ(0, test_memio_setup(&test_ctx, &ctx_cli, &ctx_srv, + &ssl_cli, &ssl_srv, params[i].client_meth, + params[i].server_meth)); + + wolfSSL_CTX_set_cert_cb(ctx_srv, certificate_authorities_server_cb, + &cb_arg); + + /* ssl_cli takes ownership of the empty stack. */ + ExpectNotNull(empty = wolfSSL_sk_X509_NAME_new(NULL)); + wolfSSL_set0_CA_list(ssl_cli, empty); + + ExpectIntEQ(0, test_memio_do_handshake(ssl_cli, ssl_srv, 10, NULL)); + + /* The peer list is only built while reading the extension, so NULL + * here means the client never sent one. */ + ExpectNull(cb_arg); + + wolfSSL_shutdown(ssl_cli); + wolfSSL_free(ssl_cli); + wolfSSL_CTX_free(ctx_cli); + wolfSSL_free(ssl_srv); + wolfSSL_CTX_free(ctx_srv); + } +#endif + return EXPECT_RESULT(); +} + /* Test that the SNI size calculation returns 0 on overflow instead of * wrapping around to a small value (integer overflow vulnerability). */ int test_TLSX_SNI_GetSize_overflow(void) diff --git a/tests/api/test_tls_ext.h b/tests/api/test_tls_ext.h index 40041c0533..7d7df6b4ef 100644 --- a/tests/api/test_tls_ext.h +++ b/tests/api/test_tls_ext.h @@ -36,6 +36,9 @@ int test_tls13_ticket_age_out_of_window(void); int test_wolfSSL_DisableExtendedMasterSecret(void); int test_certificate_authorities_certificate_request(void); int test_certificate_authorities_client_hello(void); +int test_certificate_authorities_empty_client_hello(void); +int test_certificate_authorities_empty_cert_request(void); +int test_certificate_authorities_short_parse(void); int test_TLSX_TCA_Find(void); int test_TLSX_SNI_GetSize_overflow(void); int test_TLSX_ECH_msg_type_validation(void);