diff --git a/src/access/AccessControl.cpp b/src/access/AccessControl.cpp index f9cb48a20f18b2..8751649df853ce 100644 --- a/src/access/AccessControl.cpp +++ b/src/access/AccessControl.cpp @@ -162,18 +162,18 @@ namespace Access { AccessControl::Entry::Delegate AccessControl::Entry::mDefaultDelegate; AccessControl::EntryIterator::Delegate AccessControl::EntryIterator::mDefaultDelegate; -CHIP_ERROR AccessControl::Init(AccessControl::Delegate * delegate) +CHIP_ERROR AccessControl::Init(AccessControl::Delegate * delegate, DeviceTypeResolver & deviceTypeResolver) { VerifyOrReturnError(!IsInitialized(), CHIP_ERROR_INCORRECT_STATE); - VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT); ChipLogProgress(DataManagement, "AccessControl: initializing"); - // delegate can never be null. This was already checked + VerifyOrReturnError(delegate != nullptr, CHIP_ERROR_INVALID_ARGUMENT); CHIP_ERROR retval = delegate->Init(); if (retval == CHIP_NO_ERROR) { - mDelegate = delegate; + mDelegate = delegate; + mDeviceTypeResolver = &deviceTypeResolver; } return retval; @@ -326,7 +326,11 @@ CHIP_ERROR AccessControl::Check(const SubjectDescriptor & subjectDescriptor, con { continue; } - // TODO(#14431): device type target not yet supported (add lookup/match when supported) + if (target.flags & Entry::Target::kDeviceType && + !mDeviceTypeResolver->IsDeviceTypeOnEndpoint(target.deviceType, requestPath.endpoint)) + { + continue; + } targetMatched = true; break; } @@ -401,8 +405,6 @@ bool AccessControl::IsValid(const Entry & entry) (!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; diff --git a/src/access/AccessControl.h b/src/access/AccessControl.h index be1c268e3628b5..dc67e89856e1ed 100644 --- a/src/access/AccessControl.h +++ b/src/access/AccessControl.h @@ -31,6 +31,17 @@ namespace Access { class AccessControl { public: + /** + * Used by access control to determine if a device type resolves to an endpoint. + */ + struct DeviceTypeResolver + { + public: + virtual ~DeviceTypeResolver() = default; + + virtual bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) = 0; + }; + /** * Handle to an entry in the access control list. * @@ -376,12 +387,10 @@ class AccessControl /** * Initialize the access control module. Must be called before first use. * - * @param delegate - The delegate to use for acces control - * * @return CHIP_NO_ERROR on success, CHIP_ERROR_INCORRECT_STATE if called more than once, * CHIP_ERROR_INVALID_ARGUMENT if delegate is null, or other fatal error. */ - CHIP_ERROR Init(AccessControl::Delegate * delegate); + CHIP_ERROR Init(AccessControl::Delegate * delegate, DeviceTypeResolver & deviceTypeResolver); /** * Deinitialize the access control module. Must be called when finished. @@ -498,6 +507,8 @@ class AccessControl bool IsValid(const Entry & entry); Delegate * mDelegate = nullptr; + + DeviceTypeResolver * mDeviceTypeResolver = nullptr; }; /** diff --git a/src/access/tests/TestAccessControl.cpp b/src/access/tests/TestAccessControl.cpp index da1000aa41902c..cc82123bfdfa6b 100644 --- a/src/access/tests/TestAccessControl.cpp +++ b/src/access/tests/TestAccessControl.cpp @@ -390,6 +390,12 @@ constexpr DeviceTypeId invalidDeviceTypes[] = { }; // clang-format on +class DeviceTypeResolver : public AccessControl::DeviceTypeResolver +{ +public: + bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) override { return false; } +} testDeviceTypeResolver; + // For testing, supports one subject and target, allows any value (valid or invalid) class TestEntryDelegate : public Entry::Delegate { @@ -1335,12 +1341,11 @@ void TestAclValidateTarget(nlTestSuite * inSuite, void * inContext) accessControl.DeleteEntry(1); } - // TODO(#14431): device type target not yet supported (flip != to == when supported) for (auto deviceType : validDeviceTypes) { NL_TEST_ASSERT(inSuite, entry.SetTarget(0, { .flags = Target::kDeviceType, .deviceType = deviceType }) == 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); + 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); } @@ -1358,7 +1363,6 @@ void TestAclValidateTarget(nlTestSuite * inSuite, void * inContext) } } - // TODO(#14431): device type target not yet supported (flip != to == when supported) for (auto cluster : validClusters) { for (auto deviceType : validDeviceTypes) @@ -1368,8 +1372,8 @@ void TestAclValidateTarget(nlTestSuite * inSuite, void * inContext) entry.SetTarget( 0, { .flags = Target::kCluster | Target::kDeviceType, .cluster = cluster, .deviceType = deviceType }) == 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); + 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); } } @@ -2135,7 +2139,7 @@ int Setup(void * inContext) { AccessControl::Delegate * delegate = Examples::GetAccessControlDelegate(nullptr); SetAccessControl(accessControl); - VerifyOrDie(GetAccessControl().Init(delegate) == CHIP_NO_ERROR); + VerifyOrDie(GetAccessControl().Init(delegate, testDeviceTypeResolver) == CHIP_NO_ERROR); return SUCCESS; } diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h index ffe0acc7a3f546..c65373d88f2d0a 100644 --- a/src/app/InteractionModelEngine.h +++ b/src/app/InteractionModelEngine.h @@ -391,5 +391,11 @@ CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDesc * Check if the given cluster has the given DataVersion. */ bool IsClusterDataVersionEqual(const ConcreteClusterPath & aConcreteClusterPath, DataVersion aRequiredVersion); + +/** + * Returns true if device type is on endpoint, false otherwise. + */ +bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint); + } // namespace app } // namespace chip diff --git a/src/app/server/Server.cpp b/src/app/server/Server.cpp index 346631b6d457a3..5ad4143cc6a31c 100644 --- a/src/app/server/Server.cpp +++ b/src/app/server/Server.cpp @@ -70,6 +70,15 @@ void StopEventLoop(intptr_t arg) } } +class DeviceTypeResolver : public chip::Access::AccessControl::DeviceTypeResolver +{ +public: + bool IsDeviceTypeOnEndpoint(chip::DeviceTypeId deviceType, chip::EndpointId endpoint) override + { + return chip::app::IsDeviceTypeOnEndpoint(deviceType, endpoint); + } +} sDeviceTypeResolver; + } // namespace namespace chip { @@ -142,7 +151,7 @@ CHIP_ERROR Server::Init(AppDelegate * delegate, uint16_t secureServicePort, uint accessDelegate = Access::Examples::GetAccessControlDelegate(&mDeviceStorage); VerifyOrExit(accessDelegate != nullptr, ChipLogError(AppServer, "Invalid access delegate found.")); - err = mAccessControl.Init(accessDelegate); + err = mAccessControl.Init(accessDelegate, sDeviceTypeResolver); SuccessOrExit(err); Access::SetAccessControl(mAccessControl); diff --git a/src/app/tests/AppTestContext.cpp b/src/app/tests/AppTestContext.cpp index 8fed737f995ff5..7ac5f4e84c1353 100644 --- a/src/app/tests/AppTestContext.cpp +++ b/src/app/tests/AppTestContext.cpp @@ -24,6 +24,12 @@ namespace { +class TestDeviceTypeResolver : public chip::Access::AccessControl::DeviceTypeResolver +{ +public: + bool IsDeviceTypeOnEndpoint(chip::DeviceTypeId deviceType, chip::EndpointId endpoint) override { return false; } +} gDeviceTypeResolver; + chip::Access::AccessControl gPermissiveAccessControl; } // namespace @@ -37,7 +43,8 @@ CHIP_ERROR AppContext::Init() ReturnErrorOnFailure(chip::app::InteractionModelEngine::GetInstance()->Init(&GetExchangeManager())); Access::SetAccessControl(gPermissiveAccessControl); - ReturnErrorOnFailure(Access::GetAccessControl().Init(chip::Access::Examples::GetPermissiveAccessControlDelegate())); + ReturnErrorOnFailure( + Access::GetAccessControl().Init(chip::Access::Examples::GetPermissiveAccessControlDelegate(), gDeviceTypeResolver)); return CHIP_NO_ERROR; } diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 5ae2a69d0a8a78..671e367f640d48 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -271,6 +271,11 @@ bool IsClusterDataVersionEqual(const ConcreteClusterPath & aConcreteClusterPath, } } +bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) +{ + return false; +} + class TestReadInteraction { public: diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp index 721fc3335be1c9..a016509340f082 100644 --- a/src/app/tests/integration/chip_im_initiator.cpp +++ b/src/app/tests/integration/chip_im_initiator.cpp @@ -672,6 +672,12 @@ bool IsClusterDataVersionEqual(const ConcreteClusterPath & aConcreteClusterPath, { return true; } + +bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) +{ + return false; +} + } // namespace app } // namespace chip diff --git a/src/app/tests/integration/chip_im_responder.cpp b/src/app/tests/integration/chip_im_responder.cpp index 01fcbbb40851de..fe793f407c8a0f 100644 --- a/src/app/tests/integration/chip_im_responder.cpp +++ b/src/app/tests/integration/chip_im_responder.cpp @@ -134,6 +134,12 @@ bool IsClusterDataVersionEqual(const ConcreteClusterPath & aConcreteClusterPath, { return true; } + +bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) +{ + return false; +} + } // namespace app } // namespace chip diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp index 3f696df7e03674..4123728334e3da 100644 --- a/src/app/util/ember-compatibility-functions.cpp +++ b/src/app/util/ember-compatibility-functions.cpp @@ -970,6 +970,12 @@ bool IsClusterDataVersionEqual(const ConcreteClusterPath & aConcreteClusterPath, return (*(version)) == aRequiredVersion; } +bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) +{ + uint16_t index = emberAfIndexFromEndpoint(endpoint); + return index != 0xFFFF && emberAfDeviceIdFromIndex(index) == deviceType; +} + } // namespace app } // namespace chip diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp index 7ce7700f1d16d9..a6b6e74ac1c6e6 100644 --- a/src/controller/tests/data_model/TestRead.cpp +++ b/src/controller/tests/data_model/TestRead.cpp @@ -152,6 +152,11 @@ bool IsClusterDataVersionEqual(const ConcreteClusterPath & aConcreteClusterPath, return false; } } + +bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) +{ + return false; +} } // namespace app } // namespace chip diff --git a/src/darwin/Framework/CHIP/CHIPControllerAccessControl.mm b/src/darwin/Framework/CHIP/CHIPControllerAccessControl.mm index e35e0c6742f699..db3ac12a5d295d 100644 --- a/src/darwin/Framework/CHIP/CHIPControllerAccessControl.mm +++ b/src/darwin/Framework/CHIP/CHIPControllerAccessControl.mm @@ -21,6 +21,7 @@ #include #include #include +#include #include using namespace chip; @@ -32,6 +33,14 @@ // CHIPIMDispatch.mm. constexpr EndpointId kSupportedEndpoint = 0; +class DeviceTypeResolver : public Access::AccessControl::DeviceTypeResolver { +public: + bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) override + { + return app::IsDeviceTypeOnEndpoint(deviceType, endpoint); + } +} gDeviceTypeResolver; + // TODO: Make the policy more configurable by consumers. class AccessControlDelegate : public Access::AccessControl::Delegate { CHIP_ERROR Check( @@ -68,7 +77,7 @@ + (void)init { static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ - GetAccessControl().Init(&gDelegate); + GetAccessControl().Init(&gDelegate, gDeviceTypeResolver); }); } diff --git a/src/darwin/Framework/CHIP/CHIPIMDispatch.mm b/src/darwin/Framework/CHIP/CHIPIMDispatch.mm index 07beba3e0788a6..3cf21bdf31fbce 100644 --- a/src/darwin/Framework/CHIP/CHIPIMDispatch.mm +++ b/src/darwin/Framework/CHIP/CHIPIMDispatch.mm @@ -153,6 +153,8 @@ bool IsClusterDataVersionEqual(const ConcreteClusterPath & aConcreteClusterPath, return false; } + bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) { return false; } + CHIP_ERROR WriteSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, const ConcreteDataAttributePath & aPath, TLV::TLVReader & aReader, WriteHandler * aWriteHandler) {