Implement the Access control event support (#13838)

* Implement the Access control event support * Update src/app/AttributeAccessInterface.h Co-authored-by: Marc Lepage <67919234+mlepage-google@users.noreply.github.com> * Update src/app/AttributeAccessInterface.h Co-authored-by: Marc Lepage <67919234+mlepage-google@users.noreply.github.com> * Update src/app/clusters/access-control-server/access-control-server.cpp Co-authored-by: Marc Lepage <67919234+mlepage-google@users.noreply.github.com> * Disable warning maybe-uninitialized via pragma in the Optional copy constructor * Add more comments Co-authored-by: Marc Lepage <67919234+mlepage-google@users.noreply.github.com>
project-chip · Sep 14, 2023 · c1a9750 · c1a9750
1 parent aba8046
commit c1a9750
Show file tree

Hide file tree

Showing 5 changed files with 112 additions and 7 deletions.
diff --git a/src/app/AttributeAccessInterface.h b/src/app/AttributeAccessInterface.h
@@ -18,6 +18,7 @@
 
 #pragma once
 
+#include <access/SubjectDescriptor.h>
 #include <app/ClusterInfo.h>
 #include <app/ConcreteAttributePath.h>
 #include <app/MessageDef/AttributeReportIBs.h>
@@ -311,8 +312,8 @@ class AttributeValueEncoder
 class AttributeValueDecoder
 {
 public:
-    AttributeValueDecoder(TLV::TLVReader & aReader, FabricIndex aAccessingFabricIndex) :
-        mReader(aReader), mAccessingFabricIndex(aAccessingFabricIndex)
+    AttributeValueDecoder(TLV::TLVReader & aReader, const Access::SubjectDescriptor & aSubjectDescriptor) :
+        mReader(aReader), mSubjectDescriptor(aSubjectDescriptor)
     {}
 
     template <typename T>
@@ -327,12 +328,17 @@ class AttributeValueDecoder
     /**
      * The accessing fabric index for this write interaction.
      */
-    FabricIndex AccessingFabricIndex() const { return mAccessingFabricIndex; }
+    FabricIndex AccessingFabricIndex() const { return mSubjectDescriptor.fabricIndex; }
+
+    /**
+     * The accessing subject descriptor for this write interaction.
+     */
+    const Access::SubjectDescriptor & GetSubjectDescriptor() const { return mSubjectDescriptor; }
 
 private:
     TLV::TLVReader & mReader;
     bool mTriedDecode = false;
-    const FabricIndex mAccessingFabricIndex;
+    const Access::SubjectDescriptor mSubjectDescriptor;
 };
 
 class AttributeAccessInterface

diff --git a/src/app/clusters/access-control-server/access-control-server.cpp b/src/app/clusters/access-control-server/access-control-server.cpp
@@ -25,6 +25,7 @@
 #include <app/AttributeAccessInterface.h>
 #include <app/CommandHandler.h>
 #include <app/ConcreteCommandPath.h>
+#include <app/EventLogging.h>
 #include <app/data-model/Encode.h>
 #include <app/util/af.h>
 #include <app/util/attribute-storage.h>
@@ -293,6 +294,84 @@ class AccessControlAttribute : public chip::app::AttributeAccessInterface
 
 constexpr uint16_t AccessControlAttribute::ClusterRevision;
 
+CHIP_ERROR LogAccessControlEvent(const AccessControl::Entry & entry, const Access::SubjectDescriptor & subjectDescriptor,
+                                 AccessControlCluster::ChangeTypeEnum changeType)
+{
+    CHIP_ERROR err;
+
+    // Record AccessControlEntry event
+    EventNumber eventNumber;
+    DataModel::Nullable<chip::NodeId> adminNodeID;
+    DataModel::Nullable<uint16_t> adminPasscodeID;
+    DataModel::Nullable<AccessControlCluster::Structs::AccessControlEntry::Type> latestValue;
+
+    // Populate AccessControlEntryStruct from AccessControl entry.
+    AccessControlCluster::Structs::AccessControlEntry::Type staging;
+
+    ReturnErrorOnFailure(entry.GetFabricIndex(staging.fabricIndex));
+
+    {
+        Privilege privilege;
+        ReturnErrorOnFailure(entry.GetPrivilege(privilege));
+        ReturnErrorOnFailure(AccessControlEntryCodec::Convert(privilege, staging.privilege));
+    }
+
+    {
+        AuthMode authMode;
+        ReturnErrorOnFailure(entry.GetAuthMode(authMode));
+        ReturnErrorOnFailure(AccessControlEntryCodec::Convert(authMode, staging.authMode));
+    }
+
+    NodeId subjectBuffer[CHIP_CONFIG_EXAMPLE_ACCESS_CONTROL_MAX_SUBJECTS_PER_ENTRY];
+    size_t subjectCount;
+    ReturnErrorOnFailure(entry.GetSubjectCount(subjectCount));
+    if (subjectCount > 0)
+    {
+        for (size_t i = 0; i < subjectCount; ++i)
+        {
+            ReturnErrorOnFailure(entry.GetSubject(i, subjectBuffer[i]));
+        }
+        staging.subjects.SetNonNull(subjectBuffer, subjectCount);
+    }
+
+    AccessControlCluster::Structs::Target::Type targetBuffer[CHIP_CONFIG_EXAMPLE_ACCESS_CONTROL_MAX_TARGETS_PER_ENTRY];
+    size_t targetCount;
+    ReturnErrorOnFailure(entry.GetTargetCount(targetCount));
+    if (targetCount > 0)
+    {
+        for (size_t i = 0; i < targetCount; ++i)
+        {
+            AccessControl::Entry::Target target;
+            ReturnErrorOnFailure(entry.GetTarget(i, target));
+            ReturnErrorOnFailure(AccessControlEntryCodec::Convert(target, targetBuffer[i]));
+        }
+        staging.targets.SetNonNull(targetBuffer, targetCount);
+    }
+
+    latestValue.SetNonNull(staging);
+
+    if (subjectDescriptor.authMode == Access::AuthMode::kCase)
+    {
+        adminNodeID.SetNonNull(subjectDescriptor.subject);
+    }
+    else if (subjectDescriptor.authMode == Access::AuthMode::kPase)
+    {
+        adminNodeID.SetNonNull(PAKEKeyIdFromNodeId(subjectDescriptor.subject));
+    }
+
+    AccessControlCluster::Events::AccessControlEntryChanged::Type event{ subjectDescriptor.fabricIndex, adminNodeID,
+                                                                         adminPasscodeID, changeType, latestValue };
+
+    // AccessControl event only occurs on endpoint 0.
+    err = LogEvent(event, 0, eventNumber);
+    if (CHIP_NO_ERROR != err)
+    {
+        ChipLogError(DataManagement, "AccessControl: Failed to record AccessControlEntryChanged event");
+    }
+
+    return err;
+}
+
 CHIP_ERROR AccessControlAttribute::Read(const ConcreteReadAttributePath & aPath, AttributeValueEncoder & aEncoder)
 {
     switch (aPath.mAttributeId)
@@ -364,10 +443,14 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(AttributeValueDecoder & aDecoder)
         if (i < oldCount)
         {
             ReturnErrorOnFailure(GetAccessControl().UpdateEntry(i, iterator.GetValue().entry));
+            ReturnErrorOnFailure(LogAccessControlEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
+                                                       AccessControlCluster::ChangeTypeEnum::kChanged));
         }
         else
         {
             ReturnErrorOnFailure(GetAccessControl().CreateEntry(nullptr, iterator.GetValue().entry));
+            ReturnErrorOnFailure(LogAccessControlEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
+                                                       AccessControlCluster::ChangeTypeEnum::kAdded));
         }
         ++i;
     }
@@ -377,6 +460,8 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(AttributeValueDecoder & aDecoder)
     {
         --oldCount;
         ReturnErrorOnFailure(GetAccessControl().DeleteEntry(oldCount));
+        ReturnErrorOnFailure(LogAccessControlEvent(iterator.GetValue().entry, aDecoder.GetSubjectDescriptor(),
+                                                   AccessControlCluster::ChangeTypeEnum::kRemoved));
     }
 
     return CHIP_NO_ERROR;

diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp
@@ -836,7 +836,7 @@ CHIP_ERROR WriteSingleClusterData(const SubjectDescriptor & aSubjectDescriptor,
 
     if (auto * attrOverride = findAttributeAccessOverride(aClusterInfo.mEndpointId, aClusterInfo.mClusterId))
     {
-        AttributeValueDecoder valueDecoder(aReader, apWriteHandler->GetAccessingFabricIndex());
+        AttributeValueDecoder valueDecoder(aReader, aSubjectDescriptor);
         ReturnErrorOnFailure(attrOverride->Write(aPath, valueDecoder));
 
         if (valueDecoder.TriedDecode())

diff --git a/src/lib/core/NodeId.h b/src/lib/core/NodeId.h
@@ -45,8 +45,9 @@ constexpr NodeId kPlaceholderNodeId   = 0xFFFF'FFFE'FFFF'FFFFULL;
 constexpr NodeId kMinCASEAuthTag = 0xFFFF'FFFD'0000'0000ULL;
 constexpr NodeId kMaxCASEAuthTag = 0xFFFF'FFFD'FFFF'FFFFULL;
 
-constexpr NodeId kMinPAKEKeyId = 0xFFFF'FFFB'0000'0000ULL;
-constexpr NodeId kMaxPAKEKeyId = 0xFFFF'FFFB'FFFF'FFFFULL;
+constexpr NodeId kMinPAKEKeyId  = 0xFFFF'FFFB'0000'0000ULL;
+constexpr NodeId kMaxPAKEKeyId  = 0xFFFF'FFFB'FFFF'FFFFULL;
+constexpr NodeId kMaskPAKEKeyId = 0x0000'0000'0000'FFFFULL;
 
 // There are more reserved ranges here, not assigned to anything yet, going down
 // all the way to 0xFFFF'FFF0'0000'0000ULL
@@ -83,4 +84,9 @@ constexpr NodeId NodeIdFromPAKEKeyId(uint16_t aPAKEKeyId)
     return kMinPAKEKeyId | aPAKEKeyId;
 }
 
+constexpr uint16_t PAKEKeyIdFromNodeId(NodeId aNodeId)
+{
+    return aNodeId & kMaskPAKEKeyId;
+}
+
 } // namespace chip
diff --git a/src/lib/core/Optional.h b/src/lib/core/Optional.h
@@ -68,7 +68,15 @@ class Optional
     {
         if (mHasValue)
         {
+            // The -Wmaybe-uninitialized warning gets confused about the fact
+            // that other.mValue is always initialized if other.mHasValue is
+            // true, so suppress it for our access to other.mValue.
+#pragma GCC diagnostic push
+#if !defined(__clang__)
+#pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
+#endif // !defined(__clang__)
             new (&mValue.mData) T(other.mValue.mData);
+#pragma GCC diagnostic pop
         }
     }