From e88696b35bf68f38823246dc8b41a2c5abc7fff9 Mon Sep 17 00:00:00 2001
From: yunhanw <yunhanw@google.com>
Date: Mon, 29 Aug 2022 12:34:12 -0700
Subject: [PATCH 1/2] Prevent Subscription to attributes with no access
privilege
---
src/app/InteractionModelEngine.cpp | 142 +++++++--
src/app/InteractionModelEngine.h | 22 ++
src/app/MessageDef/AttributePathIB.cpp | 57 ++++
src/app/MessageDef/AttributePathIB.h | 8 +
src/app/MessageDef/EventPathIB.cpp | 46 +++
src/app/MessageDef/EventPathIB.h | 8 +
src/app/ReadHandler.cpp | 114 +------
src/app/ReadHandler.h | 2 +-
src/app/tests/BUILD.gn | 2 +
src/app/tests/TestAclAttribute.cpp | 283 ++++++++++++++++++
src/app/tests/TestAclEvent.cpp | 8 +-
src/app/tests/TestReadInteraction.cpp | 47 +++
.../tests/integration/chip_im_initiator.cpp | 5 +
.../tests/integration/chip_im_responder.cpp | 5 +
.../util/ember-compatibility-functions.cpp | 12 +
src/controller/tests/TestReadChunking.cpp | 29 +-
src/controller/tests/data_model/TestRead.cpp | 56 +++-
src/darwin/Framework/CHIP/MTRIMDispatch.mm | 5 +
18 files changed, 692 insertions(+), 159 deletions(-)
create mode 100644 src/app/tests/TestAclAttribute.cpp
diff --git a/src/app/InteractionModelEngine.cpp b/src/app/InteractionModelEngine.cpp
index 3f88c40f5f9910..61af4c1b19b77b 100644
--- a/src/app/InteractionModelEngine.cpp
+++ b/src/app/InteractionModelEngine.cpp
@@ -27,6 +27,9 @@
#include <cinttypes>
+#include "access/RequestPath.h"
+#include "access/SubjectDescriptor.h"
+#include <app/RequiredPrivilege.h>
#include <lib/core/CHIPTLVUtilities.hpp>
#include <lib/support/CodeUtils.h>
@@ -323,6 +326,79 @@ Status InteractionModelEngine::OnInvokeCommandRequest(Messaging::ExchangeContext
return Status::Success;
}
+CHIP_ERROR InteractionModelEngine::ParseAttributePaths(const Access::SubjectDescriptor & aSubjectDescriptor,
+ AttributePathIBs::Parser & aAttributePathListParser,
+ bool & aHasValidAttributePath, size_t & aRequestedAttributePathCount)
+{
+ TLV::TLVReader pathReader;
+ aAttributePathListParser.GetReader(&pathReader);
+ CHIP_ERROR err = CHIP_NO_ERROR;
+
+ aHasValidAttributePath = false;
+ aRequestedAttributePathCount = 0;
+
+ while (CHIP_NO_ERROR == (err = pathReader.Next()))
+ {
+ VerifyOrReturnError(TLV::AnonymousTag() == pathReader.GetTag(), CHIP_ERROR_INVALID_TLV_TAG);
+
+ AttributePathIB::Parser path;
+ //
+ // We create an iterator to point to a single item object list that tracks the path we just parsed.
+ // This avoids the 'parse all paths' approach that is employed in ReadHandler since we want to
+ // avoid allocating out of the path store during this minimal initial processing stage.
+ //
+ ObjectList<AttributePathParams> paramsList;
+
+ ReturnErrorOnFailure(path.Init(pathReader));
+ ReturnErrorOnFailure(path.ParsePath(paramsList.mValue));
+
+ if (paramsList.mValue.IsWildcardPath())
+ {
+ AttributePathExpandIterator pathIterator(¶msList);
+ ConcreteAttributePath readPath;
+
+ // The definition of "valid path" is "path exists and ACL allows access". The "path exists" part is handled by
+ // AttributePathExpandIterator. So we just need to check the ACL bits.
+ for (; pathIterator.Get(readPath); pathIterator.Next())
+ {
+ Access::RequestPath requestPath{ .cluster = readPath.mClusterId, .endpoint = readPath.mEndpointId };
+ err = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath,
+ RequiredPrivilege::ForReadAttribute(readPath));
+ if (err == CHIP_NO_ERROR)
+ {
+ aHasValidAttributePath = true;
+ break;
+ }
+ }
+ }
+ else
+ {
+ ConcreteAttributePath concretePath(paramsList.mValue.mEndpointId, paramsList.mValue.mClusterId,
+ paramsList.mValue.mAttributeId);
+ if (ConcreteAttributePathExists(concretePath))
+ {
+ Access::RequestPath requestPath{ .cluster = concretePath.mClusterId, .endpoint = concretePath.mEndpointId };
+
+ err = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath,
+ RequiredPrivilege::ForReadAttribute(concretePath));
+ if (err == CHIP_NO_ERROR)
+ {
+ aHasValidAttributePath = true;
+ }
+ }
+ }
+
+ aRequestedAttributePathCount++;
+ }
+
+ if (err == CHIP_ERROR_END_OF_TLV)
+ {
+ err = CHIP_NO_ERROR;
+ }
+
+ return err;
+}
+
Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest(Messaging::ExchangeContext * apExchangeContext,
const PayloadHeader & aPayloadHeader,
System::PacketBufferHandle && aPayload,
@@ -349,28 +425,25 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest
reader.Init(aPayload.Retain());
SubscribeRequestMessage::Parser subscribeRequestParser;
- CHIP_ERROR err = subscribeRequestParser.Init(reader);
- if (err != CHIP_NO_ERROR)
- {
- return Status::InvalidAction;
- }
-
+ VerifyOrReturnError(subscribeRequestParser.Init(reader) == CHIP_NO_ERROR, Status::InvalidAction);
{
size_t requestedAttributePathCount = 0;
size_t requestedEventPathCount = 0;
AttributePathIBs::Parser attributePathListParser;
- err = subscribeRequestParser.GetAttributeRequests(&attributePathListParser);
+ bool hasValidAttributePath = false;
+
+ CHIP_ERROR err = subscribeRequestParser.GetAttributeRequests(&attributePathListParser);
if (err == CHIP_NO_ERROR)
{
- TLV::TLVReader pathReader;
- attributePathListParser.GetReader(&pathReader);
- err = TLV::Utilities::Count(pathReader, requestedAttributePathCount, false);
- }
- else if (err == CHIP_ERROR_END_OF_TLV)
- {
- err = CHIP_NO_ERROR;
+ auto subjectDescriptor = apExchangeContext->GetSessionHandle()->AsSecureSession()->GetSubjectDescriptor();
+ err = ParseAttributePaths(subjectDescriptor, attributePathListParser, hasValidAttributePath,
+ requestedAttributePathCount);
+ if (err != CHIP_NO_ERROR)
+ {
+ return Status::InvalidAction;
+ }
}
- if (err != CHIP_NO_ERROR)
+ else if (err != CHIP_ERROR_END_OF_TLV)
{
return Status::InvalidAction;
}
@@ -381,14 +454,34 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest
{
TLV::TLVReader pathReader;
eventpathListParser.GetReader(&pathReader);
- err = TLV::Utilities::Count(pathReader, requestedEventPathCount, false);
+ ReturnErrorCodeIf(TLV::Utilities::Count(pathReader, requestedEventPathCount, false) != CHIP_NO_ERROR,
+ Status::InvalidAction);
}
- else if (err == CHIP_ERROR_END_OF_TLV)
+ else if (err != CHIP_ERROR_END_OF_TLV)
{
- err = CHIP_NO_ERROR;
+ return Status::InvalidAction;
}
- if (err != CHIP_NO_ERROR)
+
+ if (requestedAttributePathCount == 0 && requestedEventPathCount == 0)
{
+ ChipLogError(InteractionModel,
+ "Subscription from [%u:" ChipLogFormatX64 "] has no attribute or event paths. Rejecting request.",
+ apExchangeContext->GetSessionHandle()->GetFabricIndex(),
+ ChipLogValueX64(apExchangeContext->GetSessionHandle()->AsSecureSession()->GetPeerNodeId()));
+ return Status::InvalidAction;
+ }
+
+ //
+ // TODO: We don't have an easy way to do a similar 'path expansion' for events to deduce
+ // access so for now, assume that the presence of any path in the event list means they
+ // might have some access to those events.
+ //
+ if (!hasValidAttributePath && requestedEventPathCount == 0)
+ {
+ ChipLogError(InteractionModel,
+ "Subscription from [%u:" ChipLogFormatX64 "] has no access at all. Rejecting request.",
+ apExchangeContext->GetSessionHandle()->GetFabricIndex(),
+ ChipLogValueX64(apExchangeContext->GetSessionHandle()->AsSecureSession()->GetPeerNodeId()));
return Status::InvalidAction;
}
@@ -400,12 +493,8 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest
}
}
- err = subscribeRequestParser.GetKeepSubscriptions(&keepExistingSubscriptions);
- if (err != CHIP_NO_ERROR)
- {
- return Status::InvalidAction;
- }
-
+ VerifyOrReturnError(subscribeRequestParser.GetKeepSubscriptions(&keepExistingSubscriptions) == CHIP_NO_ERROR,
+ Status::InvalidAction);
if (!keepExistingSubscriptions)
{
//
@@ -426,8 +515,7 @@ Protocols::InteractionModel::Status InteractionModelEngine::OnReadInitialRequest
});
}
}
-
- if (aInteractionType == ReadHandler::InteractionType::Read)
+ else
{
System::PacketBufferTLVReader reader;
reader.Init(aPayload.Retain());
diff --git a/src/app/InteractionModelEngine.h b/src/app/InteractionModelEngine.h
index 0bd2c6738a3818..6475f8bceda547 100644
--- a/src/app/InteractionModelEngine.h
+++ b/src/app/InteractionModelEngine.h
@@ -378,6 +378,21 @@ class InteractionModelEngine : public Messaging::UnsolicitedMessageHandler,
System::PacketBufferHandle && aPayload) override;
void OnResponseTimeout(Messaging::ExchangeContext * ec) override;
+ /**
+ * This parses the attribute path list to ensure it is well formed. If so, for each path in the list, it will expand to a list
+ * of concrete paths and walk each path to check if it has privileges to read that attribute.
+ *
+ * If there is AT LEAST one "existent path" (as the spec calls it) that has sufficient privilege, aHasValidAttributePath
+ * will be set to true. Otherwise, it will be set to false.
+ *
+ * aRequestedAttributePathCount will be updated to reflect the number of attribute paths in the request.
+ *
+ *
+ */
+ static CHIP_ERROR ParseAttributePaths(const Access::SubjectDescriptor & aSubjectDescriptor,
+ AttributePathIBs::Parser & aAttributePathListParser, bool & aHasValidAttributePath,
+ size_t & aRequestedAttributePathCount);
+
/**
* Called when Interaction Model receives a Read Request message. Errors processing
* the Read Request are handled entirely within this function. If the
@@ -617,6 +632,13 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr
const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports,
AttributeValueEncoder::AttributeEncodeState * apEncoderState);
+/**
+ * Check whether concrete attribute path is an "existent attribute path" in spec terms.
+ * @param[in] aPath The concrete path of the data being read.
+ * @retval boolean
+ */
+bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath);
+
/**
* Get the registered attribute access override. nullptr when attribute access override is not found.
*
diff --git a/src/app/MessageDef/AttributePathIB.cpp b/src/app/MessageDef/AttributePathIB.cpp
index c8d0fa51d29f4c..97d7f60568af0e 100644
--- a/src/app/MessageDef/AttributePathIB.cpp
+++ b/src/app/MessageDef/AttributePathIB.cpp
@@ -227,6 +227,63 @@ CHIP_ERROR AttributePathIB::Parser::GetListIndex(ConcreteDataAttributePath & aAt
return err;
}
+CHIP_ERROR AttributePathIB::Parser::ParsePath(AttributePathParams & aAttribute) const
+{
+ CHIP_ERROR err = CHIP_NO_ERROR;
+
+ err = GetEndpoint(&(aAttribute.mEndpointId));
+ if (err == CHIP_NO_ERROR)
+ {
+ VerifyOrReturnError(!aAttribute.HasWildcardEndpointId(), CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ }
+ else if (err == CHIP_END_OF_TLV)
+ {
+ err = CHIP_NO_ERROR;
+ }
+ VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
+ err = GetCluster(&aAttribute.mClusterId);
+ if (err == CHIP_NO_ERROR)
+ {
+ VerifyOrReturnError(IsValidClusterId(aAttribute.mClusterId), CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ }
+ else if (err == CHIP_END_OF_TLV)
+ {
+ err = CHIP_NO_ERROR;
+ }
+ VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
+ err = GetAttribute(&aAttribute.mAttributeId);
+ if (err == CHIP_NO_ERROR)
+ {
+ VerifyOrReturnError(IsValidAttributeId(aAttribute.mAttributeId), CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ }
+ else if (err == CHIP_END_OF_TLV)
+ {
+ err = CHIP_NO_ERROR;
+ }
+ VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
+ // A wildcard cluster requires that the attribute path either be
+ // wildcard or a global attribute.
+ VerifyOrReturnError(!aAttribute.HasWildcardClusterId() || aAttribute.HasWildcardAttributeId() ||
+ IsGlobalAttribute(aAttribute.mAttributeId),
+ CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
+ err = GetListIndex(&aAttribute.mListIndex);
+ if (err == CHIP_NO_ERROR)
+ {
+ VerifyOrReturnError(!aAttribute.HasWildcardAttributeId() && !aAttribute.HasWildcardListIndex(),
+ CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ }
+ else if (err == CHIP_END_OF_TLV)
+ {
+ err = CHIP_NO_ERROR;
+ }
+ VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ return CHIP_NO_ERROR;
+}
+
AttributePathIB::Builder & AttributePathIB::Builder::EnableTagCompression(const bool aEnableTagCompression)
{
// skip if error has already been set
diff --git a/src/app/MessageDef/AttributePathIB.h b/src/app/MessageDef/AttributePathIB.h
index e5192ccb119739..81b1a33e58a904 100644
--- a/src/app/MessageDef/AttributePathIB.h
+++ b/src/app/MessageDef/AttributePathIB.h
@@ -153,6 +153,14 @@ class Parser : public ListParser
CHIP_ERROR GetListIndex(ConcreteDataAttributePath & aAttributePath) const;
// TODO(#14934) Add a function to get ConcreteDataAttributePath from AttributePathIB::Parser directly.
+
+ /**
+ * @brief Parse the attribute path into an AttributePathParams object. As part of parsing,
+ * validity checks for each path item will be done as well.
+ *
+ * If any errors are encountered, an IM error of 'InvalidAction' will be returned.
+ */
+ CHIP_ERROR ParsePath(AttributePathParams & attribute) const;
};
class Builder : public ListBuilder
diff --git a/src/app/MessageDef/EventPathIB.cpp b/src/app/MessageDef/EventPathIB.cpp
index 35c5b098b94c55..84c9f6680bf260 100644
--- a/src/app/MessageDef/EventPathIB.cpp
+++ b/src/app/MessageDef/EventPathIB.cpp
@@ -26,6 +26,8 @@
#include <app/AppConfig.h>
+#include <protocols/interaction_model/Constants.h>
+
namespace chip {
namespace app {
#if CHIP_CONFIG_IM_ENABLE_SCHEMA_CHECK
@@ -171,6 +173,50 @@ CHIP_ERROR EventPathIB::Parser::GetIsUrgent(bool * const apIsUrgent) const
return GetSimpleValue(to_underlying(Tag::kIsUrgent), TLV::kTLVType_Boolean, apIsUrgent);
}
+CHIP_ERROR EventPathIB::Parser::ParsePath(EventPathParams & aEvent) const
+{
+ CHIP_ERROR err = GetEndpoint(&(aEvent.mEndpointId));
+ if (err == CHIP_NO_ERROR)
+ {
+ VerifyOrReturnError(!aEvent.HasWildcardEndpointId(), CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ }
+ else if (err == CHIP_END_OF_TLV)
+ {
+ err = CHIP_NO_ERROR;
+ }
+ VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
+ err = GetCluster(&(aEvent.mClusterId));
+ if (err == CHIP_NO_ERROR)
+ {
+ VerifyOrReturnError(!aEvent.HasWildcardClusterId(), CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ }
+ else if (err == CHIP_END_OF_TLV)
+ {
+ err = CHIP_NO_ERROR;
+ }
+ VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
+ err = GetEvent(&(aEvent.mEventId));
+ if (CHIP_END_OF_TLV == err)
+ {
+ err = CHIP_NO_ERROR;
+ }
+ else if (err == CHIP_NO_ERROR)
+ {
+ VerifyOrReturnError(!aEvent.HasWildcardEventId(), CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ }
+ VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction));
+
+ err = GetIsUrgent(&(aEvent.mIsUrgentEvent));
+ if (CHIP_END_OF_TLV == err)
+ {
+ err = CHIP_NO_ERROR;
+ }
+ VerifyOrReturnError(err == CHIP_NO_ERROR, CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ return CHIP_NO_ERROR;
+}
+
EventPathIB::Builder & EventPathIB::Builder::Node(const NodeId aNode)
{
// skip if error has already been set
diff --git a/src/app/MessageDef/EventPathIB.h b/src/app/MessageDef/EventPathIB.h
index 32b7948dfc6d54..0ad2000938ac86 100644
--- a/src/app/MessageDef/EventPathIB.h
+++ b/src/app/MessageDef/EventPathIB.h
@@ -127,6 +127,14 @@ class Parser : public ListParser
* #CHIP_ERROR_IM_MALFORMED_EVENT_PATH_IB if the path from the reader is not a valid concrere event path.
*/
CHIP_ERROR GetEventPath(ConcreteEventPath * const apPath) const;
+
+ /**
+ * @brief Parse the event path into an EventPathParams object. As part of parsing,
+ * validity checks for each path item will be done as well.
+ *
+ * If any errors are encountered, an IM error of 'InvalidAction' will be returned.
+ */
+ CHIP_ERROR ParsePath(EventPathParams & aEvent) const;
};
class Builder : public ListBuilder
diff --git a/src/app/ReadHandler.cpp b/src/app/ReadHandler.cpp
index 913dcce726ca10..64bcbb1a7e516b 100644
--- a/src/app/ReadHandler.cpp
+++ b/src/app/ReadHandler.cpp
@@ -315,7 +315,7 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa
}
else if (err == CHIP_NO_ERROR)
{
- ReturnErrorOnFailure(ProcessAttributePathList(attributePathListParser));
+ ReturnErrorOnFailure(ProcessAttributePaths(attributePathListParser));
DataVersionFilterIBs::Parser dataVersionFilterListParser;
err = readRequestParser.GetDataVersionFilters(&dataVersionFilterListParser);
if (err == CHIP_END_OF_TLV)
@@ -363,75 +363,19 @@ CHIP_ERROR ReadHandler::ProcessReadRequest(System::PacketBufferHandle && aPayloa
return CHIP_NO_ERROR;
}
-CHIP_ERROR ReadHandler::ProcessAttributePathList(AttributePathIBs::Parser & aAttributePathListParser)
+CHIP_ERROR ReadHandler::ProcessAttributePaths(AttributePathIBs::Parser & aAttributePathListParser)
{
CHIP_ERROR err = CHIP_NO_ERROR;
TLV::TLVReader reader;
aAttributePathListParser.GetReader(&reader);
while (CHIP_NO_ERROR == (err = reader.Next()))
{
- VerifyOrExit(TLV::AnonymousTag() == reader.GetTag(), err = CHIP_ERROR_INVALID_TLV_TAG);
+ VerifyOrReturnError(TLV::AnonymousTag() == reader.GetTag(), CHIP_ERROR_INVALID_TLV_TAG);
AttributePathParams attribute;
AttributePathIB::Parser path;
- err = path.Init(reader);
- SuccessOrExit(err);
-
- err = path.GetEndpoint(&(attribute.mEndpointId));
- if (err == CHIP_NO_ERROR)
- {
- VerifyOrExit(!attribute.HasWildcardEndpointId(), err = CHIP_IM_GLOBAL_STATUS(InvalidAction));
- }
- else if (err == CHIP_END_OF_TLV)
- {
- err = CHIP_NO_ERROR;
- }
- SuccessOrExit(err);
-
- ClusterId clusterId = kInvalidClusterId;
- err = path.GetCluster(&clusterId);
- if (err == CHIP_NO_ERROR)
- {
- VerifyOrExit(IsValidClusterId(clusterId), err = CHIP_IM_GLOBAL_STATUS(InvalidAction));
- attribute.mClusterId = clusterId;
- }
- else if (err == CHIP_END_OF_TLV)
- {
- err = CHIP_NO_ERROR;
- }
- SuccessOrExit(err);
-
- AttributeId attributeId = kInvalidAttributeId;
- err = path.GetAttribute(&attributeId);
- if (CHIP_END_OF_TLV == err)
- {
- err = CHIP_NO_ERROR;
- }
- else if (err == CHIP_NO_ERROR)
- {
- VerifyOrExit(IsValidAttributeId(attributeId), err = CHIP_IM_GLOBAL_STATUS(InvalidAction));
- attribute.mAttributeId = attributeId;
- }
- SuccessOrExit(err);
-
- // A wildcard cluster requires that the attribute path either be
- // wildcard or a global attribute.
- VerifyOrExit(!attribute.HasWildcardClusterId() || attribute.HasWildcardAttributeId() ||
- IsGlobalAttribute(attribute.mAttributeId),
- err = CHIP_IM_GLOBAL_STATUS(InvalidAction));
-
- err = path.GetListIndex(&(attribute.mListIndex));
- if (CHIP_NO_ERROR == err)
- {
- VerifyOrExit(!attribute.HasWildcardAttributeId() && !attribute.HasWildcardListIndex(),
- err = CHIP_IM_GLOBAL_STATUS(InvalidAction));
- }
- else if (CHIP_END_OF_TLV == err)
- {
- err = CHIP_NO_ERROR;
- }
- SuccessOrExit(err);
- err = InteractionModelEngine::GetInstance()->PushFrontAttributePathList(mpAttributePathList, attribute);
- SuccessOrExit(err);
+ ReturnErrorOnFailure(path.Init(reader));
+ ReturnErrorOnFailure(path.ParsePath(attribute));
+ ReturnErrorOnFailure(InteractionModelEngine::GetInstance()->PushFrontAttributePathList(mpAttributePathList, attribute));
}
// if we have exhausted this container
if (CHIP_END_OF_TLV == err)
@@ -440,8 +384,6 @@ CHIP_ERROR ReadHandler::ProcessAttributePathList(AttributePathIBs::Parser & aAtt
mAttributePathExpandIterator = AttributePathExpandIterator(mpAttributePathList);
err = CHIP_NO_ERROR;
}
-
-exit:
return err;
}
@@ -487,47 +429,7 @@ CHIP_ERROR ReadHandler::ProcessEventPaths(EventPathIBs::Parser & aEventPathsPars
EventPathParams event;
EventPathIB::Parser path;
ReturnErrorOnFailure(path.Init(reader));
-
- err = path.GetEndpoint(&(event.mEndpointId));
- if (err == CHIP_NO_ERROR)
- {
- VerifyOrReturnError(!event.HasWildcardEndpointId(), err = CHIP_ERROR_IM_MALFORMED_EVENT_PATH_IB);
- }
- else if (err == CHIP_END_OF_TLV)
- {
- err = CHIP_NO_ERROR;
- }
- ReturnErrorOnFailure(err);
-
- err = path.GetCluster(&(event.mClusterId));
- if (err == CHIP_NO_ERROR)
- {
- VerifyOrReturnError(!event.HasWildcardClusterId(), err = CHIP_ERROR_IM_MALFORMED_EVENT_PATH_IB);
- }
- else if (err == CHIP_END_OF_TLV)
- {
- err = CHIP_NO_ERROR;
- }
- ReturnErrorOnFailure(err);
-
- err = path.GetEvent(&(event.mEventId));
- if (CHIP_END_OF_TLV == err)
- {
- err = CHIP_NO_ERROR;
- }
- else if (err == CHIP_NO_ERROR)
- {
- VerifyOrReturnError(!event.HasWildcardEventId(), err = CHIP_ERROR_IM_MALFORMED_EVENT_PATH_IB);
- }
- ReturnErrorOnFailure(err);
-
- err = path.GetIsUrgent(&(event.mIsUrgentEvent));
- if (CHIP_END_OF_TLV == err)
- {
- err = CHIP_NO_ERROR;
- }
- ReturnErrorOnFailure(err);
-
+ ReturnErrorOnFailure(path.ParsePath(event));
ReturnErrorOnFailure(InteractionModelEngine::GetInstance()->PushFrontEventPathParamsList(mpEventPathList, event));
}
@@ -667,7 +569,7 @@ CHIP_ERROR ReadHandler::ProcessSubscribeRequest(System::PacketBufferHandle && aP
}
else if (err == CHIP_NO_ERROR)
{
- ReturnErrorOnFailure(ProcessAttributePathList(attributePathListParser));
+ ReturnErrorOnFailure(ProcessAttributePaths(attributePathListParser));
DataVersionFilterIBs::Parser dataVersionFilterListParser;
err = subscribeRequestParser.GetDataVersionFilters(&dataVersionFilterListParser);
if (err == CHIP_END_OF_TLV)
diff --git a/src/app/ReadHandler.h b/src/app/ReadHandler.h
index b07ef01b2c956a..7e32fa920a2636 100644
--- a/src/app/ReadHandler.h
+++ b/src/app/ReadHandler.h
@@ -362,7 +362,7 @@ class ReadHandler : public Messaging::ExchangeDelegate
CHIP_ERROR SendSubscribeResponse();
CHIP_ERROR ProcessSubscribeRequest(System::PacketBufferHandle && aPayload);
CHIP_ERROR ProcessReadRequest(System::PacketBufferHandle && aPayload);
- CHIP_ERROR ProcessAttributePathList(AttributePathIBs::Parser & aAttributePathListParser);
+ CHIP_ERROR ProcessAttributePaths(AttributePathIBs::Parser & aAttributePathListParser);
CHIP_ERROR ProcessEventPaths(EventPathIBs::Parser & aEventPathsParser);
CHIP_ERROR ProcessEventFilters(EventFilterIBs::Parser & aEventFiltersParser);
CHIP_ERROR OnStatusResponse(Messaging::ExchangeContext * apExchangeContext, System::PacketBufferHandle && aPayload,
diff --git a/src/app/tests/BUILD.gn b/src/app/tests/BUILD.gn
index 7d57e72ebb8667..d11f29a66c72c2 100644
--- a/src/app/tests/BUILD.gn
+++ b/src/app/tests/BUILD.gn
@@ -102,6 +102,8 @@ chip_test_suite("tests") {
test_sources += [ "TestFailSafeContext.cpp" ]
}
+ test_sources += [ "TestAclAttribute.cpp" ]
+
#
# On NRF platforms, the allocation of a large number of pbufs in this test
# to exercise chunking causes it to run out of memory. For now, disable it there.
diff --git a/src/app/tests/TestAclAttribute.cpp b/src/app/tests/TestAclAttribute.cpp
new file mode 100644
index 00000000000000..ca51e96401ebdc
--- /dev/null
+++ b/src/app/tests/TestAclAttribute.cpp
@@ -0,0 +1,283 @@
+/*
+ *
+ * Copyright (c) 2021 Project CHIP Authors
+ * All rights reserved.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "lib/support/CHIPMem.h"
+#include <access/examples/PermissiveAccessControlDelegate.h>
+#include <app/AttributeAccessInterface.h>
+#include <app/InteractionModelEngine.h>
+#include <app/MessageDef/AttributeReportIBs.h>
+#include <app/MessageDef/EventDataIB.h>
+#include <app/tests/AppTestContext.h>
+#include <app/util/basic-types.h>
+#include <app/util/mock/Constants.h>
+#include <app/util/mock/Functions.h>
+#include <lib/core/CHIPCore.h>
+#include <lib/core/CHIPTLV.h>
+#include <lib/core/CHIPTLVDebug.hpp>
+#include <lib/core/CHIPTLVUtilities.hpp>
+#include <lib/support/CHIPCounter.h>
+#include <lib/support/ErrorStr.h>
+#include <lib/support/UnitTestContext.h>
+#include <lib/support/UnitTestRegistration.h>
+#include <messaging/ExchangeContext.h>
+#include <messaging/Flags.h>
+#include <nlunit-test.h>
+#include <protocols/interaction_model/Constants.h>
+
+#include <type_traits>
+
+namespace {
+using namespace chip;
+using namespace chip::Access;
+
+chip::ClusterId kTestClusterId = 1;
+chip::ClusterId kTestDeniedClusterId1 = 1000;
+chip::ClusterId kTestDeniedClusterId2 = 3;
+chip::EndpointId kTestEndpointId = 4;
+
+class TestAccessControlDelegate : public AccessControl::Delegate
+{
+public:
+ CHIP_ERROR Check(const SubjectDescriptor & subjectDescriptor, const chip::Access::RequestPath & requestPath,
+ Privilege requestPrivilege) override
+ {
+ if (requestPath.cluster == kTestDeniedClusterId2)
+ {
+ return CHIP_ERROR_ACCESS_DENIED;
+ }
+ return CHIP_NO_ERROR;
+ }
+};
+
+AccessControl::Delegate * GetTestAccessControlDelegate()
+{
+ static TestAccessControlDelegate accessControlDelegate;
+ return &accessControlDelegate;
+}
+
+class TestDeviceTypeResolver : public AccessControl::DeviceTypeResolver
+{
+public:
+ bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint) override { return false; }
+} gDeviceTypeResolver;
+
+class TestAccessContext : public chip::Test::AppContext
+{
+public:
+ static int Initialize(void * context)
+ {
+ if (AppContext::Initialize(context) != SUCCESS)
+ return FAILURE;
+ Access::GetAccessControl().Finish();
+ Access::GetAccessControl().Init(GetTestAccessControlDelegate(), gDeviceTypeResolver);
+ return SUCCESS;
+ }
+
+ static int Finalize(void * context)
+ {
+ if (AppContext::Finalize(context) != SUCCESS)
+ return FAILURE;
+ return SUCCESS;
+ }
+
+private:
+ chip::MonotonicallyIncreasingCounter<chip::EventNumber> mEventCounter;
+};
+
+class MockInteractionModelApp : public chip::app::ReadClient::Callback
+{
+public:
+ void OnAttributeData(const chip::app::ConcreteDataAttributePath & aPath, chip::TLV::TLVReader * apData,
+ const chip::app::StatusIB & status) override
+ {
+ mGotReport = true;
+ mLastStatusReceived = status;
+ }
+
+ void OnError(CHIP_ERROR aError) override { mError = aError; }
+
+ void OnDone(chip::app::ReadClient *) override {}
+
+ void OnDeallocatePaths(chip::app::ReadPrepareParams && aReadPrepareParams) override
+ {
+ if (aReadPrepareParams.mpAttributePathParamsList != nullptr)
+ {
+ delete[] aReadPrepareParams.mpAttributePathParamsList;
+ }
+
+ if (aReadPrepareParams.mpDataVersionFilterList != nullptr)
+ {
+ delete[] aReadPrepareParams.mpDataVersionFilterList;
+ }
+ }
+
+ bool mGotReport = false;
+ chip::app::StatusIB mLastStatusReceived;
+ CHIP_ERROR mError = CHIP_NO_ERROR;
+};
+} // namespace
+
+namespace chip {
+namespace app {
+
+bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath)
+{
+ return aPath.mClusterId != kTestDeniedClusterId1;
+}
+
+class TestAclAttribute
+{
+public:
+ static void TestACLDeniedAttribute(nlTestSuite * apSuite, void * apContext);
+};
+
+// Read Client sends a malformed subscribe request, interaction model engine fails to parse the request and generates a status
+// report to client, and client is closed.
+void TestAclAttribute::TestACLDeniedAttribute(nlTestSuite * apSuite, void * apContext)
+{
+ TestAccessContext & ctx = *static_cast<TestAccessContext *>(apContext);
+ CHIP_ERROR err = CHIP_NO_ERROR;
+
+ Messaging::ReliableMessageMgr * rm = ctx.GetExchangeManager().GetReliableMessageMgr();
+ NL_TEST_ASSERT(apSuite, rm->TestGetCountRetransTable() == 0);
+
+ MockInteractionModelApp delegate;
+ auto * engine = chip::app::InteractionModelEngine::GetInstance();
+ err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable());
+ NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+
+ {
+ app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate,
+ chip::app::ReadClient::InteractionType::Subscribe);
+
+ chip::app::AttributePathParams attributePathParams[2];
+ attributePathParams[0].mEndpointId = kTestEndpointId;
+ attributePathParams[0].mClusterId = kTestDeniedClusterId1;
+ attributePathParams[0].mAttributeId = 1;
+
+ attributePathParams[1].mEndpointId = kTestEndpointId;
+ attributePathParams[1].mClusterId = kTestDeniedClusterId1;
+ attributePathParams[1].mAttributeId = 2;
+
+ ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());
+ readPrepareParams.mpAttributePathParamsList = attributePathParams;
+ readPrepareParams.mAttributePathParamsListSize = 2;
+
+ err = readClient.SendRequest(readPrepareParams);
+ NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+
+ ctx.DrainAndServiceIO();
+ NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ NL_TEST_ASSERT(apSuite, !delegate.mGotReport);
+ delegate.mError = CHIP_NO_ERROR;
+ delegate.mGotReport = false;
+ }
+
+ {
+ app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate,
+ chip::app::ReadClient::InteractionType::Subscribe);
+
+ chip::app::AttributePathParams attributePathParams[2];
+
+ attributePathParams[0].mClusterId = kTestDeniedClusterId2;
+ attributePathParams[0].mAttributeId = 1;
+
+ attributePathParams[1].mClusterId = kTestDeniedClusterId2;
+ attributePathParams[1].mAttributeId = 2;
+
+ ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());
+ readPrepareParams.mpAttributePathParamsList = attributePathParams;
+ readPrepareParams.mAttributePathParamsListSize = 2;
+
+ err = readClient.SendRequest(readPrepareParams);
+ NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+
+ ctx.DrainAndServiceIO();
+ NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ NL_TEST_ASSERT(apSuite, !delegate.mGotReport);
+ delegate.mError = CHIP_NO_ERROR;
+ delegate.mGotReport = false;
+ }
+
+ {
+ app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate,
+ chip::app::ReadClient::InteractionType::Subscribe);
+
+ chip::app::AttributePathParams attributePathParams[2];
+ attributePathParams[0].mEndpointId = kTestEndpointId;
+ attributePathParams[0].mClusterId = kTestDeniedClusterId1;
+ attributePathParams[0].mAttributeId = 1;
+
+ attributePathParams[1].mEndpointId = kTestEndpointId;
+ attributePathParams[1].mClusterId = kTestClusterId;
+ attributePathParams[1].mAttributeId = 2;
+
+ ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());
+ readPrepareParams.mpAttributePathParamsList = attributePathParams;
+ readPrepareParams.mAttributePathParamsListSize = 2;
+
+ err = readClient.SendRequest(readPrepareParams);
+ NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+
+ ctx.DrainAndServiceIO();
+ NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_NO_ERROR);
+ NL_TEST_ASSERT(apSuite, delegate.mGotReport);
+ NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadHandlers(ReadHandler::InteractionType::Subscribe) == 1);
+ delegate.mError = CHIP_NO_ERROR;
+ delegate.mGotReport = false;
+ }
+
+ NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0);
+ engine->Shutdown();
+ NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);
+}
+} // namespace app
+} // namespace chip
+
+namespace {
+
+/**
+ * Test Suite. It lists all the test functions.
+ */
+
+// clang-format off
+const nlTest sTests[] =
+{
+ NL_TEST_DEF("TestACLDeniedAttribute", chip::app::TestAclAttribute::TestACLDeniedAttribute),
+ NL_TEST_SENTINEL()
+};
+// clang-format on
+
+// clang-format off
+nlTestSuite sSuite =
+{
+ "TestAclAttribute",
+ &sTests[0],
+ TestAccessContext::Initialize,
+ TestAccessContext::Finalize
+};
+// clang-format on
+
+} // namespace
+
+int TestAclAttribute()
+{
+ return chip::ExecuteTestsWithContext<TestAccessContext>(&sSuite);
+}
+
+CHIP_REGISTER_TEST_SUITE(TestAclAttribute)
diff --git a/src/app/tests/TestAclEvent.cpp b/src/app/tests/TestAclEvent.cpp
index b67ca621c7e44f..1238e7c7142ee3 100644
--- a/src/app/tests/TestAclEvent.cpp
+++ b/src/app/tests/TestAclEvent.cpp
@@ -16,12 +16,6 @@
* limitations under the License.
*/
-/**
- * @file
- * This file implements unit tests for CHIP Interaction Model Read Interaction
- *
- */
-
#include "lib/support/CHIPMem.h"
#include <access/AccessControl.h>
#include <app/AttributeAccessInterface.h>
@@ -55,7 +49,7 @@ uint8_t gDebugEventBuffer[128];
uint8_t gInfoEventBuffer[128];
uint8_t gCritEventBuffer[128];
chip::app::CircularEventBuffer gCircularEventBuffer[3];
-chip::ClusterId kTestClusterId1 = 6;
+chip::ClusterId kTestClusterId1 = 100;
chip::ClusterId kTestClusterId2 = 7;
chip::EndpointId kTestEndpointId = 1;
chip::EventId kTestEventIdDebug = 1;
diff --git a/src/app/tests/TestReadInteraction.cpp b/src/app/tests/TestReadInteraction.cpp
index e0527b9e4c66ff..44ba49c400a4b9 100644
--- a/src/app/tests/TestReadInteraction.cpp
+++ b/src/app/tests/TestReadInteraction.cpp
@@ -338,6 +338,7 @@ class TestReadInteraction
static void TestReadHandlerInvalidSubscribeRequest(nlTestSuite * apSuite, void * apContext);
static void TestSubscribeInvalidateFabric(nlTestSuite * apSuite, void * apContext);
static void TestShutdownSubscription(nlTestSuite * apSuite, void * apContext);
+ static void TestReadHandlerMalformedSubscribeRequest(nlTestSuite * apSuite, void * apContext);
private:
static void GenerateReportData(nlTestSuite * apSuite, void * apContext, System::PacketBufferHandle & aPayload,
@@ -3521,6 +3522,51 @@ void TestReadInteraction::TestReadChunkingInvalidSubscriptionId(nlTestSuite * ap
ctx.CreateSessionBobToAlice();
}
+// Read Client sends a malformed subscribe request, interaction model engine fails to parse the request and generates a status
+// report to client, and client is closed.
+void TestReadInteraction::TestReadHandlerMalformedSubscribeRequest(nlTestSuite * apSuite, void * apContext)
+{
+ TestContext & ctx = *static_cast<TestContext *>(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);
+
+ MockInteractionModelApp delegate;
+ auto * engine = chip::app::InteractionModelEngine::GetInstance();
+ err = engine->Init(&ctx.GetExchangeManager(), &ctx.GetFabricTable());
+ NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+
+ ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());
+
+ {
+ app::ReadClient readClient(chip::app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), delegate,
+ chip::app::ReadClient::InteractionType::Subscribe);
+ System::PacketBufferHandle msgBuf;
+ ReadRequestMessage::Builder request;
+ System::PacketBufferTLVWriter writer;
+
+ chip::app::InitWriterWithSpaceReserved(writer, 0);
+ err = request.Init(&writer);
+ NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+ err = writer.Finalize(&msgBuf);
+ NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+ auto exchange = readClient.mpExchangeMgr->NewContext(readPrepareParams.mSessionHolder.Get().Value(), &readClient);
+ NL_TEST_ASSERT(apSuite, exchange != nullptr);
+ readClient.mExchange.Grab(exchange);
+ readClient.MoveToState(app::ReadClient::ClientState::AwaitingInitialReport);
+ err = readClient.mExchange->SendMessage(Protocols::InteractionModel::MsgType::ReadRequest, std::move(msgBuf),
+ Messaging::SendFlags(Messaging::SendMessageFlags::kExpectResponse));
+ NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+ ctx.DrainAndServiceIO();
+ NL_TEST_ASSERT(apSuite, delegate.mError == CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ }
+ NL_TEST_ASSERT(apSuite, engine->GetNumActiveReadClients() == 0);
+ engine->Shutdown();
+ NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);
+}
+
// Read Client sends a malformed read request, interaction model engine fails to parse the request and generates a status report to
// client, and client is closed.
void TestReadInteraction::TestReadHandlerMalformedReadRequest1(nlTestSuite * apSuite, void * apContext)
@@ -3953,6 +3999,7 @@ const nlTest sTests[] =
NL_TEST_DEF("TestReadChunkingInvalidSubscriptionId", chip::app::TestReadInteraction::TestReadChunkingInvalidSubscriptionId),
NL_TEST_DEF("TestReadHandlerMalformedReadRequest1", chip::app::TestReadInteraction::TestReadHandlerMalformedReadRequest1),
NL_TEST_DEF("TestReadHandlerMalformedReadRequest2", chip::app::TestReadInteraction::TestReadHandlerMalformedReadRequest2),
+ NL_TEST_DEF("TestReadHandlerMalformedSubscribeRequest", chip::app::TestReadInteraction::TestReadHandlerMalformedSubscribeRequest),
NL_TEST_DEF("TestSubscribeSendUnknownMessage", chip::app::TestReadInteraction::TestSubscribeSendUnknownMessage),
NL_TEST_DEF("TestSubscribeSendInvalidStatusReport", chip::app::TestReadInteraction::TestSubscribeSendInvalidStatusReport),
NL_TEST_DEF("TestReadHandlerInvalidSubscribeRequest", chip::app::TestReadInteraction::TestReadHandlerInvalidSubscribeRequest),
diff --git a/src/app/tests/integration/chip_im_initiator.cpp b/src/app/tests/integration/chip_im_initiator.cpp
index 4636e14bc3fc0f..5457bbb3fd79ef 100644
--- a/src/app/tests/integration/chip_im_initiator.cpp
+++ b/src/app/tests/integration/chip_im_initiator.cpp
@@ -654,6 +654,11 @@ const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePat
return &stub;
}
+bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath)
+{
+ return true;
+}
+
CHIP_ERROR WriteSingleClusterData(const Access::SubjectDescriptor & aSubjectDescriptor, const ConcreteDataAttributePath & aPath,
TLV::TLVReader & aReader, WriteHandler *)
{
diff --git a/src/app/tests/integration/chip_im_responder.cpp b/src/app/tests/integration/chip_im_responder.cpp
index 9952256c4d479e..34978611e4f3bf 100644
--- a/src/app/tests/integration/chip_im_responder.cpp
+++ b/src/app/tests/integration/chip_im_responder.cpp
@@ -122,6 +122,11 @@ CHIP_ERROR ReadSingleClusterData(const Access::SubjectDescriptor & aSubjectDescr
return CHIP_NO_ERROR;
}
+bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath)
+{
+ return true;
+}
+
const EmberAfAttributeMetadata * GetAttributeMetadata(const ConcreteAttributePath & aConcreteClusterPath)
{
// Note: This test does not make use of the real attribute metadata.
diff --git a/src/app/util/ember-compatibility-functions.cpp b/src/app/util/ember-compatibility-functions.cpp
index d90c97a50b6f8e..3ac3b3258c41bc 100644
--- a/src/app/util/ember-compatibility-functions.cpp
+++ b/src/app/util/ember-compatibility-functions.cpp
@@ -516,6 +516,18 @@ Protocols::InteractionModel::Status UnsupportedAttributeStatus(const ConcreteAtt
} // anonymous namespace
+bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath)
+{
+ for (auto & attr : GlobalAttributesNotInMetadata)
+ {
+ if (attr == aPath.mAttributeId)
+ {
+ return (emberAfFindCluster(aPath.mEndpointId, aPath.mClusterId, CLUSTER_MASK_SERVER) != nullptr);
+ }
+ }
+ return (emberAfLocateAttributeMetadata(aPath.mEndpointId, aPath.mClusterId, aPath.mAttributeId) != nullptr);
+}
+
CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, bool aIsFabricFiltered,
const ConcreteReadAttributePath & aPath, AttributeReportIBs::Builder & aAttributeReports,
AttributeValueEncoder::AttributeEncodeState * apEncoderState)
diff --git a/src/controller/tests/TestReadChunking.cpp b/src/controller/tests/TestReadChunking.cpp
index 5dc6006edc09ba..1e1f0c41fd6e7d 100644
--- a/src/controller/tests/TestReadChunking.cpp
+++ b/src/controller/tests/TestReadChunking.cpp
@@ -68,10 +68,10 @@ constexpr AttributeId kTestListAttribute = 6;
constexpr AttributeId kTestBadAttribute =
7; // Reading this attribute will return CHIP_ERROR_NO_MEMORY but nothing is actually encoded.
-class TestCommandInteraction
+class TestReadChunking
{
public:
- TestCommandInteraction() {}
+ TestReadChunking() {}
static void TestChunking(nlTestSuite * apSuite, void * apContext);
static void TestListChunking(nlTestSuite * apSuite, void * apContext);
static void TestBadChunking(nlTestSuite * apSuite, void * apContext);
@@ -359,7 +359,7 @@ void TestMutableReadCallback::OnAttributeData(const app::ConcreteDataAttributePa
* as we can possibly cover.
*
*/
-void TestCommandInteraction::TestChunking(nlTestSuite * apSuite, void * apContext)
+void TestReadChunking::TestChunking(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();
@@ -424,7 +424,7 @@ void TestCommandInteraction::TestChunking(nlTestSuite * apSuite, void * apContex
}
// Similar to the test above, but for the list chunking feature.
-void TestCommandInteraction::TestListChunking(nlTestSuite * apSuite, void * apContext)
+void TestReadChunking::TestListChunking(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();
@@ -488,7 +488,7 @@ void TestCommandInteraction::TestListChunking(nlTestSuite * apSuite, void * apCo
}
// Read an attribute that can never fit into the buffer. Result in an empty report, server should shutdown the transaction.
-void TestCommandInteraction::TestBadChunking(nlTestSuite * apSuite, void * apContext)
+void TestReadChunking::TestBadChunking(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();
@@ -540,7 +540,7 @@ void TestCommandInteraction::TestBadChunking(nlTestSuite * apSuite, void * apCon
/*
* This test contains two parts, one is to enable a new endpoint on the fly, another is to disable it and re-enable it.
*/
-void TestCommandInteraction::TestDynamicEndpoint(nlTestSuite * apSuite, void * apContext)
+void TestReadChunking::TestDynamicEndpoint(nlTestSuite * apSuite, void * apContext)
{
TestContext & ctx = *static_cast<TestContext *>(apContext);
auto sessionHandle = ctx.GetSessionBobToAlice();
@@ -565,16 +565,16 @@ void TestCommandInteraction::TestDynamicEndpoint(nlTestSuite * apSuite, void * a
app::ReadClient readClient(engine, &ctx.GetExchangeManager(), readCallback.mBufferedCallback,
app::ReadClient::InteractionType::Subscribe);
+ // Enable the new endpoint
+ emberAfSetDynamicEndpoint(0, kTestEndpointId, &testEndpoint, Span<DataVersion>(dataVersionStorage));
NL_TEST_ASSERT(apSuite, readClient.SendRequest(readParams) == CHIP_NO_ERROR);
ctx.DrainAndServiceIO();
- // We should not receive any reports in initial reports, so check mOnSubscriptionEstablished instead.
NL_TEST_ASSERT(apSuite, readCallback.mOnSubscriptionEstablished);
readCallback.mAttributeCount = 0;
- // Enable the new endpoint
emberAfSetDynamicEndpoint(0, kTestEndpointId4, &testEndpoint4, Span<DataVersion>(dataVersionStorage));
ctx.DrainAndServiceIO();
@@ -623,7 +623,6 @@ void TestCommandInteraction::TestDynamicEndpoint(nlTestSuite * apSuite, void * a
emberAfClearDynamicEndpoint(0);
}
-
/*
* The tests below are for testing deatiled bwhavior when the attributes are modified between two chunks. In this test, we only care
* above whether we will receive correct attribute values in reasonable messages with reduced reporting traffic.
@@ -737,7 +736,7 @@ void DoTest(TestMutableReadCallback * callback, Instruction instruction)
}; // namespace TestSetDirtyBetweenChunksUtil
-void TestCommandInteraction::TestSetDirtyBetweenChunks(nlTestSuite * apSuite, void * apContext)
+void TestReadChunking::TestSetDirtyBetweenChunks(nlTestSuite * apSuite, void * apContext)
{
using namespace TestSetDirtyBetweenChunksUtil;
TestContext & ctx = *static_cast<TestContext *>(apContext);
@@ -895,11 +894,11 @@ void TestCommandInteraction::TestSetDirtyBetweenChunks(nlTestSuite * apSuite, vo
// clang-format off
const nlTest sTests[] =
{
- NL_TEST_DEF("TestChunking", TestCommandInteraction::TestChunking),
- NL_TEST_DEF("TestListChunking", TestCommandInteraction::TestListChunking),
- NL_TEST_DEF("TestBadChunking", TestCommandInteraction::TestBadChunking),
- NL_TEST_DEF("TestDynamicEndpoint", TestCommandInteraction::TestDynamicEndpoint),
- NL_TEST_DEF("TestSetDirtyBetweenChunks", TestCommandInteraction::TestSetDirtyBetweenChunks),
+ NL_TEST_DEF("TestChunking", TestReadChunking::TestChunking),
+ NL_TEST_DEF("TestListChunking", TestReadChunking::TestListChunking),
+ NL_TEST_DEF("TestBadChunking", TestReadChunking::TestBadChunking),
+ NL_TEST_DEF("TestDynamicEndpoint", TestReadChunking::TestDynamicEndpoint),
+ NL_TEST_DEF("TestSetDirtyBetweenChunks", TestReadChunking::TestSetDirtyBetweenChunks),
NL_TEST_SENTINEL()
};
diff --git a/src/controller/tests/data_model/TestRead.cpp b/src/controller/tests/data_model/TestRead.cpp
index 15b2ee6c1f78bf..ff4359659f1f47 100644
--- a/src/controller/tests/data_model/TestRead.cpp
+++ b/src/controller/tests/data_model/TestRead.cpp
@@ -232,6 +232,12 @@ bool IsDeviceTypeOnEndpoint(DeviceTypeId deviceType, EndpointId endpoint)
{
return false;
}
+
+bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath)
+{
+ return true;
+}
+
} // namespace app
} // namespace chip
@@ -275,6 +281,7 @@ class TestReadInteraction : public app::ReadHandler::ApplicationCallback
static void TestReadAttribute_ManyDataValues(nlTestSuite * apSuite, void * apContext);
static void TestReadAttribute_ManyDataValuesWrongPath(nlTestSuite * apSuite, void * apContext);
static void TestReadAttribute_ManyErrors(nlTestSuite * apSuite, void * apContext);
+ static void TestSubscribeAttributeDeniedNotExistPath(nlTestSuite * apSuite, void * apContext);
private:
static uint16_t mMaxInterval;
@@ -1550,9 +1557,9 @@ void TestReadInteraction::TestResubscribeAttributeTimeout(nlTestSuite * apSuite,
app::AttributePathParams attributePathParams[1];
readPrepareParams.mpAttributePathParamsList = attributePathParams;
readPrepareParams.mAttributePathParamsListSize = ArraySize(attributePathParams);
-
- attributePathParams[0].mClusterId = app::Clusters::TestCluster::Id;
- attributePathParams[0].mAttributeId = app::Clusters::TestCluster::Attributes::Boolean::Id;
+ attributePathParams[0].mEndpointId = kTestEndpointId;
+ attributePathParams[0].mClusterId = app::Clusters::TestCluster::Id;
+ attributePathParams[0].mAttributeId = app::Clusters::TestCluster::Attributes::Boolean::Id;
readPrepareParams.mMaxIntervalCeilingSeconds = 1;
@@ -1626,6 +1633,7 @@ void TestReadInteraction::TestSubscribeAttributeTimeout(nlTestSuite * apSuite, v
app::AttributePathParams attributePathParams[1];
readPrepareParams.mpAttributePathParamsList = attributePathParams;
readPrepareParams.mAttributePathParamsListSize = ArraySize(attributePathParams);
+ attributePathParams[0].mEndpointId = kTestEndpointId;
attributePathParams[0].mClusterId = app::Clusters::TestCluster::Id;
attributePathParams[0].mAttributeId = app::Clusters::TestCluster::Attributes::Boolean::Id;
@@ -2885,6 +2893,47 @@ void EstablishReadOrSubscriptions(nlTestSuite * apSuite, const SessionHandle & s
} // namespace SubscriptionPathQuotaHelpers
+void TestReadInteraction::TestSubscribeAttributeDeniedNotExistPath(nlTestSuite * apSuite, void * apContext)
+{
+ TestContext & ctx = *static_cast<TestContext *>(apContext);
+ auto sessionHandle = ctx.GetSessionBobToAlice();
+
+ ctx.SetMRPMode(Test::MessagingContext::MRPMode::kResponsive);
+
+ {
+ SubscriptionPathQuotaHelpers::TestReadCallback callback;
+ app::ReadClient readClient(app::InteractionModelEngine::GetInstance(), &ctx.GetExchangeManager(), callback,
+ app::ReadClient::InteractionType::Subscribe);
+
+ app::ReadPrepareParams readPrepareParams(ctx.GetSessionBobToAlice());
+
+ app::AttributePathParams attributePathParams[1];
+ readPrepareParams.mpAttributePathParamsList = attributePathParams;
+ readPrepareParams.mAttributePathParamsListSize = ArraySize(attributePathParams);
+ attributePathParams[0].mClusterId = app::Clusters::TestCluster::Id;
+ attributePathParams[0].mAttributeId = app::Clusters::TestCluster::Attributes::ListStructOctetString::Id;
+
+ //
+ // Request a max interval that's very small to reduce time to discovering a liveness failure.
+ //
+ readPrepareParams.mMaxIntervalCeilingSeconds = 1;
+
+ auto err = readClient.SendRequest(readPrepareParams);
+ NL_TEST_ASSERT(apSuite, err == CHIP_NO_ERROR);
+
+ ctx.DrainAndServiceIO();
+
+ NL_TEST_ASSERT(apSuite, callback.mOnError == 1);
+ NL_TEST_ASSERT(apSuite, callback.mLastError == CHIP_IM_GLOBAL_STATUS(InvalidAction));
+ NL_TEST_ASSERT(apSuite, callback.mOnDone == 1);
+ }
+
+ ctx.SetMRPMode(Test::MessagingContext::MRPMode::kDefault);
+
+ app::InteractionModelEngine::GetInstance()->ShutdownActiveReads();
+ NL_TEST_ASSERT(apSuite, ctx.GetExchangeManager().GetNumActiveExchanges() == 0);
+}
+
void TestReadInteraction::TestReadHandler_KillOverQuotaSubscriptions(nlTestSuite * apSuite, void * apContext)
{
// Note: We cannot use ctx.DrainAndServiceIO() since the perpetual read will make DrainAndServiceIO never return.
@@ -4436,6 +4485,7 @@ const nlTest sTests[] =
NL_TEST_DEF("TestReadAttribute_ManyDataValues", TestReadInteraction::TestReadAttribute_ManyDataValues),
NL_TEST_DEF("TestReadAttribute_ManyDataValuesWrongPath", TestReadInteraction::TestReadAttribute_ManyDataValuesWrongPath),
NL_TEST_DEF("TestReadAttribute_ManyErrors", TestReadInteraction::TestReadAttribute_ManyErrors),
+ NL_TEST_DEF("TestSubscribeAttributeDeniedNotExistPath", TestReadInteraction::TestSubscribeAttributeDeniedNotExistPath),
NL_TEST_DEF("TestResubscribeAttributeTimeout", TestReadInteraction::TestResubscribeAttributeTimeout),
NL_TEST_DEF("TestSubscribeAttributeTimeout", TestReadInteraction::TestSubscribeAttributeTimeout),
NL_TEST_SENTINEL()
diff --git a/src/darwin/Framework/CHIP/MTRIMDispatch.mm b/src/darwin/Framework/CHIP/MTRIMDispatch.mm
index c8b1df9f980a21..7979c34a832329 100644
--- a/src/darwin/Framework/CHIP/MTRIMDispatch.mm
+++ b/src/darwin/Framework/CHIP/MTRIMDispatch.mm
@@ -121,6 +121,11 @@ CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, b
return aAttributeReports.EncodeAttributeStatus(aPath, StatusIB(status));
}
+ bool ConcreteAttributePathExists(const ConcreteAttributePath & aPath)
+ {
+ return DetermineAttributeStatus(aPath, /* aIsWrite = */ false) == Status::UnsupportedAccess;
+ }
+
Status ServerClusterCommandExists(const ConcreteCommandPath & aPath)
{
// TODO: Consider making this configurable for applications that are not
From 8016e0fdf6bd977c5a1e3cd337797ccac206e49b Mon Sep 17 00:00:00 2001
From: Boris Zbarsky <bzbarsky@apple.com>
Date: Thu, 8 Sep 2022 12:35:10 -0400
Subject: [PATCH 2/2] Fix Darwin subscription test to follow the new failure
semantics.
---
src/darwin/Framework/CHIP/MTRBaseDevice.mm | 8 +++++++-
src/darwin/Framework/CHIPTests/MTRDeviceTests.m | 10 +++++++---
2 files changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/darwin/Framework/CHIP/MTRBaseDevice.mm b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
index 631291b9963fec..d5ffc41f529127 100644
--- a/src/darwin/Framework/CHIP/MTRBaseDevice.mm
+++ b/src/darwin/Framework/CHIP/MTRBaseDevice.mm
@@ -1218,7 +1218,11 @@ - (void)subscribeAttributeWithEndpointId:(NSNumber * _Nullable)endpointId
auto readClient = Platform::New<app::ReadClient>(
engine, exchangeManager, callback->GetBufferedCallback(), chip::app::ReadClient::InteractionType::Subscribe);
- err = readClient->SendAutoResubscribeRequest(std::move(readParams));
+ if (params != nil && params.autoResubscribe != nil && ![params.autoResubscribe boolValue]) {
+ err = readClient->SendRequest(readParams);
+ } else {
+ err = readClient->SendAutoResubscribeRequest(std::move(readParams));
+ }
if (err != CHIP_NO_ERROR) {
if (reportHandler) {
@@ -1227,6 +1231,8 @@ - (void)subscribeAttributeWithEndpointId:(NSNumber * _Nullable)endpointId
});
}
Platform::Delete(readClient);
+ Platform::Delete(container.pathParams);
+ container.pathParams = nullptr;
return;
}
diff --git a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
index 1e95514ecc1e34..bb6f81a6e12be8 100644
--- a/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
+++ b/src/darwin/Framework/CHIPTests/MTRDeviceTests.m
@@ -643,10 +643,12 @@ - (void)test009_SubscribeFailure
__block void (^reportHandler)(id _Nullable values, NSError * _Nullable error) = nil;
// Set up expectation for report
- XCTestExpectation * errorReportExpectation = [self expectationWithDescription:@"receive OnOff attribute report"];
+ XCTestExpectation * errorReportExpectation = [self expectationWithDescription:@"receive subscription error"];
reportHandler = ^(id _Nullable value, NSError * _Nullable error) {
+ // Because our subscription has no existent paths, it gets an
+ // InvalidAction response, which is EMBER_ZCL_STATUS_MALFORMED_COMMAND.
XCTAssertNil(value);
- XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], EMBER_ZCL_STATUS_UNSUPPORTED_ENDPOINT);
+ XCTAssertEqual([MTRErrorTestUtils errorToZCLErrorCode:error], EMBER_ZCL_STATUS_MALFORMED_COMMAND);
[errorReportExpectation fulfill];
};
@@ -662,12 +664,14 @@ - (void)test009_SubscribeFailure
}];
[self waitForExpectations:@[ cleanSubscriptionExpectation ] timeout:kTimeoutInSeconds];
+ __auto_type * params = [[MTRSubscribeParams alloc] init];
+ params.autoResubscribe = @(NO);
[device subscribeAttributeWithEndpointId:@10000
clusterId:@6
attributeId:@0
minInterval:@2
maxInterval:@10
- params:nil
+ params:params
clientQueue:queue
reportHandler:^(id _Nullable values, NSError * _Nullable error) {
NSLog(@"report attribute: OnOff values: %@, error: %@", values, error);