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.24k stars 2.56k forks source link

Adapt config.py to configuration file split #9229

Closed gabor-mezei-arm closed 1 month ago

gabor-mezei-arm commented 3 months ago

Description

For the repo split, we plan to move all configuration options but the TLS and x509 ones from mbedtls_config.h to crypto_config.h that will become the TF-PSA-Crypto configuration file and probably be renamed tf_psa_crypto_config.h.

We need to adapt config.py. Same options, same interface but handles both mbedtls_config.h and tf_psa_crypto_config.h.

Resolve #9158 Depends on Mbed-TLS/mbedtls-framework#35

PR checklist

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

gabor-mezei-arm commented 3 months ago

In addition to that I think we should think about moving a decent chunk of the code into the framework repository.

Due to dependencies a separate issue (#9325) is created for it.

tom-daubney-arm commented 3 months ago

We have some CI issues which I have posted here:

[2024-07-01T12:58:13.652Z] ******************************************************************

[2024-07-01T12:58:13.652Z] * check_python_files: Lint: Python scripts 

[2024-07-01T12:58:13.652Z] * Mon Jul  1 12:58:13 UTC 2024

[2024-07-01T12:58:13.652Z] ******************************************************************

[2024-07-01T12:58:13.652Z] Running pylint ...

[2024-07-01T12:58:45.710Z] ************* Module config

[2024-07-01T12:58:45.710Z] scripts/config.py:483:40: W0613: Unused argument 'name' (unused-argument)

[2024-07-01T12:58:45.710Z] scripts/config.py:483:4: R0201: Method could be a function (no-self-use)

[2024-07-01T12:58:45.710Z] 
tom-daubney-arm commented 2 months ago

Just a note on the testing I have been doing on the latest commits. I have been invoking the various CLI arguments such as full, realfull etc and this appears to behave as I would expect which is great. I have also been using options like set-all/unset-all and then giving a list of options from both configs and they all seem to get set correctly and seamlessly.

Does this sound like sufficient testing @gabor-mezei-arm ? Or do you have any suggestions for additional things that I should try?

gilles-peskine-arm commented 2 months ago

We don't have tests for config.py as such. But for what it's worth, there's a script tests/scripts/test_config_script.py that tests whether two implementations of config.py have the same behavior. You can use it to validate refactorings (with a copy of the old version of config.py: the test script doesn't access git). Note that we don't run test_config_script.py often, so it may have bitrotted.

tom-daubney-arm commented 2 months ago

Thanks for the info, will take a look. I know we don't test config.py in the usual way, I was wondering if there was maybe further testing I could do, that Gabor has tried while developing this, which might be worth me doing as well. But other than what you mentioned I think I have probably covered enough bases now.

gabor-mezei-arm commented 2 months ago

Does this sound like sufficient testing @gabor-mezei-arm ? Or do you have any suggestions for additional things that I should try?

There is test script (test_config_script.py) written for config.py but is not updated for the crypto config.

gabor-mezei-arm commented 2 months ago

Rebased due to merge conflict

tom-daubney-arm commented 2 months ago

Looks like the latest changes have caused issues in generate_config_tests.py:

[2024-07-04T14:16:53.179Z] Traceback (most recent call last):
[2024-07-04T14:16:53.179Z]   File "../framework/scripts/generate_config_tests.py", line 179, in <module>
[2024-07-04T14:16:53.179Z]     test_data_generation.main(sys.argv[1:], __doc__, ConfigTestGenerator)
[204-07-04T14:16:53.179Z]   File "/var/lib/build/framework/scripts/mbedtls_framework/test_data_generation.py", line 204, in main
[2024-07-04T14:16:53.179Z]     generator = generator_class(options)
[2024-07-04T14:16:53.179Z]   File "../framework/scripts/generate_config_tests.py", line 162, in __init__
[2024-07-04T14:16:53.179Z]     self.mbedtls_config = config.ConfigFile()
[2024-07-04T14:16:53.179Z] TypeError: __init__() missing 2 required positional arguments: 'default_path' and 'name'
[2024-07-04T14:16:53.179Z] Makefile:34: *** "python3 ../framework/scripts/generate_config_tests.py --list" failed.  Stop.
[2024-07-04T14:16:53.179Z] make: *** [clean] Error 2
[2024-07-04T14:16:53.179Z] Makefile:154: recipe for target 'clean' failed
gabor-mezei-arm commented 2 months ago

Rebased due to merge conflict

ronald-cron-arm commented 1 month ago

@tom-daubney-arm please have another look