[3.6] Fix build C++ apps with MSVC

[aslze]

Description

This fixes Bug #7087 in the 3.6 branch (unable to build C++ apps that use mbedTLS on MSVC 2019 Windows).

Ports the main commit in TF-PSA-Crypto#258, changing crypto_extra.h. Also fixes building a couple test programs inside mbedtls due to a warning in winbase.h in C11 mode.

PR checklist

Please remove the segment/s on either side of the | symbol as appropriate, and add any relevant link/s to the end of the line. If the provided content is part of the present PR remove the # symbol.

• [x] changelog provided.
• [x] development PR not required because: done Mbed-TLS/TF-PSA-Crypto#258
• [x] TF-PSA-Crypto PR Mbed-TLS/TF-PSA-Crypto#258
• [x] framework PR not required
• [x] 3.6 PR provided
• tests not required because: minor changes, existing tests are enough

[aslze]

Oops. I missed the signing-off part. That means commits must be done by actual team members and not external contributors? How can this proceed?

[gilles-peskine-arm]

@aslze Thanks for the contribution. You need to add a sign-off line to every commit message, to convey that you are allowed to make this contribution and that you allow us to use it as described in dco.txt.

Practically, you can run git rebase -i --signoff mbedtls-3.6 to add a signoff line to the new commits since mbedtls-3.6, or e.g. git rebase -i --signoff HEAD~3 to sign off the last 3 commits.

[ronald-cron-arm]

@aslze thanks for this pull request. Revisiting the changes in crypto_extra.h in this LTS context, I think we should do better actually in terms of not changing the Doxygen documentation and aligning with the re-organization done in crypto.h (#10196). Instead of 101b0e9d51a7ceafe7b9cb13d2bf0375234d63e6 I propose the two last commits of https://github.com/ronald-cron-arm/mbedtls/tree/fix-crypto-extra-for-msvc-3.6. I've tested the changes against our CI and I believe they should fix the issue MSVC as well. Would you be able to check that it is actually the case?

[aslze]

@aslze thanks for this pull request. Revisiting the changes in crypto_extra.h in this LTS context, I think we should do better actually in terms of not changing the Doxygen documentation and aligning with the re-organization done in crypto.h (#10196). Instead of 101b0e9 I propose the two last commits of https://github.com/ronald-cron-arm/mbedtls/tree/fix-crypto-extra-for-msvc-3.6. I've tested the changes against our CI and I believe they should fix the issue MSVC as well. Would you be able to check that it is actually the case?

I'll check later today. I don't understand the reference to Doxygen. IIRC, the idea of the commit was to move the definition of some structs up in the file. Struct names are the same, I think.

[aslze]

@ronald-cron-arm yes, those two commits fix the issue, thanks.

Would you add this commit from my PR? It's a tiny change. Fixes compilation of two test programs due to an apparent bug in a Windows header in C11 mode (as the compilation sets warnings as errors).

[ronald-cron-arm]

@ronald-cron-arm yes, those two commits fix the issue, thanks.

Would you add this commit from my PR? It's a tiny change. Fixes compilation of two test programs due to an apparent bug in a Windows header in C11 mode (as the compilation sets warnings as errors).

Yes sure. Actually, would you mind updating this PR with the two last commits of https://github.com/ronald-cron-arm/mbedtls/tree/fix-crypto-extra-for-msvc-3.6 instead of https://github.com/Mbed-TLS/mbedtls/commit/101b0e9d51a7ceafe7b9cb13d2bf0375234d63e6 and address https://github.com/Mbed-TLS/mbedtls/pull/10200#discussion_r2125806434?

Otherwise, if you prefer I can create a new PR based on https://github.com/ronald-cron-arm/mbedtls/tree/fix-crypto-extra-for-msvc-3.6 including your commit.

[ronald-cron-arm]

@aslze Thanks for the update. I've just undone the squash in the last commit to three commits. That's better for the git history and that should help for the reviews. And I've started a run of CI.

[aslze]

Can someone please take a look. The system is complaining about API changes in the naming of some structs:

psa_crypto_driver_pake_inputs_t -> struct psa_crypto_driver_pake_inputs_s
psa_pake_cipher_suite_t -> struct psa_pake_cipher_suite_s
psa_jpake_computation_stage_t -> struct psa_jpake_computation_stage_s

But I thought that was intentional and the _t types are aliases of the _s structs. So there is no API breakage.

[ronald-cron-arm]

Can someone please take a look. The system is complaining about API changes in the naming of some structs:

psa_crypto_driver_pake_inputs_t -> struct psa_crypto_driver_pake_inputs_s
psa_pake_cipher_suite_t -> struct psa_pake_cipher_suite_s
psa_jpake_computation_stage_t -> struct psa_jpake_computation_stage_s

But I thought that was intentional and the _t types are aliases of the _s structs. So there is no API breakage.

Yes to me this is a false positive of our API/ABI checker script.

[ronald-cron-arm]

While reviewing Mbed-TLS/TF-PSA-Crypto#258 I noticed that commit #31f7446957212fb95089bdb45c87a631fefaa8e1 is missing.

It looks like a minor fix that is easy to back-port. If it was needed in the tf-psa-crypto version we should have it here too.

I've added it.

[irwir]

Yes to me this is a false positive of our API/ABI checker script.

Not really. Declarations and definitions should match fully. As was explained in my issue, such differences could break navigation to declaration/definition in IDE, and static analysis might generate diagnostic messages. The issue was closed as not planned.