Fix corner cases of handling of Common Name fallback encoding #28911
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
Problem: - Appearance of a Mpid:/Mvid: in a DAC/PAI/PAA DN was deemed OK by previous code, but this caused a critical ambiguity in PAIs which would possibly cause fall-back to non-PID-scoped PAI interpretation. - Related to CHIP-Specifications/connectedhomeip-spec#7470 - Fixes project-chip#28898 This PR: - Replaces the logic for fallback encodign conversion to take the first legitimate fully matching case for Mvid: and Mpid: and detect errors where either of these is present but without a following Mpid/Mvid. - Updates unit tests to improve coverage and to properly mark as invalid some cases marked invalid in spec which where deemed valid by prior code by mistake Testing done: - Integration tests still pass (relater to Commissioner DUT). - Test vectors updated. - New unit tests added.
pullapprove
bot
requested review from
andy31415,
andyg-apple,
anush-apple,
arkq,
bzbarsky-apple,
carol-apple,
cecille,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
Damian-Nordic,
dhrishi,
electrocucaracha,
emargolis,
gjc13,
harsha-rajendran,
hawk248,
hicklin,
jepenven-silabs,
jmartinez-silabs,
jmeg-sfy and
joonhaengHeo
pullapprove
bot
requested review from
tobiasgraf,
turon,
vivien-apple,
woody-apple,
younghak-hwang,
yufengwangca and
yunhanw-google
|
PR #28911: Size comparison from 2d5fe2c to a6d0f0d Increases (16 builds for bl702, bl702l, cc32xx, esp32, linux, psoc6)
Decreases (15 builds for bl702, bl702l, cc32xx, efr32, linux, psoc6)
Full report (62 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
andy31415
approved these changes
Aug 28, 2023
|
PR #28911: Size comparison from c43d6f7 to 27fe59f Increases (14 builds for bl702, bl702l, cc32xx, linux, psoc6)
Decreases (15 builds for bl702, bl702l, cc32xx, efr32, linux, psoc6, telink)
Full report (62 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
andy31415
reviewed
Aug 29, 2023
andy31415
reviewed
Aug 29, 2023
andy31415
approved these changes
Aug 29, 2023
|
PR #28911: Size comparison from c43d6f7 to 515a9d0 Increases (16 builds for bl702, bl702l, cc32xx, linux, nrfconnect, psoc6)
Decreases (17 builds for bl702, bl702l, cc32xx, efr32, linux, psoc6)
Full report (62 builds for bl602, bl702, bl702l, cc32xx, cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg, telink)
|
jmartinez-silabs
approved these changes
Aug 29, 2023
github-actions bot
pushed a commit
that referenced
this pull request
Aug 29, 2023
* Fix corner cases of handling of Common Name fallback encoding Problem: - Appearance of a Mpid:/Mvid: in a DAC/PAI/PAA DN was deemed OK by previous code, but this caused a critical ambiguity in PAIs which would possibly cause fall-back to non-PID-scoped PAI interpretation. - Related to https://github.com/CHIP-Specifications/connectedhomeip-spec/issues/7470 - Fixes #28898 This PR: - Replaces the logic for fallback encodign conversion to take the first legitimate fully matching case for Mvid: and Mpid: and detect errors where either of these is present but without a following Mpid/Mvid. - Updates unit tests to improve coverage and to properly mark as invalid some cases marked invalid in spec which where deemed valid by prior code by mistake Testing done: - Integration tests still pass (relater to Commissioner DUT). - Test vectors updated. - New unit tests added. * Restyled by clang-format * Restyled by prettier-json * Address review comments by revamping algorithm * Fix leftover comment follow-ups from @bzbarsky-apple from #28899 * Restyled by clang-format * Add more comments and fix clang-tidy * Address more review comments --------- Co-authored-by: tennessee.carmelveilleux@gmail.com <tennessee@google.com> Co-authored-by: Restyled.io <commits@restyled.io>
HunsupJung
pushed a commit
to HunsupJung/connectedhomeip
that referenced
this pull request
Oct 23, 2023
…t-chip#28911) * Fix corner cases of handling of Common Name fallback encoding Problem: - Appearance of a Mpid:/Mvid: in a DAC/PAI/PAA DN was deemed OK by previous code, but this caused a critical ambiguity in PAIs which would possibly cause fall-back to non-PID-scoped PAI interpretation. - Related to CHIP-Specifications/connectedhomeip-spec#7470 - Fixes project-chip#28898 This PR: - Replaces the logic for fallback encodign conversion to take the first legitimate fully matching case for Mvid: and Mpid: and detect errors where either of these is present but without a following Mpid/Mvid. - Updates unit tests to improve coverage and to properly mark as invalid some cases marked invalid in spec which where deemed valid by prior code by mistake Testing done: - Integration tests still pass (relater to Commissioner DUT). - Test vectors updated. - New unit tests added. * Restyled by clang-format * Restyled by prettier-json * Address review comments by revamping algorithm * Fix leftover comment follow-ups from @bzbarsky-apple from project-chip#28899 * Restyled by clang-format * Add more comments and fix clang-tidy * Address more review comments --------- Co-authored-by: tennessee.carmelveilleux@gmail.com <tennessee@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.
Problem:
Mpid:/Mvid:without value in a DAC/PAI/PAA DN was deemed OK by previous code, but this caused a critical ambiguity in PAIs which would possibly cause fall-back to non-PID-scoped PAI interpretation.This PR:
Testing done: