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();
     }
 }