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 verifier interface to ConfigurationManager to allow per-device PAKE Verifier #15543

Closed
tcarmelveilleux opened this issue Feb 24, 2022 · 0 comments · Fixed by #16020
Closed

Comments

@tcarmelveilleux
Copy link
Contributor

Problem

Spec requires every device to have a completely different PAKE verifier (every unit). This is usually achieved by access to a factory process which cannot be expressed unambiguously in Matter SDK. Current ConfigurationManager expects verifier to be writable and accessible, and passcode to be writable and accessible from same flash/location (which is technically forbidden by spec).

There must be a method to allow a product to provide access to the current PAKE verifier without requiring it to be written by going through example debug interfaces of Matter SDK samples.

Proposed Solution

Extract all verifier/passcode-related methods of ConfigurationManager into a new interface whose implementation is owned by products/platforms, while still retaining an example version to support SDK examples.

tcarmelveilleux added a commit to tcarmelveilleux/connectedhomeip that referenced this issue Mar 9, 2022
- Current API in ConfigurationManager makes it very hard to provide
  spec-mandated per-device SPAKE2+ parameters (verifier, salt, iteration
  counts) and discovery discriminator, since it expects "in-app" common
  SDK handling of the data for something that actually usually
  comes from the factory.

This PR does the following:

- Adds CommissionableDataProvider interface, of which an instance
  is known to the ConfigurationManager.
- Adds a legacy path to avoid requiring setting a
  CommissionableDataProvider in the short term
- A linux implementation of CommissionableDataProvider showcasing
  providing externally generated SPAKE2+ verifier/salt/iterations
- Refactoring of all direct usage of ConfigurationManager to
  get discriminator/SPAKE2+ parameters

Testing done:
- Cert tests still pass with no change
- All unit tests still pass, including those modified
- Setting GN arg `chip_use_transitional_commissionable_data_provider=0`
  properly causes targets that don't provide an implementation
  from running.
- Manually verified the new Linux command-line arguments to
  all-clusters-app allow passing custom version of verifier or iteration
  counts.

Fixes project-chip#15543
tcarmelveilleux added a commit that referenced this issue Mar 11, 2022
…16020)

* Introduce CommissionableDataProvider for discriminator and verifier

- Current API in ConfigurationManager makes it very hard to provide
  spec-mandated per-device SPAKE2+ parameters (verifier, salt, iteration
  counts) and discovery discriminator, since it expects "in-app" common
  SDK handling of the data for something that actually usually
  comes from the factory.

This PR does the following:

- Adds CommissionableDataProvider interface, of which an instance
  is known to the ConfigurationManager.
- Adds a legacy path to avoid requiring setting a
  CommissionableDataProvider in the short term
- A linux implementation of CommissionableDataProvider showcasing
  providing externally generated SPAKE2+ verifier/salt/iterations
- Refactoring of all direct usage of ConfigurationManager to
  get discriminator/SPAKE2+ parameters

Testing done:
- Cert tests still pass with no change
- All unit tests still pass, including those modified
- Setting GN arg `chip_use_transitional_commissionable_data_provider=0`
  properly causes targets that don't provide an implementation
  from running.
- Manually verified the new Linux command-line arguments to
  all-clusters-app allow passing custom version of verifier or iteration
  counts.

Fixes #15543

* Restyled by clang-format

* Restyled by gn

* Fix PRIu32

* Fix more presubmit rework failures

* Fix one missing override

* Address review comments

* Restyled by clang-format

* Fix more CI, do more renames

* Restyled by clang-format

* Address review comments

* Restyled by clang-format

* Restyled by gn

* Fix doxygen stuff

* Fix fake ConfigurationaManagerImpl.h

* Fix cirque

* Address review comments from PR #16020

* Fix CI after refactor based on review comments

Co-authored-by: Restyled.io <commits@restyled.io>
andrei-menzopol pushed a commit to andrei-menzopol/connectedhomeip that referenced this issue Apr 14, 2022
…roject-chip#16020)

* Introduce CommissionableDataProvider for discriminator and verifier

- Current API in ConfigurationManager makes it very hard to provide
  spec-mandated per-device SPAKE2+ parameters (verifier, salt, iteration
  counts) and discovery discriminator, since it expects "in-app" common
  SDK handling of the data for something that actually usually
  comes from the factory.

This PR does the following:

- Adds CommissionableDataProvider interface, of which an instance
  is known to the ConfigurationManager.
- Adds a legacy path to avoid requiring setting a
  CommissionableDataProvider in the short term
- A linux implementation of CommissionableDataProvider showcasing
  providing externally generated SPAKE2+ verifier/salt/iterations
- Refactoring of all direct usage of ConfigurationManager to
  get discriminator/SPAKE2+ parameters

Testing done:
- Cert tests still pass with no change
- All unit tests still pass, including those modified
- Setting GN arg `chip_use_transitional_commissionable_data_provider=0`
  properly causes targets that don't provide an implementation
  from running.
- Manually verified the new Linux command-line arguments to
  all-clusters-app allow passing custom version of verifier or iteration
  counts.

Fixes project-chip#15543

* Restyled by clang-format

* Restyled by gn

* Fix PRIu32

* Fix more presubmit rework failures

* Fix one missing override

* Address review comments

* Restyled by clang-format

* Fix more CI, do more renames

* Restyled by clang-format

* Address review comments

* Restyled by clang-format

* Restyled by gn

* Fix doxygen stuff

* Fix fake ConfigurationaManagerImpl.h

* Fix cirque

* Address review comments from PR project-chip#16020

* Fix CI after refactor based on review comments

Co-authored-by: Restyled.io <commits@restyled.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants