Validate values of PSA Crypto macros against spec #3936

bensze01 · 2020-12-03T18:30:20Z

Description

This PR adds a test case which validates the values of the PSA macros in our headers (except _IS_ and _GET_ macros already covered elsewhere) against the PSA crypto API spec.

It also warns about macros present in the spec, but not defined by us.

Closes #3922

Status

IN DEVELOPMENT

Requires Backporting

No, PSA only

Migrations

Still need to update the PSA macros in our headers to match the spec.

Additional comments

I'll push a new commit to update PSA macros after verifying the tests fail correctly.

Todos

Update PSA macros to their 1.0.0 values

Revision: 76df6622ac6508d477733f5f45f5c0bed472e192 include/psa/crypto.h sphinx-source/appendix/specdef_values.rst Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

bensze01 · 2020-12-04T10:22:27Z

Force-pushed PR to fix a permissions issue. Content unchanged.

gilles-peskine-arm

I gave this a quick overview and I think the test achieves its intended purpose, but the code should be refactored to avoid duplication and the integration isn't quite right. So I propose to use the script manually for now to validate that we make the right changes to crypto*.h to conform to the PSA specification 1.0.1, and take our time to finish the script and include it in our tests.

gilles-peskine-arm · 2020-12-06T21:14:12Z

scripts/generate_verify_psa_macros.py

+    Based on the MacroCollector in generate_psa_constants.py,
+    but *just* different enough not to warrant inheriting anything from it.
+
+    Should probably create a shared base class for the two of them at some point.


Yes. Let's use this script to validate that we get the values right, but not merge it until we've cleaned it up and got rid of code duplication.

gilles-peskine-arm · 2020-12-06T21:16:05Z

scripts/generate_verify_psa_macros.py

+#endif
+'''
+
+class MacroCollector():


For information, I'm working on a script that generates storage test cases, and it also uses the MacroCollector class. So far I've only made a tiny patch and then I derive from it. As discussed internally, I'm going to create a directory scripts/mbedtls_dev and start putting non-executable Python modules there.

#3953 creates scripts/mbedtls_dev. MacroCollector could be moved to a new module there. (import generate_psa_constants.MacroCollector would work, but creating a new module is cleaner.)

gilles-peskine-arm · 2020-12-06T21:18:16Z

tests/scripts/all.sh

+component_check_verify_psa_macros () {
+    msg "Check: verify PSA Crypto macros against spec" # ~ 2s
+    make -C programs test/verify_psa_macros
+    if_build_succeeded programs/test/verify_psa_macros


This is independent of the library configuration, so I think it should be part of an existing component, like tests/scripts/test_psa_constant_names.py.

gilles-peskine-arm · 2020-12-06T21:20:16Z

scripts/generate_verify_psa_macros.py

+        os.chdir('..')
+    generate_verify_psa_macros(('scripts/data_files/psa_spec/crypto.h',
+                                'scripts/data_files/psa_spec/specdef_values.rst'),
+                               'programs/test/verify_psa_macros.c')


Is it useful to have verify_psa_macros.c as a test program checked into the repository? It seems to me like this program has no value except to compile it, run it and see its output as part of a test, so it could be generated and run on the fly like test_psa_constant_names.py does.

gilles-peskine-arm · 2020-12-06T21:25:52Z

scripts/data_files/psa_spec/crypto.h

@@ -0,0 +1,562 @@
+typedef /* implementation-defined type */ psa_aead_operation_t;


This is generated from a commit which is slightly anterior to the psa-crypto-api-1.0.1 tag, and slightly different (1.0.1 has additional lifetime-related macros). Let's base our copy on the official released version.

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Tag: psa-crypto-api-1.0.1 Revision: 9ce27ef1c6b196184cf7b1665b62c0371d1b81b6 include/psa/crypto.h sphinx-source/appendix/specdef_values.rst Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

To PSA Crypto API version 1.0.1 Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

For PSA Crypto API 1.0.1 Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

gilles-peskine-arm · 2020-12-07T21:31:32Z

scripts/generate_verify_psa_macros.py

+{header}
+
+#define hash_alg    PSA_ALG_SHA_256
+#define mac_alg     PSA_ALG_CMAC


Missing: aead_alg (for PSA_ALG_AEAD_WITH_DEFAULT_LENGTH_TAG and PSA_ALG_AEAD_WITH_SHORTENED_TAG).

gilles-peskine-arm · 2020-12-07T21:31:45Z

scripts/generate_verify_psa_macros.py

+#define mac_alg     PSA_ALG_CMAC
+#define ka_alg      PSA_ALG_ECDH
+#define kdf_alg     PSA_ALG_HKDF(PSA_ALG_SHA_256)
+#define mac_length  4


Missing: tag_length (for PSA_ALG_AEAD_WITH_SHORTENED_TAG).

gilles-peskine-arm · 2020-12-07T21:32:18Z

scripts/generate_verify_psa_macros.py

+{header}
+
+#define hash_alg    PSA_ALG_SHA_256
+#define mac_alg     PSA_ALG_CMAC


Using a single value isn't enough to test the shortened/default-length macros. There needs to be at least one shortened value as well.

More generally, this program should test several parameter values for each type of parameter. Possibly have the Python code enumerate all possible arguments by reusing code from test_psa_constant_names.py.

yanesca · 2021-09-07T11:09:12Z

This is a PR, moved the corresponding issue (#3922) into the epic and backlog boards instead. I have added it to the unified board as well.

tom-daubney-arm · 2023-06-02T11:15:16Z

We are now converting older PRs to draft PRs where the following conditions are met: They have not been updated in the last 3 months, and they need more than non-trivial work to complete.

bensze01 requested a review from gilles-peskine-arm December 3, 2020 18:34

bensze01 added mbed TLS team needs-ci Needs to pass CI tests needs-review Every commit must be reviewed by at least two team members, labels Dec 3, 2020

Import example headers from PSA Crypto API spec

510cfbe

Revision: 76df6622ac6508d477733f5f45f5c0bed472e192 include/psa/crypto.h sphinx-source/appendix/specdef_values.rst Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

bensze01 force-pushed the psa_validate branch from 72523c6 to 71c509d Compare December 3, 2020 19:05

Validate values of PSA Crypto macros against spec

75c5710

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

bensze01 force-pushed the psa_validate branch from 71c509d to 75c5710 Compare December 4, 2020 10:18

gilles-peskine-arm reviewed Dec 6, 2020

View reviewed changes

bensze01 added 2 commits December 7, 2020 11:57

Sort macros to preserve order on python <= 3.5

07ef428

Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Update example headers to PSA Crypto API 1.0.1

79670d7

Tag: psa-crypto-api-1.0.1 Revision: 9ce27ef1c6b196184cf7b1665b62c0371d1b81b6 include/psa/crypto.h sphinx-source/appendix/specdef_values.rst Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

bensze01 force-pushed the psa_validate branch from efea61e to 6bb6395 Compare December 7, 2020 10:58

bensze01 added 2 commits December 7, 2020 11:59

Update list of macro arguments

4f7c043

To PSA Crypto API version 1.0.1 Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

Regenerate verify_psa_macros.c

4b11b25

For PSA Crypto API 1.0.1 Signed-off-by: Bence Szépkúti <bence.szepkuti@arm.com>

bensze01 force-pushed the psa_validate branch from 6bb6395 to 4b11b25 Compare December 7, 2020 11:01

bensze01 added needs-work and removed needs-review Every commit must be reviewed by at least two team members, labels Dec 7, 2020

bensze01 removed the needs-ci Needs to pass CI tests label Dec 7, 2020

gilles-peskine-arm reviewed Dec 7, 2020

View reviewed changes

gilles-peskine-arm mentioned this pull request Dec 7, 2020

Update the values of PSA Crypto API macros to version 1.0.0 #3948

Merged

2 tasks

mpg self-requested a review December 8, 2020 09:04

bensze01 self-assigned this Jan 18, 2021

gilles-peskine-arm mentioned this pull request Jan 18, 2021

Test not-supported cases in PSA crypto #4051

Open

bensze01 added the Product Backlog label Jul 12, 2021

yanesca removed the Product Backlog label Sep 7, 2021

daverodgman added the historical-reviewed Reviewed & agreed to keep legacy PR/issue label May 6, 2022

daverodgman added component-psa PSA keystore/dispatch layer (storage, drivers, …) priority-medium Medium priority - this can be reviewed as time permits and removed mbed TLS team labels May 13, 2022

tom-daubney-arm marked this pull request as draft June 2, 2023 11:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate values of PSA Crypto macros against spec #3936

Validate values of PSA Crypto macros against spec #3936

bensze01 commented Dec 3, 2020 •

edited

Loading

bensze01 commented Dec 4, 2020

gilles-peskine-arm left a comment

gilles-peskine-arm Dec 6, 2020

gilles-peskine-arm Dec 6, 2020

gilles-peskine-arm Dec 10, 2020

gilles-peskine-arm Dec 6, 2020

gilles-peskine-arm Dec 6, 2020

gilles-peskine-arm Dec 6, 2020

gilles-peskine-arm Dec 7, 2020

gilles-peskine-arm Dec 7, 2020

gilles-peskine-arm Dec 7, 2020

gilles-peskine-arm Dec 7, 2020 •

edited

Loading

yanesca commented Sep 7, 2021

tom-daubney-arm commented Jun 2, 2023

		@@ -0,0 +1,562 @@
		typedef /* implementation-defined type */ psa_aead_operation_t;

Validate values of PSA Crypto macros against spec #3936

Are you sure you want to change the base?

Validate values of PSA Crypto macros against spec #3936

Conversation

bensze01 commented Dec 3, 2020 • edited Loading

Description

Status

Requires Backporting

Migrations

Additional comments

Todos

bensze01 commented Dec 4, 2020

gilles-peskine-arm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gilles-peskine-arm Dec 7, 2020 • edited Loading

Choose a reason for hiding this comment

yanesca commented Sep 7, 2021

tom-daubney-arm commented Jun 2, 2023

bensze01 commented Dec 3, 2020 •

edited

Loading

gilles-peskine-arm Dec 7, 2020 •

edited

Loading