feat: S28.11 FreeRtosLwip TLS/mTLS (mbedTLS over LwipRawTcpStream)#481
Conversation
Wire a TLS/mTLS sender into the FreeRTOS+lwIP BDD target: mbedTLS over a second LwipRawTcpStream in the SwitchingSender's TLS slot, mirroring the +TCP target. Adds the mbedTLS CMake subproject + baked PEMs, the lwIP mbedtls_user_config.h, opens the lwIP BDD lane to @tls/@Mtls, adds analyze-tidy/iwyu-freertos-lwip lanes, and promotes the lwIP jobs into summary.needs. Two transport fixes found via packet capture: - Pin the TLS/mTLS host to the numeric slirp gateway (10.0.2.2) with SNI "syslog-ng": the numeric-only LwipRawResolver silently rejected the "syslog-ng" hostname, so the sender never connected. - Disable Nagle on the lwIP TCP pcb (tcp_nagle_disable): Nagle stalled the multi-segment TLS handshake flight mid-exchange. TCP_NODELAY is the right default for this small-record workload; covered by a new unit test. Plain TLS passes the oracle. Mutual TLS is still failing server-side (syslog-ng validates the client cert and ACKs the app-data but does not log it) and is under investigation — the lwIP BDD lane is RED until that lands. The lwipopts pbuf/segment/window bump is provisional (not the mTLS cause) and will be trimmed once mTLS is root-caused. See DEVLOG. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR finalizes FreeRTOS lwIP TLS/mTLS support by implementing mbedTLS-based TLS and mTLS transport over lwIP Raw TCP, fixing TCP handshake stalls via Nagle disable, integrating mbedTLS cross-compilation with baked certificate headers, wiring endpoint selection in the main target, and promoting all lwIP CI jobs to required status. ChangesFreeRTOS lwIP TLS/mTLS Implementation
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 docstrings
🧪 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 |
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1344 passed) 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: 2
🤖 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_LwipRawTcp.c`:
- Around line 91-110: In FreeRtosMbedTlsCalloc, the multiplication bytes = nmemb
* size can overflow before you detect it; change the logic to check for overflow
before multiplying (e.g., if (size != 0 && nmemb <= SIZE_MAX / size) { bytes =
nmemb * size; ... } else return NULL) so that you validate nmemb and size
against SIZE_MAX (or sizeof(size_t) max) prior to calling pvPortMalloc; update
the function FreeRtosMbedTlsCalloc to perform the overflow guard using nmemb <=
SIZE_MAX / size (and keep the existing zero checks, pvPortMalloc/memset
behavior) and return NULL on overflow.
In `@DEVLOG.md`:
- Around line 72-75: The DEVLOG entry for "Resolve-failure logging" is
contradictory: it declares resolve-failure logging as implemented ("not
deferred") while it sits under the Deferred section; update DEVLOG so status is
unambiguous by either moving the bullet (the text referencing
numeric-resolver-rejects-a-hostname and SolidSyslog_Error) out of the Deferred
section into a non-deferred/Done section, or reword the bullet to explicitly
mark it deferred (e.g., prefix with "Deferred:") and remove the
TDD/implementation phrasing; ensure the text still references SolidSyslog_Error
and the numeric-resolver-rejects-a-hostname path so reviewers can find the
related work.
🪄 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: 3ea5f283-3464-4cf0-b9b5-5d09d1365a25
📒 Files selected for processing (10)
.github/workflows/ci.ymlBdd/Targets/Common/BddTargetTlsSender_MbedTls_LwipRawTcp.cBdd/Targets/FreeRtosLwip/CMakeLists.txtBdd/Targets/FreeRtosLwip/lwipopts.hBdd/Targets/FreeRtosLwip/main.cBdd/Targets/FreeRtosLwip/mbedtls_user_config.hDEVLOG.mdPlatform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.cTests/Lwip/SolidSyslogLwipRawTcpStreamTest.cppci/docker-compose.bdd.yml
- FreeRtosMbedTlsCalloc: guard the nmemb*size multiply with nmemb <= SIZE_MAX/size *before* multiplying, instead of the post-multiply (bytes/nmemb)==size check. The old form was correct (unsigned wrap is well-defined and the division detected it) but the check-before-multiply idiom is clearer and analyzer-friendly. Applied to both the LwipRaw and PlusTcp mbedTLS senders to keep the clones in sync. - DEVLOG: reword the resolve-failure-logging item so it no longer reads as "not deferred" while sitting under Deferred — it's pending work queued on this branch. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1344 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
…6/16 + 4xMSS The mTLS-over-lwIP failure recorded during S28.11 was a measurement artifact (rm-while-open in the manual probe loop destroying the file syslog-ng held open), not a defect. The real fix was the already-committed tcp_nagle_disable. Re-tested at main's original lwipopts sizing: mTLS delivers 5/5 manually and the full lwIP BDD lane (@tls, @Mtls, TLS 1.3, TCP->TLS switch) is green, so the provisional pbuf/segment/window bump is reverted. DEVLOG updated with the root-cause correction. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Lifted out of draft — TLS and mTLS over lwIP are greenThe mTLS-over-lwIP "blocker" tracked during this story turned out to be a measurement artifact in the manual verification harness, not a defect:
Verified this session
|
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1344 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
1 participant
Purpose
Closes #474 (E28). Wire a TLS/mTLS sender into the FreeRTOS + lwIP BDD target so the lwIP backend reaches the syslog-ng oracle over RFC 5425 TLS, completing the UDP (S28.09) / TCP (S28.10) / TLS (S28.11) progression — plus the CI plumbing to make the lwIP lanes required.
Change Description
Bdd/Targets/Common/BddTargetTlsSender_MbedTls_LwipRawTcp.c: a transport-swap clone of the +TCP..._PlusTcpTcp.c(mbedTLS over a secondSolidSyslogLwipRawTcpStream,SolidSyslogLwipRawAddress). All mbedTLS init (entropy/DRBG/PSA, baked PEMs, tls/mtls endpoint dispatcher) is transport-agnostic and copied verbatim.Bdd/Targets/FreeRtos/CMakeLists.txt); newFreeRtosLwip/mbedtls_user_config.h.main.cwires the TLS slot, bumps the interactive task stack for mbedTLS init, and pins the TLS/mTLS host.BddTargetTlsConfigdefaults the host to"syslog-ng"(a DNS name). The numeric-onlySolidSyslogLwipRawResolverrejected it and the sender never connected, with no error logged. Fixed inmain.cby pinning host10.0.2.2+ServerName="syslog-ng"(mirrors the +TCP target, whosePlusTcpResolverignores the host).tcp_writebut never put on the wire (Nagle holding sub-MSS segments).tcp_nagle_disable()on the pcb fixes it;TCP_NODELAYis the correct default for this small-record request/response workload.@tls/@mtls; newanalyze-tidy-freertos-lwip/analyze-iwyu-freertos-lwiplanes;build-freertos-target-lwip,bdd-freertos-qemu-lwip, and both analyze-lwip lanes promoted intosummary.needs.lwipopts.hpbuf/segment/window bump (48/48/32, 6×MSS) was made while chasing the mTLS failure; packet capture later showed it is not the cause (RX queue peaks at 2, noERR_MEM). Kept for now, to be trimmed to the minimum once mTLS is root-caused.Test Evidence
Tests/Lwip57 tests green, incl. newOpenDisablesNagleOnPcb(assertsTF_NODELAYon the pcb after Open).tls_transport) — encrypted record delivered, ACK'd, logged.CN=solidsyslog-bdd-client, chains to the trusted CA, no error/RST) and ACKs the app-data record — but never logs it. Ruled out: heap (53 KB free), sendERR_MEM, RX pbuf exhaustion, 0.5-RTT timing, cert rejection. Both transports negotiate TLS 1.3 (oracle disables ≤1.2); the +TCP target passes the same 6515 listener, so the difference is lwIP's on-wire segmentation vs the server's 1-RTT app-data handling. Next: server-side TLS-keylog decrypt +straceof syslog-ng'sSSL_read.Areas Affected
Bdd/Targets/FreeRtosLwip/,Bdd/Targets/Common/(Tier 3 BDD targets).Platform/LwipRaw/Source/SolidSyslogLwipRawTcpStream.c(Tier 2 — Nagle disable; benefits every lwIP integrator)..github/workflows/ci.yml,ci/docker-compose.bdd.yml.summary.needs; adding them to the GitHub required-checks set is a separate manual step.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores