Everest integration #140

yanesca · 2019-06-05T15:27:18Z

This PR is the result of rebasing Mbed-TLS/mbedtls#2073 on the top of Mbed Crypto development.

This PR has to be merged shortly before Mbed-TLS/mbedtls#2799 to avoid CI failures.

yanesca · 2019-06-07T07:55:35Z

On the CI the crypto job is passing, the TLS part of the pr jobs is failing, because the Mbed TLS build scripts seem to pick up the the TLS version of the crypto related headers and not the crypto version. (Changing this might make sense since crypto does not have the ssl headers anymore, but since the crypto files will be deleted soon, I don't think it is worth the effort.)

wintersteiger · 2019-07-17T17:10:18Z

Just checking: could you give us a rough idea of when this will be merged?

yanesca · 2019-07-18T07:19:46Z

Splitting the repository into Mbed TLS and Mbed Crypto complicated things: This PR is the part of the changes that need to go in Mbed Crypto. There is another part that needs to go in Mbed TLS. Before we could raise this second PR, we need to finish up a splitting related task in Mbed TLS first. Once it is done we will raise the second PR as soon as possible and we can get both of them merged.

wintersteiger · 2019-07-18T09:39:17Z

Sounds good, thanks for the update!

achamayou · 2019-08-16T11:08:19Z

@yanesca any update on this PR? We would be very keen to pick up a release of mbedtls with these changes in for our project.

These files are automatically generated by the Everest toolchain from F* files. They do not respect the mbedTLS code style guidelines as manual modification would invalidate verification guarantees. The files in 3rdparty/everest/include/kremli{n,b} are a customized (minimzed) version of the support headers expected by the code extracted using KreMLin.

This allows the use of #ifdef ... #endif in enum definitions (e.g., mbedtls_ecdh_variant in ecdh.h).

This being the first 3rdparty-contribution, we may want to consider the structure of the project file generation scripts. Perhaps add small, constribution-specific scripts to each directory in 3rdparty instead of adding all constraints to generate_visualc_files.pl?

yanesca · 2019-08-19T12:42:26Z

@achamayou I have rebased this PR, I'll still need to fix the CI failures (if any) and rebase Mbed-TLS/mbedtls#2073 before we could do the reviews and eventually merge.

yanesca · 2019-08-19T14:22:37Z

CI is only failing, because the PSA API is out of sync between Mbed TLS and Mbed Crypto. This is going to be resolved after merging Mbed-TLS/mbedtls#2767.

achamayou · 2019-08-20T19:28:57Z

@yanesca thank you for the update, we look forward to using x25519 in mbedtls.

yanesca · 2019-08-21T08:08:35Z

The CI fails only on the check-names script when testing with Mbed TLS. The reason is that MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED is not present in the Mbed TLS config file yet. This will be fixed when Mbed-TLS/mbedtls#2799 is merged.

At this point Mbed TLS and Mbed Crypto headers with the same name, including the Mbed Crypto headers in `everest_inc` breaks Mbed TLS builds.

yanesca · 2019-08-22T07:51:29Z

API checking fails on the head, because the development has been updated since the PR had been raised. Merging to development should fix this failure.

dgreen-arm

I've reviewed this by comparing it to the approved PR at Mbed-TLS/mbedtls#2073, this PR implements all the relevant parts in the same manner.

Patater · 2019-08-22T15:24:40Z

I see a check-names failure. Anything we can do to avoid it?

******************************************************************
* check_names: test/build: declared and exported names 
* Wed Aug 21 14:55:44 UTC 2019
******************************************************************
Analysing source code...
1494 macros
     210 enum-consts
1390 identifiers
413 exported-symbols

Exported symbols declared in header: PASS
Names of actual-macros: PASS
Names of enum-consts: FAIL
#endif
#ifdefined(MBEDTLS_ECDH_VARIANT_EVEREST_ENABLED)
Names of identifiers: PASS
Likely typos: PASS

Overall: FAILED
^^^^check_names: test/build: declared and exported names: tests/scripts/check-names.sh -v -> 1^^^^

yanesca · 2019-08-22T15:33:08Z

@Patater That failure is in the TLS tests, and uses TLS's scripts that can't be updated by this PR but are updated by Mbed-TLS/mbedtls#2799. The failures should go away when 2799 is merged into Mbed TLS. (PR2799 uses this PR as a submodule and the CI is green over there.)

Patater

LGTM

Before merge, we should have automated tests for Everest crypto on Windows.

Patater · 2019-09-05T18:32:50Z

@wintersteiger Thanks a lot for your contribution! And especially thanks for your patience working with us to get this in. Much appreciated!

wintersteiger · 2019-09-06T09:20:28Z

Many thanks for getting this merged! It took a little longer than expected, but we're very happy it's now in!

yanesca force-pushed the everest_integration branch 2 times, most recently from a2d1cc8 to e6707db Compare June 6, 2019 15:00

This was referenced Aug 16, 2019

OE upgrade microsoft/CCF#308

Merged

Build and link against a separate copy of mbedtls in the enclave microsoft/CCF#311

Closed

Christoph M. Wintersteiger added 21 commits August 19, 2019 13:19

ECDH: Add VS2010 support files for Everest Curve25519

2a9684e

ECDH: Add new ECDH context for Everest Curve25519

de4fcf2

ECDH: Include Everest Curve25519 in build scripts

d5fd766

ECDH: Add new (non-legacy) ECDH benchmark

696deda

ECDH: Enable Everest Curve25519 in ECDH/ECDSA/ECP

c9f737b

ECDH: Add #ifdef filter to tests/scripts/list-enum-consts.pl

6817b9e

This allows the use of #ifdef ... #endif in enum definitions (e.g., mbedtls_ecdh_variant in ecdh.h).

ECDH: Rename full handshake benchmark

65bab97

ECDH: Fix typo in ecdh.c

78c9c46

ECDH: Fix error code in mbedtls_ecdsa_sign

fba94e9

ECDH: Remove old code from mbedtls_everest_make_params

fb72367

ECDH: Replace hex literal with decimal in ecp.c

86e36c4

ECDH: Add #ifdef to cleanly disable the Everest code

6acfbb5

ECDH: Improve ECDH full handshake benchmark

79acf95

ECDH: Clean up the interface to Everest code

4936beb

ECDH: Remove YOTTA config #define

999f3b5

ECDH: Fix whitespace, permissions

c3cbdde

ECDH: Use LOCAL_CFLAGS instead of CFLAGS

34811a8

ECDH: Fix error checks in benchmark.c

3c44962

ECDH: Everest: Remove unnecessary file

24fbcef

yanesca force-pushed the everest_integration branch from e6707db to c25df68 Compare August 19, 2019 12:38

yanesca added enhancement New feature or request needs: ci Needs a passing full CI run needs: preceding PR Requires another PR to be merged first labels Aug 19, 2019

yanesca added needs: review The pull request is ready for review. This generally means that it has no known issues. and removed needs: ci Needs a passing full CI run labels Aug 19, 2019

Patater added needs: ci Needs a passing full CI run and removed needs: preceding PR Requires another PR to be merged first labels Aug 20, 2019

Correct 3rdparty include path for Mbed TLS

20082cb

yanesca mentioned this pull request Aug 20, 2019

Everest Mbed TLS integration Mbed-TLS/mbedtls#2799

Merged

yanesca removed the needs: ci Needs a passing full CI run label Aug 21, 2019

Remove Mbed Crypto headers from everest_inc

71dcefb

At this point Mbed TLS and Mbed Crypto headers with the same name, including the Mbed Crypto headers in `everest_inc` breaks Mbed TLS builds.

dgreen-arm approved these changes Aug 22, 2019

View reviewed changes

Patater approved these changes Aug 27, 2019

View reviewed changes

Patater removed the needs: review The pull request is ready for review. This generally means that it has no known issues. label Aug 29, 2019

Patater merged commit f071654 into ARMmbed:development Aug 29, 2019

gilles-peskine-arm mentioned this pull request Sep 29, 2020

Reduce the number of allocations in ECP operations Mbed-TLS/mbedtls#3512

Merged

gilles-peskine-arm mentioned this pull request Nov 2, 2023

License change Mbed-TLS/mbedtls#8462

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Everest integration #140

Everest integration #140

yanesca commented Jun 5, 2019 •

edited

Loading

yanesca commented Jun 7, 2019

wintersteiger commented Jul 17, 2019

yanesca commented Jul 18, 2019

wintersteiger commented Jul 18, 2019

achamayou commented Aug 16, 2019

yanesca commented Aug 19, 2019

yanesca commented Aug 19, 2019

achamayou commented Aug 20, 2019

yanesca commented Aug 21, 2019

yanesca commented Aug 22, 2019

dgreen-arm left a comment

Patater commented Aug 22, 2019

yanesca commented Aug 22, 2019

Patater left a comment

Patater commented Sep 5, 2019

wintersteiger commented Sep 6, 2019

Everest integration #140

Everest integration #140

Conversation

yanesca commented Jun 5, 2019 • edited Loading

yanesca commented Jun 7, 2019

wintersteiger commented Jul 17, 2019

yanesca commented Jul 18, 2019

wintersteiger commented Jul 18, 2019

achamayou commented Aug 16, 2019

yanesca commented Aug 19, 2019

yanesca commented Aug 19, 2019

achamayou commented Aug 20, 2019

yanesca commented Aug 21, 2019

yanesca commented Aug 22, 2019

dgreen-arm left a comment

Choose a reason for hiding this comment

Patater commented Aug 22, 2019

yanesca commented Aug 22, 2019

Patater left a comment

Choose a reason for hiding this comment

Patater commented Sep 5, 2019

wintersteiger commented Sep 6, 2019

yanesca commented Jun 5, 2019 •

edited

Loading