Skip to content

Commit

Permalink
Merge branch 'master' into whm-follow-up-part1
Browse files Browse the repository at this point in the history
  • Loading branch information
jamesharrow authored Aug 29, 2024
2 parents 7c549c3 + e772b69 commit fab10c7
Show file tree
Hide file tree
Showing 43 changed files with 278 additions and 162 deletions.
2 changes: 2 additions & 0 deletions docs/ERROR_CODES.md
Original file line number Diff line number Diff line change
Expand Up @@ -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` |
Expand Down Expand Up @@ -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` |
Expand Down
2 changes: 1 addition & 1 deletion docs/guides/fabric_synchronization_guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 10 additions & 1 deletion examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 11 additions & 3 deletions examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
};
Expand Down Expand Up @@ -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;
Expand All @@ -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);
Expand Down
13 changes: 5 additions & 8 deletions examples/fabric-admin/device_manager/DeviceManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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

Expand All @@ -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());
}
Expand All @@ -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());
}
Expand Down
10 changes: 9 additions & 1 deletion examples/fabric-admin/device_manager/DeviceManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@

#include <set>

constexpr uint32_t kDefaultSetupPinCode = 20202021;
constexpr uint16_t kDefaultLocalBridgePort = 5540;
constexpr uint16_t kResponseTimeoutSeconds = 30;

class Device
Expand Down Expand Up @@ -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; }
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 8 additions & 0 deletions integrations/docker/images/base/chip-build/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
17 changes: 16 additions & 1 deletion integrations/docker/images/chip-cert-bins/Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand All @@ -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 \
Expand All @@ -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 \
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -2217,6 +2217,7 @@
}; \
const EmberAfGenericClusterFunction chipFuncArrayThermostatServer[] = { \
(EmberAfGenericClusterFunction) emberAfThermostatClusterServerInitCallback, \
(EmberAfGenericClusterFunction) MatterThermostatClusterServerAttributeChangedCallback, \
(EmberAfGenericClusterFunction) MatterThermostatClusterServerShutdownCallback, \
(EmberAfGenericClusterFunction) MatterThermostatClusterServerPreAttributeChangedCallback, \
}; \
Expand Down Expand Up @@ -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 ), \
Expand Down
5 changes: 0 additions & 5 deletions src/access/AccessControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
20 changes: 0 additions & 20 deletions src/access/AccessRestrictionProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/access/tests/TestAccessRestrictionProvider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
5 changes: 3 additions & 2 deletions src/app/CommandHandlerImpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/app/CommandHandlerImpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading

0 comments on commit fab10c7

Please sign in to comment.