fix: S26.02 MbedTlsStream Open unwinds inner transport and SSL state on failure

DEVLOG.md

@@ -11743,3 +11743,71 @@ MISRA rule — different category, doesn't set precedent.
 ### Open questions
 
 - None.
+
+## 2026-05-24 — S26.02 MbedTlsStream Open unwinds + per-failure-point error reporting
+
+### Decisions
+
+- **Single-line tail unwind, not per-helper unwind.** Open's failure
+  tail is `if (!ok) MbedTlsStream_Close(base);` — one statement, one
+  exit point. `Close` is already idempotent (eager `mbedtls_ssl_init`
+  / `_config_init` in `Initialise` means the `*_free` calls are safe
+  whether or not Open reached its allocating step) so it does the
+  right thing regardless of which step tripped. Considered extracting
+  a `MbedTlsStream_UnwindOnFailure` helper; rejected — one statement
+  is one statement.
+- **Five protocol-level error codes, not five C-call codes.** New
+  `MbedTlsStreamErrors.h` entries (`DEFAULTS_NOT_APPLIED`,
+  `SESSION_INIT_FAILED`, `SERVER_NAME_NOT_SET`, `HANDSHAKE_REJECTED`,
+  `HANDSHAKE_TIMEOUT`) describe what the negotiation failed to
+  achieve, not which mbedTLS function returned non-zero. David's
+  guidance: the integrator should see "TLS handshake rejected by
+  peer or local verification" — not "mbedtls_ssl_handshake returned
+  MBEDTLS_ERR_SSL_BAD_INPUT_DATA". Sets the pattern for S26.03
+  (OpenSSL parity).
+- **PerformHandshake `||` split into two `else if` branches.** Was
+  one combined exit condition for both hard-error and budget-
+  exhausted exits; now two branches so each can emit its own typed
+  code. clang-tidy's `bugprone-branch-clone` does not fire — the
+  two emissions take different enum constants.
+- **Inner-transport Open failure is not our error to report.**
+  When `SolidSyslogStream_Open(transport, addr)` returns false (the
+  first `&&` short-circuit in MbedTlsStream_Open), MbedTlsStream
+  emits nothing and just returns false. The transport owns its own
+  diagnostics; the unwind tail still runs and closes the not-quite-
+  opened transport (idempotent on `INVALID_FD`).
+- **`ReCreateHandleWithUpdatedConfig` extended to fully reset the
+  fixture.** Was destroy-handle-only; now also destroys+recreates
+  the transport, resets MbedTlsFake counters, and re-installs the
+  ErrorHandlerFake. Required so the new
+  `CHECK_OPEN_UNWOUND_WITH_ERROR(transport, code)` macro's `== 1`
+  count assertions see counts from the post-ReCreate Open only. The
+  existing five callers (CaChain / Rng / hostname / OwnCert tests)
+  were unaffected — they only ever asserted counts produced by the
+  one Open they invoke.
+- **No defensive close-before-open in PosixTcpStream.** Considered
+  hardening `PosixTcpStream_Open` to close any pre-existing fd
+  before allocating a new one (would mask the bug class entirely).
+  Rejected per David: the contract today is "every `Stream_Open` is
+  preceded by `Stream_Close`", StreamSender honours it, and the
+  unwind added by this story restores the contract for the mbedTLS
+  failure path. Defensive close at the transport would paper over
+  future contract violations rather than surface them.
+
+### Deferred
+
+- **OpenSSL parity (S26.03).** Same shape: `TlsStream_Open` has the
+  same chained-`&&` failure paths, the same SSL state to free, the
+  same socket fd to close. Story to be filed after this one merges,
+  using the test pattern (`CHECK_OPEN_UNWOUND_WITH_ERROR` macro,
+  protocol-level error codes, recovery test) proven here.
+- **`mbedtls_ssl_close_notify` on a never-handshaked context.**
+  Today's `Close` calls close_notify unconditionally; on a context
+  where `ssl_setup` was never reached, close_notify returns
+  `MBEDTLS_ERR_SSL_BAD_INPUT_DATA` which we silently swallow.
+  Harmless but noisy in mbedTLS debug builds. Could add a "session
+  ever live?" guard; out of scope for S26.02.
+
+### Open questions
+
+- None.

Platform/MbedTls/Interface/SolidSyslogMbedTlsStreamErrors.h

@@ -11,6 +11,11 @@ EXTERN_C_BEGIN
     {
         MBEDTLSSTREAM_ERROR_POOL_EXHAUSTED,
         MBEDTLSSTREAM_ERROR_UNKNOWN_DESTROY,
+        MBEDTLSSTREAM_ERROR_DEFAULTS_NOT_APPLIED,
+        MBEDTLSSTREAM_ERROR_SESSION_INIT_FAILED,
+        MBEDTLSSTREAM_ERROR_SERVER_NAME_NOT_SET,
+        MBEDTLSSTREAM_ERROR_HANDSHAKE_REJECTED,
+        MBEDTLSSTREAM_ERROR_HANDSHAKE_TIMEOUT,
         MBEDTLSSTREAM_ERROR_MAX
     };
 

Platform/MbedTls/Source/SolidSyslogMbedTlsStream.c

@@ -4,9 +4,13 @@
 #include <mbedtls/ssl.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdint.h>
 
+#include "SolidSyslogError.h"
+#include "SolidSyslogMbedTlsStreamErrors.h"
 #include "SolidSyslogMbedTlsStreamPrivate.h"
 #include "SolidSyslogNullStream.h"
+#include "SolidSyslogPrival.h"
 #include "SolidSyslogStream.h"
 #include "SolidSyslogStreamDefinition.h"
 
@@ -97,17 +101,30 @@ static inline bool MbedTlsStream_Open(struct SolidSyslogStream* base, const stru
         MbedTlsStream_InstallTransportCallbacks(self);
         ok = MbedTlsStream_PerformHandshake(self);
     }
+    if (!ok)
+    {
+        MbedTlsStream_Close(base);
+    }
     return ok;
 }
 
 static inline bool MbedTlsStream_ApplySslConfigDefaults(struct SolidSyslogMbedTlsStream* self)
 {
-    return mbedtls_ssl_config_defaults(
-               &self->SslConfig,
-               MBEDTLS_SSL_IS_CLIENT,
-               MBEDTLS_SSL_TRANSPORT_STREAM,
-               MBEDTLS_SSL_PRESET_DEFAULT
-           ) == 0;
+    bool ok = mbedtls_ssl_config_defaults(
+                  &self->SslConfig,
+                  MBEDTLS_SSL_IS_CLIENT,
+                  MBEDTLS_SSL_TRANSPORT_STREAM,
+                  MBEDTLS_SSL_PRESET_DEFAULT
+              ) == 0;
+    if (!ok)
+    {
+        SolidSyslog_Error(
+            SOLIDSYSLOG_SEVERITY_ERROR,
+            &MbedTlsStreamErrorSource,
+            (uint8_t) MBEDTLSSTREAM_ERROR_DEFAULTS_NOT_APPLIED
+        );
+    }
+    return ok;
 }
 
 /* TLS policy owned by the library — set per-ssl_config so it cannot leak
@@ -125,7 +142,16 @@ static inline void MbedTlsStream_ApplyTlsPolicy(struct SolidSyslogMbedTlsStream*
 
 static inline bool MbedTlsStream_BindContextToConfig(struct SolidSyslogMbedTlsStream* self)
 {
-    return mbedtls_ssl_setup(&self->SslContext, &self->SslConfig) == 0;
+    bool ok = mbedtls_ssl_setup(&self->SslContext, &self->SslConfig) == 0;
+    if (!ok)
+    {
+        SolidSyslog_Error(
+            SOLIDSYSLOG_SEVERITY_ERROR,
+            &MbedTlsStreamErrorSource,
+            (uint8_t) MBEDTLSSTREAM_ERROR_SESSION_INIT_FAILED
+        );
+    }
+    return ok;
 }
 
 static inline bool MbedTlsStream_ConfigureExpectedHostname(struct SolidSyslogMbedTlsStream* self)
@@ -134,6 +160,14 @@ static inline bool MbedTlsStream_ConfigureExpectedHostname(struct SolidSyslogMbe
     if (self->Config.ServerName != NULL)
     {
         ok = mbedtls_ssl_set_hostname(&self->SslContext, self->Config.ServerName) == 0;
+        if (!ok)
+        {
+            SolidSyslog_Error(
+                SOLIDSYSLOG_SEVERITY_ERROR,
+                &MbedTlsStreamErrorSource,
+                (uint8_t) MBEDTLSSTREAM_ERROR_SERVER_NAME_NOT_SET
+            );
+        }
     }
     return ok;
 }
@@ -147,7 +181,9 @@ static inline void MbedTlsStream_InstallTransportCallbacks(struct SolidSyslogMbe
  * Each call may return WANT_READ/WANT_WRITE while waiting for the multi-RTT
  * handshake to progress; we sleep briefly between attempts (avoiding a busy
  * spin) until either the handshake completes, hits a hard error, or the
- * bounded budget expires. Same shape as OpenSSL's TlsStream_PerformHandshake. */
+ * bounded budget expires. Each non-success exit emits a distinct
+ * protocol-level error code so the integrator can tell rejection from
+ * timeout. Same shape as OpenSSL's TlsStream_PerformHandshake. */
 static inline bool MbedTlsStream_PerformHandshake(struct SolidSyslogMbedTlsStream* self)
 {
     int totalSleptMs = 0;
@@ -162,8 +198,22 @@ static inline bool MbedTlsStream_PerformHandshake(struct SolidSyslogMbedTlsStrea
             result = true;
             done = true;
         }
-        else if (!MbedTlsStream_IsRetryableHandshakeRc(rc) || MbedTlsStream_IsHandshakeBudgetExhausted(totalSleptMs))
+        else if (!MbedTlsStream_IsRetryableHandshakeRc(rc))
+        {
+            SolidSyslog_Error(
+                SOLIDSYSLOG_SEVERITY_ERROR,
+                &MbedTlsStreamErrorSource,
+                (uint8_t) MBEDTLSSTREAM_ERROR_HANDSHAKE_REJECTED
+            );
+            done = true;
+        }
+        else if (MbedTlsStream_IsHandshakeBudgetExhausted(totalSleptMs))
         {
+            SolidSyslog_Error(
+                SOLIDSYSLOG_SEVERITY_ERROR,
+                &MbedTlsStreamErrorSource,
+                (uint8_t) MBEDTLSSTREAM_ERROR_HANDSHAKE_TIMEOUT
+            );
             done = true;
         }
         else

Platform/MbedTls/Source/SolidSyslogMbedTlsStreamMessages.c

@@ -10,6 +10,16 @@ static const char* MbedTlsStreamError_AsString(uint8_t code)
             "SolidSyslogMbedTlsStream_Create pool exhausted; returning fallback stream",
         [MBEDTLSSTREAM_ERROR_UNKNOWN_DESTROY] =
             "SolidSyslogMbedTlsStream_Destroy called with a handle not issued by this pool",
+        [MBEDTLSSTREAM_ERROR_DEFAULTS_NOT_APPLIED] =
+            "TLS client could not be configured with its default protocol settings; connection not established",
+        [MBEDTLSSTREAM_ERROR_SESSION_INIT_FAILED] =
+            "TLS session could not be initialised against the configured context; connection not established",
+        [MBEDTLSSTREAM_ERROR_SERVER_NAME_NOT_SET] =
+            "TLS server name (SNI / cert match) could not be configured; connection not established",
+        [MBEDTLSSTREAM_ERROR_HANDSHAKE_REJECTED] =
+            "TLS handshake rejected by peer or local verification; connection not established",
+        [MBEDTLSSTREAM_ERROR_HANDSHAKE_TIMEOUT] =
+            "TLS handshake did not complete within the bounded retry budget; connection not established",
     };
     const char* result = "unknown";
     if (code < (uint8_t) MBEDTLSSTREAM_ERROR_MAX)

Tests/MbedTls/SolidSyslogMbedTlsStreamTest.cpp

@@ -5,8 +5,11 @@ extern "C"
 #include <mbedtls/ctr_drbg.h>
 #include <mbedtls/ssl.h>
 
+#include "ErrorHandlerFake.h"
 #include "MbedTlsFake.h"
 #include "SolidSyslogMbedTlsStream.h"
+#include "SolidSyslogMbedTlsStreamErrors.h"
+#include "SolidSyslogPrival.h"
 #include "AddressFake.h"
 #include "SolidSyslogStream.h"
 #include "SolidSyslogStreamDefinition.h"
@@ -17,6 +20,20 @@ extern "C"
 
 using namespace CososoTesting; // NOLINT(google-build-using-namespace) -- test-file scope only; brings ONCE/NEVER/TWICE into scope for CALLED_FUNCTION / CALLED_FAKE
 
+// NOLINTBEGIN(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while) -- macro preserves __FILE__/__LINE__ in failure output; do-while wraps the multi-statement body for safe single-statement use
+#define CHECK_OPEN_UNWOUND_WITH_ERROR(transport, expectedCode)                    \
+    do                                                                            \
+    {                                                                             \
+        LONGS_EQUAL(1, StreamFake_CloseCallCount(transport));                     \
+        LONGS_EQUAL(1, MbedTlsFake_SslFreeCallCount());                           \
+        LONGS_EQUAL(1, MbedTlsFake_SslConfigFreeCallCount());                     \
+        CALLED_FAKE(ErrorHandlerFake_Handle, ONCE);                               \
+        POINTERS_EQUAL(&MbedTlsStreamErrorSource, ErrorHandlerFake_LastSource()); \
+        UNSIGNED_LONGS_EQUAL((expectedCode), ErrorHandlerFake_LastCode());        \
+        LONGS_EQUAL(SOLIDSYSLOG_SEVERITY_ERROR, ErrorHandlerFake_LastSeverity()); \
+    } while (0)
+// NOLINTEND(cppcoreguidelines-macro-usage,cppcoreguidelines-avoid-do-while)
+
 static int NoOpSleepCallCount;
 static int g_lastSleepMs;
 
@@ -37,6 +54,7 @@ TEST_GROUP(SolidSyslogMbedTlsStream)
     void setup() override
     {
         MbedTlsFake_Reset();
+        ErrorHandlerFake_Install(nullptr);
         NoOpSleepCallCount = 0;
         g_lastSleepMs = 0;
         transport = StreamFake_Create();
@@ -54,10 +72,19 @@ TEST_GROUP(SolidSyslogMbedTlsStream)
     }
 
     /* Tests needing config tweaks (CaChain, Rng, ServerName, …) call this
-     * to release setup()'s pool slot, mutate `config`, then re-Create. */
+     * to release setup()'s pool slot, mutate `config`, then re-Create.
+     * Fully resets the fixture (transport, MbedTls fake counters, error
+     * handler) so the test body observes counts from this Open onwards
+     * only — matters for assertions like CHECK_OPEN_UNWOUND_WITH_ERROR
+     * that pin counts at == 1. */
     void ReCreateHandleWithUpdatedConfig()
     {
         SolidSyslogMbedTlsStream_Destroy(handle);
+        StreamFake_Destroy(transport);
+        MbedTlsFake_Reset();
+        ErrorHandlerFake_Install(nullptr);
+        transport = StreamFake_Create();
+        config.Transport = transport;
         handle = SolidSyslogMbedTlsStream_Create(&config);
     }
 
@@ -196,16 +223,34 @@ TEST(SolidSyslogMbedTlsStream, OpenRetriesHandshakeOnWantWrite)
     CALLED_FAKE(MbedTlsFake_SslHandshake, TWICE);
 }
 
-TEST(SolidSyslogMbedTlsStream, OpenFailsWhenHandshakeNeverCompletes)
+TEST(SolidSyslogMbedTlsStream, OpenClosesTransportAndFreesSslStateWhenHandshakeBudgetExhausts)
 {
     /* mbedtls_ssl_handshake always returns WANT_READ — handshake never makes
      * progress, so the bounded budget should expire and Open returns false. */
     ArrangePersistentHandshakeError(MBEDTLS_ERR_SSL_WANT_READ);
 
     CHECK_FALSE(SolidSyslogStream_Open(handle, addr));
+    CHECK_OPEN_UNWOUND_WITH_ERROR(transport, MBEDTLSSTREAM_ERROR_HANDSHAKE_TIMEOUT);
+}
+
+TEST(SolidSyslogMbedTlsStream, SecondOpenAfterFailedFirstOpenSucceeds)
+{
+    /* The recovery contract that the per-failure-point unwinds enable: once
+     * Open's failure tail Closes the transport and frees the SSL state, the
+     * next Open is a clean Open-Close-Open cycle on the transport — Connected
+     * goes false, StreamSender's next reconnect tick re-enters, and the
+     * second handshake completes. Without the unwind, the inner transport
+     * would stay open and PosixTcpStream_Open would clobber its fd. */
+    int handshakeSequence[] = {MBEDTLS_ERR_SSL_BAD_INPUT_DATA, 0};
+    MbedTlsFake_SetSslHandshakeReturnSequence(handshakeSequence, 2);
+
+    CHECK_FALSE(SolidSyslogStream_Open(handle, addr));
+    CHECK_TRUE(SolidSyslogStream_Open(handle, addr));
+    LONGS_EQUAL(2, StreamFake_OpenCallCount(transport));
+    LONGS_EQUAL(1, StreamFake_CloseCallCount(transport));
 }
 
-TEST(SolidSyslogMbedTlsStream, OpenFailsImmediatelyOnHardSslError)
+TEST(SolidSyslogMbedTlsStream, OpenClosesTransportAndFreesSslStateWhenHandshakeFailsHard)
 {
     /* Non-WANT error (e.g. a verify/connection failure) is fail-fast — no
      * retry budget burn, no Sleep. */
@@ -214,6 +259,42 @@ TEST(SolidSyslogMbedTlsStream, OpenFailsImmediatelyOnHardSslError)
     CHECK_FALSE(SolidSyslogStream_Open(handle, addr));
     CALLED_FAKE(MbedTlsFake_SslHandshake, ONCE);
     CALLED_FUNCTION(NoOpSleep, NEVER);
+    CHECK_OPEN_UNWOUND_WITH_ERROR(transport, MBEDTLSSTREAM_ERROR_HANDSHAKE_REJECTED);
+}
+
+/* -------------------------------------------------------------------------
+ * Open failure unwind + error reporting (S26.02). Every failure path after
+ * the first allocating operation must close the inner transport, free both
+ * mbedTLS structs, and emit the matching typed error code so the integrator
+ * sees a protocol-level diagnostic.
+ * ------------------------------------------------------------------------- */
+
+TEST(SolidSyslogMbedTlsStream, OpenClosesTransportAndFreesSslStateWhenSslConfigDefaultsFails)
+{
+    MbedTlsFake_SetSslConfigDefaultsReturn(-1);
+
+    CHECK_FALSE(SolidSyslogStream_Open(handle, addr));
+    CHECK_OPEN_UNWOUND_WITH_ERROR(transport, MBEDTLSSTREAM_ERROR_DEFAULTS_NOT_APPLIED);
+}
+
+TEST(SolidSyslogMbedTlsStream, OpenClosesTransportAndFreesSslStateWhenSslSetupFails)
+{
+    MbedTlsFake_SetSslSetupReturn(-1);
+
+    CHECK_FALSE(SolidSyslogStream_Open(handle, addr));
+    CHECK_OPEN_UNWOUND_WITH_ERROR(transport, MBEDTLSSTREAM_ERROR_SESSION_INIT_FAILED);
+}
+
+TEST(SolidSyslogMbedTlsStream, OpenClosesTransportAndFreesSslStateWhenSetHostnameFails)
+{
+    /* ServerName must be set for ConfigureExpectedHostname to invoke
+     * mbedtls_ssl_set_hostname — otherwise the helper short-circuits to true. */
+    config.ServerName = "syslog.example.com";
+    ReCreateHandleWithUpdatedConfig();
+    MbedTlsFake_SetSslSetHostnameReturn(-1);
+
+    CHECK_FALSE(SolidSyslogStream_Open(handle, addr));
+    CHECK_OPEN_UNWOUND_WITH_ERROR(transport, MBEDTLSSTREAM_ERROR_SERVER_NAME_NOT_SET);
 }
 
 TEST(SolidSyslogMbedTlsStream, SendForwardsBufferToSslWrite)