Implement PSA protected storage & restructure PSA storage implementation #9708

davidsaada · 2019-02-13T13:43:46Z

Description

This PR's main purpose is the implementation of the PSA Protected Storage (PS) feature. As this implementation is almost identical to the one we already have in PSA's Internal Trusted Storage (ITS), it triggered a deeper change in the entire implementation of PSA storage.

So, changes included here are:

Move all PSA storage code under psa/storage directory
Create a global PSA error codes header, eliminating ITS specific ones (backed by a change in the PSA spec)
Create a common header file for PSA storage type definitions, eliminating ITS specific ones (backed by the same change in the PSA spec)
Create a common implementation for PS & ITS
Implement protected storage feature
Change ITS test to be common to PS as well
Modify affected PSA crypto code (temporary, for CI sake only)

Important notes:
- Changes in the mbed-crypto code, included in this PR, are temporary. Changes should eventually be taken from PR #55 in the mbed-crypto repo. They are included here for CI sake only. After both PRs are approved, they should be merged together.

This PR is based on the current PSA spec. There's an ongoing discussion regarding the way PSA get functions should behave that's still open. As we strive to push this PR for 5.12, we can't wait for this discussion to be resolved, but if it's resolved in a reasonable time frame, PR may change accordingly.

Tested with all PSA tests on K64F and FUTURE_SEQUANA boards.

Pull request type

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

Release Notes

Restructure PSA storage implementation (directory structure and common code)
Add the PSA protected storage feature

ciarmcom · 2019-02-13T14:00:38Z

@davidsaada, thank you for your changes.
@ARMmbed/mbed-os-tls @ARMmbed/mbed-os-crypto @ARMmbed/mbed-os-maintainers please review.

simonbutcher · 2019-02-13T14:25:01Z

This has no impact on @ARMmbed/mbed-os-tls so please remove as approvers.

0xc0170 · 2019-02-13T14:37:13Z

@davidsaada This will need release notes section https://os.mbed.com/docs/mbed-os/v5.11/contributing/workflow.html#functionality-change

davidsaada · 2019-02-13T15:16:39Z

This depends on #9702 now in order to fix travis issues.

mikisch81 · 2019-02-13T15:59:28Z

@davidsaada Can you look at #9653 and see what is the impact of the changes there on this PR (or the opposite, whichever is merged first)

cmonr · 2019-02-13T20:04:12Z

This depends on #9702 now in order to fix travis issues.

@davidsaada Travis issues?

davidsaada · 2019-02-13T20:07:08Z

This depends on #9702 now in order to fix travis issues.

@davidsaada Travis issues?

Should have been more specific: It failed the psa-autogen travis check before depending on that PR.

davidsaada · 2019-02-14T12:51:20Z

@davidsaada Can you look at #9653 and see what is the impact of the changes there on this PR (or the opposite, whichever is merged first)

@mikisch81 The PRs are almost orthogonal. They only collide in its_init, where you added get_its_kvstore_instance, which can easily be merged (in either direction).

davidsaada · 2019-02-14T13:38:02Z

Rebased, no longer depends on #9702 .
In addition, split the change into two commits, where mbed-crypto files are separated, so they can easily be removed from change once brought from mbed-crypto repo.

0xc0170 · 2019-02-18T15:50:40Z

Can you add "Release notes" section , following https://os.mbed.com/docs/mbed-os/v5.11/contributing/workflow.html#functionality-change

0xc0170 · 2019-02-18T15:52:04Z

Implement PSA protected storage & restructure PSA storage implementation - why was this squashed into one commit?

davidsaada · 2019-02-18T16:32:27Z

Implement PSA protected storage & restructure PSA storage implementation - why was this squashed into one commit?

These were never squashed - they were all part of the same commit from the beginning (the second commit only includes the mbed-crypto files, which should be discarded after CI is successful and before merge). This is also explained in the PR description: Most of the work includes the restructuring and turning the previous ITS code to become the common code. The Protected Storage is just a very thin layer on top of it.

Patater · 2019-02-18T16:45:18Z

Hi @davidsaada

Could you please remove Temporary commit for mbed-crypto files. and instead cherry-pick Patater@3638862 and then Patater@5c5b6b4 into this PR? We can update Mbed Crypto with the necessary modifications though this PR, now that the changes have landed in Mbed Crypto.

davidsaada · 2019-02-18T17:13:25Z

Hi @davidsaada

Could you please remove Temporary commit for mbed-crypto files. and instead cherry-pick Patater@3638862 and then Patater@5c5b6b4 into this PR? We can update Mbed Crypto with the necessary modifications though this PR, now that the changes have landed in Mbed Crypto.

Done. Temporary commit removed in favour of these two.
Note to @ARMmbed/mbed-os-maintainers: This will now fail build until rebased tomorrow on top of expected changes coming from mbed-crypto.

davidsaada · 2019-02-21T15:08:20Z

Waiting for final approval from @ARMmbed/mbed-os-storage

My team just hates me...

0xc0170 · 2019-02-21T15:37:32Z

@AnotherButler Review the docs please

cmonr · 2019-02-21T15:43:13Z

~~CI started~~

Whoops. Stopped the accidental restart.

cmonr · 2019-02-21T15:46:41Z

CI started

cmonr · 2019-02-21T15:47:08Z

Sneaky sneaky.

The success results were for the build before the rebase.

AnotherButler · 2019-02-21T16:37:17Z

@0xc0170 Please feel free to test and merge without our review. We do not have the bandwidth to review this much Doxygen at this point in time.

AnotherButler · 2019-02-21T16:39:47Z

@davidsaada Should any of these classes be added to our published, rendered documentation?

dannybenor · 2019-02-21T16:42:40Z

@AnotherButler A pointer to the spec of protected storage (like in ITS) will be good enough

davidsaada · 2019-02-21T16:52:35Z

@ARMmbed/mbed-os-maintainers Was just told that I needed to add few more deprecated definitions for our PSA compliance tests. This should have no effect on current code (only used by the external code). Will add them after CI finishes, so please don't stop it (so I can see if there's other stuff I need to fix if it fails). If it succeeds, please don't perform the merge yet.

cmonr · 2019-02-21T17:07:06Z

@davidsaada Thanks for the heads up. Will try to keep CI light today so that we can accelerate this through.

mbed-ci · 2019-02-21T18:28:53Z

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 3
Build artifacts

- Move all PSA storage code under psa/storage directory - Create a global PSA error codes header, eliminating ITS specific ones - Create a common header file for PSA storage type definitions, eliminating ITS specific ones - Create a common implementation for PS & ITS - Implement protected storage feature - Change ITS test to be common to PS as well

davidsaada · 2019-02-21T19:01:35Z

Pushed an update including deprecated ITS error codes (squashed into main commit) following tests success. Please rerun CI.

cmonr · 2019-02-21T19:11:38Z

CI started

mbed-ci · 2019-02-21T22:51:43Z

Test run: SUCCESS

Summary: 12 of 12 test jobs passed
Build number : 4
Build artifacts

davidsaada force-pushed the david_protected_storage branch 2 times, most recently from c98fd6a to a42794a Compare February 13, 2019 13:59

ciarmcom requested review from a team February 13, 2019 14:00

ciarmcom added the needs: review label Feb 13, 2019

NirSonnenschein requested a review from a team February 13, 2019 14:06

orenc17 approved these changes Feb 13, 2019

View reviewed changes

0xc0170 removed the request for review from a team February 13, 2019 14:35

0xc0170 added the release-version: 5.12.0-rc1 label Feb 13, 2019

davidsaada force-pushed the david_protected_storage branch from a42794a to 91ba024 Compare February 13, 2019 15:15

orenc17 mentioned this pull request Feb 13, 2019

Use new delivery for FUTURE_SEQUANA_PSA #9666

Merged

davidsaada mentioned this pull request Feb 14, 2019

Modify PSA related error codes and types ARMmbed/mbed-crypto#55

Merged

davidsaada force-pushed the david_protected_storage branch from 91ba024 to 9962a15 Compare February 14, 2019 13:36

davidsaada force-pushed the david_protected_storage branch from 9962a15 to 46a0d54 Compare February 18, 2019 12:38

davidsaada force-pushed the david_protected_storage branch from 46a0d54 to 3291fc3 Compare February 18, 2019 17:10

dannybenor approved these changes Feb 21, 2019

View reviewed changes

0xc0170 added ready for merge and removed needs: review labels Feb 21, 2019

0xc0170 requested a review from AnotherButler February 21, 2019 15:37

0xc0170 added needs: review and removed ready for merge labels Feb 21, 2019

cmonr added needs: CI and removed needs: review labels Feb 21, 2019

orenc17 mentioned this pull request Feb 21, 2019

PSA compliance tests suite #9312

Merged

David Saada and others added 3 commits February 21, 2019 20:58

Update crypto IPC handling of negative errors

922cdea

Add flash IAP to sequana_psa m4

3c5c205

davidsaada force-pushed the david_protected_storage branch from 9ab9f8b to 3c5c205 Compare February 21, 2019 18:59

cmonr merged commit 870bd05 into ARMmbed:master Feb 21, 2019

cmonr removed the needs: CI label Feb 21, 2019

davidsaada deleted the david_protected_storage branch March 16, 2019 08:19

Implement PSA protected storage & restructure PSA storage implementation #9708

Implement PSA protected storage & restructure PSA storage implementation #9708

Uh oh!

Conversation

davidsaada commented Feb 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Pull request type

Release Notes

Uh oh!

ciarmcom commented Feb 13, 2019

Uh oh!

simonbutcher commented Feb 13, 2019

Uh oh!

0xc0170 commented Feb 13, 2019

Uh oh!

davidsaada commented Feb 13, 2019

Uh oh!

mikisch81 commented Feb 13, 2019

Uh oh!

cmonr commented Feb 13, 2019

Uh oh!

davidsaada commented Feb 13, 2019

Uh oh!

davidsaada commented Feb 14, 2019

Uh oh!

davidsaada commented Feb 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

0xc0170 commented Feb 18, 2019

Uh oh!

0xc0170 commented Feb 18, 2019

Uh oh!

davidsaada commented Feb 18, 2019

Uh oh!

Patater commented Feb 18, 2019

Uh oh!

davidsaada commented Feb 18, 2019

Uh oh!

davidsaada commented Feb 21, 2019

Uh oh!

0xc0170 commented Feb 21, 2019

Uh oh!

cmonr commented Feb 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cmonr commented Feb 21, 2019

Uh oh!

cmonr commented Feb 21, 2019

Uh oh!

AnotherButler commented Feb 21, 2019

Uh oh!

AnotherButler commented Feb 21, 2019

Uh oh!

dannybenor commented Feb 21, 2019

Uh oh!

davidsaada commented Feb 21, 2019

Uh oh!

cmonr commented Feb 21, 2019

Uh oh!

mbed-ci commented Feb 21, 2019

Test run: SUCCESS

Uh oh!

davidsaada commented Feb 21, 2019

Uh oh!

cmonr commented Feb 21, 2019

Uh oh!

mbed-ci commented Feb 21, 2019

Test run: SUCCESS

Uh oh!

Uh oh!

davidsaada commented Feb 13, 2019 •

edited

Loading

davidsaada commented Feb 14, 2019 •

edited

Loading

cmonr commented Feb 21, 2019 •

edited

Loading