Skip to content

Commit

Permalink
Implement ACL attribute read/write (#12305)
Browse files Browse the repository at this point in the history
* Add access control server skeleton

Add empty cluster implementation to all-clusters-app example.

* Regen code.

* Restyle code

* Update from upstream changes

* Restyle

* Add access-control-server to CMakeLists.txt

For esp32 and mbed, for all-clusters-app.

* Remove nullable from list attribute

Code gen isn't working yet for this possibility.

* Regen and restyle

* Regen and restyle

* Fix test

Descriptor cluster test expects certain clusters in certain order.
(This is fragile and we should find a way to improve it, but just
fix up the expected order for now.)

* Regen and restyle

* Regen and restyle

* Regen and restyle

* Regen and restyle

* Regen and restyle

* Regen and restyle

* Add AccessControl cluster to Darwin test helper

Until the tests use Descriptor cluster, they have a hardcoded list of
clusters expected on endpoints, which must be kept accurate.

* Remove AccessControl cluster from some tests

Some Darwin tests are not relevant for this cluster (as for others).

* Fix unless in test file

Those special tags need to be closed.

* Restyle and regen

* Implement ACL attribute

Just basic read/write of the entire list.
- Not a specific index.
- Not fabric-filtered.
- Not with rollback if errors occur.

Minor small changes to the AccessControl interface to support this work.

Progress toward issue #10254

* Regen and restyle

* Regen and restyle

* Change extension attribute storage to RAM

Until it's implemented in external storage, change its storage to RAM,
so there's less issues with unimplemented cluster attributes.

* Re-enable access control cluster tests

Tests for ACL and extension attribute were disabled while these
attributes were unimplemented. Now, ACL is implemented and extension has
its storage moved to RAM (for now) so both should work.

* Fix extension attribute

Apparently the regen emits corrupt code if it doesn't have a length.

* Restyle and regen

* Change lambda arg to auto in access control server

It was TagBoundEncoder, but now it's ListEncodeHelper.

Also had to fix the perfect forwarding in AttributeValueEncoder and
ListEncodeHelper and other template functions in the file, so it
doesn't try to copy its args.

* Add missing variable definitions

These static constexpr member variables were declared but not defined.

In C++14 and lower, the definitions are required. Otherwise, undefined
reference ensues. C++17 handles this but Matter is still using C++14.

Not sure why this was working before...

* Regen and restyle
  • Loading branch information
mlepage-google authored and pull[bot] committed Aug 8, 2023
1 parent ba969dc commit 1237104
Show file tree
Hide file tree
Showing 13 changed files with 918 additions and 454 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -969,7 +969,7 @@
"bounded": 0,
"defaultValue": "",
"reportable": 1,
"minInterval": 1,
"minInterval": 0,
"maxInterval": 65534,
"reportableChange": 0
},
Expand All @@ -979,12 +979,12 @@
"mfgCode": null,
"side": "server",
"included": 1,
"storageOption": "External",
"storageOption": "RAM",
"singleton": 0,
"bounded": 0,
"defaultValue": "",
"reportable": 1,
"minInterval": 1,
"minInterval": 0,
"maxInterval": 65534,
"reportableChange": 0
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class BridgedActionsAttrAccess : public AttributeAccessInterface
CHIP_ERROR ReadClusterRevision(EndpointId endpoint, AttributeValueEncoder & aEncoder);
};

constexpr uint16_t BridgedActionsAttrAccess::ClusterRevision;

CHIP_ERROR BridgedActionsAttrAccess::ReadActionListAttribute(EndpointId endpoint, AttributeValueEncoder & aEncoder)
{
// Just return an empty list
Expand Down
25 changes: 22 additions & 3 deletions src/access/AccessControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ class AccessControl
static constexpr Flags kCluster = 1 << 0;
static constexpr Flags kEndpoint = 1 << 1;
static constexpr Flags kDeviceType = 1 << 2;
Flags flags;
Flags flags = 0;
ClusterId cluster;
EndpointId endpoint;
DeviceTypeId deviceType;
Expand Down Expand Up @@ -89,6 +89,19 @@ class AccessControl

Entry() = default;

Entry(Entry && other) : mDelegate(other.mDelegate) { other.mDelegate = &mDefaultDelegate; }

Entry & operator=(Entry && other)
{
if (this != &other)
{
mDelegate->Release();
mDelegate = other.mDelegate;
other.mDelegate = &mDefaultDelegate;
}
return *this;
}

Entry(const Entry &) = delete;
Entry & operator=(const Entry &) = delete;

Expand Down Expand Up @@ -292,9 +305,12 @@ class AccessControl
virtual CHIP_ERROR Finish() { return CHIP_ERROR_NOT_IMPLEMENTED; }

// Capabilities
virtual CHIP_ERROR GetMaxEntries(int & value) const { return CHIP_ERROR_NOT_IMPLEMENTED; }
virtual CHIP_ERROR GetMaxEntryCount(size_t & value) const { return CHIP_ERROR_NOT_IMPLEMENTED; }
// TODO: more capabilities

// Actualities
virtual CHIP_ERROR GetEntryCount(size_t & value) const { return CHIP_ERROR_NOT_IMPLEMENTED; }

// Preparation
virtual CHIP_ERROR PrepareEntry(Entry & entry) { return CHIP_ERROR_NOT_IMPLEMENTED; }

Expand Down Expand Up @@ -352,7 +368,10 @@ class AccessControl
CHIP_ERROR Finish();

// Capabilities
CHIP_ERROR GetMaxEntries(int & value) const { return mDelegate.GetMaxEntries(value); }
CHIP_ERROR GetMaxEntryCount(size_t & value) const { return mDelegate.GetMaxEntryCount(value); }

// Actualities
CHIP_ERROR GetEntryCount(size_t & value) const { return mDelegate.GetEntryCount(value); }

/**
* Prepares an entry.
Expand Down
16 changes: 15 additions & 1 deletion src/access/examples/ExampleAccessControlDelegate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -968,12 +968,26 @@ class AccessControlDelegate : public AccessControl::Delegate
return SaveToFlash();
}

CHIP_ERROR GetMaxEntries(int & value) const override
CHIP_ERROR GetMaxEntryCount(size_t & value) const override
{
value = ArraySize(EntryStorage::acl);
return CHIP_NO_ERROR;
}

CHIP_ERROR GetEntryCount(size_t & value) const override
{
value = 0;
for (const auto & storage : EntryStorage::acl)
{
if (!storage.InUse())
{
break;
}
value++;
}
return CHIP_NO_ERROR;
}

CHIP_ERROR PrepareEntry(Entry & entry) override
{
if (auto * delegate = EntryDelegate::Find(entry.GetDelegate()))
Expand Down
2 changes: 1 addition & 1 deletion src/access/tests/TestAccessControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ struct EntryData
NodeId subjects[kMaxSubjects] = { 0 };
Target targets[kMaxTargets] = { { 0 } };

void Clear() { memset(this, 0, sizeof(*this)); }
void Clear() { *this = EntryData(); }

bool IsEmpty() const { return authMode == AuthMode::kNone; }

Expand Down
10 changes: 5 additions & 5 deletions src/app/AttributeAccessInterface.h
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class AttributeReportBuilder
* EncodeValue encodes the value field of the report, it should be called exactly once.
*/
template <typename... Ts>
CHIP_ERROR EncodeValue(AttributeReportIBs::Builder & aAttributeReportIBs, Ts... aArgs)
CHIP_ERROR EncodeValue(AttributeReportIBs::Builder & aAttributeReportIBs, Ts &&... aArgs)
{
return DataModel::Encode(*(aAttributeReportIBs.GetAttributeReport().GetAttributeData().GetWriter()),
TLV::ContextTag(to_underlying(AttributeDataIB::Tag::kData)), std::forward<Ts>(aArgs)...);
Expand All @@ -98,7 +98,7 @@ class AttributeValueEncoder
ListEncodeHelper(AttributeValueEncoder & encoder) : mAttributeValueEncoder(encoder) {}

template <typename... Ts>
CHIP_ERROR Encode(Ts... aArgs) const
CHIP_ERROR Encode(Ts &&... aArgs) const
{
return mAttributeValueEncoder.EncodeListItem(std::forward<Ts>(aArgs)...);
}
Expand Down Expand Up @@ -152,7 +152,7 @@ class AttributeValueEncoder
* operation.
*/
template <typename... Ts>
CHIP_ERROR Encode(Ts... aArgs)
CHIP_ERROR Encode(Ts &&... aArgs)
{
mTriedEncode = true;
return EncodeAttributeReportIB(std::forward<Ts>(aArgs)...);
Expand Down Expand Up @@ -209,7 +209,7 @@ class AttributeValueEncoder
friend class ListEncodeHelper;

template <typename... Ts>
CHIP_ERROR EncodeListItem(Ts... aArgs)
CHIP_ERROR EncodeListItem(Ts &&... aArgs)
{
// EncodeListItem must be called after EncodeEmptyList(), thus mCurrentEncodingListIndex and
// mEncodeState.mCurrentEncodingListIndex are not invalid values.
Expand Down Expand Up @@ -242,7 +242,7 @@ class AttributeValueEncoder
* Actual logic for encoding a single AttributeReportIB in AttributeReportIBs.
*/
template <typename... Ts>
CHIP_ERROR EncodeAttributeReportIB(Ts... aArgs)
CHIP_ERROR EncodeAttributeReportIB(Ts &&... aArgs)
{
mTriedEncode = true;
AttributeReportBuilder builder;
Expand Down
1 change: 1 addition & 0 deletions src/app/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ static_library("app") {

public_deps = [
":app_buildconfig",
"${chip_root}/src/access",
"${chip_root}/src/app/util:device_callbacks_manager",
"${chip_root}/src/lib/support",
"${chip_root}/src/messaging",
Expand Down
Loading

0 comments on commit 1237104

Please sign in to comment.