Skip to content

feat: S08.07 mbedTLS reference integration (host-side adapter + integration tests) #419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
DavidCozens merged 13 commits into from
May 21, 2026
Merged

feat: S08.07 mbedTLS reference integration (host-side adapter + integration tests) #419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
1a0468b
feat: S08.07 slice 1 MbedTlsStream pool scaffold
DavidCozens May 21, 2026
9aa22f6
feat: S08.07 slice 2 MbedTlsStream walking-skeleton happy path
DavidCozens May 21, 2026
29a6b36
feat: S08.07 slice 3 MbedTlsStream cert-validation policy + integrati…
DavidCozens May 21, 2026
1372cc9
fix: S08.07 slice 3 unblock negative-handshake integration tests
DavidCozens May 21, 2026
75bf3a3
feat: S08.07 slice 4 MbedTlsStream bounded handshake retry
DavidCozens May 21, 2026
bc5786f
feat: S08.07 slice 5 MbedTlsStream mTLS + integration scenarios
DavidCozens May 21, 2026
5a4cf61
fix: S08.07 MbedTlsStream BioRecv would-block returns WANT_READ
DavidCozens May 21, 2026
26cc5e5
refactor: S08.07 MbedTlsStream lifecycle + fail-fast Send/Read
DavidCozens May 21, 2026
72856d2
chore: S08.07 MbedTlsFake cleanup
DavidCozens May 21, 2026
8de7b01
chore: S08.07 MbedTls integration harness cleanup
DavidCozens May 21, 2026
73e8d2d
chore: S08.07 MbedTls test cleanup pass
DavidCozens May 21, 2026
f7e8785
ci: S08.07 add integration-linux-mbedtls job
DavidCozens May 21, 2026
6eb7d48
chore: S08.07 address CodeRabbit findings on PR #419
DavidCozens May 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 49 additions & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,49 @@ jobs:
path: build/debug/cpputest_*.xml
retention-days: 1

integration-linux-mbedtls:
runs-on: ubuntu-latest
permissions:
contents: read
checks: write
# cpputest-freertos image carries /opt/mbedtls (and exports MBEDTLS_DIR);
# Tests/MbedTlsIntegration/CMakeLists.txt requires that env var.
container:
image: ghcr.io/davidcozens/cpputest-freertos:sha-bbc958b
options: --user root
env:
GIT_CONFIG_COUNT: 1
GIT_CONFIG_KEY_0: safe.directory
GIT_CONFIG_VALUE_0: '*'

steps:
- uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6

- name: Configure
run: cmake --preset debug

- name: Build integration tests
run: cmake --build --preset debug --target MbedTlsIntegrationTests

- name: Run integration tests
run: cd build/debug && ./Tests/MbedTlsIntegration/MbedTlsIntegrationTests -v -ojunit -k MbedTlsIntegrationTests

- name: Test Report
uses: dorny/test-reporter@v3
if: success() || failure()
Comment on lines +130 to +143

@coderabbitai coderabbitai Bot May 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify checkout hardening + floating refs in this workflow file.
python - <<'PY'
from pathlib import Path
import re

p = Path(".github/workflows/ci.yml")
lines = p.read_text().splitlines()

# Find checkout steps missing persist-credentials: false nearby
missing = []
for i, line in enumerate(lines):
    if re.search(r'^\s*-\s+uses:\s+actions/checkout@', line):
        window = lines[i+1:i+8]
        if not any(re.search(r'^\s*persist-credentials:\s*false\s*$', w) for w in window):
            missing.append(i + 1)

# Find floating dorny/test-reporter refs
floating = []
for i, line in enumerate(lines):
    m = re.search(r'uses:\s*dorny/test-reporter@(.+)$', line.strip())
    if m and not re.fullmatch(r'[0-9a-f]{40}(?:\s+#.*)?', m.group(1)):
        floating.append((i + 1, m.group(1)))

print("checkout_without_persist_credentials_false:", missing)
print("floating_test_reporter_refs:", floating)
PY

Repository: DavidCozens/solid-syslog

Length of output: 390

Harden workflow steps by disabling checkout credential persistence and pinning the test-reporter action to a commit SHA.

  • Add persist-credentials: false to actions/checkout steps missing it (including the checkout step around line 130).
  • Replace all dorny/test-reporter@v3 usages with a full 40-char commit SHA (including line 142).
🔒 Suggested patch 
       - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6
+        with:
+          persist-credentials: false
@@
       - name: Test Report
-        uses: dorny/test-reporter@v3
+        uses: dorny/test-reporter@<full-40-char-commit-sha> # v3
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 130-130: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

[error] 142-142: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 130 - 143, The checkout step using
actions/checkout (the step with uses:
actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd) should include
persist-credentials: false to avoid leaving GITHUB_TOKEN credentials in the
workspace; add that key under the same step. Also replace the test reporter
usage dorny/test-reporter@v3 with a pinned full 40-character commit SHA for the
action (keep the same step name "Test Report" and same inputs) so the workflow
references a fixed revision instead of a floating tag.

@DavidCozens DavidCozens May 21, 2026

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deferred — captured the action items in memory as a CI hardening chore to run on return to main. The same gaps exist on every other actions/checkout and dorny/test-reporter step in this workflow (see the file overall), so a per-PR partial fix would be inconsistent with the surrounding pattern. Will land as a focused chore PR that sweeps the whole workflow file, picking the test-reporter SHA from the latest v3.x release at that time.

@coderabbitai coderabbitai Bot May 21, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DavidCozens — understood, that's a sensible approach. Doing a whole-file sweep in a single chore PR is much cleaner than a piecemeal fix that leaves all the sibling steps inconsistent.

Would you like me to open a GitHub issue to track the CI hardening chore? I can scope it to:

  1. Add persist-credentials: false to all actions/checkout steps in .github/workflows/ci.yml.
  2. Pin all dorny/test-reporter@v3 references to the latest v3.x commit SHA at the time of the chore.

Just let me know and I'll create it now.

🧠 Learnings used 
Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 156
File: .github/workflows/ci.yml:0-0
Timestamp: 2026-04-18T17:13:24.155Z
Learning: In GitHub Actions workflows that run on `windows-2025` runners, avoid using PowerShell `Start-Process` to launch background/daemon-style processes from CI steps. `Start-Process` is unreliable on these runners (the child may not start or may be silently killed), which can leave output/redirect files empty and services/ports not bound.

Use a Bash backgrounding approach instead: `nohup … &` (Git Bash is the default shell on `windows-2025`). For example:
`nohup ./Bdd/otel/bin/otelcol-contrib.exe --config=Bdd/otel/config.yaml > Bdd/output/otelcol.out 2> Bdd/output/otelcol.err &`

Prefer `nohup … &` over `Start-Process` for any background process launched in a CI step on `windows-2025`.

Learnt from: DavidCozens
Repo: DavidCozens/solid-syslog PR: 261
File: .github/workflows/ci.yml:580-580
Timestamp: 2026-05-04T09:20:07.939Z
Learning: In GitHub Actions workflows (notably `quality-monitorv1`), the Quality Monitor only allows `analysis` entries whose `id` matches a parser registry that is bundled/registered by that action version. If `quality-monitorv1` does not register the `iwyu` parser, do not add an `analyze-iwyu`/`analysis` block with `{"id":"iwyu"}`—it will fail at runtime with `No such parser registered: iwyu`. IWYU artifacts may still be downloadable for archival, but the reliable pass/fail gate for IWYU must come from the job-level check that branch protection requires (i.e., the required check’s outcome), not from an `analysis` parser block.

with:
name: Test Results (mbedTLS Integration)
path: build/debug/cpputest_*.xml
reporter: java-junit

- name: Upload JUnit XML
if: success() || failure()
uses: actions/upload-artifact@330a01c490aca151604b8cf639adc76d48f6c5d4 # v5.0.0
with:
name: junit-integration-linux-mbedtls
path: build/debug/cpputest_*.xml
retention-days: 1

integration-windows-openssl:
runs-on: windows-latest
permissions:
Expand Down Expand Up @@ -926,7 +969,7 @@ jobs:

summary:
if: always() && github.event_name == 'pull_request'
needs: [build-linux-gcc, build-linux-tunable-override, build-linux-clang, sanitize-linux-gcc, coverage-linux-gcc, analyze-tidy, analyze-cppcheck, analyze-format, analyze-iwyu, bdd-linux-syslog-ng, build-windows-msvc, bdd-windows-otel, integration-linux-openssl, integration-windows-openssl, build-freertos-host-tdd, build-freertos-target, bdd-freertos-qemu]
needs: [build-linux-gcc, build-linux-tunable-override, build-linux-clang, sanitize-linux-gcc, coverage-linux-gcc, analyze-tidy, analyze-cppcheck, analyze-format, analyze-iwyu, bdd-linux-syslog-ng, build-windows-msvc, bdd-windows-otel, integration-linux-openssl, integration-linux-mbedtls, integration-windows-openssl, build-freertos-host-tdd, build-freertos-target, bdd-freertos-qemu]
runs-on: ubuntu-latest
permissions:
contents: read
Expand Down Expand Up @@ -999,6 +1042,11 @@ jobs:
"name": "integration-linux-openssl",
"pattern": "**/junit-integration-linux-openssl/cpputest_*.xml"
},
{
"id": "junit",
"name": "integration-linux-mbedtls",
"pattern": "**/junit-integration-linux-mbedtls/cpputest_*.xml"
},
{
"id": "junit",
"name": "integration-windows-openssl",
Expand Down
12 changes: 12 additions & 0 deletions .vscode/c_cpp_properties.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"version": 4,
"configurations": [
{
"name": "Linux (devcontainer, debug preset)",
"compileCommands": "${workspaceFolder}/build/debug/compile_commands.json",
"intelliSenseMode": "linux-gcc-x64",
"cStandard": "gnu11",
"cppStandard": "gnu++17"
}
]
}
24 changes: 24 additions & 0 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,26 @@ if(SOLIDSYSLOG_OPENSSL)
endif()
endif()

# mbedTLS-based TLS is the embedded-target reference TLS backend (OpenSSL is too
# large for Cortex-M). Default ON when the MBEDTLS_DIR environment variable is
# set (the cpputest-freertos devcontainer image provides /opt/mbedtls); pass
# -DSOLIDSYSLOG_MBEDTLS=OFF to disable explicitly. Unlike OpenSSL there is no
# find_package — mbedTLS is header-configured (mbedtls_config.h changes ABI),
# so the adapter ships as an INTERFACE pack and consumers compile the sources
# themselves with their own config. See Platform/MbedTls/CMakeLists.txt.
if(DEFINED ENV{MBEDTLS_DIR})
set(_solidsyslog_mbedtls_default ON)
else()
set(_solidsyslog_mbedtls_default OFF)
endif()
option(SOLIDSYSLOG_MBEDTLS "Include mbedTLS-based TLS support" ${_solidsyslog_mbedtls_default})
unset(_solidsyslog_mbedtls_default)
if(SOLIDSYSLOG_MBEDTLS AND NOT DEFINED ENV{MBEDTLS_DIR})
message(FATAL_ERROR
"SOLIDSYSLOG_MBEDTLS=ON but MBEDTLS_DIR is not set; "
"export MBEDTLS_DIR=/path/to/mbedtls or pass -DSOLIDSYSLOG_MBEDTLS=OFF.")
endif()

# Allocation strategy (E11). Currently only "static" ships — internal static
# pools allocate instance bookkeeping for stateful classes. A future epic
# may add "dynamic" (heap-backed). Selecting "dynamic" now errors hard so
Expand Down Expand Up @@ -170,6 +190,10 @@ if(SOLIDSYSLOG_OPENSSL)
add_subdirectory(Platform/OpenSsl)
endif()

if(SOLIDSYSLOG_MBEDTLS)
add_subdirectory(Platform/MbedTls)
endif()

if(HAVE_WINDOWS_INTERLOCKED OR HAVE_WINDOWS_PLATFORM OR SOLIDSYSLOG_WINSOCK)
add_subdirectory(Platform/Windows)
endif()
Expand Down
23 changes: 23 additions & 0 deletions Core/Interface/SolidSyslogTunablesDefaults.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,4 +647,27 @@
#error "SOLIDSYSLOG_TLS_STREAM_POOL_SIZE must be >= 1"
#endif

/*
* Number of SolidSyslogMbedTlsStream instances the library's internal static
* pool can simultaneously hold. Each instance carries an mbedtls_ssl_context,
* mbedtls_ssl_config, and the integrator's MbedTlsStreamConfig (transport
* pointer, sleep callback, mbedTLS handle pointers — Rng, CaChain, optional
* ClientCertChain/ClientKey).
*
* Default 1 — TLS senders are scoped per destination and almost all
* integrators wire a single TLS sender per process. Bump via
* SOLIDSYSLOG_USER_TUNABLES_FILE if more than one is genuinely needed
* (e.g. multi-destination egress with separate TLS sessions per peer).
*
* Floor: 1. Sub-floor values rejected at compile time.
*/
#ifndef SOLIDSYSLOG_MBED_TLS_STREAM_POOL_SIZE
/* NOLINTNEXTLINE(cppcoreguidelines-macro-usage) -- macro form required for preprocessor visibility (floor #if) and C array-size const-expr. */
#define SOLIDSYSLOG_MBED_TLS_STREAM_POOL_SIZE 1U
#endif

#if SOLIDSYSLOG_MBED_TLS_STREAM_POOL_SIZE < 1
#error "SOLIDSYSLOG_MBED_TLS_STREAM_POOL_SIZE must be >= 1"
#endif

#endif /* SOLIDSYSLOG_TUNABLES_DEFAULTS_H */
4 changes: 4 additions & 0 deletions Core/Source/SolidSyslogErrorMessages.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,5 +141,9 @@
"SolidSyslogTlsStream_Create pool exhausted; returning fallback stream"
#define SOLIDSYSLOG_ERROR_MSG_TLSSTREAM_UNKNOWN_DESTROY \
"SolidSyslogTlsStream_Destroy called with a handle not issued by this pool"
#define SOLIDSYSLOG_ERROR_MSG_MBEDTLSSTREAM_POOL_EXHAUSTED \
"SolidSyslogMbedTlsStream_Create pool exhausted; returning fallback stream"
#define SOLIDSYSLOG_ERROR_MSG_MBEDTLSSTREAM_UNKNOWN_DESTROY \
"SolidSyslogMbedTlsStream_Destroy called with a handle not issued by this pool"

#endif /* SOLIDSYSLOGERRORMESSAGES_H */
29 changes: 29 additions & 0 deletions Platform/MbedTls/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
# mbedTLS adapter pack — shipped as sources, not a precompiled library.
#
# mbedTLS is header-configured: the integrator's mbedtls_config.h flows
# through upstream headers and changes the size/layout of mbedtls_ssl_context,
# mbedtls_x509_crt, mbedtls_pk_context, and the visible API surface.
# Compiling our adapter once into libSolidSyslog.a would silently break
# consumers that use a different mbedtls_config.h.
#
# Instead, this is an INTERFACE library — each consumer (Bdd/Targets/FreeRtos,
# Tests/MbedTls/*Test, downstream integrator apps) recompiles the adapter
# sources from Source/ with its own mbedtls_config.h on the include path.
# Mirrors Platform/FreeRtos/ and Platform/FatFs/ — see
# project_header_configured_platforms memory.

add_library(SolidSyslogMbedTls INTERFACE)

# No adapter sources at S08.07 slice 1 — first content
# (SolidSyslogMbedTlsStream.h plus the three-TU split) lands in the
# slice-1 green step. The INTERFACE target exists so the directory and
# CMake wiring are in place from the plumbing slice.

target_include_directories(SolidSyslogMbedTls INTERFACE
${CMAKE_CURRENT_SOURCE_DIR}/Interface
)

install(DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}/Interface/
DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}
FILES_MATCHING PATTERN "*.h"
)
35 changes: 35 additions & 0 deletions Platform/MbedTls/Interface/SolidSyslogMbedTlsStream.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#ifndef SOLIDSYSLOGMBEDTLSSTREAM_H
#define SOLIDSYSLOGMBEDTLSSTREAM_H

#include "ExternC.h"
#include "SolidSyslogSleep.h"

struct SolidSyslogStream;

/* Forward declarations keep the public header free of any mbedTLS include.
* Integrators include the relevant mbedTLS headers themselves before this
* one to bring the types into scope. See project_mbedtls_di_handles. */
struct mbedtls_ctr_drbg_context;
struct mbedtls_x509_crt;
struct mbedtls_pk_context;

EXTERN_C_BEGIN

struct SolidSyslogMbedTlsStreamConfig
{
struct SolidSyslogStream* Transport; /* underlying byte stream — caller owns */
SolidSyslogSleepFunction
Sleep; /* drives bounded handshake retry between WANT_READ/WANT_WRITE polls — required */
struct mbedtls_ctr_drbg_context* Rng; /* seeded CTR-DRBG — caller owns */
struct mbedtls_x509_crt* CaChain; /* trust anchors — caller owns */
const char* ServerName; /* SNI + cert hostname check; NULL to skip */
struct mbedtls_x509_crt* ClientCertChain; /* leaf (+ intermediates); NULL = no mTLS */
struct mbedtls_pk_context* ClientKey; /* matching private key; NULL = no mTLS */
};

struct SolidSyslogStream* SolidSyslogMbedTlsStream_Create(const struct SolidSyslogMbedTlsStreamConfig* config);
void SolidSyslogMbedTlsStream_Destroy(struct SolidSyslogStream * base);

EXTERN_C_END

#endif /* SOLIDSYSLOGMBEDTLSSTREAM_H */
Loading
Loading