From 0fab5605d586c802474410ca13a6b3d7f99e7ea9 Mon Sep 17 00:00:00 2001 From: "tennessee.carmelveilleux@gmail.com" Date: Fri, 25 Aug 2023 17:03:57 -0400 Subject: [PATCH 1/8] 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. --- .../test_case_vector.json | 14 +- .../test_case_vector.json | 14 +- src/crypto/CHIPCryptoPAL.cpp | 153 ++++++++++++++---- src/crypto/tests/CHIPCryptoPALTest.cpp | 68 +++++--- src/lib/support/BufferReader.cpp | 1 + src/lib/support/BufferReader.h | 19 +++ src/lib/support/tests/TestBufferReader.cpp | 15 ++ 7 files changed, 211 insertions(+), 73 deletions(-) diff --git a/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_08/test_case_vector.json b/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_08/test_case_vector.json index 85eb673f5d51fd..072948ec1b08dc 100644 --- a/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_08/test_case_vector.json +++ b/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_08/test_case_vector.json @@ -1,9 +1,9 @@ { - "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", - "dac_cert": "308201d93082017fa003020102020846cb41af3a7fc9c3300a06082a8648ce3d040302303b3139303706035504030c3041434d45204d617474657220446576656c20504149203543444139383939204d7669643a46464631204d7069643a42313020170d3232303932333030303030305a180f39393939313233313233353935395a30463118301606035504030c0f4d617474657220546573742044414331143012060a2b0601040182a27c02010c044646463131143012060a2b0601040182a27c02020c04303042313059301306072a8648ce3d020106082a8648ce3d03010703420004d3e1c5422ab213f118f959a33d2204342cf598dffcf5cec0474a1bba6e4c1a7263a59c605cf892038a704fd266149b5c89fe35e17e8dda279bb03cb1c7bf8ca6a360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e0416041430616dcabe874ac65e3dce88de7c5cc5defd86f9301f0603551d230418301680148fa036268e8eedbc73e12a0eeadcb2fbec8faaac300a06082a8648ce3d040302034800304502204ef134d60bb150def30f7c2df8f88c46755e36f48a12831aaedf17c4944cd70d022100f61555085107e4e783a5ad6abc64e1ee1f0d719b7d83a5f8bace41949f85c284", - "pai_cert": "308201c93082016fa003020102020821b17d362fe9afae300a06082a8648ce3d04030230303118301606035504030c0f4d617474657220546573742050414131143012060a2b0601040182a27c02010c04464646313020170d3232303932333030303030305a180f39393939313233313233353935395a303b3139303706035504030c3041434d45204d617474657220446576656c20504149203543444139383939204d7669643a46464631204d7069643a42313059301306072a8648ce3d020106082a8648ce3d03010703420004e72dbe2a0f600358978022ea70fb7ef3ef14de92a0dfb9cfb83b542fbccb7b8a7b13d3f37a26dfd2400224ae4ab82e1c13ca57a45e4ee54e862a192a6d4f1f3aa366306430120603551d130101ff040830060101ff020100300e0603551d0f0101ff040403020106301d0603551d0e041604148fa036268e8eedbc73e12a0eeadcb2fbec8faaac301f0603551d230418301680146afd22771f511fecbf1641976710dcdc31a1717e300a06082a8648ce3d040302034800304502210093a63c6f397d39dfa5b60c06c2839448ab5db0bf0728c7449c66bdc55e3ff6c302207b0c997ff95f1795b86828d1137b34f3f2aef1ffb2183a6e4c02836c1c735171", - "certification_declaration": "3081e906092a864886f70d010702a081db3081d8020103310d300b0609608648016503040201304406092a864886f70d010701a0370435152400012501f1ff360204b118250334122c04135a494732303134315a423333303030312d32342405002406002507769824080018317e307c020103801462fa823359acfaa9963e1cfa140addf504f37160300b0609608648016503040201300a06082a8648ce3d04030204483046022100e1d4967878513c474101db44c68061689be47443e21f6675c34b872edc5a319c022100a963e07bcd8e4abeb8b4fcb19f6a59c92f1a3bad4d7153f333684f93a836e3f9", - "dac_private_key": "05da60e1debbee09f31881ac82eb6ce5116db802606077661ff968312465fa75", - "dac_public_key": "04d3e1c5422ab213f118f959a33d2204342cf598dffcf5cec0474a1bba6e4c1a7263a59c605cf892038a704fd266149b5c89fe35e17e8dda279bb03cb1c7bf8ca6" + "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": "false", + "dac_cert": "308201d93082017fa003020102020846cb41af3a7fc9c3300a06082a8648ce3d040302303b3139303706035504030c3041434d45204d617474657220446576656c20504149203543444139383939204d7669643a46464631204d7069643a42313020170d3232303932333030303030305a180f39393939313233313233353935395a30463118301606035504030c0f4d617474657220546573742044414331143012060a2b0601040182a27c02010c044646463131143012060a2b0601040182a27c02020c04303042313059301306072a8648ce3d020106082a8648ce3d03010703420004d3e1c5422ab213f118f959a33d2204342cf598dffcf5cec0474a1bba6e4c1a7263a59c605cf892038a704fd266149b5c89fe35e17e8dda279bb03cb1c7bf8ca6a360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e0416041430616dcabe874ac65e3dce88de7c5cc5defd86f9301f0603551d230418301680148fa036268e8eedbc73e12a0eeadcb2fbec8faaac300a06082a8648ce3d040302034800304502204ef134d60bb150def30f7c2df8f88c46755e36f48a12831aaedf17c4944cd70d022100f61555085107e4e783a5ad6abc64e1ee1f0d719b7d83a5f8bace41949f85c284", + "pai_cert": "308201c93082016fa003020102020821b17d362fe9afae300a06082a8648ce3d04030230303118301606035504030c0f4d617474657220546573742050414131143012060a2b0601040182a27c02010c04464646313020170d3232303932333030303030305a180f39393939313233313233353935395a303b3139303706035504030c3041434d45204d617474657220446576656c20504149203543444139383939204d7669643a46464631204d7069643a42313059301306072a8648ce3d020106082a8648ce3d03010703420004e72dbe2a0f600358978022ea70fb7ef3ef14de92a0dfb9cfb83b542fbccb7b8a7b13d3f37a26dfd2400224ae4ab82e1c13ca57a45e4ee54e862a192a6d4f1f3aa366306430120603551d130101ff040830060101ff020100300e0603551d0f0101ff040403020106301d0603551d0e041604148fa036268e8eedbc73e12a0eeadcb2fbec8faaac301f0603551d230418301680146afd22771f511fecbf1641976710dcdc31a1717e300a06082a8648ce3d040302034800304502210093a63c6f397d39dfa5b60c06c2839448ab5db0bf0728c7449c66bdc55e3ff6c302207b0c997ff95f1795b86828d1137b34f3f2aef1ffb2183a6e4c02836c1c735171", + "certification_declaration": "3081e906092a864886f70d010702a081db3081d8020103310d300b0609608648016503040201304406092a864886f70d010701a0370435152400012501f1ff360204b118250334122c04135a494732303134315a423333303030312d32342405002406002507769824080018317e307c020103801462fa823359acfaa9963e1cfa140addf504f37160300b0609608648016503040201300a06082a8648ce3d04030204483046022100e1d4967878513c474101db44c68061689be47443e21f6675c34b872edc5a319c022100a963e07bcd8e4abeb8b4fcb19f6a59c92f1a3bad4d7153f333684f93a836e3f9", + "dac_private_key": "05da60e1debbee09f31881ac82eb6ce5116db802606077661ff968312465fa75", + "dac_public_key": "04d3e1c5422ab213f118f959a33d2204342cf598dffcf5cec0474a1bba6e4c1a7263a59c605cf892038a704fd266149b5c89fe35e17e8dda279bb03cb1c7bf8ca6" } diff --git a/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_09/test_case_vector.json b/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_09/test_case_vector.json index 6f714c607caacf..6f24f4af768b0c 100644 --- a/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_09/test_case_vector.json +++ b/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_09/test_case_vector.json @@ -1,9 +1,9 @@ { - "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", - "dac_cert": "308201d83082017da0030201020208697ea481e17f0f9c300a06082a8648ce3d04030230393137303506035504030c2e41434d45204d617474657220446576656c20504149203543444139383939204d7069643a204d7669643a464646313020170d3232303932333030303030305a180f39393939313233313233353935395a30463118301606035504030c0f4d617474657220546573742044414331143012060a2b0601040182a27c02010c044646463131143012060a2b0601040182a27c02020c04303042313059301306072a8648ce3d020106082a8648ce3d030107034200046eb8078776faf91061011baca81d4577b5b6b64e6f3009ddbc05aea20eaa044dbde2f70716de1d0ea2221a57ba5ca2ab23995fff18e63c6c675e403f7bf7ed53a360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e041604141c1bdaa7b3c18076e81187da63295ee068a7fe29301f0603551d23041830168014bd781517ec524b8b752e7e529063de96d6c2deb1300a06082a8648ce3d0403020349003046022100ca59fb5cc9fd777641ba761c9fbf16dc24f044fcfca5dead17ed7cae02a3f64d022100bb501e52bef2e0ff3d9b6c9bc8dac1c6adf94a19d66f38cf840b89e0bbd7dc12", - "pai_cert": "308201c63082016da00302010202082dbfc034ee2ebe32300a06082a8648ce3d04030230303118301606035504030c0f4d617474657220546573742050414131143012060a2b0601040182a27c02010c04464646313020170d3232303932333030303030305a180f39393939313233313233353935395a30393137303506035504030c2e41434d45204d617474657220446576656c20504149203543444139383939204d7069643a204d7669643a464646313059301306072a8648ce3d020106082a8648ce3d030107034200041a872e39c4f8337945c51b1a5dfa009e0f8402a59d996da057aa370acb18450bc3316babcf7c6eed0555300d7e9e4ac42c0e9556857835e8e8de2d91a1bbd27da366306430120603551d130101ff040830060101ff020100300e0603551d0f0101ff040403020106301d0603551d0e04160414bd781517ec524b8b752e7e529063de96d6c2deb1301f0603551d230418301680146afd22771f511fecbf1641976710dcdc31a1717e300a06082a8648ce3d040302034700304402205c38ff93c407961a98e61bc23ea65381a760f97ac0481ece8ee21bcd451b8c2002201d6c7ee510aec10ea02402f5f6d80bc74489fb8793d64a77e06b8fece69cd4b8", - "certification_declaration": "3081e706092a864886f70d010702a081d93081d6020103310d300b0609608648016503040201304406092a864886f70d010701a0370435152400012501f1ff360204b118250334122c04135a494732303134315a423333303030312d32342405002406002507769824080018317c307a020103801462fa823359acfaa9963e1cfa140addf504f37160300b0609608648016503040201300a06082a8648ce3d0403020446304402201f38b426df25ab4f552ded014845afb1a353e808e9250fc19dcfcab3d717679f0220575a610c996f0221628ff9a76123d3462922f91b6011ba727e6d42884ae88c08", - "dac_private_key": "8c0efeb0d08af4384bf3cf75256c3a47a88a514426d911375ac5bc7d03564212", - "dac_public_key": "046eb8078776faf91061011baca81d4577b5b6b64e6f3009ddbc05aea20eaa044dbde2f70716de1d0ea2221a57ba5ca2ab23995fff18e63c6c675e403f7bf7ed53" + "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": "false", + "dac_cert": "308201d83082017da0030201020208697ea481e17f0f9c300a06082a8648ce3d04030230393137303506035504030c2e41434d45204d617474657220446576656c20504149203543444139383939204d7069643a204d7669643a464646313020170d3232303932333030303030305a180f39393939313233313233353935395a30463118301606035504030c0f4d617474657220546573742044414331143012060a2b0601040182a27c02010c044646463131143012060a2b0601040182a27c02020c04303042313059301306072a8648ce3d020106082a8648ce3d030107034200046eb8078776faf91061011baca81d4577b5b6b64e6f3009ddbc05aea20eaa044dbde2f70716de1d0ea2221a57ba5ca2ab23995fff18e63c6c675e403f7bf7ed53a360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e041604141c1bdaa7b3c18076e81187da63295ee068a7fe29301f0603551d23041830168014bd781517ec524b8b752e7e529063de96d6c2deb1300a06082a8648ce3d0403020349003046022100ca59fb5cc9fd777641ba761c9fbf16dc24f044fcfca5dead17ed7cae02a3f64d022100bb501e52bef2e0ff3d9b6c9bc8dac1c6adf94a19d66f38cf840b89e0bbd7dc12", + "pai_cert": "308201c63082016da00302010202082dbfc034ee2ebe32300a06082a8648ce3d04030230303118301606035504030c0f4d617474657220546573742050414131143012060a2b0601040182a27c02010c04464646313020170d3232303932333030303030305a180f39393939313233313233353935395a30393137303506035504030c2e41434d45204d617474657220446576656c20504149203543444139383939204d7069643a204d7669643a464646313059301306072a8648ce3d020106082a8648ce3d030107034200041a872e39c4f8337945c51b1a5dfa009e0f8402a59d996da057aa370acb18450bc3316babcf7c6eed0555300d7e9e4ac42c0e9556857835e8e8de2d91a1bbd27da366306430120603551d130101ff040830060101ff020100300e0603551d0f0101ff040403020106301d0603551d0e04160414bd781517ec524b8b752e7e529063de96d6c2deb1301f0603551d230418301680146afd22771f511fecbf1641976710dcdc31a1717e300a06082a8648ce3d040302034700304402205c38ff93c407961a98e61bc23ea65381a760f97ac0481ece8ee21bcd451b8c2002201d6c7ee510aec10ea02402f5f6d80bc74489fb8793d64a77e06b8fece69cd4b8", + "certification_declaration": "3081e706092a864886f70d010702a081d93081d6020103310d300b0609608648016503040201304406092a864886f70d010701a0370435152400012501f1ff360204b118250334122c04135a494732303134315a423333303030312d32342405002406002507769824080018317c307a020103801462fa823359acfaa9963e1cfa140addf504f37160300b0609608648016503040201300a06082a8648ce3d0403020446304402201f38b426df25ab4f552ded014845afb1a353e808e9250fc19dcfcab3d717679f0220575a610c996f0221628ff9a76123d3462922f91b6011ba727e6d42884ae88c08", + "dac_private_key": "8c0efeb0d08af4384bf3cf75256c3a47a88a514426d911375ac5bc7d03564212", + "dac_public_key": "046eb8078776faf91061011baca81d4577b5b6b64e6f3009ddbc05aea20eaa044dbde2f70716de1d0ea2221a57ba5ca2ab23995fff18e63c6c675e403f7bf7ed53" } diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index 8d42fd49726156..bf4e8ac9d8bc5b 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -38,6 +38,8 @@ using chip::Encoding::LittleEndian::Reader; using namespace chip::ASN1; +namespace chip { +namespace Crypto { namespace { constexpr uint8_t kIntegerTag = 0x02u; @@ -192,10 +194,103 @@ CHIP_ERROR ConvertIntegerRawToDerInternal(const ByteSpan & raw_integer, MutableB return CHIP_NO_ERROR; } -} // namespace +/** + * @brief Find a 4 uppercase hex digit hex value after a prefix string. Used to implement + * fallback CN VID/PID encoding for PAA/PAI/DAC. + * + * @param[in] buffer - buffer in which to find the substring. + * @param[in] prefix - prefix to match, which must be followed by 4 uppercase hex characters + * @param[out] out_hex_value - on CHIP_NO_ERROR return, this will be the 16-bit hex value decoded. + * @return CHIP_NO_ERROR on success, CHIP_ERROR_NOT_FOUND if not detected and + * CHIP_ERROR_WRONG_CERT_DN if we saw the prefix but no valid hex string. + */ +CHIP_ERROR Find16BitUpperCaseHexAfterPrefix(const ByteSpan & buffer, const char* prefix, uint16_t & out_hex_value) +{ + chip::CharSpan prefix_span = chip::CharSpan::fromCharString(prefix); + + enum State : int { + kFindPrefix = 0, + kFindHex = 1, + kFoundHex = 2, + }; + + size_t min_len_required = prefix_span.size(); + bool found_prefix_at_least_once = false; + + for (size_t start_idx = 0; start_idx < buffer.size(); start_idx++) { + State state = State::kFindPrefix; + char hex_buf[4]; + size_t hex_len = 0; + + // Make a reader starting at the new cursor space as we traverse. + Reader reader{buffer}; + reader.Skip(start_idx); + if (!reader.HasAtLeast(min_len_required)) { + // We possibly can't match anymore since the prefix would not be there. + break; + } + + Reader prefix_reader{reinterpret_cast(prefix_span.data()), prefix_span.size()}; + + char current_char = '\0'; + bool done = false; + while (!done) { + if (!reader.ReadChar(¤t_char).IsSuccess()) + { + // Got to the end before getting all we wanted. + break; + } -namespace chip { -namespace Crypto { + switch (state) { + case State::kFindPrefix: { + char prefix_char = '\0'; + if (!prefix_reader.ReadChar(&prefix_char).IsSuccess()) { + // Not enough chars to finish prefix. + done = true; + break; + } + + if (current_char != prefix_char) { + // Mismatching char. + done = true; + break; + } + + if (prefix_reader.Remaining() == 0) { + // Reached end of prefix, change to finding the hex string. + state = State::kFindHex; + found_prefix_at_least_once = true; + } + break; + } + case State::kFindHex: + // Accumulate the chars, and just rely on strongly typed hex conversion + // to validate. + hex_buf[hex_len++] = current_char; + if (hex_len == sizeof(hex_buf)) + { + if (!Encoding::UppercaseHexToUint16(&hex_buf[0], sizeof(hex_buf), out_hex_value)) + { + // Invalid hex, we don't have a candidate. + done = true; + break; + } + + state = State::kFoundHex; + return CHIP_NO_ERROR; + } + break; + default: + done = true; + break; + } + } + } + + return found_prefix_at_least_once ? CHIP_ERROR_WRONG_CERT_DN : CHIP_ERROR_NOT_FOUND; +} + +} // namespace using HKDF_sha_crypto = HKDF_sha; @@ -871,41 +966,33 @@ CHIP_ERROR ExtractVIDPIDFromAttributeString(DNAttrType attrType, const ByteSpan // Otherwise, it is a CommonName attribute. else if (!vidpidFromCNAttr.Initialized()) { - char cnAttr[kMax_CommonNameAttr_Length + 1]; - if (attr.size() <= chip::Crypto::kMax_CommonNameAttr_Length) + ByteSpan attr_source_span{attr}; + if (attr_source_span.size() > chip::Crypto::kMax_CommonNameAttr_Length) { - memcpy(cnAttr, attr.data(), attr.size()); - cnAttr[attr.size()] = 0; + attr_source_span.reduce_size(chip::Crypto::kMax_CommonNameAttr_Length); + } - char * vid = strstr(cnAttr, kVIDPrefixForCNEncoding); - if (vid != nullptr) - { - vid += strlen(kVIDPrefixForCNEncoding); - if (cnAttr + attr.size() >= vid + kVIDandPIDHexLength) - { - uint16_t matterAttr; - if (Encoding::UppercaseHexToUint16(vid, kVIDandPIDHexLength, matterAttr) == sizeof(matterAttr)) - { - vidpidFromCNAttr.mVendorId.SetValue(static_cast(matterAttr)); - } - } - } + // Try to find a valid Vendor ID encoded in fallback method. + uint16_t vid = 0; + CHIP_ERROR err = Find16BitUpperCaseHexAfterPrefix(attr_source_span, kVIDPrefixForCNEncoding, vid); + if (err == CHIP_NO_ERROR) { + vidpidFromCNAttr.mVendorId.SetValue(static_cast(vid)); + } else if (err != CHIP_ERROR_NOT_FOUND) { + // This indicates a bad/ambiguous format. + return err; + } - char * pid = strstr(cnAttr, kPIDPrefixForCNEncoding); - if (pid != nullptr) - { - pid += strlen(kPIDPrefixForCNEncoding); - if (cnAttr + attr.size() >= pid + kVIDandPIDHexLength) - { - uint16_t matterAttr; - if (Encoding::UppercaseHexToUint16(pid, kVIDandPIDHexLength, matterAttr) == sizeof(matterAttr)) - { - vidpidFromCNAttr.mProductId.SetValue(matterAttr); - } - } - } + // Try to find a valid Product ID encoded in fallback method. + uint16_t pid = 0; + err = Find16BitUpperCaseHexAfterPrefix(attr_source_span, kPIDPrefixForCNEncoding, pid); + if (err == CHIP_NO_ERROR) { + vidpidFromCNAttr.mProductId.SetValue(pid); + } else if (err != CHIP_ERROR_NOT_FOUND) { + // This indicates a bad/ambiguous format. + return err; } } + return CHIP_NO_ERROR; } diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index bcf858d8b53e10..9de71768166533 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -2432,10 +2432,12 @@ static void TestVIDPID_StringExtraction(nlTestSuite * inSuite, void * inContext) const char * sTestCNAttribute11 = "ACME Matter Devel DAC 5CDA9899 Mvid:FFF1 Mpid:B1"; const char * sTestCNAttribute12 = "ACME Matter Devel DAC 5CDA9899 Mpid: Mvid:FFF1"; - // Common Name (CN) VID/PID encoding error cases (more examples): + // Common Name (CN) VID/PID encoding more cases (more examples): const char * sTestCNAttribute13 = "Mpid:987Mvid:FFF10x"; - const char * sTestCNAttribute14 = "MpidMvid:FFF10 Matter Test Mpid:FE67"; + const char * sTestCNAttribute14 = "MpidMvid:FFF10 Matter Test Mpid:FE67"; // Valid, even if there is run-in. const char * sTestCNAttribute15 = "Matter Devel Mpid:Mvid:Fff1"; + // Even though "Mpid:" appears thrice, only the value with correct hex afterwards is taken + const char * sTestCNAttribute16 = "Mpid:Mvid:FFF1 Mpid:12cd Matter Test Mpid:FE67"; struct TestCase { @@ -2476,50 +2478,64 @@ static void TestVIDPID_StringExtraction(nlTestSuite * inSuite, void * inContext) { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute07), strlen(sTestCNAttribute07)), true, true, chip::VendorId::TestVendor1, 0x00B1, CHIP_NO_ERROR }, { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute08), strlen(sTestCNAttribute08)), true, true, chip::VendorId::TestVendor1, 0x00B1, CHIP_NO_ERROR }, // Common Name (CN) VID/PID encoding error cases: - { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute09), strlen(sTestCNAttribute09)), false, true, chip::VendorId::NotSpecified, 0x00B1, CHIP_NO_ERROR }, - { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute10), strlen(sTestCNAttribute10)), false, true, chip::VendorId::NotSpecified, 0x00B1, CHIP_NO_ERROR }, - { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute11), strlen(sTestCNAttribute11)), true, false, chip::VendorId::TestVendor1, 0, CHIP_NO_ERROR }, - { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute12), strlen(sTestCNAttribute12)), true, false, chip::VendorId::TestVendor1, 0, CHIP_NO_ERROR }, - { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute13), strlen(sTestCNAttribute13)), true, false, chip::VendorId::TestVendor1, 0, CHIP_NO_ERROR }, + { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute09), strlen(sTestCNAttribute09)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN }, + { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute10), strlen(sTestCNAttribute10)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN }, + { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute11), strlen(sTestCNAttribute11)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN }, + { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute12), strlen(sTestCNAttribute12)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN }, + // Common Name (CN) VID/PID encoding additional cases: + { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute13), strlen(sTestCNAttribute13)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN }, { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute14), strlen(sTestCNAttribute14)), true, true, chip::VendorId::TestVendor1, 0xFE67, CHIP_NO_ERROR }, - { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute15), strlen(sTestCNAttribute15)), false, false, chip::VendorId::NotSpecified, 0, CHIP_NO_ERROR }, + { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute15), strlen(sTestCNAttribute15)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN }, + { DNAttrType::kCommonName, ByteSpan(reinterpret_cast(sTestCNAttribute16), strlen(sTestCNAttribute16)), true, true, chip::VendorId::TestVendor1, 0xFE67, CHIP_NO_ERROR }, // Other input combinations: { DNAttrType::kUnspecified, ByteSpan(reinterpret_cast(sTestCNAttribute15), strlen(sTestCNAttribute15)), false, false, chip::VendorId::NotSpecified, 0, CHIP_NO_ERROR }, { DNAttrType::kCommonName, ByteSpan(nullptr, 0), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_INVALID_ARGUMENT }, }; // clang-format on + int caseIdx = 0; for (const auto & testCase : kTestCases) { AttestationCertVidPid vidpid; AttestationCertVidPid vidpidFromCN; AttestationCertVidPid vidpidToCheck; CHIP_ERROR result = ExtractVIDPIDFromAttributeString(testCase.attrType, testCase.attr, vidpid, vidpidFromCN); - NL_TEST_ASSERT(inSuite, result == testCase.expectedResult); + ChipLogProgress(Crypto, "Checking VID/PID DN case %d. Expected: %" CHIP_ERROR_FORMAT , caseIdx, testCase.expectedResult.Format()); - if (testCase.attrType == DNAttrType::kMatterVID || testCase.attrType == DNAttrType::kMatterPID) - { - NL_TEST_ASSERT(inSuite, !vidpidFromCN.Initialized()); - vidpidToCheck = vidpid; + if (result != testCase.expectedResult) { + ChipLogError(Crypto, "Actual result: %" CHIP_ERROR_FORMAT, result.Format()); } - else if (testCase.attrType == DNAttrType::kCommonName) + NL_TEST_ASSERT(inSuite, result == testCase.expectedResult); + + // Only do assertions on output params in case of success since otherwise + // many of the output params are intermediate outputs. + if (result == CHIP_NO_ERROR) { - NL_TEST_ASSERT(inSuite, !vidpid.Initialized()); - vidpidToCheck = vidpidFromCN; - } + if (testCase.attrType == DNAttrType::kMatterVID || testCase.attrType == DNAttrType::kMatterPID) + { + NL_TEST_ASSERT(inSuite, !vidpidFromCN.Initialized()); + vidpidToCheck = vidpid; + } + else if (testCase.attrType == DNAttrType::kCommonName) + { + NL_TEST_ASSERT(inSuite, !vidpid.Initialized()); + vidpidToCheck = vidpidFromCN; + } - NL_TEST_ASSERT(inSuite, vidpidToCheck.mVendorId.HasValue() == testCase.expectedVidPresent); - NL_TEST_ASSERT(inSuite, vidpidToCheck.mProductId.HasValue() == testCase.expectedPidPresent); + NL_TEST_ASSERT(inSuite, vidpidToCheck.mVendorId.HasValue() == testCase.expectedVidPresent); + NL_TEST_ASSERT(inSuite, vidpidToCheck.mProductId.HasValue() == testCase.expectedPidPresent); - if (testCase.expectedVidPresent) - { - NL_TEST_ASSERT(inSuite, vidpidToCheck.mVendorId.Value() == testCase.expectedVid); - } + if (testCase.expectedVidPresent) + { + NL_TEST_ASSERT(inSuite, vidpidToCheck.mVendorId.Value() == testCase.expectedVid); + } - if (testCase.expectedPidPresent) - { - NL_TEST_ASSERT(inSuite, vidpidToCheck.mProductId.Value() == testCase.expectedPid); + if (testCase.expectedPidPresent) + { + NL_TEST_ASSERT(inSuite, vidpidToCheck.mProductId.Value() == testCase.expectedPid); + } } + ++caseIdx; } } diff --git a/src/lib/support/BufferReader.cpp b/src/lib/support/BufferReader.cpp index 0d214774e41efd..02bce8f3fe0615 100644 --- a/src/lib/support/BufferReader.cpp +++ b/src/lib/support/BufferReader.cpp @@ -92,6 +92,7 @@ Reader & Reader::ReadBytes(uint8_t * dest, size_t size) } // Explicit Read instantiations for the data types we want to support. +template void Reader::RawReadLowLevelBeCareful(char *); template void Reader::RawReadLowLevelBeCareful(bool *); template void Reader::RawReadLowLevelBeCareful(int8_t *); template void Reader::RawReadLowLevelBeCareful(int16_t *); diff --git a/src/lib/support/BufferReader.h b/src/lib/support/BufferReader.h index 14c251bd33f94d..f37667453e3512 100644 --- a/src/lib/support/BufferReader.h +++ b/src/lib/support/BufferReader.h @@ -108,6 +108,25 @@ class Reader CHECK_RETURN_VALUE Reader & ReadBool(bool * dest) { + static_assert(sizeof(bool) == 1, "Expect single-byte bools"); + RawReadLowLevelBeCareful(dest); + return *this; + } + + /** + * Read a char, assuming single byte storage. + * + * @param [out] dest Where the char just read should be placed. + * + * @note The read can put the reader in a failed-status state if there are + * not enough octets available. Callers must either continue to do + * more reads on the return value or check its status to see whether + * the sequence of reads that has been performed succeeded. + */ + CHECK_RETURN_VALUE + Reader & ReadChar(char * dest) + { + static_assert(sizeof(char) == 1, "Expect single-byte chars"); RawReadLowLevelBeCareful(dest); return *this; } diff --git a/src/lib/support/tests/TestBufferReader.cpp b/src/lib/support/tests/TestBufferReader.cpp index 9af64c9c1b148f..b054fae1e93082 100644 --- a/src/lib/support/tests/TestBufferReader.cpp +++ b/src/lib/support/tests/TestBufferReader.cpp @@ -255,6 +255,21 @@ static void TestBufferReader_LittleEndianScalars(nlTestSuite * inSuite, void * i NL_TEST_ASSERT(inSuite, val2 == true); NL_TEST_ASSERT(inSuite, val3 == true); } + + // Chars + { + uint8_t test_buf2[5] = { 'a', '\0', static_cast('\xff'), 'b', 'c' }; + chip::Encoding::LittleEndian::Reader reader{ ByteSpan{ test_buf2 } }; + char val1 = 'z'; + char val2 = 'z'; + char val3 = 'z'; + + NL_TEST_ASSERT(inSuite, reader.ReadChar(&val1).ReadChar(&val2).ReadChar(&val3).IsSuccess()); + NL_TEST_ASSERT(inSuite, reader.Remaining() == 2); + NL_TEST_ASSERT(inSuite, val1 == 'a'); + NL_TEST_ASSERT(inSuite, val2 == '\0'); + NL_TEST_ASSERT(inSuite, val3 == '\xff'); + } } #define NL_TEST_DEF_FN(fn) NL_TEST_DEF("Test " #fn, fn) From af9e2ce519045cf0ee88c62a509789f24ff5cf87 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Sat, 26 Aug 2023 16:00:09 +0000 Subject: [PATCH 2/8] Restyled by clang-format --- src/crypto/CHIPCryptoPAL.cpp | 164 ++++++++++++++----------- src/crypto/tests/CHIPCryptoPALTest.cpp | 6 +- 2 files changed, 93 insertions(+), 77 deletions(-) diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index bf4e8ac9d8bc5b..330aa271fab7be 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -204,87 +204,95 @@ CHIP_ERROR ConvertIntegerRawToDerInternal(const ByteSpan & raw_integer, MutableB * @return CHIP_NO_ERROR on success, CHIP_ERROR_NOT_FOUND if not detected and * CHIP_ERROR_WRONG_CERT_DN if we saw the prefix but no valid hex string. */ -CHIP_ERROR Find16BitUpperCaseHexAfterPrefix(const ByteSpan & buffer, const char* prefix, uint16_t & out_hex_value) +CHIP_ERROR Find16BitUpperCaseHexAfterPrefix(const ByteSpan & buffer, const char * prefix, uint16_t & out_hex_value) { chip::CharSpan prefix_span = chip::CharSpan::fromCharString(prefix); - enum State : int { - kFindPrefix = 0, - kFindHex = 1, - kFoundHex = 2, + enum State : int + { + kFindPrefix = 0, + kFindHex = 1, + kFoundHex = 2, }; - size_t min_len_required = prefix_span.size(); + size_t min_len_required = prefix_span.size(); bool found_prefix_at_least_once = false; - for (size_t start_idx = 0; start_idx < buffer.size(); start_idx++) { - State state = State::kFindPrefix; - char hex_buf[4]; - size_t hex_len = 0; - - // Make a reader starting at the new cursor space as we traverse. - Reader reader{buffer}; - reader.Skip(start_idx); - if (!reader.HasAtLeast(min_len_required)) { - // We possibly can't match anymore since the prefix would not be there. - break; - } - - Reader prefix_reader{reinterpret_cast(prefix_span.data()), prefix_span.size()}; - - char current_char = '\0'; - bool done = false; - while (!done) { - if (!reader.ReadChar(¤t_char).IsSuccess()) + for (size_t start_idx = 0; start_idx < buffer.size(); start_idx++) + { + State state = State::kFindPrefix; + char hex_buf[4]; + size_t hex_len = 0; + + // Make a reader starting at the new cursor space as we traverse. + Reader reader{ buffer }; + reader.Skip(start_idx); + if (!reader.HasAtLeast(min_len_required)) { - // Got to the end before getting all we wanted. - break; + // We possibly can't match anymore since the prefix would not be there. + break; } - switch (state) { - case State::kFindPrefix: { - char prefix_char = '\0'; - if (!prefix_reader.ReadChar(&prefix_char).IsSuccess()) { - // Not enough chars to finish prefix. - done = true; - break; - } + Reader prefix_reader{ reinterpret_cast(prefix_span.data()), prefix_span.size() }; - if (current_char != prefix_char) { - // Mismatching char. - done = true; - break; + char current_char = '\0'; + bool done = false; + while (!done) + { + if (!reader.ReadChar(¤t_char).IsSuccess()) + { + // Got to the end before getting all we wanted. + break; } - if (prefix_reader.Remaining() == 0) { - // Reached end of prefix, change to finding the hex string. - state = State::kFindHex; - found_prefix_at_least_once = true; - } - break; - } - case State::kFindHex: - // Accumulate the chars, and just rely on strongly typed hex conversion - // to validate. - hex_buf[hex_len++] = current_char; - if (hex_len == sizeof(hex_buf)) + switch (state) { - if (!Encoding::UppercaseHexToUint16(&hex_buf[0], sizeof(hex_buf), out_hex_value)) - { - // Invalid hex, we don't have a candidate. - done = true; - break; - } - - state = State::kFoundHex; - return CHIP_NO_ERROR; + case State::kFindPrefix: { + char prefix_char = '\0'; + if (!prefix_reader.ReadChar(&prefix_char).IsSuccess()) + { + // Not enough chars to finish prefix. + done = true; + break; + } + + if (current_char != prefix_char) + { + // Mismatching char. + done = true; + break; + } + + if (prefix_reader.Remaining() == 0) + { + // Reached end of prefix, change to finding the hex string. + state = State::kFindHex; + found_prefix_at_least_once = true; + } + break; + } + case State::kFindHex: + // Accumulate the chars, and just rely on strongly typed hex conversion + // to validate. + hex_buf[hex_len++] = current_char; + if (hex_len == sizeof(hex_buf)) + { + if (!Encoding::UppercaseHexToUint16(&hex_buf[0], sizeof(hex_buf), out_hex_value)) + { + // Invalid hex, we don't have a candidate. + done = true; + break; + } + + state = State::kFoundHex; + return CHIP_NO_ERROR; + } + break; + default: + done = true; + break; } - break; - default: - done = true; - break; } - } } return found_prefix_at_least_once ? CHIP_ERROR_WRONG_CERT_DN : CHIP_ERROR_NOT_FOUND; @@ -966,28 +974,34 @@ CHIP_ERROR ExtractVIDPIDFromAttributeString(DNAttrType attrType, const ByteSpan // Otherwise, it is a CommonName attribute. else if (!vidpidFromCNAttr.Initialized()) { - ByteSpan attr_source_span{attr}; + ByteSpan attr_source_span{ attr }; if (attr_source_span.size() > chip::Crypto::kMax_CommonNameAttr_Length) { - attr_source_span.reduce_size(chip::Crypto::kMax_CommonNameAttr_Length); + attr_source_span.reduce_size(chip::Crypto::kMax_CommonNameAttr_Length); } // Try to find a valid Vendor ID encoded in fallback method. - uint16_t vid = 0; + uint16_t vid = 0; CHIP_ERROR err = Find16BitUpperCaseHexAfterPrefix(attr_source_span, kVIDPrefixForCNEncoding, vid); - if (err == CHIP_NO_ERROR) { - vidpidFromCNAttr.mVendorId.SetValue(static_cast(vid)); - } else if (err != CHIP_ERROR_NOT_FOUND) { + if (err == CHIP_NO_ERROR) + { + vidpidFromCNAttr.mVendorId.SetValue(static_cast(vid)); + } + else if (err != CHIP_ERROR_NOT_FOUND) + { // This indicates a bad/ambiguous format. return err; } // Try to find a valid Product ID encoded in fallback method. uint16_t pid = 0; - err = Find16BitUpperCaseHexAfterPrefix(attr_source_span, kPIDPrefixForCNEncoding, pid); - if (err == CHIP_NO_ERROR) { - vidpidFromCNAttr.mProductId.SetValue(pid); - } else if (err != CHIP_ERROR_NOT_FOUND) { + err = Find16BitUpperCaseHexAfterPrefix(attr_source_span, kPIDPrefixForCNEncoding, pid); + if (err == CHIP_NO_ERROR) + { + vidpidFromCNAttr.mProductId.SetValue(pid); + } + else if (err != CHIP_ERROR_NOT_FOUND) + { // This indicates a bad/ambiguous format. return err; } diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index 9de71768166533..b689e01842fde3 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -2500,9 +2500,11 @@ static void TestVIDPID_StringExtraction(nlTestSuite * inSuite, void * inContext) AttestationCertVidPid vidpidFromCN; AttestationCertVidPid vidpidToCheck; CHIP_ERROR result = ExtractVIDPIDFromAttributeString(testCase.attrType, testCase.attr, vidpid, vidpidFromCN); - ChipLogProgress(Crypto, "Checking VID/PID DN case %d. Expected: %" CHIP_ERROR_FORMAT , caseIdx, testCase.expectedResult.Format()); + ChipLogProgress(Crypto, "Checking VID/PID DN case %d. Expected: %" CHIP_ERROR_FORMAT, caseIdx, + testCase.expectedResult.Format()); - if (result != testCase.expectedResult) { + if (result != testCase.expectedResult) + { ChipLogError(Crypto, "Actual result: %" CHIP_ERROR_FORMAT, result.Format()); } NL_TEST_ASSERT(inSuite, result == testCase.expectedResult); From a6d0f0ded9607ccafc16abbece171b3d307a8594 Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Sat, 26 Aug 2023 16:00:10 +0000 Subject: [PATCH 3/8] Restyled by prettier-json --- .../test_case_vector.json | 14 +++++++------- .../test_case_vector.json | 14 +++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_08/test_case_vector.json b/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_08/test_case_vector.json index 072948ec1b08dc..41d5d3d94211cb 100644 --- a/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_08/test_case_vector.json +++ b/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_08/test_case_vector.json @@ -1,9 +1,9 @@ { - "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": "false", - "dac_cert": "308201d93082017fa003020102020846cb41af3a7fc9c3300a06082a8648ce3d040302303b3139303706035504030c3041434d45204d617474657220446576656c20504149203543444139383939204d7669643a46464631204d7069643a42313020170d3232303932333030303030305a180f39393939313233313233353935395a30463118301606035504030c0f4d617474657220546573742044414331143012060a2b0601040182a27c02010c044646463131143012060a2b0601040182a27c02020c04303042313059301306072a8648ce3d020106082a8648ce3d03010703420004d3e1c5422ab213f118f959a33d2204342cf598dffcf5cec0474a1bba6e4c1a7263a59c605cf892038a704fd266149b5c89fe35e17e8dda279bb03cb1c7bf8ca6a360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e0416041430616dcabe874ac65e3dce88de7c5cc5defd86f9301f0603551d230418301680148fa036268e8eedbc73e12a0eeadcb2fbec8faaac300a06082a8648ce3d040302034800304502204ef134d60bb150def30f7c2df8f88c46755e36f48a12831aaedf17c4944cd70d022100f61555085107e4e783a5ad6abc64e1ee1f0d719b7d83a5f8bace41949f85c284", - "pai_cert": "308201c93082016fa003020102020821b17d362fe9afae300a06082a8648ce3d04030230303118301606035504030c0f4d617474657220546573742050414131143012060a2b0601040182a27c02010c04464646313020170d3232303932333030303030305a180f39393939313233313233353935395a303b3139303706035504030c3041434d45204d617474657220446576656c20504149203543444139383939204d7669643a46464631204d7069643a42313059301306072a8648ce3d020106082a8648ce3d03010703420004e72dbe2a0f600358978022ea70fb7ef3ef14de92a0dfb9cfb83b542fbccb7b8a7b13d3f37a26dfd2400224ae4ab82e1c13ca57a45e4ee54e862a192a6d4f1f3aa366306430120603551d130101ff040830060101ff020100300e0603551d0f0101ff040403020106301d0603551d0e041604148fa036268e8eedbc73e12a0eeadcb2fbec8faaac301f0603551d230418301680146afd22771f511fecbf1641976710dcdc31a1717e300a06082a8648ce3d040302034800304502210093a63c6f397d39dfa5b60c06c2839448ab5db0bf0728c7449c66bdc55e3ff6c302207b0c997ff95f1795b86828d1137b34f3f2aef1ffb2183a6e4c02836c1c735171", - "certification_declaration": "3081e906092a864886f70d010702a081db3081d8020103310d300b0609608648016503040201304406092a864886f70d010701a0370435152400012501f1ff360204b118250334122c04135a494732303134315a423333303030312d32342405002406002507769824080018317e307c020103801462fa823359acfaa9963e1cfa140addf504f37160300b0609608648016503040201300a06082a8648ce3d04030204483046022100e1d4967878513c474101db44c68061689be47443e21f6675c34b872edc5a319c022100a963e07bcd8e4abeb8b4fcb19f6a59c92f1a3bad4d7153f333684f93a836e3f9", - "dac_private_key": "05da60e1debbee09f31881ac82eb6ce5116db802606077661ff968312465fa75", - "dac_public_key": "04d3e1c5422ab213f118f959a33d2204342cf598dffcf5cec0474a1bba6e4c1a7263a59c605cf892038a704fd266149b5c89fe35e17e8dda279bb03cb1c7bf8ca6" + "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": "false", + "dac_cert": "308201d93082017fa003020102020846cb41af3a7fc9c3300a06082a8648ce3d040302303b3139303706035504030c3041434d45204d617474657220446576656c20504149203543444139383939204d7669643a46464631204d7069643a42313020170d3232303932333030303030305a180f39393939313233313233353935395a30463118301606035504030c0f4d617474657220546573742044414331143012060a2b0601040182a27c02010c044646463131143012060a2b0601040182a27c02020c04303042313059301306072a8648ce3d020106082a8648ce3d03010703420004d3e1c5422ab213f118f959a33d2204342cf598dffcf5cec0474a1bba6e4c1a7263a59c605cf892038a704fd266149b5c89fe35e17e8dda279bb03cb1c7bf8ca6a360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e0416041430616dcabe874ac65e3dce88de7c5cc5defd86f9301f0603551d230418301680148fa036268e8eedbc73e12a0eeadcb2fbec8faaac300a06082a8648ce3d040302034800304502204ef134d60bb150def30f7c2df8f88c46755e36f48a12831aaedf17c4944cd70d022100f61555085107e4e783a5ad6abc64e1ee1f0d719b7d83a5f8bace41949f85c284", + "pai_cert": "308201c93082016fa003020102020821b17d362fe9afae300a06082a8648ce3d04030230303118301606035504030c0f4d617474657220546573742050414131143012060a2b0601040182a27c02010c04464646313020170d3232303932333030303030305a180f39393939313233313233353935395a303b3139303706035504030c3041434d45204d617474657220446576656c20504149203543444139383939204d7669643a46464631204d7069643a42313059301306072a8648ce3d020106082a8648ce3d03010703420004e72dbe2a0f600358978022ea70fb7ef3ef14de92a0dfb9cfb83b542fbccb7b8a7b13d3f37a26dfd2400224ae4ab82e1c13ca57a45e4ee54e862a192a6d4f1f3aa366306430120603551d130101ff040830060101ff020100300e0603551d0f0101ff040403020106301d0603551d0e041604148fa036268e8eedbc73e12a0eeadcb2fbec8faaac301f0603551d230418301680146afd22771f511fecbf1641976710dcdc31a1717e300a06082a8648ce3d040302034800304502210093a63c6f397d39dfa5b60c06c2839448ab5db0bf0728c7449c66bdc55e3ff6c302207b0c997ff95f1795b86828d1137b34f3f2aef1ffb2183a6e4c02836c1c735171", + "certification_declaration": "3081e906092a864886f70d010702a081db3081d8020103310d300b0609608648016503040201304406092a864886f70d010701a0370435152400012501f1ff360204b118250334122c04135a494732303134315a423333303030312d32342405002406002507769824080018317e307c020103801462fa823359acfaa9963e1cfa140addf504f37160300b0609608648016503040201300a06082a8648ce3d04030204483046022100e1d4967878513c474101db44c68061689be47443e21f6675c34b872edc5a319c022100a963e07bcd8e4abeb8b4fcb19f6a59c92f1a3bad4d7153f333684f93a836e3f9", + "dac_private_key": "05da60e1debbee09f31881ac82eb6ce5116db802606077661ff968312465fa75", + "dac_public_key": "04d3e1c5422ab213f118f959a33d2204342cf598dffcf5cec0474a1bba6e4c1a7263a59c605cf892038a704fd266149b5c89fe35e17e8dda279bb03cb1c7bf8ca6" } diff --git a/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_09/test_case_vector.json b/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_09/test_case_vector.json index 6f24f4af768b0c..210bc861b49deb 100644 --- a/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_09/test_case_vector.json +++ b/credentials/development/commissioner_dut/struct_pai_vidpid_fallback_encoding_09/test_case_vector.json @@ -1,9 +1,9 @@ { - "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": "false", - "dac_cert": "308201d83082017da0030201020208697ea481e17f0f9c300a06082a8648ce3d04030230393137303506035504030c2e41434d45204d617474657220446576656c20504149203543444139383939204d7069643a204d7669643a464646313020170d3232303932333030303030305a180f39393939313233313233353935395a30463118301606035504030c0f4d617474657220546573742044414331143012060a2b0601040182a27c02010c044646463131143012060a2b0601040182a27c02020c04303042313059301306072a8648ce3d020106082a8648ce3d030107034200046eb8078776faf91061011baca81d4577b5b6b64e6f3009ddbc05aea20eaa044dbde2f70716de1d0ea2221a57ba5ca2ab23995fff18e63c6c675e403f7bf7ed53a360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e041604141c1bdaa7b3c18076e81187da63295ee068a7fe29301f0603551d23041830168014bd781517ec524b8b752e7e529063de96d6c2deb1300a06082a8648ce3d0403020349003046022100ca59fb5cc9fd777641ba761c9fbf16dc24f044fcfca5dead17ed7cae02a3f64d022100bb501e52bef2e0ff3d9b6c9bc8dac1c6adf94a19d66f38cf840b89e0bbd7dc12", - "pai_cert": "308201c63082016da00302010202082dbfc034ee2ebe32300a06082a8648ce3d04030230303118301606035504030c0f4d617474657220546573742050414131143012060a2b0601040182a27c02010c04464646313020170d3232303932333030303030305a180f39393939313233313233353935395a30393137303506035504030c2e41434d45204d617474657220446576656c20504149203543444139383939204d7069643a204d7669643a464646313059301306072a8648ce3d020106082a8648ce3d030107034200041a872e39c4f8337945c51b1a5dfa009e0f8402a59d996da057aa370acb18450bc3316babcf7c6eed0555300d7e9e4ac42c0e9556857835e8e8de2d91a1bbd27da366306430120603551d130101ff040830060101ff020100300e0603551d0f0101ff040403020106301d0603551d0e04160414bd781517ec524b8b752e7e529063de96d6c2deb1301f0603551d230418301680146afd22771f511fecbf1641976710dcdc31a1717e300a06082a8648ce3d040302034700304402205c38ff93c407961a98e61bc23ea65381a760f97ac0481ece8ee21bcd451b8c2002201d6c7ee510aec10ea02402f5f6d80bc74489fb8793d64a77e06b8fece69cd4b8", - "certification_declaration": "3081e706092a864886f70d010702a081d93081d6020103310d300b0609608648016503040201304406092a864886f70d010701a0370435152400012501f1ff360204b118250334122c04135a494732303134315a423333303030312d32342405002406002507769824080018317c307a020103801462fa823359acfaa9963e1cfa140addf504f37160300b0609608648016503040201300a06082a8648ce3d0403020446304402201f38b426df25ab4f552ded014845afb1a353e808e9250fc19dcfcab3d717679f0220575a610c996f0221628ff9a76123d3462922f91b6011ba727e6d42884ae88c08", - "dac_private_key": "8c0efeb0d08af4384bf3cf75256c3a47a88a514426d911375ac5bc7d03564212", - "dac_public_key": "046eb8078776faf91061011baca81d4577b5b6b64e6f3009ddbc05aea20eaa044dbde2f70716de1d0ea2221a57ba5ca2ab23995fff18e63c6c675e403f7bf7ed53" + "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": "false", + "dac_cert": "308201d83082017da0030201020208697ea481e17f0f9c300a06082a8648ce3d04030230393137303506035504030c2e41434d45204d617474657220446576656c20504149203543444139383939204d7069643a204d7669643a464646313020170d3232303932333030303030305a180f39393939313233313233353935395a30463118301606035504030c0f4d617474657220546573742044414331143012060a2b0601040182a27c02010c044646463131143012060a2b0601040182a27c02020c04303042313059301306072a8648ce3d020106082a8648ce3d030107034200046eb8078776faf91061011baca81d4577b5b6b64e6f3009ddbc05aea20eaa044dbde2f70716de1d0ea2221a57ba5ca2ab23995fff18e63c6c675e403f7bf7ed53a360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e041604141c1bdaa7b3c18076e81187da63295ee068a7fe29301f0603551d23041830168014bd781517ec524b8b752e7e529063de96d6c2deb1300a06082a8648ce3d0403020349003046022100ca59fb5cc9fd777641ba761c9fbf16dc24f044fcfca5dead17ed7cae02a3f64d022100bb501e52bef2e0ff3d9b6c9bc8dac1c6adf94a19d66f38cf840b89e0bbd7dc12", + "pai_cert": "308201c63082016da00302010202082dbfc034ee2ebe32300a06082a8648ce3d04030230303118301606035504030c0f4d617474657220546573742050414131143012060a2b0601040182a27c02010c04464646313020170d3232303932333030303030305a180f39393939313233313233353935395a30393137303506035504030c2e41434d45204d617474657220446576656c20504149203543444139383939204d7069643a204d7669643a464646313059301306072a8648ce3d020106082a8648ce3d030107034200041a872e39c4f8337945c51b1a5dfa009e0f8402a59d996da057aa370acb18450bc3316babcf7c6eed0555300d7e9e4ac42c0e9556857835e8e8de2d91a1bbd27da366306430120603551d130101ff040830060101ff020100300e0603551d0f0101ff040403020106301d0603551d0e04160414bd781517ec524b8b752e7e529063de96d6c2deb1301f0603551d230418301680146afd22771f511fecbf1641976710dcdc31a1717e300a06082a8648ce3d040302034700304402205c38ff93c407961a98e61bc23ea65381a760f97ac0481ece8ee21bcd451b8c2002201d6c7ee510aec10ea02402f5f6d80bc74489fb8793d64a77e06b8fece69cd4b8", + "certification_declaration": "3081e706092a864886f70d010702a081d93081d6020103310d300b0609608648016503040201304406092a864886f70d010701a0370435152400012501f1ff360204b118250334122c04135a494732303134315a423333303030312d32342405002406002507769824080018317c307a020103801462fa823359acfaa9963e1cfa140addf504f37160300b0609608648016503040201300a06082a8648ce3d0403020446304402201f38b426df25ab4f552ded014845afb1a353e808e9250fc19dcfcab3d717679f0220575a610c996f0221628ff9a76123d3462922f91b6011ba727e6d42884ae88c08", + "dac_private_key": "8c0efeb0d08af4384bf3cf75256c3a47a88a514426d911375ac5bc7d03564212", + "dac_public_key": "046eb8078776faf91061011baca81d4577b5b6b64e6f3009ddbc05aea20eaa044dbde2f70716de1d0ea2221a57ba5ca2ab23995fff18e63c6c675e403f7bf7ed53" } From 754918ac096c5b2fa3324bb984ac98c3eeadf180 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Mon, 28 Aug 2023 23:33:25 -0400 Subject: [PATCH 4/8] Address review comments by revamping algorithm --- src/crypto/CHIPCryptoPAL.cpp | 100 ++++++++++------------------------- 1 file changed, 29 insertions(+), 71 deletions(-) diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index 8e75daff77ce88..77435b59ada71d 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include using chip::ByteSpan; @@ -175,91 +176,48 @@ CHIP_ERROR Find16BitUpperCaseHexAfterPrefix(const ByteSpan & buffer, const char { chip::CharSpan prefix_span = chip::CharSpan::fromCharString(prefix); - enum State : int - { - kFindPrefix = 0, - kFindHex = 1, - kFoundHex = 2, - }; - - size_t min_len_required = prefix_span.size(); bool found_prefix_at_least_once = false; + // Scan string from left to right, to find the desired full matching substring. for (size_t start_idx = 0; start_idx < buffer.size(); start_idx++) { - State state = State::kFindPrefix; - char hex_buf[4]; - size_t hex_len = 0; + const uint8_t *cursor = buffer.data() + start_idx; + size_t remaining = buffer.size() - start_idx; - // Make a reader starting at the new cursor space as we traverse. - Reader reader{ buffer }; - reader.Skip(start_idx); - if (!reader.HasAtLeast(min_len_required)) + if (remaining < prefix_span.size()) { - // We possibly can't match anymore since the prefix would not be there. + // We can't possibly match prefix if not enough bytes left. break; } - Reader prefix_reader{ reinterpret_cast(prefix_span.data()), prefix_span.size() }; - - char current_char = '\0'; - bool done = false; - while (!done) + // Try to match the prefix at current position. + if (memcmp(cursor, prefix_span.data(), prefix_span.size()) != 0) { - if (!reader.ReadChar(¤t_char).IsSuccess()) - { - // Got to the end before getting all we wanted. - break; - } + // Did not find prefix, move to next position. + continue; + } else { + // Found prefix, skip to possible hex value. + found_prefix_at_least_once = true; + cursor += prefix_span.size(); + remaining -= prefix_span.size(); + } - switch (state) - { - case State::kFindPrefix: { - char prefix_char = '\0'; - if (!prefix_reader.ReadChar(&prefix_char).IsSuccess()) - { - // Not enough chars to finish prefix. - done = true; - break; - } + if (remaining < HEX_ENCODED_LENGTH(sizeof(uint16_t))) + { + // We can't possibly match the hex values if not enough bytes left. + break; + } - if (current_char != prefix_char) - { - // Mismatching char. - done = true; - break; - } + char hex_buf[4]; + memcpy(&hex_buf[0], cursor, sizeof(hex_buf)); - if (prefix_reader.Remaining() == 0) - { - // Reached end of prefix, change to finding the hex string. - state = State::kFindHex; - found_prefix_at_least_once = true; - } - break; - } - case State::kFindHex: - // Accumulate the chars, and just rely on strongly typed hex conversion - // to validate. - hex_buf[hex_len++] = current_char; - if (hex_len == sizeof(hex_buf)) - { - if (!Encoding::UppercaseHexToUint16(&hex_buf[0], sizeof(hex_buf), out_hex_value)) - { - // Invalid hex, we don't have a candidate. - done = true; - break; - } - - state = State::kFoundHex; - return CHIP_NO_ERROR; - } - break; - default: - done = true; - break; - } + if (Encoding::UppercaseHexToUint16(&hex_buf[0], sizeof(hex_buf), out_hex_value) != 0) + { + // Found first full valid match, return success, out_hex_value already updated. + return CHIP_NO_ERROR; } + + // Otherwise, did not find what we were looking for, try next position until exhausted. } return found_prefix_at_least_once ? CHIP_ERROR_WRONG_CERT_DN : CHIP_ERROR_NOT_FOUND; From 53350921fc84ba73d7bac752d6a11681e88904e0 Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Mon, 28 Aug 2023 23:34:22 -0400 Subject: [PATCH 5/8] Fix leftover comment follow-ups from @bzbarsky-apple from #28899 --- src/crypto/CHIPCryptoPAL.h | 2 +- src/crypto/OperationalKeystore.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/crypto/CHIPCryptoPAL.h b/src/crypto/CHIPCryptoPAL.h index 9fba6d989cddbb..be19881c342c43 100644 --- a/src/crypto/CHIPCryptoPAL.h +++ b/src/crypto/CHIPCryptoPAL.h @@ -755,7 +755,7 @@ CHIP_ERROR AES_CTR_crypt(const uint8_t * input, size_t input_length, const Aes12 * be configured to ignore CSR requested subject. * * @param keypair The key pair for which a CSR should be generated. Must not be null. - * @param csr_span Span to hold the resulting CSR. Must be at least kMIN_CSR_Buffer_Size. + * @param csr_span Span to hold the resulting CSR. Must have size at least kMIN_CSR_Buffer_Size. * Otherwise returns CHIP_ERROR_BUFFER_TOO_SMALL. It will get resized to * actual size needed on success. diff --git a/src/crypto/OperationalKeystore.h b/src/crypto/OperationalKeystore.h index 58eba784281786..8ecc461f49b9dd 100644 --- a/src/crypto/OperationalKeystore.h +++ b/src/crypto/OperationalKeystore.h @@ -67,7 +67,7 @@ class OperationalKeystore * Only one pending operational keypair is supported at a time. * * @param fabricIndex - FabricIndex for which a new keypair must be made available - * @param outCertificateSigningRequest - Buffer to contain the CSR. Must be at least `kMIN_CSR_Buffer_Size` large. + * @param outCertificateSigningRequest - Buffer to contain the CSR. Must have size at least `kMIN_CSR_Buffer_Size`. * * @retval CHIP_NO_ERROR on success * @retval CHIP_ERROR_BUFFER_TOO_SMALL if `outCertificateSigningRequest` buffer is too small From 27fe59f5f938282a62b6bb9c4e4e4e3cf154cb0b Mon Sep 17 00:00:00 2001 From: "Restyled.io" Date: Tue, 29 Aug 2023 03:34:54 +0000 Subject: [PATCH 6/8] Restyled by clang-format --- src/crypto/CHIPCryptoPAL.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index 77435b59ada71d..72605b1265e216 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -181,8 +181,8 @@ CHIP_ERROR Find16BitUpperCaseHexAfterPrefix(const ByteSpan & buffer, const char // Scan string from left to right, to find the desired full matching substring. for (size_t start_idx = 0; start_idx < buffer.size(); start_idx++) { - const uint8_t *cursor = buffer.data() + start_idx; - size_t remaining = buffer.size() - start_idx; + const uint8_t * cursor = buffer.data() + start_idx; + size_t remaining = buffer.size() - start_idx; if (remaining < prefix_span.size()) { @@ -195,7 +195,9 @@ CHIP_ERROR Find16BitUpperCaseHexAfterPrefix(const ByteSpan & buffer, const char { // Did not find prefix, move to next position. continue; - } else { + } + else + { // Found prefix, skip to possible hex value. found_prefix_at_least_once = true; cursor += prefix_span.size(); From 52ba070a57c1b3961c72c672958a42f388b07f81 Mon Sep 17 00:00:00 2001 From: "tennessee.carmelveilleux@gmail.com" Date: Tue, 29 Aug 2023 10:44:03 -0400 Subject: [PATCH 7/8] Add more comments and fix clang-tidy --- src/crypto/CHIPCryptoPAL.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index 72605b1265e216..d6d6199e302177 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -179,6 +179,13 @@ CHIP_ERROR Find16BitUpperCaseHexAfterPrefix(const ByteSpan & buffer, const char bool found_prefix_at_least_once = false; // Scan string from left to right, to find the desired full matching substring. + // + // IMPORTANT NOTE: We are trying to find the equivalent of prefix + [0-9A-F]{4}. + // The appearance of the full prefix, but not followed by the hex value, must + // be detected, as it is illegal if there isn't a valid prefix within the string. + // This is why we first check for the prefix and then maybe check for the hex + // value, rather than doing a single check of making sure there is enough space + // for both. for (size_t start_idx = 0; start_idx < buffer.size(); start_idx++) { const uint8_t * cursor = buffer.data() + start_idx; @@ -196,13 +203,11 @@ CHIP_ERROR Find16BitUpperCaseHexAfterPrefix(const ByteSpan & buffer, const char // Did not find prefix, move to next position. continue; } - else - { - // Found prefix, skip to possible hex value. - found_prefix_at_least_once = true; - cursor += prefix_span.size(); - remaining -= prefix_span.size(); - } + + // Otherwise, found prefix, skip to possible hex value. + found_prefix_at_least_once = true; + cursor += prefix_span.size(); + remaining -= prefix_span.size(); if (remaining < HEX_ENCODED_LENGTH(sizeof(uint16_t))) { From 515a9d000cd0bbf0d14661a422ab5c9a3a20d731 Mon Sep 17 00:00:00 2001 From: "tennessee.carmelveilleux@gmail.com" Date: Tue, 29 Aug 2023 10:46:47 -0400 Subject: [PATCH 8/8] Address more review comments --- src/crypto/CHIPCryptoPAL.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index d6d6199e302177..f8a611875a5667 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -209,13 +209,14 @@ CHIP_ERROR Find16BitUpperCaseHexAfterPrefix(const ByteSpan & buffer, const char cursor += prefix_span.size(); remaining -= prefix_span.size(); - if (remaining < HEX_ENCODED_LENGTH(sizeof(uint16_t))) + constexpr size_t expected_hex_len = HEX_ENCODED_LENGTH(sizeof(uint16_t)); + if (remaining < expected_hex_len) { // We can't possibly match the hex values if not enough bytes left. break; } - char hex_buf[4]; + char hex_buf[expected_hex_len]; memcpy(&hex_buf[0], cursor, sizeof(hex_buf)); if (Encoding::UppercaseHexToUint16(&hex_buf[0], sizeof(hex_buf), out_hex_value) != 0)