PSA ITS update #9192

orenc17 · 2018-12-24T13:00:33Z

Description

Update PSA-ITS to revision 1.0 as described here

Add Versioning
Change key uid type from uint32_t to psa_its_uid_t (uint64_t)
Adaptation for PSA-Crypto usage of ITS
Early implementation of PSA Lifecycle API's
Relies on Update PSA defines and headers #9214
Relies on Update usage of PSA ITS to comply with v1.0 mbed-crypto#23 -V
Relies on Use new ITS uid type mbed-crypto#30 - V

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[X] Breaking change

Reviewers

@alzix @davidsaada @Patater @moranpeker

ciarmcom · 2018-12-24T14:00:25Z

@orenc17, thank you for your changes.
@alzix @Patater @davidsaada @moranpeker@ARMmbed/mbed-os-maintainers please review.

davidsaada · 2018-12-24T14:15:35Z

components/TARGET_PSA/services/psa_prot_internal_storage/psa_prot_internal_storage.h

+#define PSA_ITS_ERROR_BAD_POINTER               5   /**<    The operation failed because one of the provided pointers is invalid, for example is `NULL` or references memory the caller cannot access */
+#define PSA_ITS_ERROR_KEY_NOT_FOUND             6   /**<    The operation failed because the provided key value was not found in the storage */
+#define PSA_ITS_ERROR_INCORRECT_SIZE            7   /**<    The operation failed because the data associated with provided key is not the same size as `data_size`, or `offset+data_size` is too large for the data, but `offset` is less than the size */
+#define PSA_ITS_ERROR_OFFSET_INVALID            8   /**<    The operation failed because an offset was supplied that is invalid for the existing data associated with the uid. For example, offset  is greater that the size of the data */


Gonna say here what I said in the PSA API repo: Some of the return codes here make the implementation inefficient. For instance, PSA_ITS_ERROR_OFFSET_INVALID imposes a redundant get_info call before get. I think that if we already have a breaking change, then we should revise the entire error codes list.

CC @dreemkiller

@davidsaada change requests better be sent via our spec repo.
This PR implements aggregated spec changes till now.

Commented in the spec repo as well (to the PR opened by @orenc17). My comment's aim here is to stress that a review to this PR won't be full until we sort out the changes to the error code list.

Can you please post a URL to your issue here?
It will allow us to keep track of it.
Thanks

It was a general comment in the PR #22 of the spec repo. I didn't elaborate regarding the problems, but they are mainly in the get implementation. I think that PSA_ITS_ERROR_OFFSET_INVALID return code is not required and that PSA_ITS_ERROR_INCORRECT_SIZE is already tested by KVStore, so no need to make the checks again.

alzix

ITS version used to store the file should be preserved. The reason for adding it was allowing future migrations between ITS versions.
I think it is better to save ITS version in flash and verify it upon first initialization.
@davidsaada @dreemkiller may I get your opinion on this?

mbed-ci · 2018-12-25T14:25:20Z

Test run: FAILED

Summary: 3 of 7 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-IAR

alzix · 2018-12-25T18:18:46Z

components/TARGET_PSA/services/psa_prot_internal_storage/COMPONENT_PSA_SRV_IMPL/pits_impl.cpp

+    its_version_t version = { 0, 0 };
+    size_t actual_size = 0;
+    KVStore::info_t kv_info;
+    int status = kvstore->get_info(ITS_VERSION_KEY, &kv_info);


is the call to kvstore->get_info really required?
it seems that we can deduce this info from kvstore->get return code and actual size.

just making sure that no one is trying to fake the size of the key

alzix · 2018-12-25T18:39:41Z

components/TARGET_PSA/services/psa_prot_internal_storage/COMPONENT_PSA_SRV_IMPL/pits_impl.cpp

+        version.minor = PSA_ITS_API_VERSION_MINOR;
+    }
+
+    if (kvstore->set(ITS_VERSION_KEY, &version, sizeof(version), 0) != MBED_SUCCESS) {


the set statement should be executed only when version file is not found or after migration.
rewriting it unconditionally is redundant.

components/TARGET_PSA/services/psa_prot_internal_storage/COMPONENT_PSA_SRV_IMPL/pits_impl.cpp

components/TARGET_PSA/services/psa_prot_internal_storage/COMPONENT_PSA_SRV_IMPL/pits_impl.h

mbed-ci · 2018-12-26T13:31:26Z

Test run: FAILED

Summary: 3 of 7 test jobs failed
Build number : 2
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-IAR

mbed-ci · 2018-12-27T12:56:27Z

Test run: FAILED

Summary: 3 of 7 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-IAR

mbed-ci · 2018-12-27T16:46:02Z

Test run: FAILED

Summary: 3 of 7 test jobs failed
Build number : 4
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_build-GCC_ARM
jenkins-ci/mbed-os-ci_build-ARM
jenkins-ci/mbed-os-ci_build-IAR

mbed-ci · 2018-12-30T14:05:45Z

Test run: FAILED

Summary: 2 of 11 test jobs failed
Build number : 5
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test
jenkins-ci/mbed-os-ci_exporter

0xc0170 · 2019-01-02T12:10:38Z

Please review test failures, related to the changes

orenc17 · 2019-01-02T14:20:15Z

@ARMmbed/mbed-os-maintainers ready for CI

0xc0170 · 2019-01-02T14:39:12Z

@ARMmbed/mbed-os-maintainers ready for CI

We can schedule a build once 5.11.1 RC passes (currently in the CI, and had already one issue so restarted again)

cmonr · 2019-01-03T02:15:44Z

CI started

mbed-ci · 2019-01-03T08:24:53Z

Test run: FAILED

Summary: 1 of 11 test jobs failed
Build number : 6
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

0xc0170 · 2019-01-03T08:27:37Z

2 failures there are related to the device rather than this PR, we will restart the job once the device is fixed

alzix

LGTM

cmonr · 2019-01-03T21:26:20Z

CI job restarted: jenkins-ci/mbed-os-ci_greentea-test

...nts/TARGET_PSA/services/psa_prot_internal_storage/COMPONENT_PSA_SRV_IMPL/pits_version_impl.h

dreemkiller · 2019-01-09T16:34:32Z

Why is the header file call psa_prot_internal_storage.h instead of psa/internal_trusted_storage.h?

Only accessible from SPE

orenc17 · 2019-01-29T15:51:36Z

@0xc0170 the breaking change in this PR is mainly the key type that has been chnaged

0xc0170 · 2019-02-04T16:15:58Z

Merged via #9529

ciarmcom requested review from alzix, davidsaada, moranpeker, Patater and a team December 24, 2018 14:00

ciarmcom added the needs: review label Dec 24, 2018

davidsaada reviewed Dec 24, 2018

View reviewed changes

alzix suggested changes Dec 24, 2018

View reviewed changes

alzix suggested changes Dec 25, 2018

View reviewed changes

alzix approved these changes Jan 3, 2019

View reviewed changes

cmonr added the BREAKING-CHANGE label Jan 3, 2019

cmonr requested a review from bulislaw January 3, 2019 21:27

0xc0170 requested changes Jan 4, 2019

View reviewed changes

...nts/TARGET_PSA/services/psa_prot_internal_storage/COMPONENT_PSA_SRV_IMPL/pits_version_impl.h Outdated Show resolved Hide resolved

cmonr added needs: work and removed needs: review labels Jan 4, 2019

orenc17 mentioned this pull request Jan 7, 2019

Fix travis astyle usage of .astyleignore #9267

Merged

orenc17 and others added 15 commits January 27, 2019 10:21

CR fixes

9f1dbc1

ITS 1.0

ac52a8b

Tests

4980fd2

PSA-Crypto fixes

011e63a

Replace psa_prot_internal_storage.h with psa/internal_trusted_storage.h

78153ee

Add FUTURE_SEQUANA prebuilt binaries

8d3ba3a

Remove ITS Reset partitions from tests

5cff49c

Add new ITS reset SID

200dc5a

Only accessible from SPE

Add new platform partition

f291781

Update tests to use new APIs

ade0608

Update ITS test

f0d550e

Update FUTURE_SEQUANA_PSA secure_binaries

08e99a2

Fix Typo

ee843a9

Regenerate SPM data

9677d27

Last update

0d48d98

mikisch81 mentioned this pull request Jan 27, 2019

Add ARM_MUSCA_A1 target #9221

Merged

Revert crypto changes

5b96557

NirSonnenschein mentioned this pull request Jan 28, 2019

crypto: Update to Mbed Crypto 1.0.0d1 #9463

Closed

Patater mentioned this pull request Jan 28, 2019

Crypto: Update to Mbed Crypto 1.0.0d2 #9517

Closed

cmonr added needs: work and removed needs: CI labels Jan 28, 2019

orenc17 mentioned this pull request Jan 29, 2019

RollUp PR Crypto with ITS #9529

Merged

orenc17 mentioned this pull request Jan 30, 2019

Update PSA defines and headers #9214

Closed

cmonr mentioned this pull request Jan 31, 2019

PSA compliance tests suite #9312

Merged

0xc0170 closed this Feb 4, 2019

0xc0170 removed needs: work BREAKING-CHANGE release-version: 5.12.0-rc1 labels Feb 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PSA ITS update #9192

PSA ITS update #9192

orenc17 commented Dec 24, 2018 •

edited

Loading

ciarmcom commented Dec 24, 2018

davidsaada Dec 24, 2018

alzix Dec 24, 2018

davidsaada Dec 24, 2018

alzix Dec 24, 2018

davidsaada Dec 24, 2018

alzix left a comment •

edited

Loading

mbed-ci commented Dec 25, 2018

alzix Dec 25, 2018

orenc17 Dec 26, 2018

alzix Dec 25, 2018

mbed-ci commented Dec 26, 2018

mbed-ci commented Dec 27, 2018

mbed-ci commented Dec 27, 2018

mbed-ci commented Dec 30, 2018

0xc0170 commented Jan 2, 2019

orenc17 commented Jan 2, 2019

0xc0170 commented Jan 2, 2019

cmonr commented Jan 3, 2019

mbed-ci commented Jan 3, 2019

0xc0170 commented Jan 3, 2019

alzix left a comment

cmonr commented Jan 3, 2019

dreemkiller commented Jan 9, 2019

orenc17 commented Jan 29, 2019

0xc0170 commented Feb 4, 2019

PSA ITS update #9192

PSA ITS update #9192

Conversation

orenc17 commented Dec 24, 2018 • edited Loading

Description

Pull request type

Reviewers

ciarmcom commented Dec 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alzix left a comment • edited Loading

Choose a reason for hiding this comment

mbed-ci commented Dec 25, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbed-ci commented Dec 26, 2018

mbed-ci commented Dec 27, 2018

mbed-ci commented Dec 27, 2018

mbed-ci commented Dec 30, 2018

0xc0170 commented Jan 2, 2019

orenc17 commented Jan 2, 2019

0xc0170 commented Jan 2, 2019

cmonr commented Jan 3, 2019

mbed-ci commented Jan 3, 2019

0xc0170 commented Jan 3, 2019

alzix left a comment

Choose a reason for hiding this comment

cmonr commented Jan 3, 2019

dreemkiller commented Jan 9, 2019

orenc17 commented Jan 29, 2019

0xc0170 commented Feb 4, 2019

orenc17 commented Dec 24, 2018 •

edited

Loading

alzix left a comment •

edited

Loading