From 36d08bb4ca8f00ce729a3c328798093de76badf3 Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Wed, 5 Oct 2022 00:47:30 -0400
Subject: [PATCH] Filter by vendor/product id (if available) in
 SetUpCodePairer. (#23019)

* Filter by vendor/product id (if available) in SetUpCodePairer.

If we have vendor/product information, from a QR code or long manual code, we
should use that to filter out advertisements for other vendors or products.

* Address review comment.
---
 src/controller/SetUpCodePairer.cpp | 42 +++++++++++++++++++++++-------
 src/controller/SetUpCodePairer.h   | 10 ++++++-
 2 files changed, 42 insertions(+), 10 deletions(-)

diff --git a/src/controller/SetUpCodePairer.cpp b/src/controller/SetUpCodePairer.cpp
index 3a7f218ea4821b..2f13f629a3e680 100644
--- a/src/controller/SetUpCodePairer.cpp
+++ b/src/controller/SetUpCodePairer.cpp
@@ -163,17 +163,20 @@ CHIP_ERROR SetUpCodePairer::StartDiscoverOverIP(SetupPayload & payload)
     auto & discriminator = payload.discriminator;
     if (discriminator.IsShortDiscriminator())
     {
-        currentFilter.type = Dnssd::DiscoveryFilterType::kShortDiscriminator;
-        currentFilter.code = discriminator.GetShortValue();
+        mCurrentFilter.type = Dnssd::DiscoveryFilterType::kShortDiscriminator;
+        mCurrentFilter.code = discriminator.GetShortValue();
     }
     else
     {
-        currentFilter.type = Dnssd::DiscoveryFilterType::kLongDiscriminator;
-        currentFilter.code = discriminator.GetLongValue();
+        mCurrentFilter.type = Dnssd::DiscoveryFilterType::kLongDiscriminator;
+        mCurrentFilter.code = discriminator.GetLongValue();
     }
+    mPayloadVendorID  = payload.vendorID;
+    mPayloadProductID = payload.productID;
+
     // Handle possibly-sync callbacks.
     mWaitingForDiscovery[kIPTransport] = true;
-    CHIP_ERROR err                     = mCommissioner->DiscoverCommissionableNodes(currentFilter);
+    CHIP_ERROR err                     = mCommissioner->DiscoverCommissionableNodes(mCurrentFilter);
     if (err != CHIP_NO_ERROR)
     {
         mWaitingForDiscovery[kIPTransport] = false;
@@ -186,7 +189,9 @@ CHIP_ERROR SetUpCodePairer::StopConnectOverIP()
     ChipLogDetail(Controller, "Stopping commissioning discovery over DNS-SD");
 
     mWaitingForDiscovery[kIPTransport] = false;
-    currentFilter.type                 = Dnssd::DiscoveryFilterType::kNone;
+    mCurrentFilter.type                = Dnssd::DiscoveryFilterType::kNone;
+    mPayloadVendorID                   = kNotAvailable;
+    mPayloadProductID                  = kNotAvailable;
 
     mCommissioner->StopCommissionableDiscovery();
     return CHIP_NO_ERROR;
@@ -286,6 +291,11 @@ void SetUpCodePairer::OnBLEDiscoveryError(CHIP_ERROR err)
 }
 #endif // CONFIG_NETWORK_LAYER_BLE
 
+bool SetUpCodePairer::IdIsPresent(uint16_t vendorOrProductID)
+{
+    return vendorOrProductID != kNotAvailable;
+}
+
 bool SetUpCodePairer::NodeMatchesCurrentFilter(const Dnssd::DiscoveredNodeData & nodeData) const
 {
     if (nodeData.commissionData.commissioningMode == 0)
@@ -293,12 +303,26 @@ bool SetUpCodePairer::NodeMatchesCurrentFilter(const Dnssd::DiscoveredNodeData &
         return false;
     }
 
-    switch (currentFilter.type)
+    // The advertisement may not include a vendor id.
+    if (IdIsPresent(mPayloadVendorID) && IdIsPresent(nodeData.commissionData.vendorId) &&
+        mPayloadVendorID != nodeData.commissionData.vendorId)
+    {
+        return false;
+    }
+
+    // The advertisement may not include a product id.
+    if (IdIsPresent(mPayloadProductID) && IdIsPresent(nodeData.commissionData.productId) &&
+        mPayloadProductID != nodeData.commissionData.productId)
+    {
+        return false;
+    }
+
+    switch (mCurrentFilter.type)
     {
     case Dnssd::DiscoveryFilterType::kShortDiscriminator:
-        return ((nodeData.commissionData.longDiscriminator >> 8) & 0x0F) == currentFilter.code;
+        return ((nodeData.commissionData.longDiscriminator >> 8) & 0x0F) == mCurrentFilter.code;
     case Dnssd::DiscoveryFilterType::kLongDiscriminator:
-        return nodeData.commissionData.longDiscriminator == currentFilter.code;
+        return nodeData.commissionData.longDiscriminator == mCurrentFilter.code;
     default:
         return false;
     }
diff --git a/src/controller/SetUpCodePairer.h b/src/controller/SetUpCodePairer.h
index 6a1381c9089a9b..ed1c6f0c2ad5ac 100644
--- a/src/controller/SetUpCodePairer.h
+++ b/src/controller/SetUpCodePairer.h
@@ -151,7 +151,15 @@ class DLL_EXPORT SetUpCodePairer : public DevicePairingDelegate
 #endif // CONFIG_NETWORK_LAYER_BLE
 
     bool NodeMatchesCurrentFilter(const Dnssd::DiscoveredNodeData & nodeData) const;
-    Dnssd::DiscoveryFilter currentFilter;
+    static bool IdIsPresent(uint16_t vendorOrProductID);
+
+    Dnssd::DiscoveryFilter mCurrentFilter;
+    // The vendor id and product id from the SetupPayload.  They may be 0, which
+    // indicates "not available" (e.g. because the SetupPayload came from a
+    // short manual code).  In that case we should not filter on those values.
+    static constexpr uint16_t kNotAvailable = 0;
+    uint16_t mPayloadVendorID               = kNotAvailable;
+    uint16_t mPayloadProductID              = kNotAvailable;
 
     DeviceCommissioner * mCommissioner = nullptr;
     System::Layer * mSystemLayer       = nullptr;