Update ACL API to support ARL use cases (#34537)

* Update ACL API to support ARL use cases ARL needs to know what attribute/command/event is being accessed and with which interaction model action. This change brings this knowledge into the AccessControl classes Check method for further enhancement with ARL feature on top. * Addressed review comments * Addressed review comments * Clarified RequestType enum values and purpose * Renamed CanAccess to CanAccessEvent * Restyled by whitespace * Restyled by clang-format * Fixed some build issues - missed a spot to populate requestType and entityId - fix unused function definition for some cases * Fixed icd-management-server for ARL * fixed typo in previous fix :-( * Restyled by clang-format * Moved ARL related checks behind ARL feature flag Instead of validating requestType in AccessControl::Check, we will validate it is not unknown only if the ARL feature is enabled. * Add conditional ARL related validation Instead of removing ARL related validation from AccessControl::Check entirely, perform it only if the ARL feature is enabled. As of this commit, it is not enabled. * Restyled by clang-format --------- Co-authored-by: Restyled.io <[email protected]>
project-chip · Sep 5, 2024 · 1457704 · 1457704
1 parent 23f5ba3
commit 1457704
Show file tree

Hide file tree

Showing 11 changed files with 115 additions and 23 deletions.
diff --git a/src/access/AccessControl.cpp b/src/access/AccessControl.cpp
@@ -171,6 +171,23 @@ char GetPrivilegeStringForLogging(Privilege privilege)
     return 'u';
 }
 
+char GetRequestTypeStringForLogging(RequestType requestType)
+{
+    switch (requestType)
+    {
+    case RequestType::kAttributeReadRequest:
+        return 'r';
+    case RequestType::kAttributeWriteRequest:
+        return 'w';
+    case RequestType::kCommandInvokeRequest:
+        return 'i';
+    case RequestType::kEventReadOrSubscribeRequest:
+        return 'e';
+    default:
+        return '?';
+    }
+}
+
 #endif // CHIP_PROGRESS_LOGGING && CHIP_CONFIG_ACCESS_CONTROL_POLICY_LOGGING_VERBOSITY > 1
 
 } // namespace
@@ -306,6 +323,11 @@ void AccessControl::RemoveEntryListener(EntryListener & listener)
     }
 }
 
+bool AccessControl::IsAccessRestrictionListSupported() const
+{
+    return false; // not yet supported
+}
+
 CHIP_ERROR AccessControl::Check(const SubjectDescriptor & subjectDescriptor, const RequestPath & requestPath,
                                 Privilege requestPrivilege)
 {
@@ -316,14 +338,20 @@ CHIP_ERROR AccessControl::Check(const SubjectDescriptor & subjectDescriptor, con
         constexpr size_t kMaxCatsToLog = 6;
         char catLogBuf[kMaxCatsToLog * kCharsPerCatForLogging];
         ChipLogProgress(DataManagement,
-                        "AccessControl: checking f=%u a=%c s=0x" ChipLogFormatX64 " t=%s c=" ChipLogFormatMEI " e=%u p=%c",
+                        "AccessControl: checking f=%u a=%c s=0x" ChipLogFormatX64 " t=%s c=" ChipLogFormatMEI " e=%u p=%c r=%c",
                         subjectDescriptor.fabricIndex, GetAuthModeStringForLogging(subjectDescriptor.authMode),
                         ChipLogValueX64(subjectDescriptor.subject),
                         GetCatStringForLogging(catLogBuf, sizeof(catLogBuf), subjectDescriptor.cats),
-                        ChipLogValueMEI(requestPath.cluster), requestPath.endpoint, GetPrivilegeStringForLogging(requestPrivilege));
+                        ChipLogValueMEI(requestPath.cluster), requestPath.endpoint, GetPrivilegeStringForLogging(requestPrivilege),
+                        GetRequestTypeStringForLogging(requestPath.requestType));
     }
 #endif // CHIP_PROGRESS_LOGGING && CHIP_CONFIG_ACCESS_CONTROL_POLICY_LOGGING_VERBOSITY > 1
 
+    if (IsAccessRestrictionListSupported())
+    {
+        VerifyOrReturnError(requestPath.requestType != RequestType::kRequestTypeUnknown, CHIP_ERROR_INVALID_ARGUMENT);
+    }
+
     {
         CHIP_ERROR result = mDelegate->Check(subjectDescriptor, requestPath, requestPrivilege);
         if (result != CHIP_ERROR_NOT_IMPLEMENTED)

diff --git a/src/access/AccessControl.h b/src/access/AccessControl.h
@@ -627,6 +627,13 @@ class AccessControl
     // Removes a listener from the listener list, if in the list.
     void RemoveEntryListener(EntryListener & listener);
 
+    /**
+     * Check whether or not Access Restriction List is supported.
+     *
+     * @retval true if Access Restriction List is supported.
+     */
+    bool IsAccessRestrictionListSupported() const;
+
     /**
      * Check whether access (by a subject descriptor, to a request path,
      * requiring a privilege) should be allowed or denied.

diff --git a/src/access/RequestPath.h b/src/access/RequestPath.h
@@ -19,15 +19,29 @@
 #pragma once
 
 #include <lib/core/DataModelTypes.h>
+#include <optional>
 
 namespace chip {
 namespace Access {
 
+enum class RequestType : uint8_t
+{
+    kRequestTypeUnknown,
+    kAttributeReadRequest,
+    kAttributeWriteRequest,
+    kCommandInvokeRequest,
+    kEventReadOrSubscribeRequest
+};
+
 struct RequestPath
 {
     // NOTE: eventually this will likely also contain node, for proxying
-    ClusterId cluster   = 0;
-    EndpointId endpoint = 0;
+    ClusterId cluster       = 0;
+    EndpointId endpoint     = 0;
+    RequestType requestType = RequestType::kRequestTypeUnknown;
+
+    // entityId represents an attribute, command, or event ID, which is determined by the requestType. Wildcard if omitted.
+    std::optional<uint32_t> entityId;
 };
 
 } // namespace Access

diff --git a/src/app/CommandHandlerImpl.cpp b/src/app/CommandHandlerImpl.cpp
@@ -402,7 +402,10 @@ Status CommandHandlerImpl::ProcessCommandDataIB(CommandDataIB::Parser & aCommand
 
     {
         Access::SubjectDescriptor subjectDescriptor = GetSubjectDescriptor();
-        Access::RequestPath requestPath{ .cluster = concretePath.mClusterId, .endpoint = concretePath.mEndpointId };
+        Access::RequestPath requestPath{ .cluster     = concretePath.mClusterId,
+                                         .endpoint    = concretePath.mEndpointId,
+                                         .requestType = Access::RequestType::kCommandInvokeRequest,
+                                         .entityId    = concretePath.mCommandId };
         Access::Privilege requestPrivilege = RequiredPrivilege::ForInvokeCommand(concretePath);
         err                                = Access::GetAccessControl().Check(subjectDescriptor, requestPath, requestPrivilege);
         if (err != CHIP_NO_ERROR)
@@ -548,7 +551,10 @@ Status CommandHandlerImpl::ProcessGroupCommandDataIB(CommandDataIB::Parser & aCo
 
         {
             Access::SubjectDescriptor subjectDescriptor = GetSubjectDescriptor();
-            Access::RequestPath requestPath{ .cluster = concretePath.mClusterId, .endpoint = concretePath.mEndpointId };
+            Access::RequestPath requestPath{ .cluster     = concretePath.mClusterId,
+                                             .endpoint    = concretePath.mEndpointId,
+                                             .requestType = Access::RequestType::kCommandInvokeRequest,
+                                             .entityId    = concretePath.mCommandId };
             Access::Privilege requestPrivilege = RequiredPrivilege::ForInvokeCommand(concretePath);
             err                                = Access::GetAccessControl().Check(subjectDescriptor, requestPath, requestPrivilege);
             if (err != CHIP_NO_ERROR)

diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp
@@ -554,7 +554,10 @@ CHIP_ERROR EventManagement::CheckEventContext(EventLoadOutContext * eventLoadOut
 
     ReturnErrorOnFailure(ret);
 
-    Access::RequestPath requestPath{ .cluster = event.mClusterId, .endpoint = event.mEndpointId };
+    Access::RequestPath requestPath{ .cluster     = event.mClusterId,
+                                     .endpoint    = event.mEndpointId,
+                                     .requestType = Access::RequestType::kEventReadOrSubscribeRequest,
+                                     .entityId    = event.mEventId };
     Access::Privilege requestPrivilege = RequiredPrivilege::ForReadEvent(path);
     CHIP_ERROR accessControlError =
         Access::GetAccessControl().Check(eventLoadOutContext->mSubjectDescriptor, requestPath, requestPrivilege);

diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
@@ -494,7 +494,10 @@ CHIP_ERROR InteractionModelEngine::ParseAttributePaths(const Access::SubjectDesc
             // AttributePathExpandIterator. So we just need to check the ACL bits.
             for (; pathIterator.Get(readPath); pathIterator.Next())
             {
-                Access::RequestPath requestPath{ .cluster = readPath.mClusterId, .endpoint = readPath.mEndpointId };
+                // leave requestPath.entityId optional value unset to indicate wildcard
+                Access::RequestPath requestPath{ .cluster     = readPath.mClusterId,
+                                                 .endpoint    = readPath.mEndpointId,
+                                                 .requestType = Access::RequestType::kAttributeReadRequest };
                 err = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath,
                                                        RequiredPrivilege::ForReadAttribute(readPath));
                 if (err == CHIP_NO_ERROR)
@@ -510,7 +513,10 @@ CHIP_ERROR InteractionModelEngine::ParseAttributePaths(const Access::SubjectDesc
                                                paramsList.mValue.mAttributeId);
             if (ConcreteAttributePathExists(concretePath))
             {
-                Access::RequestPath requestPath{ .cluster = concretePath.mClusterId, .endpoint = concretePath.mEndpointId };
+                Access::RequestPath requestPath{ .cluster     = concretePath.mClusterId,
+                                                 .endpoint    = concretePath.mEndpointId,
+                                                 .requestType = Access::RequestType::kAttributeReadRequest,
+                                                 .entityId    = paramsList.mValue.mAttributeId };
 
                 err = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath,
                                                        RequiredPrivilege::ForReadAttribute(concretePath));
@@ -532,17 +538,27 @@ CHIP_ERROR InteractionModelEngine::ParseAttributePaths(const Access::SubjectDesc
     return err;
 }
 
-static bool CanAccess(const Access::SubjectDescriptor & aSubjectDescriptor, const ConcreteClusterPath & aPath,
-                      Access::Privilege aNeededPrivilege)
+#if !CHIP_CONFIG_ENABLE_EVENTLIST_ATTRIBUTE
+static bool CanAccessEvent(const Access::SubjectDescriptor & aSubjectDescriptor, const ConcreteClusterPath & aPath,
+                           Access::Privilege aNeededPrivilege)
 {
-    Access::RequestPath requestPath{ .cluster = aPath.mClusterId, .endpoint = aPath.mEndpointId };
+    Access::RequestPath requestPath{ .cluster     = aPath.mClusterId,
+                                     .endpoint    = aPath.mEndpointId,
+                                     .requestType = Access::RequestType::kEventReadOrSubscribeRequest };
+    // leave requestPath.entityId optional value unset to indicate wildcard
     CHIP_ERROR err = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath, aNeededPrivilege);
     return (err == CHIP_NO_ERROR);
 }
+#endif
 
-static bool CanAccess(const Access::SubjectDescriptor & aSubjectDescriptor, const ConcreteEventPath & aPath)
+static bool CanAccessEvent(const Access::SubjectDescriptor & aSubjectDescriptor, const ConcreteEventPath & aPath)
 {
-    return CanAccess(aSubjectDescriptor, aPath, RequiredPrivilege::ForReadEvent(aPath));
+    Access::RequestPath requestPath{ .cluster     = aPath.mClusterId,
+                                     .endpoint    = aPath.mEndpointId,
+                                     .requestType = Access::RequestType::kEventReadOrSubscribeRequest,
+                                     .entityId    = aPath.mEventId };
+    CHIP_ERROR err = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath, RequiredPrivilege::ForReadEvent(aPath));
+    return (err == CHIP_NO_ERROR);
 }
 
 /**
@@ -559,7 +575,7 @@ static bool HasValidEventPathForEndpointAndCluster(EndpointId aEndpoint, const E
         {
             ConcreteEventPath path(aEndpoint, aCluster->clusterId, aCluster->eventList[idx]);
             // If we get here, the path exists.  We just have to do an ACL check for it.
-            bool isValid = CanAccess(aSubjectDescriptor, path);
+            bool isValid = CanAccessEvent(aSubjectDescriptor, path);
             if (isValid)
             {
                 return true;
@@ -571,7 +587,7 @@ static bool HasValidEventPathForEndpointAndCluster(EndpointId aEndpoint, const E
         // We have no way to expand wildcards.  Just assume that we would need
         // View permissions for whatever events are involved.
         ConcreteClusterPath clusterPath(aEndpoint, aCluster->clusterId);
-        return CanAccess(aSubjectDescriptor, clusterPath, Access::Privilege::kView);
+        return CanAccessEvent(aSubjectDescriptor, clusterPath, Access::Privilege::kView);
 #endif
     }
 
@@ -581,7 +597,7 @@ static bool HasValidEventPathForEndpointAndCluster(EndpointId aEndpoint, const E
         // Not an existing event path.
         return false;
     }
-    return CanAccess(aSubjectDescriptor, path);
+    return CanAccessEvent(aSubjectDescriptor, path);
 }
 
 /**

diff --git a/src/app/clusters/icd-management-server/icd-management-server.cpp b/src/app/clusters/icd-management-server/icd-management-server.cpp
@@ -240,7 +240,10 @@ CHIP_ERROR IcdManagementAttributeAccess::ReadMaximumCheckInBackOff(EndpointId en
  */
 CHIP_ERROR CheckAdmin(CommandHandler * commandObj, const ConcreteCommandPath & commandPath, bool & isClientAdmin)
 {
-    RequestPath requestPath{ .cluster = commandPath.mClusterId, .endpoint = commandPath.mEndpointId };
+    RequestPath requestPath{ .cluster     = commandPath.mClusterId,
+                             .endpoint    = commandPath.mEndpointId,
+                             .requestType = RequestType::kCommandInvokeRequest,
+                             .entityId    = commandPath.mCommandId };
     CHIP_ERROR err = GetAccessControl().Check(commandObj->GetSubjectDescriptor(), requestPath, Privilege::kAdminister);
     if (CHIP_NO_ERROR == err)
     {

diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp
@@ -273,7 +273,10 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::ReadAttribute(const Data
     {
         ReturnErrorCodeIf(!request.subjectDescriptor.has_value(), CHIP_ERROR_INVALID_ARGUMENT);
 
-        Access::RequestPath requestPath{ .cluster = request.path.mClusterId, .endpoint = request.path.mEndpointId };
+        Access::RequestPath requestPath{ .cluster     = request.path.mClusterId,
+                                         .endpoint    = request.path.mEndpointId,
+                                         .requestType = Access::RequestType::kAttributeReadRequest,
+                                         .entityId    = request.path.mAttributeId };
         CHIP_ERROR err = Access::GetAccessControl().Check(*request.subjectDescriptor, requestPath,
                                                           RequiredPrivilege::ForReadAttribute(request.path));
         if (err != CHIP_NO_ERROR)

diff --git a/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp b/src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp
@@ -278,7 +278,10 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat
     {
         ReturnErrorCodeIf(!request.subjectDescriptor.has_value(), Status::UnsupportedAccess);
 
-        Access::RequestPath requestPath{ .cluster = request.path.mClusterId, .endpoint = request.path.mEndpointId };
+        Access::RequestPath requestPath{ .cluster     = request.path.mClusterId,
+                                         .endpoint    = request.path.mEndpointId,
+                                         .requestType = Access::RequestType::kAttributeWriteRequest,
+                                         .entityId    = request.path.mAttributeId };
         CHIP_ERROR err = Access::GetAccessControl().Check(*request.subjectDescriptor, requestPath,
                                                           RequiredPrivilege::ForWriteAttribute(request.path));
 

diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp
@@ -339,7 +339,10 @@ CHIP_ERROR Engine::CheckAccessDeniedEventPaths(TLV::TLVWriter & aWriter, bool &
             aHasEncodedData = true;
         }
 
-        Access::RequestPath requestPath{ .cluster = current->mValue.mClusterId, .endpoint = current->mValue.mEndpointId };
+        Access::RequestPath requestPath{ .cluster     = current->mValue.mClusterId,
+                                         .endpoint    = current->mValue.mEndpointId,
+                                         .requestType = RequestType::kEventReadOrSubscribeRequest,
+                                         .entityId    = current->mValue.mEventId };
         Access::Privilege requestPrivilege = RequiredPrivilege::ForReadEvent(path);
 
         err = Access::GetAccessControl().Check(apReadHandler->GetSubjectDescriptor(), requestPath, requestPrivilege);

diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp
@@ -294,7 +294,10 @@ CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, b
     // depending on whether the path was expanded.
 
     {
-        Access::RequestPath requestPath{ .cluster = aPath.mClusterId, .endpoint = aPath.mEndpointId };
+        Access::RequestPath requestPath{ .cluster     = aPath.mClusterId,
+                                         .endpoint    = aPath.mEndpointId,
+                                         .requestType = Access::RequestType::kAttributeReadRequest,
+                                         .entityId    = aPath.mAttributeId };
         Access::Privilege requestPrivilege = RequiredPrivilege::ForReadAttribute(aPath);
         CHIP_ERROR err                     = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath, requestPrivilege);
         if (err != CHIP_NO_ERROR)
@@ -686,7 +689,10 @@ CHIP_ERROR WriteSingleClusterData(const SubjectDescriptor & aSubjectDescriptor,
     }
 
     {
-        Access::RequestPath requestPath{ .cluster = aPath.mClusterId, .endpoint = aPath.mEndpointId };
+        Access::RequestPath requestPath{ .cluster     = aPath.mClusterId,
+                                         .endpoint    = aPath.mEndpointId,
+                                         .requestType = Access::RequestType::kAttributeWriteRequest,
+                                         .entityId    = aPath.mAttributeId };
         Access::Privilege requestPrivilege = RequiredPrivilege::ForWriteAttribute(aPath);
         CHIP_ERROR err                     = CHIP_NO_ERROR;
         if (!apWriteHandler->ACLCheckCacheHit({ aPath, requestPrivilege }))