Merge remote-tracking branch 'PeterC1965/whm-follow-up-part1' into whm-follow-up-part1

Author: jamesharrow

docs/ERROR_CODES.md

@@ -118,6 +118,7 @@ This file was **AUTOMATICALLY** generated by
 | 165       | 0xA5  | `CHIP_ERROR_ACCESS_DENIED`                         |
 | 166       | 0xA6  | `CHIP_ERROR_UNKNOWN_RESOURCE_ID`                   |
 | 167       | 0xA7  | `CHIP_ERROR_VERSION_MISMATCH`                      |
+| 168       | 0xA8  | `CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL`              |
 | 171       | 0xAB  | `CHIP_ERROR_EVENT_ID_FOUND`                        |
 | 172       | 0xAC  | `CHIP_ERROR_INTERNAL`                              |
 | 173       | 0xAD  | `CHIP_ERROR_OPEN_FAILED`                           |
@@ -252,6 +253,7 @@ This file was **AUTOMATICALLY** generated by
 | 1426      | 0x592 | `DATA_VERSION_MISMATCH`    |
 | 1428      | 0x594 | `TIMEOUT`                  |
 | 1436      | 0x59C | `BUSY`                     |
+| 1437      | 0x59D | `ACCESS_RESTRICTED`        |
 | 1475      | 0x5C3 | `UNSUPPORTED_CLUSTER`      |
 | 1477      | 0x5C5 | `NO_UPSTREAM_SUBSCRIPTION` |
 | 1478      | 0x5C6 | `NEEDS_TIMED_INTERACTION`  |

docs/guides/fabric_synchronization_guide.md

@@ -107,7 +107,7 @@ fabricsync add-local-bridge 1
 Pair the Ecosystem 2 bridge to Ecosystem 1 with node ID 2:
 
 ```
-fabricsync add-bridge 2 <e2-fabric-bridge-ip>
+fabricsync add-bridge 2 <setup-pin-code> <e2-fabric-bridge-ip> <e2-fabric-bridge-port>
 ```
 
 This command will initiate the reverse commissioning process. After a few

examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp

@@ -104,7 +104,7 @@ CHIP_ERROR FabricSyncAddBridgeCommand::RunCommand(NodeId remoteId)
     pairingCommand->RegisterCommissioningDelegate(this);
     mBridgeNodeId = remoteId;
 
-    DeviceMgr().PairRemoteFabricBridge(remoteId, reinterpret_cast<const char *>(mRemoteAddr.data()));
+    DeviceMgr().PairRemoteFabricBridge(remoteId, mSetupPINCode, reinterpret_cast<const char *>(mRemoteAddr.data()), mRemotePort);
 
     return CHIP_NO_ERROR;
 }
@@ -207,6 +207,15 @@ CHIP_ERROR FabricSyncAddLocalBridgeCommand::RunCommand(NodeId deviceId)
     pairingCommand->RegisterCommissioningDelegate(this);
     mLocalBridgeNodeId = deviceId;
 
+    if (mSetupPINCode.HasValue())
+    {
+        DeviceMgr().SetLocalBridgeSetupPinCode(mSetupPINCode.Value());
+    }
+    if (mLocalPort.HasValue())
+    {
+        DeviceMgr().SetLocalBridgePort(mLocalPort.Value());
+    }
+
     DeviceMgr().PairLocalFabricBridge(deviceId);
 
     return CHIP_NO_ERROR;

examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h

@@ -31,8 +31,10 @@ class FabricSyncAddBridgeCommand : public CHIPCommand, public CommissioningDeleg
 public:
     FabricSyncAddBridgeCommand(CredentialIssuerCommands * credIssuerCommands) : CHIPCommand("add-bridge", credIssuerCommands)
     {
-        AddArgument("nodeid", 0, UINT64_MAX, &mNodeId);
-        AddArgument("device-remote-ip", &mRemoteAddr);
+        AddArgument("node-id", 0, UINT64_MAX, &mNodeId);
+        AddArgument("setup-pin-code", 0, 0x7FFFFFF, &mSetupPINCode, "Setup PIN code for the remote bridge device.");
+        AddArgument("device-remote-ip", &mRemoteAddr, "The IP address of the remote bridge device.");
+        AddArgument("device-remote-port", 0, UINT16_MAX, &mRemotePort, "The secured device port of the remote bridge device.");
     }
 
     void OnCommissioningComplete(chip::NodeId deviceId, CHIP_ERROR err) override;
@@ -45,7 +47,9 @@ class FabricSyncAddBridgeCommand : public CHIPCommand, public CommissioningDeleg
 private:
     chip::NodeId mNodeId;
     chip::NodeId mBridgeNodeId;
+    uint32_t mSetupPINCode;
     chip::ByteSpan mRemoteAddr;
+    uint16_t mRemotePort;
 
     CHIP_ERROR RunCommand(NodeId remoteId);
 };
@@ -73,7 +77,9 @@ class FabricSyncAddLocalBridgeCommand : public CHIPCommand, public Commissioning
     FabricSyncAddLocalBridgeCommand(CredentialIssuerCommands * credIssuerCommands) :
         CHIPCommand("add-local-bridge", credIssuerCommands)
     {
-        AddArgument("nodeid", 0, UINT64_MAX, &mNodeId);
+        AddArgument("node-id", 0, UINT64_MAX, &mNodeId);
+        AddArgument("setup-pin-code", 0, 0x7FFFFFF, &mSetupPINCode, "Setup PIN code for the local bridge device.");
+        AddArgument("local-port", 0, UINT16_MAX, &mLocalPort, "The secured device port of the local bridge device.");
     }
 
     void OnCommissioningComplete(NodeId deviceId, CHIP_ERROR err) override;
@@ -85,6 +91,8 @@ class FabricSyncAddLocalBridgeCommand : public CHIPCommand, public Commissioning
 
 private:
     chip::NodeId mNodeId;
+    chip::Optional<uint32_t> mSetupPINCode;
+    chip::Optional<uint16_t> mLocalPort;
     chip::NodeId mLocalBridgeNodeId;
 
     CHIP_ERROR RunCommand(chip::NodeId deviceId);

examples/fabric-admin/device_manager/DeviceManager.cpp

@@ -30,10 +30,6 @@ using namespace chip::app::Clusters;
 
 namespace {
 
-// Constants
-constexpr uint32_t kSetupPinCode               = 20202021;
-constexpr uint16_t kRemoteBridgePort           = 5540;
-constexpr uint16_t kLocalBridgePort            = 5540;
 constexpr uint16_t kWindowTimeout              = 300;
 constexpr uint16_t kIteration                  = 1000;
 constexpr uint16_t kSubscribeMinInterval       = 0;
@@ -137,7 +133,7 @@ void DeviceManager::OpenRemoteDeviceCommissioningWindow(EndpointId remoteEndpoin
     // that is part of a different fabric, accessed through a fabric bridge.
     StringBuilder<kMaxCommandSize> commandBuilder;
 
-    // Use random discriminator to have less chance of collission.
+    // Use random discriminator to have less chance of collision.
     uint16_t discriminator =
         Crypto::GetRandU16() % (kMaxDiscriminatorLength + 1); // Include the upper limit kMaxDiscriminatorLength
 
@@ -148,12 +144,13 @@ void DeviceManager::OpenRemoteDeviceCommissioningWindow(EndpointId remoteEndpoin
     PushCommand(commandBuilder.c_str());
 }
 
-void DeviceManager::PairRemoteFabricBridge(NodeId nodeId, const char * deviceRemoteIp)
+void DeviceManager::PairRemoteFabricBridge(chip::NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp,
+                                           uint16_t deviceRemotePort)
 {
     StringBuilder<kMaxCommandSize> commandBuilder;
 
     commandBuilder.Add("pairing already-discovered ");
-    commandBuilder.AddFormat("%lu %d %s %d", nodeId, kSetupPinCode, deviceRemoteIp, kRemoteBridgePort);
+    commandBuilder.AddFormat("%lu %d %s %d", nodeId, setupPINCode, deviceRemoteIp, deviceRemotePort);
 
     PushCommand(commandBuilder.c_str());
 }
@@ -173,7 +170,7 @@ void DeviceManager::PairLocalFabricBridge(NodeId nodeId)
     StringBuilder<kMaxCommandSize> commandBuilder;
 
     commandBuilder.Add("pairing already-discovered ");
-    commandBuilder.AddFormat("%lu %d ::1 %d", nodeId, kSetupPinCode, kLocalBridgePort);
+    commandBuilder.AddFormat("%lu %d ::1 %d", nodeId, mLocalBridgeSetupPinCode, mLocalBridgePort);
 
     PushCommand(commandBuilder.c_str());
 }

examples/fabric-admin/device_manager/DeviceManager.h

@@ -24,6 +24,8 @@
 
 #include <set>
 
+constexpr uint32_t kDefaultSetupPinCode    = 20202021;
+constexpr uint16_t kDefaultLocalBridgePort = 5540;
 constexpr uint16_t kResponseTimeoutSeconds = 30;
 
 class Device
@@ -61,6 +63,8 @@ class DeviceManager : public PairingDelegate
 
     void SetRemoteBridgeNodeId(chip::NodeId nodeId) { mRemoteBridgeNodeId = nodeId; }
 
+    void SetLocalBridgePort(uint16_t port) { mLocalBridgePort = port; }
+    void SetLocalBridgeSetupPinCode(uint32_t pinCode) { mLocalBridgeSetupPinCode = pinCode; }
     void SetLocalBridgeNodeId(chip::NodeId nodeId) { mLocalBridgeNodeId = nodeId; }
 
     bool IsAutoSyncEnabled() const { return mAutoSyncEnabled; }
@@ -125,9 +129,11 @@ class DeviceManager : public PairingDelegate
 
      * @param nodeId            The user-defined ID for the node being commissioned. It doesn’t need to be the same ID,
      *                          as for the first fabric.
+     * @param setupPINCode      The setup PIN code used to authenticate the pairing process.
      * @param deviceRemoteIp    The IP address of the remote device that is being paired as part of the fabric bridge.
+     * @param deviceRemotePort  The secured device port of the remote device that is being paired as part of the fabric bridge.
      */
-    void PairRemoteFabricBridge(chip::NodeId nodeId, const char * deviceRemoteIp);
+    void PairRemoteFabricBridge(chip::NodeId nodeId, uint32_t setupPINCode, const char * deviceRemoteIp, uint16_t deviceRemotePort);
 
     /**
      * @brief Pair a remote Matter device to the current fabric.
@@ -190,6 +196,8 @@ class DeviceManager : public PairingDelegate
     // This represents the bridge on the other ecosystem.
     chip::NodeId mRemoteBridgeNodeId = chip::kUndefinedNodeId;
 
+    uint16_t mLocalBridgePort         = kDefaultLocalBridgePort;
+    uint32_t mLocalBridgeSetupPinCode = kDefaultSetupPinCode;
     // The Node ID of the local bridge used for Fabric-Sync
     // This represents the bridge within its own ecosystem.
     chip::NodeId mLocalBridgeNodeId = chip::kUndefinedNodeId;

examples/fabric-admin/device_manager/DeviceSynchronization.cpp

@@ -72,6 +72,13 @@ void DeviceSynchronizer::OnAttributeData(const ConcreteDataAttributePath & path,
     VerifyOrDie(path.mEndpointId == kRootEndpointId);
     VerifyOrDie(path.mClusterId == Clusters::BasicInformation::Id);
 
+    CHIP_ERROR error = status.ToChipError();
+    if (CHIP_NO_ERROR != error)
+    {
+        ChipLogError(NotSpecified, "Response Failure: %" CHIP_ERROR_FORMAT, error.Format());
+        return;
+    }
+
     switch (path.mAttributeId)
     {
     case Clusters::BasicInformation::Attributes::UniqueID::Id:

integrations/docker/images/base/chip-build/Dockerfile

@@ -123,6 +123,14 @@ RUN set -x \
     ruff \
     && : # last line
 
+#TODO Issue #35280: this is only added as a workaround to bloaty build failures, remove it once bloaty fixes issue
+# Clone and install abseil-cpp
+RUN git clone https://github.com/abseil/abseil-cpp.git /tmp/abseil-cpp \
+    && cd /tmp/abseil-cpp \
+    && cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/usr/local \
+    && cmake --build build --target install \
+    && rm -rf /tmp/abseil-cpp
+
 # Install bloat comparison tools
 RUN set -x \
     && git clone https://github.com/google/bloaty.git \

integrations/docker/images/chip-cert-bins/Dockerfile

@@ -193,6 +193,9 @@ RUN case ${TARGETPLATFORM} in \
     --target linux-x64-energy-management-ipv6only \
     --target linux-x64-microwave-oven-ipv6only \
     --target linux-x64-rvc-ipv6only \
+    --target linux-x64-fabric-bridge-rpc-ipv6only \
+    --target linux-x64-fabric-admin-rpc-ipv6only \
+    --target linux-x64-network-manager-ipv6only \
     build \
     && mv out/linux-x64-chip-tool-ipv6only-platform-mdns/chip-tool out/chip-tool \
     && mv out/linux-x64-shell-ipv6only-platform-mdns/chip-shell out/chip-shell \
@@ -213,6 +216,9 @@ RUN case ${TARGETPLATFORM} in \
     && mv out/linux-x64-energy-management-ipv6only/chip-energy-management-app out/chip-energy-management-app \
     && mv out/linux-x64-microwave-oven-ipv6only/chip-microwave-oven-app out/chip-microwave-oven-app \
     && mv out/linux-x64-rvc-ipv6only/chip-rvc-app out/chip-rvc-app \
+    && mv out/linux-x64-fabric-bridge-rpc-ipv6only/fabric-bridge-app out/fabric-bridge-app \
+    && mv out/linux-x64-fabric-admin-rpc-ipv6only/fabric-admin out/fabric-admin \
+    && mv out/linux-x64-network-manager-ipv6only/matter-network-manager-app out/matter-network-manager-app \
     ;; \
     "linux/arm64")\
     set -x \
@@ -237,6 +243,9 @@ RUN case ${TARGETPLATFORM} in \
     --target linux-arm64-energy-management-ipv6only \
     --target linux-arm64-microwave-oven-ipv6only \
     --target linux-arm64-rvc-ipv6only \
+    --target linux-arm64-fabric-bridge-rpc-ipv6only \
+    --target linux-arm64-fabric-admin-rpc-ipv6only \
+    --target linux-arm64-network-manager-ipv6only \
     build \
     && mv out/linux-arm64-chip-tool-ipv6only-platform-mdns/chip-tool out/chip-tool \
     && mv out/linux-arm64-shell-ipv6only-platform-mdns/chip-shell out/chip-shell \
@@ -257,6 +266,9 @@ RUN case ${TARGETPLATFORM} in \
     && mv out/linux-arm64-energy-management-ipv6only/chip-energy-management-app out/chip-energy-management-app \
     && mv out/linux-arm64-microwave-oven-ipv6only/chip-microwave-oven-app out/chip-microwave-oven-app \
     && mv out/linux-arm64-rvc-ipv6only/chip-rvc-app out/chip-rvc-app \
+    && mv out/linux-arm64-fabric-bridge-rpc-ipv6only/fabric-bridge-app out/fabric-bridge-app \
+    && mv out/linux-arm64-fabric-admin-rpc-ipv6only/fabric-admin out/fabric-admin \
+    && mv out/linux-arm64-network-manager-ipv6only/matter-network-manager-app out/matter-network-manager-app \
     ;; \
     *) ;; \
     esac
@@ -290,6 +302,9 @@ COPY --from=chip-build-cert-bins /root/connectedhomeip/out/lit-icd-app lit-icd-a
 COPY --from=chip-build-cert-bins /root/connectedhomeip/out/chip-energy-management-app chip-energy-management-app
 COPY --from=chip-build-cert-bins /root/connectedhomeip/out/chip-microwave-oven-app chip-microwave-oven-app
 COPY --from=chip-build-cert-bins /root/connectedhomeip/out/chip-rvc-app chip-rvc-app
+COPY --from=chip-build-cert-bins /root/connectedhomeip/out/fabric-bridge-app fabric-bridge-app
+COPY --from=chip-build-cert-bins /root/connectedhomeip/out/fabric-admin fabric-admin
+COPY --from=chip-build-cert-bins /root/connectedhomeip/out/matter-network-manager-app matter-network-manager-app
 
 # Stage 3.1: Setup the Matter Python environment
 COPY --from=chip-build-cert-bins /root/connectedhomeip/out/python_lib python_lib
@@ -304,6 +319,6 @@ COPY --from=chip-build-cert-bins /root/connectedhomeip/src/python_testing/requir
 RUN pip install --break-system-packages -r /tmp/requirements.txt && rm /tmp/requirements.txt
 
 # PIP requires MASON package compilation, which seems to require a JDK
-RUN set -x && DEBIAN_FRONTEND=noninteractive apt-get install -fy openjdk-8-jdk
+RUN set -x && DEBIAN_FRONTEND=noninteractive apt-get update; apt-get install -fy openjdk-8-jdk
 
 RUN pip install --break-system-packages --no-cache-dir python_lib/controller/python/chip*.whl

scripts/tools/zap/tests/outputs/all-clusters-app/app-templates/endpoint_config.h

@@ -2217,6 +2217,7 @@
     };                                                                                                                             \
     const EmberAfGenericClusterFunction chipFuncArrayThermostatServer[] = {                                                        \
         (EmberAfGenericClusterFunction) emberAfThermostatClusterServerInitCallback,                                                \
+        (EmberAfGenericClusterFunction) MatterThermostatClusterServerAttributeChangedCallback,                                     \
         (EmberAfGenericClusterFunction) MatterThermostatClusterServerShutdownCallback,                                             \
         (EmberAfGenericClusterFunction) MatterThermostatClusterServerPreAttributeChangedCallback,                                  \
     };                                                                                                                             \
@@ -3756,7 +3757,7 @@
       .attributes = ZAP_ATTRIBUTE_INDEX(616), \
       .attributeCount = 26, \
       .clusterSize = 72, \
-      .mask = ZAP_CLUSTER_MASK(SERVER) | ZAP_CLUSTER_MASK(INIT_FUNCTION) | ZAP_CLUSTER_MASK(SHUTDOWN_FUNCTION) | ZAP_CLUSTER_MASK(PRE_ATTRIBUTE_CHANGED_FUNCTION), \
+      .mask = ZAP_CLUSTER_MASK(SERVER) | ZAP_CLUSTER_MASK(INIT_FUNCTION) | ZAP_CLUSTER_MASK(ATTRIBUTE_CHANGED_FUNCTION) | ZAP_CLUSTER_MASK(SHUTDOWN_FUNCTION) | ZAP_CLUSTER_MASK(PRE_ATTRIBUTE_CHANGED_FUNCTION), \
       .functions = chipFuncArrayThermostatServer, \
       .acceptedCommandList = ZAP_GENERATED_COMMANDS_INDEX( 241 ), \
       .generatedCommandList = ZAP_GENERATED_COMMANDS_INDEX( 246 ), \

src/access/AccessControl.cpp

@@ -538,12 +538,7 @@ CHIP_ERROR AccessControl::CheckARL(const SubjectDescriptor & subjectDescriptor,
     if (result != CHIP_NO_ERROR)
     {
         ChipLogProgress(DataManagement, "AccessControl: %s",
-#if 0
-                        // TODO(#35177): new error code coming when access check plumbing are fixed in callers
                         (result == CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL) ? "denied (restricted)" : "denied (restriction error)");
-#else
-                        (result == CHIP_ERROR_ACCESS_DENIED) ? "denied (restricted)" : "denied (restriction error)");
-#endif
         return result;
     }
 

src/access/AccessRestrictionProvider.cpp

@@ -197,45 +197,25 @@ CHIP_ERROR AccessRestrictionProvider::DoCheck(const std::vector<Entry> & entries
                 if (requestPath.requestType == RequestType::kAttributeReadRequest ||
                     requestPath.requestType == RequestType::kAttributeWriteRequest)
                 {
-#if 0
-                    // TODO(#35177): use new ARL error code when access checks are fixed
                     return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL;
-#else
-                    return CHIP_ERROR_ACCESS_DENIED;
-#endif
                 }
                 break;
             case Type::kAttributeWriteForbidden:
                 if (requestPath.requestType == RequestType::kAttributeWriteRequest)
                 {
-#if 0
-                    // TODO(#35177): use new ARL error code when access checks are fixed
                     return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL;
-#else
-                    return CHIP_ERROR_ACCESS_DENIED;
-#endif
                 }
                 break;
             case Type::kCommandForbidden:
                 if (requestPath.requestType == RequestType::kCommandInvokeRequest)
                 {
-#if 0
-                    // TODO(#35177): use new ARL error code when access checks are fixed
                     return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL;
-#else
-                    return CHIP_ERROR_ACCESS_DENIED;
-#endif
                 }
                 break;
             case Type::kEventForbidden:
                 if (requestPath.requestType == RequestType::kEventReadRequest)
                 {
-#if 0
-                    // TODO(#35177): use new ARL error code when access checks are fixed
                     return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL;
-#else
-                    return CHIP_ERROR_ACCESS_DENIED;
-#endif
                 }
                 break;
             }

src/access/tests/TestAccessRestrictionProvider.cpp

@@ -174,7 +174,7 @@ void RunChecks(const CheckData * checkData, size_t count)
 {
     for (size_t i = 0; i < count; i++)
     {
-        CHIP_ERROR expectedResult = checkData[i].allow ? CHIP_NO_ERROR : CHIP_ERROR_ACCESS_DENIED;
+        CHIP_ERROR expectedResult = checkData[i].allow ? CHIP_NO_ERROR : CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL;
         EXPECT_EQ(accessControl.Check(checkData[i].subjectDescriptor, checkData[i].requestPath, checkData[i].privilege),
                   expectedResult);
     }

src/app/CommandHandlerImpl.cpp

@@ -410,12 +410,13 @@ Status CommandHandlerImpl::ProcessCommandDataIB(CommandDataIB::Parser & aCommand
         err                                = Access::GetAccessControl().Check(subjectDescriptor, requestPath, requestPrivilege);
         if (err != CHIP_NO_ERROR)
         {
-            if (err != CHIP_ERROR_ACCESS_DENIED)
+            if ((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL))
             {
                 return FallibleAddStatus(concretePath, Status::Failure) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
             }
             // TODO: when wildcard invokes are supported, handle them to discard rather than fail with status
-            return FallibleAddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
+            Status status = err == CHIP_ERROR_ACCESS_DENIED ? Status::UnsupportedAccess : Status::AccessRestricted;
+            return FallibleAddStatus(concretePath, status) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
         }
     }
 

src/app/CommandHandlerImpl.h

@@ -294,7 +294,12 @@ class CommandHandlerImpl : public CommandHandler
         {
             return CHIP_NO_ERROR;
         }
-        ReturnErrorOnFailure(RollbackResponse());
+        // The error value of RollbackResponse is not important if it fails, we prioritize
+        // conveying the error generated by addResponseFunction to the caller.
+        if (RollbackResponse() != CHIP_NO_ERROR)
+        {
+            return err;
+        }
         // If we failed to add a command due to lack of space in the
         // packet, we will make another attempt to add the response using
         // an additional InvokeResponseMessage.