Skip to content

Commit

Permalink
Add access control check to IM read, write, invoke (#12645)
Browse files Browse the repository at this point in the history
* Add access control check to IM read, write, invoke

- Access control check still not "enabled"
- Subject descriptor not hooked up
- Request privilege still default (not custom)
- Check not necessarily in spec-required sequence (but OK for now)
- Status always returned (not omitted for denied wildcard interactions)

* Improve access control check calls in IM

- still not "enabled"
- still not checking for events (will add later)
- removed check for read with attribute override (will add later)
- ensured checks for read, write, invoke are in correct stage
- ensured works for wildcard reads (when they are ready)

* Address review comments

- move attribute read check back up
- respond with status if access control has serious error during
  command handling
  • Loading branch information
mlepage-google authored and pull[bot] committed Sep 21, 2023
1 parent fcae8a5 commit 2329169
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 4 deletions.
18 changes: 18 additions & 0 deletions src/app/CommandHandler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "InteractionModelEngine.h"
#include "messaging/ExchangeContext.h"

#include <access/AccessControl.h>
#include <app/util/MatterCallbacks.h>
#include <lib/support/TypeTraits.h>
#include <protocols/secure_channel/Constants.h>
Expand Down Expand Up @@ -259,6 +260,23 @@ CHIP_ERROR CommandHandler::ProcessCommandDataIB(CommandDataIB::Parser & aCommand

VerifyOrExit(mpCallback->CommandExists(concretePath), err = CHIP_ERROR_INVALID_PROFILE_ID);

{
Access::SubjectDescriptor subjectDescriptor; // TODO: get actual subject descriptor
Access::RequestPath requestPath{ .cluster = concretePath.mClusterId, .endpoint = concretePath.mEndpointId };
Access::Privilege requestPrivilege = Access::Privilege::kOperate; // TODO: get actual request privilege
err = Access::GetAccessControl().Check(subjectDescriptor, requestPath, requestPrivilege);
err = CHIP_NO_ERROR; // TODO: remove override
if (err != CHIP_NO_ERROR)
{
if (err != CHIP_ERROR_ACCESS_DENIED)
{
return AddStatus(concretePath, Protocols::InteractionModel::Status::Failure);
}
// TODO: when wildcard/group invokes are supported, handle them to discard rather than fail with status
return AddStatus(concretePath, Protocols::InteractionModel::Status::UnsupportedAccess);
}
}

err = aCommandElement.GetData(&commandDataReader);
if (CHIP_END_OF_TLV == err)
{
Expand Down
44 changes: 40 additions & 4 deletions src/app/util/ember-compatibility-functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
* when calling ember callbacks.
*/

#include <access/AccessControl.h>
#include <app/ClusterInfo.h>
#include <app/ConcreteAttributePath.h>
#include <app/InteractionModelEngine.h>
Expand Down Expand Up @@ -388,10 +389,32 @@ CHIP_ERROR ReadSingleClusterData(FabricIndex aAccessingFabricIndex, const Concre
return SendFailureStatus(aPath, attributeReport, Protocols::InteractionModel::Status::UnsupportedAttribute, nullptr);
}

AttributeAccessInterface * attrOverride = findAttributeAccessOverride(aPath.mEndpointId, aPath.mClusterId);
{
Access::SubjectDescriptor subjectDescriptor; // TODO: get actual subject descriptor
Access::RequestPath requestPath{ .cluster = aPath.mClusterId, .endpoint = aPath.mEndpointId };
Access::Privilege requestPrivilege = Access::Privilege::kView; // TODO: get actual request privilege
bool pathWasExpanded = false; // TODO: get actual expanded flag
CHIP_ERROR err = Access::GetAccessControl().Check(subjectDescriptor, requestPath, requestPrivilege);
err = CHIP_NO_ERROR; // TODO: remove override
if (err != CHIP_NO_ERROR)
{
ReturnErrorCodeIf(err != CHIP_ERROR_ACCESS_DENIED, err);
if (pathWasExpanded)
{
return CHIP_NO_ERROR;
}
else
{
AttributeReportIB::Builder & attributeReport = aAttributeReports.CreateAttributeReport();
ReturnErrorOnFailure(aAttributeReports.GetError());
return SendFailureStatus(aPath, attributeReport, Protocols::InteractionModel::Status::UnsupportedAccess, nullptr);
}
}
}

// Value encoder will encode the whole AttributeReport, including the path, value and the version.
// The AttributeValueEncoder may encode more than one AttributeReportIB for the list chunking feature.
if (attrOverride != nullptr)
if (auto * attrOverride = findAttributeAccessOverride(aPath.mEndpointId, aPath.mClusterId))
{
bool triedEncode;
ReturnErrorOnFailure(
Expand All @@ -404,14 +427,13 @@ CHIP_ERROR ReadSingleClusterData(FabricIndex aAccessingFabricIndex, const Concre
}

AttributeReportIB::Builder & attributeReport = aAttributeReports.CreateAttributeReport();

ReturnErrorOnFailure(aAttributeReports.GetError());

TLV::TLVWriter backup;
attributeReport.Checkpoint(backup);

// We have verified that the attribute exists.
AttributeDataIB::Builder & attributeDataIBBuilder = attributeReport.CreateAttributeData();

ReturnErrorOnFailure(attributeDataIBBuilder.GetError());

attributeDataIBBuilder.DataVersion(kTemporaryDataVersion);
Expand Down Expand Up @@ -818,6 +840,20 @@ CHIP_ERROR WriteSingleClusterData(ClusterInfo & aClusterInfo, TLV::TLVReader & a
return apWriteHandler->AddStatus(attributePathParams, Protocols::InteractionModel::Status::UnsupportedWrite);
}

{
Access::SubjectDescriptor subjectDescriptor; // TODO: get actual subject descriptor
Access::RequestPath requestPath{ .cluster = aPath.mClusterId, .endpoint = aPath.mEndpointId };
Access::Privilege requestPrivilege = Access::Privilege::kOperate; // TODO: get actual request privilege
CHIP_ERROR err = Access::GetAccessControl().Check(subjectDescriptor, requestPath, requestPrivilege);
err = CHIP_NO_ERROR; // TODO: remove override
if (err != CHIP_NO_ERROR)
{
ReturnErrorCodeIf(err != CHIP_ERROR_ACCESS_DENIED, err);
// TODO: when wildcard/group writes are supported, handle them to discard rather than fail with status
return apWriteHandler->AddStatus(attributePathParams, Protocols::InteractionModel::Status::UnsupportedAccess);
}
}

if (attributeMetadata->MustUseTimedWrite() && !apWriteHandler->IsTimedWrite())
{
return apWriteHandler->AddStatus(attributePathParams, Protocols::InteractionModel::Status::NeedsTimedInteraction);
Expand Down

0 comments on commit 2329169

Please sign in to comment.