Upgrade OE SDK to use mbedTLS 2.16.2 #2184

CodeMonkeyLeet · 2019-09-16T23:48:23Z

This PR consolidates and updates #2043 to target the next minor version of 2.16 LTS lineage. It ports the following changes:

Update CHANGELOG.md to reflect new version of mbedTLS.
Update 3rdparty/mbedtls/update.make to target v2.16.2.
- This version no longer has a yotta/.gitignore that needs to be removed.
Update mbed tests to use Python instead of Perl to build tests.
- MbedTLS 2.16 no longer has the Perl script for building its tests.
- Add Python dependency to install-windows-prereqs.ps1
Update mbed tests to pass new .datax files from build folders to test.
- MbedTLS 2.16 no longer parses .data files from the source folders
Update add_enclave_test.cmake to copy /enc folder instead of just the
enclave binary when under ADD_WINDOWS_ENCLAVE_TESTS.
- Change the target dependency to the build folder instead of the test
  enclave binary to prevent file conflicts for tests like mbed that
  include multiple enclave test binaries under the same folder.
Add LSEEK syscall test hook required by updated mbed test suite.
Add platform_util.c to host and mbedtls build sources.
- This provides symbols referenced by other mbedTLS source files.
Apply patch to x509write_crt.c for attested TLS support.

Fixes #1457.

Co-authored-by: Marius Oprin [email protected]
Co-authored-by: John Kordich [email protected]

CodeMonkeyLeet · 2019-09-16T23:51:12Z

@johnkord, your comment from #2043 is the only remaining blocker for the original PR:

@EmilAlexandruStoica Looks like Windows fails with mbed tests because we reference some python scripts, and we don't install python as a dependency on Windows!

The changeset already has modifications to install Python as a dependency on Windows, but there's no automated system to redeploy dependencies on Windows machines today. All that needs to be done is to install Python on the CI/CD servers.

johnkord · 2019-09-16T23:56:33Z

@CodeMonkeyLeet Thanks Simon! Yep, we just need python on the Windows systems. @EmilAlexandruStoica has been working on that, I believe. Any updates on this @EmilAlexandruStoica ?

anakrish · 2019-09-17T15:30:23Z

3rdparty/mbedtls/mbedtls/library/x509write_crt.c

     */
-    c = tmp_buf + sizeof( tmp_buf );
+    c = buf + size;


Is there plans to upstream this change?

Not to my knowledge, so I filed #2179 when I came across this during the update to 2.7.11

There was an issue opened by @soccerGB with mbedTLS several months ago about this: Mbed-TLS/mbedtls#2631

From the comments, it looks like they've merged the changes in successfully, although I'm not sure which release they're available in: Mbed-TLS/mbedtls#2632

CHANGELOG.md

tests/mbed/enc/enc.c

radhikaj

Thanks for updating the mbedtls version. Much appreciated. I also see that changelog is changed. Thanks.

EmilAlexandruStoica · 2019-09-19T13:59:14Z

@CodeMonkeyLeet Thanks Simon! Yep, we just need python on the Windows systems. @EmilAlexandruStoica has been working on that, I believe. Any updates on this @EmilAlexandruStoica ?

I've connected to oe-bastion and forwarded the port 3389 so I can connect from the localhost to the Windows VM's. Then I tried to connect to the Windows VM's to install Python, but it seems that the RDP connection is not enabled.

BRMcLaren · 2019-09-23T17:07:32Z

3rdparty/mbedtls/mbedtls/.travis.yml

@@ -35,7 +42,7 @@ addons:
  coverity_scan:
    project:
      name: "ARMmbed/mbedtls"
-    notification_email: [email protected]
+    notification_email: [email protected]


I'm curious if there is a better solution than a hard coded individual e-mail address, perhaps an org?

The rationale being if Simon leaves the company, can we guarantee the correct person is notified or that their e-mail would continue to be monitored? Thus creating further changes needed.

It seems this has happened before.

That's not part of the OE SDK project (it's just a clone of the ARM mbedTLS repo) so I wouldn't worry about it. If/when we move to a git submodule approach for dependencies, then this noise will go away, but in the meantime, you can ignore 72f5a73 as being just a straight clone of the 2.16.2 mbedTLS sources.

Sounds good, appreciate the clarification!

johnkord · 2019-09-24T18:16:20Z

I've updated the systems with Python 3.7.4 and ran the mbed tests manually on one of the systems and all the tests passed. Let's see what Jenkins thinks :)

bors try

oe-bors · 2019-09-24T18:36:08Z

try

Build failed

continuous-integration/jenkins/branch

johnkord · 2019-09-24T18:42:43Z

There appear to be two errors, not sure if they're related.

Building ELF enclaves on Linux first and then copying them to Windows for testing seems to have a problem with these tests because there appear to be some hard coded path embedded in the enclave:

test 13
        Start  13: tests/mbed/asn1write

13: Test command: C:\Jenkins\workspace\Bors_trying\build\tests\mbed\host\libmbedtest_host.exe "C:/Jenkins/workspace/Bors_trying/build/tests/mbed/host/mbedtest_suite_asn1write"
13: Test timeout computed to be: 480
13: RUNNING: asn1write
13: Failed to open test file: /home/jenkins/work_dir/workspace/Bors_trying@3/build/tests/mbed/enc/test_suite_asn1write.datax
13: Test failed: ..\tests\mbed\host\host.c(26): Test args.total > 0
 13/109 Test  #13: tests/mbed/asn1write ..................................***Exception: Exit code 0xc0000409
  0.15 sec

Notice the "Failed to open test file: /home/jenkins/work_dir/workspace/Bors_trying@3/build/tests/mbed/enc/test_suite_asn1write.datax" line. Associated logs: https://oe-jenkins.eastus.cloudapp.azure.com/blue/rest/organizations/jenkins/pipelines/Bors/branches/trying/runs/995/nodes/64/steps/1042/log/?start=0

report_enc was not able to be copied for one of the stages from the linuxbin directory. I'm not sure why (I assume because it doesn't exist), but here's the output:

[218/427] cmd.exe /C "cd /D C:\Jenkins\workspace\Bors_trying\tests\report && "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E copy C:/Jenkins/workspace/Bors_trying\linuxbin\tests/report/enc/report_enc C:/Jenkins/workspace/Bors_trying/build/tests/report/host/report_enc"
FAILED: tests/report/tests/report_windows_include 
cmd.exe /C "cd /D C:\Jenkins\workspace\Bors_trying\tests\report && "C:\Program Files (x86)\Microsoft Visual Studio\2017\BuildTools\Common7\IDE\CommonExtensions\Microsoft\CMake\CMake\bin\cmake.exe" -E copy C:/Jenkins/workspace/Bors_trying\linuxbin\tests/report/enc/report_enc C:/Jenkins/workspace/Bors_trying/build/tests/report/host/report_enc"
Error copying file "C:/Jenkins/workspace/Bors_trying\linuxbin\tests/report/enc/report_enc" to "C:/Jenkins/workspace/Bors_trying/build/tests/report/host/report_enc".

I just checked on our CI systems and report_enc is indeed both in the linuxbin path and the copied destination, so I can't diagnose why this failed. Since this failure happened only on one stage, my guess is this failure occurred and then some other stage came along and overwrote it. This may point to some sort of intermittent bug. I'd suggest ignoring this failure and seeing if it reproduces again.

Associated logs: https://oe-jenkins.eastus.cloudapp.azure.com/blue/rest/organizations/jenkins/pipelines/Bors/branches/trying/runs/995/nodes/64/steps/1042/log/?start=0

CodeMonkeyLeet · 2019-09-24T19:00:31Z

@johnkord Much appreciated!

The first looks like a regression introduced by the removal of the explicit test data path specification from the mbed tests, so I'll restore that and retry.
The latter looks like a sporadic failure, given that the other similar configs that follow don't hit the same issue; something to keep track of in terms of the CI/CD robustness perhaps.

- Update CHANGELOG.md to reflect new version of mbedTLS. - Update 3rdparty/mbedtls/update.make to target v2.16.2. - This version no longer has a yotta/.gitignore that needs to be removed. - Update mbed tests to use Python instead of Perl to build tests. - MbedTLS 2.16 no longer has the Perl script for building its tests. - Add Python dependency to install-windows-prereqs.ps1 - Update mbed tests to pass new .datax files from build folders to test. - MbedTLS 2.16 no longer parses .data files from the source folders - Update add_enclave_test.cmake to copy /enc folder instead of just the enclave binary when under ADD_WINDOWS_ENCLAVE_TESTS. - Change the target dependency to the build folder instead of the test enclave binary to prevent file conflicts for tests like mbed that include multiple enclave test binaries under the same folder. - Add LSEEK syscall test hook required by updated mbed test suite. - Add platform_util.c to host and mbedtls build sources. - This provides symbols referenced by other mbedTLS source files. - Apply patch to x509write_crt.c for attested TLS support. Fixes #1457. Co-authored-by: Marius Oprin <[email protected]> Co-authored-by: John Kordich <[email protected]>

CodeMonkeyLeet · 2019-09-25T21:33:46Z

bors r+

2184: Upgrade OE SDK to use mbedTLS 2.16.2 r=CodeMonkeyLeet a=CodeMonkeyLeet This PR consolidates and updates #2043 to target the next minor version of 2.16 LTS lineage. It ports the following changes: - Update CHANGELOG.md to reflect new version of mbedTLS. - Update 3rdparty/mbedtls/update.make to target v2.16.2. - This version no longer has a yotta/.gitignore that needs to be removed. - Update mbed tests to use Python instead of Perl to build tests. - MbedTLS 2.16 no longer has the Perl script for building its tests. - Add Python dependency to install-windows-prereqs.ps1 - Remove redundant search for data file to pass to mbed tests. - Add LSEEK syscall test hook required by updated mbed test suite. - Add platform_util.c to host and mbedtls build sources. - This provides symbols referenced by other mbedTLS source files. - Apply patch to x509write_crt.c for attested TLS support. Fixes #1457. Co-authored-by: Simon Leet <[email protected]>

oe-bors · 2019-09-25T21:59:07Z

Build succeeded

continuous-integration/jenkins/branch

CodeMonkeyLeet added the core Issue is related to the core design of Open Enclave and its default 3rd party libs label Sep 16, 2019

CodeMonkeyLeet requested review from achamayou, andyleejordan, BRMcLaren, EmilAlexandruStoica, johnkord, mikbras and radhikaj as code owners September 16, 2019 23:48

CodeMonkeyLeet added the build Issue is related to the build system label Sep 16, 2019

CodeMonkeyLeet mentioned this pull request Sep 16, 2019

Bump Mbed TLS to 2.16.1 LTS #2043

Closed

anakrish reviewed Sep 17, 2019

View reviewed changes

radhikaj approved these changes Sep 18, 2019

View reviewed changes

CodeMonkeyLeet mentioned this pull request Sep 18, 2019

Remove customization of mbedTLS x509write_crt.c #2179

Open

anakrish approved these changes Sep 18, 2019

View reviewed changes

BRMcLaren reviewed Sep 23, 2019

View reviewed changes

BRMcLaren approved these changes Sep 23, 2019

View reviewed changes

johnkord approved these changes Sep 24, 2019

View reviewed changes

oe-bors bot pushed a commit that referenced this pull request Sep 24, 2019

Try #2184:

3e26412

CodeMonkeyLeet requested review from Britel, dthaler, gupta-ak and HernanGatta as code owners September 25, 2019 21:30

Clone mbedTLS v2.16.2 into 3rdparty/mbedtls/mbedtls

2d57a50

oe-bors bot merged commit 90b578f into openenclave:master Sep 25, 2019

CodeMonkeyLeet deleted the mbedtls_2.16.2 branch August 21, 2020 21:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade OE SDK to use mbedTLS 2.16.2 #2184

Upgrade OE SDK to use mbedTLS 2.16.2 #2184

CodeMonkeyLeet commented Sep 16, 2019 •

edited

Loading

CodeMonkeyLeet commented Sep 16, 2019

johnkord commented Sep 16, 2019

anakrish Sep 17, 2019

CodeMonkeyLeet Sep 17, 2019

johnkord Sep 18, 2019

radhikaj left a comment

EmilAlexandruStoica commented Sep 19, 2019

BRMcLaren Sep 23, 2019

CodeMonkeyLeet Sep 23, 2019

BRMcLaren Sep 23, 2019

johnkord commented Sep 24, 2019

oe-bors bot commented Sep 24, 2019

johnkord commented Sep 24, 2019 •

edited

Loading

CodeMonkeyLeet commented Sep 24, 2019

CodeMonkeyLeet commented Sep 25, 2019

oe-bors bot commented Sep 25, 2019

Upgrade OE SDK to use mbedTLS 2.16.2 #2184

Upgrade OE SDK to use mbedTLS 2.16.2 #2184

Conversation

CodeMonkeyLeet commented Sep 16, 2019 • edited Loading

CodeMonkeyLeet commented Sep 16, 2019

johnkord commented Sep 16, 2019

anakrish Sep 17, 2019

Choose a reason for hiding this comment

CodeMonkeyLeet Sep 17, 2019

Choose a reason for hiding this comment

johnkord Sep 18, 2019

Choose a reason for hiding this comment

radhikaj left a comment

Choose a reason for hiding this comment

EmilAlexandruStoica commented Sep 19, 2019

BRMcLaren Sep 23, 2019

Choose a reason for hiding this comment

CodeMonkeyLeet Sep 23, 2019

Choose a reason for hiding this comment

BRMcLaren Sep 23, 2019

Choose a reason for hiding this comment

johnkord commented Sep 24, 2019

oe-bors bot commented Sep 24, 2019

try

Build failed

johnkord commented Sep 24, 2019 • edited Loading

CodeMonkeyLeet commented Sep 24, 2019

CodeMonkeyLeet commented Sep 25, 2019

oe-bors bot commented Sep 25, 2019

Build succeeded

CodeMonkeyLeet commented Sep 16, 2019 •

edited

Loading

johnkord commented Sep 24, 2019 •

edited

Loading