constant_time.h: add #ifdef __cplusplus guard

[nbfalcon]

All other headers have a __cplusplus guard, except for constant_time.h**. This allows it to be conveniently included in C++ code.

** as verified using grep -L "#ifdef _cplusplus" mbedtls/.h psa/.h, excepting "config" headers.

Workaround: Users using the older library can simply work around this by

extern "C" {
#include <mbedtls/constant_time.h>
}

Description

Allow constant_time.h to be included in a c++ source file without linker errors.

PR checklist

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

• changelog not required, since this is a trivial change?
• 3.6 backport TODO
• 2.28 backport TODO
• tests not required, since this does not change run-time behaviour. A check for this could be done using a lint?

Notes for the submitter

Please refer to the contributing guidelines, especially the
checklist for PR contributors.

Help make review efficient:

• Multiple simple commits
  • please structure your PR into a series of small commits, each of which does one thing
• Avoid force-push
  • please do not force-push to update your PR - just add new commit(s)
• See our Guidelines for Contributors for more details about the review process.

[gilles-peskine-arm]

LGTM

[gilles-peskine-arm]

We currently validate C++ compatibility only with by compiling a trivial program that includes all headers but does nothing. This rejects, for example, extern "C" { without the matching }, but doesn't detect if it's misplaced or missing. Can we do better and catch a missing or misplaced extern "C", without going through the trouble of calling every function?

[nbfalcon]

An idea would be to add a lint to CI that:

• Has a list of "important" headers that actually declare functions. I think there are some macro-only header-files, which don't need extern C.
• Greps each file in the list for the exact multi-line strings

#ifdef __cplusplus
extern "C" {
#endif

I can extend the script you linked if you'd like.

[nbfalcon]

These are the errors generated currently for ref. We'd need some more elegant way to mark the headers that are to be excluded.

nikita@fedora-3:~/src/mbedtls$ ./programs/test/generate_cpp_dummy_build.sh
Error: include/mbedtls/build_info.h does not contain a __cplusplus guard
Error: include/mbedtls/check_config.h does not contain a __cplusplus guard
Error: include/mbedtls/compat-2.x.h does not contain a __cplusplus guard
Error: include/mbedtls/private_access.h does not contain a __cplusplus guard
Error: include/mbedtls/psa_util.h does not contain a __cplusplus guard
Error: include/psa/build_info.h does not contain a __cplusplus guard
Error: include/psa/crypto_adjust_auto_enabled.h does not contain a __cplusplus guard
Error: include/psa/crypto_adjust_config_key_pair_types.h does not contain a __cplusplus guard
Error: include/psa/crypto_adjust_config_synonyms.h does not contain a __cplusplus guard
Error: include/psa/crypto_builtin_composites.h does not contain a __cplusplus guard
Error: include/psa/crypto_builtin_key_derivation.h does not contain a __cplusplus guard
Error: include/psa/crypto_builtin_primitives.h does not contain a __cplusplus guard
Error: include/psa/crypto_driver_common.h does not contain a __cplusplus guard
Error: include/psa/crypto_driver_contexts_composites.h does not contain a __cplusplus guard
Error: include/psa/crypto_driver_contexts_key_derivation.h does not contain a __cplusplus guard
Error: include/psa/crypto_driver_contexts_primitives.h does not contain a __cplusplus guard
Error: include/psa/crypto_legacy.h does not contain a __cplusplus guard
Error: include/psa/crypto_platform.h does not contain a __cplusplus guard
Error: include/psa/crypto_sizes.h does not contain a __cplusplus guard
Error: include/psa/crypto_types.h does not contain a __cplusplus guard
Error: include/psa/crypto_values.h does not contain a __cplusplus guard
Some headers are missing __cplusplus guards

[gilles-peskine-arm]

Thanks for checking! The only testing we're currently doing is compiling a generated program in one configuration. (Testing more configurations would be good, but out of scope here.)

Several headers are skipped because they aren't meant to be included directory. In addition, some headers only declare macros (*/build_info.h, */private_access.h). That leaves mbedtls/compat-2.x.h (can't be very important since nobody's complained) and mbedtls/psa_util.h (a recent addition). Would you mind adding guards to those as well while you're at it?

[gilles-peskine-arm]

I won't ask for a non-regression test because this is a long-standing issue and we don't add headers often. But if you're willing to dive into it, linting is done from the component_check_* functions in tests/scripts/all.sh.

[gilles-peskine-arm]

I consider this a bug in 3.28 and 3.6, so I would prefer to backport it. However, we maintain ABI compatibility in LTS branches. We've never explicitly stated if that's just about the ABI itself, or also encompasses the ABI as seen through C++ linkage. Would adding extern "C" be considered an ABI break? I think not, since it doesn't change the generated binary, but maybe I'm missing something? What about a library that was written in C++ and built against Mbed TLS 3.6.0: would this patch break it if it's applied to 3.6.1?

[nbfalcon]

No, this does not affect the ABI at all, since the function definition itself is still written in C and compiled as such (i.e. not compiled by g++). The only problem was that the C++ declaration was wrong, causing a linker error.

I noticed this myself in a platformio project where I included this library (as a git submodule, not the outdated package), and found a strange link error, where only mbedtls_ct_memcmp wasn't found.

As for C++ linkage: well including this file without wrapping it in extern "C" just causes a link error, so there are no well-formed programs that could break by this.

[gilles-peskine-arm]

Thanks for the information! Then please make backports to the mbedtls-3.6 and mbedtls-2.28 long-time support branches.