Skip to content

Commit

Permalink
Add ACL validation (#14756)
Browse files Browse the repository at this point in the history
* Add ACL validation

- There was some before, but this is more stringent.
- Also refactor some of the "IsValid"-like functions.
- Update unit tests.
- Remove more operational PASE support.

Part of #14460

* Fix some unit tests after changes

* Fix variable name

* Fix another unit test after changes

* Add more unit tests

* Add more unit tests

* More work on ACL validation

- Simplify validation function
- Add more unit tests

* More unit tests

* Final touches

- More test cases
- Move some of the IsValid functions into access control
- Slight simplification to AccessControl::IsValid

* Fix unused variable warnings on other platforms

* Fix destruction order in test code

Delegate must outlive the thing holding it.

* Quell unused variable warning
  • Loading branch information
mlepage-google authored and pull[bot] committed Jan 9, 2024
1 parent 5eee313 commit 1284187
Show file tree
Hide file tree
Showing 7 changed files with 1,267 additions and 85 deletions.
76 changes: 75 additions & 1 deletion src/access/AccessControl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ bool CheckRequestPrivilegeAgainstEntryPrivilege(Privilege requestPrivilege, Priv
return false;
}

constexpr bool IsValidCaseNodeId(NodeId aNodeId)
{
return chip::IsOperationalNodeId(aNodeId) || (chip::IsCASEAuthTag(aNodeId) && ((aNodeId & chip::kTagVersionMask) != 0));
}

constexpr bool IsValidGroupNodeId(NodeId aNodeId)
{
return chip::IsGroupId(aNodeId) && chip::IsValidGroupId(chip::GroupIdFromNodeId(aNodeId));
}

#if CHIP_DETAIL_LOGGING

char GetAuthModeStringForLogging(AuthMode authMode)
Expand Down Expand Up @@ -273,7 +283,7 @@ CHIP_ERROR AccessControl::Check(const SubjectDescriptor & subjectDescriptor, con
{
continue;
}
// TODO: check against target.deviceType (requires lookup)
// TODO(#14431): device type target not yet supported (add lookup/match when supported)
targetMatched = true;
break;
}
Expand All @@ -291,6 +301,70 @@ CHIP_ERROR AccessControl::Check(const SubjectDescriptor & subjectDescriptor, con
return CHIP_ERROR_ACCESS_DENIED;
}

bool AccessControl::IsValid(const Entry & entry)
{
const char * log = "unexpected error";
IgnoreUnusedVariable(log); // logging may be disabled

AuthMode authMode;
FabricIndex fabricIndex;
Privilege privilege;
size_t subjectCount = 0;
size_t targetCount = 0;

SuccessOrExit(entry.GetAuthMode(authMode));
SuccessOrExit(entry.GetFabricIndex(fabricIndex));
SuccessOrExit(entry.GetPrivilege(privilege));
SuccessOrExit(entry.GetSubjectCount(subjectCount));
SuccessOrExit(entry.GetTargetCount(targetCount));

// Fabric index must be defined.
VerifyOrExit(fabricIndex != kUndefinedFabricIndex, log = "invalid fabric index");

if (authMode != AuthMode::kCase)
{
// Operational PASE not supported for v1.0 (so must be group).
VerifyOrExit(authMode == AuthMode::kGroup, log = "invalid auth mode");

// 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)
{
NodeId subject;
SuccessOrExit(entry.GetSubject(i, subject));
const bool kIsCase = authMode == AuthMode::kCase;
const bool kIsGroup = authMode == AuthMode::kGroup;
VerifyOrExit((kIsCase && IsValidCaseNodeId(subject)) || (kIsGroup && IsValidGroupNodeId(subject)), log = "invalid subject");
}

for (size_t i = 0; i < targetCount; ++i)
{
Entry::Target target;
SuccessOrExit(entry.GetTarget(i, target));
const bool kHasCluster = target.flags & Entry::Target::kCluster;
const bool kHasEndpoint = target.flags & Entry::Target::kEndpoint;
const bool kHasDeviceType = target.flags & Entry::Target::kDeviceType;
VerifyOrExit((kHasCluster || kHasEndpoint || kHasDeviceType) && !(kHasEndpoint && kHasDeviceType) &&
(!kHasCluster || IsValidClusterId(target.cluster)) &&
(!kHasEndpoint || IsValidEndpointId(target.endpoint)) &&
(!kHasDeviceType || IsValidDeviceTypeId(target.deviceType)),
log = "invalid target");
// TODO(#14431): device type target not yet supported (remove check when supported)
VerifyOrExit(!kHasDeviceType, log = "device type target not yet supported");
}

return true;

exit:
ChipLogError(DataManagement, "AccessControl: %s", log);
return false;
}

AccessControl & GetAccessControl()
{
return *globalAccessControl;
Expand Down
4 changes: 4 additions & 0 deletions src/access/AccessControl.h
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ class AccessControl
*/
CHIP_ERROR CreateEntry(size_t * index, const Entry & entry, FabricIndex * fabricIndex = nullptr)
{
ReturnErrorCodeIf(!IsValid(entry), CHIP_ERROR_INVALID_ARGUMENT);
return mDelegate.CreateEntry(index, entry, fabricIndex);
}

Expand All @@ -417,6 +418,7 @@ class AccessControl
*/
CHIP_ERROR UpdateEntry(size_t index, const Entry & entry, const FabricIndex * fabricIndex = nullptr)
{
ReturnErrorCodeIf(!IsValid(entry), CHIP_ERROR_INVALID_ARGUMENT);
return mDelegate.UpdateEntry(index, entry, fabricIndex);
}

Expand Down Expand Up @@ -453,6 +455,8 @@ class AccessControl
CHIP_ERROR Check(const SubjectDescriptor & subjectDescriptor, const RequestPath & requestPath, Privilege requestPrivilege);

private:
bool IsValid(const Entry & entry);

static Delegate mDefaultDelegate;
Delegate & mDelegate = mDefaultDelegate;
};
Expand Down
Loading

0 comments on commit 1284187

Please sign in to comment.