Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

[ssl] Migrate to mbedtls 3 #290

Merged
merged 8 commits into from
Jul 3, 2024
Merged

Conversation

tobil4sk
Copy link
Member

@tobil4sk tobil4sk commented Jul 2, 2024

Closes #252

Adapted from HaxeFoundation/hashlink#648

tobil4sk added 2 commits July 2, 2024 16:35
This means that mbedtls 3 is supported for dynamic builds
@tobil4sk tobil4sk force-pushed the feature/mbedtls3 branch from 13cc88d to e73664b Compare July 2, 2024 17:19
@tobil4sk tobil4sk changed the title Migrate to mbedtls 3 [ssl] Migrate to mbedtls 3 Jul 2, 2024
tobil4sk and others added 3 commits July 3, 2024 09:54
Use MBEDTLS_USER_CONFIG_FILE instead of patching the sources.
In mbedtls 3.6 TLS 1.3 support is turned on by default which uses PSA crypto

See HaxeFoundation/hashlink#681
Xenial went EoL in 2021, and we cannot build new mbedtls versions on it.
@tobil4sk tobil4sk force-pushed the feature/mbedtls3 branch from e73664b to 47d72ff Compare July 3, 2024 08:58
@tobil4sk tobil4sk marked this pull request as ready for review July 3, 2024 13:15
@@ -1,3 +1,4 @@
#define WIN32_LEAN_AND_MEAN
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, I thought this define was to exclude some things. How does this "fix" anything?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Later on after including threading_alt.h, mbedtls files go on to include other headers (presumably winsock2.h). But since the full windows.h has already been included, this results in re-definitions and compilation errors. By setting WIN32_LEAN_AND_MEAN, a lot of the unnecessary definitions are excluded from windows.h which avoids these conflicts later on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I mean this is a good define to make regardless, I just find it strange that it's strictly necessary for compilation. But then again it probably won't matter. Thanks!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just about the order of includes. Including windows.h in threading_alt.h like we do results in this ordering, which breaks compilation:

#include <windows.h> // threading_alt.h
// ...
#include <winsock2.h> // mbedtls sources

Since at the time when windows.h is included we haven't included winsock2.h yet, windows.h includes winsock.h instead which contains conflicting definitions. Then when winsock2.h is included we get redefinition errors.

If WIN32_LEAN_AND_MEAN is defined, then winsock.h is excluded from windows.h so we don't end up with these redefinition errors when including winsock2.h.

More details here: https://stackoverflow.com/questions/21399650/cannot-include-both-files-winsock2-windows-h

@Simn Simn merged commit 31e9db6 into HaxeFoundation:master Jul 3, 2024
16 checks passed
@tobil4sk tobil4sk deleted the feature/mbedtls3 branch July 3, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libmbedtls 3 not supported
3 participants