crypto: Update to Mbed Crypto 1.0.0d1 #9463

Patater · 2019-01-22T19:16:19Z

Description

Update Mbed Crypto to development version mbedcrypto-1.0.0d1 to facilitate rest-of-Mbed-OS integration with PSA Crypto API changes sooner.

In order to do this, we needed to break out the Mbed Crypto importing code from the Mbed TLS importer. This, in effect, adds a separate importer script for Mbed Crypto which can be used to directly import Mbed Crypto into Mbed OS.

Pull request type

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

Reviewers

CC @avolinski @alzix @sbutcher-arm

ciarmcom · 2019-01-22T20:00:34Z

@Patater, thank you for your changes.
@avolinski @sbutcher-arm @alzix @ARMmbed/mbed-os-crypto @ARMmbed/mbed-os-tls @ARMmbed/mbed-os-maintainers please review.

Patater · 2019-01-23T11:58:44Z

Rebased to correct explanation of NSPE and SPE.

Obtain the version of Mbed Crypto to use not from the Mbed TLS submodule, but independently through the Mbed Crypto importer instead.

Use the Mbed-Crypto-specific importer script to re-import Mbed Crypto 0.1.0b2 to its new location.

Instead of doing a "pull --rebase" to update to the latest development branch, do a "fetch" followed by a "checkout" to update to the specified release. This enables us to get any new tags created since the last update to the development branch, and removes the noise of updating a local "development" branch.

Patater · 2019-01-23T12:00:01Z

Rebased to remove stray period.

yanesca

Looks good to me.

Update tests in TESTS/mbed-crypto/sanity/main.cpp

Test key handles by adding a test to TESTS/mbed-crypto/sanity/main.cpp

Patater · 2019-01-25T15:40:03Z

Rebased to update to Mbed Crypto 1.0.0d1. Added new commits from @itayzafrir to update the Mbed OS crypto tests.

simonbutcher · 2019-01-27T23:46:48Z

@mpg - Could you please review this PR and check it's consistent with your architecture proposal?

mpg · 2019-01-28T09:58:34Z

@Patater I'm not that familiar with the importer, so I think it would help my review if you could describe at a high level what this does. Since you selected "refactor" as a PR type, I'm assuming the goal of this PR is to ensure that Mbed Crypto can be fetched from a different source (ie directly from the mbed-crypto repo rather than indirectly via the mbedtls repo and its submodule), but still be placed the same way in the Mbed OS source tree (ie still as a subdirectory of the mbedtls feature) and still built the same way (ie using config.h from the Mbed TLS tree and so on).

Can you confirm if that is the case, or clarify if I made wrong assumptions?

On an unrelated front, I'm getting the impression that updating to mbedcrypto 1.0.0d1 will break Mbed TLS, due to API changes that we haven't adapted to yet (and even if we had, we'd still need to import the relevant development version of Mbed TLS in Mbed OS for it to work). It looks like the CI isn't finding that issue so far, so perhaps it should be expanded as well?

NirSonnenschein · 2019-01-28T12:03:31Z

because of interdependency issues with PR#9192 (#9192) they will need to go through CI together (each will fail on it's own, but both should pass together). If they can be unified into a single PR this might make this easier.

Patater · 2019-01-28T12:13:15Z

@mpg

Can you confirm if that is the case, or clarify if I made wrong assumptions?

Yes, that's the case. At a high level, this PR essentially reverts the Mbed TLS importer to how it was before Mbed OS 5.11. It takes the changes we added to the Mbed TLS importer in Mbed OS 5.11 to import Mbed Crypto and places those changes into their own Makefile which solely imports Mbed Crypto. This enables us to do development releases of Mbed Crypto into Mbed OS to help give others time to integrate with it well ahead of the next major Mbed OS release.

I'm getting the impression that updating to mbedcrypto 1.0.0d1 will break Mbed TLS, due to API changes that we haven't adapted to yet.

Yes, that's also the case for when the disabled-by-default option MBEDTLS_USE_PSA_CRYPTO is set. After the API is stable, we will avoid breaking this configuration by further imports into Mbed OS. Any API breakage will become obvious when Mbed OS CI tests with the MBEDTLS_USE_PSA_CRYPTO option set begin failing.

Patater · 2019-01-28T12:21:42Z

@NirSonnenschein This PR doesn't depend on #9192 as it doesn't bring in ITS changes yet. I'll raise a new PR for both the ITS changes from #9192 as well as ARMmbed/mbed-crypto#23, so that the review and merge process is simpler for this PR.

NirSonnenschein · 2019-01-28T12:48:27Z

@Patater as long as there is no interdependency then this sounds like a good way forward.

NirSonnenschein · 2019-01-29T07:51:14Z

@Patater : I'm not sure if this requires final approval from the TLS team, but started CI in the mean time.

mikisch81 · 2019-01-29T09:19:26Z

@Patater looks like the PSA targets fail in compilation

mbed-ci · 2019-01-29T09:34:25Z

Test run: FAILED

Summary: 4 of 8 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

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

NirSonnenschein · 2019-01-29T09:35:51Z

Hi @Patater : FYI , CI ran into 2 failures at the build stage:

on sequana board compilation there were multiple build errors:
10:14:47 [FUTURE_SEQUANA_PSA:ARM] [DEBUG] Output: "./components/TARGET_PSA/services/crypto/COMPONENT_PSA_SRV_IPC/crypto_platform_spe.h", line 105: Error: CAN-Related Pull Request #20: identifier "psa_key_slot_t" is undefined
on K64F there was also an error:
10:18:58 [K64F:ARM] ################################################################################
10:18:58 [K64F:ARM] # Failed example combinations
10:18:58 [K64F:ARM] ################################################################################
10:18:58 [K64F:ARM] # mbed-os-example-mbed-crypto K64F ARM
10:18:58 [K64F:ARM] #
10:18:58 [K64F:ARM] ################################################################################
10:18:58 [K64F:ARM] Number of failures = 1

itayzafrir · 2019-01-29T10:18:07Z

@Patater compilation failed for target FUTURE_SEQUANA_PSA, this PR need additional commits from my PR #9409. We can cherry-pick all the commits from #9409 (except for 563701d which is the same as bd0037f from this PR) or only cherry-pick 3c10e5d.
If only cherry-pick 3c10e5d then function psa_hash_clone will return PSA_ERROR_NOT_SUPPORTED when doing crypto IPC.

mikisch81 · 2019-01-29T11:02:18Z

@Patater compilation failed for target FUTURE_SEQUANA_PSA, this PR need additional commits from my PR #9409. We can cherry-pick all the commits from #9409 (except for 563701d which is the same as bd0037f from this PR) or only cherry-pick 3c10e5d.
If only cherry-pick 3c10e5d then function psa_hash_clone will return PSA_ERROR_NOT_SUPPORTED when doing crypto IPC.

#9529 should have all the dependent PRs inside and tests should pass.

Individually it's not possible as all the PRs are mutually dependent.

Patater · 2019-01-29T20:45:12Z

Closing in favor of #9529

ciarmcom requested review from alzix, alekshex, simonbutcher and a team January 22, 2019 20:00

ciarmcom added the needs: review label Jan 22, 2019

cmonr added the release-version: 5.12.0-rc1 label Jan 23, 2019

0xc0170 approved these changes Jan 23, 2019

View reviewed changes

Patater force-pushed the update-mbedcrypto-1.0.0d0 branch from ad1a56f to adc242e Compare January 23, 2019 11:58

Patater added 3 commits January 23, 2019 11:59

tls/crypto: Make mbed-crypto importer independent

c64108d

Obtain the version of Mbed Crypto to use not from the Mbed TLS submodule, but independently through the Mbed Crypto importer instead.

crypto: Re-import Mbed Crypto 0.1.0b2

0f9f45f

Use the Mbed-Crypto-specific importer script to re-import Mbed Crypto 0.1.0b2 to its new location.

Patater force-pushed the update-mbedcrypto-1.0.0d0 branch from adc242e to e71374f Compare January 23, 2019 11:59

alzix mentioned this pull request Jan 23, 2019

Update PSA defines and headers #9214

Closed

yanesca approved these changes Jan 25, 2019

View reviewed changes

Patater and others added 3 commits January 25, 2019 15:37

crypto: Update to Mbed Crypto 1.0.0d1

9bcbfc5

crypto: Update tests to use key handles

d422fe8

Update tests in TESTS/mbed-crypto/sanity/main.cpp

crypto: Add test of key handles

bd0037f

Test key handles by adding a test to TESTS/mbed-crypto/sanity/main.cpp

Patater force-pushed the update-mbedcrypto-1.0.0d0 branch from e71374f to bd0037f Compare January 25, 2019 15:39

Patater changed the title ~~Update mbedcrypto 1.0.0d0~~ Update mbedcrypto 1.0.0d1 Jan 25, 2019

alzix approved these changes Jan 27, 2019

View reviewed changes

0xc0170 requested a review from mpg January 28, 2019 08:48

alekshex approved these changes Jan 28, 2019

View reviewed changes

mpg mentioned this pull request Jan 28, 2019

[DRAFT] Split generated files for error and version_features Mbed-TLS/mbedtls#2351

Closed

itayzafrir mentioned this pull request Jan 28, 2019

Crypto integration with SPM #9409

Closed

Patater changed the title ~~Update mbedcrypto 1.0.0d1~~ crypto: Update to Mbed Crypto 1.0.0d1 Jan 28, 2019

Patater mentioned this pull request Jan 28, 2019

Crypto: Update to Mbed Crypto 1.0.0d2 #9517

Closed

orenc17 mentioned this pull request Jan 29, 2019

RollUp PR Crypto with ITS #9529

Merged

itayzafrir mentioned this pull request Jan 29, 2019

Crypto API update - slots to handles ARMmbed/mbed-os-example-mbed-crypto#13

Merged

Patater closed this Jan 29, 2019

studavekar removed the needs: review label Jan 29, 2019

0xc0170 removed the release-version: 5.12.0-rc1 label Feb 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

crypto: Update to Mbed Crypto 1.0.0d1 #9463

crypto: Update to Mbed Crypto 1.0.0d1 #9463

Patater commented Jan 22, 2019 •

edited

Loading

ciarmcom commented Jan 22, 2019

Patater commented Jan 23, 2019

Patater commented Jan 23, 2019

yanesca left a comment

Patater commented Jan 25, 2019

simonbutcher commented Jan 27, 2019

mpg commented Jan 28, 2019 •

edited

Loading

NirSonnenschein commented Jan 28, 2019

Patater commented Jan 28, 2019 •

edited

Loading

Patater commented Jan 28, 2019

NirSonnenschein commented Jan 28, 2019

NirSonnenschein commented Jan 29, 2019 •

edited

Loading

mikisch81 commented Jan 29, 2019

mbed-ci commented Jan 29, 2019

NirSonnenschein commented Jan 29, 2019 •

edited

Loading

itayzafrir commented Jan 29, 2019

mikisch81 commented Jan 29, 2019

Patater commented Jan 29, 2019

crypto: Update to Mbed Crypto 1.0.0d1 #9463

crypto: Update to Mbed Crypto 1.0.0d1 #9463

Conversation

Patater commented Jan 22, 2019 • edited Loading

Description

Pull request type

Reviewers

ciarmcom commented Jan 22, 2019

Patater commented Jan 23, 2019

Patater commented Jan 23, 2019

yanesca left a comment

Choose a reason for hiding this comment

Patater commented Jan 25, 2019

simonbutcher commented Jan 27, 2019

mpg commented Jan 28, 2019 • edited Loading

NirSonnenschein commented Jan 28, 2019

Patater commented Jan 28, 2019 • edited Loading

Patater commented Jan 28, 2019

NirSonnenschein commented Jan 28, 2019

NirSonnenschein commented Jan 29, 2019 • edited Loading

mikisch81 commented Jan 29, 2019

mbed-ci commented Jan 29, 2019

Test run: FAILED

NirSonnenschein commented Jan 29, 2019 • edited Loading

itayzafrir commented Jan 29, 2019

mikisch81 commented Jan 29, 2019

Patater commented Jan 29, 2019

Patater commented Jan 22, 2019 •

edited

Loading

mpg commented Jan 28, 2019 •

edited

Loading

Patater commented Jan 28, 2019 •

edited

Loading

NirSonnenschein commented Jan 29, 2019 •

edited

Loading

NirSonnenschein commented Jan 29, 2019 •

edited

Loading