Skip to content

Commit

Permalink
Check for Valid Node Id, Fabric Id, and CATs Values When Processing C…
Browse files Browse the repository at this point in the history 
…HIP Certs.

  -- Node Id should be in a range [1, 0xFFFFFFEFFFFFFFFF]
  -- Fabric Id should be different from 0
  -- CAT should be different from 0
  • Loading branch information
@emargolis
emargolis committed Feb 24, 2022
1 parent 924c01c commit c06188b
Show file tree
Hide file tree
Showing 4 changed files with 54 additions and 19 deletions.
26 changes: 19 additions & 7 deletions src/credentials/CHIPCert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -618,7 +618,7 @@ CHIP_ERROR ChipDN::GetCertType(uint8_t & certType) const
else if (rdn[i].mAttrOID == kOID_AttributeType_ChipNodeId)
{
VerifyOrExit(lCertType == kCertType_NotSpecified, err = CHIP_ERROR_WRONG_CERT_TYPE);

VerifyOrReturnError(IsOperationalNodeId(rdn[i].mChipVal), CHIP_ERROR_WRONG_CERT_TYPE);
lCertType = kCertType_Node;
}
else if (rdn[i].mAttrOID == kOID_AttributeType_ChipFirmwareSigningId)
Expand All @@ -631,7 +631,7 @@ CHIP_ERROR ChipDN::GetCertType(uint8_t & certType) const
{
// Only one fabricId attribute is allowed per DN.
VerifyOrExit(!fabricIdPresent, err = CHIP_ERROR_WRONG_CERT_TYPE);

VerifyOrReturnError(IsValidFabricId(rdn[i].mChipVal), CHIP_ERROR_WRONG_CERT_TYPE);
fabricIdPresent = true;
}
}
Expand Down Expand Up @@ -677,24 +677,24 @@ CHIP_ERROR ChipDN::GetCertFabricId(uint64_t & fabricId) const
{
uint8_t rdnCount = RDNCount();

fabricId = UINT64_MAX;
fabricId = kUndefinedFabricId;

for (uint8_t i = 0; i < rdnCount; i++)
{
switch (rdn[i].mAttrOID)
{
case kOID_AttributeType_ChipFabricId:
// Ensure only one FabricID RDN present, since start value is UINT64_MAX, which is reserved and never seen.
VerifyOrReturnError(fabricId == UINT64_MAX, CHIP_ERROR_WRONG_CERT_TYPE);

// Ensure only one FabricID RDN present, since start value is kUndefinedFabricId, which is reserved and never seen.
VerifyOrReturnError(fabricId == kUndefinedFabricId, CHIP_ERROR_WRONG_CERT_TYPE);
VerifyOrReturnError(IsValidFabricId(rdn[i].mChipVal), CHIP_ERROR_WRONG_CERT_TYPE);
fabricId = rdn[i].mChipVal;
break;
default:
break;
}
}

VerifyOrReturnError(fabricId != UINT64_MAX, CHIP_ERROR_WRONG_CERT_TYPE);
VerifyOrReturnError(IsValidFabricId(fabricId), CHIP_ERROR_WRONG_CERT_TYPE);
return CHIP_NO_ERROR;
}

Expand Down Expand Up @@ -740,6 +740,14 @@ CHIP_ERROR ChipDN::DecodeFromTLV(TLVReader & reader)
uint64_t chipAttr;
VerifyOrReturnError(attrIsPrintableString == false, CHIP_ERROR_INVALID_TLV_TAG);
ReturnErrorOnFailure(reader.Get(chipAttr));
if (attrOID == chip::ASN1::kOID_AttributeType_ChipNodeId)
{
VerifyOrReturnError(IsOperationalNodeId(attrOID), CHIP_ERROR_INVALID_ARGUMENT);
}
else if (attrOID == chip::ASN1::kOID_AttributeType_ChipFabricId)
{
VerifyOrReturnError(IsValidFabricId(attrOID), CHIP_ERROR_INVALID_ARGUMENT);
}
ReturnErrorOnFailure(AddAttribute(attrOID, chipAttr));
}
// For 32-bit CHIP-defined DN attributes.
Expand All @@ -748,6 +756,10 @@ CHIP_ERROR ChipDN::DecodeFromTLV(TLVReader & reader)
uint32_t chipAttr;
VerifyOrReturnError(attrIsPrintableString == false, CHIP_ERROR_INVALID_TLV_TAG);
ReturnErrorOnFailure(reader.Get(chipAttr));
if (attrOID == chip::ASN1::kOID_AttributeType_ChipCASEAuthenticatedTag)
{
VerifyOrReturnError(chipAttr != kUndefinedCAT, CHIP_ERROR_INVALID_ARGUMENT);
}
ReturnErrorOnFailure(AddAttribute(attrOID, chipAttr));
}
// Otherwise the attribute is one of the supported X.509 attributes
Expand Down
20 changes: 15 additions & 5 deletions src/credentials/CHIPCertFromX509.cpp
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*
*
* Copyright (c) 2020-2021 Project CHIP Authors
* Copyright (c) 2020-2022 Project CHIP Authors
* Copyright (c) 2013-2017 Nest Labs, Inc.
* All rights reserved.
*
Expand Down Expand Up @@ -141,10 +141,6 @@ static CHIP_ERROR ConvertDistinguishedName(ASN1Reader & reader, TLVWriter & writ
err = ParseChipAttribute(reader, chipAttr);
SuccessOrExit(err);

// Write the CHIP attribute value into the TLV.
err = writer.Put(ContextTag(tlvTagNum), chipAttr);
SuccessOrExit(err);

// Certificates use a combination of OIDs for Issuer and Subject.
// NOC: Issuer = kOID_AttributeType_ChipRootId or kOID_AttributeType_ChipICAId
// Subject = kOID_AttributeType_ChipNodeId
Expand All @@ -160,12 +156,26 @@ static CHIP_ERROR ConvertDistinguishedName(ASN1Reader & reader, TLVWriter & writ
attrOID == chip::ASN1::kOID_AttributeType_ChipICAId ||
attrOID == chip::ASN1::kOID_AttributeType_ChipRootId)
{
if (attrOID == chip::ASN1::kOID_AttributeType_ChipNodeId)
{
VerifyOrReturnError(IsOperationalNodeId(chipAttr), CHIP_ERROR_INVALID_ARGUMENT);
}
subjectOrIssuer = chipAttr;
}
else if (attrOID == chip::ASN1::kOID_AttributeType_ChipFabricId)
{
VerifyOrReturnError(IsValidFabricId(chipAttr), CHIP_ERROR_INVALID_ARGUMENT);
fabric.SetValue(chipAttr);
}
else if (attrOID == chip::ASN1::kOID_AttributeType_ChipCASEAuthenticatedTag)
{
VerifyOrReturnError(CanCastTo<uint32_t>(chipAttr), CHIP_ERROR_INVALID_ARGUMENT);
VerifyOrReturnError(chipAttr != kUndefinedCAT, CHIP_ERROR_INVALID_ARGUMENT);
}

// Write the CHIP attribute value into the TLV.
err = writer.Put(ContextTag(tlvTagNum), chipAttr);
SuccessOrExit(err);
}

//
Expand Down
5 changes: 5 additions & 0 deletions src/lib/core/PeerId.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ constexpr CompressedFabricId kUndefinedCompressedFabricId = 0ULL;

constexpr FabricId kUndefinedFabricId = 0ULL;

constexpr bool IsValidFabricId(FabricId aFabricId)
{
return aFabricId != kUndefinedFabricId;
}

/// A peer is identified by a node id within a compressed fabric ID
class PeerId
{
Expand Down
22 changes: 15 additions & 7 deletions src/tools/chip-cert/Cmd_GenCert.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,19 +73,21 @@ const char * const gCmdOptionHelp =
"\n"
" -i, --subject-chip-id <hex-digits>\n"
"\n"
" Subject DN CHIP Id attribute (in hex). For Node Certificate it is CHIP Node Id attribute.\n"
" Subject DN CHIP Id attribute in hexadecimal format with upto 8 octets with or without '0x' prefix.\n"
" - for Root certificate it is ChipRootId\n"
" - for intermediate CA certificate it is ChipICAId\n"
" - for Node certificate it is ChipNodeId\n"
" - for Node certificate it is ChipNodeId. The value should be in a range [1, 0xFFFFFFEFFFFFFFFF]\n"
" - for Firmware Signing certificate it is ChipFirmwareSigningId\n"
"\n"
" -f, --subject-fab-id <hex-digits>\n"
"\n"
" Subject DN Fabric Id attribute (in hex).\n"
" Subject DN Fabric Id attribute in hexadecimal format with upto 8 octets with or without '0x' prefix.\n"
" The value should be different from 0.\n"
"\n"
" -a, --subject-cat <hex-digits>\n"
"\n"
" Subject DN CHIP CASE Authentication Tag (in hex).\n"
" Subject DN CHIP CASE Authentication Tag in hexadecimal format with upto 4 octets with or without '0x' prefix.\n"
" The value should be different from 0.\n"
"\n"
" -c, --subject-cn-u <string>\n"
"\n"
Expand Down Expand Up @@ -236,6 +238,11 @@ bool HandleOption(const char * progName, OptionSet * optSet, int id, const char
switch (gCertType)
{
case kCertType_Node:
if (!chip::IsOperationalNodeId(chip64bitAttr))
{
PrintArgError("%s: Invalid value specified for chip node-id attribute: %s\n", progName, arg);
return false;
}
attrOID = kOID_AttributeType_ChipNodeId;
break;
case kCertType_FirmwareSigning:
Expand All @@ -261,9 +268,10 @@ bool HandleOption(const char * progName, OptionSet * optSet, int id, const char
break;

case 'a':
if (!ParseInt(arg, chip32bitAttr, 16))
if (!ParseInt(arg, chip32bitAttr, 16) || chip32bitAttr == chip::kUndefinedCAT)
{
PrintArgError("%s: Invalid value specified for the subject authentication tag attribute: %s\n", progName, arg);
PrintArgError("%s: Invalid value specified for the subject CASE Authenticated Tag (CAT) attribute: %s\n", progName,
arg);
return false;
}
attrOID = kOID_AttributeType_ChipCASEAuthenticatedTag;
Expand All @@ -284,7 +292,7 @@ bool HandleOption(const char * progName, OptionSet * optSet, int id, const char
}
break;
case 'f':
if (!ParseInt(arg, chip64bitAttr, 16))
if (!ParseInt(arg, chip64bitAttr, 16) || !chip::IsValidFabricId(chip64bitAttr))
{
PrintArgError("%s: Invalid value specified for subject fabric id attribute: %s\n", progName, arg);
return false;
Expand Down

0 comments on commit c06188b

Please sign in to comment.