Skip to content

Commit 1096376

Browse files
mlepage-googlepull[bot]
authored andcommitted
Support empty subjects in group ACL entries (#15608)
Spec was revised to allow empty subjects as wildcard for group auth mode (as for CASE auth mode). This PR updates the code, unit tests, and YAML tests. Fixes #15355
1 parent bd49aa3 commit 1096376

File tree

4 files changed

+98
-37
lines changed

4 files changed

+98
-37
lines changed

src/access/AccessControl.cpp

-3
Original file line numberDiff line numberDiff line change
@@ -341,9 +341,6 @@ bool AccessControl::IsValid(const Entry & entry)
341341

342342
// Privilege must not be administer.
343343
VerifyOrExit(privilege != Privilege::kAdminister, log = "invalid privilege");
344-
345-
// Subject must be present.
346-
VerifyOrExit(subjectCount > 0, log = "invalid subject count");
347344
}
348345

349346
for (size_t i = 0; i < subjectCount; ++i)

src/access/tests/TestAccessControl.cpp

+18-9
Original file line numberDiff line numberDiff line change
@@ -1067,6 +1067,23 @@ void TestAclValidateAuthModeSubject(nlTestSuite * inSuite, void * inContext)
10671067
// Each case tries to update the first entry, then add a second entry, then unconditionally delete it
10681068
NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(nullptr, entry) == CHIP_NO_ERROR);
10691069

1070+
// CASE and group may have empty subjects list
1071+
{
1072+
NL_TEST_ASSERT(inSuite, entry.RemoveSubject(0) == CHIP_NO_ERROR);
1073+
1074+
NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kCase) == CHIP_NO_ERROR);
1075+
NL_TEST_ASSERT(inSuite, accessControl.UpdateEntry(0, entry) == CHIP_NO_ERROR);
1076+
NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(nullptr, entry) == CHIP_NO_ERROR);
1077+
accessControl.DeleteEntry(1);
1078+
1079+
NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kGroup) == CHIP_NO_ERROR);
1080+
NL_TEST_ASSERT(inSuite, accessControl.UpdateEntry(0, entry) == CHIP_NO_ERROR);
1081+
NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(nullptr, entry) == CHIP_NO_ERROR);
1082+
accessControl.DeleteEntry(1);
1083+
1084+
NL_TEST_ASSERT(inSuite, entry.AddSubject(nullptr, kOperationalNodeId0) == CHIP_NO_ERROR);
1085+
}
1086+
10701087
NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kCase) == CHIP_NO_ERROR);
10711088
for (auto subject : validCaseSubjects)
10721089
{
@@ -1200,17 +1217,9 @@ void TestAclValidateAuthModeSubject(nlTestSuite * inSuite, void * inContext)
12001217
// Next cases have no subject
12011218
NL_TEST_ASSERT(inSuite, entry.RemoveSubject(0) == CHIP_NO_ERROR);
12021219

1203-
// Group must have subject
1204-
{
1205-
NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kGroup) == CHIP_NO_ERROR);
1206-
NL_TEST_ASSERT(inSuite, accessControl.UpdateEntry(0, entry) != CHIP_NO_ERROR);
1207-
NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(nullptr, entry) != CHIP_NO_ERROR);
1208-
accessControl.DeleteEntry(1);
1209-
}
1210-
12111220
// PASE must have subject
12121221
{
1213-
NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kGroup) == CHIP_NO_ERROR);
1222+
NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kPase) == CHIP_NO_ERROR);
12141223
NL_TEST_ASSERT(inSuite, accessControl.UpdateEntry(0, entry) != CHIP_NO_ERROR);
12151224
NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(nullptr, entry) != CHIP_NO_ERROR);
12161225
accessControl.DeleteEntry(1);

src/app/tests/suites/TestGroupMessaging.yaml

+26-10
Original file line numberDiff line numberDiff line change
@@ -116,26 +116,26 @@ tests:
116116
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 },
117117
]
118118

119-
- label: "Install ACLs for test"
119+
- label: "Install ACLs"
120120
cluster: "Access Control"
121121
command: "writeAttribute"
122122
attribute: "ACL"
123123
arguments:
124124
value: [
125-
# Any CASE can admin
125+
# Any CASE can administer
126126
{
127-
FabricIndex: 1,
128-
Privilege: 5,
129-
AuthMode: 2,
127+
FabricIndex: 0,
128+
Privilege: 5, # administer
129+
AuthMode: 2, # case
130130
Subjects: null,
131131
Targets: null,
132132
},
133-
# These groups can operate
133+
# Any group can operate
134134
{
135-
FabricIndex: 1,
136-
Privilege: 3,
137-
AuthMode: 3,
138-
Subjects: [0x0101, 0x0102],
135+
FabricIndex: 0,
136+
Privilege: 3, # operate
137+
AuthMode: 3, # group
138+
Subjects: null,
139139
Targets: null,
140140
},
141141
]
@@ -185,3 +185,19 @@ tests:
185185
endpoint: 1
186186
response:
187187
value: 1
188+
189+
- label: "Cleanup ACLs"
190+
cluster: "Access Control"
191+
command: "writeAttribute"
192+
attribute: "ACL"
193+
arguments:
194+
value: [
195+
# Any CASE can administer
196+
{
197+
FabricIndex: 0,
198+
Privilege: 5, # administer
199+
AuthMode: 2, # case
200+
Subjects: null,
201+
Targets: null,
202+
},
203+
]

zzz_generated/chip-tool/zap-generated/test/Commands.h

+54-15
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)