Include non-fabric-associated pending roots in the RootCertificates a…

…ttribute. (#21016)

* Include non-fabric-associated pending roots in the RootCertificates attribute.

Fixes #20791

Also moves a function that was supposed to be private from public to private.

* Add unit test.

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

@@ -206,6 +206,24 @@ CHIP_ERROR OperationalCredentialsAttrAccess::ReadRootCertificates(EndpointId end
             ReturnErrorOnFailure(encoder.Encode(ByteSpan{ cert }));
         }
 
+        {
+            uint8_t certBuf[kMaxCHIPCertLength];
+            MutableByteSpan cert{ certBuf };
+            CHIP_ERROR err = fabricTable.FetchPendingNonFabricAssociatedRootCert(cert);
+            if (err == CHIP_ERROR_NOT_FOUND)
+            {
+                // No pending root cert, do nothing
+            }
+            else if (err != CHIP_NO_ERROR)
+            {
+                return err;
+            }
+            else
+            {
+                ReturnErrorOnFailure(encoder.Encode(ByteSpan{ cert }));
+            }
+        }
+
         return CHIP_NO_ERROR;
     });
 }

src/credentials/FabricTable.cpp

@@ -554,6 +554,24 @@ CHIP_ERROR FabricTable::FetchRootCert(FabricIndex fabricIndex, MutableByteSpan &
     return mOpCertStore->GetCertificate(fabricIndex, CertChainElement::kRcac, outCert);
 }
 
+CHIP_ERROR FabricTable::FetchPendingNonFabricAssociatedRootCert(MutableByteSpan & outCert) const
+{
+    VerifyOrReturnError(mOpCertStore != nullptr, CHIP_ERROR_INCORRECT_STATE);
+    if (!mStateFlags.Has(StateFlags::kIsTrustedRootPending))
+    {
+        return CHIP_ERROR_NOT_FOUND;
+    }
+
+    if (mStateFlags.Has(StateFlags::kIsAddPending))
+    {
+        // The root certificate is already associated with a pending fabric, so
+        // does not exist for purposes of this API.
+        return CHIP_ERROR_NOT_FOUND;
+    }
+
+    return FetchRootCert(mFabricIndexWithPendingState, outCert);
+}
+
 CHIP_ERROR FabricTable::FetchICACert(FabricIndex fabricIndex, MutableByteSpan & outCert) const
 {
     VerifyOrReturnError(mOpCertStore != nullptr, CHIP_ERROR_INCORRECT_STATE);

src/credentials/FabricTable.h

@@ -514,19 +514,6 @@ class DLL_EXPORT FabricTable
     ConstFabricIterator begin() const { return cbegin(); }
     ConstFabricIterator end() const { return cend(); }
 
-    /**
-     * @brief Get a mutable FabricInfo entry from the table by FabricIndex.
-     *
-     * NOTE: This is private for use within the FabricTable itself. All mutations have to go through the
-     *       FabricTable public methods that take a FabricIndex so that there are no mutations about which
-     *       the FabricTable is unaware, since this would break expectations regarding shadow/pending
-     *       entries used during fail-safe.
-     *
-     * @param fabricIndex - fabric index for which to get a mutable FabricInfo entry
-     * @return the FabricInfo entry for the fabricIndex if found, or nullptr if not found
-     */
-    FabricInfo * GetMutableFabricByIndex(FabricIndex fabricIndex);
-
     /**
      * @brief Get the RCAC (operational root certificate) associated with a fabric.
      *
@@ -541,6 +528,22 @@ class DLL_EXPORT FabricTable
      */
     CHIP_ERROR FetchRootCert(FabricIndex fabricIndex, MutableByteSpan & outCert) const;
 
+    /**
+     * @brief Get the pending root certificate which is not associated with a fabric, if there is one.
+     *
+     * If a root is pending from `AddNewPendingTrustedRootCert`, and there is no
+     * fabric associated with the corresponding fabric index yet
+     * (i.e. `AddNewPendingFabric*` has not been called yet) it is returned.
+     *
+     * @param outCert - MutableByteSpan to receive the certificate. Resized to actual size.
+     * @retval CHIP_NO_ERROR on success
+     * @retval CHIP_ERROR_BUFFER_TOO_SMALL if `outCert` is too small.
+     * @retval CHIP_ERROR_NOT_FOUND if there is no pending root certificate
+     *                              that's not yet associated with a fabric.
+     * @retval other CHIP_ERROR values on invalid arguments or internal errors.
+     */
+    CHIP_ERROR FetchPendingNonFabricAssociatedRootCert(MutableByteSpan & outCert) const;
+
     /**
      * @brief Get the ICAC (operational intermediate certificate) associated with a fabric.
      *
@@ -948,6 +951,19 @@ class DLL_EXPORT FabricTable
         bool isAddition         = false;
     };
 
+    /**
+     * @brief Get a mutable FabricInfo entry from the table by FabricIndex.
+     *
+     * NOTE: This is private for use within the FabricTable itself. All mutations have to go through the
+     *       FabricTable public methods that take a FabricIndex so that there are no mutations about which
+     *       the FabricTable is unaware, since this would break expectations regarding shadow/pending
+     *       entries used during fail-safe.
+     *
+     * @param fabricIndex - fabric index for which to get a mutable FabricInfo entry
+     * @return the FabricInfo entry for the fabricIndex if found, or nullptr if not found
+     */
+    FabricInfo * GetMutableFabricByIndex(FabricIndex fabricIndex);
+
     // Load a FabricInfo metatada item from storage for a given new fabric index. Returns internal error on failure.
     CHIP_ERROR LoadFromStorage(FabricInfo * fabric, FabricIndex newFabricIndex);
 

src/credentials/tests/TestFabricTable.cpp

@@ -478,7 +478,21 @@ void TestBasicAddNocUpdateNocFlow(nlTestSuite * inSuite, void * inContext)
             NL_TEST_ASSERT(inSuite, saw1 == false);
         }
 
+        uint8_t rcacBuf[Credentials::kMaxCHIPCertLength];
+        {
+            // No pending root cert yet.
+            MutableByteSpan fetchedSpan{ rcacBuf };
+            NL_TEST_ASSERT(inSuite, fabricTable.FetchPendingNonFabricAssociatedRootCert(fetchedSpan) == CHIP_ERROR_NOT_FOUND);
+        }
+
         NL_TEST_ASSERT_SUCCESS(inSuite, fabricTable.AddNewPendingTrustedRootCert(rcac));
+        {
+            // Now have a pending root cert.
+            MutableByteSpan fetchedSpan{ rcacBuf };
+            NL_TEST_ASSERT_SUCCESS(inSuite, fabricTable.FetchPendingNonFabricAssociatedRootCert(fetchedSpan));
+            NL_TEST_ASSERT(inSuite, fetchedSpan.data_equal(rcac));
+        }
+
         FabricIndex newFabricIndex = kUndefinedFabricIndex;
         bool keyIsExternallyOwned  = true;
 
@@ -488,6 +502,11 @@ void TestBasicAddNocUpdateNocFlow(nlTestSuite * inSuite, void * inContext)
                                                                                 keyIsExternallyOwned, &newFabricIndex));
         NL_TEST_ASSERT(inSuite, newFabricIndex == 1);
         NL_TEST_ASSERT_EQUALS(inSuite, fabricTable.FabricCount(), 1);
+        {
+            // No more pending root cert; it's associated with a fabric now.
+            MutableByteSpan fetchedSpan{ rcacBuf };
+            NL_TEST_ASSERT(inSuite, fabricTable.FetchPendingNonFabricAssociatedRootCert(fetchedSpan) == CHIP_ERROR_NOT_FOUND);
+        }
 
         // No storage yet
         NL_TEST_ASSERT(inSuite, storage.GetNumKeys() == numStorageKeysAtStart);
@@ -1750,6 +1769,51 @@ void TestUpdateNocFailSafe(nlTestSuite * inSuite, void * inContext)
     }
 }
 
+void TestAddRootCertFailSafe(nlTestSuite * inSuite, void * inContext)
+{
+    Credentials::TestOnlyLocalCertificateAuthority fabric11CertAuthority;
+
+    chip::TestPersistentStorageDelegate storage;
+
+    NL_TEST_ASSERT(inSuite, fabric11CertAuthority.Init().IsSuccess());
+
+    // Initialize a fabric table.
+    ScopedFabricTable fabricTableHolder;
+    NL_TEST_ASSERT(inSuite, fabricTableHolder.Init(&storage) == CHIP_NO_ERROR);
+    FabricTable & fabricTable = fabricTableHolder.GetFabricTable();
+
+    NL_TEST_ASSERT_EQUALS(inSuite, fabricTable.FabricCount(), 0);
+
+    // Add a root cert, see that pending works, and that revert works
+    {
+        ByteSpan rcac = fabric11CertAuthority.GetRcac();
+
+        uint8_t rcacBuf[Credentials::kMaxCHIPCertLength];
+        {
+            // No pending root cert yet.
+            MutableByteSpan fetchedSpan{ rcacBuf };
+            NL_TEST_ASSERT(inSuite, fabricTable.FetchPendingNonFabricAssociatedRootCert(fetchedSpan) == CHIP_ERROR_NOT_FOUND);
+        }
+
+        NL_TEST_ASSERT_SUCCESS(inSuite, fabricTable.AddNewPendingTrustedRootCert(rcac));
+        {
+            // Now have a pending root cert.
+            MutableByteSpan fetchedSpan{ rcacBuf };
+            NL_TEST_ASSERT_SUCCESS(inSuite, fabricTable.FetchPendingNonFabricAssociatedRootCert(fetchedSpan));
+            NL_TEST_ASSERT(inSuite, fetchedSpan.data_equal(rcac));
+        }
+
+        // Revert
+        fabricTable.RevertPendingFabricData();
+
+        {
+            // No pending root cert anymore.
+            MutableByteSpan fetchedSpan{ rcacBuf };
+            NL_TEST_ASSERT(inSuite, fabricTable.FetchPendingNonFabricAssociatedRootCert(fetchedSpan) == CHIP_ERROR_NOT_FOUND);
+        }
+    }
+}
+
 void TestSequenceErrors(nlTestSuite * inSuite, void * inContext)
 {
     // TODO: Write test
@@ -2436,6 +2500,7 @@ static const nlTest sTests[] =
     NL_TEST_DEF("Validate fabrics are loaded from persistence at FabricTable::init", TestPersistence),
     NL_TEST_DEF("Test fail-safe handling during AddNOC", TestAddNocFailSafe),
     NL_TEST_DEF("Test fail-safe handling during UpdateNoc", TestUpdateNocFailSafe),
+    NL_TEST_DEF("Test fail-safe handling for root cert", TestAddRootCertFailSafe),
     NL_TEST_DEF("Test interlock sequencing errors", TestSequenceErrors),
     NL_TEST_DEF("Test fabric label changes", TestFabricLabelChange),
     NL_TEST_DEF("Test compressed fabric ID is properly generated", TestCompressedFabricId),