From 1ccc2af18a51b663ef33a41ab311a2323af4adee Mon Sep 17 00:00:00 2001 From: Song GUO Date: Thu, 3 Mar 2022 01:53:33 +0800 Subject: [PATCH] [AccessControl] Add Access Control checks to event management (#15376) * [ACL] Add ACL checks to event management * Address comments * Update to ToT * Change ACL to Access Control * Do not run TestReadRoundtripWithEventStatusIBInEventReport on ESP32 etc. * Address comments * remove mBypassAccessControl --- src/app/EventLoggingTypes.h | 6 +- src/app/EventManagement.cpp | 144 +++++++++++++----- src/app/EventManagement.h | 48 +++++- src/app/MessageDef/EventPathIB.cpp | 8 + src/app/MessageDef/EventPathIB.h | 10 ++ src/app/ReadClient.cpp | 31 +++- src/app/reporting/Engine.cpp | 2 +- src/app/tests/TestEventLogging.cpp | 3 +- src/app/tests/TestReadInteraction.cpp | 96 +++++++++++- src/controller/TypedReadCallback.h | 4 + .../python/chip/clusters/Attribute.py | 54 ++++--- .../python/chip/clusters/attribute.cpp | 15 +- 12 files changed, 336 insertions(+), 85 deletions(-) diff --git a/src/app/EventLoggingTypes.h b/src/app/EventLoggingTypes.h index e01b4c2bda19d2..d685113d852080 100644 --- a/src/app/EventLoggingTypes.h +++ b/src/app/EventLoggingTypes.h @@ -17,6 +17,7 @@ #pragma once +#include #include #include #include @@ -152,8 +153,7 @@ class EventOptions struct EventLoadOutContext { EventLoadOutContext(TLV::TLVWriter & aWriter, PriorityLevel aPriority, EventNumber aStartingEventNumber) : - mWriter(aWriter), mPriority(aPriority), mStartingEventNumber(aStartingEventNumber), mCurrentEventNumber(0), mFirst(true), - mFabricIndex(0) + mWriter(aWriter), mPriority(aPriority), mStartingEventNumber(aStartingEventNumber), mCurrentEventNumber(0), mFirst(true) {} TLV::TLVWriter & mWriter; @@ -165,7 +165,7 @@ struct EventLoadOutContext size_t mEventCount = 0; ClusterInfo * mpInterestedEventPaths = nullptr; bool mFirst = true; - FabricIndex mFabricIndex = kUndefinedFabricIndex; + Access::SubjectDescriptor mSubjectDescriptor; }; } // namespace app } // namespace chip diff --git a/src/app/EventManagement.cpp b/src/app/EventManagement.cpp index abe3084d612342..49e5460baa59f5 100644 --- a/src/app/EventManagement.cpp +++ b/src/app/EventManagement.cpp @@ -16,8 +16,12 @@ * limitations under the License. */ +#include +#include +#include #include #include +#include #include #include #include @@ -77,29 +81,6 @@ struct CopyAndAdjustDeltaTimeContext EventLoadOutContext * mpContext = nullptr; }; -/** - * @brief - * Internal structure for traversing events. - */ -struct EventEnvelopeContext -{ - EventEnvelopeContext() {} - - int mFieldsToRead = 0; - /* PriorityLevel and DeltaTime are there if that is not first event when putting events in report*/ -#if CHIP_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS & CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME - Timestamp mCurrentTime = Timestamp::System(System::Clock::kZero); -#else - Timestamp mCurrentTime = Timestamp::Epoch(System::Clock::kZero); -#endif - PriorityLevel mPriority = PriorityLevel::First; - ClusterId mClusterId = 0; - EndpointId mEndpointId = 0; - EventId mEventId = 0; - EventNumber mEventNumber = 0; - FabricIndex mFabricIndex = kUndefinedFabricIndex; -}; - void EventManagement::InitializeCounter(Platform::PersistedStorage::Key * apCounterKey, uint32_t aCounterEpoch, PersistedCounter * apPersistedCounter) { @@ -590,46 +571,102 @@ CHIP_ERROR EventManagement::CopyEvent(const TLVReader & aReader, TLVWriter & aWr return CHIP_NO_ERROR; } -static bool IsInterestedEventPaths(EventLoadOutContext * eventLoadOutContext, const EventEnvelopeContext & event) +CHIP_ERROR EventManagement::WriteEventStatusIB(TLVWriter & aWriter, const ConcreteEventPath & aEvent, StatusIB aStatus) +{ + TLVType containerType; + ReturnErrorOnFailure(aWriter.StartContainer(AnonymousTag(), kTLVType_Structure, containerType)); + + EventStatusIB::Builder builder; + builder.Init(&aWriter, to_underlying(EventReportIB::Tag::kEventStatus)); + + ReturnErrorOnFailure(builder.CreatePath() + .Endpoint(aEvent.mEndpointId) + .Cluster(aEvent.mClusterId) + .Event(aEvent.mEventId) + .EndOfEventPathIB() + .GetError()); + + ReturnErrorOnFailure(builder.CreateErrorStatus().EncodeStatusIB(aStatus).GetError()); + + ReturnErrorOnFailure(builder.EndOfEventStatusIB().GetError()); + + ReturnErrorOnFailure(aWriter.EndContainer(containerType)); + ReturnErrorOnFailure(aWriter.Finalize()); + return CHIP_NO_ERROR; +} + +CHIP_ERROR EventManagement::CheckEventContext(EventLoadOutContext * eventLoadOutContext, + const EventManagement::EventEnvelopeContext & event) { if (eventLoadOutContext->mCurrentEventNumber < eventLoadOutContext->mStartingEventNumber) { - return false; + return CHIP_ERROR_UNEXPECTED_EVENT; } - if (event.mFabricIndex != kUndefinedFabricIndex && eventLoadOutContext->mFabricIndex != event.mFabricIndex) + if (event.mFabricIndex != kUndefinedFabricIndex && eventLoadOutContext->mSubjectDescriptor.fabricIndex != event.mFabricIndex) { - return false; + return CHIP_ERROR_UNEXPECTED_EVENT; } ConcreteEventPath path(event.mEndpointId, event.mClusterId, event.mEventId); + CHIP_ERROR ret = CHIP_ERROR_UNEXPECTED_EVENT; + + bool eventReadViaConcretePath = false; + for (auto * interestedPath = eventLoadOutContext->mpInterestedEventPaths; interestedPath != nullptr; interestedPath = interestedPath->mpNext) { if (interestedPath->IsEventPathSupersetOf(path)) { - return true; + ret = CHIP_NO_ERROR; + if (!interestedPath->HasEventWildcard()) + { + eventReadViaConcretePath = true; + break; + } + } + } + + ReturnErrorOnFailure(ret); + + Access::RequestPath requestPath{ .cluster = event.mClusterId, .endpoint = event.mEndpointId }; + Access::Privilege requestPrivilege = RequiredPrivilege::ForReadEvent(path); + CHIP_ERROR accessControlError = + Access::GetAccessControl().Check(eventLoadOutContext->mSubjectDescriptor, requestPath, requestPrivilege); + + if (accessControlError != CHIP_NO_ERROR) + { + ReturnErrorCodeIf(accessControlError != CHIP_ERROR_ACCESS_DENIED, accessControlError); + if (eventReadViaConcretePath) + { + ret = CHIP_ERROR_ACCESS_DENIED; + } + else + { + ret = CHIP_ERROR_UNEXPECTED_EVENT; } } - return false; + + return ret; } -CHIP_ERROR EventManagement::EventIterator(const TLVReader & aReader, size_t aDepth, EventLoadOutContext * apEventLoadOutContext) +CHIP_ERROR EventManagement::EventIterator(const TLVReader & aReader, size_t aDepth, EventLoadOutContext * apEventLoadOutContext, + EventEnvelopeContext * event) { CHIP_ERROR err = CHIP_NO_ERROR; TLVReader innerReader; TLVType tlvType; TLVType tlvType1; - EventEnvelopeContext event; innerReader.Init(aReader); + VerifyOrDie(event != nullptr); ReturnErrorOnFailure(innerReader.EnterContainer(tlvType)); ReturnErrorOnFailure(innerReader.Next()); ReturnErrorOnFailure(innerReader.EnterContainer(tlvType1)); - err = TLV::Utilities::Iterate(innerReader, FetchEventParameters, &event, false /*recurse*/); + err = TLV::Utilities::Iterate(innerReader, FetchEventParameters, event, false /*recurse*/); - if (event.mFieldsToRead != kRequiredEventField) + if (event->mFieldsToRead != kRequiredEventField) { return CHIP_ERROR_INVALID_ARGUMENT; } @@ -640,20 +677,27 @@ CHIP_ERROR EventManagement::EventIterator(const TLVReader & aReader, size_t aDep } ReturnErrorOnFailure(err); - apEventLoadOutContext->mCurrentTime = event.mCurrentTime; - apEventLoadOutContext->mCurrentEventNumber = event.mEventNumber; - if (IsInterestedEventPaths(apEventLoadOutContext, event)) + apEventLoadOutContext->mCurrentTime = event->mCurrentTime; + apEventLoadOutContext->mCurrentEventNumber = event->mEventNumber; + + err = CheckEventContext(apEventLoadOutContext, *event); + if (err == CHIP_NO_ERROR) + { + err = CHIP_EVENT_ID_FOUND; + } + else if (err == CHIP_ERROR_UNEXPECTED_EVENT) { - return CHIP_EVENT_ID_FOUND; + err = CHIP_NO_ERROR; } - return CHIP_NO_ERROR; + return err; } CHIP_ERROR EventManagement::CopyEventsSince(const TLVReader & aReader, size_t aDepth, void * apContext) { EventLoadOutContext * const loadOutContext = static_cast(apContext); - CHIP_ERROR err = EventIterator(aReader, aDepth, loadOutContext); + EventEnvelopeContext event; + CHIP_ERROR err = EventIterator(aReader, aDepth, loadOutContext, &event); if (err == CHIP_EVENT_ID_FOUND) { // checkpoint the writer @@ -675,11 +719,29 @@ CHIP_ERROR EventManagement::CopyEventsSince(const TLVReader & aReader, size_t aD loadOutContext->mFirst = false; loadOutContext->mEventCount++; } + else if (err == CHIP_ERROR_ACCESS_DENIED) + { + // checkpoint the writer + TLV::TLVWriter checkpoint = loadOutContext->mWriter; + + err = WriteEventStatusIB(loadOutContext->mWriter, ConcreteEventPath(event.mEndpointId, event.mClusterId, event.mEventId), + StatusIB(Protocols::InteractionModel::Status::UnsupportedAccess)); + + if (err != CHIP_NO_ERROR) + { + loadOutContext->mWriter = checkpoint; + return err; + } + + loadOutContext->mPreviousTime.mValue = loadOutContext->mCurrentTime.mValue; + loadOutContext->mFirst = false; + loadOutContext->mEventCount++; + } return err; } CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo * apClusterInfolist, EventNumber & aEventMin, - size_t & aEventCount, FabricIndex aFabricIndex) + size_t & aEventCount, const Access::SubjectDescriptor & aSubjectDescriptor) { // TODO: Add particular set of event Paths in FetchEventsSince so that we can filter the interested paths CHIP_ERROR err = CHIP_NO_ERROR; @@ -692,7 +754,7 @@ CHIP_ERROR EventManagement::FetchEventsSince(TLVWriter & aWriter, ClusterInfo * ScopedLock lock(sInstance); #endif // !CHIP_SYSTEM_CONFIG_NO_LOCKING - context.mFabricIndex = aFabricIndex; + context.mSubjectDescriptor = aSubjectDescriptor; context.mpInterestedEventPaths = apClusterInfolist; err = GetEventReader(reader, PriorityLevel::Critical, &bufWrapper); SuccessOrExit(err); diff --git a/src/app/EventManagement.h b/src/app/EventManagement.h index 9daadfadc5f5e6..1257255e58f5d7 100644 --- a/src/app/EventManagement.h +++ b/src/app/EventManagement.h @@ -28,8 +28,10 @@ #include "EventLoggingDelegate.h" #include "EventLoggingTypes.h" +#include #include #include +#include #include #include #include @@ -339,7 +341,7 @@ class EventManagement * */ CHIP_ERROR FetchEventsSince(chip::TLV::TLVWriter & aWriter, ClusterInfo * apClusterInfolist, EventNumber & aEventMin, - size_t & aEventCount, FabricIndex aFabricIndex); + size_t & aEventCount, const Access::SubjectDescriptor & aSubjectDescriptor); /** * @brief @@ -361,6 +363,29 @@ class EventManagement void SetScheduledEventInfo(EventNumber & aEventNumber, uint32_t & aInitialWrittenEventBytes); private: + /** + * @brief + * Internal structure for traversing events. + */ + struct EventEnvelopeContext + { + EventEnvelopeContext() {} + + int mFieldsToRead = 0; + /* PriorityLevel and DeltaTime are there if that is not first event when putting events in report*/ +#if CHIP_CONFIG_EVENT_LOGGING_UTC_TIMESTAMPS & CHIP_SYSTEM_CONFIG_PLATFORM_PROVIDES_TIME + Timestamp mCurrentTime = Timestamp::System(System::Clock::kZero); +#else + Timestamp mCurrentTime = Timestamp::Epoch(System::Clock::kZero); +#endif + PriorityLevel mPriority = PriorityLevel::First; + ClusterId mClusterId = 0; + EndpointId mEndpointId = 0; + EventId mEventId = 0; + EventNumber mEventNumber = 0; + FabricIndex mFabricIndex = kUndefinedFabricIndex; + }; + void VendEventNumber(); CHIP_ERROR CalculateEventSize(EventLoggingDelegate * apDelegate, const EventOptions * apOptions, uint32_t & requiredSize); /** @@ -423,7 +448,8 @@ class EventManagement * The function is used to scan through the event log to find events matching the spec in the supplied context. * Particularly, it would check against mStartingEventNumber, and skip fetched event. */ - static CHIP_ERROR EventIterator(const TLV::TLVReader & aReader, size_t aDepth, EventLoadOutContext * apEventLoadOutContext); + static CHIP_ERROR EventIterator(const TLV::TLVReader & aReader, size_t aDepth, EventLoadOutContext * apEventLoadOutContext, + EventEnvelopeContext * event); /** * @brief Internal iterator function used to fetch event into EventEnvelopeContext, then EventIterator would filter event @@ -449,11 +475,29 @@ class EventManagement return CHIP_ERROR_NO_MEMORY; }; + /** + * @brief Check whether the event instance represented by the EventEnvelopeContext should be included in the report. + * + * @retval CHIP_ERROR_UNEXPECTED_EVENT This path should be excluded in the generated event report. + * @retval CHIP_EVENT_ID_FOUND This path should be included in the generated event report. + * @retval CHIP_ERROR_ACCESS_DENIED This path should be included in the generated event report, but the client does not have + * . enough privilege to access it. + * + * TODO: Consider using CHIP_NO_ERROR, CHIP_ERROR_SKIP_EVENT, CHIP_ERROR_ACCESS_DENINED or some enum to represent the checking + * result. + */ + static CHIP_ERROR CheckEventContext(EventLoadOutContext * eventLoadOutContext, const EventEnvelopeContext & event); + /** * @brief copy event from circular buffer to target buffer for report */ static CHIP_ERROR CopyEvent(const TLV::TLVReader & aReader, TLV::TLVWriter & aWriter, EventLoadOutContext * apContext); + /** + * @brief construct EventStatusIB to target buffer for report + */ + static CHIP_ERROR WriteEventStatusIB(TLV::TLVWriter & aWriter, const ConcreteEventPath & aEvent, StatusIB aStatus); + /** * @brief * A function to get the circular buffer for particular priority diff --git a/src/app/MessageDef/EventPathIB.cpp b/src/app/MessageDef/EventPathIB.cpp index 74668e320efedb..486c9ceddf652f 100644 --- a/src/app/MessageDef/EventPathIB.cpp +++ b/src/app/MessageDef/EventPathIB.cpp @@ -156,6 +156,14 @@ CHIP_ERROR EventPathIB::Parser::GetEvent(EventId * const apEvent) const return GetUnsignedInteger(to_underlying(Tag::kEvent), apEvent); } +CHIP_ERROR EventPathIB::Parser::GetEventPath(ConcreteEventPath * const apPath) const +{ + VerifyOrReturnError(GetEndpoint(&(apPath->mEndpointId)) == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH); + VerifyOrReturnError(GetCluster(&(apPath->mClusterId)) == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH); + VerifyOrReturnError(GetEvent(&(apPath->mEventId)) == CHIP_NO_ERROR, CHIP_ERROR_IM_MALFORMED_EVENT_PATH); + return CHIP_NO_ERROR; +} + CHIP_ERROR EventPathIB::Parser::GetIsUrgent(bool * const apIsUrgent) const { return GetSimpleValue(to_underlying(Tag::kIsUrgent), TLV::kTLVType_Boolean, apIsUrgent); diff --git a/src/app/MessageDef/EventPathIB.h b/src/app/MessageDef/EventPathIB.h index ee914f162e78c4..a11a7d22518836 100644 --- a/src/app/MessageDef/EventPathIB.h +++ b/src/app/MessageDef/EventPathIB.h @@ -115,6 +115,16 @@ class Parser : public ListParser * #CHIP_END_OF_TLV if there is no such element */ CHIP_ERROR GetIsUrgent(bool * const apIsUrgent) const; + + /** + * @brief Fill the fields in apPath from the parser, the path in the parser should be a concrete path. + * + * @param [in] apEvent A pointer to apEvent + * + * @return #CHIP_NO_ERROR on success + * #CHIP_ERROR_IM_MALFORMED_EVENT_PATH if the path from the reader is not a valid concrere event path. + */ + CHIP_ERROR GetEventPath(ConcreteEventPath * const apPath) const; }; class Builder : public ListBuilder diff --git a/src/app/ReadClient.cpp b/src/app/ReadClient.cpp index 8356407b90bf22..9ee2e760f41141 100644 --- a/src/app/ReadClient.cpp +++ b/src/app/ReadClient.cpp @@ -643,19 +643,36 @@ CHIP_ERROR ReadClient::ProcessEventReportIBs(TLV::TLVReader & aEventReportIBsRea EventReportIB::Parser report; EventDataIB::Parser data; EventHeader header; + StatusIB statusIB; // Default value for statusIB is success. TLV::TLVReader reader = aEventReportIBsReader; ReturnErrorOnFailure(report.Init(reader)); - ReturnErrorOnFailure(report.GetEventData(&data)); + err = report.GetEventData(&data); - header.mTimestamp = mEventTimestamp; - ReturnErrorOnFailure(data.DecodeEventHeader(header)); - mEventTimestamp = header.mTimestamp; - mEventMin = header.mEventNumber + 1; - ReturnErrorOnFailure(data.GetData(&dataReader)); + if (err == CHIP_NO_ERROR) + { + header.mTimestamp = mEventTimestamp; + ReturnErrorOnFailure(data.DecodeEventHeader(header)); + mEventTimestamp = header.mTimestamp; + mEventMin = header.mEventNumber + 1; + ReturnErrorOnFailure(data.GetData(&dataReader)); - mpCallback.OnEventData(header, &dataReader, nullptr); + mpCallback.OnEventData(header, &dataReader, nullptr); + } + else if (err == CHIP_END_OF_TLV) + { + EventStatusIB::Parser status; + EventPathIB::Parser pathIB; + StatusIB::Parser statusIBParser; + ReturnErrorOnFailure(report.GetEventStatus(&status)); + ReturnErrorOnFailure(status.GetPath(&pathIB)); + ReturnErrorOnFailure(pathIB.GetEventPath(&header.mPath)); + ReturnErrorOnFailure(status.GetErrorStatus(&statusIBParser)); + ReturnErrorOnFailure(statusIBParser.DecodeStatusIB(statusIB)); + + mpCallback.OnEventData(header, nullptr, &statusIB); + } } if (CHIP_END_OF_TLV == err) diff --git a/src/app/reporting/Engine.cpp b/src/app/reporting/Engine.cpp index 09f8d066b74504..ac35a882cabc2d 100644 --- a/src/app/reporting/Engine.cpp +++ b/src/app/reporting/Engine.cpp @@ -292,7 +292,7 @@ CHIP_ERROR Engine::BuildSingleReportDataEventReports(ReportDataMessage::Builder EventReportIBs::Builder & eventReportIBs = aReportDataBuilder.CreateEventReports(); SuccessOrExit(err = aReportDataBuilder.GetError()); err = eventManager.FetchEventsSince(*(eventReportIBs.GetWriter()), clusterInfoList, eventMin, eventCount, - apReadHandler->GetAccessingFabricIndex()); + apReadHandler->GetSubjectDescriptor()); if ((err == CHIP_END_OF_TLV) || (err == CHIP_ERROR_TLV_UNDERRUN) || (err == CHIP_NO_ERROR)) { diff --git a/src/app/tests/TestEventLogging.cpp b/src/app/tests/TestEventLogging.cpp index aa61e76849ffa1..db9104bec40b55 100644 --- a/src/app/tests/TestEventLogging.cpp +++ b/src/app/tests/TestEventLogging.cpp @@ -22,6 +22,7 @@ * */ +#include #include #include #include @@ -141,7 +142,7 @@ static void CheckLogReadOut(nlTestSuite * apSuite, chip::app::EventManagement & uint8_t backingStore[1024]; size_t totalNumElements; writer.Init(backingStore, 1024); - err = alogMgmt.FetchEventsSince(writer, clusterInfo, startingEventNumber, eventCount, 0); + err = alogMgmt.FetchEventsSince(writer, clusterInfo, startingEventNumber, eventCount, chip::Access::SubjectDescriptor{}); NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR || err == CHIP_END_OF_TLV); reader.Init(backingStore, writer.GetLengthWritten()); diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp index 275526e36aad1d..fc3ccd00366714 100644 --- a/src/app/tests/TestReadInteraction.cpp +++ b/src/app/tests/TestReadInteraction.cpp @@ -149,6 +149,15 @@ class MockInteractionModelApp : public chip::app::ReadClient::Callback { ++mNumDataElementIndex; mGotEventResponse = true; + if (apStatus != nullptr && !apStatus->IsSuccess()) + { + mNumReadEventFailureStatusReceived++; + mLastStatusReceived = *apStatus; + } + else + { + mLastStatusReceived = chip::app::StatusIB(); + } } void OnAttributeData(const chip::app::ConcreteDataAttributePath & aPath, chip::TLV::TLVReader * apData, @@ -159,6 +168,7 @@ class MockInteractionModelApp : public chip::app::ReadClient::Callback mNumAttributeResponse++; mGotReport = true; } + mLastStatusReceived = status; } void OnError(CHIP_ERROR aError) override @@ -189,11 +199,13 @@ class MockInteractionModelApp : public chip::app::ReadClient::Callback int mNumDataElementIndex = 0; bool mGotEventResponse = false; + int mNumReadEventFailureStatusReceived = 0; int mNumAttributeResponse = 0; bool mGotReport = false; bool mReadError = false; chip::app::ReadHandler * mpReadHandler = nullptr; - CHIP_ERROR mError = CHIP_NO_ERROR; + chip::app::StatusIB mLastStatusReceived; + CHIP_ERROR mError = CHIP_NO_ERROR; }; // @@ -277,6 +289,7 @@ class TestReadInteraction static void TestReadRoundtripWithNoMatchPathDataVersionFilter(nlTestSuite * apSuite, void * apContext); static void TestReadRoundtripWithMultiSamePathDifferentDataVersionFilter(nlTestSuite * apSuite, void * apContext); static void TestReadRoundtripWithSameDifferentPathsDataVersionFilter(nlTestSuite * apSuite, void * apContext); + static void TestReadRoundtripWithEventStatusIBInEventReport(nlTestSuite * apSuite, void * apContext); static void TestReadWildcard(nlTestSuite * apSuite, void * apContext); static void TestReadChunking(nlTestSuite * apSuite, void * apContext); static void TestSetDirtyBetweenChunks(nlTestSuite * apSuite, void * apContext); @@ -1004,6 +1017,84 @@ void TestReadInteraction::TestReadRoundtripWithSameDifferentPathsDataVersionFilt NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); } +void TestReadInteraction::TestReadRoundtripWithEventStatusIBInEventReport(nlTestSuite * apSuite, void * apContext) +{ + TestContext & ctx = *static_cast(apContext); + CHIP_ERROR err = CHIP_NO_ERROR; + + Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr(); + // Shouldn't have anything in the retransmit table when starting the test. + NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0); + + GenerateEvents(apSuite, apContext); + + auto * engine = chip::app::InteractionModelEngine::GetInstance(); + err = engine->Init(&ctx.GetExchangeManager()); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + // When reading events with concrete paths without enough privilege, we will get a EventStatusIB + { + chip::app::EventPathParams eventPathParams[1]; + eventPathParams[0].mEndpointId = kTestEndpointId; + eventPathParams[0].mClusterId = kTestClusterId; + eventPathParams[0].mEventId = kTestEventIdDebug; + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + readPrepareParams.mpEventPathParamsList = eventPathParams; + readPrepareParams.mEventPathParamsListSize = 1; + readPrepareParams.mEventNumber = 1; + + MockInteractionModelApp delegate; + NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); + + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Read); + + err = readClient.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + + NL_TEST_ASSERT(apSuite, delegate.mGotEventResponse); + NL_TEST_ASSERT(apSuite, delegate.mNumReadEventFailureStatusReceived > 0); + NL_TEST_ASSERT(apSuite, delegate.mLastStatusReceived.mStatus == Protocols::InteractionModel::Status::UnsupportedAccess); + NL_TEST_ASSERT(apSuite, !delegate.mReadError); + } + + GenerateEvents(apSuite, apContext); + + // When reading events with withcard paths without enough privilege for reading one or more event, we will exclude the events + // when generating the report. + { + chip::app::EventPathParams eventPathParams[1]; + eventPathParams[0].mEndpointId = kTestEndpointId; + eventPathParams[0].mClusterId = kTestClusterId; + + ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice()); + readPrepareParams.mpEventPathParamsList = eventPathParams; + readPrepareParams.mEventPathParamsListSize = 1; + readPrepareParams.mEventNumber = 1; + + MockInteractionModelApp delegate; + NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); + + app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate, + chip::app::ReadClient::InteractionType::Read); + + err = readClient.SendRequest(readPrepareParams); + NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR); + + ctx.DrainAndServiceIO(); + NL_TEST_ASSERT(apSuite, !delegate.mGotEventResponse); + NL_TEST_ASSERT(apSuite, delegate.mNumReadEventFailureStatusReceived == 0); + NL_TEST_ASSERT(apSuite, !delegate.mReadError); + } + + NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0); + engine->Shutdown(); + NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0); +} + void TestReadInteraction::TestReadWildcard(nlTestSuite * apSuite, void * apContext) { TestContext & ctx = *static_cast(apContext); @@ -2372,6 +2463,9 @@ const nlTest sTests[] = NL_TEST_DEF("TestReadRoundtripWithNoMatchPathDataVersionFilter", chip::app::TestReadInteraction::TestReadRoundtripWithNoMatchPathDataVersionFilter), NL_TEST_DEF("TestReadRoundtripWithMultiSamePathDifferentDataVersionFilter", chip::app::TestReadInteraction::TestReadRoundtripWithMultiSamePathDifferentDataVersionFilter), NL_TEST_DEF("TestReadRoundtripWithSameDifferentPathsDataVersionFilter", chip::app::TestReadInteraction::TestReadRoundtripWithSameDifferentPathsDataVersionFilter), + // TODO(#10253): In unit tests, the access control delegate will always pass, in this case, we will never get a StatusIB for + // UnsupportedAccess status code, the test below can be re-enabled after we have a better access control function in unit tests. + // NL_TEST_DEF("TestReadRoundtripWithEventStatusIBInEventReport", chip::app::TestReadInteraction::TestReadRoundtripWithEventStatusIBInEventReport), NL_TEST_DEF("TestReadWildcard", chip::app::TestReadInteraction::TestReadWildcard), NL_TEST_DEF("TestReadChunking", chip::app::TestReadInteraction::TestReadChunking), NL_TEST_DEF("TestSetDirtyBetweenChunks", chip::app::TestReadInteraction::TestSetDirtyBetweenChunks), diff --git a/src/controller/TypedReadCallback.h b/src/controller/TypedReadCallback.h index 12b420091e93f2..da1c5f241a314c 100644 --- a/src/controller/TypedReadCallback.h +++ b/src/controller/TypedReadCallback.h @@ -149,6 +149,10 @@ class TypedReadEventCallback final : public app::ReadClient::Callback CHIP_ERROR err = CHIP_NO_ERROR; DecodableEventType value; + // Only one of the apData and apStatus can be non-null, so apStatus will always indicate a failure status when it is not + // nullptr. + VerifyOrExit(apStatus == nullptr, err = apStatus->ToChipError()); + VerifyOrExit(apData != nullptr, err = CHIP_ERROR_INVALID_ARGUMENT); VerifyOrExit((aEventHeader.mPath.mEventId == value.GetEventId()) && (aEventHeader.mPath.mClusterId == value.GetClusterId()), diff --git a/src/controller/python/chip/clusters/Attribute.py b/src/controller/python/chip/clusters/Attribute.py index 4214a45364791f..d306e16617ae48 100644 --- a/src/controller/python/chip/clusters/Attribute.py +++ b/src/controller/python/chip/clusters/Attribute.py @@ -604,32 +604,36 @@ def _handleAttributeData(self, path: AttributePathWithListIndex, dataVersion: in def handleAttributeData(self, path: AttributePath, dataVersion: int, status: int, data: bytes): self._handleAttributeData(path, dataVersion, status, data) - def _handleEventData(self, header: EventHeader, path: EventPath, data: bytes): + def _handleEventData(self, header: EventHeader, path: EventPath, data: bytes, status: int): try: eventType = _EventIndex.get(str(path), None) eventValue = None - tlvData = chip.tlv.TLVReader(data).get().get("Any", {}) - if eventType is None: - eventValue = ValueDecodeFailure( - tlvData, LookupError("event schema not found")) - else: - try: - eventValue = eventType.FromTLV(data) - except Exception as ex: - logging.error( - f"Error convering TLV to Cluster Object for path: Endpoint = {path.EndpointId}/Cluster = {path.ClusterId}/Event = {path.EventId}") - logging.error( - f"Failed Cluster Object: {str(eventType)}") - logging.error(ex) + + if data: + # data will be an empty buffer when we received an EventStatusIB instead of an EventDataIB. + tlvData = chip.tlv.TLVReader(data).get().get("Any", {}) + + if eventType is None: eventValue = ValueDecodeFailure( - tlvData, ex) + tlvData, LookupError("event schema not found")) + else: + try: + eventValue = eventType.FromTLV(data) + except Exception as ex: + logging.error( + f"Error convering TLV to Cluster Object for path: Endpoint = {path.EndpointId}/Cluster = {path.ClusterId}/Event = {path.EventId}") + logging.error( + f"Failed Cluster Object: {str(eventType)}") + logging.error(ex) + eventValue = ValueDecodeFailure( + tlvData, ex) - # If we're in debug mode, raise the exception so that we can better debug what's happening. - if (builtins.enableDebugMode): - raise + # If we're in debug mode, raise the exception so that we can better debug what's happening. + if (builtins.enableDebugMode): + raise eventResult = EventReadResult( - Header=header, Data=eventValue, Status=chip.interaction_model.Status.Success) + Header=header, Data=eventValue, Status=chip.interaction_model.Status(status)) self._events.append(eventResult) if (self._subscription_handler is not None): @@ -639,8 +643,8 @@ def _handleEventData(self, header: EventHeader, path: EventPath, data: bytes): except Exception as ex: logging.exception(ex) - def handleEventData(self, header: EventHeader, path: EventPath, data: bytes): - self._handleEventData(header, path, data) + def handleEventData(self, header: EventHeader, path: EventPath, data: bytes, status: int): + self._handleEventData(header, path, data, status) def _handleError(self, chipError: int): self._future.set_exception( @@ -730,10 +734,10 @@ def handleDone(self): _OnReadAttributeDataCallbackFunct = CFUNCTYPE( - None, py_object, c_uint32, c_uint16, c_uint32, c_uint32, c_uint32, c_void_p, c_size_t) + None, py_object, c_uint32, c_uint16, c_uint32, c_uint32, c_uint8, c_void_p, c_size_t) _OnSubscriptionEstablishedCallbackFunct = CFUNCTYPE(None, py_object, c_uint64) _OnReadEventDataCallbackFunct = CFUNCTYPE( - None, py_object, c_uint16, c_uint32, c_uint32, c_uint32, c_uint8, c_uint64, c_uint8, c_void_p, c_size_t) + None, py_object, c_uint16, c_uint32, c_uint32, c_uint32, c_uint8, c_uint64, c_uint8, c_void_p, c_size_t, c_uint8) _OnReadErrorCallbackFunct = CFUNCTYPE( None, py_object, c_uint32) _OnReadDoneCallbackFunct = CFUNCTYPE( @@ -752,11 +756,11 @@ def _OnReadAttributeDataCallback(closure, dataVersion: int, endpoint: int, clust @_OnReadEventDataCallbackFunct -def _OnReadEventDataCallback(closure, endpoint: int, cluster: int, event: int, number: int, priority: int, timestamp: int, timestampType: int, data, len): +def _OnReadEventDataCallback(closure, endpoint: int, cluster: int, event: int, number: int, priority: int, timestamp: int, timestampType: int, data, len, status): dataBytes = ctypes.string_at(data, len) path = EventPath(ClusterId=cluster, EventId=event) closure.handleEventData(EventHeader( - EndpointId=endpoint, EventNumber=number, Priority=EventPriority(priority), Timestamp=timestamp, TimestampType=EventTimestampType(timestampType)), path, dataBytes[:]) + EndpointId=endpoint, EventNumber=number, Priority=EventPriority(priority), Timestamp=timestamp, TimestampType=EventTimestampType(timestampType)), path, dataBytes[:], status) @_OnSubscriptionEstablishedCallbackFunct diff --git a/src/controller/python/chip/clusters/attribute.cpp b/src/controller/python/chip/clusters/attribute.cpp index 3156b077f6137b..99b378f97dcfc4 100644 --- a/src/controller/python/chip/clusters/attribute.cpp +++ b/src/controller/python/chip/clusters/attribute.cpp @@ -66,7 +66,8 @@ using OnReadAttributeDataCallback = void (*)(PyObject * appContext, chip:: uint32_t dataLen); using OnReadEventDataCallback = void (*)(PyObject * appContext, chip::EndpointId endpointId, chip::ClusterId clusterId, chip::EventId eventId, chip::EventNumber eventNumber, uint8_t priority, uint64_t timestamp, - uint8_t timestampType, uint8_t * data, uint32_t dataLen); + uint8_t timestampType, uint8_t * data, uint32_t dataLen, + std::underlying_type_t imstatus); using OnSubscriptionEstablishedCallback = void (*)(PyObject * appContext, uint64_t subscriptionId); using OnReadErrorCallback = void (*)(PyObject * appContext, uint32_t chiperror); using OnReadDoneCallback = void (*)(PyObject * appContext); @@ -156,15 +157,21 @@ class ReadClientCallback : public ReadClient::Callback } size = writer.GetLengthWritten(); } + else if (apStatus != nullptr) + { + size = 0; + } else { err = CHIP_ERROR_INCORRECT_STATE; this->OnError(err); } - gOnReadEventDataCallback(mAppContext, aEventHeader.mPath.mEndpointId, aEventHeader.mPath.mClusterId, - aEventHeader.mPath.mEventId, aEventHeader.mEventNumber, to_underlying(aEventHeader.mPriorityLevel), - aEventHeader.mTimestamp.mValue, to_underlying(aEventHeader.mTimestamp.mType), buffer, size); + gOnReadEventDataCallback( + mAppContext, aEventHeader.mPath.mEndpointId, aEventHeader.mPath.mClusterId, aEventHeader.mPath.mEventId, + aEventHeader.mEventNumber, to_underlying(aEventHeader.mPriorityLevel), aEventHeader.mTimestamp.mValue, + to_underlying(aEventHeader.mTimestamp.mType), buffer, size, + to_underlying(apStatus == nullptr ? Protocols::InteractionModel::Status::Success : apStatus->mStatus)); } void OnError(CHIP_ERROR aError) override { gOnReadErrorCallback(mAppContext, aError.AsInteger()); }