Bypasses ACL checks when updating fabric scoped attributes in a sub (#…

…1223) * Bypasses ACL checks when updating fabric scoped attributes in a subscription * add todo * Enhance logging * Fix missing rename place * revert local testing changes
project-chip · Sep 21, 2024 · 28290b0 · 28290b0
1 parent cad50a6
commit 28290b0
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 8 deletions.
diff --git a/packages/matter.js/src/device/LegacyInteractionServer.ts b/packages/matter.js/src/device/LegacyInteractionServer.ts
@@ -92,8 +92,12 @@ export class LegacyInteractionServer extends InteractionServer {
         isFabricFiltered: boolean,
         message: Message,
         endpoint: EndpointInterface,
+        offline = false,
     ) {
-        this.#assertAccess(path, exchange, attribute.readAcl);
+        // Offline read do not require ACL checks
+        if (!offline) {
+            this.#assertAccess(path, exchange, attribute.readAcl);
+        }
         const data = await super.readAttribute(path, attribute, exchange, isFabricFiltered, message, endpoint);
         if (attribute instanceof FabricScopedAttributeServer && !isFabricFiltered) {
             const { value, version } = data;

diff --git a/packages/node/src/node/server/TransactionalInteractionServer.ts b/packages/node/src/node/server/TransactionalInteractionServer.ts
@@ -8,6 +8,7 @@ import { AccessControl } from "#behavior/AccessControl.js";
 import { ActionContext } from "#behavior/context/ActionContext.js";
 import { ActionTracer } from "#behavior/context/ActionTracer.js";
 import { NodeActivity } from "#behavior/context/NodeActivity.js";
+import { OfflineContext } from "#behavior/context/server/OfflineContext.js";
 import { OnlineContext } from "#behavior/context/server/OnlineContext.js";
 import { AccessControlCluster } from "#clusters/access-control";
 import { Endpoint } from "#endpoint/Endpoint.js";
@@ -153,8 +154,15 @@ export class TransactionalInteractionServer extends InteractionServer {
         fabricFiltered: boolean,
         message: Message,
         endpoint: EndpointInterface,
+        offline = false,
     ) {
-        const readAttribute = () => super.readAttribute(path, attribute, exchange, fabricFiltered, message, endpoint);
+        const readAttribute = () =>
+            super.readAttribute(path, attribute, exchange, fabricFiltered, message, endpoint, offline);
+
+        // Offline read do not require ACL checks
+        if (offline) {
+            return OfflineContext.act("offline-read", this.#activity, readAttribute);
+        }
 
         return OnlineContext({
             activity: (exchange as WithActivity)[activityKey],

diff --git a/packages/protocol/src/interaction/InteractionServer.ts b/packages/protocol/src/interaction/InteractionServer.ts
@@ -566,8 +566,9 @@ export class InteractionServer implements ProtocolHandler, InteractionRecipient
         isFabricFiltered: boolean,
         message: Message,
         _endpoint: EndpointInterface,
+        offline = false,
     ) {
-        return attribute.getWithVersion(exchange.session, isFabricFiltered, message);
+        return attribute.getWithVersion(exchange.session, isFabricFiltered, offline ? undefined : message);
     }
 
     protected async readEvent(
@@ -1025,14 +1026,15 @@ export class InteractionServer implements ProtocolHandler, InteractionRecipient
             maxIntervalCeiling: maxIntervalCeilingSeconds,
             cancelCallback: () => this.#subscriptionMap.delete(subscriptionId),
             subscriptionOptions: this.#subscriptionConfig,
-            readAttribute: (path, attribute) =>
+            readAttribute: (path, attribute, offline) =>
                 this.readAttribute(
                     path,
                     attribute,
                     exchange,
                     isFabricFiltered,
                     message,
                     this.#endpointStructure.getEndpoint(path.endpointId)!,
+                    offline,
                 ),
             readEvent: (path, event, eventFilters) =>
                 this.readEvent(

diff --git a/packages/protocol/src/interaction/SubscriptionHandler.ts b/packages/protocol/src/interaction/SubscriptionHandler.ts
@@ -78,7 +78,11 @@ export class SubscriptionHandler {
     private readonly eventFilters?: TypeFromSchema<typeof TlvEventFilter>[];
     private readonly isFabricFiltered: boolean;
     private readonly cancelCallback: () => void;
-    private readonly readAttribute: (path: AttributePath, attribute: AnyAttributeServer<any>) => Promise<any>;
+    private readonly readAttribute: (
+        path: AttributePath,
+        attribute: AnyAttributeServer<any>,
+        offline?: boolean,
+    ) => Promise<any>;
     private readonly readEvent: (
         path: EventPath,
         event: AnyEventServer<any, any>,
@@ -131,7 +135,7 @@ export class SubscriptionHandler {
         maxIntervalCeiling: number;
         cancelCallback: () => void;
         subscriptionOptions: SubscriptionOptions.Configuration;
-        readAttribute: (path: AttributePath, attribute: AnyAttributeServer<any>) => Promise<any>;
+        readAttribute: (path: AttributePath, attribute: AnyAttributeServer<any>, offline?: boolean) => Promise<any>;
         readEvent: (
             path: EventPath,
             event: AnyEventServer<any, any>,
@@ -737,8 +741,8 @@ export class SubscriptionHandler {
         if (attribute instanceof FabricScopedAttributeServer) {
             // We cannot be sure what value we got for fabric filtered attributes (and from which fabric),
             // so get it again for this relevant fabric. This also makes sure that fabric sensitive fields are filtered
-            // TODO: Maybe add try/catch when we add ACL handling and ignore the update if we cannot get the value?
-            return this.readAttribute(path, attribute).then(({ value }) => {
+            // TODO: Remove this once we remove the legacy API and go away from using AttributeServers in the background
+            return this.readAttribute(path, attribute, true).then(({ value }) => {
                 this.outstandingAttributeUpdates.set(attributePathToId(path), {
                     attribute,
                     path,

diff --git a/packages/protocol/src/session/SecureSession.ts b/packages/protocol/src/session/SecureSession.ts
@@ -168,6 +168,7 @@ export class SecureSession extends Session {
                 interactionModelRevision: this.interactionModelRevision,
                 specificationVersion: this.specificationVersion,
                 maxPathsPerInvoke: this.maxPathsPerInvoke,
+                caseAuthenticatedTags: this.#caseAuthenticatedTags,
             }),
         );
     }