Check for Valid Node Id, Fabric Id, and CATs Values When Processing C…

…HIP Certs. (project-chip#15475)

-- Node Id should be in a range [1, 0xFFFFFFEFFFFFFFFF]
  -- Fabric Id should be different from 0
  -- CAT should be different from 0

src/app/clusters/operational-credentials-server/operational-credentials-server.cpp

@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2021 Project CHIP Authors
+ *    Copyright (c) 2021-2022 Project CHIP Authors
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
  *    you may not use this file except in compliance with the License.
@@ -500,7 +500,7 @@ OperationalCertStatus ConvertToNOCResponseStatus(CHIP_ERROR err)
              err == CHIP_ERROR_CERT_PATH_TOO_LONG || err == CHIP_ERROR_CERT_USAGE_NOT_ALLOWED || err == CHIP_ERROR_CERT_EXPIRED ||
              err == CHIP_ERROR_CERT_NOT_VALID_YET || err == CHIP_ERROR_UNSUPPORTED_CERT_FORMAT ||
              err == CHIP_ERROR_UNSUPPORTED_ELLIPTIC_CURVE || err == CHIP_ERROR_CERT_LOAD_FAILED ||
-             err == CHIP_ERROR_CERT_NOT_TRUSTED || err == CHIP_ERROR_WRONG_CERT_SUBJECT)
+             err == CHIP_ERROR_CERT_NOT_TRUSTED || err == CHIP_ERROR_WRONG_CERT_DN)
     {
         return OperationalCertStatus::kInvalidNOC;
     }

src/credentials/CHIPCert.cpp

@@ -605,40 +605,40 @@ CHIP_ERROR ChipDN::GetCertType(uint8_t & certType) const
     {
         if (rdn[i].mAttrOID == kOID_AttributeType_ChipRootId)
         {
-            VerifyOrExit(lCertType == kCertType_NotSpecified, err = CHIP_ERROR_WRONG_CERT_TYPE);
+            VerifyOrExit(lCertType == kCertType_NotSpecified, err = CHIP_ERROR_WRONG_CERT_DN);
 
             lCertType = kCertType_Root;
         }
         else if (rdn[i].mAttrOID == kOID_AttributeType_ChipICAId)
         {
-            VerifyOrExit(lCertType == kCertType_NotSpecified, err = CHIP_ERROR_WRONG_CERT_TYPE);
+            VerifyOrExit(lCertType == kCertType_NotSpecified, err = CHIP_ERROR_WRONG_CERT_DN);
 
             lCertType = kCertType_ICA;
         }
         else if (rdn[i].mAttrOID == kOID_AttributeType_ChipNodeId)
         {
-            VerifyOrExit(lCertType == kCertType_NotSpecified, err = CHIP_ERROR_WRONG_CERT_TYPE);
-
+            VerifyOrExit(lCertType == kCertType_NotSpecified, err = CHIP_ERROR_WRONG_CERT_DN);
+            VerifyOrReturnError(IsOperationalNodeId(rdn[i].mChipVal), CHIP_ERROR_WRONG_CERT_DN);
             lCertType = kCertType_Node;
         }
         else if (rdn[i].mAttrOID == kOID_AttributeType_ChipFirmwareSigningId)
         {
-            VerifyOrExit(lCertType == kCertType_NotSpecified, err = CHIP_ERROR_WRONG_CERT_TYPE);
+            VerifyOrExit(lCertType == kCertType_NotSpecified, err = CHIP_ERROR_WRONG_CERT_DN);
 
             lCertType = kCertType_FirmwareSigning;
         }
         else if (rdn[i].mAttrOID == kOID_AttributeType_ChipFabricId)
         {
             // Only one fabricId attribute is allowed per DN.
-            VerifyOrExit(!fabricIdPresent, err = CHIP_ERROR_WRONG_CERT_TYPE);
-
+            VerifyOrExit(!fabricIdPresent, err = CHIP_ERROR_WRONG_CERT_DN);
+            VerifyOrReturnError(IsValidFabricId(rdn[i].mChipVal), CHIP_ERROR_WRONG_CERT_DN);
             fabricIdPresent = true;
         }
     }
 
     if (lCertType == kCertType_Node)
     {
-        VerifyOrExit(fabricIdPresent, err = CHIP_ERROR_WRONG_CERT_TYPE);
+        VerifyOrExit(fabricIdPresent, err = CHIP_ERROR_WRONG_CERT_DN);
     }
 
     certType = lCertType;
@@ -661,7 +661,7 @@ CHIP_ERROR ChipDN::GetCertChipId(uint64_t & chipId) const
         case kOID_AttributeType_ChipICAId:
         case kOID_AttributeType_ChipNodeId:
         case kOID_AttributeType_ChipFirmwareSigningId:
-            VerifyOrReturnError(chipId == 0, CHIP_ERROR_WRONG_CERT_TYPE);
+            VerifyOrReturnError(chipId == 0, CHIP_ERROR_WRONG_CERT_DN);
 
             chipId = rdn[i].mChipVal;
             break;
@@ -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_DN);
+            VerifyOrReturnError(IsValidFabricId(rdn[i].mChipVal), CHIP_ERROR_WRONG_CERT_DN);
             fabricId = rdn[i].mChipVal;
             break;
         default:
             break;
         }
     }
 
-    VerifyOrReturnError(fabricId != UINT64_MAX, CHIP_ERROR_WRONG_CERT_TYPE);
+    VerifyOrReturnError(IsValidFabricId(fabricId), CHIP_ERROR_WRONG_CERT_DN);
     return CHIP_NO_ERROR;
 }
 
@@ -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.
@@ -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(IsValidCASEAuthTag(chipAttr), CHIP_ERROR_INVALID_ARGUMENT);
+            }
             ReturnErrorOnFailure(AddAttribute(attrOID, chipAttr));
         }
         // Otherwise the attribute is one of the supported X.509 attributes

src/credentials/CHIPCertFromX509.cpp

@@ -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.
  *
@@ -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
@@ -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_WRONG_CERT_DN);
+                            }
                             subjectOrIssuer = chipAttr;
                         }
                         else if (attrOID == chip::ASN1::kOID_AttributeType_ChipFabricId)
                         {
+                            VerifyOrReturnError(IsValidFabricId(chipAttr), CHIP_ERROR_WRONG_CERT_DN);
                             fabric.SetValue(chipAttr);
                         }
+                        else if (attrOID == chip::ASN1::kOID_AttributeType_ChipCASEAuthenticatedTag)
+                        {
+                            VerifyOrReturnError(CanCastTo<CASEAuthTag>(chipAttr), CHIP_ERROR_WRONG_CERT_DN);
+                            VerifyOrReturnError(IsValidCASEAuthTag(static_cast<CASEAuthTag>(chipAttr)), CHIP_ERROR_WRONG_CERT_DN);
+                        }
+
+                        // Write the CHIP attribute value into the TLV.
+                        err = writer.Put(ContextTag(tlvTagNum), chipAttr);
+                        SuccessOrExit(err);
                     }
 
                     //

src/credentials/tests/TestChipCert.cpp

@@ -892,7 +892,7 @@ static void TestChipCert_GenerateNOCRoot(nlTestSuite * inSuite, void * inContext
 
     NL_TEST_ASSERT(inSuite,
                    NewNodeOperationalX509Cert(noc_params, noc_keypair.Pubkey(), keypair, signed_cert_span1) ==
-                       CHIP_ERROR_WRONG_CERT_TYPE);
+                       CHIP_ERROR_WRONG_CERT_DN);
 
     // Test error case: issuer cert DN type is Node certificate
     noc_params.SubjectDN.Clear();

src/lib/core/CASEAuthTag.h

@@ -70,4 +70,9 @@ constexpr CASEAuthTag CASEAuthTagFromNodeId(NodeId aNodeId)
     return aNodeId & kMaskCASEAuthTag;
 }
 
+constexpr CASEAuthTag IsValidCASEAuthTag(CASEAuthTag aCAT)
+{
+    return (aCAT & kTagVersionMask) > 0;
+}
+
 } // namespace chip

src/lib/core/CHIPError.cpp

@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2020 Project CHIP Authors
+ *    Copyright (c) 2020-2022 Project CHIP Authors
  *    Copyright (c) 2019 Google LLC.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -350,8 +350,8 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err)
     case CHIP_ERROR_INVALID_ACCESS_TOKEN.AsInteger():
         desc = "Invalid access token";
         break;
-    case CHIP_ERROR_WRONG_CERT_SUBJECT.AsInteger():
-        desc = "Wrong certificate subject";
+    case CHIP_ERROR_WRONG_CERT_DN.AsInteger():
+        desc = "Wrong certificate distinguished name";
         break;
     case CHIP_ERROR_INVALID_PROVISIONING_BUNDLE.AsInteger():
         desc = "Invalid provisioning bundle";

src/lib/core/CHIPError.h

@@ -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.
  *
  *    Licensed under the Apache License, Version 2.0 (the "License");
@@ -1232,16 +1232,13 @@ using CHIP_ERROR = ::chip::ChipError;
 #define CHIP_ERROR_INVALID_ACCESS_TOKEN                        CHIP_CORE_ERROR(0x58)
 
 /**
- *  @def CHIP_ERROR_WRONG_CERT_SUBJECT
+ *  @def CHIP_ERROR_WRONG_CERT_DN
  *
  *  @brief
- *    A certificate subject is wrong.
+ *    A certificate subject/issuer distinguished name is wrong.
  *
  */
-#define CHIP_ERROR_WRONG_CERT_SUBJECT                          CHIP_CORE_ERROR(0x59)
-
-// deprecated alias
-#define CHIP_ERROR_WRONG_CERTIFICATE_SUBJECT CHIP_ERROR_WRONG_CERT_SUBJECT
+#define CHIP_ERROR_WRONG_CERT_DN                               CHIP_CORE_ERROR(0x59)
 
 /**
  *  @def CHIP_ERROR_INVALID_PROVISIONING_BUNDLE

src/lib/core/PeerId.h

@@ -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
 {

src/lib/core/tests/TestCHIPErrorStr.cpp

@@ -1,6 +1,6 @@
 /*
  *
- *    Copyright (c) 2020 Project CHIP Authors
+ *    Copyright (c) 2020-2022 Project CHIP Authors
  *    Copyright (c) 2016-2017 Nest Labs, Inc.
  *    All rights reserved.
  *
@@ -137,8 +137,7 @@ static const CHIP_ERROR kTestElements[] =
     CHIP_ERROR_CERT_LOAD_FAILED,
     CHIP_ERROR_CERT_NOT_TRUSTED,
     CHIP_ERROR_INVALID_ACCESS_TOKEN,
-    CHIP_ERROR_WRONG_CERT_SUBJECT,
-    CHIP_ERROR_WRONG_CERTIFICATE_SUBJECT,
+    CHIP_ERROR_WRONG_CERT_DN,
     CHIP_ERROR_INVALID_PROVISIONING_BUNDLE,
     CHIP_ERROR_PROVISIONING_BUNDLE_DECRYPTION_ERROR,
     CHIP_ERROR_WRONG_NODE_ID,

src/tools/chip-cert/Cmd_GenCert.cpp

@@ -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 version subfield (lower 16 bits) should be different from 0.\n"
     "\n"
     "   -c, --subject-cn-u <string>\n"
     "\n"
@@ -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:
@@ -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) || chip::IsValidCASEAuthTag(chip32bitAttr))
         {
-            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;
@@ -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;