gh-Mbed-TLS/TF-PSA-Crypto
Watch
1
0
Fork
You've already forked TF-PSA-Crypto
 0
Code Issues 167 Pull requests 92 Releases 5 Activity

Add outcome analysis to TF-PSA-Crypto #729

Merged
gilles-peskine-arm merged 14 commits from analyze_outcomes-add_to_crypto into development 2026-04-10 11:53:19 -05:00
Conversation 15 Commits 14 Files changed 4 +346 -17
gilles-peskine-arm commented 2026-03-26 08:44:55 -05:00 (Migrated from github.com)
  • Add analyze_outcomes.py to TF-PSA-Crypto.
  • There is a list of ignored test cases (fully ignored, not just must-not-be-covered) for coverage analysis. It is still the responsibility of Mbed TLS to run these tests. We will gradually remove elements from this list as we add more configurations to TF-PSA-Crypto's all.sh. Tracked in https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/740.
  • TF-PSA-Crypto defines INTERNAL_TEST_CASES, which Mbed TLS's outcome analysis can ignore.

Resolves https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/315. Contributes to https://github.com/Mbed-TLS/mbedtls/issues/10221, which the companion Mbed TLS PR will resolve.

PR checklist

* Add `analyze_outcomes.py` to TF-PSA-Crypto. * There is a list of ignored test cases (fully ignored, not just must-not-be-covered) for coverage analysis. It is still the responsibility of Mbed TLS to run these tests. We will gradually remove elements from this list as we add more configurations to TF-PSA-Crypto's `all.sh`. Tracked in https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/740. * TF-PSA-Crypto defines `INTERNAL_TEST_CASES`, which Mbed TLS's outcome analysis can ignore. Resolves https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/315. Contributes to https://github.com/Mbed-TLS/mbedtls/issues/10221, which the companion Mbed TLS PR will resolve. ## PR checklist - [x] **changelog** not required because: test only - [x] **development PR** https://github.com/Mbed-TLS/mbedtls/pull/10676 - [x] **TF-PSA-Crypto development PR** https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/729 - [x] **TF-PSA-Crypto 1.1 PR** https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/748 - [x] **framework PR** provided https://github.com/Mbed-TLS/mbedtls-framework/pull/292 - [ ] **4.1 PR** TODO - [x] **3.6 PR** https://github.com/Mbed-TLS/mbedtls/pull/10677 - **tests** provided
bjwtaylor (Migrated from github.com) reviewed 2026-04-08 03:30:45 -05:00
tests/crypto-tests.make
@ -2,7 +2,13 @@
# This file is only meant to be included by tests/Makefile in Mbed TLS and
bjwtaylor (Migrated from github.com) commented 2026-04-08 03:27:15 -05:00

Do we need a copyright header?

Do we need a copyright header?
tests/scripts/analyze_outcomes.py
@ -0,0 +1,282 @@
#!/usr/bin/env python3
bjwtaylor (Migrated from github.com) commented 2026-04-08 03:26:47 -05:00

Do we need a copyright header?

Do we need a copyright header?
tests/scripts/analyze_outcomes.py
@ -0,0 +35,4 @@
re.compile('.*XTS.*'),
],
'test_suite_config.psa_boolean': [
re.compile('.* !.*'),
bjwtaylor (Migrated from github.com) commented 2026-04-08 03:26:09 -05:00

Minor: I think these broad ignores overlap many other defined tests. Not sure if this is intentional or desired, however it could lead to maintenance issued in the future.

Minor: I think these broad ignores overlap many other defined tests. Not sure if this is intentional or desired, however it could lead to maintenance issued in the future.
tests/scripts/analyze_outcomes.py
@ -0,0 +116,4 @@
# Tests that are not covered for a tracked reason, and that
# were also not covered by Mbed TLS testing as of Mbed TLS 4.1.0.
UNCOVERED_TESTS = {
bjwtaylor (Migrated from github.com) commented 2026-04-08 03:29:45 -05:00

I think the following could be stale entries:

  • test_suite_config.psa_boolean: Config: PSA_WANT_ALG_CBC_MAC at tests/scripts/analyze_outcomes.py:160
  • test_suite_config.psa_boolean: Config: PSA_WANT_ALG_XTS at tests/scripts/analyze_outcomes.py:162
  • test_suite_config.psa_boolean: Config: PSA_WANT_KEY_TYPE_ECC_KEY_PAIR at tests/scripts/analyze_outcomes.py:167
  • test_suite_config.psa_boolean: Config: PSA_WANT_KEY_TYPE_RSA_KEY_PAIR at tests/scripts/analyze_outcomes.py:168
  • test_suite_pkcs12: PBE Decrypt, (Invalid padding & PKCS7 padding disabled) at tests/scripts/analyze_outcomes.py:207
  • test_suite_pkcs12: PBE Encrypt, pad = 8 (PKCS7 padding disabled) at tests/scripts/analyze_outcomes.py:208
  • test_suite_psa_crypto_metadata: MAC: CBC_MAC-3DES at tests/scripts/analyze_outcomes.py:230
  • test_suite_psa_crypto_op_fail.generated: re.compile(r'.: !ECDSA but DETERMINISTIC_ECDSA with ECC_.') at tests/scripts/analyze_outcomes.py:251
    Not sure if this is an issue, as the whole point it they are ignored. However might be worth cleaning them up if they are not required to aid future maintanence.
I think the following could be stale entries: - test_suite_config.psa_boolean: Config: PSA_WANT_ALG_CBC_MAC at tests/scripts/analyze_outcomes.py:160 - test_suite_config.psa_boolean: Config: PSA_WANT_ALG_XTS at tests/scripts/analyze_outcomes.py:162 - test_suite_config.psa_boolean: Config: PSA_WANT_KEY_TYPE_ECC_KEY_PAIR at tests/scripts/analyze_outcomes.py:167 - test_suite_config.psa_boolean: Config: PSA_WANT_KEY_TYPE_RSA_KEY_PAIR at tests/scripts/analyze_outcomes.py:168 - test_suite_pkcs12: PBE Decrypt, (Invalid padding & PKCS7 padding disabled) at tests/scripts/analyze_outcomes.py:207 - test_suite_pkcs12: PBE Encrypt, pad = 8 (PKCS7 padding disabled) at tests/scripts/analyze_outcomes.py:208 - test_suite_psa_crypto_metadata: MAC: CBC_MAC-3DES at tests/scripts/analyze_outcomes.py:230 - test_suite_psa_crypto_op_fail.generated: re.compile(r'.*: !ECDSA but DETERMINISTIC_ECDSA with ECC_.*') at tests/scripts/analyze_outcomes.py:251 Not sure if this is an issue, as the whole point it they are ignored. However might be worth cleaning them up if they are not required to aid future maintanence.
gilles-peskine-arm (Migrated from github.com) reviewed 2026-04-08 03:36:52 -05:00
tests/scripts/analyze_outcomes.py
@ -0,0 +35,4 @@
re.compile('.*XTS.*'),
],
'test_suite_config.psa_boolean': [
re.compile('.* !.*'),
gilles-peskine-arm (Migrated from github.com) commented 2026-04-08 03:36:52 -05:00

This is broad by design: the goal of this PR is only to have the script run and pass. Any test case that isn't covered by INTERNAL_TEST_CASES could be in IGNORED_TESTS here: as long as a test case isn't in both lists, we know that it's run in at least one of the projects. So I made an overapproximation here, and there's a task https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/740 to gradually shrink this list.

This is broad by design: the goal of this PR is only to have the script run and pass. Any test case that isn't covered by `INTERNAL_TEST_CASES` could be in `IGNORED_TESTS` here: as long as a test case isn't in both lists, we know that it's run in at least one of the projects. So I made an overapproximation here, and there's a task https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/740 to gradually shrink this list.
gilles-peskine-arm (Migrated from github.com) reviewed 2026-04-08 03:37:47 -05:00
tests/scripts/analyze_outcomes.py
@ -0,0 +1,282 @@
#!/usr/bin/env python3
gilles-peskine-arm (Migrated from github.com) commented 2026-04-08 03:37:47 -05:00

This is a copy of an existing script, so it's a preexisting issue, but our python scripts usually do have copyright headers so I'll add one if I do some other rework.

This is a copy of an existing script, so it's a preexisting issue, but our python scripts usually do have copyright headers so I'll add one if I do some other rework.
gilles-peskine-arm (Migrated from github.com) reviewed 2026-04-08 03:38:20 -05:00
tests/crypto-tests.make
@ -2,7 +2,13 @@
# This file is only meant to be included by tests/Makefile in Mbed TLS and
gilles-peskine-arm (Migrated from github.com) commented 2026-04-08 03:38:19 -05:00

Our makefiles historically didn't have copyright headers, so it's a preexisting issue and I'm not going to start adding copyright statements to all the makefiles.

Our makefiles historically didn't have copyright headers, so it's a preexisting issue and I'm not going to start adding copyright statements to all the makefiles.
gilles-peskine-arm (Migrated from github.com) reviewed 2026-04-08 03:40:44 -05:00
tests/scripts/analyze_outcomes.py
@ -0,0 +116,4 @@
# Tests that are not covered for a tracked reason, and that
# were also not covered by Mbed TLS testing as of Mbed TLS 4.1.0.
UNCOVERED_TESTS = {
gilles-peskine-arm (Migrated from github.com) commented 2026-04-08 03:40:44 -05:00

We know that these test cases aren't covered by Mbed TLS's all.sh. It's likely that they aren't covered by TF-PSA-Crypto's all.sh either, since that started out as equivalent to a subset of Mbed TLS's all.sh. Even if some happen to be also ignored because they're the responsibility of Mbed TLS right now, we want to make them the responsibility of TF-PSA-Crypto, so they should stay.

We know that these test cases aren't covered by Mbed TLS's `all.sh`. It's likely that they aren't covered by TF-PSA-Crypto's `all.sh` either, since that started out as equivalent to a subset of Mbed TLS's `all.sh`. Even if some happen to be also ignored because they're the responsibility of Mbed TLS right now, we want to make them the responsibility of TF-PSA-Crypto, so they should stay.
bjwtaylor (Migrated from github.com) reviewed 2026-04-08 07:06:37 -05:00
tests/scripts/analyze_outcomes.py
@ -0,0 +35,4 @@
re.compile('.*XTS.*'),
],
'test_suite_config.psa_boolean': [
re.compile('.* !.*'),
bjwtaylor (Migrated from github.com) commented 2026-04-08 07:06:37 -05:00

Ok, sounds good if it's covered elsewhere I'll close this one.

Ok, sounds good if it's covered elsewhere I'll close this one.
bjwtaylor (Migrated from github.com) reviewed 2026-04-08 07:06:51 -05:00
tests/crypto-tests.make
@ -2,7 +2,13 @@
# This file is only meant to be included by tests/Makefile in Mbed TLS and
bjwtaylor (Migrated from github.com) commented 2026-04-08 07:06:51 -05:00

OK, sounds reasonable.

OK, sounds reasonable.
ronald-cron-arm (Migrated from github.com) requested changes 2026-04-08 09:46:38 -05:00
ronald-cron-arm (Migrated from github.com) left a comment

This looks almost good to me. Only some minor comments.

This looks almost good to me. Only some minor comments.
tests/scripts/analyze_outcomes.py
@ -0,0 +23,4 @@
# Tests that may not be covered by TF-PSA-Crypto testing, but are
# covered by Mbed TLS testing.
# https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/740
ronald-cron-arm (Migrated from github.com) commented 2026-04-08 09:33:34 -05:00

I've run the CI without IGNORED_TESTS (commit ea8f3e43fe, see https://ci.trustedfirmware.org/view/Mbed-TLS/job/mbed-tls-restricted-pr-test-parametrized/125/) and I do not see any not executed tests from test_suite_ccm. Thus do we need that one?

I've run the CI without IGNORED_TESTS (commit ea8f3e43fe82bc8c451a792d8110705f7504e394, see https://ci.trustedfirmware.org/view/Mbed-TLS/job/mbed-tls-restricted-pr-test-parametrized/125/) and I do not see any not executed tests from `test_suite_ccm`. Thus do we need that one?
tests/scripts/tf_psa_crypto_test_case_info.py
@ -0,0 +1,31 @@
"""Information about TF-PSA-Crypto test cases that Mbed TLS can access."""
ronald-cron-arm (Migrated from github.com) commented 2026-04-08 09:02:41 -05:00
```suggestion ```
ronald-cron-arm (Migrated from github.com) commented 2026-04-08 09:10:05 -05:00 
# Test cases that it does not make sense to execute in Mbed TLS, because they
# relate to TF-PSA-Crypto implementation details, e.g. optimization options.
# This has the same format as `CoverageTask.IGNORED_TESTS` in outcome_analysis:
# ```{ test_suite_name: [matcher, ...], ... }```
# with each ``matcher`` being either:
# - a string, representing an exact test case description;
# - a regex, which must match the full test case description.
# A test suite with no "." matches all test suites with the same .function
# file. A test suite with a "." only matches that specific .data file.
```suggestion # Test cases that it does not make sense to execute in Mbed TLS, because they # relate to TF-PSA-Crypto implementation details, e.g. optimization options. # This has the same format as `CoverageTask.IGNORED_TESTS` in outcome_analysis: # ```{ test_suite_name: [matcher, ...], ... }``` # with each ``matcher`` being either: # - a string, representing an exact test case description; # - a regex, which must match the full test case description. # A test suite with no "." matches all test suites with the same .function # file. A test suite with a "." only matches that specific .data file. ```
gilles-peskine-arm (Migrated from github.com) reviewed 2026-04-08 10:02:33 -05:00
tests/scripts/analyze_outcomes.py
@ -0,0 +23,4 @@
# Tests that may not be covered by TF-PSA-Crypto testing, but are
# covered by Mbed TLS testing.
# https://github.com/Mbed-TLS/TF-PSA-Crypto/issues/740
gilles-peskine-arm (Migrated from github.com) commented 2026-04-08 10:02:33 -05:00

Thanks for checking. IIRC I started from a run that didn't have the release mergeback, and I guess I accidentally kept that entry even though it's no longer required.

Thanks for checking. IIRC I started from a run that didn't have the release mergeback, and I guess I accidentally kept that entry even though it's no longer required.
gilles-peskine-arm (Migrated from github.com) reviewed 2026-04-08 10:03:46 -05:00
tests/scripts/tf_psa_crypto_test_case_info.py
@ -0,0 +1,31 @@
"""Information about TF-PSA-Crypto test cases that Mbed TLS can access."""
gilles-peskine-arm (Migrated from github.com) commented 2026-04-08 10:03:46 -05:00

That was deliberate: we're likely to start using re.compile soon, so this is a signpost suggesting that it's ok to do so.

Or maybe I should leave it uncommented, and shut pylint up about a useless import?

That was deliberate: we're likely to start using `re.compile` soon, so this is a signpost suggesting that it's ok to do so. Or maybe I should leave it uncommented, and shut pylint up about a useless import?
ronald-cron-arm (Migrated from github.com) reviewed 2026-04-08 10:09:37 -05:00
tests/scripts/tf_psa_crypto_test_case_info.py
@ -0,0 +1,31 @@
"""Information about TF-PSA-Crypto test cases that Mbed TLS can access."""
ronald-cron-arm (Migrated from github.com) commented 2026-04-08 10:09:37 -05:00

sounds good to me, maybe with a comment

sounds good to me, maybe with a comment
ronald-cron-arm (Migrated from github.com) reviewed 2026-04-08 10:20:51 -05:00
ronald-cron-arm (Migrated from github.com) left a comment
No description provided.
LGTM, thanks.
gilles-peskine-arm commented 2026-04-09 08:43:56 -05:00 (Migrated from github.com)

The framework has been updated in another PR. So I'm going to rebase on top of development, with no framework update here.

The framework has been updated in another PR. So I'm going to rebase on top of `development`, with no framework update here.
ronald-cron-arm (Migrated from github.com) approved these changes 2026-04-10 02:58:21 -05:00
bjwtaylor (Migrated from github.com) approved these changes 2026-04-10 05:15:41 -05:00
Sign in to join this conversation.
Reviewers
No reviewers
ronald-cron-arm
bjwtaylor
Labels
Clear labels   
api-break
This issue/PR breaks the API and must wait for a new major version

  
approved
Design and code approved - may be waiting for CI or backports

  
bug
Something isn't working

  
component-docs
Docs / website issues filed here for tracking

  
component-platform
Portability layer and build scripts

  
component-test
Test framework and CI scripts

  
dependencies
Pull requests that update a dependency file

  
deprecation
needs deprecation announcement

  
DO-NOT-MERGE

   
enhancement
New feature or request

  
good first issue
Good for newcomers

  
help wanted
This issue is not being actively worked on, but PRs welcome.

  
needs-adoption
stalled PR that someone should pick up and complete

  
needs-backports
Backports are missing or are pending review and approval.

  
needs-ci
Needs to pass CI tests

  
needs-design-approval
Needs design discussion / approval

  
needs-info
An issue or PR which needs further info from the reporter / author

  
needs-preceding-pr
Requires another PR to be merged first

  
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-work

   
priority-high
High priority - will be reviewed soon

  
priority-low
Low priority - this may not receive review soon

  
priority-medium
Medium priority - this can be reviewed as time permits

  
priority-scheduled
This PR is big - it will require time to be scheduled for review

  
priority-very-high
Highest priority - prioritise this over other review work

  
resolved-after-1.1
The issue is resolved in development, but still present in 1.1 LTS.

  
single-reviewer
This PR qualifies for having only one reviewer

  
size-l
Estimated task size: large (2w+)

  
size-m
Estimated task size: medium (~1w)

  
size-optimisation

   
size-s
Estimated task size: small (~2d)

  
size-xs
Estimated task size: extra small (a few hours at most)

No labels
api-break
approved
bug
component-docs
component-platform
component-test
dependencies
deprecation
DO-NOT-MERGE
enhancement
good first issue
help wanted
needs-adoption
needs-backports
needs-ci
needs-design-approval
needs-info
needs-preceding-pr
needs-review
needs-reviewer
needs-work
priority-high
priority-low
priority-medium
priority-scheduled
priority-very-high
resolved-after-1.1
single-reviewer
size-l
size-m
size-optimisation
size-s
size-xs
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-Mbed-TLS/TF-PSA-Crypto!729
No description provided.