Fix cert authorities empty list

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));
         }

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),

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)

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);