Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix corner cases of handling of Common Name fallback encoding #28911

Merged
merged 10 commits into from
Aug 29, 2023
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
Original file line number Diff line number Diff line change
@@ -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",
Expand Down
167 changes: 134 additions & 33 deletions src/crypto/CHIPCryptoPAL.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ using chip::Encoding::LittleEndian::Reader;

using namespace chip::ASN1;

namespace chip {
namespace Crypto {
namespace {

constexpr uint8_t kIntegerTag = 0x02u;
Expand Down Expand Up @@ -192,10 +194,111 @@ 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 {
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<const uint8_t *>(prefix_span.data()), prefix_span.size() };

char current_char = '\0';
bool done = false;
while (!done)
{
if (!reader.ReadChar(&current_char).IsSuccess())
{
// Got to the end before getting all we wanted.
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;
}

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;

Expand Down Expand Up @@ -871,41 +974,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<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;
}

Expand Down
68 changes: 43 additions & 25 deletions src/crypto/tests/CHIPCryptoPALTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down Expand Up @@ -2476,50 +2478,66 @@ static void TestVIDPID_StringExtraction(nlTestSuite * inSuite, void * inContext)
{ 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);
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;
}
}

Expand Down
1 change: 1 addition & 0 deletions src/lib/support/BufferReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 *);
Expand Down
Loading