-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Introduce CommissionableDataProvider for discriminator and verifier #16020
Introduce CommissionableDataProvider for discriminator and verifier #16020
Conversation
- 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
PR #16020: Size comparison from c49b115 to 82b4b25 Increases above 0.2%:
Increases (28 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (26 builds for cyw30739, efr32, esp32, k32w, linux, mbed, p6)
Full report (28 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #16020: Size comparison from c49b115 to 19194fa Increases above 0.2%:
Increases (30 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, p6)
Full report (30 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #16020: Size comparison from c49b115 to 0fa9255 Increases above 0.2%:
Increases (30 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, p6)
Full report (30 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
PR #16020: Size comparison from c49b115 to f36fc92 Increases above 0.2%:
Increases (30 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, p6)
Full report (30 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have some further suggestions and there's one duplication in BUILD.gn, but looking good overall.
Will fix as a follow-up today |
PR #16020: Size comparison from 31037c1 to 3f7ed46 Increases above 0.2%:
Increases (30 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
Decreases (27 builds for cyw30739, efr32, esp32, k32w, linux, mbed, p6)
Full report (30 builds for cyw30739, efr32, esp32, k32w, linux, mbed, nrfconnect, p6, telink)
|
…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>
Problem
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.
Fixes #15543
Change overview
is known to the ConfigurationManager.
CommissionableDataProvider in the short term
providing externally generated SPAKE2+ verifier/salt/iterations
get discriminator/SPAKE2+ parameters
Testing
chip_use_transitional_commissionable_data_provider=0
properly causes targets that don't provide an implementation
from running.
all-clusters-app allow passing custom version of verifier or iteration
counts.