Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access control module hookup #10581

Conversation

mlepage-google
Copy link
Contributor

Add initial prototype hookup of AccessControl

@todo
Copy link

todo bot commented Oct 15, 2021

until we have an actual implementation, allow access

// TODO: until we have an actual implementation, allow access
ReturnErrorCodeIf(iterator == nullptr, CHIP_NO_ERROR);
#endif
while (iterator->HasNext())
{
ChipLogDetail(DataManagement, "Checking entry");
auto & entry = iterator->Next();
if (!entry.MatchesPrivilege(privilege))
continue;


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@boring-cyborg boring-cyborg bot added the app label Oct 15, 2021
@todo
Copy link

todo bot commented Oct 15, 2021

check CATs (subject1, subject2)

// TODO: check CATs (subject1, subject2)
if (!entry.MatchesSubject(subjectDescriptor.subject))
continue;
ChipLogDetail(DataManagement, " --> matched subject");
if (!entry.MatchesTarget(requestPath.endpoint, requestPath.cluster))
continue;
ChipLogDetail(DataManagement, " --> matched target");
err = CHIP_NO_ERROR;
break;
}


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

these basic types need to come from a lower layer

// TODO: these basic types need to come from a lower layer
// "app/util/basic-types.h" has a lot but isn't a lower layer
// so organization and refactoring needs to happen sometime
// and this will change the namespace also
typedef uint64_t CatId;
typedef uint32_t ClusterId;
typedef uint32_t DeviceTypeId;
typedef uint16_t EndpointId;
typedef uint8_t FabricIndex;
typedef uint16_t GroupId;


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

provide iterator

// TODO: provide iterator
return nullptr;
}
EntryIterator* DataProviderImpl::Entries(FabricIndex fabricIndex) const
{
// TODO: provide iterator
return nullptr;
}
} // namespace access


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

provide iterator

// TODO: provide iterator
return nullptr;
}
} // namespace access
} // namespace chip


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

don't worry about node for now (proxy source)

// TODO: don't worry about node for now (proxy source)
EndpointId endpoint = 0;
ClusterId cluster = 0;
};
} // namespace access
} // namespace chip


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@CLAassistant
Copy link

CLAassistant commented Oct 15, 2021

CLA assistant check
All committers have signed the CLA.

@todo
Copy link

todo bot commented Oct 15, 2021

make this table driven, add a bunch more test cases

// TODO: make this table driven, add a bunch more test cases
err = context.Check(
{.subject = 0x1122334455667788, .authMode = AuthMode::Case, .fabricIndex = 1},
{.endpoint = 1, .cluster = ONOFF},
Privilege::Administer);
NL_TEST_ASSERT(inSuite, err == CHIP_NO_ERROR);
err = context.Check(
{.subject = 0x8877665544332211, .authMode = AuthMode::Case, .fabricIndex = 1},
{.endpoint = 1, .cluster = ONOFF},


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

get required privilege from yet-to-be-written ember api

// TODO: get required privilege from yet-to-be-written ember api
#else
// TEMP: assume view privilege required
Privilege privilege = Privilege::View;
#endif
{
access::RequestPath requestPath {
.endpoint = aPath.mEndpointId,
.cluster = aPath.mClusterId
};


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

In some cases (wildcards) we'll want to just discard request path

// TODO: In some cases (wildcards) we'll want to just discard request path
return apWriter->Put(chip::TLV::ContextTag(AttributeDataElement::kCsTag_Status), Protocols::InteractionModel::Status::UnsupportedAccess);
}
else
{
return err;
}
}
ChipLogProgress(DataManagement, "ACCESS CONTROL --> ALLOWED");
}


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

filter fabric sensitive data if fabric doesn't match

// TODO: filter fabric sensitive data if fabric doesn't match
// TODO: ZCL_STRUCT_ATTRIBUTE_TYPE is not included in this switch case currently, should add support for structures.
switch (BaseType(attributeType))
{


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

get required privilege from yet-to-be-written ember api

// TODO: get required privilege from yet-to-be-written ember api
#else
// TEMP: assume operate privilege required
Privilege privilege = Privilege::Operate;
#endif
{
access::RequestPath requestPath {
.endpoint = aClusterInfo.mEndpointId,
.cluster = aClusterInfo.mClusterId
};


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

better mapping of chip error to IM status

// TODO: better mapping of chip error to IM status
return Protocols::InteractionModel::Status::Failure;
}
}
ChipLogProgress(DataManagement, "ACCESS CONTROL --> ALLOWED");
}
// TODO: don't write fabric scoped data if fabric doesn't match
CHIP_ERROR preparationError = CHIP_NO_ERROR;
uint16_t dataLen = 0;


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

don't write fabric scoped data if fabric doesn't match

// TODO: don't write fabric scoped data if fabric doesn't match
CHIP_ERROR preparationError = CHIP_NO_ERROR;
uint16_t dataLen = 0;
if ((preparationError = prepareWriteData(attributeMetadata->attributeType, aReader, dataLen)) != CHIP_NO_ERROR)
{
ChipLogDetail(Zcl, "Failed to prepare data to write: %s", ErrorStr(preparationError));
return Protocols::InteractionModel::Status::InvalidValue;
}


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

these are just placeholder values for now, need to get appropriate

// TODO: these are just placeholder values for now, need to get appropriate
// fields (auth mode, node id, etc.) from session handle (or whereever)
// and put in SubjectDescriptor appropriately, and ensure it's correct for
// all cases (e.g. peer node ID is good)
auto& session = mSecureSession.Value();
subjectDescriptor.authMode = access::AuthMode::Case;
subjectDescriptor.subject = session.GetPeerNodeId();
subjectDescriptor.fabricIndex = session.GetFabricIndex();
}
return subjectDescriptor;


This comment was generated by todo based on a TODO comment in 014ac99 in #10581. cc @mlepage-google.

@github-actions
Copy link

github-actions bot commented Oct 15, 2021

PR #10581: Size comparison from efc17de to 014ac99

Increases above 1.0% from efc17de to 014ac99:

platform target config section efc17de 014ac99 change % change
linux all-clusters-app debug .init_array 512 520 8 1.6
chip-tool debug .data 1584 1616 32 2.0
.init_array 416 424 8 1.9
ota-provider-app debug .data 752 768 16 2.1
.init_array 440 448 8 1.8
ota-requestor-app debug .data 752 768 16 2.1
.init_array 512 520 8 1.6
tv-app debug .data 2032 2064 32 1.6
.init_array 608 616 8 1.3
bridge-app debug+rpc .data 976 992 16 1.6
.init_array 400 408 8 2.0
lighting-app debug+rpc .data 1106 1138 32 2.9
.init_array 528 536 8 1.5
qpg lighting-app qpg6100+debug .data 996 1008 12 1.2
lock-app qpg6100+debug .data 952 964 12 1.3
22 builds
platform target config section efc17de 014ac99 change % change
efr32 lighting-app BRD4161A .bss 118020 118020 0 0.0
.data 1800 1812 12 0.7
.text 782420 783260 840 0.1
lock-app BRD4161A .bss 115892 115892 0 0.0
.data 1760 1772 12 0.7
.text 761724 762564 840 0.1
window-app BRD4161A .bss 116212 116212 0 0.0
.data 1764 1776 12 0.7
.text 762632 763472 840 0.1
lighting-app BRD4161A+rpc .bss 131348 131348 0 0.0
.data 1852 1864 12 0.6
.text 762148 763004 856 0.1
k32w lock-app k32w061+debug .bss 69052 69052 0 0.0
.data 1864 1876 12 0.6
.text 515176 515896 720 0.1
shell k32w061+debug .bss 55080 55080 0 0.0
.data 672 672 0 0.0
.text 357172 357172 0 0.0
lighting-app k32w061+se05x+release .bss 78560 78568 8 0.0
.data 1900 1912 12 0.6
.text 613800 614524 724 0.1
linux all-clusters-app debug .bss 52176 52208 32 0.1
.data 978 978 0 0.0
.data.rel.ro 58432 58560 128 0.2
.dynamic 592 592 0 0.0
.got 4072 4072 0 0.0
.init 27 27 0 0.0
.init_array 512 520 8 1.6
.rodata 135989 136661 672 0.5
.text 1326066 1328402 2336 0.2
chip-tool debug .bss 17552 17584 32 0.2
.data 1584 1616 32 2.0
.data.rel.ro 83312 83440 128 0.2
.dynamic 592 592 0 0.0
.got 4328 4328 0 0.0
.init 27 27 0 0.0
.init_array 416 424 8 1.9
.rodata 174560 175136 576 0.3
.text 2421973 2424325 2352 0.1
ota-provider-app debug .bss 37472 37504 32 0.1
.data 752 768 16 2.1
.data.rel.ro 23176 23304 128 0.6
.dynamic 592 592 0 0.0
.got 4008 4008 0 0.0
.init 27 27 0 0.0
.init_array 440 448 8 1.8
.rodata 109848 110520 672 0.6
.text 1009314 1011666 2352 0.2
ota-requestor-app debug .bss 205728 205760 32 0.0
.data 752 768 16 2.1
.data.rel.ro 24488 24616 128 0.5
.dynamic 592 592 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 512 520 8 1.6
.rodata 127896 128568 672 0.5
.text 1128626 1130962 2336 0.2
shell debug .bss 16072 16072 0 0.0
.data 242 242 0 0.0
.data.rel.ro 35120 35120 0 0.0
.dynamic 592 592 0 0.0
.got 3496 3496 0 0.0
.init 27 27 0 0.0
.init_array 336 336 0 0.0
.rodata 71855 71855 0 0.0
.text 568898 568898 0 0.0
tv-app debug .bss 216368 216400 32 0.0
.data 2032 2064 32 1.6
.data.rel.ro 55536 55664 128 0.2
.dynamic 592 592 0 0.0
.got 4400 4400 0 0.0
.init 27 27 0 0.0
.init_array 608 616 8 1.3
.rodata 151464 152040 576 0.4
.text 1462818 1465154 2336 0.2
bridge-app debug+rpc .bss 52880 52912 32 0.1
.data 976 992 16 1.6
.data.rel.ro 25496 25624 128 0.5
.dynamic 592 592 0 0.0
.got 3944 3944 0 0.0
.init 27 27 0 0.0
.init_array 400 408 8 2.0
.rodata 110484 111156 672 0.6
.text 1050821 1053157 2336 0.2
lighting-app debug+rpc .bss 42200 42232 32 0.1
.data 1106 1138 32 2.9
.data.rel.ro 52240 52368 128 0.2
.dynamic 608 608 0 0.0
.got 4104 4104 0 0.0
.init 27 27 0 0.0
.init_array 528 536 8 1.5
.rodata 127665 128305 640 0.5
.text 1252226 1254578 2352 0.2
mbed lighting-app CY8CPROTO_062_4343W+release .bss 172148 172148 0 0.0
.data 5464 5472 8 0.1
.heap 858832 858824 -8 -0.0
.text 1219384 1220080 696 0.1
lock-app CY8CPROTO_062_4343W+release .bss 171084 171084 0 0.0
.data 5432 5448 16 0.3
.heap 859928 859912 -16 -0.0
.text 1197440 1198136 696 0.1
p6 lock-app default .bss 68216 68224 8 0.0
.data 2416 2424 8 0.3
.heap 962712 962696 -16 -0.0
.text 1126280 1127120 840 0.1
qpg lighting-app qpg6100+debug .bss 53536 53544 8 0.0
.data 996 1008 12 1.2
.text 486104 486612 508 0.1
lock-app qpg6100+debug .bss 52488 52496 8 0.0
.data 952 964 12 1.3
.text 462352 462868 516 0.1
persistent-storage-app qpg6100+debug .bss 17778 17778 0 0.0
.data 280 280 0 0.0
.text 102704 102704 0 0.0
telink lighting-app tlsr9518adk80d bss 70984 70992 8 0.0
noinit 33216 33216 0 0.0
text 458146 458776 630 0.1
12 builds
platform target config section efc17de 014ac99 change % change
esp32 all-clusters-app c3devkit .dram0.bss 60280 60280 0 0.0
.dram0.data 16192 16200 8 0.0
.flash.rodata 198624 198832 208 0.1
.flash.text 870142 870742 600 0.1
.iram0.text 57330 57330 0 0.0
m5stack .dram0.bss 62776 62784 8 0.0
.dram0.data 32084 32092 8 0.0
.flash.rodata 207056 207252 196 0.1
.flash.text 900507 901175 668 0.1
.iram0.text 125115 125115 0 0.0
nrfconnect lighting-app nrf52840dk_nrf52840 bss 112316 112320 4 0.0
rodata 97528 97716 188 0.2
text 577428 577956 528 0.1
lock-app nrf52840dk_nrf52840 bss 111348 111352 4 0.0
rodata 94028 94212 184 0.2
text 558928 559456 528 0.1
pigweed-app nrf52840dk_nrf52840 bss 51772 51772 0 0.0
rodata 45772 45772 0 0.0
text 339392 339392 0 0.0
pump-app nrf52840dk_nrf52840 bss 111416 111420 4 0.0
rodata 95008 95192 184 0.2
text 562084 562612 528 0.1
pump-controller-app nrf52840dk_nrf52840 bss 111356 111360 4 0.0
rodata 94088 94272 184 0.2
text 558720 559248 528 0.1
shell nrf52840dk_nrf52840 bss 107316 107316 0 0.0
rodata 71640 71640 0 0.0
text 518908 518908 0 0.0
lighting-app nrf52840dk_nrf52840+rpc bss 108556 108560 4 0.0
rodata 88320 88508 188 0.2
text 550632 551160 528 0.1
nrf5340dk_nrf5340_cpuapp bss 113688 113692 4 0.0
rodata 92772 92956 184 0.2
text 506888 507416 528 0.1
lock-app nrf5340dk_nrf5340_cpuapp bss 112720 112724 4 0.0
rodata 89284 89472 188 0.2
text 488380 488908 528 0.1
shell nrf5340dk_nrf5340_cpuapp bss 108300 108300 0 0.0
rodata 66284 66284 0 0.0
text 439504 439504 0 0.0

@github-actions
Copy link

Size increase report for "nrfconnect-example-build" from efc17de

File Section File VM
chip-lock.elf text 528 528
chip-lock.elf rodata 184 184
chip-lock.elf datas 8 8
chip-lock.elf init_array 4 4
chip-lock.elf device_handles -4 -4
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-shell.elf and ./pull_artifact/chip-shell.elf:

sections,vmsize,filesize
.debug_info,0,5567
.debug_line,0,1139
.debug_str,0,210
.debug_abbrev,0,110
.debug_loc,0,-58

Comparing ./master_artifact/chip-lock.elf and ./pull_artifact/chip-lock.elf:

sections,vmsize,filesize
.debug_info,0,97636
.debug_line,0,11959
.debug_abbrev,0,6393
.debug_str,0,1996
.debug_loc,0,1960
.strtab,0,698
text,528,528
.symtab,0,496
.debug_ranges,0,336
.debug_frame,0,296
rodata,184,184
.debug_aranges,0,144
datas,8,8
init_array,4,4
.shstrtab,0,2
device_handles,-4,-4


@todo
Copy link

todo bot commented Oct 15, 2021

don't worry about node for now (proxy source)

// TODO: don't worry about node for now (proxy source)
EndpointId endpoint = 0;
ClusterId cluster = 0;
};
} // namespace access
} // namespace chip


This comment was generated by todo based on a TODO comment in 25ccc69 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

In some cases (wildcards) we'll want to just discard request path

// TODO: In some cases (wildcards) we'll want to just discard request path
return apWriter->Put(chip::TLV::ContextTag(AttributeDataElement::kCsTag_Status), Protocols::InteractionModel::Status::UnsupportedAccess);
}
else
{
return err;
}
}
ChipLogProgress(DataManagement, "ACCESS CONTROL --> ALLOWED");
}


This comment was generated by todo based on a TODO comment in 25ccc69 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

filter fabric sensitive data if fabric doesn't match

// TODO: filter fabric sensitive data if fabric doesn't match
// TODO: ZCL_STRUCT_ATTRIBUTE_TYPE is not included in this switch case currently, should add support for structures.
switch (BaseType(attributeType))
{


This comment was generated by todo based on a TODO comment in 25ccc69 in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

don't write fabric scoped data if fabric doesn't match

// TODO: don't write fabric scoped data if fabric doesn't match
CHIP_ERROR preparationError = CHIP_NO_ERROR;
uint16_t dataLen = 0;
if ((preparationError = prepareWriteData(attributeMetadata->attributeType, aReader, dataLen)) != CHIP_NO_ERROR)
{
ChipLogDetail(Zcl, "Failed to prepare data to write: %s", ErrorStr(preparationError));
return Protocols::InteractionModel::Status::InvalidValue;
}


This comment was generated by todo based on a TODO comment in 25ccc69 in #10581. cc @mlepage-google.

@github-actions
Copy link

Size increase report for "esp32-example-build" from efc17de

File Section File VM
chip-all-clusters-app.elf .flash.text 600 600
chip-all-clusters-app.elf .flash.rodata 208 208
chip-all-clusters-app.elf .dram0.data 8 16
Full report output
BLOAT REPORT

Files found only in the build output:
    report.csv

Comparing ./master_artifact/chip-all-clusters-app.elf and ./pull_artifact/chip-all-clusters-app.elf:

sections,vmsize,filesize
.debug_info,0,146842
.debug_line,0,17920
.debug_abbrev,0,7145
.debug_str,0,2265
.debug_loc,0,1237
.strtab,0,689
.flash.text,600,600
.debug_ranges,0,304
.debug_frame,0,296
.symtab,0,240
.flash.rodata,208,208
.debug_aranges,0,128
.dram0.data,16,8
.shstrtab,0,3
.riscv.attributes,0,-1
[Unmapped],0,-816


@github-actions
Copy link

github-actions bot commented Oct 15, 2021

PR #10581: Size comparison from efc17de to 25ccc69

5 builds
platform target config section efc17de 25ccc69 change % change
k32w lock-app k32w061+debug .bss 69052 69052 0 0.0
.data 1864 1876 12 0.6
.text 515176 515896 720 0.1
shell k32w061+debug .bss 55080 55080 0 0.0
.data 672 672 0 0.0
.text 357172 357172 0 0.0
lighting-app k32w061+se05x+release .bss 78560 78568 8 0.0
.data 1900 1912 12 0.6
.text 613800 614524 724 0.1
p6 lock-app default .bss 68216 68224 8 0.0
.data 2416 2424 8 0.3
.heap 962712 962696 -16 -0.0
.text 1126280 1127120 840 0.1
telink lighting-app tlsr9518adk80d bss 70984 70992 8 0.0
noinit 33216 33216 0 0.0
text 458146 458776 630 0.1

Increases above 1.0% from efc17de to 25ccc69:

platform target config section efc17de 25ccc69 change % change
linux all-clusters-app debug .init_array 512 520 8 1.6
chip-tool debug .data 1584 1616 32 2.0
.init_array 416 424 8 1.9
ota-provider-app debug .data 752 768 16 2.1
.init_array 440 448 8 1.8
ota-requestor-app debug .data 752 768 16 2.1
.init_array 512 520 8 1.6
tv-app debug .data 2032 2064 32 1.6
.init_array 608 616 8 1.3
bridge-app debug+rpc .data 976 992 16 1.6
.init_array 400 408 8 2.0
lighting-app debug+rpc .data 1106 1138 32 2.9
.init_array 528 536 8 1.5
qpg lighting-app qpg6100+debug .data 996 1008 12 1.2
lock-app qpg6100+debug .data 952 964 12 1.3
17 builds
platform target config section efc17de 25ccc69 change % change
efr32 lighting-app BRD4161A .bss 118020 118020 0 0.0
.data 1800 1812 12 0.7
.text 782420 783260 840 0.1
lock-app BRD4161A .bss 115892 115892 0 0.0
.data 1760 1772 12 0.7
.text 761724 762564 840 0.1
window-app BRD4161A .bss 116212 116212 0 0.0
.data 1764 1776 12 0.7
.text 762632 763472 840 0.1
lighting-app BRD4161A+rpc .bss 131348 131348 0 0.0
.data 1852 1864 12 0.6
.text 762148 763004 856 0.1
linux all-clusters-app debug .bss 52176 52208 32 0.1
.data 978 978 0 0.0
.data.rel.ro 58432 58560 128 0.2
.dynamic 592 592 0 0.0
.got 4072 4072 0 0.0
.init 27 27 0 0.0
.init_array 512 520 8 1.6
.rodata 135989 136661 672 0.5
.text 1326066 1328402 2336 0.2
chip-tool debug .bss 17552 17584 32 0.2
.data 1584 1616 32 2.0
.data.rel.ro 83312 83440 128 0.2
.dynamic 592 592 0 0.0
.got 4328 4328 0 0.0
.init 27 27 0 0.0
.init_array 416 424 8 1.9
.rodata 174560 175136 576 0.3
.text 2421973 2424325 2352 0.1
ota-provider-app debug .bss 37472 37504 32 0.1
.data 752 768 16 2.1
.data.rel.ro 23176 23304 128 0.6
.dynamic 592 592 0 0.0
.got 4008 4008 0 0.0
.init 27 27 0 0.0
.init_array 440 448 8 1.8
.rodata 109848 110520 672 0.6
.text 1009314 1011666 2352 0.2
ota-requestor-app debug .bss 205728 205760 32 0.0
.data 752 768 16 2.1
.data.rel.ro 24488 24616 128 0.5
.dynamic 592 592 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 512 520 8 1.6
.rodata 127896 128568 672 0.5
.text 1128626 1130962 2336 0.2
shell debug .bss 16072 16072 0 0.0
.data 242 242 0 0.0
.data.rel.ro 35120 35120 0 0.0
.dynamic 592 592 0 0.0
.got 3496 3496 0 0.0
.init 27 27 0 0.0
.init_array 336 336 0 0.0
.rodata 71855 71855 0 0.0
.text 568898 568898 0 0.0
tv-app debug .bss 216368 216400 32 0.0
.data 2032 2064 32 1.6
.data.rel.ro 55536 55664 128 0.2
.dynamic 592 592 0 0.0
.got 4400 4400 0 0.0
.init 27 27 0 0.0
.init_array 608 616 8 1.3
.rodata 151464 152040 576 0.4
.text 1462818 1465154 2336 0.2
bridge-app debug+rpc .bss 52880 52912 32 0.1
.data 976 992 16 1.6
.data.rel.ro 25496 25624 128 0.5
.dynamic 592 592 0 0.0
.got 3944 3944 0 0.0
.init 27 27 0 0.0
.init_array 400 408 8 2.0
.rodata 110484 111156 672 0.6
.text 1050821 1053157 2336 0.2
lighting-app debug+rpc .bss 42200 42232 32 0.1
.data 1106 1138 32 2.9
.data.rel.ro 52240 52368 128 0.2
.dynamic 608 608 0 0.0
.got 4104 4104 0 0.0
.init 27 27 0 0.0
.init_array 528 536 8 1.5
.rodata 127665 128305 640 0.5
.text 1252226 1254578 2352 0.2
mbed lighting-app CY8CPROTO_062_4343W+release .bss 172148 172148 0 0.0
.data 5464 5472 8 0.1
.heap 858832 858824 -8 -0.0
.text 1219384 1220080 696 0.1
lock-app CY8CPROTO_062_4343W+release .bss 171084 171084 0 0.0
.data 5432 5448 16 0.3
.heap 859928 859912 -16 -0.0
.text 1197440 1198136 696 0.1
qpg lighting-app qpg6100+debug .bss 53536 53544 8 0.0
.data 996 1008 12 1.2
.text 486104 486612 508 0.1
lock-app qpg6100+debug .bss 52488 52496 8 0.0
.data 952 964 12 1.3
.text 462352 462868 516 0.1
persistent-storage-app qpg6100+debug .bss 17778 17778 0 0.0
.data 280 280 0 0.0
.text 102704 102704 0 0.0

@todo
Copy link

todo bot commented Oct 15, 2021

don't worry about node for now (proxy source)

// TODO: don't worry about node for now (proxy source)
EndpointId endpoint = 0;
ClusterId cluster = 0;
};
} // namespace access
} // namespace chip


This comment was generated by todo based on a TODO comment in 30556bf in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

In some cases (wildcards) we'll want to just discard request path

// TODO: In some cases (wildcards) we'll want to just discard request path
return apWriter->Put(chip::TLV::ContextTag(AttributeDataElement::kCsTag_Status), Protocols::InteractionModel::Status::UnsupportedAccess);
}
else
{
return err;
}
}
ChipLogProgress(DataManagement, "ACCESS CONTROL --> ALLOWED");
}


This comment was generated by todo based on a TODO comment in 30556bf in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

filter fabric sensitive data if fabric doesn't match

// TODO: filter fabric sensitive data if fabric doesn't match
// TODO: ZCL_STRUCT_ATTRIBUTE_TYPE is not included in this switch case currently, should add support for structures.
switch (BaseType(attributeType))
{


This comment was generated by todo based on a TODO comment in 30556bf in #10581. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 15, 2021

don't write fabric scoped data if fabric doesn't match

// TODO: don't write fabric scoped data if fabric doesn't match
CHIP_ERROR preparationError = CHIP_NO_ERROR;
uint16_t dataLen = 0;
if ((preparationError = prepareWriteData(attributeMetadata->attributeType, aReader, dataLen)) != CHIP_NO_ERROR)
{
ChipLogDetail(Zcl, "Failed to prepare data to write: %s", ErrorStr(preparationError));
return Protocols::InteractionModel::Status::InvalidValue;
}


This comment was generated by todo based on a TODO comment in 30556bf in #10581. cc @mlepage-google.

@github-actions
Copy link

github-actions bot commented Oct 15, 2021

PR #10581: Size comparison from efc17de to 30556bf

2 builds
platform target config section efc17de 30556bf change % change
p6 lock-app default .bss 68216 68224 8 0.0
.data 2416 2424 8 0.3
.heap 962712 962696 -16 -0.0
.text 1126280 1127120 840 0.1
telink lighting-app tlsr9518adk80d bss 70984 70992 8 0.0
noinit 33216 33216 0 0.0
text 458146 458776 630 0.1

Increases above 1.0% from efc17de to 30556bf:

platform target config section efc17de 30556bf change % change
linux all-clusters-app debug .init_array 512 520 8 1.6
chip-tool debug .data 1584 1616 32 2.0
.init_array 416 424 8 1.9
ota-provider-app debug .data 752 768 16 2.1
.init_array 440 448 8 1.8
ota-requestor-app debug .data 752 768 16 2.1
.init_array 512 520 8 1.6
tv-app debug .data 2032 2064 32 1.6
.init_array 608 616 8 1.3
bridge-app debug+rpc .data 976 992 16 1.6
.init_array 400 408 8 2.0
lighting-app debug+rpc .data 1106 1138 32 2.9
.init_array 528 536 8 1.5
qpg lighting-app qpg6100+debug .data 996 1008 12 1.2
lock-app qpg6100+debug .data 952 964 12 1.3
14 builds
platform target config section efc17de 30556bf change % change
k32w lock-app k32w061+debug .bss 69052 69052 0 0.0
.data 1864 1876 12 0.6
.text 515176 515896 720 0.1
shell k32w061+debug .bss 55080 55080 0 0.0
.data 672 672 0 0.0
.text 357172 357172 0 0.0
lighting-app k32w061+se05x+release .bss 78560 78568 8 0.0
.data 1900 1912 12 0.6
.text 613800 614524 724 0.1
linux all-clusters-app debug .bss 52176 52208 32 0.1
.data 978 978 0 0.0
.data.rel.ro 58432 58560 128 0.2
.dynamic 592 592 0 0.0
.got 4072 4072 0 0.0
.init 27 27 0 0.0
.init_array 512 520 8 1.6
.rodata 135989 136661 672 0.5
.text 1326066 1328402 2336 0.2
chip-tool debug .bss 17552 17584 32 0.2
.data 1584 1616 32 2.0
.data.rel.ro 83312 83440 128 0.2
.dynamic 592 592 0 0.0
.got 4328 4328 0 0.0
.init 27 27 0 0.0
.init_array 416 424 8 1.9
.rodata 174560 175136 576 0.3
.text 2421973 2424325 2352 0.1
ota-provider-app debug .bss 37472 37504 32 0.1
.data 752 768 16 2.1
.data.rel.ro 23176 23304 128 0.6
.dynamic 592 592 0 0.0
.got 4008 4008 0 0.0
.init 27 27 0 0.0
.init_array 440 448 8 1.8
.rodata 109848 110520 672 0.6
.text 1009314 1011666 2352 0.2
ota-requestor-app debug .bss 205728 205760 32 0.0
.data 752 768 16 2.1
.data.rel.ro 24488 24616 128 0.5
.dynamic 592 592 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 512 520 8 1.6
.rodata 127896 128568 672 0.5
.text 1128626 1130962 2336 0.2
shell debug .bss 16072 16072 0 0.0
.data 242 242 0 0.0
.data.rel.ro 35120 35120 0 0.0
.dynamic 592 592 0 0.0
.got 3496 3496 0 0.0
.init 27 27 0 0.0
.init_array 336 336 0 0.0
.rodata 71855 71855 0 0.0
.text 568898 568898 0 0.0
tv-app debug .bss 216368 216400 32 0.0
.data 2032 2064 32 1.6
.data.rel.ro 55536 55664 128 0.2
.dynamic 592 592 0 0.0
.got 4400 4400 0 0.0
.init 27 27 0 0.0
.init_array 608 616 8 1.3
.rodata 151464 152040 576 0.4
.text 1462818 1465154 2336 0.2
bridge-app debug+rpc .bss 52880 52912 32 0.1
.data 976 992 16 1.6
.data.rel.ro 25496 25624 128 0.5
.dynamic 592 592 0 0.0
.got 3944 3944 0 0.0
.init 27 27 0 0.0
.init_array 400 408 8 2.0
.rodata 110484 111156 672 0.6
.text 1050821 1053157 2336 0.2
lighting-app debug+rpc .bss 42200 42232 32 0.1
.data 1106 1138 32 2.9
.data.rel.ro 52240 52368 128 0.2
.dynamic 608 608 0 0.0
.got 4104 4104 0 0.0
.init 27 27 0 0.0
.init_array 528 536 8 1.5
.rodata 127665 128305 640 0.5
.text 1252226 1254578 2352 0.2
qpg lighting-app qpg6100+debug .bss 53536 53544 8 0.0
.data 996 1008 12 1.2
.text 486104 486612 508 0.1
lock-app qpg6100+debug .bss 52488 52496 8 0.0
.data 952 964 12 1.3
.text 462352 462868 516 0.1
persistent-storage-app qpg6100+debug .bss 17778 17778 0 0.0
.data 280 280 0 0.0
.text 102704 102704 0 0.0
6 builds
platform target config section efc17de 30556bf change % change
efr32 lighting-app BRD4161A .bss 118020 118020 0 0.0
.data 1800 1812 12 0.7
.text 782420 783260 840 0.1
lock-app BRD4161A .bss 115892 115892 0 0.0
.data 1760 1772 12 0.7
.text 761724 762564 840 0.1
window-app BRD4161A .bss 116212 116212 0 0.0
.data 1764 1776 12 0.7
.text 762632 763472 840 0.1
lighting-app BRD4161A+rpc .bss 131348 131348 0 0.0
.data 1852 1864 12 0.6
.text 762148 763004 856 0.1
mbed lighting-app CY8CPROTO_062_4343W+release .bss 172148 172148 0 0.0
.data 5464 5472 8 0.1
.heap 858832 858824 -8 -0.0
.text 1219384 1220080 696 0.1
lock-app CY8CPROTO_062_4343W+release .bss 171084 171084 0 0.0
.data 5432 5448 16 0.3
.heap 859928 859912 -16 -0.0
.text 1197440 1198136 696 0.1

@@ -110,6 +110,10 @@ CHIP_ERROR WriteHandler::SendWriteResponse()
CHIP_ERROR WriteHandler::ProcessAttributeDataIBs(TLV::TLVReader & aAttributeDataIBsReader)
{
CHIP_ERROR err = CHIP_NO_ERROR;

// TODO: does exchange context always have session handle? what if it doesn't?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's hard to imagine how this could happen. But I think if it does, it means we just have to disallow everything.

If in the future we decide to store some of the subject descriptor information in the underlying secure session instead of the session handle, we'll have to make similar choices if we encounter a dangling session handle for which the underlying session has been evicted.

That also seems unlikely, but would probably still need some handling in the code.

@mlepage-google
Copy link
Contributor Author

This PR will have conflicts now because I already started breaking it into smaller ones. I'll probably have to turn it back into a draft, but I want to see how the builds pass/fail first.

@stale
Copy link

stale bot commented Nov 29, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Nov 29, 2021
@mlepage-google mlepage-google removed the stale Stale issue or PR label Nov 29, 2021
@pullapprove pullapprove bot requested a review from vijs December 1, 2021 18:15
@stale
Copy link

stale bot commented Dec 8, 2021

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the stale Stale issue or PR label Dec 8, 2021
@mlepage-google mlepage-google removed the stale Stale issue or PR label Dec 8, 2021
@mlepage-google
Copy link
Contributor Author

mlepage-google commented Dec 14, 2021

This PR can be closed now without merging as it has been broken into a series of smaller incremental PRs such as:
#12109 #12110 #12645 #12684 #12756 #12820 #12893 #12953 #13003

@mlepage-google mlepage-google deleted the access-control-module-hookup branch February 15, 2022 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants