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

[v4] Rem core mbedtls #6317

Closed
wants to merge 3 commits into from

Conversation

jenswi-linaro
Copy link
Contributor

This is a part of jenswi-linaro#11.

As a first step, we remove support for mbedtls in OP-TEE core, except we still use the bignum implementation with LTC.

Another pull request will follow to update mbedtls to version 3.4.0.

@jenswi-linaro jenswi-linaro changed the title Rem core mbedtls [v4] Rem core mbedtls Sep 25, 2023
Copy link
Contributor

@etienne-lms etienne-lms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Acked-by: Etienne Carriere <etienne.carriere@foss.st.com> for the series.

@larper2axis
Copy link
Contributor

This is unfortunate. We run with mbedtls in the core for its superior RSA performance.

@jenswi-linaro
Copy link
Contributor Author

The RSA implementation in MBedTLS 3.x is unfortunately incompatible with GlobalPlatform TEE Internal Core API, see jenswi-linaro#9.

Speaking of performance, I've been toying with the idea of using OpenSSL now that it has a permissive license. But that's not in scope here and will likely not happen either unless someone starts to push for it.

Removes build test of CFG_CRYPTOLIB_NAME=mbedtls since support for
mbedtls in core is about to be removed.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
Removes support for MbedTLS as main crypto library.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
Removes support for using MbedTLS as crypto lib in core.

Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org>
Acked-by: Etienne Carriere <etienne.carriere@foss.st.com>
@jenswi-linaro
Copy link
Contributor Author

Tag applied.

@larper2axis
Copy link
Contributor

larper2axis commented Sep 27, 2023

Improved blinding is very much appreciated and necessary to counter the ever improving techniques for extracting keys over side channels. The GP API is the way it is, but if the user provides a key with too few components, we can always hit them with the runtime penalty of calling mbedtls_rsa_complete(). All in the name of improved security.

@jenswi-linaro
Copy link
Contributor Author

I wasn't aware of the mbedtls_rsa_complete() function. I'll test it a bit and get back here.
In the meantime, please don't merge this PR.

@jforissier
Copy link
Contributor

In the meantime, please don't merge this PR.

I intend to keep the [v4] PRs open for a bit more time than usual if that's OK with you. I think we need to leave enough time for people to give feedback.

@jenswi-linaro
Copy link
Contributor Author

I intend to keep the [v4] PRs open for a bit more time than usual if that's OK with you. I think we need to leave enough time for people to give feedback.

Makes sense, we only need to merge them before tagging the rc1.

@jenswi-linaro
Copy link
Contributor Author

@larper2axis Here's take 3 for updating to MbedTLS 3.4.0. jenswi-linaro#12

If it makes sense, please review the few commits without review tags.

I'll push the branch to import/mbedtls-3.4.0 once we're happy with the pull request and issue another squashed pull request and the master branch.

@jenswi-linaro
Copy link
Contributor Author

Doing #6351 instead.

@jenswi-linaro jenswi-linaro deleted the rem_core_mbedtls branch October 23, 2023 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants