Cleanup CCTRL attestation logic from PairingCommand

project-chip · Oct 3, 2024 · 6575f83 · 6575f83
1 parent 78aac89
commit 6575f83
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 92 deletions.
diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.cpp b/examples/fabric-admin/commands/pairing/PairingCommand.cpp
@@ -30,7 +30,6 @@
 #include <protocols/secure_channel/PASESession.h>
 #include <setup_payload/ManualSetupPayloadParser.h>
 #include <setup_payload/QRCodeSetupPayloadParser.h>
-#include <setup_payload/SetupPayload.h>
 
 #include <string>
 
@@ -41,28 +40,6 @@
 using namespace ::chip;
 using namespace ::chip::Controller;
 
-namespace {
-
-CHIP_ERROR GetPayload(const char * setUpCode, SetupPayload & payload)
-{
-    VerifyOrReturnValue(setUpCode, CHIP_ERROR_INVALID_ARGUMENT);
-    bool isQRCode = strncmp(setUpCode, kQRCodePrefix, strlen(kQRCodePrefix)) == 0;
-    if (isQRCode)
-    {
-        ReturnErrorOnFailure(QRCodeSetupPayloadParser(setUpCode).populatePayload(payload));
-        VerifyOrReturnError(payload.isValidQRCodePayload(), CHIP_ERROR_INVALID_ARGUMENT);
-    }
-    else
-    {
-        ReturnErrorOnFailure(ManualSetupPayloadParser(setUpCode).populatePayload(payload));
-        VerifyOrReturnError(payload.isValidManualCode(), CHIP_ERROR_INVALID_ARGUMENT);
-    }
-
-    return CHIP_NO_ERROR;
-}
-
-} // namespace
-
 CHIP_ERROR PairingCommand::RunCommand()
 {
     CurrentCommissioner().RegisterPairingDelegate(this);
@@ -128,7 +105,10 @@ CommissioningParameters PairingCommand::GetCommissioningParameters()
 {
     auto params = CommissioningParameters();
     params.SetSkipCommissioningComplete(mSkipCommissioningComplete.ValueOr(false));
-    params.SetDeviceAttestationDelegate(this);
+    if (mBypassAttestationVerifier.ValueOr(false))
+    {
+        params.SetDeviceAttestationDelegate(this);
+    }
 
     switch (mNetworkType)
     {
@@ -579,77 +559,13 @@ Optional<uint16_t> PairingCommand::FailSafeExpiryTimeoutSecs() const
     return Optional<uint16_t>();
 }
 
-bool PairingCommand::ShouldWaitAfterDeviceAttestation()
-{
-    // If there is a vendor ID and product ID, request OnDeviceAttestationCompleted().
-    // Currently this is added in the case that the example is performing reverse commissioning,
-    // but it would be an improvement to store that explicitly.
-    // TODO: Issue #35297 - [Fabric Sync] Improve where we get VID and PID when validating CCTRL CommissionNode command
-    SetupPayload payload;
-    CHIP_ERROR err = GetPayload(mOnboardingPayload, payload);
-    return err == CHIP_NO_ERROR && (payload.vendorID != 0 || payload.productID != 0);
-}
-
 void PairingCommand::OnDeviceAttestationCompleted(Controller::DeviceCommissioner * deviceCommissioner, DeviceProxy * device,
                                                   const Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info,
                                                   Credentials::AttestationVerificationResult attestationResult)
 {
-    SetupPayload payload;
-    CHIP_ERROR parse_error = GetPayload(mOnboardingPayload, payload);
-    if (parse_error == CHIP_NO_ERROR && (payload.vendorID != 0 || payload.productID != 0))
-    {
-        if (payload.vendorID == 0 || payload.productID == 0)
-        {
-            ChipLogProgress(NotSpecified,
-                            "Failed validation: vendorID or productID must not be 0."
-                            "Requested VID: %u, Requested PID: %u.",
-                            payload.vendorID, payload.productID);
-            deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(
-                device, Credentials::AttestationVerificationResult::kInvalidArgument);
-            return;
-        }
-
-        if (payload.vendorID != info.BasicInformationVendorId() || payload.productID != info.BasicInformationProductId())
-        {
-            ChipLogProgress(NotSpecified,
-                            "Failed validation of vendorID or productID."
-                            "Requested VID: %u, Requested PID: %u,"
-                            "Detected VID: %u, Detected PID %u.",
-                            payload.vendorID, payload.productID, info.BasicInformationVendorId(), info.BasicInformationProductId());
-            deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(
-                device,
-                payload.vendorID == info.BasicInformationVendorId()
-                    ? Credentials::AttestationVerificationResult::kDacProductIdMismatch
-                    : Credentials::AttestationVerificationResult::kDacVendorIdMismatch);
-            return;
-        }
-
-        // NOTE: This will log errors even if the attestion was successful.
-        auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult);
-        if (CHIP_NO_ERROR != err)
-        {
-            SetCommandExitStatus(err);
-        }
-        return;
-    }
-
-    // OnDeviceAttestationCompleted() is called if ShouldWaitAfterDeviceAttestation() returns true
-    // or if there is an attestation error. The conditions for ShouldWaitAfterDeviceAttestation() have
-    // already been checked, so the call to OnDeviceAttestationCompleted() was an error.
-    if (mBypassAttestationVerifier.ValueOr(false))
-    {
-        // Bypass attestation verification, continue with success
-        auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(
-            device, Credentials::AttestationVerificationResult::kSuccess);
-        if (CHIP_NO_ERROR != err)
-        {
-            SetCommandExitStatus(err);
-        }
-        return;
-    }
-
-    // Don't bypass attestation, continue with error.
-    auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(device, attestationResult);
+    // Bypass attestation verification, continue with success
+    auto err = deviceCommissioner->ContinueCommissioningAfterDeviceAttestation(
+        device, Credentials::AttestationVerificationResult::kSuccess);
     if (CHIP_NO_ERROR != err)
     {
         SetCommandExitStatus(err);

diff --git a/examples/fabric-admin/commands/pairing/PairingCommand.h b/examples/fabric-admin/commands/pairing/PairingCommand.h
@@ -207,7 +207,6 @@ class PairingCommand : public CHIPCommand,
 
     /////////// DeviceAttestationDelegate Interface /////////
     chip::Optional<uint16_t> FailSafeExpiryTimeoutSecs() const override;
-    bool ShouldWaitAfterDeviceAttestation() override;
     void OnDeviceAttestationCompleted(chip::Controller::DeviceCommissioner * deviceCommissioner, chip::DeviceProxy * device,
                                       const chip::Credentials::DeviceAttestationVerifier::AttestationDeviceInfo & info,
                                       chip::Credentials::AttestationVerificationResult attestationResult) override;