Issue 22318 - commissioner attestation delegate should be able to ove…

…rride success

src/controller/CHIPDeviceController.cpp

@@ -796,8 +796,8 @@ CHIP_ERROR DeviceCommissioner::Commission(NodeId remoteDeviceId)
 }
 
 CHIP_ERROR
-DeviceCommissioner::ContinueCommissioningAfterDeviceAttestationFailure(DeviceProxy * device,
-                                                                       Credentials::AttestationVerificationResult attestationResult)
+DeviceCommissioner::ContinueCommissioningAfterDeviceAttestation(DeviceProxy * device,
+                                                                Credentials::AttestationVerificationResult attestationResult)
 {
     MATTER_TRACE_EVENT_SCOPE("continueCommissioningDevice", "DeviceCommissioner");
     if (device == nullptr || device != mDeviceBeingCommissioned)
@@ -993,7 +993,8 @@ void DeviceCommissioner::OnAttestationResponse(void * context,
     commissioner->CommissioningStageComplete(CHIP_NO_ERROR, report);
 }
 
-void DeviceCommissioner::OnDeviceAttestationInformationVerification(void * context, AttestationVerificationResult result)
+void DeviceCommissioner::OnDeviceAttestationInformationVerification(
+    void * context, const Credentials::DeviceAttestationVerifier::AttestationInfo & info, AttestationVerificationResult result)
 {
     MATTER_TRACE_EVENT_SCOPE("OnDeviceAttestationInformationVerification", "DeviceCommissioner");
     DeviceCommissioner * commissioner = reinterpret_cast<DeviceCommissioner *>(context);
@@ -1004,6 +1005,9 @@ void DeviceCommissioner::OnDeviceAttestationInformationVerification(void * conte
         return;
     }
 
+    auto & params = commissioner->mDefaultCommissioner->GetCommissioningParameters();
+    Credentials::DeviceAttestationDelegate * deviceAttestationDelegate = params.GetDeviceAttestationDelegate();
+
     if (result != AttestationVerificationResult::kSuccess)
     {
         CommissioningDelegate::CommissioningReport report;
@@ -1024,14 +1028,11 @@ void DeviceCommissioner::OnDeviceAttestationInformationVerification(void * conte
         // Go look at AttestationVerificationResult enum in src/credentials/attestation_verifier/DeviceAttestationVerifier.h to
         // understand the errors.
 
-        auto & params = commissioner->mDefaultCommissioner->GetCommissioningParameters();
-        Credentials::DeviceAttestationDelegate * deviceAttestationDelegate = params.GetDeviceAttestationDelegate();
-
         // If a device attestation status delegate is installed, delegate handling of failure to the client and let them
         // decide on whether to proceed further or not.
         if (deviceAttestationDelegate)
         {
-            commissioner->ExtendArmFailSafeForFailedDeviceAttestation(result);
+            commissioner->ExtendArmFailSafeForDeviceAttestation(info, result);
         }
         else
         {
@@ -1040,15 +1041,22 @@ void DeviceCommissioner::OnDeviceAttestationInformationVerification(void * conte
     }
     else
     {
-        ChipLogProgress(Controller, "Successfully validated 'Attestation Information' command received from the device.");
-        commissioner->CommissioningStageComplete(CHIP_NO_ERROR);
+        if (deviceAttestationDelegate && deviceAttestationDelegate->ShouldWaitAfterDeviceAttestation())
+        {
+            commissioner->ExtendArmFailSafeForDeviceAttestation(info, result);
+        }
+        else
+        {
+            ChipLogProgress(Controller, "Successfully validated 'Attestation Information' command received from the device.");
+            commissioner->CommissioningStageComplete(CHIP_NO_ERROR);
+        }
     }
 }
 
-void DeviceCommissioner::OnArmFailSafeExtendedForFailedDeviceAttestation(
+void DeviceCommissioner::OnArmFailSafeExtendedForDeviceAttestation(
     void * context, const GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data)
 {
-    // If this function starts using "data", need to fix ExtendArmFailSafeForFailedDeviceAttestation accordingly.
+    // If this function starts using "data", need to fix ExtendArmFailSafeForDeviceAttestation accordingly.
     DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
 
     if (!commissioner->mDeviceBeingCommissioned)
@@ -1061,8 +1069,9 @@ void DeviceCommissioner::OnArmFailSafeExtendedForFailedDeviceAttestation(
     if (deviceAttestationDelegate)
     {
         ChipLogProgress(Controller, "Device attestation failed, delegating error handling to client");
-        deviceAttestationDelegate->OnDeviceAttestationFailed(commissioner, commissioner->mDeviceBeingCommissioned,
-                                                             commissioner->mAttestationResult);
+        deviceAttestationDelegate->OnDeviceAttestationCompleted(commissioner, commissioner->mDeviceBeingCommissioned,
+                                                                *commissioner->mAttestationDeviceInfo,
+                                                                commissioner->mAttestationResult);
     }
     else
     {
@@ -1073,7 +1082,7 @@ void DeviceCommissioner::OnArmFailSafeExtendedForFailedDeviceAttestation(
     }
 }
 
-void DeviceCommissioner::OnFailedToExtendedArmFailSafeFailedDeviceAttestation(void * context, CHIP_ERROR error)
+void DeviceCommissioner::OnFailedToExtendedArmFailSafeDeviceAttestation(void * context, CHIP_ERROR error)
 {
     ChipLogProgress(Controller, "Failed to extend fail-safe timer to handle attestation failure %s", chip::ErrorStr(error));
     DeviceCommissioner * commissioner = static_cast<DeviceCommissioner *>(context);
@@ -1083,13 +1092,29 @@ void DeviceCommissioner::OnFailedToExtendedArmFailSafeFailedDeviceAttestation(vo
     commissioner->CommissioningStageComplete(CHIP_ERROR_INTERNAL, report);
 }
 
-void DeviceCommissioner::ExtendArmFailSafeForFailedDeviceAttestation(AttestationVerificationResult result)
+void DeviceCommissioner::ClearAttestationDeviceInfo()
+{
+    if (mAttestationDeviceInfo != nullptr)
+    {
+        chip::Platform::Delete(mAttestationDeviceInfo);
+        mAttestationDeviceInfo = nullptr;
+    }
+}
+
+void DeviceCommissioner::ExtendArmFailSafeForDeviceAttestation(const Credentials::DeviceAttestationVerifier::AttestationInfo & info,
+                                                               Credentials::AttestationVerificationResult result)
 {
     mAttestationResult = result;
 
     auto & params                                                      = mDefaultCommissioner->GetCommissioningParameters();
     Credentials::DeviceAttestationDelegate * deviceAttestationDelegate = params.GetDeviceAttestationDelegate();
-    auto expiryLengthSeconds                                           = deviceAttestationDelegate->FailSafeExpiryTimeoutSecs();
+
+    if (deviceAttestationDelegate->ShouldWaitAfterDeviceAttestation())
+    {
+        mAttestationDeviceInfo = chip::Platform::New<Credentials::DeviceAttestationVerifier::AttestationDeviceInfo>(info);
+    }
+
+    auto expiryLengthSeconds = deviceAttestationDelegate->FailSafeExpiryTimeoutSecs();
     if (expiryLengthSeconds.HasValue())
     {
         GeneralCommissioning::Commands::ArmFailSafe::Type request;
@@ -1098,16 +1123,16 @@ void DeviceCommissioner::ExtendArmFailSafeForFailedDeviceAttestation(Attestation
         ChipLogProgress(Controller, "Changing fail-safe timer to %u seconds to handle DA failure", request.expiryLengthSeconds);
         // Per spec, anything we do with the fail-safe armed must not time out
         // in less than kMinimumCommissioningStepTimeout.
-        SendCommand<GeneralCommissioningCluster>(mDeviceBeingCommissioned, request, OnArmFailSafeExtendedForFailedDeviceAttestation,
-                                                 OnFailedToExtendedArmFailSafeFailedDeviceAttestation,
+        SendCommand<GeneralCommissioningCluster>(mDeviceBeingCommissioned, request, OnArmFailSafeExtendedForDeviceAttestation,
+                                                 OnFailedToExtendedArmFailSafeDeviceAttestation,
                                                  MakeOptional(kMinimumCommissioningStepTimeout));
     }
     else
     {
         ChipLogProgress(Controller, "Proceeding without changing fail-safe timer value as delegate has not set it");
         // Callee does not use data argument.
         const GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType data;
-        OnArmFailSafeExtendedForFailedDeviceAttestation(this, data);
+        OnArmFailSafeExtendedForDeviceAttestation(this, data);
     }
 }
 
@@ -1474,6 +1499,9 @@ void OnBasicFailure(void * context, CHIP_ERROR error)
 void DeviceCommissioner::CleanupCommissioning(DeviceProxy * proxy, NodeId nodeId, const CompletionStatus & completionStatus)
 {
     commissioningCompletionStatus = completionStatus;
+
+    ClearAttestationDeviceInfo();
+
     if (completionStatus.err == CHIP_NO_ERROR)
     {
 

src/controller/CHIPDeviceController.h

@@ -487,15 +487,14 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
     /**
      * @brief
      *   This function instructs the commissioner to proceed to the next stage of commissioning after
-     *   attestation failure is reported to an installed attestation delegate.
+     *   attestation is reported to an installed attestation delegate.
      *
      * @param[in] device                The device being commissioned.
      * @param[in] attestationResult     The attestation result to use instead of whatever the device
      *                                  attestation verifier came up with. May be a success or an error result.
      */
     CHIP_ERROR
-    ContinueCommissioningAfterDeviceAttestationFailure(DeviceProxy * device,
-                                                       Credentials::AttestationVerificationResult attestationResult);
+    ContinueCommissioningAfterDeviceAttestation(DeviceProxy * device, Credentials::AttestationVerificationResult attestationResult);
 
     CHIP_ERROR GetDeviceBeingCommissioned(NodeId deviceId, CommissioneeDeviceProxy ** device);
 
@@ -715,7 +714,9 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
     /* Callback when the previously sent CSR request results in failure */
     static void OnCSRFailureResponse(void * context, CHIP_ERROR error);
 
-    void ExtendArmFailSafeForFailedDeviceAttestation(Credentials::AttestationVerificationResult result);
+    void ClearAttestationDeviceInfo();
+    void ExtendArmFailSafeForDeviceAttestation(const Credentials::DeviceAttestationVerifier::AttestationInfo & info,
+                                               Credentials::AttestationVerificationResult result);
     static void OnCertificateChainFailureResponse(void * context, CHIP_ERROR error);
     static void OnCertificateChainResponse(
         void * context, const app::Clusters::OperationalCredentials::Commands::CertificateChainResponse::DecodableType & response);
@@ -754,7 +755,9 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
     static void OnDeviceConnectedFn(void * context, Messaging::ExchangeManager & exchangeMgr, SessionHandle & sessionHandle);
     static void OnDeviceConnectionFailureFn(void * context, const ScopedNodeId & peerId, CHIP_ERROR error);
 
-    static void OnDeviceAttestationInformationVerification(void * context, Credentials::AttestationVerificationResult result);
+    static void OnDeviceAttestationInformationVerification(void * context,
+                                                           const Credentials::DeviceAttestationVerifier::AttestationInfo & info,
+                                                           Credentials::AttestationVerificationResult result);
 
     static void OnDeviceNOCChainGeneration(void * context, CHIP_ERROR status, const ByteSpan & noc, const ByteSpan & icac,
                                            const ByteSpan & rcac, Optional<AesCcm128KeySpan> ipk, Optional<NodeId> adminSubject);
@@ -779,9 +782,9 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
                                  const app::Clusters::GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data);
     static void OnDisarmFailsafeFailure(void * context, CHIP_ERROR error);
     void DisarmDone();
-    static void OnArmFailSafeExtendedForFailedDeviceAttestation(
+    static void OnArmFailSafeExtendedForDeviceAttestation(
         void * context, const chip::app::Clusters::GeneralCommissioning::Commands::ArmFailSafeResponse::DecodableType & data);
-    static void OnFailedToExtendedArmFailSafeFailedDeviceAttestation(void * context, CHIP_ERROR error);
+    static void OnFailedToExtendedArmFailSafeDeviceAttestation(void * context, CHIP_ERROR error);
 
     /**
      * @brief
@@ -860,7 +863,8 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
     chip::Callback::Callback<OnDeviceConnected> mOnDeviceConnectedCallback;
     chip::Callback::Callback<OnDeviceConnectionFailure> mOnDeviceConnectionFailureCallback;
 
-    chip::Callback::Callback<Credentials::OnAttestationInformationVerification> mDeviceAttestationInformationVerificationCallback;
+    chip::Callback::Callback<Credentials::DeviceAttestationVerifier::OnAttestationInformationVerification>
+        mDeviceAttestationInformationVerificationCallback;
 
     chip::Callback::Callback<OnNOCChainGeneration> mDeviceNOCChainCallback;
     SetUpCodePairer mSetUpCodePairer;
@@ -874,7 +878,8 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
     Platform::UniquePtr<app::ClusterStateCache> mAttributeCache;
     Platform::UniquePtr<app::ReadClient> mReadClient;
     Credentials::AttestationVerificationResult mAttestationResult;
-    Credentials::DeviceAttestationVerifier * mDeviceAttestationVerifier = nullptr;
+    Credentials::DeviceAttestationVerifier::AttestationDeviceInfo * mAttestationDeviceInfo = nullptr;
+    Credentials::DeviceAttestationVerifier * mDeviceAttestationVerifier                    = nullptr;
 };
 
 } // namespace Controller

src/controller/CommissioningDelegate.h

@@ -414,6 +414,7 @@ class CommissioningParameters
     CompletionStatus completionStatus;
     Credentials::DeviceAttestationDelegate * mDeviceAttestationDelegate =
         nullptr; // Delegate to handle device attestation failures during commissioning
+    Optional<bool> mShouldWaitPostDeviceAttestation;
     Optional<bool> mAttemptWiFiNetworkScan;
     Optional<bool> mAttemptThreadNetworkScan; // This automatically gets set to false when a ThreadOperationalDataset is set
     Optional<bool> mSkipCommissioningComplete;

src/credentials/attestation_verifier/DacOnlyPartialAttestationVerifier.cpp

@@ -148,7 +148,7 @@ void PartialDACVerifier::VerifyAttestationInformation(const DeviceAttestationVer
     }
 
 exit:
-    onCompletion->mCall(onCompletion->mContext, attestationError);
+    onCompletion->mCall(onCompletion->mContext, info, attestationError);
 }
 
 } // namespace Credentials

src/credentials/attestation_verifier/DefaultDeviceAttestationVerifier.cpp

@@ -277,7 +277,7 @@ void DefaultDACVerifier::VerifyAttestationInformation(const DeviceAttestationVer
     }
 
 exit:
-    onCompletion->mCall(onCompletion->mContext, attestationError);
+    onCompletion->mCall(onCompletion->mContext, info, attestationError);
 }
 
 AttestationVerificationResult DefaultDACVerifier::ValidateCertificationDeclarationSignature(const ByteSpan & cmsEnvelopeBuffer,

src/credentials/attestation_verifier/DeviceAttestationDelegate.h

@@ -48,14 +48,29 @@ class DeviceAttestationDelegate
     /**
      * @brief
      *   This method is invoked when device attestation fails for a device that is being commissioned. The client
-     *   handling the failure has the option to continue commissionning or fail the operation.
+     *   handling the failure has the option to continue commissioning or fail the operation.
+     *
+     *   If ShouldWaitAfterDeviceAttestation returns false, then in the case attestationResult is successful, the
+     *   commissioner would finish commissioning the device after OnDeviceAttestationCompleted returns.
+     *
+     *   If ShouldWaitAfterDeviceAttestation returns true, then the commissioner will always wait for a
+     *   ContinueCommissioningAfterDeviceAttestation call after calling OnDeviceAttestationCompleted.
      *
      *   @param deviceCommissioner The commissioner object that is commissioning the device
      *   @param device The proxy represent the device being commissioned
+     *   @param info The structure holding device infor for additional verification by the application
      *   @param attestationResult The failure code for the device attestation validation operation
      */
-    virtual void OnDeviceAttestationFailed(Controller::DeviceCommissioner * deviceCommissioner, DeviceProxy * device,
-                                           AttestationVerificationResult attestationResult) = 0;
+    virtual void OnDeviceAttestationCompleted(Controller::DeviceCommissioner * deviceCommissioner, DeviceProxy * device,
+                                              const DeviceAttestationVerifier::AttestationDeviceInfo & info,
+                                              AttestationVerificationResult attestationResult) = 0;
+
+    /**
+     * @brief
+     *   Override this method to return whether the attestation delegate wants the commissioner to wait for a
+     *   ContinueCommissioningAfterDeviceAttestation call in the case attestationResult is successful.
+     */
+    virtual bool ShouldWaitAfterDeviceAttestation() { return false; };
 };
 
 } // namespace Credentials

src/credentials/attestation_verifier/DeviceAttestationVerifier.cpp

@@ -16,7 +16,9 @@
  */
 #include "DeviceAttestationVerifier.h"
 
+#include <credentials/DeviceAttestationConstructor.h>
 #include <crypto/CHIPCryptoPAL.h>
+#include <lib/support/CHIPMem.h>
 
 using namespace chip::Crypto;
 
@@ -111,5 +113,42 @@ void SetDeviceAttestationVerifier(DeviceAttestationVerifier * verifier)
     gDacVerifier = verifier;
 }
 
+ByteSpan DupByteSpanHelper(const ByteSpan & span_to_dup)
+{
+    uint8_t * bytes = (uint8_t *) chip::Platform::MemoryAlloc(span_to_dup.size());
+    memcpy(bytes, span_to_dup.data(), span_to_dup.size());
+    return ByteSpan(bytes, span_to_dup.size());
+}
+
+DeviceAttestationVerifier::AttestationDeviceInfo::AttestationDeviceInfo(const AttestationInfo & attestationInfo) :
+    mPaiDerBuffer(DupByteSpanHelper(attestationInfo.paiDerBuffer)), mDacDerBuffer(DupByteSpanHelper(attestationInfo.dacDerBuffer))
+{
+    mAttestationElementsBufferCopy = chip::Platform::MemoryAlloc(attestationInfo.attestationElementsBuffer.size());
+    memcpy(mAttestationElementsBufferCopy, attestationInfo.attestationElementsBuffer.data(),
+           attestationInfo.attestationElementsBuffer.size());
+
+    ByteSpan certificationDeclarationSpan;
+    ByteSpan attestationNonceSpan;
+    uint32_t timestampDeconstructed;
+    ByteSpan firmwareInfoSpan;
+    DeviceAttestationVendorReservedDeconstructor vendorReserved;
+
+    ByteSpan attestationElementsBufferSpan =
+        ByteSpan((uint8_t *) mAttestationElementsBufferCopy, attestationInfo.attestationElementsBuffer.size());
+
+    if (DeconstructAttestationElements(attestationElementsBufferSpan, certificationDeclarationSpan, attestationNonceSpan,
+                                       timestampDeconstructed, firmwareInfoSpan, vendorReserved) == CHIP_NO_ERROR)
+    {
+        mCdBuffer = MakeOptional(certificationDeclarationSpan);
+    }
+}
+
+DeviceAttestationVerifier::AttestationDeviceInfo::~AttestationDeviceInfo()
+{
+    chip::Platform::MemoryFree(mAttestationElementsBufferCopy);
+    chip::Platform::MemoryFree((void *) mPaiDerBuffer.data());
+    chip::Platform::MemoryFree((void *) mPaiDerBuffer.data());
+}
+
 } // namespace Credentials
 } // namespace chip