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

Backport 2.28: Fix and test MBEDTLS_PSA_INJECT_ENTROPY #7965

Conversation

gilles-peskine-arm
Copy link
Contributor

@gilles-peskine-arm gilles-peskine-arm commented Jul 20, 2023

Backport of #7518. Differences:

  • "Regroup component that had gotten separated from its close siblings": skipped because not applicable to 2.28.
  • "Fix a build error when MBEDTLS_PSA_INJECT_ENTROPY is enabled": entropy_poll.h moved in 3.0.
  • "MBEDTLS_PSA_INJECT_ENTROPY: Skip incompatible tests": same change, but with !MBEDTLS_TEST_NULL_ENTROPY also present on the line.
  • In several commits, the change is the same but with a different context due to code immediately after or before the change that is different in 2.28 and development.
  • New commit "Add missing dependencies on real entropy", fixing some missing test dependencies that we hadn't noticed until now.

PR checklist

Please tick as appropriate and edit the reasons (e.g.: "backport: not needed because this is a new feature")

The build option MBEDTLS_PSA_INJECT_ENTROPY requires some extra platform
functions, for historical reasons. To enable us to test this option, provide
a version of these functions for testing.

(These versions would actually work in production, but providing them in the
library in a way that doesn't break existing users might be slightly tricky,
so it's out of scope of this commit.)

Signed-off-by: Gilles Peskine <[email protected]>
…g tests

The seed file must exist before running tests. Because the location is
somewhat platform- and configuration-dependent, and to be friendly to
developers who run test suites individually and aren't familiar with this
feature, rely on the test framework code rather than on test scripts to
create the seed file.

Signed-off-by: Gilles Peskine <[email protected]>
When MBEDTLS_PSA_INJECT_ENTROPY is enabled, we disable standard entropy
sources, so mbedtls_entropy_func() doesn't work out of the box. Disable
tests that rely on it. MBEDTLS_PSA_INJECT_ENTROPY is intended for PSA-only
environments anyway, so it doesn't matter if some legacy features don't work
normally.

Signed-off-by: Gilles Peskine <[email protected]>
Until now, we were never enabling this option in any test.

MBEDTLS_PSA_INJECT_ENTROPY requires MBEDTLS_NO_DEFAULT_ENTROPY_SOURCES, so
it cannot be enabled in the full config and it gets its own component.

Test with MBEDTLS_USE_PSA_CRYPTO enabled, since MBEDTLS_PSA_INJECT_ENTROPY
is a very PSA feature (which can break non-PSA applications), and Mbed
OS (for whch MBEDTLS_PSA_INJECT_ENTROPY was designed) enables
MBEDTLS_USE_PSA_CRYPTO when it enables MBEDTLS_PSA_INJECT_ENTROPY.

Signed-off-by: Gilles Peskine <[email protected]>
This function was written before the PSA storage layer switched to the PSA
ITS API as its storage abstraction. Now we can just call PSA ITS functions
unconditionally.

Signed-off-by: Gilles Peskine <[email protected]>
The seed file is part of the stable interface of PSA_CRYPTO_INJECT_ENTROPY,
because it has to survive a library upgrade on a device. So check that its
existence and content are as expected at each point in the tested life cycle.

Signed-off-by: Gilles Peskine <[email protected]>
The seed file UID is part of the library's stable interface.

Signed-off-by: Gilles Peskine <[email protected]>
The test framework leaves the seed file behind (like it does with the
corresponding file in the legacy API, namely seedfile), so ignore it.

Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm gilles-peskine-arm added bug enhancement needs-ci Needs to pass CI tests component-psa PSA keystore/dispatch layer (storage, drivers, …) size-s Estimated task size: small (~2d) priority-medium Medium priority - this can be reviewed as time permits labels Jul 20, 2023
Tests that call mbedtls_test_ssl_endpoint_init() need mbedtls_entropy_func()
to work.

Signed-off-by: Gilles Peskine <[email protected]>
@gilles-peskine-arm gilles-peskine-arm marked this pull request as ready for review July 20, 2023 19:06
@gilles-peskine-arm gilles-peskine-arm added needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review labels Jul 20, 2023
@ronald-cron-arm ronald-cron-arm self-requested a review July 21, 2023 07:44
Copy link
Contributor

@ronald-cron-arm ronald-cron-arm left a comment

Choose a reason for hiding this comment

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

LGTM. I've been through the backport myself, I've seen and understood the differences compared to #7518 as explained in the PR description.

Copy link
Contributor

@tom-cosgrove-arm tom-cosgrove-arm left a comment

Choose a reason for hiding this comment

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

LGTM, faithful backport except as noted by Gilles in the description

@tom-cosgrove-arm tom-cosgrove-arm added approved Design and code approved - may be waiting for CI or backports and removed needs-review Every commit must be reviewed by at least two team members, needs-reviewer This PR needs someone to pick it up for review needs-ci Needs to pass CI tests labels Jul 21, 2023
@gilles-peskine-arm gilles-peskine-arm merged commit b98d39c into Mbed-TLS:mbedtls-2.28 Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Design and code approved - may be waiting for CI or backports bug component-psa PSA keystore/dispatch layer (storage, drivers, …) enhancement priority-medium Medium priority - this can be reviewed as time permits size-s Estimated task size: small (~2d)
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants