Hardened: present a client certificate for mutual TLS - #24
Conversation
|
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 (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe syslog collector port changes to 6515, TLS setup adds optional client certificate and key material from ChangesSyslog mTLS transport
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant Syslog_Start
participant DeviceCertStore
participant TlsStream
participant SyslogPipelineSd
participant SyslogCollector
Syslog_Start->>DeviceCertStore: Read client certificate and key
Syslog_Start->>TlsStream: Configure port 6515 and optional credentials
Syslog_Start->>SyslogPipelineSd: Initialize transport metadata
TlsStream->>SyslogCollector: Establish TLS or mTLS connection
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
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 `@app/syslog/SyslogPipelineSd.c`:
- Line 20: Update the transport assignment in SyslogPipelineSd.c to reflect the
active authentication mode: emit "mtls" only when client certificate
authentication is actually enabled, and otherwise derive the appropriate
non-mTLS value or omit the transport element. Reuse the same configuration or
mode determination used by Syslog.c so missing client certificate or key cannot
produce transport="mtls".
In `@run-report.txt`:
- Line 21: Update the size cross-check row in run-report.txt to match the
reported flash_text/flash_data metrics for the same baseline. If the row
represents a different artifact, explicitly label it with that binary instead.
🪄 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: 4b1557f8-5b60-45f2-88f1-f7ef4f139f7c
📒 Files selected for processing (3)
app/syslog/Syslog.capp/syslog/SyslogPipelineSd.crun-report.txt
ClientCertChain and ClientKey join the stream config, and the collector moves to 6515. The device now authenticates itself to the collector as well as verifying it. Flash 14,504 B (+124) Static RAM 5,052 B (+4) Heap 17,776 B (+1,256) Log stack 712 B (0) Service stack 3,800 B (0) Neither stack moves: client authentication is another leg of a handshake the service task already had the depth for. The heap is what the handshake holds to prove the device's identity. Both handles come from the cert store, which has parsed them since boot. This figure is therefore the cost of *using* credentials the device already holds — a device doing server-authenticated TLS only would also have to provision, store and parse a client certificate and key to reach the same place. Both must be set: either one NULL disables mTLS and the connection falls back to server authentication only, without failing. That is why the pipeline element reports what was configured rather than what was intended — an element claiming protection the device does not have would be worse than not reporting it, since it is exactly the weakening a collector is watching for. Delivery is the proof. 6515 requires a client certificate, and scripts/smoke-oracle.sh shows it refusing a client that presents none, so a record arriving there is a record whose sender authenticated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
36bfd1c to
e8f8d0b
Compare
What this tag adds
Mutual TLS:
ClientCertChainandClientKeyjoin the stream config, and the collectormoves to 6515. The device now authenticates itself to the collector as well as
verifying it.
72 bytes of flash, and neither stack moves — client authentication is another leg of
a handshake the service task already had the depth for. The heap is what the handshake
holds to prove the device's identity.
Both handles come from the cert store, which has parsed them since boot. A device that
already speaks mTLS elsewhere was carrying them before SolidSyslog arrived, so what is
charged here is presenting them, not holding them.
Delivery is the proof
6515 is configured
peer-verify(required-trusted), andscripts/smoke-oracle.shprovesit refusing a client that presents no certificate — every run, before the device starts.
So a record arriving on 6515 is a record whose sender authenticated. That pairing matters:
the certless-rejection check alone would pass if there were no listener at all.
One trap worth knowing
ClientCertChainandClientKeymust both be set. Either one NULL disables mTLS andthe connection silently falls back to server authentication only — it still succeeds
against a permissive listener, which is exactly how this regression would go unnoticed.
Using a
required-trustedport is what turns that silence into a failure.Checklist
app/syslog/Syslog.cand the pipeline element'stransportvalue, which this step makes true.
measurements/<State>.csvcommitted, and a row added tomeasurements/tags.tsv.Tag boundaries deferred to the release pass.
./run.shgreen; record delivered on 6515 and confirmed in the collector's parsedoutput.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
mtlswhen enabled.Bug Fixes
tlstomtls.