Implement User Directed Commissioning - Phase 1

src/app/server/Mdns.cpp

@@ -21,6 +21,7 @@
 
 #include <core/Optional.h>
 #include <mdns/Advertiser.h>
+#include <mdns/ServiceNaming.h>
 #include <messaging/ReliableMessageProtocolConfig.h>
 #if CONFIG_DEVICE_LAYER
 #include <platform/CHIPDeviceLayer.h>
@@ -87,6 +88,12 @@ chip::ByteSpan FillMAC(uint8_t (&mac)[8])
 
 } // namespace
 
+CHIP_ERROR GetCommissionableInstanceName(char * buffer, size_t bufferLen)
+{
+    auto & mdnsAdvertiser = chip::Mdns::ServiceAdvertiser::Instance();
+    return mdnsAdvertiser.GetCommissionableInstanceName(buffer, bufferLen);
+}
+
 /// Set MDNS operational advertisement
 CHIP_ERROR AdvertiseOperational()
 {

src/app/server/Mdns.h

@@ -42,6 +42,9 @@ void StartServer();
 
 CHIP_ERROR GenerateRotatingDeviceId(char rotatingDeviceIdHexBuffer[], size_t rotatingDeviceIdHexBufferSize);
 
+/// Generates the (random) instance name that a CHIP device is to use for pre-commissioning DNS-SD
+CHIP_ERROR GetCommissionableInstanceName(char * buffer, size_t bufferLen);
+
 } // namespace Mdns
 } // namespace app
 } // namespace chip

src/controller/python/ChipDeviceController-ScriptBinding.cpp

@@ -252,7 +252,7 @@ CHIP_ERROR pychip_DeviceController_ConnectIP(chip::Controller::DeviceCommissione
 
 CHIP_ERROR pychip_DeviceController_DiscoverAllCommissionableNodes(chip::Controller::DeviceCommissioner * devCtrl)
 {
-    Mdns::DiscoveryFilter filter(Mdns::DiscoveryFilterType::kNone, 0);
+    Mdns::DiscoveryFilter filter(Mdns::DiscoveryFilterType::kNone, (uint16_t) 0);
     return devCtrl->DiscoverCommissionableNodes(filter);
 }
 

src/lib/mdns/Advertiser.h

@@ -25,6 +25,7 @@
 #include <core/PeerId.h>
 #include <inet/InetLayer.h>
 #include <lib/support/Span.h>
+#include <support/ChipMemString.h>
 
 namespace chip {
 namespace Mdns {
@@ -179,34 +180,55 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams<CommissionA
     {
         if (deviceName.HasValue())
         {
-            strncpy(sDeviceName, deviceName.Value(), min(strlen(deviceName.Value()) + 1, sizeof(sDeviceName)));
-            mDeviceName = Optional<const char *>::Value(static_cast<const char *>(sDeviceName));
+            chip::Platform::CopyString(mDeviceName, sizeof(mDeviceName), deviceName.Value());
+            mDeviceNameHasValue = true;
+        }
+        else
+        {
+            mDeviceNameHasValue = false;
         }
         return *this;
     }
-    Optional<const char *> GetDeviceName() const { return mDeviceName; }
+    Optional<const char *> GetDeviceName() const
+    {
+        return mDeviceNameHasValue ? Optional<const char *>::Value(mDeviceName) : Optional<const char *>::Missing();
+    }
 
     CommissionAdvertisingParameters & SetRotatingId(Optional<const char *> rotatingId)
     {
         if (rotatingId.HasValue())
         {
-            strncpy(sRotatingId, rotatingId.Value(), min(strlen(rotatingId.Value()) + 1, sizeof(sRotatingId)));
-            mRotatingId = Optional<const char *>::Value(static_cast<const char *>(sRotatingId));
+            chip::Platform::CopyString(mRotatingId, sizeof(mRotatingId), rotatingId.Value());
+            mRotatingIdHasValue = true;
+        }
+        else
+        {
+            mRotatingIdHasValue = false;
         }
         return *this;
     }
-    Optional<const char *> GetRotatingId() const { return mRotatingId; }
+    Optional<const char *> GetRotatingId() const
+    {
+        return mRotatingIdHasValue ? Optional<const char *>::Value(mRotatingId) : Optional<const char *>::Missing();
+    }
 
     CommissionAdvertisingParameters & SetPairingInstr(Optional<const char *> pairingInstr)
     {
         if (pairingInstr.HasValue())
         {
-            strncpy(sPairingInstr, pairingInstr.Value(), min(strlen(pairingInstr.Value()) + 1, sizeof(sPairingInstr)));
-            mPairingInstr = Optional<const char *>::Value(static_cast<const char *>(sPairingInstr));
+            chip::Platform::CopyString(mPairingInstr, sizeof(mPairingInstr), pairingInstr.Value());
+            mPairingInstrHasValue = true;
+        }
+        else
+        {
+            mPairingInstrHasValue = false;
         }
         return *this;
     }
-    Optional<const char *> GetPairingInstr() const { return mPairingInstr; }
+    Optional<const char *> GetPairingInstr() const
+    {
+        return mPairingInstrHasValue ? Optional<const char *>::Value(mPairingInstr) : Optional<const char *>::Missing();
+    }
 
     CommissionAdvertisingParameters & SetPairingHint(Optional<uint16_t> pairingHint)
     {
@@ -233,14 +255,14 @@ class CommissionAdvertisingParameters : public BaseAdvertisingParams<CommissionA
     chip::Optional<uint16_t> mDeviceType;
     chip::Optional<uint16_t> mPairingHint;
 
-    char sDeviceName[kKeyDeviceNameMaxLength + 1];
-    chip::Optional<const char *> mDeviceName;
+    char mDeviceName[kKeyDeviceNameMaxLength + 1];
+    bool mDeviceNameHasValue = false;
 
-    char sRotatingId[kKeyRotatingIdMaxLength + 1];
-    chip::Optional<const char *> mRotatingId;
+    char mRotatingId[kKeyRotatingIdMaxLength + 1];
+    bool mRotatingIdHasValue = false;
 
-    char sPairingInstr[kKeyPairingInstructionMaxLength + 1];
-    chip::Optional<const char *> mPairingInstr;
+    char mPairingInstr[kKeyPairingInstructionMaxLength + 1];
+    bool mPairingInstrHasValue = false;
 };
 
 /// Handles advertising of CHIP nodes
@@ -264,6 +286,9 @@ class ServiceAdvertiser
 
     /// Provides the system-wide implementation of the service advertiser
     static ServiceAdvertiser & Instance();
+
+    /// Returns DNS-SD instance name formatted as hex string
+    virtual CHIP_ERROR GetCommissionableInstanceName(char * instanceName, size_t maxLength) = 0;
 };
 
 } // namespace Mdns

src/lib/mdns/Advertiser_ImplMinimalMdns.cpp

@@ -113,6 +113,7 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
     CHIP_ERROR Advertise(const OperationalAdvertisingParameters & params) override;
     CHIP_ERROR Advertise(const CommissionAdvertisingParameters & params) override;
     CHIP_ERROR StopPublishDevice() override;
+    CHIP_ERROR GetCommissionableInstanceName(char * instanceName, size_t maxLength) override;
 
     // MdnsPacketDelegate
     void OnMdnsPacketData(const BytesRange & data, const chip::Inet::IPPacketInfo * info) override;
@@ -138,6 +139,8 @@ class AdvertiserMinMdns : public ServiceAdvertiser,
     QueryResponderAllocator<kMaxRecords> mQueryResponderAllocator;
 
     ResponseSender mResponseSender;
+    uint32_t mCommissionInstanceName1;
+    uint32_t mCommissionInstanceName2;
 
     // current request handling
     const chip::Inet::IPPacketInfo * mCurrentSource = nullptr;
@@ -183,6 +186,11 @@ CHIP_ERROR AdvertiserMinMdns::Start(chip::Inet::InetLayer * inetLayer, uint16_t
 {
     GlobalMinimalMdnsServer::Server().Shutdown();
 
+    mQueryResponderAllocator.Clear();
+
+    mCommissionInstanceName1 = GetRandU32();
+    mCommissionInstanceName2 = GetRandU32();
+
     ReturnErrorOnFailure(GlobalMinimalMdnsServer::Instance().StartServer(inetLayer, port));
 
     ChipLogProgress(Discovery, "CHIP minimal mDNS started advertising.");
@@ -201,7 +209,6 @@ CHIP_ERROR AdvertiserMinMdns::StopPublishDevice()
 
 CHIP_ERROR AdvertiserMinMdns::Advertise(const OperationalAdvertisingParameters & params)
 {
-    mQueryResponderAllocator.Clear();
     char nameBuffer[64] = "";
 
     /// need to set server name
@@ -265,16 +272,25 @@ CHIP_ERROR AdvertiserMinMdns::Advertise(const OperationalAdvertisingParameters &
     return CHIP_NO_ERROR;
 }
 
-CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & params)
+CHIP_ERROR AdvertiserMinMdns::GetCommissionableInstanceName(char * instanceName, size_t maxLength)
 {
-    mQueryResponderAllocator.Clear();
-    // TODO: need to detect colisions here
-    char nameBuffer[64] = "";
-    size_t len          = snprintf(nameBuffer, sizeof(nameBuffer), ChipLogFormatX64, GetRandU32(), GetRandU32());
-    if (len >= sizeof(nameBuffer))
+    if (maxLength < (kMaxInstanceNameSize + 1))
     {
         return CHIP_ERROR_NO_MEMORY;
     }
+    size_t len = snprintf(instanceName, maxLength, ChipLogFormatX64, mCommissionInstanceName1, mCommissionInstanceName2);
+    if (len >= maxLength)
+    {
+        return CHIP_ERROR_NO_MEMORY;
+    }
+    return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR AdvertiserMinMdns::Advertise(const CommissionAdvertisingParameters & params)
+{
+    char nameBuffer[64] = "";
+    ReturnErrorOnFailure(GetCommissionableInstanceName(nameBuffer, sizeof(nameBuffer)));
+
     const char * serviceType = params.GetCommissionAdvertiseMode() == CommssionAdvertiseMode::kCommissionableNode
         ? kCommissionableServiceName
         : kCommissionerServiceName;
@@ -457,26 +473,26 @@ FullQName AdvertiserMinMdns::GetCommisioningTextEntries(const CommissionAdvertis
     char txtVidPid[chip::Mdns::kKeyVendorProductMaxLength + 4];
     if (params.GetProductId().HasValue() && params.GetVendorId().HasValue())
     {
-        sprintf(txtVidPid, "VP=%d+%d", params.GetVendorId().Value(), params.GetProductId().Value());
+        snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d+%d", params.GetVendorId().Value(), params.GetProductId().Value());
         txtFields[numTxtFields++] = txtVidPid;
     }
     else if (params.GetVendorId().HasValue())
     {
-        sprintf(txtVidPid, "VP=%d", params.GetVendorId().Value());
+        snprintf(txtVidPid, sizeof(txtVidPid), "VP=%d", params.GetVendorId().Value());
         txtFields[numTxtFields++] = txtVidPid;
     }
 
     char txtDeviceType[chip::Mdns::kKeyDeviceTypeMaxLength + 4];
     if (params.GetDeviceType().HasValue())
     {
-        sprintf(txtDeviceType, "DT=%d", params.GetDeviceType().Value());
+        snprintf(txtDeviceType, sizeof(txtDeviceType), "DT=%d", params.GetDeviceType().Value());
         txtFields[numTxtFields++] = txtDeviceType;
     }
 
     char txtDeviceName[chip::Mdns::kKeyDeviceNameMaxLength + 4];
     if (params.GetDeviceName().HasValue())
     {
-        sprintf(txtDeviceName, "DN=%s", params.GetDeviceName().Value());
+        snprintf(txtDeviceName, sizeof(txtDeviceType), "DN=%s", params.GetDeviceName().Value());
         txtFields[numTxtFields++] = txtDeviceName;
     }
 
@@ -485,7 +501,7 @@ FullQName AdvertiserMinMdns::GetCommisioningTextEntries(const CommissionAdvertis
     {
         // a discriminator always exists
         char txtDiscriminator[chip::Mdns::kKeyDiscriminatorMaxLength + 3];
-        sprintf(txtDiscriminator, "D=%d", params.GetLongDiscriminator());
+        snprintf(txtDiscriminator, sizeof(txtDiscriminator), "D=%d", params.GetLongDiscriminator());
         txtFields[numTxtFields++] = txtDiscriminator;
 
         if (!params.GetVendorId().HasValue())
@@ -494,34 +510,34 @@ FullQName AdvertiserMinMdns::GetCommisioningTextEntries(const CommissionAdvertis
         }
 
         char txtCommissioningMode[chip::Mdns::kKeyCommissioningModeMaxLength + 4];
-        sprintf(txtCommissioningMode, "CM=%d", params.GetCommissioningMode() ? 1 : 0);
+        snprintf(txtCommissioningMode, sizeof(txtCommissioningMode), "CM=%d", params.GetCommissioningMode() ? 1 : 0);
         txtFields[numTxtFields++] = txtCommissioningMode;
 
         char txtOpenWindowCommissioningMode[chip::Mdns::kKeyAdditionalPairingMaxLength + 4];
         if (params.GetCommissioningMode() && params.GetOpenWindowCommissioningMode())
         {
-            sprintf(txtOpenWindowCommissioningMode, "AP=1");
+            snprintf(txtOpenWindowCommissioningMode, sizeof(txtOpenWindowCommissioningMode), "AP=1");
             txtFields[numTxtFields++] = txtOpenWindowCommissioningMode;
         }
 
         char txtRotatingDeviceId[chip::Mdns::kKeyRotatingIdMaxLength + 4];
         if (params.GetRotatingId().HasValue())
         {
-            sprintf(txtRotatingDeviceId, "RI=%s", params.GetRotatingId().Value());
+            snprintf(txtRotatingDeviceId, sizeof(txtRotatingDeviceId), "RI=%s", params.GetRotatingId().Value());
             txtFields[numTxtFields++] = txtRotatingDeviceId;
         }
 
         char txtPairingHint[chip::Mdns::kKeyPairingInstructionMaxLength + 4];
         if (params.GetPairingHint().HasValue())
         {
-            sprintf(txtPairingHint, "PH=%d", params.GetPairingHint().Value());
+            snprintf(txtPairingHint, sizeof(txtPairingHint), "PH=%d", params.GetPairingHint().Value());
             txtFields[numTxtFields++] = txtPairingHint;
         }
 
         char txtPairingInstr[chip::Mdns::kKeyPairingInstructionMaxLength + 4];
         if (params.GetPairingInstr().HasValue())
         {
-            sprintf(txtPairingInstr, "PI=%s", params.GetPairingInstr().Value());
+            snprintf(txtPairingInstr, sizeof(txtPairingInstr), "PI=%s", params.GetPairingInstr().Value());
             txtFields[numTxtFields++] = txtPairingInstr;
         }
     }

src/lib/mdns/Discovery_ImplPlatform.cpp

@@ -103,6 +103,21 @@ void DiscoveryImplPlatform::HandleMdnsError(void * context, CHIP_ERROR error)
     }
 }
 
+CHIP_ERROR DiscoveryImplPlatform::GetCommissionableInstanceName(char * instanceName, size_t maxLength)
+{
+    if (max_length < (chip::Mdns::kMaxInstanceNameSize + 1))
+    {
+        return CHIP_ERROR_NO_MEMORY;
+    }
+    size_t len = snprintf(instanceName, maxLength, "%08" PRIX32 "%08" PRIX32, static_cast<uint32_t>(mCommissionInstanceName >> 32),
+                          static_cast<uint32_t>(mCommissionInstanceName));
+    if (len >= maxLength)
+    {
+        return CHIP_ERROR_NO_MEMORY;
+    }
+    return CHIP_NO_ERROR;
+}
+
 CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameters & params)
 {
     CHIP_ERROR error = CHIP_NO_ERROR;
@@ -143,8 +158,8 @@ CHIP_ERROR DiscoveryImplPlatform::Advertise(const CommissionAdvertisingParameter
         return error;
     }
 
-    snprintf(service.mName, sizeof(service.mName), "%08" PRIX32 "%08" PRIX32, static_cast<uint32_t>(mCommissionInstanceName >> 32),
-             static_cast<uint32_t>(mCommissionInstanceName));
+    ReturnErrorOnFailure(GetCommissionableInstanceName(service.mName, sizeof(service.mName)));
+
     if (params.GetCommissionAdvertiseMode() == CommssionAdvertiseMode::kCommissionableNode)
     {
         strncpy(service.mType, kCommissionableServiceName, sizeof(service.mType));

src/lib/mdns/Discovery_ImplPlatform.h

@@ -48,6 +48,9 @@ class DiscoveryImplPlatform : public ServiceAdvertiser, public Resolver
     /// This function stops publishing the device on mDNS.
     CHIP_ERROR StopPublishDevice() override;
 
+    /// Returns DNS-SD instance name formatted as hex string
+    CHIP_ERROR GetCommissionableInstanceName(char * instanceName, size_t maxLength) override;
+
     /// Registers a resolver delegate if none has been registered before
     CHIP_ERROR SetResolverDelegate(ResolverDelegate * delegate) override;
 

src/lib/mdns/Resolver.h

@@ -52,13 +52,16 @@ struct ResolvedNodeData
 constexpr size_t kMaxDeviceNameLen         = 32;
 constexpr size_t kMaxRotatingIdLen         = 50;
 constexpr size_t kMaxPairingInstructionLen = 128;
+
+// Largest host name is 64-bits in hex.
+static constexpr int kMaxHostNameSize     = 16;
+static constexpr int kMaxInstanceNameSize = 16;
 struct DiscoveredNodeData
 {
     // TODO(cecille): is 4 OK? IPv6 LL, GUA, ULA, IPv4?
     static constexpr int kMaxIPAddresses = 5;
-    // Largest host name is 64-bits in hex.
-    static constexpr int kHostNameSize = 16;
-    char hostName[kHostNameSize + 1];
+    char hostName[kMaxHostNameSize + 1];
+    char instanceName[kMaxInstanceNameSize + 1];
     uint16_t longDiscriminator;
     uint16_t vendorId;
     uint16_t productId;
@@ -76,6 +79,7 @@ struct DiscoveredNodeData
     void Reset()
     {
         memset(hostName, 0, sizeof(hostName));
+        memset(instanceName, 0, sizeof(instanceName));
         longDiscriminator = 0;
         vendorId          = 0;
         productId         = 0;
@@ -107,14 +111,17 @@ enum class DiscoveryFilterType : uint8_t
     kDeviceType,
     kCommissioningMode,
     kCommissioningModeFromCommand,
+    kInstanceName,
     kCommissioner
 };
 struct DiscoveryFilter
 {
     DiscoveryFilterType type;
     uint16_t code;
+    char * instanceName;
     DiscoveryFilter() : type(DiscoveryFilterType::kNone), code(0) {}
     DiscoveryFilter(DiscoveryFilterType newType, uint16_t newCode) : type(newType), code(newCode) {}
+    DiscoveryFilter(DiscoveryFilterType newType, char * newInstanceName) : type(newType), instanceName(newInstanceName) {}
 };
 enum class DiscoveryType
 {