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.

Sign up for GitHub

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

Jump to bottom

CASESession does not make use of real IPK #15583

Closed
tcarmelveilleux opened this issue Feb 25, 2022 · 0 comments · Fixed by #16737
Closed

CASESession does not make use of real IPK #15583

tcarmelveilleux opened this issue Feb 25, 2022 · 0 comments · Fixed by #16737
Assignees
@tcarmelveilleux
Labels
secure channel security spec Mismatch between spec and implementation V1.0

Comments

@tcarmelveilleux
Copy link
Contributor

Problem

The Identity Protection Key (IPK) is a critical component of CASE security in Matter. SDK currently uses a fixed value of all zeros, rather than using the IPK logic from the spec

Proposed Solution

Implement IPK processing by:

  • Using current IPK for fabric from Group Data Provider when generating DestinationID in Sigma 1 for transmit
    • Recording which of the IPKs it was since it will be needed for subsequent key derivation steps
  • Properly matching DestinationID using the installed IPKs in processing Sigma1.
    • Recording which IPK was used on receipt when processing Sigma1, since it is required in steps after Sigma1 in several key derivations
@tcarmelveilleux tcarmelveilleux added secure channel V1.0 security spec Mismatch between spec and implementation V1_FC labels Feb 25, 2022
@tcarmelveilleux tcarmelveilleux self-assigned this Mar 28, 2022
tcarmelveilleux added a commit to tcarmelveilleux/connectedhomeip that referenced this issue Mar 28, 2022
@tcarmelveilleux
Implement CASE processing of IPK
46b290b 
Problem:
- IPK processing used placeholder IPK at innermost levels of
  CASE and never used the values set in GroupDataProvider as
  set by Commissioner with AddNOC or KeySetWrite cluster
  commands.
- IPK Processing requires significant state keeping by
  controllers and commissioners to work, and none of the
  plumbing existed

Changes:
- Fixed NOC Cluster setting of IPK in AddNOC that was using
  the wrong compressed fabric ID.
- Properly use GroupDataProvider to get IPK for controllers
  sending Sigma1
- Properly iterate through all IPK on receiving Sigma1, from
  GroupDataProvider
- Add plumbing to properly initialize GroupDataProvider at
  CHIPDeviceControllerFactory and Server
- Added a central point for a default IPK, that is at the very
  outermost level, rather than innermost level. A follow-up
  PR will allow reconfiguration of it for CHIP-tool. Code paths
  will properly use the OperationalCredentialsDelegate's IPK
  value passed in Callback, if you it's provided. Controllers and
  commissioners can setup their GroupDataProvider context to
  properly use the right group keys even without the
  follow-up to allow non-default IPK in chip-tool.
- Cleaned-up the loose ends around all the injection points

Testing done:
- Updated all necessary Unit tests
- Added IPK and Destination ID unit test cases from spec
- All cert tests still pass
- All unit tests still pass

Fixes project-chip#15583
@tcarmelveilleux tcarmelveilleux mentioned this issue Mar 28, 2022
Merged
tcarmelveilleux added a commit that referenced this issue Mar 30, 2022
@tcarmelveilleux @restyled-commits
@msandstedt @bzbarsky-apple
Implement CASE processing of IPK (#16737)
1c03d9c 
* Implement CASE processing of IPK

Problem:
- IPK processing used placeholder IPK at innermost levels of
  CASE and never used the values set in GroupDataProvider as
  set by Commissioner with AddNOC or KeySetWrite cluster
  commands.
- IPK Processing requires significant state keeping by
  controllers and commissioners to work, and none of the
  plumbing existed

Changes:
- Fixed NOC Cluster setting of IPK in AddNOC that was using
  the wrong compressed fabric ID.
- Properly use GroupDataProvider to get IPK for controllers
  sending Sigma1
- Properly iterate through all IPK on receiving Sigma1, from
  GroupDataProvider
- Add plumbing to properly initialize GroupDataProvider at
  CHIPDeviceControllerFactory and Server
- Added a central point for a default IPK, that is at the very
  outermost level, rather than innermost level. A follow-up
  PR will allow reconfiguration of it for CHIP-tool. Code paths
  will properly use the OperationalCredentialsDelegate's IPK
  value passed in Callback, if you it's provided. Controllers and
  commissioners can setup their GroupDataProvider context to
  properly use the right group keys even without the
  follow-up to allow non-default IPK in chip-tool.
- Cleaned-up the loose ends around all the injection points

Testing done:
- Updated all necessary Unit tests
- Added IPK and Destination ID unit test cases from spec
- All cert tests still pass
- All unit tests still pass

Fixes #15583

* Restyled by clang-format

* Fix a few remaining TODOs

* First pass of fixing leftover CI

* Restyled by clang-format

* More CI fixes

* Restyled by clang-format

* Apply suggestions from code review

Co-authored-by: Michael Sandstedt <[email protected]>

* Restyled by clang-format

* Revert pHYRate change

* Hook up IPKs on Darwin.

* Fix more CI on Python, Android, TV app

* Restyled by clang-format

* Apply review comment

* Fix Python repl

* Restyled by clang-format

* Fix shutdown of group data provider

* Fix Darwin and Android CI

* More fixing of Android CI

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Michael Sandstedt <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
rochaferraz pushed a commit to rochaferraz/connectedhomeip that referenced this issue Mar 31, 2022
@tcarmelveilleux @restyled-commits
@msandstedt @bzbarsky-apple @rochaferraz
Implement CASE processing of IPK (project-chip#16737)
e26032e 
* Implement CASE processing of IPK

Problem:
- IPK processing used placeholder IPK at innermost levels of
  CASE and never used the values set in GroupDataProvider as
  set by Commissioner with AddNOC or KeySetWrite cluster
  commands.
- IPK Processing requires significant state keeping by
  controllers and commissioners to work, and none of the
  plumbing existed

Changes:
- Fixed NOC Cluster setting of IPK in AddNOC that was using
  the wrong compressed fabric ID.
- Properly use GroupDataProvider to get IPK for controllers
  sending Sigma1
- Properly iterate through all IPK on receiving Sigma1, from
  GroupDataProvider
- Add plumbing to properly initialize GroupDataProvider at
  CHIPDeviceControllerFactory and Server
- Added a central point for a default IPK, that is at the very
  outermost level, rather than innermost level. A follow-up
  PR will allow reconfiguration of it for CHIP-tool. Code paths
  will properly use the OperationalCredentialsDelegate's IPK
  value passed in Callback, if you it's provided. Controllers and
  commissioners can setup their GroupDataProvider context to
  properly use the right group keys even without the
  follow-up to allow non-default IPK in chip-tool.
- Cleaned-up the loose ends around all the injection points

Testing done:
- Updated all necessary Unit tests
- Added IPK and Destination ID unit test cases from spec
- All cert tests still pass
- All unit tests still pass

Fixes project-chip#15583

* Restyled by clang-format

* Fix a few remaining TODOs

* First pass of fixing leftover CI

* Restyled by clang-format

* More CI fixes

* Restyled by clang-format

* Apply suggestions from code review

Co-authored-by: Michael Sandstedt <[email protected]>

* Restyled by clang-format

* Revert pHYRate change

* Hook up IPKs on Darwin.

* Fix more CI on Python, Android, TV app

* Restyled by clang-format

* Apply review comment

* Fix Python repl

* Restyled by clang-format

* Fix shutdown of group data provider

* Fix Darwin and Android CI

* More fixing of Android CI

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Michael Sandstedt <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this issue Apr 14, 2022
@tcarmelveilleux @restyled-commits
@msandstedt @bzbarsky-apple @andrei-menzopol
Implement CASE processing of IPK (project-chip#16737)
72b4bfe 
* Implement CASE processing of IPK

Problem:
- IPK processing used placeholder IPK at innermost levels of
  CASE and never used the values set in GroupDataProvider as
  set by Commissioner with AddNOC or KeySetWrite cluster
  commands.
- IPK Processing requires significant state keeping by
  controllers and commissioners to work, and none of the
  plumbing existed

Changes:
- Fixed NOC Cluster setting of IPK in AddNOC that was using
  the wrong compressed fabric ID.
- Properly use GroupDataProvider to get IPK for controllers
  sending Sigma1
- Properly iterate through all IPK on receiving Sigma1, from
  GroupDataProvider
- Add plumbing to properly initialize GroupDataProvider at
  CHIPDeviceControllerFactory and Server
- Added a central point for a default IPK, that is at the very
  outermost level, rather than innermost level. A follow-up
  PR will allow reconfiguration of it for CHIP-tool. Code paths
  will properly use the OperationalCredentialsDelegate's IPK
  value passed in Callback, if you it's provided. Controllers and
  commissioners can setup their GroupDataProvider context to
  properly use the right group keys even without the
  follow-up to allow non-default IPK in chip-tool.
- Cleaned-up the loose ends around all the injection points

Testing done:
- Updated all necessary Unit tests
- Added IPK and Destination ID unit test cases from spec
- All cert tests still pass
- All unit tests still pass

Fixes project-chip#15583

* Restyled by clang-format

* Fix a few remaining TODOs

* First pass of fixing leftover CI

* Restyled by clang-format

* More CI fixes

* Restyled by clang-format

* Apply suggestions from code review

Co-authored-by: Michael Sandstedt <[email protected]>

* Restyled by clang-format

* Revert pHYRate change

* Hook up IPKs on Darwin.

* Fix more CI on Python, Android, TV app

* Restyled by clang-format

* Apply review comment

* Fix Python repl

* Restyled by clang-format

* Fix shutdown of group data provider

* Fix Darwin and Android CI

* More fixing of Android CI

Co-authored-by: Restyled.io <[email protected]>
Co-authored-by: Michael Sandstedt <[email protected]>
Co-authored-by: Boris Zbarsky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tcarmelveilleux tcarmelveilleux

Labels
secure channel security spec Mismatch between spec and implementation V1.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Implement CASE processing of IPK tcarmelveilleux/connectedhomeip
1 participant
@tcarmelveilleux