Omit fabric sensitive data for Reads

This builds upon the PR project-chip#15284 to omit fabric sensitive fields in the
call to `EncodeForRead` if the fabric index present in the object
doesn't match the accessing fabric.

The test-cluster.xml has been updated to enhance the TestFabricScoped
struct to include a number of different data types to ensure adequate
coverage.

Cluster definitions for OperationalCredentials and Acl clusters have
been updated to reflect the spec correctly w.r.t fabric sensitive
fields.

Tests:

Adds a YAML TestMultiFabric test that writes/reads fabric-sensitive data
from multiple fabrics and validates the read back data.

Adds a similar Python version of the test that is identical but is
more robust towards the fabric indices and order of data read back from
the server.

examples/chip-tool/BUILD.gn

@@ -69,6 +69,7 @@ static_library("chip-tool-utils") {
 
   public_deps = [
     "${chip_root}/src/app/server",
+    "${chip_root}/src/app/tests/suites/commands/commissioner",
     "${chip_root}/src/app/tests/suites/commands/delay",
     "${chip_root}/src/app/tests/suites/commands/discovery",
     "${chip_root}/src/app/tests/suites/commands/log",

examples/chip-tool/commands/tests/TestCommand.h

@@ -19,6 +19,7 @@
 #pragma once
 
 #include "../common/CHIPCommand.h"
+#include <app/tests/suites/commands/commissioner/CommissionerCommands.h>
 #include <app/tests/suites/commands/delay/DelayCommands.h>
 #include <app/tests/suites/commands/discovery/DiscoveryCommands.h>
 #include <app/tests/suites/commands/log/LogCommands.h>
@@ -36,6 +37,7 @@ class TestCommand : public CHIPCommand,
                     public ConstraintsChecker,
                     public PICSChecker,
                     public LogCommands,
+                    public CommissionerCommands,
                     public DiscoveryCommands,
                     public SystemCommands,
                     public DelayCommands
@@ -77,6 +79,8 @@ class TestCommand : public CHIPCommand,
         return CHIP_NO_ERROR;
     }
 
+    chip::Controller::DeviceCommissioner & GetCurrentCommissioner() override { return CurrentCommissioner(); };
+
     void Exit(std::string message) override;
     void ThrowFailureResponse();
     void ThrowSuccessResponse();

examples/chip-tool/templates/tests.js

@@ -443,6 +443,7 @@ function getTests()
   const Others = [
     'TestCluster',
     'TestClusterComplexTypes',
+    'TestClusterMultiFabric',
     'TestConstraints',
     'TestDelayCommands',
     'TestLogCommands',

src/app/clusters/test-cluster-server/test-cluster-server.cpp

@@ -75,6 +75,8 @@ class TestAttrAccess : public AttributeAccessInterface
     CHIP_ERROR Write(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder) override;
 
 private:
+    CHIP_ERROR WriteListFabricScopedListEntry(const Structs::TestFabricScoped::DecodableType & entry, size_t index);
+
     CHIP_ERROR ReadListInt8uAttribute(AttributeValueEncoder & aEncoder);
     CHIP_ERROR WriteListInt8uAttribute(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder);
     CHIP_ERROR ReadListOctetStringAttribute(AttributeValueEncoder & aEncoder);
@@ -91,6 +93,7 @@ class TestAttrAccess : public AttributeAccessInterface
     CHIP_ERROR ReadNullableStruct(AttributeValueEncoder & aEncoder);
     CHIP_ERROR WriteNullableStruct(AttributeValueDecoder & aDecoder);
     CHIP_ERROR ReadListFabricScopedAttribute(AttributeValueEncoder & aEncoder);
+    CHIP_ERROR WriteListFabricScopedAttribute(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder);
 };
 
 TestAttrAccess gAttrAccess;
@@ -106,6 +109,11 @@ OctetStringData gStructAttributeByteSpanData;
 Structs::SimpleStruct::Type gStructAttributeValue;
 NullableStruct::TypeInfo::Type gNullableStructAttributeValue;
 
+TestCluster::Structs::TestFabricScoped::Type gListFabricScopedAttributeValue[kAttributeListLength];
+uint8_t gListFabricScoped_fabricSensitiveInt8uList[kAttributeListLength][64];
+size_t gListFabricScopedAttributeLen = 0;
+char gListFabricScoped_fabricSensitiveCharBuf[kAttributeListLength][128];
+
 //                                                     /16             /32             /48             /64
 const char sLongOctetStringBuf[513] = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"  // 64
                                       "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"  // 128
@@ -177,6 +185,9 @@ CHIP_ERROR TestAttrAccess::Write(const ConcreteDataAttributePath & aPath, Attrib
     case ListLongOctetString::Id: {
         return WriteListLongOctetStringAttribute(aPath, aDecoder);
     }
+    case ListFabricScoped::Id: {
+        return WriteListFabricScopedAttribute(aPath, aDecoder);
+    }
     case ListStructOctetString::Id: {
         return WriteListStructOctetStringAttribute(aPath, aDecoder);
     }
@@ -538,18 +549,132 @@ CHIP_ERROR TestAttrAccess::WriteStructAttribute(AttributeValueDecoder & aDecoder
 CHIP_ERROR TestAttrAccess::ReadListFabricScopedAttribute(AttributeValueEncoder & aEncoder)
 {
     return aEncoder.EncodeList([](const auto & encoder) -> CHIP_ERROR {
-        chip::app::Clusters::TestCluster::Structs::TestFabricScoped::Type val;
-
-        for (const auto & fb : Server::GetInstance().GetFabricTable())
+        for (size_t index = 0; index < gListFabricScopedAttributeLen; index++)
         {
-            val.fabricIndex = fb.GetFabricIndex();
-            ReturnErrorOnFailure(encoder.Encode(val));
+            ReturnErrorOnFailure(encoder.Encode(gListFabricScopedAttributeValue[index]));
         }
 
         return CHIP_NO_ERROR;
     });
 }
 
+CHIP_ERROR TestAttrAccess::WriteListFabricScopedListEntry(const Structs::TestFabricScoped::DecodableType & entry, size_t index)
+{
+    VerifyOrReturnError(index < kAttributeListLength, CHIP_ERROR_BUFFER_TOO_SMALL);
+
+    //
+    // The fabric index in the entry has already been set to the right index
+    // by the decoder.
+    //
+    gListFabricScopedAttributeValue[index].fabricIndex = entry.fabricIndex;
+
+    gListFabricScopedAttributeValue[index].optionalFabricSensitiveInt8u         = entry.optionalFabricSensitiveInt8u;
+    gListFabricScopedAttributeValue[index].nullableFabricSensitiveInt8u         = entry.nullableFabricSensitiveInt8u;
+    gListFabricScopedAttributeValue[index].nullableOptionalFabricSensitiveInt8u = entry.nullableOptionalFabricSensitiveInt8u;
+
+    VerifyOrReturnError(entry.fabricSensitiveCharString.size() < 128, CHIP_ERROR_BUFFER_TOO_SMALL);
+    strncpy(gListFabricScoped_fabricSensitiveCharBuf[index], entry.fabricSensitiveCharString.begin(),
+            entry.fabricSensitiveCharString.size());
+    gListFabricScopedAttributeValue[index].fabricSensitiveCharString =
+        CharSpan(gListFabricScoped_fabricSensitiveCharBuf[index], entry.fabricSensitiveCharString.size());
+
+    //
+    // For now, we're not permitting the SimpleStruct's contents to have valid strings, since that just
+    // increases the complexity of this logic. We don't really need to validate that since there are other tests
+    // that validate that struct, so let's just do the bare minimum here.
+    //
+    VerifyOrReturnError(entry.fabricSensitiveStruct.d.size() == 0, CHIP_ERROR_INVALID_ARGUMENT);
+    VerifyOrReturnError(entry.fabricSensitiveStruct.e.size() == 0, CHIP_ERROR_INVALID_ARGUMENT);
+
+    gListFabricScopedAttributeValue[index].fabricSensitiveStruct = entry.fabricSensitiveStruct;
+    gListFabricScopedAttributeValue[index].fabricSensitiveInt8u  = entry.fabricSensitiveInt8u;
+
+    auto intIter = entry.fabricSensitiveInt8uList.begin();
+    size_t i     = 0;
+    while (intIter.Next())
+    {
+        VerifyOrReturnError(i < 64, CHIP_ERROR_BUFFER_TOO_SMALL);
+        gListFabricScoped_fabricSensitiveInt8uList[index][i++] = intIter.GetValue();
+    }
+    ReturnErrorOnFailure(intIter.GetStatus());
+
+    gListFabricScopedAttributeValue[index].fabricSensitiveInt8uList =
+        DataModel::List<uint8_t>(gListFabricScoped_fabricSensitiveInt8uList[index], i);
+
+    return CHIP_NO_ERROR;
+}
+
+CHIP_ERROR TestAttrAccess::WriteListFabricScopedAttribute(const ConcreteDataAttributePath & aPath, AttributeValueDecoder & aDecoder)
+{
+    if (!aPath.IsListItemOperation())
+    {
+        ListFabricScoped::TypeInfo::DecodableType list;
+
+        ReturnErrorOnFailure(aDecoder.Decode(list));
+
+        //
+        // Delete all existing entries matching the accessing fabric. This is achieved by 'shifting down'
+        // entries that don't match the accessing fabric into the slots occupied by the deleted entries.
+        //
+        size_t srcIndex = 0, dstIndex = 0;
+        while (srcIndex < gListFabricScopedAttributeLen)
+        {
+            if (gListFabricScopedAttributeValue[srcIndex].fabricIndex != aDecoder.AccessingFabricIndex())
+            {
+                auto & dstEntry = gListFabricScopedAttributeValue[dstIndex];
+                auto & srcEntry = gListFabricScopedAttributeValue[srcIndex];
+
+                dstEntry = srcEntry;
+
+                //
+                // We copy the data referenced by spans over to the right slot in the backing buffers.
+                //
+                strncpy(gListFabricScoped_fabricSensitiveCharBuf[dstIndex], srcEntry.fabricSensitiveCharString.begin(),
+                        srcEntry.fabricSensitiveCharString.size());
+                dstEntry.fabricSensitiveCharString =
+                    CharSpan(gListFabricScoped_fabricSensitiveCharBuf[dstIndex], srcEntry.fabricSensitiveCharString.size());
+
+                memcpy(gListFabricScoped_fabricSensitiveInt8uList[dstIndex], gListFabricScoped_fabricSensitiveInt8uList[srcIndex],
+                       srcEntry.fabricSensitiveInt8uList.size() * sizeof(uint8_t));
+                gListFabricScopedAttributeValue[dstIndex].fabricSensitiveInt8uList = DataModel::List<uint8_t>(
+                    gListFabricScoped_fabricSensitiveInt8uList[dstIndex], srcEntry.fabricSensitiveInt8uList.size());
+
+                dstIndex++;
+            }
+
+            srcIndex++;
+        }
+
+        size_t size;
+        ReturnErrorOnFailure(list.ComputeSize(&size));
+
+        auto iter = list.begin();
+        while (iter.Next())
+        {
+            auto & entry = iter.GetValue();
+            ReturnErrorOnFailure(WriteListFabricScopedListEntry(entry, dstIndex++));
+        }
+
+        gListFabricScopedAttributeLen = dstIndex;
+        return iter.GetStatus();
+    }
+    else if (aPath.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
+    {
+        VerifyOrReturnError(gListFabricScopedAttributeLen < kAttributeListLength, CHIP_ERROR_INVALID_ARGUMENT);
+
+        Structs::TestFabricScoped::DecodableType listEntry;
+        ReturnErrorOnFailure(aDecoder.Decode(listEntry));
+        ReturnErrorOnFailure(WriteListFabricScopedListEntry(listEntry, gListFabricScopedAttributeLen));
+
+        gListFabricScopedAttributeLen++;
+        return CHIP_NO_ERROR;
+    }
+    else
+    {
+        return CHIP_ERROR_UNSUPPORTED_CHIP_FEATURE;
+    }
+}
+
 } // namespace
 
 bool emberAfTestClusterClusterTestCallback(app::CommandHandler *, const app::ConcreteCommandPath & commandPath,

src/app/data-model/DecodableList.h

@@ -155,6 +155,15 @@ class DecodableList
 
             if (mStatus == CHIP_NO_ERROR)
             {
+                //
+                // Re-construct mValue to reset its state back to cluster object defaults.
+                // This is especially important when decoding successive list elements
+                // that do not contain all of the fields for a given struct because
+                // they are marked optional/fabric-sensitive. Without this re-construction,
+                // data from previous decode attempts will continue to linger and give
+                // an incorrect view of the state as seen from a client.
+                //
+                mValue  = T();
                 mStatus = DataModel::Decode(mReader, mValue);
             }
 

src/app/zap-templates/partials/cluster-objects-struct.zapt

@@ -78,15 +78,22 @@ CHIP_ERROR Type::EncodeForRead(TLV::TLVWriter &writer, TLV::Tag tag, FabricIndex
 
 CHIP_ERROR Type::DoEncode(TLV::TLVWriter &writer, TLV::Tag tag, const Optional<FabricIndex> & accessingFabricIndex) const
 {
+    {{#if struct_has_sensitive_fields}}
+    bool includeSensitive = !accessingFabricIndex.HasValue() || (accessingFabricIndex.HasValue() && accessingFabricIndex.Value() == {{ asLowerCamelCase struct_fabric_idx_field }});
+    {{/if}}
     TLV::TLVType outer;
     ReturnErrorOnFailure(writer.StartContainer(tag, TLV::kTLVType_Structure, outer));
     {{#zcl_struct_items}}
     {{#if (isStrEqual label ../struct_fabric_idx_field)}}
-      if (accessingFabricIndex.HasValue()) {
-        ReturnErrorOnFailure(DataModel::Encode(writer, TLV::ContextTag(to_underlying(Fields::k{{asUpperCamelCase label}})), {{asLowerCamelCase label}}));
-      }
+    if (accessingFabricIndex.HasValue()) {
+      ReturnErrorOnFailure(DataModel::Encode(writer, TLV::ContextTag(to_underlying(Fields::k{{asUpperCamelCase label}})), {{asLowerCamelCase label}}));
+    }
+    {{else if isFabricSensitive}}
+    if (includeSensitive) {
+       ReturnErrorOnFailure(DataModel::Encode(writer, TLV::ContextTag(to_underlying(Fields::k{{asUpperCamelCase label}})), {{asLowerCamelCase label}}));
+    }
     {{else}}
-        ReturnErrorOnFailure(DataModel::Encode(writer, TLV::ContextTag(to_underlying(Fields::k{{asUpperCamelCase label}})), {{asLowerCamelCase label}}));
+    ReturnErrorOnFailure(DataModel::Encode(writer, TLV::ContextTag(to_underlying(Fields::k{{asUpperCamelCase label}})), {{asLowerCamelCase label}}));
     {{/if}}
     {{/zcl_struct_items}}
     ReturnErrorOnFailure(writer.EndContainer(outer));

src/app/zap-templates/templates/app/helper.js

@@ -602,7 +602,11 @@ async function _zapTypeToPythonClusterObjectType(type, options)
       return 'bytes';
     }
 
-    if ([ 'single', 'double' ].includes(type.toLowerCase())) {
+    if (type.toLowerCase() == 'single') {
+      return 'float32';
+    }
+
+    if (type.toLowerCase() == 'double') {
       return 'float';
     }
 

src/app/zap-templates/zcl/data-model/chip/access-control-cluster.xml

@@ -49,11 +49,11 @@ limitations under the License.
 
   <struct name="AccessControlEntry">
     <cluster code="0x001F"/>
-    <item fieldId="0" name="FabricIndex" type="fabric_idx"/>
-    <item fieldId="1" name="Privilege" type="Privilege"/>
-    <item fieldId="2" name="AuthMode" type="AuthMode"/>
-    <item fieldId="3" name="Subjects" type="INT64U" isNullable="true" array="true"/>
-    <item fieldId="4" name="Targets" type="Target" isNullable="true" array="true"/>
+    <item fieldId="0" name="FabricIndex" type="fabric_idx" isFabricSensitive="true"/>
+    <item fieldId="1" name="Privilege" type="Privilege" isFabricSensitive="true"/>
+    <item fieldId="2" name="AuthMode" type="AuthMode" isFabricSensitive="true"/>
+    <item fieldId="3" name="Subjects" type="INT64U" isNullable="true" array="true" isFabricSensitive="true"/>
+    <item fieldId="4" name="Targets" type="Target" isNullable="true" array="true" isFabricSensitive="true"/>
   </struct>
 
   <struct name="ExtensionEntry">

src/app/zap-templates/zcl/data-model/chip/operational-credentials-cluster.xml

@@ -44,8 +44,8 @@ limitations under the License.
   <struct name="NOCStruct">
     <cluster code="0x003E"/>
     <item name="FabricIndex" type="fabric_idx"/>
-    <item name="NOC" type="OCTET_STRING"/>
-    <item name="ICAC" type="OCTET_STRING" isNullable="true"/>    
+    <item name="NOC" type="OCTET_STRING" isFabricSensitive="true"/>
+    <item name="ICAC" type="OCTET_STRING" isNullable="true" isFabricSensitive="true"/>    
   </struct>
 
   <cluster>

src/app/zap-templates/zcl/data-model/chip/test-cluster.xml

@@ -25,6 +25,13 @@ limitations under the License.
   <struct name="TestFabricScoped">
     <cluster code="0x050F"/>
     <item name="fabricIndex" type="fabric_idx"/>
+    <item name="fabricSensitiveInt8u" type="INT8U" isFabricSensitive="true"/>
+    <item name="optionalFabricSensitiveInt8u" type="INT8U" optional="true" isFabricSensitive="true"/>
+    <item name="nullableFabricSensitiveInt8u" type="INT8U" isNullable="true" isFabricSensitive="true"/>
+    <item name="nullableOptionalFabricSensitiveInt8u" type="INT8U" optional="true" isNullable="true" isFabricSensitive="true"/>
+    <item name="fabricSensitiveCharString" type="CHAR_STRING" isFabricSensitive="true"/>
+    <item name="fabricSensitiveStruct" type="SimpleStruct" isFabricSensitive="true"/>
+    <item name="fabricSensitiveInt8uList" type="INT8U" array="true" isFabricSensitive="true"/>
   </struct>
 
   <enum name="SimpleEnum" type="ENUM8">
@@ -150,7 +157,7 @@ limitations under the License.
     <attribute side="server" code="0x0028" define="RANGE_RESTRICTED_INT16U" type="int16u" min="100" max="1000" writable="true" optional="false" default="200">range_restricted_int16u</attribute>
     <attribute side="server" code="0x0029" define="RANGE_RESTRICTED_INT16s" type="int16s" min="-150" max="200" writable="true" optional="false" default="-5">range_restricted_int16s</attribute>
     <attribute side="server" code="0x002A" define="LIST_LONG_OCTET_STRING" type="ARRAY" entryType="LONG_OCTET_STRING" length="1000" writable="true" optional="false">list_long_octet_string</attribute>
-    <attribute side="server" code="0x002B" define="LIST_FABRIC_SCOPED" type="ARRAY" entryType="TestFabricScoped" length="10" writable="false" optional="false">list_fabric_scoped</attribute>
+    <attribute side="server" code="0x002B" define="LIST_FABRIC_SCOPED" type="ARRAY" entryType="TestFabricScoped" length="10" writable="true" optional="false">list_fabric_scoped</attribute>
     <attribute side="server" code="0x0030" define="TIMED_WRITE_BOOLEAN"
                type="BOOLEAN" writable="true" mustUseTimedWrite="true">timed_write_boolean</attribute>
     <!-- Reading/writing the following two attributes will respond with a

src/controller/data_model/controller-clusters.zap

@@ -17029,7 +17029,7 @@
               "code": 43,
               "mfgCode": null,
               "side": "server",
-              "included": 0,
+              "included": 1,
               "storageOption": "RAM",
               "singleton": 0,
               "bounded": 0,
@@ -17948,4 +17948,4 @@
     }
   ],
   "log": []
-}
+}

src/controller/python/chip/tlv/__init__.py

@@ -123,6 +123,17 @@ def __init__(self, val: int):
                 'expecting positive value, got negative value of %d instead' % val)
 
 
+class float32(float):
+    ''' A type for single precision floats distinct from the double precision 'float'
+        type offered by default in Python. This type distinction is present in the Matter
+        data model types so we need it here as well.
+
+        It is backed by an ordinary float, which means there will be precision loss at the time
+        the value is converted to TLV.
+    '''
+    pass
+
+
 class TLVWriter(object):
     def __init__(self, encoding=None, implicitProfile=None):
         self._encoding = encoding if encoding is not None else bytearray()
@@ -194,8 +205,10 @@ def put(self, tag, val):
             self.putUnsignedInt(tag, val)
         elif isinstance(val, int):
             self.putSignedInt(tag, val)
-        elif isinstance(val, float):
+        elif isinstance(val, float32):
             self.putFloat(tag, val)
+        elif isinstance(val, float):
+            self.putDouble(tag, val)
         elif isinstance(val, str):
             self.putString(tag, val)
         elif isinstance(val, bytes) or isinstance(val, bytearray):
@@ -248,6 +261,15 @@ def putUnsignedInt(self, tag, val):
 
     def putFloat(self, tag, val):
         """Write a value as a TLV float with the specified TLV tag."""
+        val = struct.pack("f", val)
+        controlAndTag = self._encodeControlAndTag(
+            TLV_TYPE_FLOATING_POINT_NUMBER, tag, lenOfLenOrVal=len(val)
+        )
+        self._encoding.extend(controlAndTag)
+        self._encoding.extend(val)
+
+    def putDouble(self, tag, val):
+        """Write a value as a TLV double with the specified TLV tag."""
         val = struct.pack("d", val)
         controlAndTag = self._encodeControlAndTag(
             TLV_TYPE_FLOATING_POINT_NUMBER, tag, lenOfLenOrVal=len(val)

src/controller/python/templates/python-cluster-Objects-py.zapt

@@ -10,7 +10,7 @@ import typing
 from enum import IntEnum
 from chip import ChipUtility
 
-from chip.tlv import uint
+from chip.tlv import uint, float32
 
 from .ClusterObjects import ClusterObject, ClusterObjectDescriptor, ClusterObjectFieldDescriptor, ClusterCommand, ClusterAttributeDescriptor, Cluster, ClusterEvent
 from .Types import Nullable, NullValue