From 70cbc02a07837a3f73ed552549f7f4ae4140211b Mon Sep 17 00:00:00 2001 From: Tennessee Carmel-Veilleux Date: Tue, 29 Aug 2023 15:28:45 -0400 Subject: [PATCH] Fix corner cases of handling of Common Name fallback encoding (#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 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 Co-authored-by: Restyled.io --- .../test_case_vector.json | 2 +- .../test_case_vector.json | 2 +- src/crypto/CHIPCryptoPAL.cpp | 133 +++++++++++++----- src/crypto/CHIPCryptoPAL.h | 2 +- src/crypto/OperationalKeystore.h | 2 +- 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 ++ 9 files changed, 182 insertions(+), 62 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..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,6 +1,6 @@ { "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", + "is_success_case": "false", "dac_cert": "308201d93082017fa003020102020846cb41af3a7fc9c3300a06082a8648ce3d040302303b3139303706035504030c3041434d45204d617474657220446576656c20504149203543444139383939204d7669643a46464631204d7069643a42313020170d3232303932333030303030305a180f39393939313233313233353935395a30463118301606035504030c0f4d617474657220546573742044414331143012060a2b0601040182a27c02010c044646463131143012060a2b0601040182a27c02020c04303042313059301306072a8648ce3d020106082a8648ce3d03010703420004d3e1c5422ab213f118f959a33d2204342cf598dffcf5cec0474a1bba6e4c1a7263a59c605cf892038a704fd266149b5c89fe35e17e8dda279bb03cb1c7bf8ca6a360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e0416041430616dcabe874ac65e3dce88de7c5cc5defd86f9301f0603551d230418301680148fa036268e8eedbc73e12a0eeadcb2fbec8faaac300a06082a8648ce3d040302034800304502204ef134d60bb150def30f7c2df8f88c46755e36f48a12831aaedf17c4944cd70d022100f61555085107e4e783a5ad6abc64e1ee1f0d719b7d83a5f8bace41949f85c284", "pai_cert": "308201c93082016fa003020102020821b17d362fe9afae300a06082a8648ce3d04030230303118301606035504030c0f4d617474657220546573742050414131143012060a2b0601040182a27c02010c04464646313020170d3232303932333030303030305a180f39393939313233313233353935395a303b3139303706035504030c3041434d45204d617474657220446576656c20504149203543444139383939204d7669643a46464631204d7069643a42313059301306072a8648ce3d020106082a8648ce3d03010703420004e72dbe2a0f600358978022ea70fb7ef3ef14de92a0dfb9cfb83b542fbccb7b8a7b13d3f37a26dfd2400224ae4ab82e1c13ca57a45e4ee54e862a192a6d4f1f3aa366306430120603551d130101ff040830060101ff020100300e0603551d0f0101ff040403020106301d0603551d0e041604148fa036268e8eedbc73e12a0eeadcb2fbec8faaac301f0603551d230418301680146afd22771f511fecbf1641976710dcdc31a1717e300a06082a8648ce3d040302034800304502210093a63c6f397d39dfa5b60c06c2839448ab5db0bf0728c7449c66bdc55e3ff6c302207b0c997ff95f1795b86828d1137b34f3f2aef1ffb2183a6e4c02836c1c735171", "certification_declaration": "3081e906092a864886f70d010702a081db3081d8020103310d300b0609608648016503040201304406092a864886f70d010701a0370435152400012501f1ff360204b118250334122c04135a494732303134315a423333303030312d32342405002406002507769824080018317e307c020103801462fa823359acfaa9963e1cfa140addf504f37160300b0609608648016503040201300a06082a8648ce3d04030204483046022100e1d4967878513c474101db44c68061689be47443e21f6675c34b872edc5a319c022100a963e07bcd8e4abeb8b4fcb19f6a59c92f1a3bad4d7153f333684f93a836e3f9", 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..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,6 +1,6 @@ { "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", + "is_success_case": "false", "dac_cert": "308201d83082017da0030201020208697ea481e17f0f9c300a06082a8648ce3d04030230393137303506035504030c2e41434d45204d617474657220446576656c20504149203543444139383939204d7069643a204d7669643a464646313020170d3232303932333030303030305a180f39393939313233313233353935395a30463118301606035504030c0f4d617474657220546573742044414331143012060a2b0601040182a27c02010c044646463131143012060a2b0601040182a27c02020c04303042313059301306072a8648ce3d020106082a8648ce3d030107034200046eb8078776faf91061011baca81d4577b5b6b64e6f3009ddbc05aea20eaa044dbde2f70716de1d0ea2221a57ba5ca2ab23995fff18e63c6c675e403f7bf7ed53a360305e300c0603551d130101ff04023000300e0603551d0f0101ff040403020780301d0603551d0e041604141c1bdaa7b3c18076e81187da63295ee068a7fe29301f0603551d23041830168014bd781517ec524b8b752e7e529063de96d6c2deb1300a06082a8648ce3d0403020349003046022100ca59fb5cc9fd777641ba761c9fbf16dc24f044fcfca5dead17ed7cae02a3f64d022100bb501e52bef2e0ff3d9b6c9bc8dac1c6adf94a19d66f38cf840b89e0bbd7dc12", "pai_cert": "308201c63082016da00302010202082dbfc034ee2ebe32300a06082a8648ce3d04030230303118301606035504030c0f4d617474657220546573742050414131143012060a2b0601040182a27c02010c04464646313020170d3232303932333030303030305a180f39393939313233313233353935395a30393137303506035504030c2e41434d45204d617474657220446576656c20504149203543444139383939204d7069643a204d7669643a464646313059301306072a8648ce3d020106082a8648ce3d030107034200041a872e39c4f8337945c51b1a5dfa009e0f8402a59d996da057aa370acb18450bc3316babcf7c6eed0555300d7e9e4ac42c0e9556857835e8e8de2d91a1bbd27da366306430120603551d130101ff040830060101ff020100300e0603551d0f0101ff040403020106301d0603551d0e04160414bd781517ec524b8b752e7e529063de96d6c2deb1301f0603551d230418301680146afd22771f511fecbf1641976710dcdc31a1717e300a06082a8648ce3d040302034700304402205c38ff93c407961a98e61bc23ea65381a760f97ac0481ece8ee21bcd451b8c2002201d6c7ee510aec10ea02402f5f6d80bc74489fb8793d64a77e06b8fece69cd4b8", "certification_declaration": "3081e706092a864886f70d010702a081d93081d6020103310d300b0609608648016503040201304406092a864886f70d010701a0370435152400012501f1ff360204b118250334122c04135a494732303134315a423333303030312d32342405002406002507769824080018317c307a020103801462fa823359acfaa9963e1cfa140addf504f37160300b0609608648016503040201300a06082a8648ce3d0403020446304402201f38b426df25ab4f552ded014845afb1a353e808e9250fc19dcfcab3d717679f0220575a610c996f0221628ff9a76123d3462922f91b6011ba727e6d42884ae88c08", diff --git a/src/crypto/CHIPCryptoPAL.cpp b/src/crypto/CHIPCryptoPAL.cpp index 57410c8b74aff2..f8a611875a5667 100644 --- a/src/crypto/CHIPCryptoPAL.cpp +++ b/src/crypto/CHIPCryptoPAL.cpp @@ -29,6 +29,7 @@ #include #include #include +#include #include using chip::ByteSpan; @@ -38,6 +39,8 @@ using chip::Encoding::LittleEndian::Reader; using namespace chip::ASN1; +namespace chip { +namespace Crypto { namespace { constexpr uint8_t kIntegerTag = 0x02u; @@ -159,10 +162,76 @@ 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); -namespace chip { -namespace Crypto { + 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; + size_t remaining = buffer.size() - start_idx; + + if (remaining < prefix_span.size()) + { + // We can't possibly match prefix if not enough bytes left. + break; + } + + // Try to match the prefix at current position. + if (memcmp(cursor, prefix_span.data(), prefix_span.size()) != 0) + { + // Did not find prefix, move to next position. + continue; + } + + // Otherwise, found prefix, skip to possible hex value. + found_prefix_at_least_once = true; + cursor += prefix_span.size(); + remaining -= prefix_span.size(); + + 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[expected_hex_len]; + memcpy(&hex_buf[0], cursor, sizeof(hex_buf)); + + 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; +} + +} // namespace using HKDF_sha_crypto = HKDF_sha; @@ -887,41 +956,39 @@ 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/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 diff --git a/src/crypto/tests/CHIPCryptoPALTest.cpp b/src/crypto/tests/CHIPCryptoPALTest.cpp index 7ba0ab1a088f12..98be91ee13c706 100644 --- a/src/crypto/tests/CHIPCryptoPALTest.cpp +++ b/src/crypto/tests/CHIPCryptoPALTest.cpp @@ -2565,10 +2565,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 { @@ -2609,50 +2611,66 @@ 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) + if (result != testCase.expectedResult) { - NL_TEST_ASSERT(inSuite, !vidpidFromCN.Initialized()); - vidpidToCheck = vidpid; + 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)