Omission of fabric sensitive fields (#15340)

* Omit fabric sensitive data for Reads

This builds upon the PR #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.

* Re-run codegen

* Forgot to add the MultiFabric YAML test

* Re-run codegen and review fixes

* Disabling the multi-fabric test for now since it's not quite working yet even on master

* Rerun code gen

* Re-gen

* Minor indent fix

* Disable more fabric-scoped tests in Python while we wait for #15688 to get resolved

examples/all-clusters-app/all-clusters-common/all-clusters-app.matter

@@ -2855,6 +2855,13 @@ server cluster TestCluster = 1295 {
 
   struct TestFabricScoped {
     fabric_idx fabricIndex = 0;
+    INT8U fabricSensitiveInt8u = 1;
+    optional INT8U optionalFabricSensitiveInt8u = 2;
+    nullable INT8U nullableFabricSensitiveInt8u = 3;
+    optional nullable INT8U nullableOptionalFabricSensitiveInt8u = 4;
+    CHAR_STRING fabricSensitiveCharString = 5;
+    SimpleStruct fabricSensitiveStruct = 6;
+    INT8U fabricSensitiveInt8uList[] = 7;
   }
 
   struct NestedStructList {
@@ -2929,7 +2936,7 @@ server cluster TestCluster = 1295 {
   attribute int16u rangeRestrictedInt16u = 40;
   attribute int16s rangeRestrictedInt16s = 41;
   attribute LONG_OCTET_STRING listLongOctetString[] = 42;
-  readonly attribute TestFabricScoped listFabricScoped[] = 43;
+  attribute TestFabricScoped listFabricScoped[] = 43;
   attribute boolean timedWriteBoolean = 48;
   attribute boolean generalErrorBoolean = 49;
   attribute boolean clusterErrorBoolean = 50;

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();

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

@@ -48,6 +48,12 @@ constexpr uint8_t kAttributeListLength = 4;
 // The maximum length of the test attribute list element in bytes
 constexpr uint8_t kAttributeEntryLength = 6;
 
+// The maximum length of the fabric sensitive string within the TestFabricScoped struct.
+constexpr uint8_t kFabricSensitiveCharLength = 128;
+
+// The maximum length of the fabric sensitive integer list within the TestFabricScoped struct.
+constexpr uint8_t kFabricSensitiveIntListLength = 8;
+
 namespace {
 
 class OctetStringData
@@ -75,6 +81,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 +99,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 +115,11 @@ OctetStringData gStructAttributeByteSpanData;
 Structs::SimpleStruct::Type gStructAttributeValue;
 NullableStruct::TypeInfo::Type gNullableStructAttributeValue;
 
+TestCluster::Structs::TestFabricScoped::Type gListFabricScopedAttributeValue[kAttributeListLength];
+uint8_t gListFabricScoped_fabricSensitiveInt8uList[kAttributeListLength][kFabricSensitiveIntListLength];
+size_t gListFabricScopedAttributeLen = 0;
+char gListFabricScoped_fabricSensitiveCharBuf[kAttributeListLength][kFabricSensitiveCharLength];
+
 //                                                     /16             /32             /48             /64
 const char sLongOctetStringBuf[513] = "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"  // 64
                                       "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"  // 128
@@ -177,6 +191,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 +555,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() < kFabricSensitiveCharLength, CHIP_ERROR_BUFFER_TOO_SMALL);
+    memcpy(gListFabricScoped_fabricSensitiveCharBuf[index], entry.fabricSensitiveCharString.data(),
+           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 < kFabricSensitiveIntListLength, 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.
+                //
+                memcpy(gListFabricScoped_fabricSensitiveCharBuf[dstIndex], srcEntry.fabricSensitiveCharString.data(),
+                       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);
             }