Make Write Client reject a first ACL entry that doesn't grant Administer privelege to at least one subject and return a new Error Code when that happens

Author: Alami-Amine

docs/ids_and_codes/ERROR_CODES.md

@@ -18,7 +18,7 @@
 #
 
 #
-# Execute as `python scripts/error_table.py > docs/ERROR_CODES.md` from root of repos
+# Execute as `python scripts/error_table.py > docs/ids_and_codes/ERROR_CODES.md` from root of repos
 #
 # This script uses heuristics scraping of the headers to generate nice tables
 #
@@ -145,7 +145,7 @@ def main():
     print("# Matter SDK `CHIP_ERROR` enums values")
     print()
     print("This file was **AUTOMATICALLY** generated by")
-    print("`python scripts/error_table.py > docs/ERROR_CODES.md`. DO NOT EDIT BY HAND!")
+    print("`python scripts/error_table.py > docs/ids_and_codes/ERROR_CODES.md`. DO NOT EDIT BY HAND!")
     print()
     print("## Table of contents")
 

scripts/error_table.py

@@ -18,6 +18,7 @@
 
 #pragma once
 
+#include <app-common/zap-generated/cluster-objects.h>
 #include <app/AttributePathParams.h>
 #include <app/ConcreteAttributePath.h>
 #include <app/InteractionModelTimeout.h>
@@ -174,21 +175,28 @@ class WriteClient : public Messaging::ExchangeDelegate
 
         ReturnErrorOnFailure(EnsureMessage());
 
-        bool emptyListNotEncoded = true;
-        // Encode an empty list for the chunking protocol.
-        if (path.mClusterId != 0x1F)
+        // Encode an empty list for the chunking protocol, except when encoding ACLs.
+        // for ACLs, we encode the first ACL entry directly, which will trigger a ReplaceAll operation on the server side.
+        // The first ACL entry must grant Administer privileges to at least one subject.
+        bool firstEntryEncoded = false;
+        if constexpr (!std::is_same_v<T, Clusters::AccessControl::Structs::AccessControlEntryStruct::Type const>)
         {
             ReturnErrorOnFailure(EncodeSingleAttributeDataIB(path, DataModel::List<uint8_t>()));
-            emptyListNotEncoded = false;
         }
         else
         {
-            // When we are trying to Encode ACL, we encode the first entry instead of an empty list
-            ReturnErrorOnFailure(EncodeSingleAttributeDataIB(path, DataModel::List<T>(value.data(), 1)));
-            emptyListNotEncoded = true;
+            if (value.data()[0].privilege != Clusters::AccessControl::AccessControlEntryPrivilegeEnum::kAdminister)
+            {
+                return CHIP_ERROR_FIRST_ACL_ENTRY_NOT_ADMIN;
+            }
+
+            ReturnErrorOnFailure(EncodeSingleAttributeDataIB(path, DataModel::List<T>(value.SubSpan(0, 1))));
+            firstEntryEncoded = true;
         }
-        path.mListOp = ConcreteDataAttributePath::ListOperation::AppendItem;
-        for (size_t i = static_cast<size_t>(emptyListNotEncoded); i < value.size(); i++)
+
+        path.mListOp      = ConcreteDataAttributePath::ListOperation::AppendItem;
+        size_t startIndex = firstEntryEncoded ? 1 : 0;
+        for (size_t i = startIndex; i < value.size(); i++)
         {
             ReturnErrorOnFailure(EncodeSingleAttributeDataIB(path, value.data()[i]));
         }

src/app/WriteClient.h

@@ -377,8 +377,7 @@ CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeData
         if (mDelegate->HasConflictWriteRequests(this, dataAttributePath) ||
             // Per chunking protocol, we are processing the list entries, but the initial empty list is not processed, so we reject
             // it with Busy status code.
-            (dataAttributePath.IsListItemOperation() && !IsSameAttribute(mProcessingAttributePath, dataAttributePath) &&
-             dataAttributePath.mClusterId != 0x1F))
+            (dataAttributePath.IsListItemOperation() && !IsSameAttribute(mProcessingAttributePath, dataAttributePath)))
         {
             err = AddStatusInternal(dataAttributePath, StatusIB(Status::Busy));
             continue;

src/app/WriteHandler.cpp

@@ -475,6 +475,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err)
     case CHIP_ERROR_HANDLER_NOT_SET.AsInteger():
         desc = "Callback function or callable object is not set";
         break;
+    case CHIP_ERROR_FIRST_ACL_ENTRY_NOT_ADMIN.AsInteger():
+        desc = "The first ACL entry must grant Administer privileges to at least one subject";
+        break;
     }
 #endif // !CHIP_CONFIG_SHORT_ERROR_STR
 

src/lib/core/CHIPError.cpp

@@ -78,6 +78,7 @@ static const CHIP_ERROR kTestElements[] =
     CHIP_ERROR_UNKNOWN_IMPLICIT_TLV_TAG,
     CHIP_ERROR_WRONG_TLV_TYPE,
     CHIP_ERROR_TLV_CONTAINER_OPEN,
+    CHIP_ERROR_FIRST_ACL_ENTRY_NOT_ADMIN,
     CHIP_ERROR_INVALID_MESSAGE_TYPE,
     CHIP_ERROR_UNEXPECTED_TLV_ELEMENT,
     CHIP_ERROR_NOT_IMPLEMENTED,