Skip to content

Commit

Permalink
Make device attestation data parser future proof
Browse files Browse the repository at this point in the history 
DeviceAttestationDeconstructor was too stringent in expecting no
unknown tags at all, preventing future tags from breaking backward
compatibility when they are benign if ignored.

- Fix DeviceAttestationDeconstructor to skip unexpected tags
- Simplify logic for ensuring in-order tags
- Fix a corner case of the vendor-specific data iterator not
  caught before
- Clean-up extraction of profileNum in CHIP tags

Fixes project-chip#11247
  • Loading branch information
@tcarmelveilleux
tcarmelveilleux committed Nov 17, 2021
1 parent e338b6e commit 553870d
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 80 deletions.
46 changes: 26 additions & 20 deletions src/credentials/DeviceAttestationConstructor.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,9 +67,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;

Expand All @@ -85,47 +85,53 @@ 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;
VerifyOrReturnError(allTagsNeededPresent, CHIP_ERROR_MISSING_TLV_ELEMENT);

size_t count = 0;
ReturnErrorOnFailure(CountVendorReservedElementsInDA(attestationElements, count));
Expand Down
44 changes: 28 additions & 16 deletions src/credentials/DeviceAttestationVendorReserved.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
mIsAtFirstToken = true;
return CHIP_NO_ERROR;
}

Expand All @@ -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 mIsAtFirstToken = false;
TLV::ContiguousBufferTLVReader mTlvReader;
TLV::TLVType containerType = TLV::kTLVType_Structure;
};
Expand Down
108 changes: 66 additions & 42 deletions src/credentials/tests/TestDeviceAttestationConstruction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -196,62 +195,87 @@ 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,
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
};
static constexpr uint8_t attestationNonceTestVector[] = { 0xe0, 0x42, 0x1b, 0x91, 0xc6, 0xfd, 0xcd, 0xb4, 0x0e, 0x2a, 0x4d,
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,
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 };
static constexpr uint8_t vendorReserved3TestVector[] = {
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;
const ByteSpan vendorReservedArrayTestVector[] = { ByteSpan{vendorReserved1TestVector},
ByteSpan{vendorReserved3TestVector} };
constexpr uint16_t vendorIdTestVector = 0xFFF1;
constexpr uint16_t profileNumTestVector = 0x003E;

ByteSpan certificationDeclarationDeconstructed;
ByteSpan attestationNonceDeconstructed;
uint32_t timestampDeconstructed;
ByteSpan firmwareInfoDeconstructed;
DeviceAttestationVendorReservedDeconstructor vendorReserved;
const ByteSpan kTestCases[] = { ByteSpan{attestationElementsTestVectorOnlyKnownTags}, ByteSpan{attestationElementsTestVectorWithOneUnknownTag} };

err = DeconstructAttestationElements(ByteSpan(attestationElementsTestVector), certificationDeclarationDeconstructed,
attestationNonceDeconstructed, timestampDeconstructed, firmwareInfoDeconstructed,
vendorReserved);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
// 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;
}
}
}
}
Expand Down Expand Up @@ -424,7 +448,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);
}

/**
Expand Down
6 changes: 4 additions & 2 deletions src/lib/core/CHIPTLVTags.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ enum TLVCommonProfiles
enum TLVTagFields
{
kProfileIdMask = 0xFFFFFFFF00000000ULL,
kProfileNumMask = 0x0000FFFF00000000ULL,
kVendorIdMask = 0xFFFF000000000000ULL,
kProfileIdShift = 32,
kVendorIdShift = 48,
kProfileNumShift = 32,
Expand Down Expand Up @@ -165,7 +167,7 @@ inline uint32_t ProfileIdFromTag(Tag tag)
*/
inline uint16_t ProfileNumFromTag(Tag tag)
{
return static_cast<uint16_t>((tag & kProfileIdMask) >> kProfileIdShift);
return static_cast<uint16_t>((tag & kProfileNumMask) >> kProfileIdShift);
}

/**
Expand Down Expand Up @@ -194,7 +196,7 @@ inline uint32_t TagNumFromTag(Tag tag)
*/
inline uint16_t VendorIdFromTag(Tag tag)
{
return static_cast<uint16_t>((tag & kProfileIdMask) >> kVendorIdShift);
return static_cast<uint16_t>((tag & kVendorIdMask) >> kVendorIdShift);
}

/**
Expand Down

0 comments on commit 553870d

Please sign in to comment.