diff --git a/src/access/AccessControl.cpp b/src/access/AccessControl.cpp index df3a5b8e8fc286..001741efb1f176 100644 --- a/src/access/AccessControl.cpp +++ b/src/access/AccessControl.cpp @@ -317,6 +317,10 @@ bool AccessControl::IsValid(const Entry & entry) SuccessOrExit(entry.GetSubjectCount(subjectCount)); SuccessOrExit(entry.GetTargetCount(targetCount)); + ChipLogDetail(DataManagement, "AccessControl: validating f=%u p=%c a=%c s=%d t=%d", fabricIndex, + GetPrivilegeStringForLogging(privilege), GetAuthModeStringForLogging(authMode), static_cast(subjectCount), + static_cast(targetCount)); + // Fabric index must be defined. VerifyOrExit(fabricIndex != kUndefinedFabricIndex, log = "invalid fabric index"); @@ -338,6 +342,7 @@ bool AccessControl::IsValid(const Entry & entry) SuccessOrExit(entry.GetSubject(i, subject)); const bool kIsCase = authMode == AuthMode::kCase; const bool kIsGroup = authMode == AuthMode::kGroup; + ChipLogDetail(DataManagement, " validating subject 0x" ChipLogFormatX64, ChipLogValueX64(subject)); VerifyOrExit((kIsCase && IsValidCaseNodeId(subject)) || (kIsGroup && IsValidGroupNodeId(subject)), log = "invalid subject"); } diff --git a/src/app/clusters/access-control-server/access-control-server.cpp b/src/app/clusters/access-control-server/access-control-server.cpp index 5e85d2afe23027..d6ba6912345bbd 100644 --- a/src/app/clusters/access-control-server/access-control-server.cpp +++ b/src/app/clusters/access-control-server/access-control-server.cpp @@ -37,6 +37,12 @@ namespace AccessControlCluster = chip::app::Clusters::AccessControl; namespace { +struct Subject +{ + NodeId nodeId; + AccessControlCluster::AuthMode authMode; +}; + struct AccessControlEntryCodec { static CHIP_ERROR Convert(AuthMode from, AccessControlCluster::AuthMode & to) @@ -127,6 +133,48 @@ struct AccessControlEntryCodec return CHIP_NO_ERROR; } + static CHIP_ERROR Convert(NodeId from, Subject & to) + { + if (IsOperationalNodeId(from) || IsCASEAuthTag(from)) + { + to = { .nodeId = from, .authMode = AccessControlCluster::AuthMode::kCase }; + } + else if (IsGroupId(from)) + { + to = { .nodeId = GroupIdFromNodeId(from), .authMode = AccessControlCluster::AuthMode::kGroup }; + } + else if (IsPAKEKeyId(from)) + { + to = { .nodeId = PAKEKeyIdFromNodeId(from), .authMode = AccessControlCluster::AuthMode::kPase }; + } + else + { + return CHIP_ERROR_INVALID_ARGUMENT; + } + return CHIP_NO_ERROR; + } + + static CHIP_ERROR Convert(Subject from, NodeId & to) + { + switch (from.authMode) + { + case AccessControlCluster::AuthMode::kPase: + ReturnErrorCodeIf(from.nodeId & ~kMaskPAKEKeyId, CHIP_ERROR_INVALID_ARGUMENT); + to = NodeIdFromPAKEKeyId(static_cast(from.nodeId)); + break; + case AccessControlCluster::AuthMode::kCase: + to = from.nodeId; + break; + case AccessControlCluster::AuthMode::kGroup: + ReturnErrorCodeIf(from.nodeId & ~kMaskGroupId, CHIP_ERROR_INVALID_ARGUMENT); + to = NodeIdFromGroupId(static_cast(from.nodeId)); + break; + default: + return CHIP_ERROR_INVALID_ARGUMENT; + } + return CHIP_NO_ERROR; + } + static CHIP_ERROR Convert(const AccessControl::Entry::Target & from, AccessControlCluster::Structs::Target::Type & to) { if (from.flags & AccessControl::Entry::Target::kCluster) @@ -202,7 +250,10 @@ struct AccessControlEntryCodec { for (size_t i = 0; i < subjectCount; ++i) { - ReturnErrorOnFailure(entry.GetSubject(i, subjectBuffer[i])); + Subject subject; + ReturnErrorOnFailure(entry.GetSubject(i, subject.nodeId)); + ReturnErrorOnFailure(Convert(subject.nodeId, subject)); + subjectBuffer[i] = subject.nodeId; } staging.subjects.SetNonNull(subjectBuffer, subjectCount); } @@ -251,7 +302,9 @@ struct AccessControlEntryCodec auto iterator = staging.subjects.Value().begin(); while (iterator.Next()) { - ReturnErrorOnFailure(entry.AddSubject(nullptr, iterator.GetValue())); + Subject subject = { .nodeId = iterator.GetValue(), .authMode = staging.authMode }; + ReturnErrorOnFailure(Convert(subject, subject.nodeId)); + ReturnErrorOnFailure(entry.AddSubject(nullptr, subject.nodeId)); } ReturnErrorOnFailure(iterator.GetStatus()); } diff --git a/src/app/tests/suites/TestGroupMessaging.yaml b/src/app/tests/suites/TestGroupMessaging.yaml index 58a548ed6cf42e..76be04d23bc7c7 100644 --- a/src/app/tests/suites/TestGroupMessaging.yaml +++ b/src/app/tests/suites/TestGroupMessaging.yaml @@ -115,6 +115,30 @@ tests: { FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 }, ] + - label: "Install ACLs for test" + cluster: "Access Control" + command: "writeAttribute" + attribute: "ACL" + arguments: + value: [ + # Any CASE can admin + { + FabricIndex: 1, + Privilege: 5, + AuthMode: 2, + Subjects: null, + Targets: null, + }, + # These groups can operate + { + FabricIndex: 1, + Privilege: 3, + AuthMode: 3, + Subjects: [0x0101, 0x0102], + Targets: null, + }, + ] + # Test Pair 1 : Sends a Group Write Attribute - label: "Group Write Attribute" command: "writeAttribute" diff --git a/zzz_generated/chip-tool/zap-generated/test/Commands.h b/zzz_generated/chip-tool/zap-generated/test/Commands.h index fe03e6fff94802..d2d6507a219e5a 100644 --- a/zzz_generated/chip-tool/zap-generated/test/Commands.h +++ b/zzz_generated/chip-tool/zap-generated/test/Commands.h @@ -91489,29 +91489,33 @@ class TestGroupMessaging : public TestCommand err = TestWriteGroupKeys_5(); break; case 6: - ChipLogProgress(chipTool, " ***** Test Step 6 : Group Write Attribute\n"); - err = TestGroupWriteAttribute_6(); + ChipLogProgress(chipTool, " ***** Test Step 6 : Install ACLs for test\n"); + err = TestInstallACLsForTest_6(); break; case 7: - ChipLogProgress(chipTool, " ***** Test Step 7 : Read back Attribute\n"); - err = TestReadBackAttribute_7(); + ChipLogProgress(chipTool, " ***** Test Step 7 : Group Write Attribute\n"); + err = TestGroupWriteAttribute_7(); break; case 8: - ChipLogProgress(chipTool, " ***** Test Step 8 : Restore initial location value\n"); - err = TestRestoreInitialLocationValue_8(); + ChipLogProgress(chipTool, " ***** Test Step 8 : Read back Attribute\n"); + err = TestReadBackAttribute_8(); break; case 9: - ChipLogProgress(chipTool, " ***** Test Step 9 : Read back Attribute\n"); - err = TestReadBackAttribute_9(); + ChipLogProgress(chipTool, " ***** Test Step 9 : Restore initial location value\n"); + err = TestRestoreInitialLocationValue_9(); break; case 10: - ChipLogProgress(chipTool, " ***** Test Step 10 : Turn On the light to see attribute change\n"); - err = TestTurnOnTheLightToSeeAttributeChange_10(); + ChipLogProgress(chipTool, " ***** Test Step 10 : Read back Attribute\n"); + err = TestReadBackAttribute_10(); break; case 11: + ChipLogProgress(chipTool, " ***** Test Step 11 : Turn On the light to see attribute change\n"); + err = TestTurnOnTheLightToSeeAttributeChange_11(); + break; + case 12: ChipLogProgress(chipTool, - " ***** Test Step 11 : Check on/off attribute value is true after on command for endpoint 1\n"); - err = TestCheckOnOffAttributeValueIsTrueAfterOnCommandForEndpoint1_11(); + " ***** Test Step 12 : Check on/off attribute value is true after on command for endpoint 1\n"); + err = TestCheckOnOffAttributeValueIsTrueAfterOnCommandForEndpoint1_12(); break; } @@ -91524,7 +91528,7 @@ class TestGroupMessaging : public TestCommand private: std::atomic_uint16_t mTestIndex; - const uint16_t mTestCount = 12; + const uint16_t mTestCount = 13; chip::Optional mNodeId; chip::Optional mCluster; @@ -91545,8 +91549,6 @@ class TestGroupMessaging : public TestCommand static void OnSuccessCallback_5(void * context) { (static_cast(context))->OnSuccessResponse_5(); } - static void OnDoneCallback_6(void * context) { (static_cast(context))->OnDoneResponse_6(); } - static void OnFailureCallback_6(void * context, CHIP_ERROR error) { (static_cast(context))->OnFailureResponse_6(error); @@ -91554,43 +91556,52 @@ class TestGroupMessaging : public TestCommand static void OnSuccessCallback_6(void * context) { (static_cast(context))->OnSuccessResponse_6(); } + static void OnDoneCallback_7(void * context) { (static_cast(context))->OnDoneResponse_7(); } + static void OnFailureCallback_7(void * context, CHIP_ERROR error) { (static_cast(context))->OnFailureResponse_7(error); } - static void OnSuccessCallback_7(void * context, chip::CharSpan location) - { - (static_cast(context))->OnSuccessResponse_7(location); - } - - static void OnDoneCallback_8(void * context) { (static_cast(context))->OnDoneResponse_8(); } + static void OnSuccessCallback_7(void * context) { (static_cast(context))->OnSuccessResponse_7(); } static void OnFailureCallback_8(void * context, CHIP_ERROR error) { (static_cast(context))->OnFailureResponse_8(error); } - static void OnSuccessCallback_8(void * context) { (static_cast(context))->OnSuccessResponse_8(); } + static void OnSuccessCallback_8(void * context, chip::CharSpan location) + { + (static_cast(context))->OnSuccessResponse_8(location); + } + + static void OnDoneCallback_9(void * context) { (static_cast(context))->OnDoneResponse_9(); } static void OnFailureCallback_9(void * context, CHIP_ERROR error) { (static_cast(context))->OnFailureResponse_9(error); } - static void OnSuccessCallback_9(void * context, chip::CharSpan location) + static void OnSuccessCallback_9(void * context) { (static_cast(context))->OnSuccessResponse_9(); } + + static void OnFailureCallback_10(void * context, CHIP_ERROR error) { - (static_cast(context))->OnSuccessResponse_9(location); + (static_cast(context))->OnFailureResponse_10(error); } - static void OnFailureCallback_11(void * context, CHIP_ERROR error) + static void OnSuccessCallback_10(void * context, chip::CharSpan location) { - (static_cast(context))->OnFailureResponse_11(error); + (static_cast(context))->OnSuccessResponse_10(location); } - static void OnSuccessCallback_11(void * context, bool onOff) + static void OnFailureCallback_12(void * context, CHIP_ERROR error) + { + (static_cast(context))->OnFailureResponse_12(error); + } + + static void OnSuccessCallback_12(void * context, bool onOff) { - (static_cast(context))->OnSuccessResponse_11(onOff); + (static_cast(context))->OnSuccessResponse_12(onOff); } // @@ -91823,7 +91834,49 @@ class TestGroupMessaging : public TestCommand void OnSuccessResponse_5() { NextTest(); } - CHIP_ERROR TestGroupWriteAttribute_6() + CHIP_ERROR TestInstallACLsForTest_6() + { + const chip::EndpointId endpoint = mEndpoint.HasValue() ? mEndpoint.Value() : 0; + chip::Controller::AccessControlClusterTest cluster; + cluster.Associate(mDevices[kIdentityAlpha], endpoint); + + chip::app::DataModel::List aclArgument; + + chip::app::Clusters::AccessControl::Structs::AccessControlEntry::Type aclList_0[2]; + + aclList_0[0].fabricIndex = 1; + aclList_0[0].privilege = static_cast(5); + aclList_0[0].authMode = static_cast(2); + aclList_0[0].subjects.SetNull(); + aclList_0[0].targets.SetNull(); + + aclList_0[1].fabricIndex = 1; + aclList_0[1].privilege = static_cast(3); + aclList_0[1].authMode = static_cast(3); + aclList_0[1].subjects.SetNonNull(); + + uint64_t subjectsList_3[2]; + subjectsList_3[0] = 257ULL; + subjectsList_3[1] = 258ULL; + aclList_0[1].subjects.Value() = subjectsList_3; + aclList_0[1].targets.SetNull(); + + aclArgument = aclList_0; + + ReturnErrorOnFailure(cluster.WriteAttribute( + aclArgument, this, OnSuccessCallback_6, OnFailureCallback_6)); + return CHIP_NO_ERROR; + } + + void OnFailureResponse_6(CHIP_ERROR error) + { + chip::app::StatusIB status(error); + ThrowFailureResponse(); + } + + void OnSuccessResponse_6() { NextTest(); } + + CHIP_ERROR TestGroupWriteAttribute_7() { const chip::GroupId groupId = 258; chip::Controller::BasicClusterTest cluster; @@ -91834,21 +91887,21 @@ class TestGroupMessaging : public TestCommand locationArgument = chip::Span("USgarbage: not in length on purpose", 2); ReturnErrorOnFailure(cluster.WriteAttribute( - locationArgument, this, OnSuccessCallback_6, OnFailureCallback_6, OnDoneCallback_6)); + locationArgument, this, OnSuccessCallback_7, OnFailureCallback_7, OnDoneCallback_7)); return CHIP_NO_ERROR; } - void OnFailureResponse_6(CHIP_ERROR error) + void OnFailureResponse_7(CHIP_ERROR error) { chip::app::StatusIB status(error); ThrowFailureResponse(); } - void OnSuccessResponse_6() { NextTest(); } + void OnSuccessResponse_7() { NextTest(); } - void OnDoneResponse_6() { NextTest(); } + void OnDoneResponse_7() { NextTest(); } - CHIP_ERROR TestReadBackAttribute_7() + CHIP_ERROR TestReadBackAttribute_8() { const chip::EndpointId endpoint = mEndpoint.HasValue() ? mEndpoint.Value() : 0; chip::Controller::BasicClusterTest cluster; @@ -91857,24 +91910,24 @@ class TestGroupMessaging : public TestCommand ListFreer listFreer; ReturnErrorOnFailure(cluster.ReadAttribute( - this, OnSuccessCallback_7, OnFailureCallback_7)); + this, OnSuccessCallback_8, OnFailureCallback_8)); return CHIP_NO_ERROR; } - void OnFailureResponse_7(CHIP_ERROR error) + void OnFailureResponse_8(CHIP_ERROR error) { chip::app::StatusIB status(error); ThrowFailureResponse(); } - void OnSuccessResponse_7(chip::CharSpan location) + void OnSuccessResponse_8(chip::CharSpan location) { VerifyOrReturn(CheckValueAsString("location", location, chip::CharSpan("US", 2))); NextTest(); } - CHIP_ERROR TestRestoreInitialLocationValue_8() + CHIP_ERROR TestRestoreInitialLocationValue_9() { const chip::GroupId groupId = 258; chip::Controller::BasicClusterTest cluster; @@ -91885,21 +91938,21 @@ class TestGroupMessaging : public TestCommand locationArgument = chip::Span("XXgarbage: not in length on purpose", 2); ReturnErrorOnFailure(cluster.WriteAttribute( - locationArgument, this, OnSuccessCallback_8, OnFailureCallback_8, OnDoneCallback_8)); + locationArgument, this, OnSuccessCallback_9, OnFailureCallback_9, OnDoneCallback_9)); return CHIP_NO_ERROR; } - void OnFailureResponse_8(CHIP_ERROR error) + void OnFailureResponse_9(CHIP_ERROR error) { chip::app::StatusIB status(error); ThrowFailureResponse(); } - void OnSuccessResponse_8() { NextTest(); } + void OnSuccessResponse_9() { NextTest(); } - void OnDoneResponse_8() { NextTest(); } + void OnDoneResponse_9() { NextTest(); } - CHIP_ERROR TestReadBackAttribute_9() + CHIP_ERROR TestReadBackAttribute_10() { const chip::EndpointId endpoint = mEndpoint.HasValue() ? mEndpoint.Value() : 0; chip::Controller::BasicClusterTest cluster; @@ -91908,24 +91961,24 @@ class TestGroupMessaging : public TestCommand ListFreer listFreer; ReturnErrorOnFailure(cluster.ReadAttribute( - this, OnSuccessCallback_9, OnFailureCallback_9)); + this, OnSuccessCallback_10, OnFailureCallback_10)); return CHIP_NO_ERROR; } - void OnFailureResponse_9(CHIP_ERROR error) + void OnFailureResponse_10(CHIP_ERROR error) { chip::app::StatusIB status(error); ThrowFailureResponse(); } - void OnSuccessResponse_9(chip::CharSpan location) + void OnSuccessResponse_10(chip::CharSpan location) { VerifyOrReturn(CheckValueAsString("location", location, chip::CharSpan("XX", 2))); NextTest(); } - CHIP_ERROR TestTurnOnTheLightToSeeAttributeChange_10() + CHIP_ERROR TestTurnOnTheLightToSeeAttributeChange_11() { const chip::GroupId groupId = 257; using RequestType = chip::app::Clusters::OnOff::Commands::On::Type; @@ -91934,31 +91987,31 @@ class TestGroupMessaging : public TestCommand RequestType request; auto success = [](void * context, const typename RequestType::ResponseType & data) { - (static_cast(context))->OnSuccessResponse_10(); + (static_cast(context))->OnSuccessResponse_11(); }; auto failure = [](void * context, CHIP_ERROR error) { - (static_cast(context))->OnFailureResponse_10(error); + (static_cast(context))->OnFailureResponse_11(error); }; - auto done = [](void * context) { (static_cast(context))->OnDoneResponse_10(); }; + auto done = [](void * context) { (static_cast(context))->OnDoneResponse_11(); }; ReturnErrorOnFailure( chip::Controller::InvokeGroupCommand(mDevices[kIdentityAlpha], this, success, failure, done, groupId, request)); return CHIP_NO_ERROR; } - void OnFailureResponse_10(CHIP_ERROR error) + void OnFailureResponse_11(CHIP_ERROR error) { chip::app::StatusIB status(error); ThrowFailureResponse(); } - void OnSuccessResponse_10() { NextTest(); } + void OnSuccessResponse_11() { NextTest(); } - void OnDoneResponse_10() { NextTest(); } + void OnDoneResponse_11() { NextTest(); } - CHIP_ERROR TestCheckOnOffAttributeValueIsTrueAfterOnCommandForEndpoint1_11() + CHIP_ERROR TestCheckOnOffAttributeValueIsTrueAfterOnCommandForEndpoint1_12() { const chip::EndpointId endpoint = mEndpoint.HasValue() ? mEndpoint.Value() : 1; chip::Controller::OnOffClusterTest cluster; @@ -91967,17 +92020,17 @@ class TestGroupMessaging : public TestCommand ListFreer listFreer; ReturnErrorOnFailure(cluster.ReadAttribute( - this, OnSuccessCallback_11, OnFailureCallback_11)); + this, OnSuccessCallback_12, OnFailureCallback_12)); return CHIP_NO_ERROR; } - void OnFailureResponse_11(CHIP_ERROR error) + void OnFailureResponse_12(CHIP_ERROR error) { chip::app::StatusIB status(error); ThrowFailureResponse(); } - void OnSuccessResponse_11(bool onOff) + void OnSuccessResponse_12(bool onOff) { VerifyOrReturn(CheckValue("onOff", onOff, 1));