Stop using Credentials::GetDefaultDACVerifier on Darwin. (#19697)

Credentials::GetDefaultDACVerifier returns a singleton object,
initialized the very first time the function is called.  The Darwin
framework wants to be able to shut down and restart with a new PAA
trust store, which means not relying on this initialized-once
singleton.

The fixes to DeviceControllerSystemState shutdown were needed to make
the tests (which start up a factory, then shut it down without
bringing up any controllers, which is a valid thing to do) pass.

src/controller/CHIPDeviceControllerFactory.cpp

@@ -327,7 +327,13 @@ void DeviceControllerFactory::Shutdown()
 
 CHIP_ERROR DeviceControllerSystemState::Shutdown()
 {
-    VerifyOrReturnError(mRefCount == 1, CHIP_ERROR_INCORRECT_STATE);
+    VerifyOrReturnError(mRefCount <= 1, CHIP_ERROR_INCORRECT_STATE);
+    if (mHaveShutDown)
+    {
+        // Nothing else to do here.
+        return CHIP_NO_ERROR;
+    }
+    mHaveShutDown = true;
 
     ChipLogDetail(Controller, "Shutting down the System State, this will teardown the CHIP Stack");
 

src/controller/CHIPDeviceControllerSystemState.h

@@ -106,7 +106,13 @@ class DeviceControllerSystemState
     using CASEClientPool        = DeviceControllerSystemStateParams::CASEClientPool;
 
 public:
-    ~DeviceControllerSystemState(){};
+    ~DeviceControllerSystemState()
+    {
+        // We could get here if a DeviceControllerFactory is shut down
+        // without ever creating any controllers, so our refcount never goes
+        // above 1.  In that case we need to make sure we call Shutdown().
+        Shutdown();
+    };
     DeviceControllerSystemState(DeviceControllerSystemStateParams params) :
         mSystemLayer(params.systemLayer), mTCPEndPointManager(params.tcpEndPointManager),
         mUDPEndPointManager(params.udpEndPointManager), mTransportMgr(params.transportMgr), mSessionMgr(params.sessionMgr),
@@ -195,6 +201,8 @@ class DeviceControllerSystemState
 
     std::atomic<uint32_t> mRefCount{ 1 };
 
+    bool mHaveShutDown = false;
+
     CHIP_ERROR Shutdown();
 };
 

src/credentials/attestation_verifier/DefaultDeviceAttestationVerifier.cpp

@@ -145,31 +145,7 @@ CHIP_ERROR GetCertificationDeclarationCertificate(const ByteSpan & skid, Mutable
     return CopySpanToMutableSpan(ByteSpan{ sCertChainLookupTable[certChainLookupTableIdx].mCertificate }, outCertificate);
 }
 
-class DefaultDACVerifier : public DeviceAttestationVerifier
-{
-public:
-    DefaultDACVerifier(const AttestationTrustStore * paaRootStore) : mAttestationTrustStore(paaRootStore) {}
-
-    void VerifyAttestationInformation(const DeviceAttestationVerifier::AttestationInfo & info,
-                                      Callback::Callback<OnAttestationInformationVerification> * onCompletion) override;
-
-    AttestationVerificationResult ValidateCertificationDeclarationSignature(const ByteSpan & cmsEnvelopeBuffer,
-                                                                            ByteSpan & certDeclBuffer) override;
-
-    AttestationVerificationResult ValidateCertificateDeclarationPayload(const ByteSpan & certDeclBuffer,
-                                                                        const ByteSpan & firmwareInfo,
-                                                                        const DeviceInfoForAttestation & deviceInfo) override;
-
-    CHIP_ERROR VerifyNodeOperationalCSRInformation(const ByteSpan & nocsrElementsBuffer,
-                                                   const ByteSpan & attestationChallengeBuffer,
-                                                   const ByteSpan & attestationSignatureBuffer, const P256PublicKey & dacPublicKey,
-                                                   const ByteSpan & csrNonce) override;
-
-protected:
-    DefaultDACVerifier() {}
-
-    const AttestationTrustStore * mAttestationTrustStore;
-};
+} // namespace
 
 void DefaultDACVerifier::VerifyAttestationInformation(const DeviceAttestationVerifier::AttestationInfo & info,
                                                       Callback::Callback<OnAttestationInformationVerification> * onCompletion)
@@ -443,8 +419,6 @@ CHIP_ERROR DefaultDACVerifier::VerifyNodeOperationalCSRInformation(const ByteSpa
     return CHIP_NO_ERROR;
 }
 
-} // namespace
-
 const AttestationTrustStore * GetTestAttestationTrustStore()
 {
     return &kTestAttestationTrustStore;

src/credentials/attestation_verifier/DefaultDeviceAttestationVerifier.h

@@ -21,6 +21,32 @@
 namespace chip {
 namespace Credentials {
 
+class DefaultDACVerifier : public DeviceAttestationVerifier
+{
+public:
+    DefaultDACVerifier(const AttestationTrustStore * paaRootStore) : mAttestationTrustStore(paaRootStore) {}
+
+    void VerifyAttestationInformation(const DeviceAttestationVerifier::AttestationInfo & info,
+                                      Callback::Callback<OnAttestationInformationVerification> * onCompletion) override;
+
+    AttestationVerificationResult ValidateCertificationDeclarationSignature(const ByteSpan & cmsEnvelopeBuffer,
+                                                                            ByteSpan & certDeclBuffer) override;
+
+    AttestationVerificationResult ValidateCertificateDeclarationPayload(const ByteSpan & certDeclBuffer,
+                                                                        const ByteSpan & firmwareInfo,
+                                                                        const DeviceInfoForAttestation & deviceInfo) override;
+
+    CHIP_ERROR VerifyNodeOperationalCSRInformation(const ByteSpan & nocsrElementsBuffer,
+                                                   const ByteSpan & attestationChallengeBuffer,
+                                                   const ByteSpan & attestationSignatureBuffer,
+                                                   const Crypto::P256PublicKey & dacPublicKey, const ByteSpan & csrNonce) override;
+
+protected:
+    DefaultDACVerifier() {}
+
+    const AttestationTrustStore * mAttestationTrustStore;
+};
+
 /**
  * @brief Get implementation of a PAA root store containing a basic set of static PAA roots
  *        sufficient for *testing* only.
@@ -35,14 +61,17 @@ namespace Credentials {
 const AttestationTrustStore * GetTestAttestationTrustStore();
 
 /**
- * @brief Get implementation of a sample DAC verifier to validate device
+ * @brief Get a singleton implementation of a sample DAC verifier to validate device
  *        attestation procedure.
  *
  * @param[in] paaRootStore Pointer to the AttestationTrustStore instance to be used by implementation
  *                         of default DeviceAttestationVerifier. Caller must ensure storage is
  *                         always available while the DeviceAttestationVerifier could be used.
  *
  * @returns a singleton DeviceAttestationVerifier that satisfies basic device attestation procedure requirements.
+ *          This has process lifetime, so the paaRootStore must also have
+ *          process lifetime.  In particular, after the first call it's not
+ *          possible to change which AttestationTrustStore is used by this verifier.
  */
 DeviceAttestationVerifier * GetDefaultDACVerifier(const AttestationTrustStore * paaRootStore);
 

src/darwin/Framework/CHIP/CHIPDeviceController.mm

@@ -44,6 +44,7 @@
 #include <controller/CommissioningWindowOpener.h>
 #include <credentials/FabricTable.h>
 #include <credentials/GroupDataProvider.h>
+#include <credentials/attestation_verifier/DefaultDeviceAttestationVerifier.h>
 #include <lib/core/CHIPVendorIdentifiers.hpp>
 #include <platform/PlatformManager.h>
 #include <setup_payload/ManualSetupPayloadGenerator.h>
@@ -277,6 +278,7 @@ - (BOOL)startup:(CHIPDeviceControllerStartupParamsInternal *)startupParams
             commissionerParams.controllerNOC = noc;
         }
         commissionerParams.controllerVendorId = static_cast<chip::VendorId>([startupParams.vendorId unsignedShortValue]);
+        commissionerParams.deviceAttestationVerifier = _factory.deviceAttestationVerifier;
 
         auto & factory = chip::Controller::DeviceControllerFactory::GetInstance();
 

src/darwin/Framework/CHIP/MatterControllerFactory.mm

@@ -45,6 +45,7 @@
 
 static NSString * const kErrorPersistentStorageInit = @"Init failure while creating a persistent storage delegate";
 static NSString * const kErrorAttestationTrustStoreInit = @"Init failure while creating the attestation trust store";
+static NSString * const kErrorDACVerifierInit = @"Init failure while creating the device attestation verifier";
 static NSString * const kInfoFactoryShutdown = @"Shutting down the Matter controller factory";
 static NSString * const kErrorGroupProviderInit = @"Init failure while initializing group data provider";
 static NSString * const kErrorControllersInit = @"Init controllers array failure";
@@ -64,6 +65,7 @@ @interface MatterControllerFactory ()
 @property (readonly) Credentials::GroupDataProviderImpl * groupDataProvider;
 @property (readonly) NSMutableArray<CHIPDeviceController *> * controllers;
 @property (readonly) PersistentStorageOperationalKeystore * keystore;
+@property () chip::Credentials::DeviceAttestationVerifier * deviceAttestationVerifier;
 
 - (BOOL)findMatchingFabric:(FabricTable &)fabricTable
                     params:(CHIPDeviceControllerStartupParams *)params
@@ -156,6 +158,11 @@ - (void)cleanupInitObjects
 
 - (void)cleanupStartupObjects
 {
+    if (_deviceAttestationVerifier) {
+        delete _deviceAttestationVerifier;
+        _deviceAttestationVerifier = nullptr;
+    }
+
     if (_attestationTrustStoreBridge) {
         delete _attestationTrustStoreBridge;
         _attestationTrustStoreBridge = nullptr;
@@ -209,17 +216,22 @@ - (BOOL)startup:(MatterControllerFactoryParams *)startupParams
         }
 
         // Initialize device attestation verifier
+        const Credentials::AttestationTrustStore * trustStore;
         if (startupParams.paaCerts) {
             _attestationTrustStoreBridge = new CHIPAttestationTrustStoreBridge(startupParams.paaCerts);
             if (_attestationTrustStoreBridge == nullptr) {
                 CHIP_LOG_ERROR("Error: %@", kErrorAttestationTrustStoreInit);
                 return;
             }
-            chip::Credentials::SetDeviceAttestationVerifier(chip::Credentials::GetDefaultDACVerifier(_attestationTrustStoreBridge));
+            trustStore = _attestationTrustStoreBridge;
         } else {
             // TODO: Replace testingRootStore with a AttestationTrustStore that has the necessary official PAA roots available
-            const chip::Credentials::AttestationTrustStore * testingRootStore = chip::Credentials::GetTestAttestationTrustStore();
-            chip::Credentials::SetDeviceAttestationVerifier(chip::Credentials::GetDefaultDACVerifier(testingRootStore));
+            trustStore = Credentials::GetTestAttestationTrustStore();
+        }
+        _deviceAttestationVerifier = new Credentials::DefaultDACVerifier(trustStore);
+        if (_deviceAttestationVerifier == nullptr) {
+            CHIP_LOG_ERROR("Error: %@", kErrorDACVerifierInit);
+            return;
         }
 
         chip::Controller::FactoryInitParams params;

src/darwin/Framework/CHIP/MatterControllerFactory_Internal.h

@@ -31,6 +31,7 @@ class CHIPPersistentStorageDelegateBridge;
 namespace chip {
 namespace Credentials {
     class GroupDataProvider;
+    class DeviceAttestationVerifier;
 } // namespace Credentials
 } // namespace chip
 
@@ -44,7 +45,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property (readonly) CHIPPersistentStorageDelegateBridge * storageDelegateBridge;
 @property (readonly) chip::Credentials::GroupDataProvider * groupData;
-
+@property (readonly) chip::Credentials::DeviceAttestationVerifier * deviceAttestationVerifier;
 @end
 
 NS_ASSUME_NONNULL_END

src/darwin/Framework/CHIPTests/CHIPClustersTests.m

@@ -103,10 +103,19 @@ - (void)testInitStack
     __auto_type * storage = [[CHIPTestStorage alloc] init];
     __auto_type * factoryParams = [[MatterControllerFactoryParams alloc] initWithStorage:storage];
     factoryParams.port = @(kLocalPort);
+    // For our first startup, use a dummy paaCerts array.
+    factoryParams.paaCerts = [[NSArray alloc] init];
 
     BOOL ok = [factory startup:factoryParams];
     XCTAssertTrue(ok);
 
+    // Test that things still work right if we then shut down and
+    // restart the factory.
+    [factory shutdown];
+    factoryParams.paaCerts = nil;
+    ok = [factory startup:factoryParams];
+    XCTAssertTrue(ok);
+
     __auto_type * testKeys = [[CHIPTestKeys alloc] init];
     XCTAssertNotNil(testKeys);