Preserve TLS 1.3 builder errors and packet ownership (N107–N109)

CHANGELOG.md

@@ -5,6 +5,8 @@ Add new entries under `Unreleased`; published release sections are history.
 
 ## Unreleased
 
+- TLS 1.3 handshake builder failures preserve the original error and release
+  the unsent packet, instead of leaking it or trying to send it.
 - TLS servers reject malformed curve and signature-algorithm lists without
   reading past them. Curve selection no longer mistakes a list's length for
   an offered curve.

tests/CMakeLists.txt

@@ -28,6 +28,7 @@ add_subdirectory(atf)
 add_subdirectory(fuzz)
 add_subdirectory(toolchain)
 add_subdirectory(x509)
+add_subdirectory(tls13_handshake_fault)
 add_subdirectory(leak)
 add_subdirectory(concurrent)
 add_subdirectory(bracket)

tests/tls13_handshake_fault/CMakeLists.txt

@@ -0,0 +1,97 @@
+# Host-only bounded fault-injection test for the TLS 1.3 handshake send
+# builders (N-107 server / N-109 client).
+#
+# tests/x509 answers "does the verifier reject what it must and accept what it
+# must". This answers the handshake half of the same question for the builder
+# failure paths: when _nx_secure_tls_send_certificate/_certificate_verify/
+# _finished/_certificate_request fails, the driver must (a) return that status
+# unchanged, (b) release the packet it allocated exactly once, and (c) not send
+# a record for the failed builder. It also asserts that a record-send failure
+# is NOT double-freed (the record layer already owns that release).
+#
+# Unlike tests/x509, this does NOT glob the nx_secure/nx_crypto trees into
+# archives. It compiles ONLY the two drivers under test
+# (_nx_secure_tls_1_3_client_handshake, _nx_secure_tls_1_3_server_handshake)
+# and links them against a hand-written stub surface for every other symbol
+# they reference -- no NX_PACKET pools, no record layer, no ThreadX scheduler,
+# no broad netstack/guest. That is the point: each builder failure can be
+# injected and its release contract observed directly.
+#
+# Base vs fix: on the fixed drivers (bda9d37d) all 9 checks pass. On the
+# unfixed drivers (0fb76b63) the three client builders leak the packet (one
+# check each) and the four server builders drop the status, skip the release
+# and still send the record (three checks each) -- 15 failures. With the
+# shipping flags above, probing the unfixed pin hangs instead: the unfixed
+# server send_certificate_request guards its builder with
+# NX_ASSERT(status == NX_SUCCESS), and NX_ASSERT is gated on NX_DISABLE_ASSERT,
+# not NX_DISABLE_ERROR_CHECKING, so the assert is live and spins forever --
+# the DoS the fix removes. Run the unfixed pin under a timeout, or with
+# -DNX_DISABLE_ASSERT for the deterministic 15-failure count.
+#
+# HOST ONLY: the stubs are native host C; nothing here runs on m68k.
+
+if(CMAKE_CROSSCOMPILING)
+    return()
+endif()
+
+set(_hs_src "${AMINETXDUO_NETXDUO}/nx_secure/src")
+
+add_library(test_tls13_hs_stubs STATIC
+    nx_secure_tls_1_3_handshake_fault_stubs.c)
+add_library(test_tls13_hs_stubs_shipping STATIC
+    nx_secure_tls_1_3_handshake_fault_stubs.c)
+
+# Identical define/port surface to src/tls/CMakeLists.txt + tests/x509, so the
+# shipping arm omits the optional server client-certificate feature. The
+# separate optional arm enables it to reach send_certificate_request; compile
+# its stubs and drivers together so the session layouts cannot be mixed.
+foreach(lib test_tls13_hs_stubs test_tls13_hs_stubs_shipping)
+    target_include_directories(${lib} PUBLIC
+        "${CMAKE_CURRENT_SOURCE_DIR}"
+        "${CMAKE_SOURCE_DIR}/tests/perf/host/shim"      # tx_port.h, first
+        "${AMINETXDUO_NETXDUO}/ports/linux/gnu/inc"     # nx_port.h
+        "${CMAKE_SOURCE_DIR}/port/netxduo-amiga/inc"    # nx_user.h, ours
+        "${AMINETXDUO_NETXDUO}/common/inc"
+        "${AMINETXDUO_THREADX}/common/inc"
+        "${AMINETXDUO_NETXDUO}/nx_secure/inc"
+        "${AMINETXDUO_NETXDUO}/nx_secure/ports"
+        "${AMINETXDUO_NETXDUO}/crypto_libraries/inc")
+
+    target_compile_definitions(${lib} PUBLIC
+        NX_PACKET_ALIGNMENT=8
+        NX_INCLUDE_USER_DEFINE_FILE
+        AMINETXDUO_HOST_VENDORED_TX_PORT="${AMINETXDUO_THREADX}/ports/linux/gnu/inc/tx_port.h"
+        NX_DISABLE_ERROR_CHECKING
+        TX_DISABLE_ERROR_CHECKING
+        NX_SECURE_KEY_CLEAR
+        NX_SECURE_ENABLE_ECC_CIPHERSUITE
+        NX_SECURE_ENABLE_AEAD_CIPHER
+        NX_SECURE_TLS_ENABLE_TLS_1_3)
+endforeach()
+target_compile_definitions(test_tls13_hs_stubs PUBLIC
+    NX_SECURE_ENABLE_CLIENT_CERTIFICATE_VERIFY)
+
+# The two drivers under test are compiled as PART of the executable so their
+# failure paths are exercised with the exact same flags as the stubs.
+add_executable(test_tls13_handshake_fault
+    nx_secure_tls_1_3_handshake_fault_test.c
+    "${_hs_src}/nx_secure_tls_1_3_client_handshake.c"
+    "${_hs_src}/nx_secure_tls_1_3_server_handshake.c")
+
+target_link_libraries(test_tls13_handshake_fault PRIVATE test_tls13_hs_stubs)
+
+target_compile_options(test_tls13_handshake_fault PRIVATE -Wall -Wextra -UAMINETXDUO_IPV6)
+
+include(CTest)
+add_test(NAME tls13_handshake_fault COMMAND test_tls13_handshake_fault)
+
+add_executable(test_tls13_handshake_fault_shipping
+    nx_secure_tls_1_3_handshake_fault_test.c
+    "${_hs_src}/nx_secure_tls_1_3_client_handshake.c"
+    "${_hs_src}/nx_secure_tls_1_3_server_handshake.c")
+target_link_libraries(test_tls13_handshake_fault_shipping PRIVATE
+    test_tls13_hs_stubs_shipping)
+target_compile_options(test_tls13_handshake_fault_shipping PRIVATE
+    -Wall -Wextra -UAMINETXDUO_IPV6)
+add_test(NAME tls13_handshake_fault_shipping
+    COMMAND test_tls13_handshake_fault_shipping)

tests/tls13_handshake_fault/nx_secure_tls_1_3_handshake_fault_stubs.c

@@ -0,0 +1,283 @@
+/*
+ * N-107 / N-109 bounded fault-injection fixture: stub surface.
+ *
+ * Defines every symbol the two handshake drivers reference but that lives
+ * elsewhere in nx_secure/ThreadX. Builders (send_*), the handshake-record
+ * sender, packet allocation, transcript/key hashing, and the message
+ * processors are all controllable so each failure path can be injected.
+ *
+ * Release semantics modelled here match the real code:
+ *   - The three client builders + four server builders own NO packet_release
+ *     internally; the CALLER (handshake driver) owns the packet until it is
+ *     handed to _nx_secure_tls_send_handshake_record.
+ *   - _nx_secure_tls_send_handshake_record owns the release on record-send
+ *     failure (real impl: nx_secure_tls_send_handshake_record.c). The stub
+ *     therefore does NOT touch g_caller_release_count on failure.
+ *   - nx_secure_tls_packet_release is _nx_secure_tls_packet_release under
+ *     NX_SECURE_KEY_CLEAR (the shipping config), so the caller release is the
+ *     single call the driver makes, counted here.
+ */
+
+#include "nx_secure_tls_1_3_handshake_fault_stubs.h"
+
+/* TLS 1.2 downgrade sentinels (defined in nx_secure_tls_send_serverhello.c). */
+const UCHAR _nx_secure_tls_1_1_random[8] = {0};
+const UCHAR _nx_secure_tls_1_2_random[8] = {0};
+
+inject_target_t g_inject                = INJECT_NONE;
+UCHAR           g_record_send_fail_type  = 0;
+UINT            g_caller_release_count   = 0;
+UINT            g_record_send_count      = 0;
+
+/* A single dummy packet handed out by the allocate stub; never dereferenced. */
+static NX_PACKET g_dummy_packet;
+
+void test_reset(void)
+{
+    g_inject               = INJECT_NONE;
+    g_record_send_fail_type = 0;
+    g_caller_release_count  = 0;
+    g_record_send_count     = 0;
+}
+
+/* --- packet allocation / release --- */
+
+UINT _nx_secure_tls_allocate_handshake_packet(NX_SECURE_TLS_SESSION *tls_session,
+                                              NX_PACKET_POOL *packet_pool,
+                                              NX_PACKET **send_packet, ULONG wait_option)
+{
+    (void)tls_session;
+    (void)packet_pool;
+    (void)wait_option;
+    *send_packet = &g_dummy_packet;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_packet_release(NX_PACKET *packet_ptr)
+{
+    (void)packet_ptr;
+    g_caller_release_count++;
+    return NX_SUCCESS;
+}
+
+/* --- record send (owns release on failure, does NOT count a caller release) --- */
+
+UINT _nx_secure_tls_send_handshake_record(NX_SECURE_TLS_SESSION *tls_session,
+                                          NX_PACKET *send_packet, UCHAR handshake_type,
+                                          ULONG wait_option)
+{
+    (void)tls_session;
+    (void)send_packet;
+    (void)wait_option;
+    g_record_send_count++;
+    if (g_record_send_fail_type != 0 && handshake_type == g_record_send_fail_type)
+    {
+        return INJECTED_STATUS;
+    }
+    return NX_SUCCESS;
+}
+
+/* --- key / transcript / hash machinery (succeed unless injected) --- */
+
+UINT _nx_secure_tls_1_3_generate_handshake_keys(NX_SECURE_TLS_SESSION *tls_session)
+{
+    (void)tls_session;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_1_3_generate_session_keys(NX_SECURE_TLS_SESSION *tls_session)
+{
+    (void)tls_session;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_1_3_session_keys_set(NX_SECURE_TLS_SESSION *tls_session, USHORT key_set)
+{
+    (void)tls_session;
+    (void)key_set;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_1_3_transcript_hash_save(NX_SECURE_TLS_SESSION *tls_session,
+                                             UINT hash_index, UINT need_copy)
+{
+    (void)tls_session;
+    (void)hash_index;
+    (void)need_copy;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_handshake_hash_init(NX_SECURE_TLS_SESSION *tls_session)
+{
+    (void)tls_session;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_handshake_hash_update(NX_SECURE_TLS_SESSION *tls_session,
+                                          UCHAR *data, UINT length)
+{
+    (void)tls_session;
+    (void)data;
+    (void)length;
+    return NX_SUCCESS;
+}
+
+/* --- message processors (set the state the driver then acts on) --- */
+
+UINT _nx_secure_tls_process_handshake_header(UCHAR *packet_buffer, USHORT *message_type,
+                                             UINT *header_size, UINT *message_length)
+{
+    /* Real TLS handshake header: 1-byte type, 3-byte big-endian length. */
+    *message_type  = (USHORT)packet_buffer[0];
+    *message_length = ((UINT)packet_buffer[1] << 16) | ((UINT)packet_buffer[2] << 8) |
+                      (UINT)packet_buffer[3];
+    *header_size   = 4;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_process_finished(NX_SECURE_TLS_SESSION *tls_session,
+                                     UCHAR *packet_buffer, UINT message_length)
+{
+    (void)packet_buffer;
+    (void)message_length;
+    /* Client: move to the state that sends Cert/CertVerify/Finished. */
+    tls_session -> nx_secure_tls_client_state = NX_SECURE_TLS_CLIENT_STATE_HANDSHAKE_FINISHED;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_process_clienthello(NX_SECURE_TLS_SESSION *tls_session,
+                                        UCHAR *packet_buffer, UINT message_length)
+{
+    (void)packet_buffer;
+    (void)message_length;
+    /* Negotiate TLS 1.3 so the server proceeds past the version gate. */
+    tls_session -> nx_secure_tls_1_3 = NX_TRUE;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_process_certificate_request(NX_SECURE_TLS_SESSION *tls_session,
+                                                UCHAR *packet_buffer, UINT message_length)
+{
+    (void)tls_session;
+    (void)packet_buffer;
+    (void)message_length;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_process_certificate_verify(NX_SECURE_TLS_SESSION *tls_session,
+                                               UCHAR *packet_buffer, UINT message_length)
+{
+    (void)tls_session;
+    (void)packet_buffer;
+    (void)message_length;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_process_encrypted_extensions(NX_SECURE_TLS_SESSION *tls_session,
+                                                 UCHAR *packet_buffer, UINT message_length)
+{
+    (void)tls_session;
+    (void)packet_buffer;
+    (void)message_length;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_process_remote_certificate(NX_SECURE_TLS_SESSION *tls_session,
+                                               UCHAR *packet_buffer, UINT message_length,
+                                               UINT data_length)
+{
+    (void)tls_session;
+    (void)packet_buffer;
+    (void)message_length;
+    (void)data_length;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_process_serverhello(NX_SECURE_TLS_SESSION *tls_session,
+                                        UCHAR *packet_buffer, UINT message_length)
+{
+    (void)tls_session;
+    (void)packet_buffer;
+    (void)message_length;
+    return NX_SUCCESS;
+}
+
+/* --- builders (client + server). Each returns INJECTED_STATUS when selected. --- */
+
+UINT _nx_secure_tls_send_certificate(NX_SECURE_TLS_SESSION *tls_session, NX_PACKET *send_packet,
+                                     ULONG wait_option)
+{
+    (void)tls_session;
+    (void)send_packet;
+    (void)wait_option;
+    if (g_inject == INJECT_CLIENT_SEND_CERTIFICATE ||
+        g_inject == INJECT_SERVER_SEND_CERTIFICATE)
+    {
+        return INJECTED_STATUS;
+    }
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_send_certificate_verify(NX_SECURE_TLS_SESSION *tls_session, NX_PACKET *send_packet)
+{
+    (void)tls_session;
+    (void)send_packet;
+    if (g_inject == INJECT_CLIENT_SEND_CERTIFICATE_VERIFY ||
+        g_inject == INJECT_SERVER_SEND_CERTIFICATE_VERIFY)
+    {
+        return INJECTED_STATUS;
+    }
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_send_finished(NX_SECURE_TLS_SESSION *tls_session, NX_PACKET *send_packet)
+{
+    (void)tls_session;
+    (void)send_packet;
+    if (g_inject == INJECT_CLIENT_SEND_FINISHED ||
+        g_inject == INJECT_SERVER_SEND_FINISHED)
+    {
+        return INJECTED_STATUS;
+    }
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_send_certificate_request(NX_SECURE_TLS_SESSION *tls_session, NX_PACKET *send_packet)
+{
+    (void)tls_session;
+    (void)send_packet;
+    if (g_inject == INJECT_SERVER_SEND_CERTIFICATE_REQUEST)
+    {
+        return INJECTED_STATUS;
+    }
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_send_serverhello(NX_SECURE_TLS_SESSION *tls_session, NX_PACKET *send_packet)
+{
+    (void)tls_session;
+    (void)send_packet;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_send_encrypted_extensions(NX_SECURE_TLS_SESSION *tls_session, NX_PACKET *send_packet)
+{
+    (void)tls_session;
+    (void)send_packet;
+    return NX_SUCCESS;
+}
+
+UINT _nx_secure_tls_send_clienthello(NX_SECURE_TLS_SESSION *tls_session, NX_PACKET *send_packet)
+{
+    (void)tls_session;
+    (void)send_packet;
+    return NX_SUCCESS;
+}
+
+/* --- ThreadX (only _tx_thread_sleep is referenced by the drivers) --- */
+
+UINT _tx_thread_sleep(ULONG timer_ticks)
+{
+    (void)timer_ticks;
+    return NX_SUCCESS;
+}