Introduce fail-safe compliant Operational Cert storage #19643
Merged
Conversation
This file contains 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
- This PR is an intermediate fully unit-tested step towards replacing all NOC/ICAC/RCAC storage from Fabric-Table to allow spec-compliant fail-safe implementation - Right now, we have hacky code in opcreds cluster server and in FabricTable to attempt to affect fail-safe properly. It doesn't actually work and prevents the TrustedRootCertificates, NOCs and Fabrics attributes from being implemented properly, and prevents UpdateNOC from being implemented to spec Issue project-chip#18633 Issue project-chip#17208 Issue project-chip#15585 This PR implements an operational cert storage interface and provides an implementation based on exact storage from existing FabricTable for backwards compatibility. Its usage in FabricTable and via opcreds cluster is a follow-up. Testing done: - Added large-scale new code unit test - Unit tests pass - Not hooked-up to device software yet, so just the unit tests need to pass
pullapprove
bot
requested review from
andy31415,
anush-apple,
arkq,
Byungjoo-Lee,
bzbarsky-apple,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
franck-apple,
gjc13,
harimau-qirex,
hawk248,
harsha-rajendran,
isiu-apple,
jelderton,
jepenven-silabs,
jmartinez-silabs,
jtung-apple,
kghost,
kpschoedel,
lazarkov,
LuDuda and
mlepage-google
pullapprove
bot
requested review from
msandstedt,
rgoliver,
sagar-apple,
saurabhst,
selissia,
tecimovic,
tehampson,
turon,
vijs,
vivien-apple,
wbschiller,
woody-apple,
xylophone21 and
yunhanw-google
|
PR #19643: Size comparison from be878d3 to d8c92de Increases (1 build for telink)
Decreases (1 build for telink)
Full report (41 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
|
PR #19643: Size comparison from be878d3 to 386392a Increases (1 build for telink)
Decreases (2 builds for esp32, telink)
Full report (41 builds for cc13x2_26x2, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
woody-apple
approved these changes
Jun 16, 2022
andy31415
approved these changes
Jun 16, 2022
| * return CHIP_ERROR_INCORRECT_STATE since it is illegal in this implementation to store an | ||
| * NOC chain without associated root. | ||
| * | ||
| * NOTE: The Matter spec allows AddNOC without AddTrustedRootCertificate if the NOC |
There was a problem hiding this comment.
Hmm.. Our AddNOC impl right now definitely does not handle this right. Is there an issue tracking that?
tcarmelveilleux
added a commit
to tcarmelveilleux/connectedhomeip
that referenced
this pull request
Jun 21, 2022
- After PR project-chip#19643 was merge, @bzbarsky-apple had several minor editorial comments. - This PR applies the comments from project-chip#19643 (review) Testing: - Unit tests still pass - No integration yet, so unit tests passing is enough
woody-apple
pushed a commit
that referenced
this pull request
Jun 22, 2022
* Follow-up minor changes to OpCertStore - After PR #19643 was merge, @bzbarsky-apple had several minor editorial comments. - This PR applies the comments from #19643 (review) Testing: - Unit tests still pass - No integration yet, so unit tests passing is enough * Restyled by clang-format * Fix grammar Co-authored-by: Restyled.io <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Problem
all NOC/ICAC/RCAC storage from Fabric-Table to allow spec-compliant
fail-safe implementation
to attempt to affect fail-safe properly. It doesn't actually work and
prevents the TrustedRootCertificates, NOCs and Fabrics attributes from
being implemented properly, and prevents UpdateNOC from being implemented
to spec
Issue #18633
Issue #17208
Issue #15585
Change overview
This PR implements an operational cert storage interface and provides
an implementation based on exact storage from existing FabricTable
for backwards compatibility. Its usage in FabricTable and via
opcreds cluster is a follow-up.
Testing
Testing done: