diff --git a/src/credentials/DeviceAttestationConstructor.cpp b/src/credentials/DeviceAttestationConstructor.cpp index eb206f2f56aa43..4d5949ffa9699a 100644 --- a/src/credentials/DeviceAttestationConstructor.cpp +++ b/src/credentials/DeviceAttestationConstructor.cpp @@ -68,8 +68,9 @@ CHIP_ERROR DeconstructAttestationElements(const ByteSpan & attestationElements, bool certificationDeclarationExists = false; bool attestationNonceExists = false; bool timestampExists = false; - bool firmwareInfoExists = false; - uint32_t lastContextTagId = UINT32_MAX; + bool gotFirstContextTag = false; + uint32_t lastContextTagId = 0; + TLV::ContiguousBufferTLVReader tlvReader; TLV::TLVType containerType = TLV::kTLVType_Structure; @@ -85,47 +86,54 @@ CHIP_ERROR DeconstructAttestationElements(const ByteSpan & attestationElements, while ((error = tlvReader.Next()) == CHIP_NO_ERROR) { TLV::Tag tag = tlvReader.GetTag(); - if (!TLV::IsContextTag(tag)) + { break; + } - switch (TLV::TagNumFromTag(tag)) + // Ensure tag-order and correct first expected tag + uint32_t contextTagId = TLV::TagNumFromTag(tag); + if (!gotFirstContextTag) + { + // First tag must always be Certification Declaration + VerifyOrReturnError(contextTagId == kCertificationDeclarationTagId, CHIP_ERROR_UNEXPECTED_TLV_ELEMENT); + gotFirstContextTag = true; + } + else + { + // Subsequent tags must always be in order + VerifyOrReturnError(contextTagId > lastContextTagId, CHIP_ERROR_UNEXPECTED_TLV_ELEMENT); + } + lastContextTagId = contextTagId; + + switch (contextTagId) { case kCertificationDeclarationTagId: - VerifyOrReturnError(lastContextTagId == UINT32_MAX, CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT); - VerifyOrReturnError(certificationDeclarationExists == false, CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT); ReturnErrorOnFailure(tlvReader.GetByteView(certificationDeclaration)); certificationDeclarationExists = true; break; case kAttestationNonceTagId: - VerifyOrReturnError(lastContextTagId == kCertificationDeclarationTagId, CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT); - VerifyOrReturnError(attestationNonceExists == false, CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT); ReturnErrorOnFailure(tlvReader.GetByteView(attestationNonce)); attestationNonceExists = true; break; case kTimestampTagId: - VerifyOrReturnError(lastContextTagId == kAttestationNonceTagId, CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT); - VerifyOrReturnError(timestampExists == false, CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT); ReturnErrorOnFailure(tlvReader.Get(timestamp)); timestampExists = true; break; case kFirmwareInfoTagId: - VerifyOrReturnError(lastContextTagId == kTimestampTagId, CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT); - VerifyOrReturnError(firmwareInfoExists == false, CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT); ReturnErrorOnFailure(tlvReader.GetByteView(firmwareInfo)); - firmwareInfoExists = true; break; default: - return CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT; + // It's OK to have future context tags before vendor specific tags. + // We already checked that the tags are in order. + break; } - - lastContextTagId = TLV::TagNumFromTag(tag); } VerifyOrReturnError(error == CHIP_NO_ERROR || error == CHIP_END_OF_TLV, error); - VerifyOrReturnError(lastContextTagId == kTimestampTagId || lastContextTagId == kFirmwareInfoTagId, - CHIP_ERROR_MISSING_TLV_ELEMENT); + const bool allTagsNeededPresent = certificationDeclarationExists && attestationNonceExists && timestampExists; + VerifyOrReturnError(allTagsNeededPresent, CHIP_ERROR_MISSING_TLV_ELEMENT); size_t count = 0; ReturnErrorOnFailure(CountVendorReservedElementsInDA(attestationElements, count)); diff --git a/src/credentials/DeviceAttestationVendorReserved.h b/src/credentials/DeviceAttestationVendorReserved.h index 576ff2dfff3e22..600dd7be2eba32 100644 --- a/src/credentials/DeviceAttestationVendorReserved.h +++ b/src/credentials/DeviceAttestationVendorReserved.h @@ -40,7 +40,7 @@ class DeviceAttestationVendorReservedDeconstructor public: DeviceAttestationVendorReservedDeconstructor() {} - // read TLV until first profile tag + // Read TLV until first profile tag CHIP_ERROR PrepareToReadVendorReservedElements(const ByteSpan & attestationElements, size_t count) { mIsInitialized = false; @@ -55,11 +55,14 @@ class DeviceAttestationVendorReservedDeconstructor while (true) { ReturnErrorOnFailure(mTlvReader.Next()); - if (!TLV::IsProfileTag(mTlvReader.GetTag())) + + TLV::Tag tag = mTlvReader.GetTag(); + if (!TLV::IsContextTag(tag)) break; } - // positioned to first context tag (vendor reserved data) - mIsInitialized = true; + // positioned to first non-context tag (vendor reserved data) + mIsInitialized = true; + mIsAtFirstToken = true; return CHIP_NO_ERROR; } @@ -72,36 +75,45 @@ class DeviceAttestationVendorReservedDeconstructor * * @returns CHIP_NO_ERROR on success * CHIP_ERROR_INCORRECT_STATE if PrepareToReadVendorReservedElements hasn't been called first + * CHIP_ERROR_UNEXPECTED_TLV_ELEMENT if we reach non-profile-specific tags or vendorId is zero * CHIP_END_OF_TLV if not further entries are present */ CHIP_ERROR GetNextVendorReservedElement(struct VendorReservedElement & element) { - CHIP_ERROR err; - VerifyOrReturnError(mIsInitialized, CHIP_ERROR_INCORRECT_STATE); - while ((err = mTlvReader.Next()) == CHIP_NO_ERROR) + if (mIsAtFirstToken) { - uint64_t tag = mTlvReader.GetTag(); - if (!TLV::IsProfileTag(tag)) - { - continue; - } - // tag is profile tag - element.vendorId = TLV::VendorIdFromTag(tag); - element.profileNum = TLV::ProfileNumFromTag(tag); - element.tagNum = TLV::TagNumFromTag(tag); + // Already had a Next() done for us by PrepareToReadVendorReservedElements + // so we don't Next() since we should be pointing at a vendor-reserved. + mIsAtFirstToken = false; + } + else + { + ReturnErrorOnFailure(mTlvReader.Next()); + } - return mTlvReader.GetByteView(element.vendorReservedData); + TLV::Tag tag = mTlvReader.GetTag(); + if (!TLV::IsProfileTag(tag)) + { + return CHIP_ERROR_UNEXPECTED_TLV_ELEMENT; } - return err; + // tag is profile tag + element.vendorId = TLV::VendorIdFromTag(tag); + element.profileNum = TLV::ProfileNumFromTag(tag); + element.tagNum = TLV::TagNumFromTag(tag); + + ReturnErrorOnFailure(mTlvReader.GetByteView(element.vendorReservedData)); + + return CHIP_NO_ERROR; } private: size_t mNumVendorReservedData; // number of VendorReserved entries (could be 0) ByteSpan mAttestationData; - bool mIsInitialized = false; + bool mIsInitialized = false; + bool mIsAtFirstToken = false; TLV::ContiguousBufferTLVReader mTlvReader; TLV::TLVType containerType = TLV::kTLVType_Structure; }; diff --git a/src/credentials/tests/TestDeviceAttestationConstruction.cpp b/src/credentials/tests/TestDeviceAttestationConstruction.cpp index f75a183ce316b1..a5030391cf683b 100644 --- a/src/credentials/tests/TestDeviceAttestationConstruction.cpp +++ b/src/credentials/tests/TestDeviceAttestationConstruction.cpp @@ -180,9 +180,8 @@ static void TestAttestationElements_Construction(nlTestSuite * inSuite, void * i static void TestAttestationElements_Deconstruction(nlTestSuite * inSuite, void * inContext) { - CHIP_ERROR err = CHIP_NO_ERROR; - - static constexpr uint8_t attestationElementsTestVector[] = { + // This is a test case with only the known TLV tags fields + constexpr uint8_t attestationElementsTestVectorOnlyKnownTags[] = { 0x15, 0x30, 0x01, 0x70, 0xd2, 0x84, 0x4b, 0xa2, 0x01, 0x26, 0x04, 0x46, 0x63, 0x73, 0x61, 0x63, 0x64, 0x30, 0xa0, 0x58, 0x1d, 0x15, 0x25, 0x01, 0x88, 0x99, 0x25, 0x02, 0xfe, 0xff, 0x25, 0x03, 0xd2, 0x04, 0x25, 0x04, 0x2e, 0x16, 0x24, 0x05, 0xaa, 0x25, 0x06, 0xde, 0xc0, 0x25, 0x07, 0x94, 0x26, 0x18, 0x58, 0x40, 0x96, 0x57, 0x2d, 0xd6, 0x3c, @@ -196,7 +195,25 @@ static void TestAttestationElements_Deconstruction(nlTestSuite * inSuite, void * 0xff, 0x3e, 0x00, 0x03, 0x00, 0x18, 0x76, 0x65, 0x6e, 0x64, 0x6f, 0x72, 0x5f, 0x72, 0x65, 0x73, 0x65, 0x72, 0x76, 0x65, 0x64, 0x33, 0x5f, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x18 }; - static constexpr uint8_t certificationDeclarationTestVector[] = { + + // The following case has an extra top-level context-specific tag (254) with value 0xDEADBEEF, in the right order, + // that should not cause a failure and merely be ignored. + constexpr uint8_t attestationElementsTestVectorWithOneUnknownTag[] = { + 0x15, 0x30, 0x01, 0x70, 0xd2, 0x84, 0x4b, 0xa2, 0x01, 0x26, 0x04, 0x46, 0x63, 0x73, 0x61, 0x63, 0x64, 0x30, 0xa0, + 0x58, 0x1d, 0x15, 0x25, 0x01, 0x88, 0x99, 0x25, 0x02, 0xfe, 0xff, 0x25, 0x03, 0xd2, 0x04, 0x25, 0x04, 0x2e, 0x16, + 0x24, 0x05, 0xaa, 0x25, 0x06, 0xde, 0xc0, 0x25, 0x07, 0x94, 0x26, 0x18, 0x58, 0x40, 0x96, 0x57, 0x2d, 0xd6, 0x3c, + 0x03, 0x64, 0x0b, 0x28, 0x67, 0x02, 0xbd, 0x6b, 0xba, 0x48, 0xac, 0x7c, 0x83, 0x54, 0x9b, 0x68, 0x73, 0x29, 0x47, + 0x48, 0xb9, 0x51, 0xd5, 0xab, 0x66, 0x62, 0x2e, 0x9d, 0x26, 0x10, 0x41, 0xf8, 0x0e, 0x97, 0x49, 0xfe, 0xff, 0x78, + 0x10, 0x02, 0x49, 0x67, 0xae, 0xdf, 0x41, 0x38, 0x36, 0x5b, 0x0a, 0x22, 0x57, 0x14, 0x9c, 0x9a, 0x12, 0x3e, 0x0d, + 0x30, 0xaa, 0x30, 0x02, 0x20, 0xe0, 0x42, 0x1b, 0x91, 0xc6, 0xfd, 0xcd, 0xb4, 0x0e, 0x2a, 0x4d, 0x2c, 0xf3, 0x1d, + 0xb2, 0xb4, 0xe1, 0x8b, 0x41, 0x1b, 0x1d, 0x3a, 0xd4, 0xd1, 0x2a, 0x9d, 0x90, 0xaa, 0x8e, 0x52, 0xfa, 0xe2, 0x26, + 0x03, 0xfd, 0xc6, 0x5b, 0x28, 0x26, 0xfe, 0xef, 0xbe, 0xad, 0xde, 0xd0, 0xf1, 0xff, 0x3e, 0x00, 0x01, 0x00, 0x17, + 0x73, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x5f, 0x76, 0x65, 0x6e, 0x64, 0x6f, 0x72, 0x5f, 0x72, 0x65, 0x73, 0x65, 0x72, + 0x76, 0x65, 0x64, 0x31, 0xd0, 0xf1, 0xff, 0x3e, 0x00, 0x03, 0x00, 0x18, 0x76, 0x65, 0x6e, 0x64, 0x6f, 0x72, 0x5f, + 0x72, 0x65, 0x73, 0x65, 0x72, 0x76, 0x65, 0x64, 0x33, 0x5f, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x18, + }; + + constexpr uint8_t certificationDeclarationTestVector[] = { 0xd2, 0x84, 0x4b, 0xa2, 0x01, 0x26, 0x04, 0x46, 0x63, 0x73, 0x61, 0x63, 0x64, 0x30, 0xa0, 0x58, 0x1d, 0x15, 0x25, 0x01, 0x88, 0x99, 0x25, 0x02, 0xfe, 0xff, 0x25, 0x03, 0xd2, 0x04, 0x25, 0x04, 0x2e, 0x16, 0x24, 0x05, 0xaa, 0x25, 0x06, 0xde, 0xc0, 0x25, 0x07, 0x94, 0x26, 0x18, 0x58, 0x40, 0x96, 0x57, 0x2d, 0xd6, 0x3c, 0x03, 0x64, 0x0b, 0x28, @@ -204,54 +221,60 @@ static void TestAttestationElements_Deconstruction(nlTestSuite * inSuite, void * 0xab, 0x66, 0x62, 0x2e, 0x9d, 0x26, 0x10, 0x41, 0xf8, 0x0e, 0x97, 0x49, 0xfe, 0xff, 0x78, 0x10, 0x02, 0x49, 0x67, 0xae, 0xdf, 0x41, 0x38, 0x36, 0x5b, 0x0a, 0x22, 0x57, 0x14, 0x9c, 0x9a, 0x12, 0x3e, 0x0d, 0x30, 0xaa }; - static constexpr uint8_t attestationNonceTestVector[] = { 0xe0, 0x42, 0x1b, 0x91, 0xc6, 0xfd, 0xcd, 0xb4, 0x0e, 0x2a, 0x4d, - 0x2c, 0xf3, 0x1d, 0xb2, 0xb4, 0xe1, 0x8b, 0x41, 0x1b, 0x1d, 0x3a, - 0xd4, 0xd1, 0x2a, 0x9d, 0x90, 0xaa, 0x8e, 0x52, 0xfa, 0xe2 }; - static constexpr uint32_t timestampTestVector = 677103357; - static constexpr uint8_t vendorReserved1TestVector[] = { 0x73, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x5f, 0x76, 0x65, 0x6e, 0x64, 0x6f, - 0x72, 0x5f, 0x72, 0x65, 0x73, 0x65, 0x72, 0x76, 0x65, 0x64, 0x31 }; - static constexpr uint8_t vendorReserved3TestVector[] = { - 0x76, 0x65, 0x6e, 0x64, 0x6f, 0x72, 0x5f, 0x72, 0x65, 0x73, 0x65, 0x72, - 0x76, 0x65, 0x64, 0x33, 0x5f, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65 - }; - static constexpr ByteSpan vendorReservedArrayTestVector[] = { ByteSpan(vendorReserved1TestVector), - ByteSpan(vendorReserved3TestVector) }; - static constexpr uint16_t vendorIdTestVector = 0xFFF1; - static constexpr uint16_t profileNumTestVector = 0x003E; - - ByteSpan certificationDeclarationDeconstructed; - ByteSpan attestationNonceDeconstructed; - uint32_t timestampDeconstructed; - ByteSpan firmwareInfoDeconstructed; - DeviceAttestationVendorReservedDeconstructor vendorReserved; - - err = DeconstructAttestationElements(ByteSpan(attestationElementsTestVector), certificationDeclarationDeconstructed, - attestationNonceDeconstructed, timestampDeconstructed, firmwareInfoDeconstructed, - vendorReserved); - NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); + constexpr uint8_t attestationNonceTestVector[] = { 0xe0, 0x42, 0x1b, 0x91, 0xc6, 0xfd, 0xcd, 0xb4, 0x0e, 0x2a, 0x4d, + 0x2c, 0xf3, 0x1d, 0xb2, 0xb4, 0xe1, 0x8b, 0x41, 0x1b, 0x1d, 0x3a, + 0xd4, 0xd1, 0x2a, 0x9d, 0x90, 0xaa, 0x8e, 0x52, 0xfa, 0xe2 }; + constexpr uint32_t timestampTestVector = 677103357; + constexpr uint8_t vendorReserved1TestVector[] = { 0x73, 0x61, 0x6d, 0x70, 0x6c, 0x65, 0x5f, 0x76, 0x65, 0x6e, 0x64, 0x6f, + 0x72, 0x5f, 0x72, 0x65, 0x73, 0x65, 0x72, 0x76, 0x65, 0x64, 0x31 }; + constexpr uint8_t vendorReserved3TestVector[] = { 0x76, 0x65, 0x6e, 0x64, 0x6f, 0x72, 0x5f, 0x72, 0x65, 0x73, 0x65, 0x72, + 0x76, 0x65, 0x64, 0x33, 0x5f, 0x65, 0x78, 0x61, 0x6d, 0x70, 0x6c, 0x65 }; + const ByteSpan vendorReservedArrayTestVector[] = { ByteSpan{ vendorReserved1TestVector }, + ByteSpan{ vendorReserved3TestVector } }; + constexpr uint16_t vendorIdTestVector = 0xFFF1; + constexpr uint16_t profileNumTestVector = 0x003E; + + const ByteSpan kTestCases[] = { ByteSpan{ attestationElementsTestVectorOnlyKnownTags }, + ByteSpan{ attestationElementsTestVectorWithOneUnknownTag } }; + + // Try all test cases and see they are all OK + for (const ByteSpan & attestationElementsTestCase : kTestCases) + { + CHIP_ERROR err = CHIP_NO_ERROR; + ByteSpan certificationDeclarationDeconstructed; + ByteSpan attestationNonceDeconstructed; + uint32_t timestampDeconstructed; + ByteSpan firmwareInfoDeconstructed; + DeviceAttestationVendorReservedDeconstructor vendorReserved; + + err = DeconstructAttestationElements(attestationElementsTestCase, certificationDeclarationDeconstructed, + attestationNonceDeconstructed, timestampDeconstructed, firmwareInfoDeconstructed, + vendorReserved); + NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR); - NL_TEST_ASSERT(inSuite, certificationDeclarationDeconstructed.data_equal(ByteSpan(certificationDeclarationTestVector))); - NL_TEST_ASSERT(inSuite, attestationNonceDeconstructed.data_equal(ByteSpan(attestationNonceTestVector))); - NL_TEST_ASSERT(inSuite, timestampTestVector == timestampDeconstructed); - NL_TEST_ASSERT(inSuite, firmwareInfoDeconstructed.empty()); - NL_TEST_ASSERT(inSuite, ArraySize(vendorReservedArrayTestVector) == vendorReserved.GetNumberOfElements()); - struct VendorReservedElement element; + NL_TEST_ASSERT(inSuite, certificationDeclarationDeconstructed.data_equal(ByteSpan(certificationDeclarationTestVector))); + NL_TEST_ASSERT(inSuite, attestationNonceDeconstructed.data_equal(ByteSpan(attestationNonceTestVector))); + NL_TEST_ASSERT(inSuite, timestampTestVector == timestampDeconstructed); + NL_TEST_ASSERT(inSuite, firmwareInfoDeconstructed.empty()); + NL_TEST_ASSERT(inSuite, ArraySize(vendorReservedArrayTestVector) == vendorReserved.GetNumberOfElements()); + struct VendorReservedElement element; - while (vendorReserved.GetNextVendorReservedElement(element) == CHIP_NO_ERROR) - { - NL_TEST_ASSERT(inSuite, vendorIdTestVector == element.vendorId); - NL_TEST_ASSERT(inSuite, profileNumTestVector == element.profileNum); - switch (element.tagNum) + while (vendorReserved.GetNextVendorReservedElement(element) == CHIP_NO_ERROR) { - case 1: - NL_TEST_ASSERT(inSuite, element.vendorReservedData.data_equal(vendorReservedArrayTestVector[0])); - break; - case 3: - NL_TEST_ASSERT(inSuite, element.vendorReservedData.data_equal(vendorReservedArrayTestVector[1])); - break; - default: - NL_TEST_ASSERT(inSuite, 0); - break; + NL_TEST_ASSERT(inSuite, vendorIdTestVector == element.vendorId); + NL_TEST_ASSERT(inSuite, profileNumTestVector == element.profileNum); + switch (element.tagNum) + { + case 1: + NL_TEST_ASSERT(inSuite, element.vendorReservedData.data_equal(vendorReservedArrayTestVector[0])); + break; + case 3: + NL_TEST_ASSERT(inSuite, element.vendorReservedData.data_equal(vendorReservedArrayTestVector[1])); + break; + default: + NL_TEST_ASSERT(inSuite, 0); + break; + } } } } @@ -424,7 +447,7 @@ static void TestAttestationElements_DeconstructionUnordered(nlTestSuite * inSuit err = DeconstructAttestationElements(ByteSpan(attestationElementsUnorderedTestVector), certificationDeclarationDeconstructed, attestationNonceDeconstructed, timestampDeconstructed, firmwareInfoDeconstructed, vendorReserved); - NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_IM_MALFORMED_COMMAND_DATA_ELEMENT); + NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_UNEXPECTED_TLV_ELEMENT); } /** diff --git a/src/lib/core/CHIPTLVTags.h b/src/lib/core/CHIPTLVTags.h index e563aa86986da6..1923dd526096b9 100644 --- a/src/lib/core/CHIPTLVTags.h +++ b/src/lib/core/CHIPTLVTags.h @@ -47,6 +47,8 @@ enum TLVCommonProfiles enum TLVTagFields { kProfileIdMask = 0xFFFFFFFF00000000ULL, + kProfileNumMask = 0x0000FFFF00000000ULL, + kVendorIdMask = 0xFFFF000000000000ULL, kProfileIdShift = 32, kVendorIdShift = 48, kProfileNumShift = 32, @@ -165,7 +167,7 @@ inline uint32_t ProfileIdFromTag(Tag tag) */ inline uint16_t ProfileNumFromTag(Tag tag) { - return static_cast((tag & kProfileIdMask) >> kProfileIdShift); + return static_cast((tag & kProfileNumMask) >> kProfileNumShift); } /** @@ -194,7 +196,7 @@ inline uint32_t TagNumFromTag(Tag tag) */ inline uint16_t VendorIdFromTag(Tag tag) { - return static_cast((tag & kProfileIdMask) >> kVendorIdShift); + return static_cast((tag & kVendorIdMask) >> kVendorIdShift); } /**