Skip to content

Commit 15532a3

Browse files
tleacmcsaaxelnxp
authored andcommitted
M-ACL: switch from UnsupportedAccess to AccessRestricted (project-chip#35263)
* Implement new AccessRestricted error code * [NXP][Zephyr] Provide AP band in connection request parameters (project-chip#35181) Signed-off-by: Axel Le Bourhis <axel.lebourhis@nxp.com> * added missed ERROR_CODES.md update * fixed build issue * restyled * fix return type in CodegenDataModelProvider_Write.cpp * fix review comments --------- Signed-off-by: Axel Le Bourhis <axel.lebourhis@nxp.com> Co-authored-by: Axel Le Bourhis <45206070+axelnxp@users.noreply.github.com>
1 parent 1e6f531 commit 15532a3

File tree

18 files changed

+55
-51
lines changed

18 files changed

+55
-51
lines changed

docs/ERROR_CODES.md

+2
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ This file was **AUTOMATICALLY** generated by
118118
| 165 | 0xA5 | `CHIP_ERROR_ACCESS_DENIED` |
119119
| 166 | 0xA6 | `CHIP_ERROR_UNKNOWN_RESOURCE_ID` |
120120
| 167 | 0xA7 | `CHIP_ERROR_VERSION_MISMATCH` |
121+
| 168 | 0xA8 | `CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL` |
121122
| 171 | 0xAB | `CHIP_ERROR_EVENT_ID_FOUND` |
122123
| 172 | 0xAC | `CHIP_ERROR_INTERNAL` |
123124
| 173 | 0xAD | `CHIP_ERROR_OPEN_FAILED` |
@@ -252,6 +253,7 @@ This file was **AUTOMATICALLY** generated by
252253
| 1426 | 0x592 | `DATA_VERSION_MISMATCH` |
253254
| 1428 | 0x594 | `TIMEOUT` |
254255
| 1436 | 0x59C | `BUSY` |
256+
| 1437 | 0x59D | `ACCESS_RESTRICTED` |
255257
| 1475 | 0x5C3 | `UNSUPPORTED_CLUSTER` |
256258
| 1477 | 0x5C5 | `NO_UPSTREAM_SUBSCRIPTION` |
257259
| 1478 | 0x5C6 | `NEEDS_TIMED_INTERACTION` |

src/access/AccessControl.cpp

-5
Original file line numberDiff line numberDiff line change
@@ -538,12 +538,7 @@ CHIP_ERROR AccessControl::CheckARL(const SubjectDescriptor & subjectDescriptor,
538538
if (result != CHIP_NO_ERROR)
539539
{
540540
ChipLogProgress(DataManagement, "AccessControl: %s",
541-
#if 0
542-
// TODO(#35177): new error code coming when access check plumbing are fixed in callers
543541
(result == CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL) ? "denied (restricted)" : "denied (restriction error)");
544-
#else
545-
(result == CHIP_ERROR_ACCESS_DENIED) ? "denied (restricted)" : "denied (restriction error)");
546-
#endif
547542
return result;
548543
}
549544

src/access/AccessRestrictionProvider.cpp

-20
Original file line numberDiff line numberDiff line change
@@ -197,45 +197,25 @@ CHIP_ERROR AccessRestrictionProvider::DoCheck(const std::vector<Entry> & entries
197197
if (requestPath.requestType == RequestType::kAttributeReadRequest ||
198198
requestPath.requestType == RequestType::kAttributeWriteRequest)
199199
{
200-
#if 0
201-
// TODO(#35177): use new ARL error code when access checks are fixed
202200
return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL;
203-
#else
204-
return CHIP_ERROR_ACCESS_DENIED;
205-
#endif
206201
}
207202
break;
208203
case Type::kAttributeWriteForbidden:
209204
if (requestPath.requestType == RequestType::kAttributeWriteRequest)
210205
{
211-
#if 0
212-
// TODO(#35177): use new ARL error code when access checks are fixed
213206
return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL;
214-
#else
215-
return CHIP_ERROR_ACCESS_DENIED;
216-
#endif
217207
}
218208
break;
219209
case Type::kCommandForbidden:
220210
if (requestPath.requestType == RequestType::kCommandInvokeRequest)
221211
{
222-
#if 0
223-
// TODO(#35177): use new ARL error code when access checks are fixed
224212
return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL;
225-
#else
226-
return CHIP_ERROR_ACCESS_DENIED;
227-
#endif
228213
}
229214
break;
230215
case Type::kEventForbidden:
231216
if (requestPath.requestType == RequestType::kEventReadRequest)
232217
{
233-
#if 0
234-
// TODO(#35177): use new ARL error code when access checks are fixed
235218
return CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL;
236-
#else
237-
return CHIP_ERROR_ACCESS_DENIED;
238-
#endif
239219
}
240220
break;
241221
}

src/access/tests/TestAccessRestrictionProvider.cpp

+1-1
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ void RunChecks(const CheckData * checkData, size_t count)
174174
{
175175
for (size_t i = 0; i < count; i++)
176176
{
177-
CHIP_ERROR expectedResult = checkData[i].allow ? CHIP_NO_ERROR : CHIP_ERROR_ACCESS_DENIED;
177+
CHIP_ERROR expectedResult = checkData[i].allow ? CHIP_NO_ERROR : CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL;
178178
EXPECT_EQ(accessControl.Check(checkData[i].subjectDescriptor, checkData[i].requestPath, checkData[i].privilege),
179179
expectedResult);
180180
}

src/app/CommandHandlerImpl.cpp

+3-2
Original file line numberDiff line numberDiff line change
@@ -410,12 +410,13 @@ Status CommandHandlerImpl::ProcessCommandDataIB(CommandDataIB::Parser & aCommand
410410
err = Access::GetAccessControl().Check(subjectDescriptor, requestPath, requestPrivilege);
411411
if (err != CHIP_NO_ERROR)
412412
{
413-
if (err != CHIP_ERROR_ACCESS_DENIED)
413+
if ((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL))
414414
{
415415
return FallibleAddStatus(concretePath, Status::Failure) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
416416
}
417417
// TODO: when wildcard invokes are supported, handle them to discard rather than fail with status
418-
return FallibleAddStatus(concretePath, Status::UnsupportedAccess) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
418+
Status status = err == CHIP_ERROR_ACCESS_DENIED ? Status::UnsupportedAccess : Status::AccessRestricted;
419+
return FallibleAddStatus(concretePath, status) != CHIP_NO_ERROR ? Status::Failure : Status::Success;
419420
}
420421
}
421422

src/app/EventManagement.cpp

+3-1
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,9 @@ CHIP_ERROR EventManagement::CheckEventContext(EventLoadOutContext * eventLoadOut
563563
Access::GetAccessControl().Check(eventLoadOutContext->mSubjectDescriptor, requestPath, requestPrivilege);
564564
if (accessControlError != CHIP_NO_ERROR)
565565
{
566-
ReturnErrorCodeIf(accessControlError != CHIP_ERROR_ACCESS_DENIED, accessControlError);
566+
ReturnErrorCodeIf((accessControlError != CHIP_ERROR_ACCESS_DENIED) &&
567+
(accessControlError != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL),
568+
accessControlError);
567569
ret = CHIP_ERROR_UNEXPECTED_EVENT;
568570
}
569571

src/app/codegen-data-model-provider/CodegenDataModelProvider_Read.cpp

+5-3
Original file line numberDiff line numberDiff line change
@@ -281,15 +281,17 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::ReadAttribute(const Data
281281
RequiredPrivilege::ForReadAttribute(request.path));
282282
if (err != CHIP_NO_ERROR)
283283
{
284-
ReturnErrorCodeIf(err != CHIP_ERROR_ACCESS_DENIED, err);
284+
ReturnErrorCodeIf((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL), err);
285285

286286
// Implementation of 8.4.3.2 of the spec for path expansion
287287
if (request.path.mExpanded)
288288
{
289289
return CHIP_NO_ERROR;
290290
}
291-
// access denied has a specific code for IM
292-
return CHIP_IM_GLOBAL_STATUS(UnsupportedAccess);
291+
292+
// access denied and access restricted have specific codes for IM
293+
return err == CHIP_ERROR_ACCESS_DENIED ? CHIP_IM_GLOBAL_STATUS(UnsupportedAccess)
294+
: CHIP_IM_GLOBAL_STATUS(AccessRestricted);
293295
}
294296
}
295297

src/app/codegen-data-model-provider/CodegenDataModelProvider_Write.cpp

+2-2
Original file line numberDiff line numberDiff line change
@@ -287,10 +287,10 @@ DataModel::ActionReturnStatus CodegenDataModelProvider::WriteAttribute(const Dat
287287

288288
if (err != CHIP_NO_ERROR)
289289
{
290-
ReturnErrorCodeIf(err != CHIP_ERROR_ACCESS_DENIED, err);
290+
ReturnErrorCodeIf((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL), err);
291291

292292
// TODO: when wildcard/group writes are supported, handle them to discard rather than fail with status
293-
return Status::UnsupportedAccess;
293+
return err == CHIP_ERROR_ACCESS_DENIED ? Status::UnsupportedAccess : Status::AccessRestricted;
294294
}
295295
}
296296

src/app/reporting/Engine.cpp

+7-4
Original file line numberDiff line numberDiff line change
@@ -346,23 +346,26 @@ CHIP_ERROR Engine::CheckAccessDeniedEventPaths(TLV::TLVWriter & aWriter, bool &
346346
Access::Privilege requestPrivilege = RequiredPrivilege::ForReadEvent(path);
347347

348348
err = Access::GetAccessControl().Check(apReadHandler->GetSubjectDescriptor(), requestPath, requestPrivilege);
349-
if (err != CHIP_ERROR_ACCESS_DENIED)
349+
if ((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL))
350350
{
351351
ReturnErrorOnFailure(err);
352352
}
353353
else
354354
{
355355
TLV::TLVWriter checkpoint = aWriter;
356-
err = EventReportIB::ConstructEventStatusIB(aWriter, path, StatusIB(Status::UnsupportedAccess));
356+
err = EventReportIB::ConstructEventStatusIB(aWriter, path,
357+
err == CHIP_ERROR_ACCESS_DENIED ? StatusIB(Status::UnsupportedAccess)
358+
: StatusIB(Status::AccessRestricted));
359+
357360
if (err != CHIP_NO_ERROR)
358361
{
359362
aWriter = checkpoint;
360363
break;
361364
}
362365
aHasEncodedData = true;
363-
ChipLogDetail(InteractionModel, "Access to event (%u, " ChipLogFormatMEI ", " ChipLogFormatMEI ") denied by ACL",
366+
ChipLogDetail(InteractionModel, "Access to event (%u, " ChipLogFormatMEI ", " ChipLogFormatMEI ") denied by %s",
364367
current->mValue.mEndpointId, ChipLogValueMEI(current->mValue.mClusterId),
365-
ChipLogValueMEI(current->mValue.mEventId));
368+
ChipLogValueMEI(current->mValue.mEventId), err == CHIP_ERROR_ACCESS_DENIED ? "ACL" : "ARL");
366369
}
367370
current = current->mpNext;
368371
}

src/app/util/ember-compatibility-functions.cpp

+8-4
Original file line numberDiff line numberDiff line change
@@ -302,12 +302,13 @@ CHIP_ERROR ReadSingleClusterData(const SubjectDescriptor & aSubjectDescriptor, b
302302
CHIP_ERROR err = Access::GetAccessControl().Check(aSubjectDescriptor, requestPath, requestPrivilege);
303303
if (err != CHIP_NO_ERROR)
304304
{
305-
ReturnErrorCodeIf(err != CHIP_ERROR_ACCESS_DENIED, err);
305+
ReturnErrorCodeIf((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL), err);
306306
if (aPath.mExpanded)
307307
{
308308
return CHIP_NO_ERROR;
309309
}
310-
return CHIP_IM_GLOBAL_STATUS(UnsupportedAccess);
310+
return err == CHIP_ERROR_ACCESS_DENIED ? CHIP_IM_GLOBAL_STATUS(UnsupportedAccess)
311+
: CHIP_IM_GLOBAL_STATUS(AccessRestricted);
311312
}
312313
}
313314

@@ -701,9 +702,12 @@ CHIP_ERROR WriteSingleClusterData(const SubjectDescriptor & aSubjectDescriptor,
701702
}
702703
if (err != CHIP_NO_ERROR)
703704
{
704-
ReturnErrorCodeIf(err != CHIP_ERROR_ACCESS_DENIED, err);
705+
ReturnErrorCodeIf((err != CHIP_ERROR_ACCESS_DENIED) && (err != CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL), err);
705706
// TODO: when wildcard/group writes are supported, handle them to discard rather than fail with status
706-
return apWriteHandler->AddStatus(aPath, Protocols::InteractionModel::Status::UnsupportedAccess);
707+
return apWriteHandler->AddStatus(aPath,
708+
err == CHIP_ERROR_ACCESS_DENIED
709+
? Protocols::InteractionModel::Status::UnsupportedAccess
710+
: Protocols::InteractionModel::Status::AccessRestricted);
707711
}
708712
apWriteHandler->CacheACLCheckResult({ aPath, requestPrivilege });
709713
}

src/controller/java/src/chip/devicecontroller/model/Status.java

+1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ public enum Code {
5757
Reserved99(0x99),
5858
Reserved9a(0x9a),
5959
Busy(0x9c),
60+
AccessRestricted(0x9d),
6061
Deprecatedc0(0xc0),
6162
Deprecatedc1(0xc1),
6263
Deprecatedc2(0xc2),

src/controller/java/src/matter/controller/model/Status.kt

+1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ data class Status(val status: Int, val clusterStatus: Int?) {
5757
RESERVED99(0X99),
5858
RESERVED9A(0X9A),
5959
BUSY(0X9C),
60+
ACCESS_RESTRICTED(0x9D),
6061
DEPRECATEDC0(0XC0),
6162
DEPRECATEDC1(0XC1),
6263
DEPRECATEDC2(0XC2),

src/controller/python/chip/interaction_model/__init__.py

+1
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ class Status(enum.IntEnum):
7373
Reserved99 = 0x99
7474
Reserved9a = 0x9a
7575
Busy = 0x9c
76+
AccessRestricted = 0x9d
7677
Deprecatedc0 = 0xc0
7778
Deprecatedc1 = 0xc1
7879
Deprecatedc2 = 0xc2

src/lib/core/CHIPError.cpp

+3
Original file line numberDiff line numberDiff line change
@@ -362,6 +362,9 @@ bool FormatCHIPError(char * buf, uint16_t bufSize, CHIP_ERROR err)
362362
case CHIP_ERROR_VERSION_MISMATCH.AsInteger():
363363
desc = "Version mismatch";
364364
break;
365+
case CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL.AsInteger():
366+
desc = "The CHIP message's access is restricted by ARL";
367+
break;
365368
case CHIP_EVENT_ID_FOUND.AsInteger():
366369
desc = "Event ID matching criteria was found";
367370
break;

src/lib/core/CHIPError.h

+8-1
Original file line numberDiff line numberDiff line change
@@ -1484,7 +1484,14 @@ using CHIP_ERROR = ::chip::ChipError;
14841484
*/
14851485
#define CHIP_ERROR_VERSION_MISMATCH CHIP_CORE_ERROR(0xa7)
14861486

1487-
// AVAILABLE: 0xa8
1487+
/**
1488+
* @def CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL
1489+
*
1490+
* @brief
1491+
* The CHIP message is not granted access for further processing due to Access Restriction List.
1492+
*/
1493+
#define CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL CHIP_CORE_ERROR(0xa8)
1494+
14881495
// AVAILABLE: 0xa9
14891496
// AVAILABLE: 0xaa
14901497

src/lib/core/tests/TestCHIPErrorStr.cpp

+1
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ static const CHIP_ERROR kTestElements[] =
140140
CHIP_ERROR_ACCESS_DENIED,
141141
CHIP_ERROR_UNKNOWN_RESOURCE_ID,
142142
CHIP_ERROR_VERSION_MISMATCH,
143+
CHIP_ERROR_ACCESS_RESTRICTED_BY_ARL,
143144
CHIP_EVENT_ID_FOUND,
144145
CHIP_ERROR_INTERNAL,
145146
CHIP_ERROR_OPEN_FAILED,

src/protocols/interaction_model/StatusCodeList.h

+1
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ CHIP_IM_STATUS_CODE(Reserved98 , Reserved98 , 0x98)
6161
CHIP_IM_STATUS_CODE(Reserved99 , Reserved99 , 0x99)
6262
CHIP_IM_STATUS_CODE(Reserved9a , Reserved9a , 0x9a)
6363
CHIP_IM_STATUS_CODE(Busy , BUSY , 0x9c)
64+
CHIP_IM_STATUS_CODE(AccessRestricted , ACCESS_RESTRICTED , 0x9d)
6465
CHIP_IM_STATUS_CODE(Deprecatedc0 , Deprecatedc0 , 0xc0)
6566
CHIP_IM_STATUS_CODE(Deprecatedc1 , Deprecatedc1 , 0xc1)
6667
CHIP_IM_STATUS_CODE(Deprecatedc2 , Deprecatedc2 , 0xc2)

src/python_testing/TC_ACL_2_11.py

+8-8
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,9 @@ def steps_TC_ACL_2_11(self) -> list[TestStep]:
7373
TestStep(2, "TH1 reads DUT Endpoint 0 AccessControl cluster CommissioningARL attribute"),
7474
TestStep(3, "TH1 reads DUT Endpoint 0 AccessControl cluster ARL attribute"),
7575
TestStep(4, "For each entry in ARL, iterate over each restriction and attempt access the restriction's ID on the Endpoint and Cluster in the ARL entry.",
76-
"If the restriction is Type AttributeAccessForbidden, read the restriction's attribute ID and verify the response is UNSUPPORTED_ACCESS."
77-
"If the restriction is Type AttributeWriteForbidden, write restriction's the attribute ID and verify the response is UNSUPPORTED_ACCESS."
78-
"If the restriction is Type CommandForbidden, invoke the restriction's command ID and verify the response is UNSUPPORTED_ACCESS."),
76+
"If the restriction is Type AttributeAccessForbidden, read the restriction's attribute ID and verify the response is ACCESS_RESTRICTED."
77+
"If the restriction is Type AttributeWriteForbidden, write restriction's the attribute ID and verify the response is ACCESS_RESTRICTED."
78+
"If the restriction is Type CommandForbidden, invoke the restriction's command ID and verify the response is ACCESS_RESTRICTED."),
7979
TestStep(5, "TH1 sends DUT Endpoint 0 AccessControl cluster command ReviewFabricRestrictions"),
8080
TestStep(6, "Wait for up to 1 hour. Follow instructions provided by device maker to remove all access restrictions",
8181
"AccessRestrictionReviewUpdate event is received"),
@@ -119,15 +119,15 @@ async def test_TC_ACL_2_11(self):
119119
command = ALL_ACCEPTED_COMMANDS[C1][ID1]
120120

121121
if restriction_type == AccessControl.Enums.AccessRestrictionTypeEnum.kAttributeAccessForbidden:
122-
await self.read_single_attribute_expect_error(cluster=cluster, attribute=attribute, error=Status.UnsupportedAccess, endpoint=E1)
122+
await self.read_single_attribute_expect_error(cluster=cluster, attribute=attribute, error=Status.AccessRestricted, endpoint=E1)
123123
elif restriction_type == AccessControl.Enums.AccessRestrictionTypeEnum.kAttributeWriteForbidden:
124124
status = await self.write_single_attribute(attribute_value=attribute, endpoint_id=E1)
125-
asserts.assert_equal(status, Status.UnsupportedAccess,
126-
f"Failed to verify UNSUPPORTED_ACCESS when writing to Attribute {ID1} Cluster {C1} Endpoint {E1}")
125+
asserts.assert_equal(status, Status.AccessRestricted,
126+
f"Failed to verify ACCESS_RESTRICTED when writing to Attribute {ID1} Cluster {C1} Endpoint {E1}")
127127
elif restriction_type == AccessControl.Enums.AccessRestrictionTypeEnum.kCommandForbidden:
128128
result = await self.send_single_cmd(cmd=command, endpoint=E1)
129-
asserts.assert_equal(result.status, Status.UnsupportedAccess,
130-
f"Failed to verify UNSUPPORTED_ACCESS when sending command {ID1} to Cluster {C1} Endpoint {E1}")
129+
asserts.assert_equal(result.status, Status.AccessRestricted,
130+
f"Failed to verify ACCESS_RESTRICTED when sending command {ID1} to Cluster {C1} Endpoint {E1}")
131131

132132
# Belongs to step 6, but needs to be subscribed before executing step 5: begin
133133
arru_queue = queue.Queue()

0 commit comments

Comments
 (0)