Mbed-TLS
Use native socket type on Windows
Description
Define properly socket type on Windows, and adjust checks for invalid socket. Resolves #10097.
PR checklist
- [x] changelog provided
- [x] TF-PSA-Crypto PR provided TF-PSA-Crypto/#262
- [x] framework PR not required
- [x] 3.6 PR not required because: not planned
- tests not required because: no additional tests needed
Two repositories require changes before test build. There was a question in https://github.com/Mbed-TLS/mbedtls/issues/10097#issuecomment-2797123664; not answered.
Our Windows builds in the CI don't like this patch. I guess because a submodule update is also required?
Note: including a TLS-specific system header in a crypto library header doesn't seem right to me. But I'm not familiar with Windows, so for all I know this may be the only way to make it work.
Our Windows builds in the CI don't like this patch. I guess because a submodule update is also required?
Yes, in the above message "both repositories" implies that build_info.h in TF-PSA-Crypto must be updated.
The first error message clearly indicated that SOCKET macro was never defined.
VS solution builds successfully here.
Note: including a TLS-specific system header in a crypto library header doesn't seem right to me.
This was the main reason for the question in the issue. Theoretically the point is valid, but the current implementation is not clean in this sense. The list of files including at least windows.h is below:
core\psa_its_file.c
drivers\builtin\src\entropy_poll.c
drivers\builtin\src\platform_util.c
drivers\builtin\src\sha256.c
As said earlier, winsock2.h if ever needed, must be included before windows.h. Since this was an SSL/TLS library, networking should be present. Hence this attempt to make the sequence of headers correct in most cases.
Can you please rebase this pull request on top of the latest head (we've made a few incompatible changes recently), then add a commit to update the crypto submodule with https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/262, so that we can get a recent CI run?
add a commit to update the crypto submodule with Mbed-TLS/TF-PSA-Crypto#262, so that we can get a recent CI run
Can this commit be done before merging crypto PR262?
Yes. Please update the submodule to the head of https://github.com/Mbed-TLS/TF-PSA-Crypto/pull/262:
cd tf-psa-crypto
git checkout e2a08a0b7f47cbff34d0918297cc313ed9f79625
cd ..
git add tf-psa-crypto
git commit -m "Update crypto submodule with winsock2 header"
Once the crypto PR is merged, you'll need to amend that commit so that it updates the submodule to the result of the merge. (We haven't found a better way to work with submodules and GitHub, so we make an exception to our general guideline to not rewrite the git history.)
git checkout consistently prints
fatal: unable to read tree (e2a08a0b7f47cbff34d0918297cc313ed9f79625)
Repository checks, re-cloning and reading a few articles did not help so far.
Let's see if .gitmodules can work.
@irwir You need to push the submodule first (to your fork is fine), then push the commit that updates the submodule in the parent repository.
Patched udp_proxy for CI run. The changes might be improved, but now it should compile without warnings.
We haven't found a better way to work with submodules and GitHub
Is it possible to run CI for mbedtls with a framework submodule being different from the framework in TF-PSA-Crypto?
This would be not quite right.