mbedtls icon indicating copy to clipboard operation
mbedtls copied to clipboard

config.py does not support some custom edits to mbedtls_config.h

Open borrrden opened this issue 8 months ago • 8 comments

Summary

config.py works in 3.6.3, but not 3.6.4 (scripting errors)

System information

Mbed TLS version (number or commit id): 3.6.4 Operating system and version: Any Configuration (if not default, please attach mbedtls_config.h): n/a Compiler and options (if you used a pre-built binary, please indicate how you obtained it): Additional environment information: Tested with python 3.8, 3.10, and 3.13

Expected behavior

config.py functions correctly

Actual behavior

Traceback (most recent call last):
  File ".\vendor\mbedtls\scripts\config.py", line 413, in <module>
    sys.exit(MbedTLSConfigTool().main())
  File ".\vendor\mbedtls\scripts\config.py", line 362, in __init__
    self.config = MbedTLSConfig(self.args.file)
  File ".\vendor\mbedtls\scripts\config.py", line 315, in __init__
    self.settings.update({name: config_common.Setting(configfile, active, name, value, section)
  File ".\vendor\mbedtls\scripts\config.py", line 315, in <dictcomp>
    self.settings.update({name: config_common.Setting(configfile, active, name, value, section)
  File "[...]\vendor\mbedtls\scripts\..\framework\scripts\mbedtls_framework\config_common.py", line 296, in parse_file
    setting = self._parse_line(line)
  File "[...]\vendor\couchbase-lite-core\vendor\mbedtls\scripts\..\framework\scripts\mbedtls_framework\config_common.py", line 285, in _parse_line
    m.group('define') + name +
TypeError: unsupported operand type(s) for +: 'NoneType' and 'NoneType'

Steps to reproduce

python scripts/config.py

Additional information

This is almost certainly caused by the migration to the "framework" submodule, which contains the actual config logic, while the top level config is in a separate repo (this one).

borrrden avatar Sep 11 '25 00:09 borrrden

I can't reproduce this: python3.N scripts/config.py gives me the expected usage message for all 6 ≤ N ≤ 14, when I run it from git checkout --recurse-submodules mbedtls-3.6.4 or from the archives attached to the 3.6.4 release. (Except the broken archives automatically added by GitHub, but there the error is ModuleNotFoundError: No module named 'mbedtls_framework', because the whole framework subdirectory is missing.)

Can you please share exactly how you constructed your working directory?

gilles-peskine-arm avatar Sep 11 '25 08:09 gilles-peskine-arm

You are right! For full transparency we are using a fork but I didn't mention that because we did not touch config.py. That was my mistake and I should know better since I've seen the butterfly effect happen like this many times. I tracked it down to a change we made to the mbedtls_config.h file.

We wanted to turn on threading, but it seems there is no Windows based threading solution so we conditionally enabled our mbedtls_config.h as so:

https://github.com/couchbasedeps/mbedtls/commit/4f44810ff8fa01114804784c5846afeb2f570bed

I think somehow the ifndef are confusing it?

borrrden avatar Sep 11 '25 21:09 borrrden

The problem is that the regex is looking for ifndef for reasons unrelated to why we have them. In fact I cannot tell why it is caring about it. There is a comment there that I can't quite understand but if I change this line https://github.com/Mbed-TLS/mbedtls-framework/blob/2a3e2c5ea053c14b745dbdf41f609b1edc6a72fa/scripts/mbedtls_framework/config_common.py#L275 to elif not m.group('inclusion_guard'): then I can get rid of the error.

borrrden avatar Sep 11 '25 22:09 borrrden

config.py was designed to understand the file we ship and toggle // in front of #define lines. I'm not surprised that it trips up if you add some unexpected structure.

It cares about #ifndef because older versions of the library had a double-inclusion guard in config.h. Since Mbed TLS 3.0, the file is no longer included directly so it no longer has a double-inclusion guard, but we forgot to remove that condition in config.py.

If you have a patch that works for you, please make a pull request. But we can't guarantee that config.py would keep working with a config file that has conditionals.

gilles-peskine-arm avatar Sep 12 '25 10:09 gilles-peskine-arm

We wanted to turn on threading, but it seems there is no Windows based threading solution

We provide a threading abstraction over pthread. For platforms without pthread, you can enable MBEDTLS_THREADING_ALT and provide your own.

Hopefully soon we'll also provide a C11 threading abstraction. (Only in TF-PSA-Crypto 1.x, not in Mbed TLS 3.6.x LTS.) So that should work on recent enough Windows toolchains.

gilles-peskine-arm avatar Sep 12 '25 10:09 gilles-peskine-arm

Thanks for the info. I’m aware of the alt threading implementation and once we implement that I plan to remove the ifndef but it sounds like possibly that entire part of the regex is not needed anymore? It’s automatically triggered as part of the CMake build so ideally either it would be skippable or not fail in this case. Is there another way this script should be used to turn on and off defines based on platform?

borrrden avatar Sep 12 '25 13:09 borrrden

Indeed the special handling of ifndef is not needed anymore.

The call to config.py from CMake is obsolete and should be removed soon in TF-PSA-Crypto. In 3.6, you can skip it if you get CMake to believe that Python is not available (I have no idea how to do that). Note that this means the other files generated by Python need to be already present in the source tree.

If you aren't already using it for something else, you could put your custom config code in a separate file and point MBEDTLS_USER_CONFIG_FILE to that file.

gilles-peskine-arm avatar Sep 12 '25 14:09 gilles-peskine-arm

Scratch that, we still need the #ifndef handling: it's still applicable to crypto_config.h. Although it probably shouldn't be since crypto_config.h is not and was never supposed to be included directly.

gilles-peskine-arm avatar Sep 12 '25 20:09 gilles-peskine-arm