Updated Fabric ID Validation in Fabric Table (#15920)

-- Updated logic that checks optional fabric id attribute in ICAC and RCAC to match
     fabric id attribute in NOC.
  -- According to spec there is no need to require fabric id in ICAC if it is present in RCAC.
  -- NOTE: validity check for NodeId, FabricId and other DN attributes is performed when
     loading certificates for validation (LoadCert()). So there is no need to check validity
     again in FabricInfo::VerifyCredentials().

  -- Changed return value of ExtractNodeIdFabricIdFromOpCert() and ExtractFabricIdFromCert()
     to CHIP_ERROR_NOT_FOUND when requested element is not found in the certificate.

src/credentials/CHIPCert.cpp

@@ -1013,7 +1013,7 @@ CHIP_ERROR ExtractNodeIdFabricIdFromOpCert(const ChipCertificateData & opcert, N
  }
  if (!foundNodeId || !foundFabricId)
  {
- return CHIP_ERROR_INVALID_ARGUMENT;
+ return CHIP_ERROR_NOT_FOUND;
  }
 
  *outNodeId = nodeId;
@@ -1054,7 +1054,7 @@ CHIP_ERROR ExtractFabricIdFromCert(const ChipCertificateData & cert, FabricId *
  }
  }
 
- return CHIP_ERROR_INVALID_ARGUMENT;
+ return CHIP_ERROR_NOT_FOUND;
 }
 
 CHIP_ERROR ExtractCATsFromOpCert(const ByteSpan & opcert, CATValues & cats)

src/credentials/CHIPCert.h

@@ -781,7 +781,7 @@ CHIP_ERROR ConvertECDSASignatureDERToRaw(ASN1::ASN1Reader & reader, chip::TLV::T
  * These certificates may not contain a NodeID, so ExtractNodeIdFabricIdFromOpCert()
  * cannot be used for such certificates.
  *
- * @return CHIP_ERROR_INVALID_ARGUMENT if the passed-in cert does not have RDN
+ * @return CHIP_ERROR_NOT_FOUND if the passed-in cert does not have RDN
  * corresponding to FabricID.
  */
 CHIP_ERROR ExtractFabricIdFromCert(const ChipCertificateData & cert, FabricId * fabricId);
@@ -790,7 +790,7 @@ CHIP_ERROR ExtractFabricIdFromCert(const ChipCertificateData & cert, FabricId *
  * Extract Node ID and Fabric ID from an operational certificate that has already been
  * parsed.
  *
- * @return CHIP_ERROR_INVALID_ARGUMENT if the passed-in cert does not have at
+ * @return CHIP_ERROR_NOT_FOUND if the passed-in cert does not have at
  * least one NodeId RDN and one FabricId RDN in the Subject DN. No other
  * validation (e.g. checkign that there is exactly one RDN of each type) is
  * performed.

src/credentials/FabricTable.cpp

@@ -312,25 +312,32 @@ CHIP_ERROR FabricInfo::VerifyCredentials(const ByteSpan & noc, const ByteSpan &
  NodeId nodeId;
  ReturnErrorOnFailure(ExtractNodeIdFabricIdFromOpCert(certificates.GetLastCert()[0], &nodeId, &fabricId));
 
+ CHIP_ERROR err;
  FabricId icacFabricId = kUndefinedFabricId;
  if (!icac.empty())
  {
- if (ExtractFabricIdFromCert(certificates.GetCertSet()[1], &icacFabricId) == CHIP_NO_ERROR &&
-  icacFabricId != kUndefinedFabricId)
+ err = ExtractFabricIdFromCert(certificates.GetCertSet()[1], &icacFabricId);
+ if (err == CHIP_NO_ERROR)
  {
  ReturnErrorCodeIf(icacFabricId != fabricId, CHIP_ERROR_FABRIC_MISMATCH_ON_ICA);
  }
+ // FabricId is optional field in ICAC and "not found" code is not treated as error.
+ else if (err != CHIP_ERROR_NOT_FOUND)
+ {
+ return err;
+ }
  }
 
  FabricId rcacFabricId = kUndefinedFabricId;
- if (ExtractFabricIdFromCert(certificates.GetCertSet()[0], &rcacFabricId) == CHIP_NO_ERROR && rcacFabricId != kUndefinedFabricId)
+ err = ExtractFabricIdFromCert(certificates.GetCertSet()[0], &rcacFabricId);
+ if (err == CHIP_NO_ERROR)
  {
  ReturnErrorCodeIf(rcacFabricId != fabricId, CHIP_ERROR_WRONG_CERT_DN);
-  if (!icac.empty())
-  {
-  // If FabricId attribute is present in RCAC then it SHOULD be present in ICAC as well.
-  ReturnErrorCodeIf(icacFabricId == kUndefinedFabricId, CHIP_ERROR_WRONG_CERT_DN);
- }
+ }
+ // FabricId is optional field in RCAC and "not found" code is not treated as error.
+ else if (err != CHIP_ERROR_NOT_FOUND)
+ {
+ return err;
  }
 
  ReturnErrorOnFailure(GeneratePeerId(fabricId, nodeId, &nocPeerId));

src/credentials/tests/TestChipCert.cpp

@@ -1196,7 +1196,7 @@ static void TestChipCert_ExtractNodeIdFabricId(nlTestSuite * inSuite, void * inC
 
  FabricId fabricId;
  err = ExtractFabricIdFromCert(cert, &fabricId);
- NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
+ NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_NOT_FOUND);
  }
 
  // Test extraction from the parsed form of ICA Cert that doesn't have FabricId.
@@ -1209,7 +1209,7 @@ static void TestChipCert_ExtractNodeIdFabricId(nlTestSuite * inSuite, void * inC
 
  FabricId fabricId;
  err = ExtractFabricIdFromCert(certSet.GetCertSet()[0], &fabricId);
- NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_INVALID_ARGUMENT);
+ NL_TEST_ASSERT(inSuite, err == CHIP_ERROR_NOT_FOUND);
  certSet.Release();
  }
 }