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 <tennessee@google.com>
Co-authored-by: Restyled.io <commits@restyled.io>
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 85eb673..41d5d3d 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 6f714c6..210bc86 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 57410c8..f8a6118 100644
--- a/src/crypto/CHIPCryptoPAL.cpp
+++ b/src/crypto/CHIPCryptoPAL.cpp
@@ -29,6 +29,7 @@
#include <lib/support/BytesToHex.h>
#include <lib/support/CodeUtils.h>
#include <lib/support/Span.h>
+#include <stdint.h>
#include <string.h>
using chip::ByteSpan;
@@ -38,6 +39,8 @@
using namespace chip::ASN1;
+namespace chip {
+namespace Crypto {
namespace {
constexpr uint8_t kIntegerTag = 0x02u;
@@ -159,10 +162,76 @@
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 @@
// 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<VendorId>(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<VendorId>(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 9fba6d9..be19881 100644
--- a/src/crypto/CHIPCryptoPAL.h
+++ b/src/crypto/CHIPCryptoPAL.h
@@ -755,7 +755,7 @@
* 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 58eba78..8ecc461 100644
--- a/src/crypto/OperationalKeystore.h
+++ b/src/crypto/OperationalKeystore.h
@@ -67,7 +67,7 @@
* 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 7ba0ab1..98be91e 100644
--- a/src/crypto/tests/CHIPCryptoPALTest.cpp
+++ b/src/crypto/tests/CHIPCryptoPALTest.cpp
@@ -2565,10 +2565,12 @@
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 @@
{ DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute07), strlen(sTestCNAttribute07)), true, true, chip::VendorId::TestVendor1, 0x00B1, CHIP_NO_ERROR },
{ DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(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<const uint8_t *>(sTestCNAttribute09), strlen(sTestCNAttribute09)), false, true, chip::VendorId::NotSpecified, 0x00B1, CHIP_NO_ERROR },
- { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute10), strlen(sTestCNAttribute10)), false, true, chip::VendorId::NotSpecified, 0x00B1, CHIP_NO_ERROR },
- { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute11), strlen(sTestCNAttribute11)), true, false, chip::VendorId::TestVendor1, 0, CHIP_NO_ERROR },
- { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute12), strlen(sTestCNAttribute12)), true, false, chip::VendorId::TestVendor1, 0, CHIP_NO_ERROR },
- { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute13), strlen(sTestCNAttribute13)), true, false, chip::VendorId::TestVendor1, 0, CHIP_NO_ERROR },
+ { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute09), strlen(sTestCNAttribute09)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN },
+ { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute10), strlen(sTestCNAttribute10)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN },
+ { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute11), strlen(sTestCNAttribute11)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN },
+ { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(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<const uint8_t *>(sTestCNAttribute13), strlen(sTestCNAttribute13)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN },
{ DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute14), strlen(sTestCNAttribute14)), true, true, chip::VendorId::TestVendor1, 0xFE67, CHIP_NO_ERROR },
- { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute15), strlen(sTestCNAttribute15)), false, false, chip::VendorId::NotSpecified, 0, CHIP_NO_ERROR },
+ { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute15), strlen(sTestCNAttribute15)), false, false, chip::VendorId::NotSpecified, 0, CHIP_ERROR_WRONG_CERT_DN },
+ { DNAttrType::kCommonName, ByteSpan(reinterpret_cast<const uint8_t *>(sTestCNAttribute16), strlen(sTestCNAttribute16)), true, true, chip::VendorId::TestVendor1, 0xFE67, CHIP_NO_ERROR },
// Other input combinations:
{ DNAttrType::kUnspecified, ByteSpan(reinterpret_cast<const uint8_t *>(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);
+ ChipLogProgress(Crypto, "Checking VID/PID DN case %d. Expected: %" CHIP_ERROR_FORMAT, caseIdx,
+ testCase.expectedResult.Format());
+
+ if (result != testCase.expectedResult)
+ {
+ ChipLogError(Crypto, "Actual result: %" CHIP_ERROR_FORMAT, result.Format());
+ }
NL_TEST_ASSERT(inSuite, result == testCase.expectedResult);
- if (testCase.attrType == DNAttrType::kMatterVID || testCase.attrType == DNAttrType::kMatterPID)
+ // 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, !vidpidFromCN.Initialized());
- vidpidToCheck = vidpid;
- }
- else if (testCase.attrType == DNAttrType::kCommonName)
- {
- 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 0d21477..02bce8f 100644
--- a/src/lib/support/BufferReader.cpp
+++ b/src/lib/support/BufferReader.cpp
@@ -92,6 +92,7 @@
}
// 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 14c251b..f376674 100644
--- a/src/lib/support/BufferReader.h
+++ b/src/lib/support/BufferReader.h
@@ -108,6 +108,25 @@
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 9af64c9..b054fae 100644
--- a/src/lib/support/tests/TestBufferReader.cpp
+++ b/src/lib/support/tests/TestBufferReader.cpp
@@ -255,6 +255,21 @@
NL_TEST_ASSERT(inSuite, val2 == true);
NL_TEST_ASSERT(inSuite, val3 == true);
}
+
+ // Chars
+ {
+ uint8_t test_buf2[5] = { 'a', '\0', static_cast<uint8_t>('\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)
