feat: S17.03 cryptographic confidentiality for at-rest records — AES-256-GCM (OpenSSL, Linux)#509
Conversation
Replace the IntegritySize/ComputeIntegrity/VerifyIntegrity vtable with TrailerSize/SealRecord/OpenRecord, using a single contiguous content region plus a headerLength offset (content[0..headerLength) = associated data, content[headerLength..contentLength) = body). MAC and checksum policies authenticate the whole content and ignore the split; this is the shape an AEAD policy needs to authenticate the cleartext header and encrypt the body in place (S17.03 AES-GCM lands next). Behaviour-preserving: Null, Crc16, OpenSsl/MbedTls HMAC all migrate with no change to on-disk bytes or tags. The RecordStore consumer passes magic|length|message as content with headerLength = magic+length. The SOLIDSYSLOG_MAX_INTEGRITY_SIZE tunable keeps its name. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
First AEAD at-rest policy: authenticated encryption of stored records (confidentiality + integrity), closing IEC 62443 SR 4.1 at the cryptographic bar. Encrypts the body in place, authenticates the cleartext header as associated data, and writes nonce ‖ tag (28 bytes) to the policy-owned trailer (fits SOLIDSYSLOG_MAX_INTEGRITY_SIZE). Key fetched on demand via SolidSyslogKeyFunction (exactly 32 bytes for AES-256) and OPENSSL_cleanse'd per operation; fresh 12-byte nonce per record via RAND_bytes. Pool-allocated (E11) via the new SOLIDSYSLOG_AES_GCM_POLICY_POOL_SIZE tunable; bad-config / exhaustion fall back to the shared NullSecurityPolicy. A tag mismatch on open is the expected tamper-detected path and returns false silently; only genuine OpenSSL errors are reported. Host unit tests drive lifecycle, round-trip, tamper (body/tag/header), wrong-key, wrong-length-key, key-unavailable, nonce/encrypt/decrypt failures, and key wipe against a new deterministic OpenSslFake GCM + RAND_bytes interposition. Real-crypto correctness follows in the OpenSslIntegration suite (S17.03 Phase C). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Exercises SolidSyslogOpenSslAesGcmPolicy on the real libcrypto (the unit tests use the deterministic OpenSslFake): genuine round-trip, body is actually encrypted, header stays in clear, each seal draws a fresh nonce, and tampered ciphertext / tag / header and a wrong key are all rejected. Runs on integration-linux-openssl and integration-windows-openssl. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Adds the --security-policy aes-256-gcm arm to the Linux BDD target (CreateSecurityPolicy/DestroySecurityPolicy + parser validator + test) and a @aesgcm power-cycle-replay scenario proving the AEAD policy is transparent to behaviour: records encrypted and tagged on write are decrypted and verified on replay-read after a power cycle. The scenario runs Linux-only (OpenSSL) — excluded from the Windows and FreeRTOS-plustcp runners (no AES-GCM wired there this story), and from FreeRTOS-lwip via @store. Documents the policy in docs/security/at-rest-cryptography.md (spectrum, key-on-demand, the ~136-year random-nonce envelope, key-management responsibility) and refreshes the now-shipped at-rest rows in docs/iec62443.md. DEVLOG updated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Pre-push Tier B: clang-format reflow of the touched files, (void)-cast the memcpy in GcmDecrypt (MISRA 17.7, matching the codebase convention), and add the AES-GCM policy's MISRA suppressions (11.3 vtable downcast, 5.7 repeated struct tag, 8.7 single-TU error source) mirroring the HMAC sibling. misra_renumber resynced the HMAC/MbedTls suppression lines the reshape shifted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 56 minutes and 42 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughReshapes the security-policy API to TrailerSize + SealRecord/OpenRecord, updates RecordStore/BlockStore to use trailers, migrates policies, implements OpenSSL AES‑256‑GCM (implementation, pool, errors, messages), updates tests/fakes/BDD/CI, and adds docs and CMake/MISRA entries. ChangesSecurity Policy reshape, RecordStore integration, and policy migrations
OpenSSL AES‑256‑GCM policy implementation
Test infrastructure and fakes
BDD, CI, build, MISRA, and docs
Sequence DiagramsequenceDiagram
participant App
participant RecordStore
participant SecurityPolicy
participant OpenSSL
App->>RecordStore: Write(magic|length|message)
RecordStore->>RecordStore: Build content (magic|length|message)
RecordStore->>SecurityPolicy: SealRecord(content, contentLen, headerLength=4, trailerOut)
SecurityPolicy->>OpenSSL: FetchKey / RAND_bytes / EVP_Encrypt (key, nonce, AAD=header, plaintext=body)
OpenSSL-->>SecurityPolicy: ciphertext + tag
SecurityPolicy->>SecurityPolicy: Write trailer (nonce||tag), OPENSSL_cleanse(key)
RecordStore->>App: Persist sealed record (content || trailer || sentFlag)
App->>RecordStore: ReadNextUnsent()
RecordStore->>SecurityPolicy: OpenRecord(content, contentLen, headerLength=4, trailerIn)
SecurityPolicy->>OpenSSL: FetchKey / EVP_Decrypt (key, nonce, AAD=header, ciphertext, tag)
OpenSSL-->>SecurityPolicy: success or auth-fail
SecurityPolicy->>RecordStore: Return opened content OR false on auth-fail
RecordStore->>App: Return record or error
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 |
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1407 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.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Bdd/Targets/Linux/main.c (1)
67-69:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate comment to reflect both keyed policies.
The comment states "Only the keyed hmac-sha256 policy needs the handle to destroy" but now
aes-256-gcmalso requires the handle (line 266).📝 Suggested comment fix
-/* Holds the created at-rest SecurityPolicy handle so DestroyStore can release - * it. Only the keyed hmac-sha256 policy needs the handle to destroy; crc16 and - * null are dispatched by name. */ +/* Holds the created at-rest SecurityPolicy handle so DestroyStore can release + * it. The keyed policies (hmac-sha256, aes-256-gcm) need the handle to destroy; + * crc16 is dispatched via singleton Destroy, and null needs no cleanup. */🤖 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/Linux/main.c` around lines 67 - 69, The comment above the at-rest SecurityPolicy handle is outdated—update it to state that both keyed policies (hmac-sha256 and aes-256-gcm) require the stored SecurityPolicy handle so DestroyStore can release it; find the comment near the at-rest SecurityPolicy handle declaration and replace the single-policy text ("Only the keyed hmac-sha256 policy needs the handle to destroy") with phrasing that includes both keyed policies (hmac-sha256 and aes-256-gcm) and clarifies that crc16 and null are dispatched by name.
🤖 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.
Outside diff comments:
In `@Bdd/Targets/Linux/main.c`:
- Around line 67-69: The comment above the at-rest SecurityPolicy handle is
outdated—update it to state that both keyed policies (hmac-sha256 and
aes-256-gcm) require the stored SecurityPolicy handle so DestroyStore can
release it; find the comment near the at-rest SecurityPolicy handle declaration
and replace the single-policy text ("Only the keyed hmac-sha256 policy needs the
handle to destroy") with phrasing that includes both keyed policies (hmac-sha256
and aes-256-gcm) and clarifies that crc16 and null are dispatched by name.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 10140669-3014-4086-ac99-fc6129e82d8a
📒 Files selected for processing (37)
.github/workflows/ci.ymlBdd/Targets/Linux/BddTargetCommandLine.cBdd/Targets/Linux/BddTargetCommandLine.hBdd/Targets/Linux/main.cBdd/features/power_cycle_replay.featureCore/Interface/SolidSyslogSecurityPolicyDefinition.hCore/Interface/SolidSyslogTunablesDefaults.hCore/Source/RecordStore.cCore/Source/SolidSyslogBlockStoreStatic.cCore/Source/SolidSyslogCrc16Policy.cCore/Source/SolidSyslogNullSecurityPolicy.cDEVLOG.mdPlatform/MbedTls/Source/SolidSyslogMbedTlsHmacSha256Policy.cPlatform/OpenSsl/CMakeLists.txtPlatform/OpenSsl/Interface/SolidSyslogOpenSslAesGcmPolicy.hPlatform/OpenSsl/Interface/SolidSyslogOpenSslAesGcmPolicyErrors.hPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicy.cPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyMessages.cPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyPrivate.hPlatform/OpenSsl/Source/SolidSyslogOpenSslAesGcmPolicyStatic.cPlatform/OpenSsl/Source/SolidSyslogOpenSslHmacSha256Policy.cTests/Bdd/Targets/BddTargetCommandLineTest.cppTests/CMakeLists.txtTests/MbedTls/SolidSyslogMbedTlsHmacSha256PolicyTest.cppTests/OpenSslIntegration/CMakeLists.txtTests/OpenSslIntegration/SolidSyslogOpenSslAesGcmPolicyIntegrationTest.cppTests/SolidSyslogBlockStoreTest.cppTests/SolidSyslogCrc16PolicyTest.cppTests/SolidSyslogNullSecurityPolicyTest.cppTests/SolidSyslogOpenSslAesGcmPolicyTest.cppTests/SolidSyslogOpenSslHmacSha256PolicyTest.cppTests/Support/OpenSslFake.cTests/Support/OpenSslFake.hci/docker-compose.bdd.ymldocs/iec62443.mddocs/security/at-rest-cryptography.mdmisra_suppressions.txt
The reshape gave every policy's SealRecord/OpenRecord two adjacent uint16_t parameters (contentLength, headerLength), tripping bugprone-easily-swappable-parameters; wrap each vtable implementation (and the BlockStore test spy) in a NOLINT with the contract rationale, matching the established codebase pattern. In the AES-GCM policy, drop the TU-local named enum OpenSslAesGcmPolicy_OpenResult (the project's EnumPrefix rule forces a SolidSyslog prefix on named enums; TU-local enums stay anonymous) — GcmDecrypt now returns bool and self-reports genuine OpenSSL errors, keeping the silent tamper-detected path. Rework the OpenSslFake AES-GCM interposition onto the char-sentinel-handle idiom (single static state, no casting the opaque handle back to a struct), matching OpenSSL's parameter names, so the fake passes tidy too. misra_renumber resynced. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…andle The at-rest SecurityPolicy handle comment said only hmac-sha256 needs the handle to destroy; aes-256-gcm (added this story) does too. Per CodeRabbit review on #509. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1407 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
…tyRecord Bundle the security-policy method arguments into a SolidSyslogSecurityRecord struct (Content, ContentLength, HeaderLength, Trailer) passed by const pointer. This removes the two adjacent uint16_t lengths from every implementation's signature — eliminating the bugprone-easily-swappable-parameters findings at the root rather than suppressing them, so all the per-policy and spy NOLINTs are gone. Named fields also make ContentLength vs HeaderLength unmistakable at call sites, and the contract is now extensible without churning every vtable signature. Const-correct by construction: the policy may not repoint the fields, but the member pointers are non-const so it still writes through Content (in-place encrypt/decrypt) and Trailer (seal) — one signature serves both seal and open. Behaviour-preserving: all five policies, the RecordStore consumer, every policy test, the BlockStore spy, and the OpenSSL integration test migrate with no change to on-disk bytes or outcomes. misra_renumber resynced the shifted suppression lines. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
☀️ Quality Summary 🚦 build-linux-gcc: 100% successful (✔️ 1407 passed) Created by Quality Monitor v1.14.0 (#f3859fd). More details are shown in the GitHub Checks Result. |
… error path The OpenSslFake's AES-GCM half was a working toy cipher (reversible XOR keystream + FNV tag). The unit tests that leaned on it — round-trip, tamper, wrong-key — only proved the fake reproduced GCM, duplicating the OpenSslIntegration suite while testing none of the adapter's own logic. Gut the fake down to a capture-and-canned-return double: EVP calls capture their arguments, copy the body through unchanged, and return canned results. Replace the coarse encrypt/decrypt/auth failure booleans with a single per-step injection (OpenSslFake_SetGcmStepFails) so each OpenSSL call in the seal/open sequence can be failed in turn. Drop the seven fake-testing semantic tests; add per-step error tests pinning every fallible EVP call (CTX_new, both Inits, SET_IVLEN, both Updates, Final, GET_TAG/SET_TAG) plus the silent DecryptFinal tamper-reject path. Coverage of SolidSyslogOpenSslAesGcmPolicy.c is now 100% line / 100% function; genuine AES-256-GCM correctness stays owned by the integration suite. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ding cppcheck-misra flagged misra-c2012-11.8 (const-strip) where SealRecord passed record->Trailer directly as the writable tag destination — the addon tracks the outer const on *record and treats the non-const member pointer as const-stripped (D.006 category 1). Bind the trailer to a local uint8_t* first, exactly as the AES-GCM sibling already does for its nonce/tag, which sidesteps the false positive without a suppression. Behaviour-identical; full cppcheck-misra now exits clean with no new suppressions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…MISRA 11.8 fix Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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. |
☀️ 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 #508 (S17.03, epic #105). Adds the first authenticated-encryption
at-rest
SolidSyslogSecurityPolicy— AES-256-GCM over OpenSSL — giving storedrecords confidentiality on top of the integrity that S17.01/S17.02 already
provided. This closes IEC 62443 SR 4.1 (information confidentiality) at the
cryptographic bar, alongside the existing SR 3.4 integrity.
One algorithm, one platform: the goal is to get the AEAD API shape right before
going public. Linux/OpenSSL only this story (Windows compiles the source but
gets no BDD wiring); an mbedTLS sibling is deferred to a future S17.04.
Change Description
Sliced into reviewable commits:
refactor:reshapeSolidSyslogSecurityPolicy—IntegritySize/ComputeIntegrity/VerifyIntegrity→TrailerSize/SealRecord/OpenRecord, using a single contiguous content region + aheaderLengthoffset (
content[0..headerLength)= associated data, the rest = body).This is the shape an AEAD needs (authenticate the cleartext header, encrypt
the body in place). Chosen over the epic's two-pointer
body/aadsketchbecause it leaves the just-shipped HMAC/CRC one-shot crypto untouched. All
four existing policies (Null, CRC-16, OpenSSL + mbedTLS HMAC) and the
RecordStore consumer migrate behaviour-preservingly — on-disk bytes and tags
are byte-identical, so the existing suite is the regression net.
SOLIDSYSLOG_MAX_INTEGRITY_SIZEkeeps its name (only the vtable fieldrenamed).
feat:SolidSyslogOpenSslAesGcmPolicy— 6-file E11 shape mirroring theHMAC sibling. 32-byte key on demand via
SolidSyslogKeyFunction(
OPENSSL_cleansed per op), fresh 12-byteRAND_bytesnonce per record,nonce ‖ tag(28 B) trailer that fits the existing 32-byte budget.Pool-allocated via a new
SOLIDSYSLOG_AES_GCM_POLICY_POOL_SIZEtunable;bad-config / exhaustion fall back to
NullSecurityPolicy. A tag mismatch onopen returns
falsesilently (the tamper-detected path); only genuineOpenSSL errors are reported.
test:real-crypto integration tests against libcrypto.feat:Linux BDD wiring —--security-policy aes-256-gcmarm + an@aesgcmpower-cycle-replay scenario; docs.chore:clang-format reflow + MISRA suppressions.Nonce strategy
Random 12-byte nonce per record (no cross-reboot state to lose). The NIST
SP 800-38D §8.3 random-nonce ceiling is ~136 years at one event/second —
documented as headroom, not a caveat. No GCM-SIV.
Test Evidence
OpenSslFake) — the GCM fake is a capture-and-canned-returndouble (not a cipher); the unit tests verify the adapter's wiring rather than
re-deriving GCM: lifecycle/pool/bad-config, fresh nonce into the trailer, key
fetched + wiped, header as AAD, body handed to encrypt, nonce read back on
open, decrypt-success propagation, and the silent tamper-reject path. Every
fallible OpenSSL call in the seal/open sequence (CTX_new, both Inits,
SET_IVLEN, both Updates, Final, GET_TAG/SET_TAG) has a dedicated error-path
test —
SolidSyslogOpenSslAesGcmPolicy.cis 100% line / 100% functioncoverage. Genuine round-trip / tamper / wrong-key correctness is owned by the
integration suite below.
integration-linux-openssl+-windows-openssl, reallibcrypto) — 7 tests: genuine round-trip, body-is-encrypted,
header-in-clear, fresh-nonce-per-seal, tampered ciphertext/tag/header
rejected, wrong key rejected.
@aesgcmpower-cycle-replay scenario passes locally end-to-end(records encrypted+tagged on write, decrypted+verified on replay after a
power cycle). Runs Linux-only; excluded from Windows / FreeRTOS runners.
integration + BDD suites green. cppcheck-MISRA gate clean.
gccimage (junit, OpenSSL integration, BDD targettests, cppcheck preset) and the
freertos-hostimage (mbedTLS HMAC unittest, to confirm the reshape).
Areas Affected
Core/Interface/SolidSyslogSecurityPolicyDefinition.h— vtable reshape(extension-point contract; affects any custom
SolidSyslogSecurityPolicy).Core/Source/RecordStore.c,SolidSyslogBlockStoreStatic.c, the Null/CRC-16policies, and both HMAC siblings — migrated to the new contract.
Platform/OpenSsl/— new AES-GCM policy (also compiled into the WindowsOpenSSL build automatically).
Core/Interface/SolidSyslogTunablesDefaults.h— new pool tunable.Tests/Support/OpenSslFake.{c,h}, Linux BDD target, the@aesgcmfeature + CI tag filters,
docs/security/at-rest-cryptography.md,docs/iec62443.md, DEVLOG.Summary by CodeRabbit
New Features
Documentation
Tests
Chores