feat: S08.07 FreeRTOS BDD target — TLS via mbedTLS#421
Conversation
Compile and link mbedTLS into the FreeRTOS QEMU BDD ELF. No behaviour yet beyond proving the build stack is healthy — the new TlsSender wrapper returns a no-op SolidSyslogSender so the linker has something to satisfy its eventual call site in slice 6b without yet pulling mbedTLS into the reachable graph (--gc-sections drops all of mbedTLS for now). - Add Platform/MbedTls sources to the FreeRTOS executable so future MbedTlsStream_Create from main.c will reach them. - Pull mbedTLS upstream into the cross build via add_subdirectory with the same EXCLUDE_FROM_ALL + -w + tidy/cppcheck-disabled pattern used by Tests/MbedTlsIntegration/CMakeLists.txt. Three new file-scope variables in the new CMake section (ENABLE_PROGRAMS, ENABLE_TESTING, MBEDTLS_FATAL_WARNINGS) force-OFF the upstream demo binaries and tests we never want in our build. - Bdd/Targets/FreeRtos/mbedtls_user_config.h: integrator overrides wired via MBEDTLS_USER_CONFIG_FILE. Disable the upstream code paths that assume a Unix/Windows host — platform entropy (Cortex-M3 has no /dev/urandom; slice 6b will register a weak entropy callback), filesystem IO (PEMs are baked into the ELF), BSD sockets (our own transport via mbedtls_ssl_set_bio), system clock (no RTC, baked certs carry 2024–2099 validity), threading hooks (per the customer- coexistence contract), PSA on-filesystem storage, and the timing helpers (the adapter manages its own bounded handshake retry). - Add BddTargetTlsSender_MbedTls_FreeRtosTcp.c as a same-shape parallel to BddTargetTlsSender_OpenSsl_PosixTcp.c. Slice 6a placeholder returns a no-op sender; slice 6b composes the real FreeRtosTcpStream + MbedTlsStream + StreamSender stack. - Commit the .devcontainer/devcontainer.json swap to freertos-target (ARM cross + QEMU + GDB + Behave), the container that this slice and the rest of slice 6 are developed in. The container image's xxd dependency for slice 6b's PEM baking is already pushed to ghcr; the in-repo SHA bump lands as part of 6b.
Adds xxd to the host + cross images (CppUTestFreertosDocker commit
d364407 → 1cb0f34 after merge). xxd is consumed by slice 6b's CMake to
bake demo CA / client cert / client key PEMs into the FreeRTOS BDD ELF
via `xxd -i` — there's no filesystem path from the host into QEMU, so
the PEMs travel as static const arrays in rodata and feed straight into
mbedtls_x509_crt_parse / _pk_parse_key.
Both cpputest-freertos (host TDD) and cpputest-freertos-cross (cross
build, QEMU, behave) get bumped together per docs/containers.md
("always update both rows together").
…target
The FreeRTOS QEMU BDD target now has a working TLS path. mbedTLS pulls
into the reachable graph (--gc-sections no longer drops it: text 125 KB
→ 642 KB) and the switching sender's slot 2 is wired to the new mbedTLS
stack instead of falling through to NullSender.
- CMake bakes Bdd/syslog-ng/tls/{ca,client}.pem + client.key into
generated headers via xxd -i (cat-then-NUL trick deliberately not
used; the wrapper TU does the NUL-terminate copy at parse time
instead).
- BddTargetTlsSender_MbedTls_FreeRtosTcp.c owns the full TLS stack
composition: entropy + CTR_DRBG seed (one-shot, idempotent),
baked-PEM parse to mbedtls_x509_crt + mbedtls_pk_context handles,
and the FreeRtosTcpStream + MbedTlsStream + StreamSender chain.
Diagnostic printfs and vTaskDelay(1) yields surround each major
init step — QEMU emulates Cortex-M3 at host speed, but cert/key
parses can still take seconds, and the yields keep lower-priority
tasks alive instead of starving them through a multi-second
monopoly of the service-thread.
- Boot diagnostics use printf rather than SolidSyslog_Error because
main installs the error handler AFTER BddTargetTlsSender_Create
runs; the default no-op handler would otherwise swallow the WARNING
that documents the demo-only entropy.
- Slice 6b ships TLS-only (mtls=false) in main.c. The FreeRTOS
BDD-harness command flow has the tls/mtls choice arrive over the
UART AFTER boot, so mTLS support either needs a fourth switching-
sender slot, a runtime endpoint-dispatch shim, or boot-time
configuration. Picked up in slice 6c.
Smoke-tested under QEMU mps2-an385: target boots cleanly, mbedTLS
init prints emit in <3s, interactive prompt appears, no regression
on the existing UDP/TCP slots.
…imeout - ci/docker-compose.bdd.yml: behave-freertos tag filter now admits @tls (was excluded with a "until S08.07 lights up TLS on FreeRTOS" note). @Mtls is excluded by virtue of not matching the (@udp or @tcp or @tls) clause — its dual-mode dispatch over the switching sender is still outstanding under slice 6c. - Bdd/features/steps/syslog_steps.py: wait_for_prompt default bumped 30s -> 120s. Linux + Windows native still complete in under a second so the change is invisible there; FreeRTOS QEMU first-boot has to parse the baked mbedTLS RSA client key plus bring up Plus-TCP's IP task, and the local smoke test measured ~60s to reach the prompt. Recorded in [[feedback-qemu-bdd-timeouts-generous]] so future QEMU deadline choices default to "budget for QEMU first" rather than reusing host integration numbers. CI's bdd-freertos-qemu now drives the real end-to-end TLS handshake against the syslog-ng oracle (port 6514, demo CA / server cert from Bdd/syslog-ng/tls/, baked into the ELF via xxd -i in slice 6b).
|
@coderabbitai pause |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR completes FreeRTOS TLS/mTLS support via mbedTLS by adding the transport sender implementation, FreeRTOS build system integration with certificate baking, runtime switching infrastructure, capacity threshold callback signaling, and comprehensive documentation. Container images, CI jobs, and devcontainer references are updated to reflect new build tooling. ChangesFreeRTOS mbedTLS Integration
Capacity Threshold Callback Feature
Infrastructure and Documentation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Actions performedReviews paused. |
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1286 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
…eds on FreeRTOS Two missing integrator-side init steps were sinking every TLS handshake on the QEMU mps2-an385 BDD target. Both surface as opaque mbedTLS errors: 1. mbedtls_ssl_setup returned MBEDTLS_ERR_SSL_ALLOC_FAILED (-0x7F00). mbedTLS calls libc calloc, which on newlib routes through _sbrk into the 4 KiB syscall heap in Bdd/Targets/FreeRtos/Common/Syscalls.c — far too small for a single TLS context. Defined MBEDTLS_PLATFORM_MEMORY in the user config and added mbedtls_platform_set_calloc_free wired to pvPortMalloc / vPortFree so allocations land in the 96 KiB FreeRTOS heap_4 region — the standard FreeRTOS+mbedTLS integration. 2. mbedtls_ssl_handshake then returned MBEDTLS_ERR_ERROR_GENERIC_ERROR (-0x0001) at the first state transition with no BIO traffic. mbedTLS 3.6's TLS 1.3 path drives PSA crypto, and PSA's built-in entropy collector returns PSA_ERROR_INSUFFICIENT_ENTROPY (-148) on MBEDTLS_NO_PLATFORM_ENTROPY targets. Defined MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG, provided mbedtls_psa_external_get_random that feeds PSA from the same CTR_DRBG the classic API uses, and call psa_crypto_init() AFTER seeding the DRBG so the first PSA op has a working source. Also shrunk MBEDTLS_SSL_IN_CONTENT_LEN to 4096 and OUT to 2048 (from the 16 KiB default) — comfortable for the BDD oracle's cert chain + the messages we send, and frees ~28 KiB per context for FreeRTOS-Plus-TCP socket buffers and the SolidSyslog tasks. Local: 39/39 freertos BDD scenarios pass, including both @tls and the @tls13 scenario. No changes under Platform/MbedTls/, so the customer coexistence contract is intact.
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1286 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
The FreeRTOS BDD target spawns the TLS sender once at boot — before the behave harness has sent `set transport tls` / `set transport mtls` over the UART — so the per-startup `mtls` flag that the POSIX and Windows targets read from argv can't reach this Create call in time. Resolve by sharing one TLS sender / TLS stream / TCP socket across both modes and dispatching the destination port at Connect time: - BddTargetSwitchConfig gains an mtlsMode bool tracked by SetByName and exposed via BddTargetSwitchConfig_IsMtlsMode(). `tls` clears it, `mtls` sets it, both route through BDD_TARGET_SWITCH_TLS. - BddTargetTlsSender_MbedTls_FreeRtosTcp wires the client cert+key unconditionally (syslog-ng's plain-TLS listener accepts an optional client cert harmlessly, and its mTLS listener requires one), then plumbs DispatchEndpoint / DispatchEndpointVersion as the StreamSender Endpoint pair. Those read IsMtlsMode at each Connect call and pick between BddTargetTlsConfig (port 6514) and BddTargetMtlsConfig (port 6515). The `mtls` parameter on the wrapper's Create signature is retained for cross-platform contract uniformity but ignored. - ci/docker-compose.bdd.yml admits @Mtls in the freertos behave filter: `(@udp or @tcp or @tls or @Mtls)`. Local full freertos suite: 40 scenarios pass / 0 fail / 9 skipped (intentional @freertoswip / @windows_wip). The new @Mtls scenario from mtls_transport.feature passes in ~3s.
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1286 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with 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.
Inline comments:
In `@Bdd/Targets/Common/BddTargetTlsSender_MbedTls_FreeRtosTcp.c`:
- Around line 209-254: The five mbedTLS calls mbedtls_ctr_drbg_seed,
psa_crypto_init, mbedtls_x509_crt_parse (for CA and client cert) and
mbedtls_pk_parse_key currently discard their return codes; change each to
capture its int return value, on non-zero log a one-line diagnostic (matching
the existing "[mbedtls]…" style) including the error code, clear/latch
mbedTlsInitialised = false (or call configASSERT for fatal), and abort the init
flow (return/cleanup) so you only set mbedTlsInitialised = true after all five
succeed; update the code paths around mbedtls_x509_crt_parse,
mbedtls_pk_parse_key, mbedtls_ctr_drbg_seed and psa_crypto_init to implement
this error-check-and-early-exit behavior.
In `@Bdd/Targets/FreeRtos/main.c`:
- Line 243: tlsSender (static struct SolidSyslogSender* tlsSender) is allocated
but never freed; update TeardownAll() to properly destroy the TLS sender by
invoking the SolidSyslogSender cleanup/destructor (e.g., call the existing
SolidSyslogSender_Destroy or equivalent cleanup function used in this codebase)
on tlsSender, close any underlying sockets/mbedtls contexts as part of that
call, and then set tlsSender = NULL to avoid dangling pointers and leaked
transport resources.
- Around line 798-809: Update the stale comment around
tlsSender/BddTargetTlsSender_Create and the SwitchingSender block to reflect
that mTLS is now supported and can be selected at runtime via the `set
transport` command (include `mtls`/`tls+mtls` option in the documented transport
list), and remove the “future work” phrasing about mTLS; specifically edit the
comment that references tlsSender = BddTargetTlsSender_Create(resolver, false)
and the SwitchingSender explanation so it documents current runtime behavior
(that `set transport` can choose udp, tcp, tls and mtls and that the system
dispatches accordingly).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9a11e43c-23a2-493f-91aa-f8dd307a0d81
📒 Files selected for processing (12)
.devcontainer/devcontainer.json.devcontainer/docker-compose.yml.github/workflows/ci.ymlBdd/Targets/Common/BddTargetSwitchConfig.cBdd/Targets/Common/BddTargetSwitchConfig.hBdd/Targets/Common/BddTargetTlsSender_MbedTls_FreeRtosTcp.cBdd/Targets/FreeRtos/CMakeLists.txtBdd/Targets/FreeRtos/main.cBdd/Targets/FreeRtos/mbedtls_user_config.hBdd/features/steps/syslog_steps.pyci/docker-compose.bdd.ymldocs/containers.md
…a UART marker
The threshold-crossed callback was deferred from S08.05 on the grounds
that the behave step inspects a host-side marker file (/tmp/...) which a
QEMU guest can't write directly. The actual gap is smaller — semihosting
fopen against a relative path would have worked, but a UART-based marker
is cleaner because it reuses the captured-stdout reader the harness
already runs for the prompt protocol.
Changes:
- Bdd/Targets/FreeRtos/main.c — OnThresholdCrossed callback prints a
line-anchored `[THRESHOLD-CROSSED]` token to the UART, wired into the
BlockStoreConfig (previously NULL). Added `set capacity-threshold N`
to OnSet so the harness's --capacity-threshold flag plumbs through.
- Bdd/features/steps/syslog_steps.py — _threshold_marker_present helper
accepts either the host marker file (Linux/Windows binaries write to
/tmp/solidsyslog_threshold_marker.log unchanged) or the
[THRESHOLD-CROSSED] token in the captured stdout buffer (FreeRTOS).
Both `was invoked` / `was not invoked` steps route through it.
- Bdd/features/steps/target_driver.py — --capacity-threshold added to
the FreeRTOS UART translation table.
- Bdd/features/capacity_threshold.feature — drops @freertoswip; the
feature header documents the dual-channel marker contract.
- ci/docker-compose.bdd.yml — comment refresh; the freertos behave
filter is unchanged (the feature is admitted via @tcp @Buffered @store
which the existing `(@udp or @tcp or @tls or @Mtls)` clause covers).
Local: FreeRTOS suite goes from 40/0 to 42/0 (+2 capacity_threshold);
Linux capacity_threshold still passes via the file path.
New docs/integrating-mbedtls.md walks integrators through wiring
SolidSyslogMbedTlsStream. Covers:
- When to use mbedTLS vs the OpenSSL-backed SolidSyslogTlsStream — the
two adapters share the SolidSyslogStream vtable so callers swap at
link time.
- The coexistence contract — Platform/MbedTls/Source/ never calls
process-global mbedTLS APIs; the integrator owns init.
- The config struct fields and their ownership.
- The process-wide setup ordering (allocator hook → entropy + DRBG seed
→ psa_crypto_init → cert parsing → adapter Create).
- FreeRTOS specifics — heap budget, newlib's syscall heap trap, PSA
external RNG on no-platform-entropy targets, bounded handshake retry.
- Pointers to the reference integration (BDD target).
- A common-failures table that maps the cryptic mbedTLS error codes
surfaced during slice-6c diagnosis back to their root causes.
No code change, no API surface change.
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1286 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
There was a problem hiding this comment.
🧹 Nitpick comments (2)
ci/docker-compose.bdd.yml (1)
116-118: 💤 Low valueComment phrasing may cause confusion.
The comment says "slice 6c (this commit)" but according to the PR objectives, slice 6c is marked as "in progress" with outstanding items. In a PR with multiple commits, "this commit" is ambiguous. Consider rephrasing to "slice 6c" or "S08.07 slice 6c" to avoid implying this is a single atomic commit.
🤖 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 `@ci/docker-compose.bdd.yml` around lines 116 - 118, Update the ambiguous inline comment in ci/docker-compose.bdd.yml that currently references "slice 6c (this commit)" to a clearer label; replace the phrase with either "slice 6c" or the explicit project identifier "S08.07 slice 6c" so the comment doesn’t imply a single atomic commit and accurately reflects the PR status.Bdd/Targets/FreeRtos/main.c (1)
569-581: ⚡ Quick winMove
OnThresholdCrossedbelow its first caller.
OnThresholdCrossedis defined before its first use (Line 623). Keep the forward declaration at the top, and place the definition immediately beneathRebuildWithFileStoreto match file ordering rules.As per coding guidelines: “Within a source file, forward-declare helper functions at top after constants/types, then define immediately beneath the function that first calls them.”
🤖 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 `@Bdd/Targets/FreeRtos/main.c` around lines 569 - 581, Move the definition of OnThresholdCrossed so it appears immediately after the function that first calls it (RebuildWithFileStore) while keeping its forward declaration at the top; locate the forward declaration for OnThresholdCrossed near the constants/types, remove or cut the current definition block starting with "static void OnThresholdCrossed(void* context)" from its current place, and paste that definition directly beneath RebuildWithFileStore so file ordering follows: forward declarations → RebuildWithFileStore → OnThresholdCrossed definition.
🤖 Prompt for all review comments with 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.
Nitpick comments:
In `@Bdd/Targets/FreeRtos/main.c`:
- Around line 569-581: Move the definition of OnThresholdCrossed so it appears
immediately after the function that first calls it (RebuildWithFileStore) while
keeping its forward declaration at the top; locate the forward declaration for
OnThresholdCrossed near the constants/types, remove or cut the current
definition block starting with "static void OnThresholdCrossed(void* context)"
from its current place, and paste that definition directly beneath
RebuildWithFileStore so file ordering follows: forward declarations →
RebuildWithFileStore → OnThresholdCrossed definition.
In `@ci/docker-compose.bdd.yml`:
- Around line 116-118: Update the ambiguous inline comment in
ci/docker-compose.bdd.yml that currently references "slice 6c (this commit)" to
a clearer label; replace the phrase with either "slice 6c" or the explicit
project identifier "S08.07 slice 6c" so the comment doesn’t imply a single
atomic commit and accurately reflects the PR status.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 67f8b331-1239-440e-8f44-f456c394e86e
📒 Files selected for processing (6)
Bdd/Targets/FreeRtos/main.cBdd/features/capacity_threshold.featureBdd/features/steps/syslog_steps.pyBdd/features/steps/target_driver.pyci/docker-compose.bdd.ymldocs/integrating-mbedtls.md
✅ Files skipped from review due to trivial changes (1)
- Bdd/features/steps/target_driver.py
…g-up Three findings on PR #421 — all valid, all in the BDD target rather than production library code: 1. **Silent `(void)`-discard on mbedTLS init returns.** The five fail-able calls (`mbedtls_ctr_drbg_seed`, `psa_crypto_init`, two `mbedtls_x509_crt_parse` and `mbedtls_pk_parse_key`) had their rcs discarded. Now each captures the rc, prints a one-line `[mbedtls] ... FAILED rc=...` diagnostic on non-zero, and returns before flipping `mbedTlsInitialised = true`. Surfacing the seed rc exposed a latent bug: DemoEntropySource was registered as `MBEDTLS_ENTROPY_SOURCE_WEAK`, so `mbedtls_entropy_func` never satisfied its `strong_size >= MBEDTLS_ENTROPY_BLOCK_SIZE` check and the seed call always returned ENTROPY_SOURCE_FAILED (-0x0034). `mbedtls_ctr_drbg_random` against the half-seeded context still produced deterministic-but-consistent bytes, which is why earlier handshakes appeared to succeed. Switched the source label to `MBEDTLS_ENTROPY_SOURCE_STRONG` (the audit-trail "demo-only entropy" printf at the end of `EnsureMbedTlsInitialised` is the real quality assertion — the STRONG/WEAK label is structural, not a quality claim). 2. **`tlsSender` not destroyed in TeardownAll.** Added `BddTargetTlsSender_Destroy()` + `tlsSender = NULL` after the existing `SolidSyslogSwitchingSender_Destroy` so the inner MbedTlsStream + FreeRtosTcpStream pool slots release on shutdown paths. 3. **Stale TLS/mTLS comments around tlsSender setup.** The block still said "mTLS support is slice 6c work and may require an extra BDD_TARGET_SWITCH_MTLS slot" — now reflects the dual-port dispatcher that already shipped. The `set transport` command-list comment includes `mtls` alongside the other transports. Local: 42/42 freertos BDD scenarios pass, Linux capacity_threshold unaffected.
…udience
The first draft of this file was structured like a generic mbedTLS
tutorial — a flat init checklist plus a footprint discussion. That left
the actual integration question unanswered: "I'm here because I want
SolidSyslog on top of mbedTLS — what do *I* plug in?"
This rewrite restructures the document around two concrete integrator
scenarios:
- **Scenario A — you already have Mbed TLS in your image.** Three steps:
pick a `SolidSyslogStream` byte transport, fill in
`SolidSyslogMbedTlsStreamConfig` with your existing handles, wire
`tlsStream` into a `SolidSyslogStreamSender`. The coexistence contract
documents the auditable not-touched list so an integrator with an
existing Mbed TLS deployment can be confident the adapter won't fight
their setup.
- **Scenario B — you don't have Mbed TLS yet.** Defers to upstream Mbed
TLS porting docs for the bring-up itself, then gives the integrator a
SolidSyslog-specific checklist of what we actually consume (seeded
CTR_DRBG with at least one STRONG entropy source, psa_crypto_init
after the DRBG seed, parsed CA chain, optional client cert + key, byte
transport).
The FreeRTOS-specific gotchas (newlib syscall heap, PSA external RNG,
record-buffer sizing) move to a dedicated section labelled as integrator
checklist items rather than baked into the body — they only apply on
that family of targets.
The "what this adapter does not own" footer makes the dependency-injection
contract explicit so integrators know which problems remain theirs (PEM
parsing, rotation, HSM, per-connection config).
No code change.
…62443.md, rfc-compliance.md
The mbedTLS reference adapter (SolidSyslogMbedTlsStream) shipped in
slices 1–5 (host) and slices 6a–6c (FreeRTOS BDD target) but was only
ever described in the parent issue, not in the existing reference docs.
This sweep registers it alongside the OpenSSL-backed reference:
- CLAUDE.md — new `SolidSyslogMbedTlsStream.h` row in the
Public-Header-Audiences table (alongside the existing
`SolidSyslogTlsStream.h` row). Updates the StreamSender row to
enumerate `SolidSyslogMbedTlsStream` alongside `SolidSyslogTlsStream`
as a TLS-capable Stream backend. Points readers at the integrator
guide for the full coexistence-contract write-up.
- docs/iec62443.md — new `### Embedded SL4` subsection mapping
`SolidSyslogMbedTlsStream` to the same CR 1.5 / CR 1.8 / CR 2.12 /
CR 3.9 controls the OpenSSL substrate satisfies, with a
coexistence-contract bullet aimed at integrators who already wire
mbedTLS for another subsystem. The SL4 setup-recipe at the bottom of
the page mentions both adapters as alternatives.
- docs/rfc-compliance.md — RFC 5425 section table widens from
OpenSSL-only language to per-row "OpenSSL: … / Mbed TLS: …" coverage,
with an intro paragraph noting the two adapters share the
SolidSyslogStream vtable so the requirement matrix is per-section,
not per-adapter.
No code change; all references are to existing code paths.
Long-form narrative of the slice-6c bring-up: three-layer mbedTLS init diagnosis (heap routing → PSA / TLS-1.3 → STRONG-entropy label), dual-mode TLS/mTLS dispatcher, capacity_threshold marker over UART, the CodeRabbit-driven discovery that ctr_drbg_seed had been failing silently (producing deterministic-but-consistent pseudo-random for the client-side handshake nonce — a happy accident that masked the real bug), and the slice-7 documentation sweep that registers SolidSyslogMbedTlsStream across CLAUDE.md, iec62443.md, rfc-compliance.md, and the new integrating-mbedtls.md.
DavidCozens
changed the title
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1286 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
Reflow the printf arg lists and ctr_drbg_seed call to match clang-format's preferred layout — picked up by analyze-format on the prior push. No behaviour change.
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1286 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
docs/integrating-mbedtls.md (1)
19-30: 💤 Low valueConsider adding language specifier to fenced code block.
The ASCII diagram at line 19 triggers a markdownlint warning for missing language specification. While ASCII art doesn't have a natural language tag, you can add
textto satisfy the linter:```text SolidSyslog_Log ─▶ Buffer ─▶ Sender ─▶ SolidSyslogStreamSender ... ```🤖 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 `@docs/integrating-mbedtls.md` around lines 19 - 30, The fenced ASCII diagram containing SolidSyslog_Log, SolidSyslogMbedTlsStream and SolidSyslogStream should include a Markdown language specifier to satisfy markdownlint; update the diagram's triple-backtick fence to use ```text (i.e., change the opening fence for the ASCII art block to ```text) so the linter recognizes it as plain text while preserving the diagram content.
🤖 Prompt for all review comments with 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.
Inline comments:
In `@Bdd/Targets/Common/BddTargetTlsSender_MbedTls_FreeRtosTcp.c`:
- Around line 224-229: The TLS initialization branches (e.g., failures from
ctr_drbg_seed, mbedtls_x509_crt_parse, mbedtls_pk_parse_key,
mbedtls_ctr_drbg_random, mbedtls_ssl_config_defaults, mbedtls_ssl_conf_rng) must
not just log-and-return from the helper — they need to propagate failure to
BddTargetTlsSender_Create so the TLS slot is disabled early; modify each failing
branch in BddTargetTlsSender_MbedTls_FreeRtosTcp.c to return an error status (or
set a clear failure flag/NULL out the partially-initialized BddTargetTlsSender
object) instead of only printing and returning void, ensure the caller
BddTargetTlsSender_Create checks that return value/flag and aborts creation
(cleaning up drbg / cert / key via existing cleanup functions) so no partial
state is used later (apply same change for the branches around ctr_drbg_seed,
x509 cert parse, pk parse, ssl config/rng setup and the other noted blocks).
---
Nitpick comments:
In `@docs/integrating-mbedtls.md`:
- Around line 19-30: The fenced ASCII diagram containing SolidSyslog_Log,
SolidSyslogMbedTlsStream and SolidSyslogStream should include a Markdown
language specifier to satisfy markdownlint; update the diagram's triple-backtick
fence to use ```text (i.e., change the opening fence for the ASCII art block to
```text) so the linter recognizes it as plain text while preserving the diagram
content.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0d7831da-6dc6-4083-80ae-189650fb643d
📒 Files selected for processing (7)
Bdd/Targets/Common/BddTargetTlsSender_MbedTls_FreeRtosTcp.cBdd/Targets/FreeRtos/main.cCLAUDE.mdDEVLOG.mddocs/iec62443.mddocs/integrating-mbedtls.mddocs/rfc-compliance.md
✅ Files skipped from review due to trivial changes (3)
- docs/rfc-compliance.md
- CLAUDE.md
- DEVLOG.md
…ailure
CodeRabbit's follow-up review on the slice-6c CR-fix push pointed out
that EnsureMbedTlsInitialised's new early-return branches log and bail
but BddTargetTlsSender_Create still builds a TLS sender around whatever
partial state was left in drbg / certs / key — pushing the failure into
later opaque handshake / send errors rather than disabling the TLS slot
cleanly at the point of init failure.
Closes that loop:
- BddTargetTlsSender_Create checks mbedTlsInitialised after
EnsureMbedTlsInitialised. On failure it returns the shared
SolidSyslogNullSender, matching the bad-setup-contract pattern
used elsewhere in the library — the SwitchingSender's tls slot
drops messages cleanly and the file-scope sender / tlsStream /
underlyingStream statics stay NULL.
- BddTargetTlsSender_Destroy now short-circuits when sender is NULL,
so the early-return-from-Create path doesn't try to release pool
slots that were never acquired.
No behaviour change on the happy path: 42/42 freertos BDD scenarios
still pass locally.
Refs CodeRabbit comment 3286071197 on PR #421.
Three small CodeRabbit nitpicks on PR #421: - ci/docker-compose.bdd.yml — drop the ambiguous "(this commit)" phrasing; "S08.07 slice 6c" survives squash-merge correctly. - Bdd/Targets/FreeRtos/main.c — move OnStoreFull, GetCapacityThreshold, and OnThresholdCrossed below RebuildWithFileStore, their first caller. Matches the file-ordering rule in CLAUDE.md ("Helper functions are forward-declared at the top of the file ... and defined immediately beneath the function that first calls them"). The forward declarations near the top of the file are unchanged. - docs/integrating-mbedtls.md — add `text` language specifier to the pipeline-shape ASCII diagram's fenced code block so markdownlint stops warning about the missing tag. No code behaviour change.
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1286 passed, 🙈 2 skipped) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
|
Nitpick acknowledgements for the two CodeRabbit review summaries that don't have an inline thread to reply to — Review 4341024920 (2026-05-21 21:56Z) — two nitpicks, both addressed in b8e3113:
Review 4342764730 (2026-05-22 05:37Z) — one nitpick, addressed in b8e3113:
The Major finding in that same review ( All inline CR threads resolved; CI run 26271306716 green. |
1 participant
Purpose
Closes #272. Lights up the TLS slot on the FreeRTOS QEMU BDD target via
SolidSyslogMbedTlsStreamoverSolidSyslogFreeRtosTcpStream, and registers the Mbed TLS reference adapter across the documentation set (CLAUDE.md row, IEC 62443 SL4 section, RFC 5425 compliance matrix, newdocs/integrating-mbedtls.mdintegrator guide).Change Description
Slice 6a + 6b (already on branch from earlier work): mbedTLS as a cross-compile subproject; integrator overrides in
mbedtls_user_config.h; demo CA + client cert + client key PEMs baked into the ELF viaxxd -i; weak demo-entropy callback + CTR_DRBG seed; PEM-to-handle parsing; fullFreeRtosTcpStream → MbedTlsStream → StreamSendercomposition.Slice 6c — TLS bring-up (this session):
MBEDTLS_PLATFORM_MEMORY+mbedtls_platform_set_calloc_free(pvPortMalloc, vPortFree). mbedTLS's default libccallocwas funnelling through newlib's 4 KiB syscall heap and failing everymbedtls_ssl_setupwithMBEDTLS_ERR_SSL_ALLOC_FAILED.MBEDTLS_PSA_CRYPTO_EXTERNAL_RNG+ anmbedtls_psa_external_get_randomhook that wraps the same CTR_DRBG the classic API uses. mbedTLS 3.6's TLS 1.3 path needspsa_crypto_init()to succeed, and PSA's built-in entropy collector returnsPSA_ERROR_INSUFFICIENT_ENTROPYonMBEDTLS_NO_PLATFORM_ENTROPYtargets.psa_crypto_init()is now called after the DRBG is seeded.MBEDTLS_SSL_IN_CONTENT_LENshrunk to 4096,OUTto 2048 (from the 16 KiB default) to keep the per-context working set tight on the FreeRTOS heap.Slice 6c — dual-mode TLS/mTLS dispatch:
BddTargetSwitchConfiggainsIsMtlsMode();set transport tls|mtlsover UART sets a shared bool. Both modes route throughBDD_TARGET_SWITCH_TLS.BddTargetTlsSender_MbedTls_FreeRtosTcpwires the client cert + key unconditionally and dispatches the Endpoint callback at Connect time — port 6514 for TLS, 6515 for mTLS. One TLS sender / one TLS stream / one TCP socket serve both modes.ci/docker-compose.bdd.ymladmits@mtlsin the freertos behave filter.Slice 6c —
capacity_threshold.featureadmitted on FreeRTOS:OnThresholdCrossedprints a line-anchored[THRESHOLD-CROSSED]token to UART;_threshold_marker_present(context)insyslog_steps.pyaccepts either the host marker file (POSIX / Windows binaries) or the token in the captured stdout buffer (FreeRTOS). Single cross-target assertion.set capacity-threshold Nhandler added to FreeRTOSOnSet;--capacity-thresholdtranslation entry added to the FreeRTOS UART flag table.@freertoswipdropped from the feature.CodeRabbit review fixes:
ctr_drbg_seed,psa_crypto_init, twox509_crt_parse,pk_parse_key) and surfaced each failure as a one-line[mbedtls] … FAILED rc=…diagnostic with early return — replaces the silent(void)casts.DemoEntropySourcewas registered asMBEDTLS_ENTROPY_SOURCE_WEAK, sombedtls_entropy_funcnever satisfied itsstrong_size >= MBEDTLS_ENTROPY_BLOCK_SIZEcheck and ctr_drbg_seed had always been returningENTROPY_SOURCE_FAILED(-0x0034). Earlier handshakes "worked" becausembedtls_ctr_drbg_randomagainst the half-seeded context produced deterministic-but-consistent bytes that syslog-ng accepted from its end of the handshake. Fixed by relabeling the demo sourceMBEDTLS_ENTROPY_SOURCE_STRONG; the audit-trail "demo-only entropy" printf at the end ofEnsureMbedTlsInitialisedis the real quality assertion.BddTargetTlsSender_Destroy()+tlsSender = NULLadded toTeardownAll— the inner pool slots were leaking on shutdown.Slice 7 — documentation sweep:
docs/integrating-mbedtls.md— practical integrator guide structured around two scenarios ("you already have Mbed TLS in your image" / "you don't have Mbed TLS yet"). Covers the byte-transport choice, the config struct, the coexistence contract (auditable not-touched list), and the FreeRTOS-specific gotchas as a labelled integrator checklist.CLAUDE.md—SolidSyslogMbedTlsStream.hrow added to the Public-Header-Audiences table; StreamSender row enumerates the Mbed TLS adapter as a TLS-capable Stream backend.docs/iec62443.md— new### Embedded SL4subsection mapping the Mbed TLS adapter to the same CR 1.5 / CR 1.8 / CR 2.12 / CR 3.9 controls as the OpenSSL substrate. SL4 setup-recipe lists both adapters as alternatives.docs/rfc-compliance.md— RFC 5425 section table widened from OpenSSL-only to per-row "OpenSSL: … / Mbed TLS: …" coverage.Long-form narrative of the bring-up is in
DEVLOG.md.Test Evidence
Local — FreeRTOS QEMU BDD suite (full):
@freertoswipis the oversize-UTF-8 MTU scenario, unreachable because FreeRTOSSOLIDSYSLOG_MAX_MESSAGE_SIZE = 512is below the path MTU)tls_transport.feature(both @tls and @tls13 scenarios),mtls_transport.feature,switching_transport.feature(TCP → TLS mid-run),capacity_threshold.feature(both threshold-fired and threshold-not-fired scenarios),tcp_reconnect.feature,store_*.feature,power_cycle_replay.feature.Local — Linux:
capacity_threshold.featurestill passes via the host marker-file path.CI: see the latest run on this PR.
Areas Affected
Bdd/Targets/Common/BddTargetTlsSender_MbedTls_FreeRtosTcp.c— error-checked init, STRONG entropy label, FreeRTOS heap-routed calloc, PSA external RNG hook, dispatcher endpoint callbacks.Bdd/Targets/Common/BddTargetSwitchConfig.{c,h}—IsMtlsMode()accessor.Bdd/Targets/FreeRtos/main.c—tlsSenderdestroy inTeardownAll,OnThresholdCrossedcallback (UART marker),set capacity-thresholdOnSet handler, refreshed comment block.Bdd/Targets/FreeRtos/mbedtls_user_config.h— PLATFORM_MEMORY, PSA_CRYPTO_EXTERNAL_RNG, SSL_IN/OUT_CONTENT_LEN.Bdd/features/capacity_threshold.feature—@freertoswipremoved; comment block describes the dual-channel marker.Bdd/features/steps/syslog_steps.py—_threshold_marker_present(context)helper; both threshold steps route through it.Bdd/features/steps/target_driver.py—--capacity-thresholdUART translation entry.ci/docker-compose.bdd.yml—@mtlsadmitted in the freertos filter; capacity_threshold scope comment updated.CLAUDE.md,docs/integrating-mbedtls.md(new),docs/iec62443.md,docs/rfc-compliance.md,DEVLOG.md.No changes to
Platform/MbedTls/Source/— the coexistence contract (no process-global Mbed TLS calls in the adapter) is intact and auditable by grep.Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests
Infrastructure