fix: check the Mbed TLS client key against its certificate - #790
Conversation
Drives mbedtls_pk_check_pair into MbedTlsStream_ApplyTlsPolicy with the certificate's public key, the private key and the RNG the configuration already carries. The result is not read yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A key that does not match its certificate reached the collector as a handshake rejection, putting the evidence at the far end of the connection. mbedtls_pk_check_pair sees it locally, so it is now reported as WARNING with CAT_BAD_CONFIG and its own detail code, and the stream connects server-authenticated as it does for every other credential fault. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reporting while installing anyway would still present a certificate the collector rejects at CertificateVerify, which is the diagnosis-at-the-wrong-end fault the check exists to remove. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Extracts MbedTlsStream_ClientKeyMatchesCertificate so the condition reads as its intent, takes the divergence note off the Mbed TLS platform page, and renumbers the MISRA suppressions the added lines moved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (7)
Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. WalkthroughThe Mbed TLS stream now validates that the configured client certificate matches its private key before installation. Tests mock and inspect ChangesClient credential validation
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR adds local validation for mismatched client keys and certificates while preserving delivery behavior; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant SolidSyslogMbedTlsStream
participant mbedtls_pk_check_pair
participant mbedtls_ssl_conf_own_cert
SolidSyslogMbedTlsStream->>mbedtls_pk_check_pair: Check client certificate and private key with configured RNG
alt Pair matches
SolidSyslogMbedTlsStream->>mbedtls_ssl_conf_own_cert: Install client credentials
else Pair does not match
SolidSyslogMbedTlsStream->>SolidSyslogMbedTlsStream: Emit mismatch warning
end
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
☀️ Quality Summary Created by Quality Monitor v4.15.0 (#82d77af). More details are shown in the GitHub Checks Result. |
1 participant
Purpose
Addresses #719.
SolidSyslogMbedTlsStreaminstalled the client credentialwithout checking that the key belonged to the certificate, so a mismatched pair
was accepted at
Openand surfaced only as a handshake rejection from thecollector. On an embedded target that puts the fault on the device and the
evidence at the far end of the connection. The OpenSSL adapter has checked this
locally since it shipped, and the two are meant to be substitutable.
No
Closeskeyword, per the integration-branch convention — the issue is closedby the final feature-to-main pull request.
Change Description
mbedtls_pk_check_pairis the counterpart toSSL_CTX_check_private_keythatthe issue's original framing said did not exist. It takes
&crt->pk, and for anRSA key compares the modulus and public exponent, for an EC key the exported
public keys — no signature round trip, so there is no code-space argument for
leaving the two adapters different.
mbedtls_ssl_conf_own_certnames it in itsown documentation as the way to check the pair it is being handed. The RNG it
takes is the
Rnghandle the stream configuration already requires, so thecheck needs nothing new from the integrator.
The check runs before
mbedtls_ssl_conf_own_certand a mismatch skips theinstall. Reporting while installing anyway would still present a credential the
collector rejects at
CertificateVerify, which is the fault this exists toremove.
A mismatch reports
WARNINGwithCAT_BAD_CONFIGunder a new detail code,SOLIDSYSLOG_MBEDTLS_STREAM_ERROR_CLIENT_CREDENTIAL_MISMATCHED, andOpensucceeds server-authenticated. That follows the credential rule in
docs/tls.md:a fault in the material we present never stops delivery. The code is placed
between
_INCOMPLETEand_NOT_INSTALLEDto match the order the OpenSSL adapteralready uses, which shifts
_NOT_INSTALLEDby one — handlers match by name, andnothing outside this branch has consumed either value.
The pairing check moves into the Mbed TLS credentials backend when the
credentials role lands later in #782. That relocation is known and accepted, so
this defect is fixed now rather than after the API work.
Test Evidence
Four commits, red confirmed before each:
OpenChecksClientKeyAgainstItsCertificate— red as a compile failure on theabsent fake accessors. Green by adding
mbedtls_pk_check_pairtoMbedTlsFakeand calling it with the certificate's public key, the privatekey,
mbedtls_ctr_drbg_randomand the configured RNG. The result is not readat this step.
OpenReportsMismatchedClientCredentialAndStillConnects— red on the absentdetail code. Green by reporting it and keeping
Opentrue.OpenSkipsOwnCertWhenClientKeyDoesNotMatchCertificate— the skip had slippedinto step 2 as an
else if, so the production code was reduced back to theminimum to get a genuine red (
expected <0> but was <1>) before theelse ifwas restored.
MbedTlsStream_ClientKeyMatchesCertificate.No test was added for the credential-absent case: the check sits inside the
existing
MbedTlsStream_HasClientCredentialbranch, andOpenSkipsOwnCertWhenClientCertChainIsNull/...WhenClientKeyIsNullalreadydrive
Openwith a NULL certificate, so moving the dereference outside the guardfails them.
Full local run in the
freertos-hostservice: 22/22 suites, includingMbedTlsIntegrationTestsandOpenSslIntegrationTestsagainst the reallibraries.
SolidSyslogMbedTlsStreamTestis 61 tests, 182 checks.clang-format --dry-run --Werrorclean over the whole tree,misra_renumber.py --applyproduced six line-number updates and no new findings,markdownlint clean on the changed page.
Areas Affected
Platform/MbedTls/— the stream adapter and its error enum.Tests/Support/gains one interposed mbedTLS entry point, used only by the mbedTLS test
executables. The divergence note comes off
docs/platforms/mbedtls/index.md,leaving five. Nothing in
Core/and no public API outside the Mbed TLS pack.Summary by CodeRabbit
Bug Fixes
Documentation
Tests