Remove hardcoded node IDs for devices (#3817)

- The node ID was not configurable for Rendezvous state machine.  This change updates the initialization logic for Rendezvous (specifically on device side) to not expect a node ID at start. The state machine detects when the commissioner sends a valid node ID, and it bootstraps the secure channel and connections using this node ID.

- The peer node ID received in message header during spake2p handshake can not be used, as these messages are not encrypted and integrity protected. So the code now relies on messages that are received post spake2p handshake (which are encrypted and integrity protected). The node ID is updated only after the received message can be decrypted.

- Also, example apps and controllers had copies of hardcoded values (which are still useful when Rendezvous is bypassed, and test pairing is used). This PR moves the hardcoded values next to the test secret.

examples/all-clusters-app/esp32/main/EchoServer.cpp

@@ -232,17 +232,11 @@ SecureSessionMgrBase & SessionManager()
 }
 } // namespace chip
 
-void PairingComplete(SecurePairingSession * pairing)
-{
-    Optional<Transport::PeerAddress> peer(Transport::Type::kUndefined);
-    sessions.NewPairing(peer, pairing);
-}
-
 // The echo server assumes the platform's networking has been setup already
-void startServer()
+void startServer(NodeId localNodeId)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
-    err            = sessions.Init(kLocalNodeId, &DeviceLayer::SystemLayer,
+    err            = sessions.Init(localNodeId, &DeviceLayer::SystemLayer,
                         UdpListenParameters(&DeviceLayer::InetLayer).SetAddressType(kIPAddressType_IPv6).SetInterfaceId(NULL),
                         UdpListenParameters(&DeviceLayer::InetLayer).SetAddressType(kIPAddressType_IPv4));
     SuccessOrExit(err);
@@ -259,3 +253,10 @@ void startServer()
         ESP_LOGI(TAG, "Echo Server Listening...");
     }
 }
+
+void PairingComplete(NodeId assignedNodeId, NodeId peerNodeId, SecurePairingSession * pairing)
+{
+    Optional<Transport::PeerAddress> peer(Transport::Type::kUndefined);
+    startServer(assignedNodeId);
+    sessions.NewPairing(peer, peerNodeId, pairing);
+}

examples/all-clusters-app/esp32/main/Globals.cpp

@@ -21,4 +21,3 @@ LEDWidget statusLED1;
 LEDWidget statusLED2;
 BluetoothWidget bluetoothLED;
 WiFiWidget wifiLED;
-const chip::NodeId kLocalNodeId = 12344321;

examples/all-clusters-app/esp32/main/RendezvousDeviceDelegate.cpp

@@ -30,7 +30,7 @@ using namespace ::chip;
 using namespace ::chip::Inet;
 using namespace ::chip::System;
 
-extern void PairingComplete(SecurePairingSession * pairing);
+extern void PairingComplete(NodeId assignedNodeId, NodeId peerNodeId, SecurePairingSession * pairing);
 
 static const char * TAG = "rendezvous-devicedelegate";
 
@@ -43,7 +43,7 @@ RendezvousDeviceDelegate::RendezvousDeviceDelegate()
     err = DeviceLayer::ConfigurationMgr().GetSetupPinCode(setupPINCode);
     SuccessOrExit(err);
 
-    params.SetSetupPINCode(setupPINCode).SetLocalNodeId(kLocalNodeId).SetBleLayer(DeviceLayer::ConnectivityMgr().GetBleLayer());
+    params.SetSetupPINCode(setupPINCode).SetBleLayer(DeviceLayer::ConnectivityMgr().GetBleLayer());
 
     mRendezvousSession = chip::Platform::New<RendezvousSession>(this);
     err                = mRendezvousSession->Init(params);
@@ -55,6 +55,20 @@ RendezvousDeviceDelegate::RendezvousDeviceDelegate()
     }
 }
 
+void RendezvousDeviceDelegate::OnRendezvousComplete()
+{
+    ESP_LOGI(TAG, "Device completed Rendezvous process\n");
+    if (mRendezvousSession->GetLocalNodeId().HasValue() && mRendezvousSession->GetRemoteNodeId().HasValue())
+    {
+        PairingComplete(mRendezvousSession->GetLocalNodeId().Value(), mRendezvousSession->GetRemoteNodeId().Value(),
+                        &mRendezvousSession->GetPairingSession());
+    }
+    else
+    {
+        ESP_LOGI(TAG, "Commissioner did not assign a node ID to the device!!!\n");
+    }
+}
+
 void RendezvousDeviceDelegate::OnRendezvousStatusUpdate(RendezvousSessionDelegate::Status status, CHIP_ERROR err)
 {
     if (err != CHIP_NO_ERROR)
@@ -66,7 +80,6 @@ void RendezvousDeviceDelegate::OnRendezvousStatusUpdate(RendezvousSessionDelegat
     {
     case RendezvousSessionDelegate::SecurePairingSuccess:
         ESP_LOGI(TAG, "Device completed SPAKE2+ handshake\n");
-        PairingComplete(&mRendezvousSession->GetPairingSession());
         bluetoothLED.Set(true);
         break;
 

examples/all-clusters-app/esp32/main/include/RendezvousDeviceDelegate.h

@@ -26,6 +26,7 @@ class RendezvousDeviceDelegate : public chip::RendezvousSessionDelegate
     RendezvousDeviceDelegate();
 
     //////////// RendezvousSession callback Implementation ///////////////
+    void OnRendezvousComplete() override;
     void OnRendezvousStatusUpdate(RendezvousSessionDelegate::Status status, CHIP_ERROR err) override;
 
 private:

examples/all-clusters-app/esp32/main/main.cpp

@@ -56,8 +56,6 @@ using namespace ::chip;
 using namespace ::chip::DeviceManager;
 using namespace ::chip::DeviceLayer;
 
-extern void startServer();
-
 #define QRCODE_BASE_URL "https://dhrishi.github.io/connectedhomeip/qrcode.html"
 
 #if CONFIG_DEVICE_TYPE_M5STACK
@@ -84,7 +82,7 @@ extern void startServer();
 // Used to indicate that an IP address has been added to the QRCode
 #define EXAMPLE_VENDOR_TAG_IP 1
 
-extern void PairingComplete(SecurePairingSession * pairing);
+extern void PairingComplete(NodeId assignedNodeId, NodeId peerNodeId, SecurePairingSession * pairing);
 
 const char * TAG = "all-clusters-app";
 
@@ -529,7 +527,6 @@ extern "C" void app_main()
 
     // Start the Echo Server
     InitDataModelHandler();
-    startServer();
 
     if (isRendezvousBLE())
     {
@@ -538,7 +535,7 @@ extern "C" void app_main()
     else if (isRendezvousBypassed())
     {
         ChipLogProgress(Ble, "Rendezvous and Secure Pairing skipped. Using test secret.");
-        PairingComplete(&gTestPairing);
+        PairingComplete(chip::kTestDeviceNodeId, chip::kTestControllerNodeId, &gTestPairing);
     }
 
     std::string qrCodeText = createSetupPayload();

examples/chip-tool/main.cpp

@@ -21,11 +21,7 @@
 #include "commands/clusters/Commands.h"
 #include "commands/echo/Commands.h"
 
-// NOTE: Remote device ID is in sync with the echo server device id
-//       At some point, we may want to add an option to connect to a device without
-//       knowing its id, because the ID can be learned on the first response that is received.
-constexpr chip::NodeId kLocalDeviceId  = 112233;
-constexpr chip::NodeId kRemoteDeviceId = 12344321;
+#include <transport/SecurePairingSession.h>
 
 // ================================================================================
 // Main Code
@@ -37,5 +33,5 @@ int main(int argc, char * argv[])
     registerCommandsEcho(commands);
     registerClusters(commands);
 
-    return commands.Run(kLocalDeviceId, kRemoteDeviceId, argc, argv);
+    return commands.Run(chip::kTestControllerNodeId, chip::kTestDeviceNodeId, argc, argv);
 }

examples/common/chip-app-server/RendezvousServer.cpp

@@ -61,6 +61,13 @@ void RendezvousServer::OnRendezvousMessageReceived(PacketBuffer * buffer)
     chip::System::PacketBuffer::Free(buffer);
 }
 
+void RendezvousServer::OnRendezvousComplete()
+{
+    ChipLogProgress(AppServer, "Device completed Rendezvous process");
+    SessionManager().NewPairing(Optional<Transport::PeerAddress>{}, mRendezvousSession.GetRemoteNodeId().Value(),
+                                &mRendezvousSession.GetPairingSession());
+}
+
 void RendezvousServer::OnRendezvousStatusUpdate(Status status, CHIP_ERROR err)
 {
     VerifyOrExit(err == CHIP_NO_ERROR, ChipLogError(AppServer, "OnRendezvousStatusUpdate: %s", chip::ErrorStr(err)));
@@ -69,7 +76,6 @@ void RendezvousServer::OnRendezvousStatusUpdate(Status status, CHIP_ERROR err)
     {
     case RendezvousSessionDelegate::SecurePairingSuccess:
         ChipLogProgress(AppServer, "Device completed SPAKE2+ handshake");
-        SessionManager().NewPairing(Optional<Transport::PeerAddress>{}, &mRendezvousSession.GetPairingSession());
         break;
     case RendezvousSessionDelegate::NetworkProvisioningSuccess:
         ChipLogProgress(AppServer, "Device was assigned network credentials");

examples/common/chip-app-server/Server.cpp

@@ -40,10 +40,6 @@ using namespace ::chip::Inet;
 using namespace ::chip::Transport;
 using namespace ::chip::DeviceLayer;
 
-#ifndef EXAMPLE_SERVER_NODEID
-#define EXAMPLE_SERVER_NODEID 12344321
-#endif // EXAMPLE_SERVER_NODEID
-
 namespace {
 
 class ServerCallback : public SecureSessionMgrDelegate
@@ -104,7 +100,7 @@ void InitServer()
 
     InitDataModelHandler();
 
-    err = gSessions.Init(EXAMPLE_SERVER_NODEID, &DeviceLayer::SystemLayer,
+    err = gSessions.Init(chip::kTestDeviceNodeId, &DeviceLayer::SystemLayer,
                          UdpListenParameters(&DeviceLayer::InetLayer).SetAddressType(kIPAddressType_IPv6));
     SuccessOrExit(err);
 
@@ -117,13 +113,13 @@ void InitServer()
 
         SuccessOrExit(err = DeviceLayer::ConfigurationMgr().GetSetupPinCode(pinCode));
         params.SetSetupPINCode(pinCode)
-            .SetLocalNodeId(EXAMPLE_SERVER_NODEID)
+            .SetLocalNodeId(chip::kTestDeviceNodeId)
             .SetBleLayer(DeviceLayer::ConnectivityMgr().GetBleLayer());
         SuccessOrExit(err = gRendezvousServer.Init(params));
     }
 #endif
 
-    err = gSessions.NewPairing(peer, &gTestPairing);
+    err = gSessions.NewPairing(peer, chip::kTestControllerNodeId, &gTestPairing);
     SuccessOrExit(err);
 
     gSessions.SetDelegate(&gCallbacks);

examples/common/chip-app-server/include/RendezvousServer.h

@@ -35,6 +35,7 @@ class RendezvousServer : public RendezvousSessionDelegate
     void OnRendezvousConnectionClosed() override;
     void OnRendezvousError(CHIP_ERROR err) override;
     void OnRendezvousMessageReceived(System::PacketBuffer * buffer) override;
+    void OnRendezvousComplete() override;
     void OnRendezvousStatusUpdate(Status status, CHIP_ERROR err) override;
 
 private:

examples/temperature-measurement-app/esp32/main/RendezvousDeviceDelegate.cpp

@@ -28,7 +28,7 @@ using namespace ::chip::Inet;
 using namespace ::chip::System;
 
 extern NodeId kLocalNodeId;
-extern void PairingComplete(SecurePairingSession * pairing);
+extern void PairingComplete(NodeId assignedNodeId, NodeId peerNodeId, SecurePairingSession * pairing);
 
 static const char * TAG = "rendezvous-devicedelegate";
 
@@ -53,6 +53,20 @@ RendezvousDeviceDelegate::RendezvousDeviceDelegate()
     }
 }
 
+void RendezvousDeviceDelegate::OnRendezvousComplete()
+{
+    ESP_LOGI(TAG, "Device completed Rendezvous process\n");
+    if (mRendezvousSession->GetLocalNodeId().HasValue() && mRendezvousSession->GetRemoteNodeId().HasValue())
+    {
+        PairingComplete(mRendezvousSession->GetLocalNodeId().Value(), mRendezvousSession->GetRemoteNodeId().Value(),
+                        &mRendezvousSession->GetPairingSession());
+    }
+    else
+    {
+        ESP_LOGI(TAG, "Commissioner did not assign a node ID to the device!!!\n");
+    }
+}
+
 void RendezvousDeviceDelegate::OnRendezvousStatusUpdate(RendezvousSessionDelegate::Status status, CHIP_ERROR err)
 {
     if (err != CHIP_NO_ERROR)
@@ -64,7 +78,6 @@ void RendezvousDeviceDelegate::OnRendezvousStatusUpdate(RendezvousSessionDelegat
     {
     case RendezvousSessionDelegate::SecurePairingSuccess:
         ESP_LOGI(TAG, "Device completed SPAKE2+ handshake\n");
-        PairingComplete(&mRendezvousSession->GetPairingSession());
         break;
 
     case RendezvousSessionDelegate::NetworkProvisioningSuccess:

examples/temperature-measurement-app/esp32/main/ResponseServer.cpp

@@ -142,17 +142,11 @@ SecureSessionMgrBase & SessionManager()
 }
 } // namespace chip
 
-void PairingComplete(SecurePairingSession * pairing)
-{
-    Optional<Transport::PeerAddress> peer(Transport::Type::kUndefined);
-    sessions.NewPairing(peer, pairing);
-}
-
 // The echo server assumes the platform's networking has been setup already
-void startServer()
+void startServer(NodeId localNodeId)
 {
     CHIP_ERROR err = CHIP_NO_ERROR;
-    err            = sessions.Init(kLocalNodeId, &DeviceLayer::SystemLayer,
+    err            = sessions.Init(localNodeId, &DeviceLayer::SystemLayer,
                         UdpListenParameters(&DeviceLayer::InetLayer).SetAddressType(kIPAddressType_IPv6).SetInterfaceId(nullptr),
                         UdpListenParameters(&DeviceLayer::InetLayer).SetAddressType(kIPAddressType_IPv4));
     SuccessOrExit(err);
@@ -169,3 +163,10 @@ void startServer()
         ESP_LOGI(TAG, "Echo Server Listening...");
     }
 }
+
+void PairingComplete(NodeId assignedNodeId, NodeId peerNodeId, SecurePairingSession * pairing)
+{
+    Optional<Transport::PeerAddress> peer(Transport::Type::kUndefined);
+    startServer(assignedNodeId);
+    sessions.NewPairing(peer, peerNodeId, pairing);
+}

examples/temperature-measurement-app/esp32/main/include/RendezvousDeviceDelegate.h

@@ -27,6 +27,7 @@ class RendezvousDeviceDelegate : public chip::RendezvousSessionDelegate
     RendezvousDeviceDelegate();
 
     //////////// RendezvousSession callback Implementation ///////////////
+    void OnRendezvousComplete() override;
     void OnRendezvousStatusUpdate(RendezvousSessionDelegate::Status status, CHIP_ERROR err) override;
 
 private:

examples/temperature-measurement-app/esp32/main/main.cpp

@@ -44,8 +44,6 @@ using namespace ::chip;
 using namespace ::chip::DeviceManager;
 using namespace ::chip::DeviceLayer;
 
-extern void startServer();
-
 // Used to indicate that an IP address has been added to the QRCode
 #define EXAMPLE_VENDOR_TAG_IP 1
 
@@ -113,7 +111,6 @@ extern "C" void app_main()
     }
 
     InitDataModelHandler();
-    startServer();
 
     if (isRendezvousBLE())
     {
@@ -122,7 +119,7 @@ extern "C" void app_main()
     else if (isRendezvousBypassed())
     {
         ChipLogProgress(Ble, "Rendezvous and Secure Pairing skipped. Using test secret.");
-        PairingComplete(&gTestPairing);
+        PairingComplete(chip::kTestDeviceNodeId, chip::kTestControllerNodeId, &gTestPairing);
     }
 
     // Run the UI Loop

src/controller/CHIPDevice.cpp

@@ -190,7 +190,7 @@ CHIP_ERROR Device::LoadSecureSessionParameters()
     SuccessOrExit(err);
 
     err = mSessionManager->NewPairing(
-        Optional<Transport::PeerAddress>::Value(Transport::PeerAddress::UDP(mDeviceAddr, mDevicePort, mInterface)),
+        Optional<Transport::PeerAddress>::Value(Transport::PeerAddress::UDP(mDeviceAddr, mDevicePort, mInterface)), mDeviceId,
         &pairingSession);
     SuccessOrExit(err);
 

src/controller/CHIPDeviceController.cpp

@@ -467,7 +467,7 @@ CHIP_ERROR DeviceCommissioner::PairDevice(NodeId remoteDeviceId, RendezvousParam
 
     mRendezvousSession = chip::Platform::New<RendezvousSession>(this);
     VerifyOrExit(mRendezvousSession != nullptr, err = CHIP_ERROR_NO_MEMORY);
-    err = mRendezvousSession->Init(params.SetLocalNodeId(mLocalDeviceId));
+    err = mRendezvousSession->Init(params.SetLocalNodeId(mLocalDeviceId).SetRemoteNodeId(remoteDeviceId));
     SuccessOrExit(err);
 
     device->Init(mSessionManager, mInetLayer, remoteDeviceId, remotePort, interfaceId);

src/controller/java/CHIPDeviceController-JNI.cpp

@@ -146,8 +146,8 @@ class JniUtfString
 // NOTE: Remote device ID is in sync with the echo server device id
 // At some point, we may want to add an option to connect to a device without
 // knowing its id, because the ID can be learned on the first response that is received.
-chip::NodeId kLocalDeviceId  = 112233;
-chip::NodeId kRemoteDeviceId = 12344321;
+chip::NodeId kLocalDeviceId  = chip::kTestControllerNodeId;
+chip::NodeId kRemoteDeviceId = chip::kTestDeviceNodeId;
 
 jint JNI_OnLoad(JavaVM * jvm, void * reserved)
 {

src/controller/python/ChipDeviceController-ScriptBinding.cpp

@@ -129,8 +129,8 @@ static chip::Inet::InetLayer sInetLayer;
 // NOTE: Remote device ID is in sync with the echo server device id
 // At some point, we may want to add an option to connect to a device without
 // knowing its id, because the ID can be learned on the first response that is received.
-chip::NodeId kLocalDeviceId  = 112233;
-chip::NodeId kRemoteDeviceId = 12344321;
+chip::NodeId kLocalDeviceId  = chip::kTestControllerNodeId;
+chip::NodeId kRemoteDeviceId = chip::kTestDeviceNodeId;
 
 #if CONFIG_NETWORK_LAYER_BLE
 static BleLayer sBle;

src/darwin/Framework/CHIP/CHIPDeviceController.mm

@@ -35,8 +35,8 @@
 // NOTE: Remote device ID is in sync with the echo server device id
 //       At some point, we may want to add an option to connect to a device without
 //       knowing its id, because the ID can be learned on the first response that is received.
-constexpr chip::NodeId kLocalDeviceId = 112233;
-constexpr chip::NodeId kRemoteDeviceId = 12344321;
+constexpr chip::NodeId kLocalDeviceId = chip::kTestControllerNodeId;
+constexpr chip::NodeId kRemoteDeviceId = chip::kTestDeviceNodeId;
 
 @implementation AddressInfo
 - (instancetype)initWithIP:(NSString *)ip

src/darwin/Framework/CHIP/CHIPPersistentStorageDelegateBridge.mm

@@ -104,14 +104,18 @@
         valueString = [mDefaultPersistentStorage objectForKey:keyString];
     }
 
-    if (value != nullptr) {
-        size = strlcpy(value, [valueString UTF8String], size);
+    if (valueString != nil) {
+        if (value != nullptr) {
+            size = strlcpy(value, [valueString UTF8String], size);
+        } else {
+            size = [valueString length];
+        }
+        // Increment size to account for null termination
+        size += 1;
+        return CHIP_NO_ERROR;
     } else {
-        size = [valueString length];
+        return CHIP_ERROR_INVALID_ARGUMENT;
     }
-    // Increment size to account for null termination
-    size += 1;
-    return CHIP_NO_ERROR;
 }
 
 void CHIPPersistentStorageDelegateBridge::SetKeyValue(const char * key, const char * value)