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

[nrf noup] Move DAC priv key from Factory Data to PSA ITS #372

Merged
merged 1 commit into from
Jan 4, 2024

Conversation

ArekBalysNordic
Copy link
Contributor

@ArekBalysNordic ArekBalysNordic commented Dec 29, 2023

We need a mechanism to move the DAC private key from the factory data set to PSA ITS NVM storage during the first boot of the device.
Then the DAC private key must be removed from the factory data set and protected by overwriting.

In this commit:

  • Added a method to FactoryDataProvider for moving and removing DAC from the factory data set.

  • Aligned the SignWithDeviceAttestationKey method to work with stored DAC private key in ITS NVM instead of raw data.

  • Extended the FactoryDataParser module by the Serialize method that is responsible for creating CBOR-formated factory data from the FactoryData struct.

  • Added a Kconfig to define the PSA ITS NVM offset for Matter keys.

  • x509 MBedTLS support seems to be not needed anymore - we can disable it and save ~20kB of FLASH.

  • Prevent the DAC private key from removal during the factory reset - for now, disable the CHIP_FACTORY_RESET_ERASE_NVS config by default.

config/nrfconnect/chip-module/Kconfig Outdated Show resolved Hide resolved
config/nrfconnect/chip-module/Kconfig Outdated Show resolved Hide resolved
src/platform/nrfconnect/CHIPPlatformConfig.h Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.c Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.c Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
@ArekBalysNordic ArekBalysNordic force-pushed the dac_removal branch 2 times, most recently from 0ec83d8 to 134c1d6 Compare January 2, 2024 08:18
src/platform/nrfconnect/FactoryDataProvider.h Outdated Show resolved Hide resolved
config/nrfconnect/chip-module/Kconfig.defaults Outdated Show resolved Hide resolved
config/nrfconnect/chip-module/Kconfig Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.c Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.c Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot added the crypto label Jan 2, 2024
@ArekBalysNordic
Copy link
Contributor Author

@Damian-Nordic @markaj-nordic @kkasperczyk-no I've removed serialization from the factory data parser and overwritten the dac private key directly from the factory data set. Please look at it once again :)

Copy link
Contributor

@Damian-Nordic Damian-Nordic left a comment

Choose a reason for hiding this comment

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

Found a few more things to remove or simplify.

config/nrfconnect/chip-module/Kconfig.defaults Outdated Show resolved Hide resolved
src/platform/nrfconnect/CHIPPlatformConfig.h Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.c Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.c Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.c Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.c Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
@ArekBalysNordic ArekBalysNordic force-pushed the dac_removal branch 3 times, most recently from 91b00dd to eeb097f Compare January 2, 2024 14:22
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.h Outdated Show resolved Hide resolved
@ArekBalysNordic
Copy link
Contributor Author

@Damian-Nordic @LuDuda I've provided the changes that were discussed F2F, now we have the config CHIP_CRYPTO_PSA_MIGRATE_DAC_PRIV_KEY to decide if we want to clear DAC or not, and I've changed a way of removing dac. Could you please verify the recent changes?

config/nrfconnect/chip-module/Kconfig Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
config/nrfconnect/chip-module/Kconfig Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.c Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataParser.c Outdated Show resolved Hide resolved
src/platform/nrfconnect/FactoryDataProvider.cpp Outdated Show resolved Hide resolved
@ArekBalysNordic ArekBalysNordic force-pushed the dac_removal branch 2 times, most recently from bf59f66 to 90cb1e8 Compare January 4, 2024 10:04
We need a mechanism to move the DAC private key from the
factory data set to PSA ITS NVM storage during the first
boot of the device.
Then the DAC private key must be removed from the factory
data set and protected by overwriting.

In this commit:
- Added a method to FactoryDataProvider for moving and
removing DAC from the factory data set.
- Aligned the SignWithDeviceAttestationKey method to work with
stored DAC priv key in ITS NVM instead of raw data.
- Added a Matter config to determine base address of ITS storage
for Matter purposes.
- x509 MBedTLS support seems to be not needed anymore
we can disable it and save ~20kB of FLASH.
- Prevent the DAC private key from removal during the
factory reset - for now, disable
the CHIP_FACTORY_RESET_ERASE_NVS config by default.
- Remove the DAC private key from the factory data set
only if the CONFIG_CHIP_CRYPTO_PSA_MIGRATE_DAC_PRIV_KEY
config is set to 'y'.
@ArekBalysNordic ArekBalysNordic merged commit 50a62b4 into nrfconnect:master Jan 4, 2024
4 of 8 checks passed
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.

5 participants