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

Add initial prototype of AccessControl module #10579

Merged

Conversation

mlepage-google
Copy link
Contributor

@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 2eed92c in #10579. 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

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 2eed92c in #10579. 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 2eed92c in #10579. 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 2eed92c in #10579. 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 2eed92c in #10579. 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 2eed92c in #10579. cc @mlepage-google.

@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 2eed92c in #10579. cc @mlepage-google.

@github-actions
Copy link

PR #10579: Size comparison from efc17de to 2eed92c

22 builds
platform target config section efc17de 2eed92c change % change
efr32 lighting-app BRD4161A .bss 118020 118020 0 0.0
.data 1800 1800 0 0.0
.text 782420 782420 0 0.0
lock-app BRD4161A .bss 115892 115892 0 0.0
.data 1760 1760 0 0.0
.text 761724 761724 0 0.0
window-app BRD4161A .bss 116212 116212 0 0.0
.data 1764 1764 0 0.0
.text 762632 762632 0 0.0
lighting-app BRD4161A+rpc .bss 131348 131348 0 0.0
.data 1852 1852 0 0.0
.text 762148 762148 0 0.0
k32w lock-app k32w061+debug .bss 69052 69052 0 0.0
.data 1864 1864 0 0.0
.text 515176 515176 0 0.0
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 78560 0 0.0
.data 1900 1900 0 0.0
.text 613800 613800 0 0.0
linux all-clusters-app debug .bss 52176 52176 0 0.0
.data 978 978 0 0.0
.data.rel.ro 58432 58432 0 0.0
.dynamic 592 592 0 0.0
.got 4072 4072 0 0.0
.init 27 27 0 0.0
.init_array 512 512 0 0.0
.rodata 135989 135989 0 0.0
.text 1326066 1326066 0 0.0
chip-tool debug .bss 17552 17552 0 0.0
.data 1584 1584 0 0.0
.data.rel.ro 83312 83312 0 0.0
.dynamic 592 592 0 0.0
.got 4328 4328 0 0.0
.init 27 27 0 0.0
.init_array 416 416 0 0.0
.rodata 174560 174560 0 0.0
.text 2421973 2421973 0 0.0
ota-provider-app debug .bss 37472 37472 0 0.0
.data 752 752 0 0.0
.data.rel.ro 23176 23176 0 0.0
.dynamic 592 592 0 0.0
.got 4008 4008 0 0.0
.init 27 27 0 0.0
.init_array 440 440 0 0.0
.rodata 109848 109848 0 0.0
.text 1009314 1009314 0 0.0
ota-requestor-app debug .bss 205728 205728 0 0.0
.data 752 752 0 0.0
.data.rel.ro 24488 24488 0 0.0
.dynamic 592 592 0 0.0
.got 4136 4136 0 0.0
.init 27 27 0 0.0
.init_array 512 512 0 0.0
.rodata 127896 127896 0 0.0
.text 1128626 1128626 0 0.0
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 216368 0 0.0
.data 2032 2032 0 0.0
.data.rel.ro 55536 55536 0 0.0
.dynamic 592 592 0 0.0
.got 4400 4400 0 0.0
.init 27 27 0 0.0
.init_array 608 608 0 0.0
.rodata 151464 151464 0 0.0
.text 1462818 1462818 0 0.0
bridge-app debug+rpc .bss 52880 52880 0 0.0
.data 976 976 0 0.0
.data.rel.ro 25496 25496 0 0.0
.dynamic 592 592 0 0.0
.got 3944 3944 0 0.0
.init 27 27 0 0.0
.init_array 400 400 0 0.0
.rodata 110484 110484 0 0.0
.text 1050821 1050821 0 0.0
lighting-app debug+rpc .bss 42200 42200 0 0.0
.data 1106 1106 0 0.0
.data.rel.ro 52240 52240 0 0.0
.dynamic 608 608 0 0.0
.got 4104 4104 0 0.0
.init 27 27 0 0.0
.init_array 528 528 0 0.0
.rodata 127665 127665 0 0.0
.text 1252226 1252226 0 0.0
mbed lighting-app CY8CPROTO_062_4343W+release .bss 172148 172148 0 0.0
.data 5464 5464 0 0.0
.heap 858832 858832 0 0.0
.text 1219384 1219384 0 0.0
lock-app CY8CPROTO_062_4343W+release .bss 171084 171084 0 0.0
.data 5432 5432 0 0.0
.heap 859928 859928 0 0.0
.text 1197440 1197440 0 0.0
p6 lock-app default .bss 68216 68216 0 0.0
.data 2416 2416 0 0.0
.heap 962712 962712 0 0.0
.text 1126280 1126280 0 0.0
qpg lighting-app qpg6100+debug .bss 53536 53536 0 0.0
.data 996 996 0 0.0
.text 486104 486104 0 0.0
lock-app qpg6100+debug .bss 52488 52488 0 0.0
.data 952 952 0 0.0
.text 462352 462352 0 0.0
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 70984 0 0.0
noinit 33216 33216 0 0.0
text 458146 458146 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 8f6ebd4 in #10579. 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 df86b45 in #10579. cc @mlepage-google.

@github-actions
Copy link

PR #10579: Size comparison from efc17de to 8f6ebd4

2 builds
platform target config section efc17de 8f6ebd4 change % change
p6 lock-app default .bss 68216 68216 0 0.0
.data 2416 2416 0 0.0
.heap 962712 962712 0 0.0
.text 1126280 1126280 0 0.0
telink lighting-app tlsr9518adk80d bss 70984 70984 0 0.0
noinit 33216 33216 0 0.0
text 458146 458146 0 0.0

src/access/AccessControl.cpp Outdated Show resolved Hide resolved
src/access/AccessControl.h Outdated Show resolved Hide resolved
src/access/AccessControl.h Outdated Show resolved Hide resolved
src/access/AccessControl.h Outdated Show resolved Hide resolved
src/access/AccessControl.h Outdated Show resolved Hide resolved
src/access/DataProvider.h Outdated Show resolved Hide resolved
src/access/DataProvider.h Outdated Show resolved Hide resolved
src/access/DataProvider.h Outdated Show resolved Hide resolved
src/access/DataProviderImpl.h Outdated Show resolved Hide resolved
src/access/tests/TestAccessControl.cpp Outdated Show resolved Hide resolved
@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 6ec8954 in #10579. cc @mlepage-google.

mlepage-google and others added 5 commits October 27, 2021 12:53
Basic types (FabricIndex etc.) were moved in PR project-chip#10925 from app to
lib/core, so now they can be used from this module.
Also, remove CatId and move PasscodeId into lib/core.
@woody-apple woody-apple force-pushed the access-control-module-prototype branch from 3704aa6 to 1a570d3 Compare October 27, 2021 12:53
@todo
Copy link

todo bot commented Oct 27, 2021

check error (but can't until we have an implementation)

// TODO: check error (but can't until we have an implementation)
#if 0
ReturnErrorCodeIf(iterator == nullptr, CHIP_ERROR_INTERNAL);
#else
ReturnErrorCodeIf(iterator == nullptr, CHIP_NO_ERROR);
#endif
// TODO: a few more cases (PASE commissioning, CASE Authenticated Tags, etc.)
while (auto entry = iterator->Next())
{


This comment was generated by todo based on a TODO comment in 1a570d3 in #10579. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 27, 2021

a few more cases (PASE commissioning, CASE Authenticated Tags, etc.)

// TODO: a few more cases (PASE commissioning, CASE Authenticated Tags, etc.)
while (auto entry = iterator->Next())
{
ChipLogDetail(DataManagement, "Checking entry");
if (!entry->MatchesPrivilege(privilege))
continue;
ChipLogDetail(DataManagement, " --> matched privilege");
if (!entry->MatchesAuthMode(subjectDescriptor.authMode))
continue;


This comment was generated by todo based on a TODO comment in 1a570d3 in #10579. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 27, 2021

handle CASE Authenticated Tags (CAT1/CAT2)

// TODO: handle CASE Authenticated Tags (CAT1/CAT2)
if (p->node == subject.node)
{
return true;
}
}
}
return false;
}
bool MatchesTarget(EndpointId endpoint, ClusterId cluster) const override


This comment was generated by todo based on a TODO comment in 1a570d3 in #10579. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 27, 2021

Consider making this a class and the various utility methods static

// TODO: Consider making this a class and the various utility methods static
// methods.
using PasscodeId = uint16_t;
constexpr PasscodeId kDefaultCommissioningPasscodeId = 0;
} // namespace chip


This comment was generated by todo based on a TODO comment in 1a570d3 in #10579. cc @mlepage-google.

Copy link
Contributor

@bzbarsky-apple bzbarsky-apple left a comment

Choose a reason for hiding this comment

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

Looks great, modulo the one outstanding comment question in #10579 (comment)

@todo
Copy link

todo bot commented Oct 27, 2021

check error (but can't until we have an implementation)

// TODO: check error (but can't until we have an implementation)
#if 0
ReturnErrorCodeIf(iterator == nullptr, CHIP_ERROR_INTERNAL);
#else
ReturnErrorCodeIf(iterator == nullptr, CHIP_NO_ERROR);
#endif
// TODO: a few more cases (PASE commissioning, CASE Authenticated Tags, etc.)
while (auto entry = iterator->Next())
{


This comment was generated by todo based on a TODO comment in 3efcbd4 in #10579. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 27, 2021

a few more cases (PASE commissioning, CASE Authenticated Tags, etc.)

// TODO: a few more cases (PASE commissioning, CASE Authenticated Tags, etc.)
while (auto entry = iterator->Next())
{
ChipLogDetail(DataManagement, "Checking entry");
if (!entry->MatchesPrivilege(privilege))
continue;
ChipLogDetail(DataManagement, " --> matched privilege");
if (!entry->MatchesAuthMode(subjectDescriptor.authMode))
continue;


This comment was generated by todo based on a TODO comment in 3efcbd4 in #10579. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 27, 2021

handle CASE Authenticated Tags (CAT1/CAT2)

// TODO: handle CASE Authenticated Tags (CAT1/CAT2)
if (p->node == subject.node)
{
return true;
}
}
}
return false;
}
bool MatchesTarget(EndpointId endpoint, ClusterId cluster) const override


This comment was generated by todo based on a TODO comment in 3efcbd4 in #10579. cc @mlepage-google.

@todo
Copy link

todo bot commented Oct 27, 2021

Consider making this a class and the various utility methods static

// TODO: Consider making this a class and the various utility methods static
// methods.
using PasscodeId = uint16_t;
constexpr PasscodeId kDefaultCommissioningPasscodeId = 0;
} // namespace chip


This comment was generated by todo based on a TODO comment in 3efcbd4 in #10579. cc @mlepage-google.

@andy31415 andy31415 merged commit d3e76ec into project-chip:master Oct 27, 2021
JasonLiuZhuoCheng pushed a commit to JasonLiuZhuoCheng/connectedhomeip that referenced this pull request Oct 28, 2021
* Add initial prototype of AccessControl module

- Not complete, always allows actions
- Not hooked up to interaction model or messaging layer
- Progress toward issues project-chip#10236 and project-chip#10249
- Fully isolated as a module
- Has unit tests

* Remove file comments from files

* Add 'k' prefix to enum values

* Restyled by whitespace

* Restyled by clang-format

* Restyled by gn

* Remove "empty" .cpp files

* Apply suggestions from code review

* Apply suggestions from code review

* Fix compatibility under different compilers

* Fix unit test compatability on different compilers

* Restyled by clang-format

* Change forward declaration to include

Allows tooling to detect circular dependencies.

* Changes from code review suggestions

- rename namespace access --> Access
- rename DataProvider --> AccessControlDataProvider
- decouple DataProvider lifecycle (Init/Finish)
- rename DataProviderImpl --> ExampleAccessControlDataProvider
- change GetInstance/SetInstance to global functions
- remove Config.h since global instance must be set
- change EntryIterator::Next to return pointer
- add comments to Privilege and AuthMode
- remove SubjectDescriptor.isCommissioning for now
- improve naming of CAT subjects in SubjectDescriptor
- change SubjectId typedef to use NodeId

* Make tests table-driven

Should also fix some build complaints on ESP32

* Restyled by clang-format

* Restyle

* Add more metatesting

Ensure not just that results are correct, but that they were correctly
obtained.

* Restyled by clang-format

* Change enums to enum classes

* Address review comments

* Use basic types in lib/core

Basic types (FabricIndex etc.) were moved in PR project-chip#10925 from app to
lib/core, so now they can be used from this module.

* A bit of cleanup

* Refactor SubjectId and SubjectDescriptor

Also, remove CatId and move PasscodeId into lib/core.

* Restyled by clang-format

* Add clarifying examples to documentation.

Co-authored-by: Restyled.io <[email protected]>
carol-apple pushed a commit to carol-apple/connectedhomeip that referenced this pull request Oct 28, 2021
* Add initial prototype of AccessControl module

- Not complete, always allows actions
- Not hooked up to interaction model or messaging layer
- Progress toward issues project-chip#10236 and project-chip#10249
- Fully isolated as a module
- Has unit tests

* Remove file comments from files

* Add 'k' prefix to enum values

* Restyled by whitespace

* Restyled by clang-format

* Restyled by gn

* Remove "empty" .cpp files

* Apply suggestions from code review

* Apply suggestions from code review

* Fix compatibility under different compilers

* Fix unit test compatability on different compilers

* Restyled by clang-format

* Change forward declaration to include

Allows tooling to detect circular dependencies.

* Changes from code review suggestions

- rename namespace access --> Access
- rename DataProvider --> AccessControlDataProvider
- decouple DataProvider lifecycle (Init/Finish)
- rename DataProviderImpl --> ExampleAccessControlDataProvider
- change GetInstance/SetInstance to global functions
- remove Config.h since global instance must be set
- change EntryIterator::Next to return pointer
- add comments to Privilege and AuthMode
- remove SubjectDescriptor.isCommissioning for now
- improve naming of CAT subjects in SubjectDescriptor
- change SubjectId typedef to use NodeId

* Make tests table-driven

Should also fix some build complaints on ESP32

* Restyled by clang-format

* Restyle

* Add more metatesting

Ensure not just that results are correct, but that they were correctly
obtained.

* Restyled by clang-format

* Change enums to enum classes

* Address review comments

* Use basic types in lib/core

Basic types (FabricIndex etc.) were moved in PR project-chip#10925 from app to
lib/core, so now they can be used from this module.

* A bit of cleanup

* Refactor SubjectId and SubjectDescriptor

Also, remove CatId and move PasscodeId into lib/core.

* Restyled by clang-format

* Add clarifying examples to documentation.

Co-authored-by: Restyled.io <[email protected]>
PSONALl pushed a commit to PSONALl/connectedhomeip that referenced this pull request Dec 3, 2021
* Add initial prototype of AccessControl module

- Not complete, always allows actions
- Not hooked up to interaction model or messaging layer
- Progress toward issues project-chip#10236 and project-chip#10249
- Fully isolated as a module
- Has unit tests

* Remove file comments from files

* Add 'k' prefix to enum values

* Restyled by whitespace

* Restyled by clang-format

* Restyled by gn

* Remove "empty" .cpp files

* Apply suggestions from code review

* Apply suggestions from code review

* Fix compatibility under different compilers

* Fix unit test compatability on different compilers

* Restyled by clang-format

* Change forward declaration to include

Allows tooling to detect circular dependencies.

* Changes from code review suggestions

- rename namespace access --> Access
- rename DataProvider --> AccessControlDataProvider
- decouple DataProvider lifecycle (Init/Finish)
- rename DataProviderImpl --> ExampleAccessControlDataProvider
- change GetInstance/SetInstance to global functions
- remove Config.h since global instance must be set
- change EntryIterator::Next to return pointer
- add comments to Privilege and AuthMode
- remove SubjectDescriptor.isCommissioning for now
- improve naming of CAT subjects in SubjectDescriptor
- change SubjectId typedef to use NodeId

* Make tests table-driven

Should also fix some build complaints on ESP32

* Restyled by clang-format

* Restyle

* Add more metatesting

Ensure not just that results are correct, but that they were correctly
obtained.

* Restyled by clang-format

* Change enums to enum classes

* Address review comments

* Use basic types in lib/core

Basic types (FabricIndex etc.) were moved in PR project-chip#10925 from app to
lib/core, so now they can be used from this module.

* A bit of cleanup

* Refactor SubjectId and SubjectDescriptor

Also, remove CatId and move PasscodeId into lib/core.

* Restyled by clang-format

* Add clarifying examples to documentation.

Co-authored-by: Restyled.io <[email protected]>
@mlepage-google mlepage-google deleted the access-control-module-prototype 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.

9 participants