Skip to content

Commit

Permalink
Support empty subjects in group ACL entries
Browse files Browse the repository at this point in the history
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 project-chip#15355
  • Loading branch information
mlepage-google committed Feb 25, 2022
1 parent c1d14cc commit b740b18
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 37 deletions.
3 changes: 0 additions & 3 deletions src/access/AccessControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -341,9 +341,6 @@ bool AccessControl::IsValid(const Entry & entry)

// Privilege must not be administer.
VerifyOrExit(privilege != Privilege::kAdminister, log = "invalid privilege");

// Subject must be present.
VerifyOrExit(subjectCount > 0, log = "invalid subject count");
}

for (size_t i = 0; i < subjectCount; ++i)
Expand Down
27 changes: 18 additions & 9 deletions src/access/tests/TestAccessControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1067,6 +1067,23 @@ void TestAclValidateAuthModeSubject(nlTestSuite * inSuite, void * inContext)
// Each case tries to update the first entry, then add a second entry, then unconditionally delete it
NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(nullptr, entry) == CHIP_NO_ERROR);

// CASE and group may have empty subjects list
{
NL_TEST_ASSERT(inSuite, entry.RemoveSubject(0) == CHIP_NO_ERROR);

NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kCase) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, accessControl.UpdateEntry(0, entry) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(nullptr, entry) == CHIP_NO_ERROR);
accessControl.DeleteEntry(1);

NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kGroup) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, accessControl.UpdateEntry(0, entry) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(nullptr, entry) == CHIP_NO_ERROR);
accessControl.DeleteEntry(1);

NL_TEST_ASSERT(inSuite, entry.AddSubject(nullptr, kOperationalNodeId0) == CHIP_NO_ERROR);
}

NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kCase) == CHIP_NO_ERROR);
for (auto subject : validCaseSubjects)
{
Expand Down Expand Up @@ -1200,17 +1217,9 @@ void TestAclValidateAuthModeSubject(nlTestSuite * inSuite, void * inContext)
// Next cases have no subject
NL_TEST_ASSERT(inSuite, entry.RemoveSubject(0) == CHIP_NO_ERROR);

// Group must have subject
{
NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kGroup) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, accessControl.UpdateEntry(0, entry) != CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(nullptr, entry) != CHIP_NO_ERROR);
accessControl.DeleteEntry(1);
}

// PASE must have subject
{
NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kGroup) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, entry.SetAuthMode(AuthMode::kPase) == CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, accessControl.UpdateEntry(0, entry) != CHIP_NO_ERROR);
NL_TEST_ASSERT(inSuite, accessControl.CreateEntry(nullptr, entry) != CHIP_NO_ERROR);
accessControl.DeleteEntry(1);
Expand Down
36 changes: 26 additions & 10 deletions src/app/tests/suites/TestGroupMessaging.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -116,26 +116,26 @@ tests:
{ FabricIndex: 1, GroupId: 0x0102, GroupKeySetID: 0x01a2 },
]

- label: "Install ACLs for test"
- label: "Install ACLs"
cluster: "Access Control"
command: "writeAttribute"
attribute: "ACL"
arguments:
value: [
# Any CASE can admin
# Any CASE can administer
{
FabricIndex: 1,
Privilege: 5,
AuthMode: 2,
FabricIndex: 0,
Privilege: 5, # administer
AuthMode: 2, # case
Subjects: null,
Targets: null,
},
# These groups can operate
# Any group can operate
{
FabricIndex: 1,
Privilege: 3,
AuthMode: 3,
Subjects: [0x0101, 0x0102],
FabricIndex: 0,
Privilege: 3, # operate
AuthMode: 3, # group
Subjects: null,
Targets: null,
},
]
Expand Down Expand Up @@ -185,3 +185,19 @@ tests:
endpoint: 1
response:
value: 1

- label: "Cleanup ACLs"
cluster: "Access Control"
command: "writeAttribute"
attribute: "ACL"
arguments:
value: [
# Any CASE can administer
{
FabricIndex: 0,
Privilege: 5, # administer
AuthMode: 2, # case
Subjects: null,
Targets: null,
},
]
69 changes: 54 additions & 15 deletions zzz_generated/chip-tool/zap-generated/test/Commands.h

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit b740b18

Please sign in to comment.