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

manifest: Update Matter SDK revision to pull new crypto changes #13540

Merged

rlubos merged 1 commit into nrfconnect:main from ArekBalysNordic:crypto_migration

Jan 25, 2024

Contributor

ArekBalysNordic commented Jan 4, 2024

In this commit added a mechanism that migrates all existing operational keys from mbedTLS-related settings storage to the PSA ITS secure storage.

After the migration the entries in the settings storage will not be avaliable anymore.

ArekBalysNordic requested review from LuDuda, kkasperczyk-no, wiba-nordic and markaj-nordic

January 4, 2024 13:47

ArekBalysNordic requested review from carlescufi and tejlmand as code owners

January 4, 2024 13:47

github-actions bot added doc-required manifest labels

NordicBuilder added manifest-matter DNM labels

Contributor

NordicBuilder commented Jan 4, 2024 •

edited

Loading

The following west manifest projects have been modified in this Pull Request:

Name	Old Revision	New Revision	Diff
matter	nrfconnect/sdk-connectedhomeip@`b6a5e9a`	nrfconnect/sdk-connectedhomeip@`288b64a` (master)	nrfconnect/[email protected]

Note: This message is automatically posted and updated by the Manifest GitHub Action.

Contributor

NordicBuilder commented Jan 4, 2024 •

edited

Loading

Test specification

CI/Jenkins/NRF

Integration Platforms

CI/Jenkins/integration

Test Module	File based changes	Manually selected	West overwrite
test-fw-nrfconnect-chip	X

test-fw-nrfconnect-chip: added because there was no .github/test-spec.yml in 'matter'

Detailed information of selected test modules

Note: This message is automatically posted and updated by the CI

Contributor

NordicBuilder commented Jan 4, 2024

You can find the documentation preview for this PR at this link. It will be updated about 10 minutes after the documentation build succeeds.

Note: This comment is automatically posted by the Documentation Publishing GitHub Action.

wiba-nordic requested changes

View reviewed changes

doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst Outdated Show resolved Hide resolved

ArekBalysNordic force-pushed the crypto_migration branch from 53d55d8 to ca05581 Compare

January 5, 2024 08:08

wiba-nordic approved these changes

View reviewed changes

LuDuda approved these changes

View reviewed changes

ArekBalysNordic force-pushed the crypto_migration branch from ca05581 to 64cc882 Compare

January 8, 2024 09:23

ArekBalysNordic force-pushed the crypto_migration branch from 64cc882 to ec6d2ca Compare

January 16, 2024 13:40

ArekBalysNordic requested a review from Damian-Nordic as a code owner

January 16, 2024 13:40

Contributor Author

ArekBalysNordic commented Jan 16, 2024

@wiba-nordic I've added some lines to the migration guide, release notes, and Matter index. We need to inform our customers that they must use PSA Crypto API to use Thread certification inheritance. In the other way, they need to build Thread libraries from sources. Could you please check the recent changes?

markaj-nordic approved these changes

View reviewed changes

doc/nrf/releases_and_maturity/migration/migration_guide_2.6.rst Outdated Show resolved Hide resolved

wiba-nordic requested changes

View reviewed changes

doc/nrf/protocols/matter/index.rst Outdated Show resolved Hide resolved

doc/nrf/protocols/matter/index.rst Outdated Show resolved Hide resolved

doc/nrf/protocols/matter/index.rst Outdated Show resolved Hide resolved

doc/nrf/protocols/matter/index.rst Outdated Show resolved Hide resolved

doc/nrf/releases_and_maturity/migration/migration_guide_2.6.rst Outdated Show resolved Hide resolved

doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst Outdated Show resolved Hide resolved

doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst Outdated Show resolved Hide resolved

doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst Outdated Show resolved Hide resolved

doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst Outdated Show resolved Hide resolved

doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst Outdated Show resolved Hide resolved

ArekBalysNordic force-pushed the crypto_migration branch from ec6d2ca to 42390da Compare

January 16, 2024 14:42

wiba-nordic requested changes

View reviewed changes

doc/nrf/releases_and_maturity/migration/migration_guide_2.6.rst Outdated Show resolved Hide resolved

doc/nrf/releases_and_maturity/migration/migration_guide_2.6.rst Show resolved Hide resolved

Contributor

wiba-nordic commented Jan 17, 2024

I am not sure what is breaking the documentation build.

kkasperczyk-no reviewed

View reviewed changes

doc/nrf/protocols/matter/index.rst Outdated Show resolved Hide resolved

doc/nrf/releases_and_maturity/migration/migration_guide_2.6.rst Outdated Show resolved Hide resolved

doc/nrf/releases_and_maturity/migration/migration_guide_2.6.rst Outdated Show resolved Hide resolved

ArekBalysNordic force-pushed the crypto_migration branch from 42390da to 4953d80 Compare

January 19, 2024 12:10

ArekBalysNordic requested review from wiba-nordic and kkasperczyk-no

January 19, 2024 12:13

wiba-nordic requested changes

View reviewed changes

doc/nrf/releases_and_maturity/releases/release-notes-changelog.rst Outdated Show resolved Hide resolved

kkasperczyk-no approved these changes

View reviewed changes

Contributor

kkasperczyk-no left a comment

I left one comment to consider. I don't have very strong opinion on that and I'm not going to insist, so approving.

Damian-Nordic approved these changes

View reviewed changes

doc/nrf/releases_and_maturity/migration/migration_guide_2.6.rst Outdated Show resolved Hide resolved

doc/nrf/releases_and_maturity/migration/migration_guide_2.6.rst Outdated Show resolved Hide resolved

ArekBalysNordic force-pushed the crypto_migration branch 4 times, most recently from 28d6116 to fccd5cc Compare

January 22, 2024 15:18

wiba-nordic approved these changes

View reviewed changes

ArekBalysNordic force-pushed the crypto_migration branch from fccd5cc to 0362789 Compare

January 23, 2024 07:13

kkasperczyk-no reviewed

View reviewed changes

samples/matter/common/src/app/migration_manager.cpp Outdated Show resolved Hide resolved

samples/matter/common/src/app/migration_manager.h Outdated Show resolved Hide resolved

samples/matter/light_bulb/CMakeLists.txt Outdated Show resolved Hide resolved

samples/matter/common/src/app/migration_manager.cpp Outdated Show resolved Hide resolved

samples/matter/common/src/app/migration_manager.cpp Outdated Show resolved Hide resolved

samples/matter/common/src/app/migration_manager.cpp Outdated Show resolved Hide resolved

ArekBalysNordic force-pushed the crypto_migration branch 3 times, most recently from 165b004 to 4f526f0 Compare

January 23, 2024 10:45

kkasperczyk-no approved these changes

View reviewed changes

Contributor

kkasperczyk-no left a comment •

edited

Loading

Ok, last round of comments from me. These are mostly minors, so approving in advance.

samples/matter/common/src/Kconfig Outdated Show resolved Hide resolved

samples/matter/common/src/migration/migration_manager.cpp Outdated Show resolved Hide resolved

samples/matter/common/src/migration/migration_manager.cpp Outdated Show resolved Hide resolved

samples/matter/common/src/migration/migration_manager.cpp Outdated Show resolved Hide resolved

samples/matter/common/src/migration/migration_manager.cpp

    
              		if (CHIP_NO_ERROR != err) {

              			chip::Server::GetInstance().ScheduleFactoryReset();

              			// Return a success to not block the Matter event Loop and allow to call scheduled factory reset.

              			err = CHIP_NO_ERROR;

Contributor

kkasperczyk-no Jan 24, 2024

Is it ok to return CHIP_NO_ERROR if migration actually failed?

Contributor Author

ArekBalysNordic Jan 24, 2024

I will add documentation to the API.

samples/matter/common/src/app/matter_event_handler.cpp Outdated Show resolved Hide resolved

ArekBalysNordic force-pushed the crypto_migration branch 2 times, most recently from d6daf62 to cf89a1a Compare

January 25, 2024 09:22

NordicBuilder removed the DNM label


          manifest: Update Matter SDK revision to pull new crypto changes

In this commit added a mechanism that migrates all existing
operational keys from mbedTLS-related settings storage to the
PSA ITS secure storage.

After the migration the entries in the settings storage will not
be avaliable anymore.

From NCS 2.6.0 the Matter supports Arm PSA Crypto API by default,
and mbedTLS is deprecated.

We should not use CommonServerInitParams to initialize PSA crypto
and we should do it directly in the Application instead of
Chip::Server. To do it we moved PSA init and declaration from
the Chip::Server to the Nordic's Matter Init.

Signed-off-by: Arkadiusz Balys <[email protected]>

ArekBalysNordic force-pushed the crypto_migration branch from cf89a1a to 7159994 Compare

January 25, 2024 09:23

wiba-nordic approved these changes

View reviewed changes

rlubos merged commit 71d287f into nrfconnect:main

14 checks passed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

LuDuda LuDuda approved these changes

kkasperczyk-no kkasperczyk-no approved these changes

Damian-Nordic Damian-Nordic approved these changes

wiba-nordic wiba-nordic approved these changes

markaj-nordic markaj-nordic approved these changes

carlescufi Awaiting requested review from carlescufi

tejlmand Awaiting requested review from tejlmand

Labels

doc-required manifest manifest-matter