Joint Fabric Mutual VID Verification Implementation #40868
+3,930
−341
Conversation
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
…istrator' into Issue#38543
pullapprove
bot
added
review - pending
and removed
examples
controller
app
vscode
messaging
labels
There was a problem hiding this comment.
Pull Request Overview
This PR implements Joint Fabric Mutual VID Verification functionality as part of Issue #38543. The changes introduce comprehensive trust verification mechanisms for Joint Commissioning Management (JCM) in the CHIP network.
Key changes include:
- Implementation of vendor ID verification procedures with certificate chain validation
- Addition of JCM test certificates and unit tests for trust verification scenarios
- Refactoring of trust verification state machine architecture with improved delegate patterns
Reviewed Changes
Copilot reviewed 56 out of 56 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/messaging/tests/MessagingContext.h/cpp | Added JF fabric test infrastructure with new session creation methods |
| src/credentials/tests/certificates/* | Added JF test certificates (root, IA, node) for fabric A and B |
| src/credentials/tests/CHIPCert_unit_test_vectors.h/cpp | Added accessor functions for JF test certificate assets |
| src/credentials/tests/TestFabricTable.cpp | Added VID verification test using JF test certificates |
| src/controller/tests/jcm/* | Added comprehensive JCM trust verification unit tests |
| src/controller/jcm/TrustVerification.h/cpp | Implemented vendor ID verification client with certificate chain validation |
| src/controller/jcm/DeviceCommissioner.h/cpp | Enhanced JCM commissioner with trust verification state machine |
| src/controller/jcm/JCMCommissionee.h/cpp | Added commissionee-side trust verification logic |
| src/controller/CHIPDeviceController.h/cpp | Updated base commissioner interface for JCM trust verification |
Comments suppressed due to low confidence (2)
src/credentials/tests/TestFabricTable.cpp:1
- The switch statement in the header file EnumToString function does not handle kVendorIdVerificationFailed, but the test file expects it to return 'VENDOR_ID_VERIFICATION_FAILED'. This will cause the test to fail as it will return 'UNKNOWN_ERROR' instead.
/*
src/controller/jcm/TrustVerification.cpp:1
- Incorrect session variable used - should be mJFSessionAToB instead of mSessionAliceToBob for JF CASE session creation.
/*
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
chrisdecenzo
approved these changes
Oct 16, 2025
|
PR #40868: Size comparison from 7e83b9f to f486c87 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #40868: Size comparison from 4f17295 to 39aee59 Full report (36 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nxp, psoc6, qpg, realtek, stm32, telink)
|
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 70 out of 70 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (2)
src/python_testing/TC_JFDS_2_1.py:1
- The passcode range 11022011-11022099 is too restrictive and may not generate valid 8-digit passcodes. Matter passcodes must be 8 digits (27-bit values), and this range only allows 89 possible values. Consider using a wider range like random.randint(10000000, 99999999) to ensure better randomness and avoid potential collisions in test runs.
#
src/controller/jcm/DeviceCommissioner.cpp:1
- Variable name mismatch: the error is logged using
errbut should useverifyErrwhich contains the actual error fromVerifyVendorId. The variableerris from an outer scope and may contain a stale or incorrect error value.
/*
|
PR #40868: Size comparison from e156205 to 56e8b49 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
apahl-cl
approved these changes
Oct 28, 2025
apahl-cl
approved these changes
Oct 28, 2025
Co-authored-by: Copilot <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 70 out of 70 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (2)
src/credentials/jcm/VendorIdVerificationClient.cpp:1
- The error log uses the wrong error variable. It should use 'verifyErr' instead of 'err', as 'err' is the outer scope error from FetchCommissionerInfo callback, not the VerifyVendorId result.
src/controller/jcm/DeviceCommissioner.cpp:1 - The function CreateJFCASESessionAToB with CAT values uses kBobKeyId instead of kJFBKeyId. This appears to be a copy-paste error and should use kJFBKeyId to match the session being created.
/*
src/app/clusters/joint-fabric-administrator-server/joint-fabric-administrator-server.cpp
Show resolved
Hide resolved
src/app/clusters/joint-fabric-administrator-server/JCMCommissionee.cpp
Outdated
Show resolved
Hide resolved
|
PR #40868: Size comparison from 3eeb701 to 41598cd Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
…onee.cpp Co-authored-by: Copilot <[email protected]>
|
PR #40868: Size comparison from a3d3ee5 to 1e4ab76 Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #40868: Size comparison from a8ce5a9 to 48f8b1c Full report (35 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 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
Add mutual VID Verification to the JCM steps. This includes VID Verifiication from Ecosystem A -> B administrators and Ecosystem B -> A administrators. General changes:
Related issues
Fixes: #38543
Testing
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)
See: Pull Request Guidelines