Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[mbedtls] Bump version from 2.18 to 2.25 #7311

Merged
merged 2 commits into from
Jun 28, 2021

Conversation

Damian-Nordic
Copy link
Contributor

Problem

The future of crypto API on embedded devices is the PSA crypto API: https://armmbed.github.io/mbed-crypto/html/index.html. We will probably want to switch to that in Matter at some point in the future and in order to experiment with that we need quite a recent version of mbedtls. The one we currently use is quite old (from 2019).

Change overview

Bump mbedtls from 2.18 to 2.25

Testing

Verified that our unit tests for mbedTLS-based crypto are passing.
Also, compiled Python CHIP Controller with chip_crypto="mbedtls" and performed the full commissioning to verify that both PASE and the secure channel work correctly.

@Damian-Nordic Damian-Nordic changed the title [mbedtls] update to 2.25 [mbedtls] Bump version from 2.18 to 2.25 Jun 2, 2021
@Damian-Nordic
Copy link
Contributor Author

@tima-q I checked the Qorvo build error and it appears that:

  1. in mbedtls 2.25 mbedtls_pk_write_key_der() refers to mbedtls_ecp_write_key() function defined in ecp.c provided that MBEDTLS_ECP_C && !MBEDTLS_ECP_ALT.
  2. Qorvo uses third_party/qpg_sdk/repo/qpg6100/comps/libmbedtls/qpg6100-mbedtls-config.h as the mbedtls config. It defines both MBEDTLS_ECP_C and MBEDTLS_ECP_ALT.
  3. Despite of defining MBEDTLS_ECP_ALT, the Qorvo's sdk doesn't seem to provide any implementation of the mbedtls_ecp_write_key() function.
    Do you know how can I fix the build? Is mbedtls_ecp_write_key provided as any static library or is the mbedtls config invalid?

@tima-q
Copy link
Contributor

tima-q commented Jun 2, 2021

@tima-q I checked the Qorvo build error and it appears that:

  1. in mbedtls 2.25 mbedtls_pk_write_key_der() refers to mbedtls_ecp_write_key() function defined in ecp.c provided that MBEDTLS_ECP_C && !MBEDTLS_ECP_ALT.
  2. Qorvo uses third_party/qpg_sdk/repo/qpg6100/comps/libmbedtls/qpg6100-mbedtls-config.h as the mbedtls config. It defines both MBEDTLS_ECP_C and MBEDTLS_ECP_ALT.
  3. Despite of defining MBEDTLS_ECP_ALT, the Qorvo's sdk doesn't seem to provide any implementation of the mbedtls_ecp_write_key() function.
    Do you know how can I fix the build? Is mbedtls_ecp_write_key provided as any static library or is the mbedtls config invalid?

@Damian-Nordic
I suspect our current mbedtls alt implementation (included in a static lib) is built against a lower mbedtls API version, hence missing the function. Will need to coordinate internally how/when we can bump this. Will keep you posted.

@woody-apple

This comment has been minimized.

@woody-apple
Copy link
Contributor

/rebase

@andy31415
Copy link
Contributor

@Damian-Nordic seems we need to fix some compilation issues.

@mspang
Copy link
Contributor

mspang commented Jun 13, 2021

@tima-q I checked the Qorvo build error and it appears that:

  1. in mbedtls 2.25 mbedtls_pk_write_key_der() refers to mbedtls_ecp_write_key() function defined in ecp.c provided that MBEDTLS_ECP_C && !MBEDTLS_ECP_ALT.
  2. Qorvo uses third_party/qpg_sdk/repo/qpg6100/comps/libmbedtls/qpg6100-mbedtls-config.h as the mbedtls config. It defines both MBEDTLS_ECP_C and MBEDTLS_ECP_ALT.
  3. Despite of defining MBEDTLS_ECP_ALT, the Qorvo's sdk doesn't seem to provide any implementation of the mbedtls_ecp_write_key() function.
    Do you know how can I fix the build? Is mbedtls_ecp_write_key provided as any static library or is the mbedtls config invalid?

@Damian-Nordic
I suspect our current mbedtls alt implementation (included in a static lib) is built against a lower mbedtls API version, hence missing the function. Will need to coordinate internally how/when we can bump this. Will keep you posted.

Can you build mbedtls from source please? Including an MbedTLS .a file in your SDK without even shipping the corresponding headers doesn't make much sense.

@woody-apple
Copy link
Contributor

/rebase

@tima-q
Copy link
Contributor

tima-q commented Jun 16, 2021

Can you build mbedtls from source please? Including an MbedTLS .a file in your SDK without even shipping the corresponding headers doesn't make much sense.

@mspang The lib only contains the _alt pieces that are Qorvo specific. The remainder is built from the third_party/mbedtls folder.
As the SDK is created to support this project, including the headers twice did not make sense at the time.

We planned in the task to update the mbedtls_alt implementation used. This should be ready by next week.
@Damian-Nordic this would come in the form of a submodule update of our qpg_sdk - do we contribute it to your PR directly ?

@Damian-Nordic
Copy link
Contributor Author

@tima-q yeah, let me know when the new sdk is ready, so I can include the update in my PR.

@woody-apple
Copy link
Contributor

/rebase

@woody-apple
Copy link
Contributor

@Damian-Nordic Looks like a real build failure on QPG

@woody-apple woody-apple merged commit 7536628 into project-chip:master Jun 28, 2021
nikita-s-wrk pushed a commit to nikita-s-wrk/connectedhomeip that referenced this pull request Sep 23, 2021
* [mbedtls] update to 2.25

* bump the QPG SDK to support mbedtls 2.25

Co-authored-by: Lukasz Duda <[email protected]>
Co-authored-by: Thomas Cuyckens <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants