feat: S17.04 cryptographic confidentiality for at-rest records — AES-256-GCM (mbedTLS, FreeRTOS)#511
Conversation
Adds SolidSyslogMbedTlsAesGcmPolicy — the embedded AEAD sibling to S17.03's OpenSSL policy. Pure addition: the SealRecord/OpenRecord vtable reshape already landed in S17.03, so no shared interface or other policy is touched. Config injects a caller-owned seeded mbedtls_ctr_drbg_context (mbedTLS has no context-free RNG, so the nonce source is injected rather than self-sourced as the OpenSSL sibling does with RAND_bytes). One-shot mbedtls_gcm_crypt_and_tag / _auth_decrypt; key fetched on demand and wiped with mbedtls_platform_zeroize. Driven against the MbedTlsFake GCM capture double: it captures the arguments to each mbedtls_gcm_* call, copies the body through unchanged, and returns canned results — NOT a cipher. The unit suite pins the adapter's wiring and one error path per fallible call (setkey, crypt_and_tag, auth_decrypt, ctr_drbg_random); genuine round-trip / tamper / wrong-key correctness is the integration suite's job (lands next). This avoids the S17.03 toy-cipher trap that had to be gutted mid-story. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Drives SolidSyslogMbedTlsAesGcmPolicy through real libmbedcrypto with a seeded CTR-DRBG: genuine round-trip (body encrypted, then restored), header stays in clear, each seal draws a fresh nonce, and tampered ciphertext / tag / header and a wrong key are all rejected. Mirrors the OpenSSL AES-GCM integration suite. Confirms in-place AEAD decryption (output == input) round-trips correctly on mbedTLS — the gcm.h "output cannot be the same as input" note is conservative; same-pointer in-place is the documented-safe full-overlap case for GCM. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…rget Wires --security-policy aes-256-gcm into Bdd/Targets/FreeRtos/main.c (validation + Create/Destroy branches + policy sources) and lets the @aesgcm power-cycle replay scenario run on the FreeRTOS-plustcp QEMU runner (drops the not @aesgcm exclusion from the behave-freertos tag filter). The AEAD nonce source reuses the TLS module's already-seeded CTR-DRBG via a new BddTargetTlsSender_GetRng() accessor rather than standing up a second entropy/DRBG pair on the constrained target — the TLS sender is created before any store-file rebuild, so the DRBG is seeded by the time the policy needs it. Closes #510. Includes the S17.04 DEVLOG entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
… analysis gates GcmEncrypt/GcmDecrypt now take (const SolidSyslogSecurityRecord*, key) instead of unpacked (key, nonce, body, ...) scalars. Two distinct-typed parameters clear clang-tidy's bugprone-easily-swappable-parameters without an inline NOLINT — the warning fired here (but not on the OpenSSL sibling) because mbedTLS's one-shot setkey/crypt_and_tag hands key and nonce to separate calls, so the check's SuppressParametersUsedTogether heuristic can't pair them. The helpers derive the body/nonce/tag/AAD from the record's documented trailer layout in one place. Also: shorten the BAD_CONFIG message to one literal (clang-format wrapped the longer text into two adjacent literals, tripping bugprone-suspicious-missing-comma), and add cppcheck-misra suppressions for 11.3 (the SelfFromBase downcast), 5.7 (anonymous-enum constant names shared with the OpenSSL sibling), and 8.7 (the public ErrorSource object) — each mirrors the existing HMAC/OpenSSL sibling entry. 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 (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds SolidSyslogMbedTlsAesGcmPolicy with injected CTR-DRBG nonce source, pooled lifecycle, error mapping, extended mbedTLS test double, unit and integration tests, and FreeRTOS BDD wiring exposing the TLS sender RNG to the policy. ChangesAES-256-GCM Security Policy Implementation
Sequence Diagram — AES-GCM policy creation and nonce injectionsequenceDiagram
participant BDD as BDD Scenario
participant Main as FreeRTOS main.c
participant Policy as SolidSyslogMbedTlsAesGcmPolicy
participant TLS as TLS Sender
participant DRBG as CTR-DRBG
BDD->>Main: set security-policy aes-256-gcm
Main->>TLS: BddTargetTlsSender_GetRng()
TLS-->>Main: mbedtls_ctr_drbg_context* (seeded)
Main->>Policy: SolidSyslogMbedTlsAesGcmPolicy_Create(config)
Policy->>DRBG: mbedtls_ctr_drbg_random(nonce)
DRBG-->>Policy: 12-byte nonce
Policy-->>Main: policy handle
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 (✔️ 1417 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: 1
🧹 Nitpick comments (3)
Tests/MbedTls/SolidSyslogMbedTlsAesGcmPolicyTest.cpp (1)
64-73: ⚡ Quick winUse the repo’s
CHECK_REPORTEDhelper here instead of defining another wrapper.
CHECK_REPORTED_ERRORis re-creating theErrorHandlerFakeassertion shape the test suite already standardizes. Reusing the canonical helper avoids drift and keeps these tests aligned with the rest of the repo.Based on learnings, when asserting
ErrorHandlerFakepayloads, use the establishedCHECK_REPORTED(severity, source, code)macro rather than re-creating the underlying expectations.🤖 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 `@Tests/MbedTls/SolidSyslogMbedTlsAesGcmPolicyTest.cpp` around lines 64 - 73, Replace the custom CHECK_REPORTED_ERROR macro with the repo-standard CHECK_REPORTED helper: remove the CHECK_REPORTED_ERROR definition and change its call sites to use CHECK_REPORTED(severity, &MbedTlsAesGcmPolicyErrorSource, code) so the test asserts ErrorHandlerFake payloads using the canonical macro; reference the macro name CHECK_REPORTED_ERROR to locate and delete it and use CHECK_REPORTED and MbedTlsAesGcmPolicyErrorSource when updating assertions.Tests/Support/MbedTlsFake.c (2)
707-722: ⚡ Quick winRestructure
mbedtls_ctr_drbg_random()to a single exit.The early
return -1;breaks the repo rule for**/*.cfiles. Keep the fake behavior, but funnel it through one return path.As per coding guidelines, "`**/*.c`: Functions must have single return point — if natural shape has early return, restructure with result local and `if` wrapper".Suggested change
int mbedtls_ctr_drbg_random(void* p_rng, unsigned char* output, size_t output_len) { + int result = 0; ctrDrbgRandomCallCount++; lastCtrDrbgRandomContext = p_rng; lastCtrDrbgRandomBuf = output; lastCtrDrbgRandomLen = output_len; if (ctrDrbgRandomFails) { - return -1; + result = -1; } - for (size_t index = 0; index < output_len; index++) + else { - output[index] = (unsigned char) (0xA0U + index); + for (size_t index = 0; index < output_len; index++) + { + output[index] = (unsigned char) (0xA0U + index); + } } - return 0; + return result; }🤖 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 `@Tests/Support/MbedTlsFake.c` around lines 707 - 722, The function mbedtls_ctr_drbg_random currently uses an early return when ctrDrbgRandomFails; change it to a single exit by introducing a local int result initialized to 0, keep the side-effect code (ctrDrbgRandomCallCount++, lastCtrDrbgRandomContext, lastCtrDrbgRandomBuf, lastCtrDrbgRandomLen) unchanged, then replace the early return with setting result = -1 inside the if (ctrDrbgRandomFails) block and otherwise run the buffer-fill loop, and finally return result at the end of mbedtls_ctr_drbg_random.
23-30: ⚡ Quick winRemove the hard caps from captured GCM AAD/body buffers.
SolidSyslogMbedTlsAesGcmPolicy.cforwardsrecord->HeaderLengthand the full body length straight intombedtls_gcm_*, but this fake only stores 16 bytes of AAD and 256 bytes of body while still reporting the uncapped lengths. Any larger-case test that compares againstMbedTlsFake_LastGcmAad()orMbedTlsFake_LastGcmPlaintext()using those lengths will read past the capture buffers and the double will misrepresent what the policy actually passed.Also applies to: 893-897, 923-926
🤖 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 `@Tests/Support/MbedTlsFake.c` around lines 23 - 30, The GCM fake currently enforces MBEDTLSFAKE_GCM_MAX_AAD and MBEDTLSFAKE_GCM_MAX_BODY (the enum values) which truncates captures while the test helpers (e.g., MbedTlsFake_LastGcmAad() and MbedTlsFake_LastGcmPlaintext()) report the uncapped lengths; remove these hard caps and instead record and store the full lengths passed into the fake (use the passed-in aad_len/plaintext_len values) by allocating/storing buffers sized to those lengths (or dynamically resizing capture buffers) and update any related size fields so lookups/readers read exactly the captured lengths rather than a fixed 16/256 limit.
🤖 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 `@Platform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicy.c`:
- Around line 16-24: The enum defines AES_GCM_TRAILER_SIZE (GCM_NONCE_SIZE +
GCM_TAG_SIZE) but there is no compile-time check that it fits the shared buffer,
risking a runtime overrun of record->Trailer; add a _Static_assert that
AES_GCM_TRAILER_SIZE <= SOLIDSYSLOG_MAX_INTEGRITY_SIZE (using the same macro
name as the shared buffer) with a clear message so compilation fails if the
shared max-integrity constant is reduced, ensuring code paths that write the
trailer (referenced by AES_GCM_TRAILER_SIZE and record->Trailer) cannot overflow
the shared buffer.
---
Nitpick comments:
In `@Tests/MbedTls/SolidSyslogMbedTlsAesGcmPolicyTest.cpp`:
- Around line 64-73: Replace the custom CHECK_REPORTED_ERROR macro with the
repo-standard CHECK_REPORTED helper: remove the CHECK_REPORTED_ERROR definition
and change its call sites to use CHECK_REPORTED(severity,
&MbedTlsAesGcmPolicyErrorSource, code) so the test asserts ErrorHandlerFake
payloads using the canonical macro; reference the macro name
CHECK_REPORTED_ERROR to locate and delete it and use CHECK_REPORTED and
MbedTlsAesGcmPolicyErrorSource when updating assertions.
In `@Tests/Support/MbedTlsFake.c`:
- Around line 707-722: The function mbedtls_ctr_drbg_random currently uses an
early return when ctrDrbgRandomFails; change it to a single exit by introducing
a local int result initialized to 0, keep the side-effect code
(ctrDrbgRandomCallCount++, lastCtrDrbgRandomContext, lastCtrDrbgRandomBuf,
lastCtrDrbgRandomLen) unchanged, then replace the early return with setting
result = -1 inside the if (ctrDrbgRandomFails) block and otherwise run the
buffer-fill loop, and finally return result at the end of
mbedtls_ctr_drbg_random.
- Around line 23-30: The GCM fake currently enforces MBEDTLSFAKE_GCM_MAX_AAD and
MBEDTLSFAKE_GCM_MAX_BODY (the enum values) which truncates captures while the
test helpers (e.g., MbedTlsFake_LastGcmAad() and MbedTlsFake_LastGcmPlaintext())
report the uncapped lengths; remove these hard caps and instead record and store
the full lengths passed into the fake (use the passed-in aad_len/plaintext_len
values) by allocating/storing buffers sized to those lengths (or dynamically
resizing capture buffers) and update any related size fields so lookups/readers
read exactly the captured lengths rather than a fixed 16/256 limit.
🪄 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: ab3a6771-98de-4dc3-875a-db913283d2cc
📒 Files selected for processing (21)
Bdd/Targets/Common/BddTargetTlsSender.hBdd/Targets/Common/BddTargetTlsSender_MbedTls_LwipRawTcp.cBdd/Targets/Common/BddTargetTlsSender_MbedTls_PlusTcpTcp.cBdd/Targets/FreeRtos/CMakeLists.txtBdd/Targets/FreeRtos/main.cBdd/features/power_cycle_replay.featureDEVLOG.mdPlatform/MbedTls/Interface/SolidSyslogMbedTlsAesGcmPolicy.hPlatform/MbedTls/Interface/SolidSyslogMbedTlsAesGcmPolicyErrors.hPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicy.cPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyMessages.cPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyPrivate.hPlatform/MbedTls/Source/SolidSyslogMbedTlsAesGcmPolicyStatic.cTests/MbedTls/CMakeLists.txtTests/MbedTls/SolidSyslogMbedTlsAesGcmPolicyTest.cppTests/MbedTlsIntegration/CMakeLists.txtTests/MbedTlsIntegration/SolidSyslogMbedTlsAesGcmPolicyIntegrationTest.cppTests/Support/MbedTlsFake.cTests/Support/MbedTlsFake.hci/docker-compose.bdd.ymlmisra_suppressions.txt
…ty buffer Address CodeRabbit: the policy hard-codes a 28-byte nonce‖tag trailer and only documented that it fits SOLIDSYSLOG_MAX_INTEGRITY_SIZE. If that shared buffer is ever tuned below 28, seal/open would overrun record->Trailer at runtime. Add a SOLIDSYSLOG_STATIC_ASSERT so the build fails instead. Renumbers the policy's 5.7 / 11.3 cppcheck suppressions shifted by the new includes + assert. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1417 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
Address CodeRabbit nitpick: the GCM capture double stored only MBEDTLSFAKE_GCM_MAX_AAD / _MAX_BODY bytes but reported the uncapped length, so a future test passing AAD > 16 or body > 256 and walking LastGcmAad() / LastGcmPlaintext() up to the reported length would read past the capture buffer. Report exactly what was captured instead. No-op for the current fixtures (4-byte header, 8-byte body); a larger case now fails loudly on length rather than reading out of bounds. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1417 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
1 participant
Purpose
Closes #510 (E17 #105). Adds
SolidSyslogMbedTlsAesGcmPolicy— the embedded AEAD at-rest policy, the mbedTLS sibling to S17.03'sSolidSyslogOpenSslAesGcmPolicy. Provides authenticated encryption of at-rest records (confidentiality + integrity) on the FreeRTOS / constrained-target path, closing IEC 62443 SR 4.1 at the cryptographic bar for embedded integrators on top of the SR 3.4 integrity S17.02 shipped there.Change Description
Pure addition — the
SealRecord/OpenRecordvtable reshape already landed in S17.03, so no shared interface or other policy is touched.Platform/MbedTls/(E11 six-file shape, mirrors the HMAC sibling for vendor wiring and the OpenSSL AES-GCM sibling for AEAD behaviour). One-shotmbedtls_gcm_crypt_and_tag/mbedtls_gcm_auth_decrypt; key fetched on demand and wiped withmbedtls_platform_zeroize.OpenRecordreturnsfalsesilently onMBEDTLS_ERR_GCM_AUTH_FAILED(tamper/wrong-key), reports only genuine errors.mbedtls_ctr_drbg_context*(Rng, same DI shape asSolidSyslogMbedTlsStreamConfig.Rng);NULL RngjoinsNULLconfig /GetKeyas a bad-config → NullSecurityPolicy fallback. Reuses the role-basedSOLIDSYSLOG_AES_GCM_POLICY_POOL_SIZEtunable andSOLIDSYSLOG_MAX_HMAC_KEY_SIZE; trailer = 28 B (fits the 32-BSOLIDSYSLOG_MAX_INTEGRITY_SIZE).SolidSyslogSecurityRecord(not unpackedkey, nonce, body, …scalars). This clears clang-tidy'sbugprone-easily-swappable-parameterswithout an inline NOLINT — the warning fires here but not on the OpenSSL sibling because mbedTLS's separate setkey/crypt calls defeat the check's used-together heuristic. The trailer/header layout lives in one place.--security-policy aes-256-gcmis wired intoBdd/Targets/FreeRtos/main.c; the existing@aesgcmpower-cycle-replay scenario now runs on the FreeRTOS-plustcp QEMU runner (dropped itsnot @aesgcmexclusion). The AEAD nonce source reuses the TLS module's already-seeded CTR-DRBG via a newBddTargetTlsSender_GetRng()accessor rather than standing up a second entropy/DRBG pair on the constrained target.Test Evidence
MbedTlsFakeGCM additions are a capture-and-canned-return double from inception — eachmbedtls_gcm_*call captures its arguments (key, key-bits, cipher id, nonce, AAD, plaintext), copies the body through unchanged, and returns canned results; it is not a cipher and deliberately not a copy of the HMAC fake's predict-the-tag pattern. The suite pins the adapter's wiring and one error path per fallible call (SetGcmStepFailsoverSETKEY/CRYPT_AND_TAG/AUTH_DECRYPT, plusSetGcmAuthFailsfor the tamper verdict andSetCtrDrbgRandomFailsfor the nonce). This is the S17.03 toy-cipher trap avoided up front.output == input) round-trips on real mbedTLS.@aesgcmpower-cycle-replay scenario runs onbdd-freertos-qemu-plustcp.cpputest-freertoscontainer: unit + integration green, both FreeRTOS cross-target ELFs link (plustcp + lwip), clang-format / clang-tidy / cppcheck-misra clean.Areas Affected
Platform/MbedTls/— new policy (Interface + Source, six files). No change to existing mbedTLS adapters or any other Platform.Tests/Support/MbedTlsFake.{c,h}— GCM capture double + CTR-DRBG capture (thembedtls_ctr_drbg_randomstub becomes capturing; existing stream/HMAC tests unaffected).Tests/MbedTls/,Tests/MbedTlsIntegration/— new unit + integration targets.Bdd/Targets/FreeRtos/,Bdd/Targets/Common/BddTargetTlsSender*— AES-GCM arm +GetRng()accessor;ci/docker-compose.bdd.yml+power_cycle_replay.featuretag wiring.misra_suppressions.txt— three suppressions mirroring the HMAC/OpenSSL siblings.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Documentation
Chores
@aesgcmscenarios; build/test wiring updated.