Backport 2.28: Start testing the PSA built-in drivers: hashes #8222

tgonzalezorlandoarm · 2023-09-18T14:08:39Z

Description

Backport of #7803

PR checklist

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

changelog no (test only)
backport this is the backport
tests provided

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

tgonzalezorlandoarm · 2023-09-18T14:12:23Z

A couple of questions:

@gilles-peskine-arm I only left your Sign-off and author as these are your commits. I have barely retouched one commit to make it fit to the backport, so I hope this is okay, let me know otherwise.
I see that the test does not get generated. I can't find why it's behaving this way, I would appreciate any pointers.

gilles-peskine-arm · 2023-09-18T14:17:26Z

@gilles-peskine-arm I only left your Sign-off and author as these are your commits. I have barely retouched one commit to make it fit to the backport, so I hope this is okay, let me know otherwise.

That's fine. The signoff line isn't the author line, it's a claim that we have the rights to contribute the code. Between arm employees, we don't really care who signs off: we're all signing off as agents of our employer.

I see that the test does not get generated. I can't find why it's behaving this way, I would appreciate any pointers.

In 2.28, all generated files are checked into git (because we don't require users to have the necessary tools). When you do something that affects a generated source file, you need to run tests/scripts/check_generated_files -u and commit the result.

tgonzalezorlandoarm · 2023-09-19T09:36:34Z

Perfect, thanks for the information @gilles-peskine-arm ! I've just updated the files accordingly

gilles-peskine-arm · 2023-09-29T09:22:43Z

Looks like I merged #7803 without waiting for its backport, which is this pull request. That's not ideal, but it's not a big deal either since this is just tests. The original pull request was motivated by improved driver support, which doesn't exist in 2.28: we wanted extra tests to make sure the driver interface supports drivers well. The reason to backport is mostly to facilitate the backports of future tests.

Since this is just tests and not bug fixes, it doesn't matter if we make a release with #7803 but not its backport.

gilles-peskine-arm

The code move wasn't backported correctly.

tests/scripts/generate_psa_tests.py

tests/suites/test_suite_psa_crypto_generate_key.generated.data

gilles-peskine-arm · 2023-09-29T09:37:02Z

scripts/mbedtls_dev/psa_information.py

@@ -0,0 +1,162 @@
+"""Collect information about PSA cryptographic mechanisms.


The commit "Move PSA information and dependency automation into their own module" should be doing the following:

Create psa_information.py and write some stuff at the top (docstring, copyright notice, imports).

Move blocks of code from generate_psa_tests.py to psa_information.py.

In generate_psa_tests.py, add psa_information. when referencing functions that have moved to the new module.

(not in 2.28) update dependencies in makefiles.

b4b5402 is not doing this correctly: it's adding code from development. This commit needs to be reconstructed rather than cherry-picked.

This has consequences, some benign, some not. One bad consequence is that PSA_WANT_KEY_TYPE_xxx_KEY_PAIR dependencies have moved to their 3.5 names, which is wrong because those names don't exist in 2.28. This causes some test cases to be never executed.

I think I now managed to do the backport properly, sorry for the confusion

scripts/mbedtls_dev/crypto_knowledge.py

This will let us use these features from other modules (yet to be created). Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com> Signed-off-by: Tomás González <tomasagustin.gonzalezorlando@arm.com>

Some basic test coverage for now: * Nominal operation. * Larger output buffer. * Clone an operation and use it after the original operation stops. Generate test data automatically. For the time being, only do that for hashes that Python supports natively. Supporting all algorithms is future work. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com> Signed-off-by: Tomás González <tomasagustin.gonzalezorlando@arm.com>

Do explain why we don't test a smaller buffer in addition to testing the nominal size and a larger buffer. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com> Signed-off-by: Tomás González <tomasagustin.gonzalezorlando@arm.com>

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

gilles-peskine-arm

LGTM

gilles-peskine-arm · 2023-10-31T19:04:33Z

scripts/mbedtls_dev/psa_information.py

+    if not all((dep.lstrip('!') in _implemented_dependencies or
+                not dep.lstrip('!').startswith('PSA_WANT'))


734d22c changed these two lines from

if not all((dep.lstrip('!') in _implemented_dependencies or 'PSA_WANT' not in dep)

The new code is better, so that's ok. It would have been better to do this in a separate commit, but never mind now.

minosgalanakis · 2023-11-10T15:42:07Z

This PR looks good code-wise but I think it should be be updated to the newer licence headers.

Signed-off-by: Tomás González <tomasagustin.gonzalezorlando@arm.com>

minosgalanakis

LGTM

gilles-peskine-arm

LGTM

tgonzalezorlandoarm added component-crypto size-s component-test priority-high labels Sep 18, 2023

tgonzalezorlandoarm requested a review from gilles-peskine-arm September 18, 2023 14:08

tgonzalezorlandoarm self-assigned this Sep 18, 2023

tgonzalezorlandoarm changed the base branch from development to mbedtls-2.28 September 18, 2023 14:09

tgonzalezorlandoarm added the needs-ci label Sep 18, 2023

tgonzalezorlandoarm force-pushed the tg/backport-psa-low-hash-mac-size branch from 74a08ac to 890ca87 Compare September 19, 2023 09:33

tgonzalezorlandoarm marked this pull request as ready for review September 19, 2023 09:42

tgonzalezorlandoarm added needs-review needs-reviewer labels Sep 19, 2023

daverodgman added priority-very-high and removed priority-high labels Sep 29, 2023

gilles-peskine-arm added priority-high and removed needs-ci priority-very-high labels Sep 29, 2023

gilles-peskine-arm requested changes Sep 29, 2023

View reviewed changes

gilles-peskine-arm added needs-work and removed needs-review needs-reviewer labels Sep 29, 2023

minosgalanakis mentioned this pull request Oct 3, 2023

Start testing the PSA built-in drivers: hashes #7803

Merged

3 tasks

tgonzalezorlandoarm force-pushed the tg/backport-psa-low-hash-mac-size branch from 890ca87 to 5f6057b Compare October 30, 2023 16:38

tgonzalezorlandoarm added needs-review and removed needs-work labels Oct 30, 2023

Move PSA information and dependency automation into their own module

734d22c

This will let us use these features from other modules (yet to be created). Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com> Signed-off-by: Tomás González <tomasagustin.gonzalezorlando@arm.com>

tgonzalezorlandoarm and others added 3 commits October 31, 2023 09:32

Remove dead code

e25a619

Do explain why we don't test a smaller buffer in addition to testing the nominal size and a larger buffer. Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com> Signed-off-by: Tomás González <tomasagustin.gonzalezorlando@arm.com>

Fix type annotation

9043a2f

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

tgonzalezorlandoarm force-pushed the tg/backport-psa-low-hash-mac-size branch from 5f6057b to 9043a2f Compare October 31, 2023 09:33

gilles-peskine-arm previously approved these changes Oct 31, 2023

View reviewed changes

tgonzalezorlandoarm added needs-reviewer needs-review and removed needs-review labels Oct 31, 2023

minosgalanakis self-requested a review November 9, 2023 14:27

Update new license headers

Loading
Loading status checks…

5fae560

Signed-off-by: Tomás González <tomasagustin.gonzalezorlando@arm.com>

tgonzalezorlandoarm dismissed gilles-peskine-arm’s stale review via 5fae560 November 13, 2023 11:45

tgonzalezorlandoarm removed the needs-reviewer label Nov 13, 2023

minosgalanakis approved these changes Nov 15, 2023

View reviewed changes

minosgalanakis requested a review from gilles-peskine-arm November 15, 2023 14:04

gilles-peskine-arm approved these changes Nov 20, 2023

View reviewed changes

tgonzalezorlandoarm added approved and removed needs-review labels Nov 20, 2023

daverodgman added this pull request to the merge queue Nov 21, 2023

Merged via the queue into Mbed-TLS:mbedtls-2.28 with commit b9c7058 Nov 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Backport 2.28: Start testing the PSA built-in drivers: hashes #8222

Backport 2.28: Start testing the PSA built-in drivers: hashes #8222

tgonzalezorlandoarm commented Sep 18, 2023

tgonzalezorlandoarm commented Sep 18, 2023

gilles-peskine-arm commented Sep 18, 2023

tgonzalezorlandoarm commented Sep 19, 2023

gilles-peskine-arm commented Sep 29, 2023

gilles-peskine-arm left a comment •

edited

Loading

gilles-peskine-arm Sep 29, 2023

tgonzalezorlandoarm Oct 30, 2023

gilles-peskine-arm left a comment

gilles-peskine-arm Oct 31, 2023

minosgalanakis commented Nov 10, 2023 •

edited

Loading

minosgalanakis left a comment

gilles-peskine-arm left a comment

		@@ -0,0 +1,162 @@
		"""Collect information about PSA cryptographic mechanisms.

		if not all((dep.lstrip('!') in _implemented_dependencies or
		not dep.lstrip('!').startswith('PSA_WANT'))

Backport 2.28: Start testing the PSA built-in drivers: hashes #8222

Backport 2.28: Start testing the PSA built-in drivers: hashes #8222

Conversation

tgonzalezorlandoarm commented Sep 18, 2023

Description

PR checklist

Notes for the submitter

tgonzalezorlandoarm commented Sep 18, 2023

gilles-peskine-arm commented Sep 18, 2023

tgonzalezorlandoarm commented Sep 19, 2023

gilles-peskine-arm commented Sep 29, 2023

gilles-peskine-arm left a comment • edited Loading

Choose a reason for hiding this comment

gilles-peskine-arm Sep 29, 2023

Choose a reason for hiding this comment

tgonzalezorlandoarm Oct 30, 2023

Choose a reason for hiding this comment

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

gilles-peskine-arm Oct 31, 2023

Choose a reason for hiding this comment

minosgalanakis commented Nov 10, 2023 • edited Loading

minosgalanakis left a comment

Choose a reason for hiding this comment

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

gilles-peskine-arm left a comment •

edited

Loading

minosgalanakis commented Nov 10, 2023 •

edited

Loading