[BUG] Fallback encoding for PAI with Mpid: blank appears to pass device attestation #28898
Labels
Comments
tcarmelveilleux
pushed a commit
to tcarmelveilleux/connectedhomeip
that referenced
this issue
Aug 26, 2023
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.
mergify bot
pushed a commit
that referenced
this issue
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 CHIP-Specifications/connectedhomeip-spec#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: [email protected] <[email protected]> Co-authored-by: Restyled.io <[email protected]>
github-actions bot
pushed a commit
that referenced
this issue
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: [email protected] <[email protected]> Co-authored-by: Restyled.io <[email protected]>
HunsupJung
pushed a commit
to HunsupJung/connectedhomeip
that referenced
this issue
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: [email protected] <[email protected]> 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
Reproduction steps
Test vector struct_pai_vidpid_fallback_encoding_09 has a test vector as follows:
"description": "PAI Test Vector: Fallback VID and PID encoding example from spec: invalid, since substring following Mpid: is not exactly 4 uppercase hexadecimal digits",
"is_success_case": "true",
It's not clear why this is marked as a success case or why this passes device attestation.
Similar problem in struct_pai_vidpid_fallback_encoding_08
The vid versions of these test are (correct) failures.
Bug prevalence
always
GitHub hash of the SDK that was being used
c3fb124
Platform
core
Platform Version(s)
No response
Anything else?
No response
The text was updated successfully, but these errors were encountered: