Support local files for DCL, Indirect CRL signing and improved code structure + bug fixes. #37593
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
* Fix line endings for master DM XMLs * Add newline at end of file for master DM XMLs * Add closures as a file exception - device types all in one file * DM XMLs: Update master to 0.7 of 1.5 and use alchemy Note that this contains a significant number of changes in one commit, and I apologise for that, but the new spec cannot be scraped with the DM editor, so I needed to swap to alchemy to get the new spec scraped. Hence this contains both the alchemy formatting changes (as outlined in project-chip#37201 as well as the changes from the last ballot to the current ballot. * Update file list in build file * Fix unit tests to expect clusters removed in 1.5 * Update device type parsing to warn on non-int IDs rather than failing This happens on ID-TBD in the ballot. * Add test to ensure all spec device types are OK. * Add a stringifier to XmlFeature XmlCommand * Restyled by autopep8 * Restyled by isort --------- Co-authored-by: Restyled.io <commits@restyled.io>
|
Review changes with |
|
PR #37593: Size comparison from 08535fd to 79f990c Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #37593: Size comparison from 33aec35 to 8ee84ee Full report (69 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #37593: Size comparison from 33aec35 to feaa0d7 Full report (3 builds for cc32xx, stm32)
|
shubhamdp
approved these changes
Feb 21, 2025
|
PR #37593: Size comparison from ab3d5ae to 6fae058 Full report (74 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
gmarcosb
pushed a commit
to gmarcosb/connectedhomeip
that referenced
this pull request
Mar 4, 2025
…tructure + bug fixes. (project-chip#37593) * DM XMLs: 1.5 0.7 ballot (project-chip#37329) * Fix line endings for master DM XMLs * Add newline at end of file for master DM XMLs * Add closures as a file exception - device types all in one file * DM XMLs: Update master to 0.7 of 1.5 and use alchemy Note that this contains a significant number of changes in one commit, and I apologise for that, but the new spec cannot be scraped with the DM editor, so I needed to swap to alchemy to get the new spec scraped. Hence this contains both the alchemy formatting changes (as outlined in project-chip#37201 as well as the changes from the last ballot to the current ballot. * Update file list in build file * Fix unit tests to expect clusters removed in 1.5 * Update device type parsing to warn on non-int IDs rather than failing This happens on ID-TBD in the ballot. * Add test to ensure all spec device types are OK. * Add a stringifier to XmlFeature XmlCommand * Restyled by autopep8 * Restyled by isort --------- Co-authored-by: Restyled.io <commits@restyled.io> * updated script * updated script * updated script * updated script * updated script * updated script * updated script * updated script * updated script * Restyled by autopep8 * Restyled by isort * updated format * Restyled by autopep8 * fix lint erros * Restyled by autopep8 * cleanup docs * Restyled by autopep8 * cleanup docs * Fixes from comments first review. * Restyled by autopep8 * Fixes from comments first review part 2. * Fixes from comments first review part 3. * Remove from-crl * Restyled by autopep8 * Restyled by isort * Remove from-crl * Restyled by autopep8 * Restyled by isort * Remove from-crl * Remove from-crl * Restyled by autopep8 * Restyled by isort * Remove from-crl * Remove from-crl * Reformat * Restyled by isort * Reformat * Reformat * Reformat * Reformat --------- Co-authored-by: C Freeman <cecille@google.com> Co-authored-by: Restyled.io <commits@restyled.io>
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.
Testing
Test was done locally with local files, the test net, and main net. Existing unit tests were also run.
Changes included:
The DCLDClient has been broken up into an interface and three different implementations so simplify. A different client is used depending on if the user of the script wants to use the DCL via a DCLD binary, the over HTTP, or based on local files.
User can now run the full algorithm against a locally defined files using 'from-dcl' with the "use-local-data" flag. They input a json containing the expected get-revocatoin-points response, the CRLs they're checking against, and the PAA certificate. This allows users to verify that their DCL entry will function properly before they push it to the DCL.
get_issuer_cert() has been removed, and replaced with get_paa_cert. It takes a certificate and repeatedly look up its issuer until it finds the DCL entry for the PAA. Step 5 in the main algo now uses the get_paa_cert function instead of get_issuer_cert. In the case of an indirect CRL signer for PAIs get_issuer_cert would have returned the PAI certificate and not the PAA certificate as is needed to verify the certificate chain
During step 7 of the algo Issuing Distribution Point was incorrectly being retrieved with "x509.OID_ISSUING_DISTRIBUTION_POINT" which does not exist. Its been corrected to use correct OID definition.
Passing around of b64 encoded x509.Name has been removed in favor of passing the x509.Name object. This allows for easier debugging and the logging of the b64 encoded name is less useful for users than the seeing the formatted readable name. Note that the algo still returns the b64 name in the entry json as it is expected to be this way by other parts of the codebase.
get_akid and get_skid will now throw an error if the extension is missing. The error is handled throughout the code and warnings are thrown to indicate that the relevant certificate or CRL is not valid.