Revert "Check VID/PID when doing device attestation (project-chip#14551…

…)" (project-chip#14795)

This reverts commit a7d7d8d.

src/controller/AutoCommissioner.cpp

@@ -316,10 +316,10 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
         switch (report.stageCompleted)
         {
         case CommissioningStage::kReadVendorId:
-            mParams.SetRemoteVendorId(report.Get<BasicVendor>().vendorId);
+            mVendorId = report.Get<BasicVendor>().vendorId;
             break;
         case CommissioningStage::kReadProductId:
-            mParams.SetRemoteProductId(report.Get<BasicProduct>().productId);
+            mProductId = report.Get<BasicProduct>().productId;
             break;
         case CommissioningStage::kReadSoftwareVersion:
             mSoftwareVersion = report.Get<BasicSoftware>().softwareVersion;
@@ -394,6 +394,7 @@ CHIP_ERROR AutoCommissioner::CommissioningStepFinished(CHIP_ERROR err, Commissio
     }
 
     mParams.SetCompletionStatus(err);
+    // TODO: Get real endpoint
     mCommissioner->PerformCommissioningStep(proxy, nextStage, mParams, this, GetEndpoint(nextStage), GetCommandTimeout(nextStage));
     return CHIP_NO_ERROR;
 }

src/controller/AutoCommissioner.h

@@ -57,7 +57,10 @@ class AutoCommissioner : public CommissioningDelegate
     OperationalDeviceProxy * mOperationalDeviceProxy                 = nullptr;
     OperationalCredentialsDelegate * mOperationalCredentialsDelegate = nullptr;
     CommissioningParameters mParams                                  = CommissioningParameters();
+    VendorId mVendorId;
+    uint16_t mProductId;
     uint32_t mSoftwareVersion;
+    // Memory space for the commisisoning parameters that come in as ByteSpans - the caller is not guaranteed to retain this memory
     uint8_t mSsid[CommissioningParameters::kMaxSsidLen];
     uint8_t mCredentials[CommissioningParameters::kMaxCredentialsLen];
     uint8_t mThreadOperationalDataset[CommissioningParameters::kMaxThreadDatasetLen];

src/controller/CHIPDeviceController.cpp

@@ -1153,8 +1153,7 @@ void DeviceCommissioner::OnDeviceAttestationInformationVerification(void * conte
 
 CHIP_ERROR DeviceCommissioner::ValidateAttestationInfo(const ByteSpan & attestationElements, const ByteSpan & signature,
                                                        const ByteSpan & attestationNonce, const ByteSpan & pai,
-                                                       const ByteSpan & dac, VendorId remoteVendorId, uint16_t remoteProductId,
-                                                       DeviceProxy * proxy)
+                                                       const ByteSpan & dac, DeviceProxy * proxy)
 {
     VerifyOrReturnError(mState == State::Initialized, CHIP_ERROR_INCORRECT_STATE);
     VerifyOrReturnError(proxy != nullptr, CHIP_ERROR_INCORRECT_STATE);
@@ -1166,7 +1165,7 @@ CHIP_ERROR DeviceCommissioner::ValidateAttestationInfo(const ByteSpan & attestat
         proxy->GetSecureSession().Value()->AsSecureSession()->GetCryptoContext().GetAttestationChallenge();
 
     dac_verifier->VerifyAttestationInformation(attestationElements, attestationChallenge, signature, pai, dac, attestationNonce,
-                                               remoteVendorId, remoteProductId, &mDeviceAttestationInformationVerificationCallback);
+                                               &mDeviceAttestationInformationVerificationCallback);
 
     // TODO: Validate Firmware Information
 
@@ -1799,6 +1798,7 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
         }
         mAttributeCache = std::move(attributeCache);
         mReadClient     = std::move(readClient);
+        return;
     }
     break;
     case CommissioningStage::kConfigRegulatory: {
@@ -1866,16 +1866,14 @@ void DeviceCommissioner::PerformCommissioningStep(DeviceProxy * proxy, Commissio
     case CommissioningStage::kAttestationVerification:
         ChipLogProgress(Controller, "Verifying attestation");
         if (!params.GetAttestationElements().HasValue() || !params.GetAttestationSignature().HasValue() ||
-            !params.GetAttestationNonce().HasValue() || !params.GetDAC().HasValue() || !params.GetPAI().HasValue() ||
-            !params.GetRemoteVendorId().HasValue() || !params.GetRemoteProductId().HasValue())
+            !params.GetAttestationNonce().HasValue() || !params.GetDAC().HasValue() || !params.GetPAI().HasValue())
         {
             ChipLogError(Controller, "Missing attestation information");
             CommissioningStageComplete(CHIP_ERROR_INVALID_ARGUMENT);
             return;
         }
         if (ValidateAttestationInfo(params.GetAttestationElements().Value(), params.GetAttestationSignature().Value(),
                                     params.GetAttestationNonce().Value(), params.GetPAI().Value(), params.GetDAC().Value(),
-                                    params.GetRemoteVendorId().Value(), params.GetRemoteProductId().Value(),
                                     proxy) != CHIP_NO_ERROR)
         {
             ChipLogError(Controller, "Error validating attestation information");

src/controller/CHIPDeviceController.h

@@ -591,13 +591,11 @@ class DLL_EXPORT DeviceCommissioner : public DeviceController,
      * @param[in] attestationNonce    Attestation nonce
      * @param[in] pai                 PAI certificate
      * @param[in] dac                 DAC certificates
-     * @param[in] remoteVendorId      vendor ID read from the device
-     * @param[in] remoteProductId     product ID read from the device
      * @param[in] proxy               device proxy that is being attested.
      */
     CHIP_ERROR ValidateAttestationInfo(const ByteSpan & attestationElements, const ByteSpan & signature,
                                        const ByteSpan & attestationNonce, const ByteSpan & pai, const ByteSpan & dac,
-                                       VendorId remoteVendorId, uint16_t remoteProductId, DeviceProxy * proxy);
+                                       DeviceProxy * proxy);
 
     /**
      * @brief

src/controller/CommissioningDelegate.h

@@ -97,8 +97,6 @@ class CommissioningParameters
     const Optional<ByteSpan> GetAttestationSignature() const { return mAttestationSignature; }
     const Optional<ByteSpan> GetPAI() const { return mPAI; }
     const Optional<ByteSpan> GetDAC() const { return mDAC; }
-    const Optional<VendorId> GetRemoteVendorId() const { return mRemoteVendorId; }
-    const Optional<uint16_t> GetRemoteProductId() const { return mRemoteProductId; }
     CHIP_ERROR GetCompletionStatus() { return completionStatus; }
 
     CommissioningParameters & SetFailsafeTimerSeconds(uint16_t seconds)
@@ -187,16 +185,6 @@ class CommissioningParameters
         mDAC = MakeOptional(dac);
         return *this;
     }
-    CommissioningParameters & SetRemoteVendorId(VendorId id)
-    {
-        mRemoteVendorId = MakeOptional(id);
-        return *this;
-    }
-    CommissioningParameters & SetRemoteProductId(uint16_t id)
-    {
-        mRemoteProductId = MakeOptional(id);
-        return *this;
-    }
     void SetCompletionStatus(CHIP_ERROR err) { completionStatus = err; }
 
 private:
@@ -215,8 +203,6 @@ class CommissioningParameters
     Optional<ByteSpan> mAttestationSignature;
     Optional<ByteSpan> mPAI;
     Optional<ByteSpan> mDAC;
-    Optional<VendorId> mRemoteVendorId;
-    Optional<uint16_t> mRemoteProductId;
     CHIP_ERROR completionStatus = CHIP_NO_ERROR;
 };
 

src/credentials/DeviceAttestationVerifier.cpp

@@ -32,8 +32,7 @@ class UnimplementedDACVerifier : public DeviceAttestationVerifier
 public:
     void VerifyAttestationInformation(const ByteSpan & attestationInfoBuffer, const ByteSpan & attestationChallengeBuffer,
                                       const ByteSpan & attestationSignatureBuffer, const ByteSpan & paiDerBuffer,
-                                      const ByteSpan & dacDerBuffer, const ByteSpan & attestationNonce, VendorId vendorId,
-                                      uint16_t productId,
+                                      const ByteSpan & dacDerBuffer, const ByteSpan & attestationNonce,
                                       Callback::Callback<OnAttestationInformationVerification> * onCompletion) override
     {
         (void) attestationInfoBuffer;

src/credentials/DeviceAttestationVerifier.h

@@ -213,8 +213,7 @@ class DeviceAttestationVerifier
      */
     virtual void VerifyAttestationInformation(const ByteSpan & attestationInfoBuffer, const ByteSpan & attestationChallengeBuffer,
                                               const ByteSpan & attestationSignatureBuffer, const ByteSpan & paiDerBuffer,
-                                              const ByteSpan & dacDerBuffer, const ByteSpan & attestationNonce, VendorId vendorId,
-                                              uint16_t productId,
+                                              const ByteSpan & dacDerBuffer, const ByteSpan & attestationNonce,
                                               Callback::Callback<OnAttestationInformationVerification> * onCompletion) = 0;
 
     /**

src/credentials/examples/DefaultDeviceAttestationVerifier.cpp

@@ -150,8 +150,7 @@ class DefaultDACVerifier : public DeviceAttestationVerifier
 
     void VerifyAttestationInformation(const ByteSpan & attestationInfoBuffer, const ByteSpan & attestationChallengeBuffer,
                                       const ByteSpan & attestationSignatureBuffer, const ByteSpan & paiDerBuffer,
-                                      const ByteSpan & dacDerBuffer, const ByteSpan & attestationNonce, VendorId vendorId,
-                                      uint16_t productId,
+                                      const ByteSpan & dacDerBuffer, const ByteSpan & attestationNonce,
                                       Callback::Callback<OnAttestationInformationVerification> * onCompletion) override;
 
     AttestationVerificationResult ValidateCertificationDeclarationSignature(const ByteSpan & cmsEnvelopeBuffer,
@@ -176,7 +175,6 @@ void DefaultDACVerifier::VerifyAttestationInformation(const ByteSpan & attestati
                                                       const ByteSpan & attestationChallengeBuffer,
                                                       const ByteSpan & attestationSignatureBuffer, const ByteSpan & paiDerBuffer,
                                                       const ByteSpan & dacDerBuffer, const ByteSpan & attestationNonce,
-                                                      VendorId vendorId, uint16_t productId,
                                                       Callback::Callback<OnAttestationInformationVerification> * onCompletion)
 {
     AttestationVerificationResult attestationError = AttestationVerificationResult::kSuccess;
@@ -273,8 +271,8 @@ void DefaultDACVerifier::VerifyAttestationInformation(const ByteSpan & attestati
         ByteSpan certificationDeclarationPayload;
 
         DeviceInfoForAttestation deviceInfo{
-            .vendorId    = vendorId,
-            .productId   = productId,
+            .vendorId    = 0xFFF1,
+            .productId   = 0x8000, // TODO: Retrieve vendorId and ProductId from Basic Information Cluster
             .dacVendorId = dacVendorId,
             .paiVendorId = dacVendorId,
         };

src/credentials/tests/TestDeviceAttestationCredentials.cpp

@@ -226,8 +226,7 @@ static void TestDACVerifierExample_AttestationInfoVerification(nlTestSuite * inS
 
     default_verifier->VerifyAttestationInformation(
         ByteSpan(attestationElementsTestVector), ByteSpan(attestationChallengeTestVector), ByteSpan(attestationSignatureTestVector),
-        pai_span, dac_span, ByteSpan(attestationNonceTestVector), static_cast<VendorId>(0xFFF1), 0x8000,
-        &attestationInformationVerificationCallback);
+        pai_span, dac_span, ByteSpan(attestationNonceTestVector), &attestationInformationVerificationCallback);
     NL_TEST_ASSERT(inSuite, attestationResult == AttestationVerificationResult::kSuccess);
 }