Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

CMake install picks up the build_info.h from TF-PSA-Crypto #10022

Open
gstrauss opened this issue Mar 2, 2025 · 4 comments
Open

CMake install picks up the build_info.h from TF-PSA-Crypto #10022

gstrauss opened this issue Mar 2, 2025 · 4 comments
Labels
bug component-platform Portability layer and build scripts

Comments

@gstrauss
Copy link
Contributor

gstrauss commented Mar 2, 2025

Summary

mbedtls development branch mbedtls/build_info.h no longer defines MBEDTLS_VERSION_NUMBER

The entire contents of mbedtls/build_info.h is effectively only #include "tf-psa-crypto/build_info.h"

System information

Mbed TLS version (number or commit id): 243e13f
Operating system and version: Fedora Linux 41, x86_64 architecture
Configuration (if not default, please attach mbedtls_config.h):
Compiler and options (if you used a pre-built binary, please indicate how you obtained it):

$ gcc --version
gcc (GCC) 14.2.1 20250110 (Red Hat 14.2.1-7)

Additional environment information:

Expected behavior

MBEDTLS_VERSION_NUMBER should be defined.

Actual behavior

MBEDTLS_VERSION_NUMBER is not defined.

Steps to reproduce

$ git clone https://github.com/Mbed-TLS/mbedtls
$ cd mbedtls
$ git submodule update --init
$ cd tf-psa-crypto
$ git submodule update --init
$ cd ..
$ cmake -DUSE_SHARED_MBEDTLS_LIBRARY=On -DCMAKE_INSTALL_PREFIX=/dev/shm/opt .
$ cmake --build .
$ cmake --install . 

$ gcc -I/dev/shm/opt/include -dM -E - < /dev/shm/opt/include/mbedtls/version.h | grep MBEDTLS_VERSION_NUMBER

Additional information

mbedtls/build_info.h should also #include <mbedtls/mbedtls_config.h>
mbedtls/mbedtls_config.h is not present in mbedtls 2.x. IIRC, that used config.h.
For portability, consumers should get the appropriate info from a single #include <mbedtls/version.h>

@gilles-peskine-arm gilles-peskine-arm changed the title mbedtls development branch build_info.h no longer defines MBEDTLS_VERSION_NUMBER CMake install picks up the build_info.h from TF-PSA-Crypto Mar 2, 2025
@gilles-peskine-arm
Copy link
Contributor

gilles-peskine-arm commented Mar 2, 2025

Thanks for reporting this! What's going on is that include/mbedtls/build_info.h is perfectly fine. But there is also a file .../mbedtls/build_info.h in TF-PSA-Crypto, which is intended to facilitate the transition for code that uses only crypto and needs to work with both Mbed TLS 3.x and TF-PSA-Crypto 1.x (which includes our test framework). That code can use #if xxx_VERSION ... guards, but it needs a common file name to include.

We should arrange that the mbedtls/build_info.h file in TF-PSA-Crypto is never picked up when TF-PSA-Crypto is used within Mbed TLS. In our own build scripts, we're careful about the order of the include path. That's not good enough in two cases:

  • As you discovered, after installing the headers. CMake happens to first install the one from include/mbedtls/build_info.h, then it calls into the tf-psa-crypto subproject and that overwrites include/mbedtls/build_info.h in the installed tree.
  • In build systems where the order of included directories cannot be controlled. This is a problem for some of our users that's been reported in the past.

As to how to resolve this: as we're making legacy crypto internal, we're planning to sort the headers from tf-psa-crypto/drivers/builtin/include into public ones (which will be installed) and private ones (which will not be installed). The current plan (from Mbed-TLS/TF-PSA-Crypto#145) is to consider mbedtls/build_info.h as private. However, now that I think about it, this would make it hard for applications to work with both Mbed TLS 3.6 and TF-PSA-Crypto 1.x. A possible workaround would be for applications to include some other mbedtls/*.h header that's still public in TF-PSA-Crypto 1.x, such as mbedtls/pk.h.

@gilles-peskine-arm gilles-peskine-arm added bug component-platform Portability layer and build scripts labels Mar 2, 2025
@gilles-peskine-arm
Copy link
Contributor

Note that although the bug only affects Mbed TLS users, the fix will be partially, maybe even fully in TF-PSA-Crypto.

@gstrauss
Copy link
Contributor Author

gstrauss commented Mar 3, 2025

Thank you for the detailed response.

Please keep in mind that I have filed a simple bug against mbedtls which trivially shows that including <mbedtls/version.h> no longer includes expected version information for mbedtls. build_info.h is an implementation detail, as is the rest of your explanation. I should have focused the bug on <mbedtls/version.h> rather than the implementation detail build_info.h in which I narrowed down the location of the bug.

I wrote:

For portability, consumers should get the appropriate version info from a single #include <mbedtls/version.h>

A proper fix for this bug is for <mbedtls/version.h> to include mbedtls version information, like it works in mbedtls 2.x and 3.x. Applications using mbedtls should not have to be modified to get version information about mbedtls.

@gilles-peskine-arm
Copy link
Contributor

build_info.h is an implementation detail, as is the rest of your explanation

Well, yes, I'm analyzing the cause of the bug. The fix is unlikely to involve any change to version.h. The actual lines with #define MBEDTLS_VERSION_... are in the file build_info.h. And the cause of the bug is that in some circumstances — which our test scripts do not emulate, but which are common for users — the wrong build_info.h gets picked up.

The missing version macros are just one symptom. Another is that the configuration isn't getting picked up! The primary job of build_info.h is to include the configuration files (mbedtls_config.h and so on), and to define derived macros. With the wrong build_info.h, only the crypto configuration is getting defined and not the X.509/TLS part.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
bug component-platform Portability layer and build scripts
Projects
None yet
Development

No branches or pull requests

2 participants