fix(cert): NotBefore=0 collides with CHIP no-expiry sentinel#454
Closed
glennswest wants to merge 1 commit into
Closed
fix(cert): NotBefore=0 collides with CHIP no-expiry sentinel#454glennswest wants to merge 1 commit into
glennswest wants to merge 1 commit into
Conversation
A Matter operational certificate (RCAC/ICAC/NOC) emitted by rs-matter's NocGenerator with NotBefore = 0 (CHIP epoch start, 2000-01-01 00:00:00 UTC) cannot be verified by the CHIP reference SDK: AddTrustedRootCertificate fails with 'Invalid signature' (CHIP Error 0x14) on the responder side. Root cause ---------- rs-matter signs the TBS over its own TLV-to-ASN.1 conversion, which encodes NotBefore = 0 as UTCTime '000101000000Z' (the CHIP epoch itself). CHIP's symmetric ChipEpochToASN1Time (src/credentials/CHIPCert.cpp:1076-1106) treats any epochTime == 0 as the X.509 no-well-defined-expiration sentinel and re-emits it as GeneralizedTime '99991231235959Z' irrespective of whether it appears in NotBefore or NotAfter. The CHIP source comment is explicit about this trap and states such certificates 'are not usable with this code'. End result: CHIP reconstructs a different TBS DER, computes a different SHA-256, ECDSA-verify returns false. Self-verification inside rs-matter passes (rs-matter is symmetric with itself), so the failure only manifests on CHIP interop. Fix --- 1. NocGenerator: shift NotBefore from 0 -> 1 (2000-01-01 00:00:01) in the three sites that build RCAC, ICAC and NOC. Keeps the cert effectively unbounded on the lower end while clearing CHIP's sentinel collision. 2. Cert builder BasicConstraints: drop pathLenConstraint from RCAC output. Matter Spec section 6.5.11.1.2 mandates 'pathLenConstraint SHALL NOT be present' for a Root CA certificate; rs-matter was emitting INTEGER 1. Both ends preserved the malformed field through TLV-to-ASN.1 conversion so signatures still matched, but strict validators (and the spec) reject it. Regression test --------------- test_rcac_self_verify builds an RCAC via RcacBuilder and re-parses it via CertRef::verify_chain_start. Confirms the TLV-to-ASN.1-to- hash-to-verify pipeline is internally consistent and catches future sign/verify drift. Diagnosis credit: confirmed end-to-end against chip-tool's all-clusters-app (built from project-chip/connectedhomeip). After the fix, commissioning completes: AddTrustedRootCertificate -> AddNOC -> fabric_index assigned -> CommissioningComplete.
|
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates the certificate builder to comply with Matter Spec §6.5.11.1 by removing the pathLenConstraint from Root Certificate Authority Certificates (RCAC). It also addresses an interoperability issue where a not_before value of 0 caused signature mismatches in external implementations by incrementing it to 1. A new RCAC self-verification test was added. Review feedback recommends using the updated not_before value in the new test for consistency and refactoring duplicated Validity initializations into a module-level constant to improve maintainability.
| ca_id: Some(0x1122_3344_5566_7788), | ||
| }; | ||
| let validity = Validity { | ||
| not_before: 0, |
Contributor
There was a problem hiding this comment.
To align with the main fix in this PR and avoid using a value known to cause interoperability issues, it's better to use 1 for not_before here instead of 0. This ensures the test validates the corrected behavior and serves as a better example for future use.
Suggested change
| not_before: 0, | |
| not_before: 1, |
Comment on lines
+126
to
142
| // NotBefore MUST NOT be 0 (Matter epoch start, 2000-01-01). | ||
| // CHIP's ChipEpochToASN1Time treats epoch=0 as the "no | ||
| // well-defined expiration date" sentinel and re-emits it as | ||
| // GeneralizedTime "99991231235959Z" regardless of which field | ||
| // it appears in (see CHIPCert.cpp:1076-1106 and the | ||
| // explanatory comment about CHIP epoch 0 NotBefore producing | ||
| // an invalid TBS signature on round-trip). | ||
| // | ||
| // We sign over UTCTime "000101000000Z" (Matter epoch); CHIP | ||
| // would reconstruct GeneralizedTime "99991231235959Z" and the | ||
| // hash would mismatch. Using 1 second past the Matter epoch | ||
| // avoids the sentinel collision while keeping the cert | ||
| // effectively unbounded on the lower end. | ||
| let validity = crate::cert::builder::Validity { | ||
| not_before: 0, // epoch start | ||
| not_after: 0, // no expiry | ||
| not_before: 1, // 2000-01-01 00:00:01 — past CHIP's epoch=0 sentinel | ||
| not_after: 0, // no expiry (NotAfter sentinel is legitimate) | ||
| }; |
Contributor
There was a problem hiding this comment.
This block of code (comment and validity initialization) is duplicated in three places within this file (new, generate_icac, and generate_noc). To improve maintainability and reduce duplication, consider defining a module-level constant for the Validity struct.
For example, you could add this before impl NocGenerator:
// NotBefore MUST NOT be 0 (Matter epoch start, 2000-01-01).
// CHIP's ChipEpochToASN1Time treats epoch=0 as the "no
// well-defined expiration date" sentinel and re-emits it as
// GeneralizedTime "99991231235959Z" regardless of which field
// it appears in (see CHIPCert.cpp:1076-1106 and the
// explanatory comment about CHIP epoch 0 NotBefore producing
// an invalid TBS signature on round-trip).
//
// We sign over UTCTime "000101000000Z" (Matter epoch); CHIP
// would reconstruct GeneralizedTime "99991231235959Z" and the
// hash would mismatch. Using 1 second past the Matter epoch
// avoids the sentinel collision while keeping the cert
// effectively unbounded on the lower end.
const DEFAULT_VALIDITY: crate::cert::builder::Validity = crate::cert::builder::Validity {
not_before: 1, // 2000-01-01 00:00:01 — past CHIP's epoch=0 sentinel
not_after: 0, // no expiry (NotAfter sentinel is legitimate)
};Then, you can replace this block and the other two occurrences with a single line.
Suggested change
| // NotBefore MUST NOT be 0 (Matter epoch start, 2000-01-01). | |
| // CHIP's ChipEpochToASN1Time treats epoch=0 as the "no | |
| // well-defined expiration date" sentinel and re-emits it as | |
| // GeneralizedTime "99991231235959Z" regardless of which field | |
| // it appears in (see CHIPCert.cpp:1076-1106 and the | |
| // explanatory comment about CHIP epoch 0 NotBefore producing | |
| // an invalid TBS signature on round-trip). | |
| // | |
| // We sign over UTCTime "000101000000Z" (Matter epoch); CHIP | |
| // would reconstruct GeneralizedTime "99991231235959Z" and the | |
| // hash would mismatch. Using 1 second past the Matter epoch | |
| // avoids the sentinel collision while keeping the cert | |
| // effectively unbounded on the lower end. | |
| let validity = crate::cert::builder::Validity { | |
| not_before: 0, // epoch start | |
| not_after: 0, // no expiry | |
| not_before: 1, // 2000-01-01 00:00:01 — past CHIP's epoch=0 sentinel | |
| not_after: 0, // no expiry (NotAfter sentinel is legitimate) | |
| }; | |
| let validity = DEFAULT_VALIDITY; |
Contributor
|
The So if all tests pass, I'll merge this PR. But with that said:
I don't think we have that at all. We have the beginnings of it in the So if you have that full flow, how about you contribute it? That is, if you don't mind and if you believe (like me) that it is already commoditized so it best to belong to Could be a separate PR of course, but wanted to open the topic. |
Contributor
|
@glennswest Forgot to mention - you also need to sign the CLA. |
|
PR #454: Size comparison from da2d79e to 357ac70 Full report (8 builds for (core), dimmable-light, onoff-light, onoff-light-bt, speaker)
|
4 tasks
Contributor
|
@glennswest Also please assess the code review feedback from Gemini. All code review feedback must be addressed with fixes, or else with justification why the feedback does not make sense. |
glennswest
added a commit
to glennswest/rs-matter
that referenced
this pull request
May 18, 2026
Introduces a new `controller` module that provides the IM-invoke and
orchestration primitives a controller needs to drive a Matter accessory
from a freshly-established PASE session through to
CommissioningComplete.
The accessory role is already well-covered in this crate. This module
is the inverse role — the thing that *commissions* accessories — and
adds a deliberately small, validated surface:
Public API (controller::commissioner):
- arm_fail_safe(matter, expiry_seconds, breadcrumb)
GeneralCommissioning::ArmFailSafe over PASE, decodes
ArmFailSafeResponse.errorCode → maps non-OK to FailSafeExpired.
- csr_request(matter, &csr_nonce) -> CsrPayload
OperationalCredentials::CSRRequest with full CSRResponse decode.
Returns NOCSRElements (octstr<400>) + AttestationSignature (64 B).
- decode_nocsr_elements(blob) -> DecodedNocsr<'_>
TLV decoder for the NOCSRElements struct (§11.18.6.5.2) — pulls
the PKCS#10 CSR DER and the device's nonce-echo so the controller
can verify nonce freshness before issuing a NOC.
- add_noc(matter, noc, icac, ipk, admin_subject, admin_vendor_id)
OperationalCredentials::AddNOC with NOCResponse status +
FabricIndex decode. Non-OK status → AddNocRejected.
- commission_pase(matter, crypto, fabric_creds, admin_subject,
admin_vendor_id, fail_safe_secs) -> PaseCommissionResult
Orchestrator that chains the above plus the two unexposed steps
(AddTrustedRootCertificate, CommissioningComplete) and calls
FabricCredentials::generate_device_credentials to mint the NOC.
Returns the device-assigned FabricIndex + NodeID + cert chain.
Public API (controller::setup_code):
- Manual pairing-code + QR-code (MT:...) parser per Matter spec §5.1.4,
with version / vendor-id / product-id / discriminator / passcode /
discovery-capabilities-bitmask decoding.
Supporting additions to existing files:
- commissioner::FabricCredentials gains `root_secret_key()`,
`rcac_id()`, and `from_persisted(...)` so a controller can
persist its CA material and reload it on next boot.
- commissioner::NocGenerator gains the matching `root_secret_key()`
accessor.
What is intentionally *not* in this PR (planned follow-ups):
- BLE central + BTP framing (the bootstrap transport).
- DCL fetch + Device Attestation chain verification.
- NetworkCommissioning cluster (Thread / Wi-Fi credential delivery).
- Operational discovery (_matter._tcp mDNS-SD client) + CASE.
- A higher-level Commissioner state machine wrapping all of the above.
Each of those is its own design conversation and they don't belong in
a single 3000-line PR.
Validation
----------
End-to-end test: examples/src/bin/pase_smoke_test.rs. With this PR
plus project-chip#454 (cert: NotBefore=0 sentinel fix — required for CHIP interop)
applied, the smoke test commissions chip-tool's all-clusters-app
end-to-end:
✓ PASE handshake
✓ ArmFailSafe(60s)
✓ CSRRequest → 243B NOCSRElements + 64B AttestationSignature
✓ NOCSRElements decoded; nonce verified
✓ AddTrustedRootCertificate
✓ AddNOC → device returns fabric_index=1
✓ CommissioningComplete
Responder log (chip-tool all-clusters-app):
OpCreds: successfully created fabric index 0x1 via AddNOC
`cargo test -p rs-matter --lib --features=os,zbus` passes (532
existing tests; no behavior change to existing crates).
Without project-chip#454 the smoke test will still complete PASE/ArmFailSafe/
CSRRequest but AddTrustedRootCertificate is rejected by CHIP as
'Invalid signature' — see project-chip#454 for the root cause.
glennswest
added a commit
to glennswest/rs-matter
that referenced
this pull request
May 18, 2026
Introduces a new `controller` module that provides the IM-invoke and
orchestration primitives a controller needs to drive a Matter accessory
from a freshly-established PASE session through to
CommissioningComplete.
The accessory role is already well-covered in this crate. This module
is the inverse role — the thing that *commissions* accessories — and
adds a deliberately small, validated surface:
Public API (controller::commissioner):
- arm_fail_safe(matter, expiry_seconds, breadcrumb)
GeneralCommissioning::ArmFailSafe over PASE, decodes
ArmFailSafeResponse.errorCode → maps non-OK to FailSafeExpired.
- csr_request(matter, &csr_nonce) -> CsrPayload
OperationalCredentials::CSRRequest with full CSRResponse decode.
Returns NOCSRElements (octstr<400>) + AttestationSignature (64 B).
- decode_nocsr_elements(blob) -> DecodedNocsr<'_>
TLV decoder for the NOCSRElements struct (§11.18.6.5.2) — pulls
the PKCS#10 CSR DER and the device's nonce-echo so the controller
can verify nonce freshness before issuing a NOC.
- add_noc(matter, noc, icac, ipk, admin_subject, admin_vendor_id)
OperationalCredentials::AddNOC with NOCResponse status +
FabricIndex decode. Non-OK status → AddNocRejected.
- commission_pase(matter, crypto, fabric_creds, admin_subject,
admin_vendor_id, fail_safe_secs) -> PaseCommissionResult
Orchestrator that chains the above plus the two unexposed steps
(AddTrustedRootCertificate, CommissioningComplete) and calls
FabricCredentials::generate_device_credentials to mint the NOC.
Returns the device-assigned FabricIndex + NodeID + cert chain.
Public API (controller::setup_code):
- Manual pairing-code + QR-code (MT:...) parser per Matter spec §5.1.4,
with version / vendor-id / product-id / discriminator / passcode /
discovery-capabilities-bitmask decoding.
Supporting additions to existing files:
- commissioner::FabricCredentials gains `root_secret_key()`,
`rcac_id()`, and `from_persisted(...)` so a controller can
persist its CA material and reload it on next boot.
- commissioner::NocGenerator gains the matching `root_secret_key()`
accessor.
What is intentionally *not* in this PR (planned follow-ups):
- BLE central + BTP framing (the bootstrap transport).
- DCL fetch + Device Attestation chain verification.
- NetworkCommissioning cluster (Thread / Wi-Fi credential delivery).
- Operational discovery (_matter._tcp mDNS-SD client) + CASE.
- A higher-level Commissioner state machine wrapping all of the above.
Each of those is its own design conversation and they don't belong in
a single 3000-line PR.
Validation
----------
End-to-end test: examples/src/bin/pase_smoke_test.rs. With this PR
plus project-chip#454 (cert: NotBefore=0 sentinel fix — required for CHIP interop)
applied, the smoke test commissions chip-tool's all-clusters-app
end-to-end:
✓ PASE handshake
✓ ArmFailSafe(60s)
✓ CSRRequest → 243B NOCSRElements + 64B AttestationSignature
✓ NOCSRElements decoded; nonce verified
✓ AddTrustedRootCertificate
✓ AddNOC → device returns fabric_index=1
✓ CommissioningComplete
Responder log (chip-tool all-clusters-app):
OpCreds: successfully created fabric index 0x1 via AddNOC
`cargo test -p rs-matter --lib --features=os,zbus` passes (532
existing tests; no behavior change to existing crates).
Without project-chip#454 the smoke test will still complete PASE/ArmFailSafe/
CSRRequest but AddTrustedRootCertificate is rejected by CHIP as
'Invalid signature' — see project-chip#454 for the root cause.
glennswest
added a commit
to glennswest/rs-matter
that referenced
this pull request
May 18, 2026
Introduces a new `controller` module that provides the IM-invoke and
orchestration primitives a controller needs to drive a Matter accessory
from a freshly-established PASE session through to
CommissioningComplete.
The accessory role is already well-covered in this crate. This module
is the inverse role — the thing that *commissions* accessories — and
adds a deliberately small, validated surface:
Public API (controller::commissioner):
- arm_fail_safe(matter, expiry_seconds, breadcrumb)
GeneralCommissioning::ArmFailSafe over PASE, decodes
ArmFailSafeResponse.errorCode → maps non-OK to FailSafeExpired.
- csr_request(matter, &csr_nonce) -> CsrPayload
OperationalCredentials::CSRRequest with full CSRResponse decode.
Returns NOCSRElements (octstr<400>) + AttestationSignature (64 B).
- decode_nocsr_elements(blob) -> DecodedNocsr<'_>
TLV decoder for the NOCSRElements struct (§11.18.6.5.2) — pulls
the PKCS#10 CSR DER and the device's nonce-echo so the controller
can verify nonce freshness before issuing a NOC.
- add_noc(matter, noc, icac, ipk, admin_subject, admin_vendor_id)
OperationalCredentials::AddNOC with NOCResponse status +
FabricIndex decode. Non-OK status → AddNocRejected.
- commission_pase(matter, crypto, fabric_creds, admin_subject,
admin_vendor_id, fail_safe_secs) -> PaseCommissionResult
Orchestrator that chains the above plus the two unexposed steps
(AddTrustedRootCertificate, CommissioningComplete) and calls
FabricCredentials::generate_device_credentials to mint the NOC.
Returns the device-assigned FabricIndex + NodeID + cert chain.
Public API (controller::setup_code):
- Manual pairing-code + QR-code (MT:...) parser per Matter spec §5.1.4,
with version / vendor-id / product-id / discriminator / passcode /
discovery-capabilities-bitmask decoding.
Supporting additions to existing files:
- commissioner::FabricCredentials gains `root_secret_key()`,
`rcac_id()`, and `from_persisted(...)` so a controller can
persist its CA material and reload it on next boot.
- commissioner::NocGenerator gains the matching `root_secret_key()`
accessor.
What is intentionally *not* in this PR (planned follow-ups):
- BLE central + BTP framing (the bootstrap transport).
- DCL fetch + Device Attestation chain verification.
- NetworkCommissioning cluster (Thread / Wi-Fi credential delivery).
- Operational discovery (_matter._tcp mDNS-SD client) + CASE.
- A higher-level Commissioner state machine wrapping all of the above.
Each of those is its own design conversation and they don't belong in
a single 3000-line PR.
Validation
----------
End-to-end test: examples/src/bin/pase_smoke_test.rs. With this PR
plus project-chip#454 (cert: NotBefore=0 sentinel fix — required for CHIP interop)
applied, the smoke test commissions chip-tool's all-clusters-app
end-to-end:
✓ PASE handshake
✓ ArmFailSafe(60s)
✓ CSRRequest → 243B NOCSRElements + 64B AttestationSignature
✓ NOCSRElements decoded; nonce verified
✓ AddTrustedRootCertificate
✓ AddNOC → device returns fabric_index=1
✓ CommissioningComplete
Responder log (chip-tool all-clusters-app):
OpCreds: successfully created fabric index 0x1 via AddNOC
`cargo test -p rs-matter --lib --features=os,zbus` passes (532
existing tests; no behavior change to existing crates).
Without project-chip#454 the smoke test will still complete PASE/ArmFailSafe/
CSRRequest but AddTrustedRootCertificate is rejected by CHIP as
'Invalid signature' — see project-chip#454 for the root cause.
Contributor
|
See #458 |
ivmarkov
added a commit
that referenced
this pull request
May 22, 2026
This is #454 except with all Gemini code review comments addressed (and future comments to be addressed too).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Matter operational certificates produced by
NocGeneratorwithNotBefore = 0(Matter epoch start, 2000-01-01 UTC) are rejected by the CHIP reference SDK onAddTrustedRootCertificatewithInvalid signature(CHIP Error 0x14). The CHIP source (CHIPCert.cpp:1076-1106) explicitly warns thatepochTime == 0is used as the X.509 no-well-defined-expiration sentinel and is always re-emitted asGeneralizedTime "99991231235959Z"— regardless of whether it lands inNotBeforeorNotAfter. On round-trip, CHIP reconstructs a different TBS DER than what rs-matter signed; the SHA-256 mismatches; ECDSA-verify fails.rs-matter's own self-verify passes because the conversion is symmetric within rs-matter. The failure only surfaces on interop with the CHIP SDK — which is exactly the production case.
Fix
NocGenerator: shiftNotBeforefrom 0 → 1 (2000-01-01 00:00:01) in the three places that build RCAC, ICAC and NOC. Cert remains effectively unbounded on the lower end, but clears CHIP's sentinel collision.cert::builderBasicConstraints: droppathLenConstraintfrom RCAC output. Matter Spec §6.5.11.1.2 mandates "pathLenConstraint SHALL NOT be present" for a Root CA cert; rs-matter was emittingINTEGER 1. Both ends carried the field through TLV→ASN.1, so signatures still verified, but strict validators (and the spec) reject it.New regression test
test_rcac_self_verifybuilds an RCAC viaRcacBuilderand re-parses it viaCertRef::verify_chain_start().finalise(). Confirms the TLV→ASN.1→hash→verify pipeline is internally consistent. None of the existing builder tests verified signature — they only checked length bounds.Test plan
cargo test -p rs-matter --lib cert::builder::tests --features=os— all 16 tests pass, including the new self-verify.chip-tool'sall-clusters-app(built from project-chip/connectedhomeip): PASE → ArmFailSafe → CSRRequest → AddTrustedRootCertificate → AddNOC → CommissioningComplete. Responder log:OpCreds: successfully created fabric index 0x1 via AddNOC.