Skip to content

Commit

Permalink
Enforce length constraints in access control (#17817)
Browse files Browse the repository at this point in the history 
Add some APIs for length constraints. Use them in system module and in
cluster to enforce length constraints.
  • Loading branch information
@mlepage-google @pull
mlepage-google authored and pull[bot] committed Aug 9, 2023
1 parent cbfffa6 commit 2214850
Show file tree
Hide file tree
Showing 7 changed files with 998 additions and 92 deletions.
58 changes: 58 additions & 0 deletions src/access/AccessControl.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,64 @@ CHIP_ERROR AccessControl::Finish()
return retval;
}

CHIP_ERROR AccessControl::CreateEntry(const SubjectDescriptor * subjectDescriptor, FabricIndex fabric, size_t * index,
const Entry & entry)
{
VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);

size_t count = 0;
size_t maxCount = 0;
ReturnErrorOnFailure(mDelegate->GetEntryCount(fabric, count));
ReturnErrorOnFailure(mDelegate->GetMaxEntriesPerFabric(maxCount));

VerifyOrReturnError((count + 1) <= maxCount, CHIP_ERROR_BUFFER_TOO_SMALL);

ReturnErrorCodeIf(!IsValid(entry), CHIP_ERROR_INVALID_ARGUMENT);

size_t i = 0;
ReturnErrorOnFailure(mDelegate->CreateEntry(&i, entry, &fabric));

if (index)
{
*index = i;
}

NotifyEntryChanged(subjectDescriptor, fabric, i, &entry, EntryListener::ChangeType::kAdded);
return CHIP_NO_ERROR;
}

CHIP_ERROR AccessControl::UpdateEntry(const SubjectDescriptor * subjectDescriptor, FabricIndex fabric, size_t index,
const Entry & entry)
{
VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
ReturnErrorCodeIf(!IsValid(entry), CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorOnFailure(mDelegate->UpdateEntry(index, entry, &fabric));
NotifyEntryChanged(subjectDescriptor, fabric, index, &entry, EntryListener::ChangeType::kUpdated);
return CHIP_NO_ERROR;
}

CHIP_ERROR AccessControl::DeleteEntry(const SubjectDescriptor * subjectDescriptor, FabricIndex fabric, size_t index)
{
VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
Entry entry;
Entry * p = nullptr;
if (mEntryListener != nullptr && ReadEntry(fabric, index, entry) == CHIP_NO_ERROR)
{
p = &entry;
}
ReturnErrorOnFailure(mDelegate->DeleteEntry(index, &fabric));
if (p && p->HasDefaultDelegate())
{
// The entry was read prior to deletion so its latest value could be provided
// to the listener after deletion. If it's been reset to its default delegate,
// that best effort attempt to retain the latest value failed. This is
// regrettable but OK.
p = nullptr;
}
NotifyEntryChanged(subjectDescriptor, fabric, index, p, EntryListener::ChangeType::kRemoved);
return CHIP_NO_ERROR;
}

void AccessControl::AddEntryListener(EntryListener & listener)
{
if (mEntryListener == nullptr)
Expand Down
83 changes: 39 additions & 44 deletions src/access/AccessControl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -345,13 +345,29 @@ class AccessControl
virtual CHIP_ERROR Finish() { return CHIP_NO_ERROR; }

// Capabilities
virtual CHIP_ERROR GetMaxEntryCount(size_t & value) const
virtual CHIP_ERROR GetMaxEntriesPerFabric(size_t & value) const
{
value = 0;
return CHIP_NO_ERROR;
}

virtual CHIP_ERROR GetMaxSubjectsPerEntry(size_t & value) const
{
value = 0;
return CHIP_NO_ERROR;
}

// TODO: add more capabilities
virtual CHIP_ERROR GetMaxTargetsPerEntry(size_t & value) const
{
value = 0;
return CHIP_NO_ERROR;
}

virtual CHIP_ERROR GetMaxEntryCount(size_t & value) const
{
value = 0;
return CHIP_NO_ERROR;
}

// Actualities
virtual CHIP_ERROR GetEntryCount(FabricIndex fabric, size_t & value) const
Expand Down Expand Up @@ -417,6 +433,24 @@ class AccessControl
CHIP_ERROR Finish();

// Capabilities
CHIP_ERROR GetMaxEntriesPerFabric(size_t & value) const
{
VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
return mDelegate->GetMaxEntriesPerFabric(value);
}

CHIP_ERROR GetMaxSubjectsPerEntry(size_t & value) const
{
VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
return mDelegate->GetMaxSubjectsPerEntry(value);
}

CHIP_ERROR GetMaxTargetsPerEntry(size_t & value) const
{
VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
return mDelegate->GetMaxTargetsPerEntry(value);
}

CHIP_ERROR GetMaxEntryCount(size_t & value) const
{
VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
Expand Down Expand Up @@ -457,19 +491,7 @@ class AccessControl
* @param [out] index (If not nullptr) index of created entry (relative to fabric).
* @param [in] entry Entry from which created entry is copied.
*/
CHIP_ERROR CreateEntry(const SubjectDescriptor * subjectDescriptor, FabricIndex fabric, size_t * index, const Entry & entry)
{
VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
ReturnErrorCodeIf(!IsValid(entry), CHIP_ERROR_INVALID_ARGUMENT);
size_t i;
ReturnErrorOnFailure(mDelegate->CreateEntry(&i, entry, &fabric));
if (index)
{
*index = i;
}
NotifyEntryChanged(subjectDescriptor, fabric, i, &entry, EntryListener::ChangeType::kAdded);
return CHIP_NO_ERROR;
}
CHIP_ERROR CreateEntry(const SubjectDescriptor * subjectDescriptor, FabricIndex fabric, size_t * index, const Entry & entry);

/**
* Creates an entry in the access control list.
Expand Down Expand Up @@ -519,14 +541,7 @@ class AccessControl
* @param [in] index Index of entry to update (relative to fabric).
* @param [in] entry Entry from which updated entry is copied.
*/
CHIP_ERROR UpdateEntry(const SubjectDescriptor * subjectDescriptor, FabricIndex fabric, size_t index, const Entry & entry)
{
VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
ReturnErrorCodeIf(!IsValid(entry), CHIP_ERROR_INVALID_ARGUMENT);
ReturnErrorOnFailure(mDelegate->UpdateEntry(index, entry, &fabric));
NotifyEntryChanged(subjectDescriptor, fabric, index, &entry, EntryListener::ChangeType::kUpdated);
return CHIP_NO_ERROR;
}
CHIP_ERROR UpdateEntry(const SubjectDescriptor * subjectDescriptor, FabricIndex fabric, size_t index, const Entry & entry);

/**
* Updates an entry in the access control list.
Expand All @@ -549,27 +564,7 @@ class AccessControl
* @param [in] fabric Index of fabric in which to delete entry.
* @param [in] index Index of entry to delete (relative to fabric).
*/
CHIP_ERROR DeleteEntry(const SubjectDescriptor * subjectDescriptor, FabricIndex fabric, size_t index)
{
VerifyOrReturnError(IsInitialized(), CHIP_ERROR_INCORRECT_STATE);
Entry entry;
Entry * p = nullptr;
if (mEntryListener != nullptr && ReadEntry(fabric, index, entry) == CHIP_NO_ERROR)
{
p = &entry;
}
ReturnErrorOnFailure(mDelegate->DeleteEntry(index, &fabric));
if (p && p->HasDefaultDelegate())
{
// The entry was read prior to deletion so its latest value could be provided
// to the listener after deletion. If it's been reset to its default delegate,
// that best effort attempt to retain the latest value failed. This is
// regretable but OK.
p = nullptr;
}
NotifyEntryChanged(subjectDescriptor, fabric, index, p, EntryListener::ChangeType::kRemoved);
return CHIP_NO_ERROR;
}
CHIP_ERROR DeleteEntry(const SubjectDescriptor * subjectDescriptor, FabricIndex fabric, size_t index);

/**
* Deletes an entry from the access control list.
Expand Down
18 changes: 18 additions & 0 deletions src/access/examples/ExampleAccessControlDelegate.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -971,6 +971,24 @@ class AccessControlDelegate : public AccessControl::Delegate
return CHIP_NO_ERROR;
}

CHIP_ERROR GetMaxEntriesPerFabric(size_t & value) const override
{
value = EntryStorage::kEntriesPerFabric;
return CHIP_NO_ERROR;
}

CHIP_ERROR GetMaxSubjectsPerEntry(size_t & value) const override
{
value = EntryStorage::kMaxSubjects;
return CHIP_NO_ERROR;
}

CHIP_ERROR GetMaxTargetsPerEntry(size_t & value) const override
{
value = EntryStorage::kMaxTargets;
return CHIP_NO_ERROR;
}

CHIP_ERROR GetMaxEntryCount(size_t & value) const override
{
value = ArraySize(EntryStorage::acl);
Expand Down
37 changes: 18 additions & 19 deletions src/app/clusters/access-control-server/access-control-server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,20 +104,20 @@ CHIP_ERROR AccessControlAttribute::Read(const ConcreteReadAttributePath & aPath,
return ReadAcl(aEncoder);
case AccessControlCluster::Attributes::Extension::Id:
return ReadExtension(aEncoder);
// TODO(#14455): use API to get actual capabilities
case AccessControlCluster::Attributes::SubjectsPerAccessControlEntry::Id: {
uint16_t value = CHIP_CONFIG_EXAMPLE_ACCESS_CONTROL_MAX_SUBJECTS_PER_ENTRY;
return aEncoder.Encode(value);
size_t value = 0;
ReturnErrorOnFailure(GetAccessControl().GetMaxSubjectsPerEntry(value));
return aEncoder.Encode(static_cast<uint16_t>(value));
}
// TODO(#14455): use API to get actual capabilities
case AccessControlCluster::Attributes::TargetsPerAccessControlEntry::Id: {
uint16_t value = CHIP_CONFIG_EXAMPLE_ACCESS_CONTROL_MAX_TARGETS_PER_ENTRY;
return aEncoder.Encode(value);
size_t value = 0;
ReturnErrorOnFailure(GetAccessControl().GetMaxTargetsPerEntry(value));
return aEncoder.Encode(static_cast<uint16_t>(value));
}
// TODO(#14455): use API to get actual capabilities
case AccessControlCluster::Attributes::AccessControlEntriesPerFabric::Id: {
uint16_t value = CHIP_CONFIG_EXAMPLE_ACCESS_CONTROL_MAX_ENTRIES_PER_FABRIC;
return aEncoder.Encode(value);
size_t value = 0;
ReturnErrorOnFailure(GetAccessControl().GetMaxEntriesPerFabric(value));
return aEncoder.Encode(static_cast<uint16_t>(value));
}
case AccessControlCluster::Attributes::ClusterRevision::Id:
return aEncoder.Encode(kClusterRevision);
Expand Down Expand Up @@ -193,23 +193,20 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(const ConcreteDataAttributePath & aP
{
FabricIndex accessingFabricIndex = aDecoder.AccessingFabricIndex();

size_t oldCount;
ReturnErrorOnFailure(GetAccessControl().GetEntryCount(accessingFabricIndex, oldCount));
size_t maxCount;
ReturnErrorOnFailure(GetAccessControl().GetMaxEntriesPerFabric(maxCount));

if (!aPath.IsListItemOperation())
{
DataModel::DecodableList<AclStorage::DecodableEntry> list;
ReturnErrorOnFailure(aDecoder.Decode(list));

size_t allCount;
size_t oldCount;
size_t newCount;
size_t maxCount;

ReturnErrorOnFailure(GetAccessControl().GetEntryCount(allCount));
ReturnErrorOnFailure(GetAccessControl().GetEntryCount(accessingFabricIndex, oldCount));
ReturnErrorOnFailure(list.ComputeSize(&newCount));
ReturnErrorOnFailure(GetAccessControl().GetMaxEntryCount(maxCount));
VerifyOrReturnError(allCount >= oldCount, CHIP_ERROR_INTERNAL);
VerifyOrReturnError(static_cast<size_t>(allCount - oldCount + newCount) <= maxCount,
CHIP_IM_GLOBAL_STATUS(ConstraintError));

VerifyOrReturnError(newCount <= maxCount, CHIP_IM_GLOBAL_STATUS(ResourceExhausted));

auto iterator = list.begin();
size_t i = 0;
Expand Down Expand Up @@ -237,6 +234,8 @@ CHIP_ERROR AccessControlAttribute::WriteAcl(const ConcreteDataAttributePath & aP
}
else if (aPath.mListOp == ConcreteDataAttributePath::ListOperation::AppendItem)
{
VerifyOrReturnError((oldCount + 1) <= maxCount, CHIP_IM_GLOBAL_STATUS(ResourceExhausted));

AclStorage::DecodableEntry decodableEntry;
ReturnErrorOnFailure(aDecoder.Decode(decodableEntry));

Expand Down
101 changes: 101 additions & 0 deletions src/app/tests/suites/TestAccessControlCluster.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -423,6 +423,107 @@ tests:
},
]

- label: "Write too many entries"
command: "writeAttribute"
attribute: "ACL"
arguments:
value: [
{
FabricIndex: 0,
Privilege: 5, # administer
AuthMode: 2, # case
Subjects: null,
Targets:
[
{ Cluster: null, Endpoint: 0, DeviceType: null },
{ Cluster: 1, Endpoint: null, DeviceType: null },
{ Cluster: 2, Endpoint: 3, DeviceType: null },
],
},
{
FabricIndex: 0,
Privilege: 1, # view
AuthMode: 2, # case
Subjects: [4, 5, 6, 7],
Targets:
[
{ Cluster: null, Endpoint: 8, DeviceType: null },
{ Cluster: 9, Endpoint: null, DeviceType: null },
{ Cluster: 10, Endpoint: 11, DeviceType: null },
],
},
{
FabricIndex: 0,
Privilege: 3, # operate
AuthMode: 3, # group
Subjects: [12, 13, 14, 15],
Targets:
[
{ Cluster: null, Endpoint: 16, DeviceType: null },
{ Cluster: 17, Endpoint: null, DeviceType: null },
{ Cluster: 18, Endpoint: 19, DeviceType: null },
],
},
{
FabricIndex: 0,
Privilege: 1, # view
AuthMode: 2, # case
Subjects: [20, 21, 22, 23],
Targets:
[
{ Cluster: null, Endpoint: 24, DeviceType: null },
{ Cluster: 25, Endpoint: null, DeviceType: null },
{ Cluster: 26, Endpoint: 27, DeviceType: null },
],
},
]
response:
error: RESOURCE_EXHAUSTED

- label: "Verify"
command: "readAttribute"
attribute: "ACL"
response:
value: [
{
FabricIndex: 1,
Privilege: 5, # administer
AuthMode: 2, # case
Subjects: null,
Targets:
[
{ Cluster: null, Endpoint: 0, DeviceType: null },
{ Cluster: 1, Endpoint: null, DeviceType: null },
{ Cluster: 2, Endpoint: 3, DeviceType: null },
],
},
{
FabricIndex: 1,
Privilege: 1, # view
AuthMode: 2, # case
Subjects: [4, 5, 6, 7],
Targets:
[
{ Cluster: null, Endpoint: 8, DeviceType: null },
{ Cluster: 9, Endpoint: null, DeviceType: null },
{ Cluster: 10, Endpoint: 11, DeviceType: null },
],
},
{
FabricIndex: 1,
Privilege: 3, # operate
AuthMode: 3, # group
Subjects: [12, 13, 14, 15],
Targets:
[
{ Cluster: null, Endpoint: 16, DeviceType: null },
{ Cluster: 17, Endpoint: null, DeviceType: null },
{ Cluster: 18, Endpoint: 19, DeviceType: null },
],
},
]

# note missing last entry
- label: "Restore ACL"
command: "writeAttribute"
attribute: "ACL"
Expand Down
Loading

0 comments on commit 2214850

Please sign in to comment.