Implement ACL attribute read/write (#12305)

* 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
project-chip · Sep 1, 2023 · 1062968 · 1062968
1 parent 000e18d
commit 1062968
Show file tree

Hide file tree

Showing 13 changed files with 918 additions and 454 deletions.
diff --git a/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap b/examples/all-clusters-app/all-clusters-common/all-clusters-app.zap
@@ -969,7 +969,7 @@
               "bounded": 0,
               "defaultValue": "",
               "reportable": 1,
-              "minInterval": 1,
+              "minInterval": 0,
               "maxInterval": 65534,
               "reportableChange": 0
             },
@@ -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
             },

diff --git a/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp b/examples/all-clusters-app/all-clusters-common/src/bridged-actions-stub.cpp
@@ -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

diff --git a/src/access/AccessControl.h b/src/access/AccessControl.h
@@ -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;
@@ -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;
 
@@ -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; }
 
@@ -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.

diff --git a/src/access/examples/ExampleAccessControlDelegate.cpp b/src/access/examples/ExampleAccessControlDelegate.cpp
@@ -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()))

diff --git a/src/access/tests/TestAccessControl.cpp b/src/access/tests/TestAccessControl.cpp
@@ -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; }
 

diff --git a/src/app/AttributeAccessInterface.h b/src/app/AttributeAccessInterface.h
@@ -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)...);
@@ -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)...);
         }
@@ -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)...);
@@ -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.
@@ -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;

diff --git a/src/app/BUILD.gn b/src/app/BUILD.gn
@@ -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",