search

Mbed-TLS / mbedtls

An open source, portable, easy to use, readable and flexible TLS library, and reference implementation of the PSA Cryptography API. Releases are on a varying cadence, typically around 3 - 6 months between releases.
https://www.trustedfirmware.org/projects/mbed-tls/
Other
5.22k stars 2.56k forks source link

PSA compliance tests fail in the new coding style #6875

Closed gilles-peskine-arm closed 1 year ago

gilles-peskine-arm commented 1 year ago

Since last night (evening of 2023-01-03), the nightly tests are failing on PSA compliance testing with the new code style. There is no such failure with the official branches.

Failure example: https://jenkins-mbedtls.oss.arm.com/blue/rest/organizations/jenkins/pipelines/mbed-tls-nightly-tests/runs/3118/nodes/432/steps/5850/log/

…
In file included from /var/lib/build/psa-arch-tests/api-tests/val/nspe/val_crypto.c:22:0:
/var/lib/build/psa-arch-tests/api-tests/val/nspe/val_crypto.h:56:0: error: "PSA_ALG_NONE" redefined [-Werror]
 #define PSA_ALG_NONE ((psa_algorithm_t)0)

In file included from /var/lib/build/include/psa/crypto.h:68:0,
                 from /var/lib/build/psa-arch-tests/api-tests/platform/targets/tgt_dev_apis_stdc/nspe/pal_config.h:88,
                 from /var/lib/build/psa-arch-tests/api-tests/platform/targets/common/nspe/pal_common.h:27,
                 from /var/lib/build/psa-arch-tests/api-tests/val/common/val.h:21,
                 from /var/lib/build/psa-arch-tests/api-tests/val/common/val_target.h:21,
                 from /var/lib/build/psa-arch-tests/api-tests/val/nspe/val_crypto.c:18:
/var/lib/build/include/psa/crypto_values.h:822:0: note: this is the location of the previous definition
 #define PSA_ALG_NONE                            ((psa_algorithm_t) 0)

In file included from /var/lib/build/psa-arch-tests/api-tests/val/nspe/val_crypto.c:22:0:
/var/lib/build/psa-arch-tests/api-tests/val/nspe/val_crypto.h:57:0: error: "PSA_KEY_ID_NULL" redefined [-Werror]
 #define PSA_KEY_ID_NULL ((psa_key_id_t)0)

In file included from /var/lib/build/include/psa/crypto.h:68:0,
                 from /var/lib/build/psa-arch-tests/api-tests/platform/targets/tgt_dev_apis_stdc/nspe/pal_config.h:88,
                 from /var/lib/build/psa-arch-tests/api-tests/platform/targets/common/nspe/pal_common.h:27,
                 from /var/lib/build/psa-arch-tests/api-tests/val/common/val.h:21,
                 from /var/lib/build/psa-arch-tests/api-tests/val/common/val_target.h:21,
                 from /var/lib/build/psa-arch-tests/api-tests/val/nspe/val_crypto.c:18:
/var/lib/build/include/psa/crypto_values.h:2088:0: note: this is the location of the previous definition
 #define PSA_KEY_ID_NULL                         ((psa_key_id_t) 0)

cc1: all warnings being treated as errors
make[2]: *** [CMakeFiles/val_nspe.dir/val/nspe/val_crypto.c.o] Error 1
CMakeFiles/val_nspe.dir/build.make:134: recipe for target 'CMakeFiles/val_nspe.dir/val/nspe/val_crypto.c.o' failed
make[2]: *** Waiting for unfinished jobs....
CMakeFiles/Makefile2:178: recipe for target 'CMakeFiles/val_nspe.dir/all' failed
make[1]: *** [CMakeFiles/val_nspe.dir/all] Error 2
make: *** [all] Error 2
Makefile:83: recipe for target 'all' failed
Traceback (most recent call last):
  File "./tests/scripts/test_psa_compliance.py", line 146, in <module>
    sys.exit(main())
  File "./tests/scripts/test_psa_compliance.py", line 91, in main
    subprocess.check_call(['cmake', '--build', '.'])
  File "/usr/lib/python3.6/subprocess.py", line 311, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['cmake', '--build', '.']' returned non-zero exit status 2.
^^^^test_psa_compliance: unit test: test_psa_compliance.py: ./tests/scripts/test_psa_compliance.py -> 1^^^^

******************************************************************
* Done, cleaning up 
* Wed Jan  4 13:46:22 UTC 2023
******************************************************************

!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
FAILED: 1 components
test_psa_compliance: unit test: test_psa_compliance.py: ./tests/scripts/test_psa_compliance.py -> 1
!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
Terminated

Seen both on development (acea8a11e9104657df6ef0ba3a1549b47aee2acc based on 7a389ddc847f25e4b98d07e641dac123d9e7a53c) and 2.28 (f0b24594495edf36d298536e8069031a59552ac6 based on a6ad7f47023b235fc8d02c3f2bbb5d5277c2f20f).

gilles-peskine-arm commented 1 year ago

The culprit is https://github.com/Mbed-TLS/mbedtls/pull/6836. It changes casts to be spaced (type) value instead of (type)value.

I made that change because it aligns with K&R, which is our reference. I may also have been influenced by a weak personal preference for having the space.

The PSA crypto specification does not mandate any particular definition for PSA_KEY_ID_NULL (it just has to have to be a compile-time constant with the correct numerical value and type), and it's a bug in the compliance test that it requires this. However, the PSA crypto specification (as well as the PSA error code specification) does mandate that there is no space in the similar definitions for error codes. For example

#define PSA_ERROR_GENERIC_ERROR         ((psa_status_t)-132)

is correct and

#define PSA_ERROR_GENERIC_ERROR         ((psa_status_t) -132)

is wrong. We will have a test for this once https://github.com/Mbed-TLS/mbedtls/pull/5998 is merged — and it would fail.

So we must preserve the absence of spaces in these casts in PSA macro definitions.