mbedtls3 migration

[Aidan63]

Mbedtls 2.28 goes EOL at the end of this year, so he's a switch over to 3.6.0 which was made LTS a few months ago. Only a few changes were needed to SSL.cpp, for the config file I just copied and pasted the default and change the few things needed.

There are no real ssl tests anywhere so I tested this by creating a quick ssl socket server and client and they were able to connect to non haxe client and servers fine so it seems to work.

Depending on when this gets merged in I'll be pulling these changes into my asys branch so I can build any tls stuff on top of the latest mbedtls version.

[Aidan63]

@tobil4sk Do you know if the c99 flag is needed for mbedtls3? I'm not setup to develop for android but if it still needed I can add that flag to this merge so android will build again when either of these two get through (#1111).

[Aidan63]

Linux CI seems to be failing due to some unrelated mariadb setup issue. Mac and Windows are failing with lots of weird errors when compiling the cppia project.

D:/a/1/s/project/thirdparty/mbedtls-3.6.0/include\psa/crypto.h(110): error C2526: 'psa_key_attributes_init': C linkage function cannot return C++ class 'psa_key_attributes_s'
D:\a\1\s\project\thirdparty\mbedtls-3.6.0\include\psa\crypto_types.h(432): note: see declaration of 'psa_key_attributes_s'

D:\a\1\s\project\thirdparty\mbedtls-3.6.0\include\psa\crypto_extra.h(1874): error C2556: 'psa_pake_operation_s psa_pake_operation_init(void)': overloaded function differs only by return type from 'void psa_pake_operation_init(void)'
D:\a\1\s\project\thirdparty\mbedtls-3.6.0\include\psa\crypto_extra.h(1051): note: see declaration of 'psa_pake_operation_init'

D:\a\1\s\project\thirdparty\mbedtls-3.6.0\include\psa\crypto_extra.h(1873): error C2371: 'psa_pake_operation_init': redefinition; different basic types
D:\a\1\s\project\thirdparty\mbedtls-3.6.0\include\psa\crypto_extra.h(1051): note: see declaration of 'psa_pake_operation_init'

Annoyingly that cppia stage of the CI compiles absolutely fine on locally on my PC, so more investigation needed there...

[Aidan63]

Ahh the joys of C++, the issue seems to be that mbedtls is written in c and uses some c features which were only added in C++20, GCC however has had extensions to add those features in C++ whereas clang and msvc do not.

Mbed-TLS/mbedtls#7087

The visual studio / msvc I have installed must be newer than the CI image and have C++20 enabled by default for it to work fine on my machine. There's still no official fix but I applied one of the patches floating around from issues linked in the above and the Windows and Mac CI now work (mac ci failing due to existing flakey test).

[tobil4sk]

Do you know if the c99 flag is needed for mbedtls3?

Yes, mbedtls is written in c99 so that's what the standard always has to be set to. Older android sdks use an old compiler where this was not yet the default standard, so it has to be set explicitly.

[Aidan63]

Yes, mbedtls is written in c99 so that's what the standard always has to be set to. Older android sdks use an old compiler where this was not yet the default standard, so it has to be set explicitly.

Thanks, I've added that change to the build.xml in this branch as well.

[Simn]

Please update the branch!

[tobil4sk]

Note, lime has previously had issues with the hxcpp mbedtls version conflicting with its own mbedtls version when an hxcpp project is statically linked. openfl/lime#1767

So I'm not sure if this may be considered a breaking change. For example if this is released with hxcpp 4.3.3, then a new release of lime can support hxcpp 4.3.2 or 4.3.3 but potentially not both (at least not without possible issues).

[Aidan63]

I guess this could be held back to haxe 5, but I don't see that coming out this year which means hxcpp will be left using an EOL tls library.

Could lime not shadow cpp.NativeSsl and do some version checks to add build xml for either the hxcpp mbedtls or its own version? (I have zero lime / openfl knowledge)

[tobil4sk]

Could lime not shadow cpp.NativeSsl and do some version checks to add build xml for either the hxcpp mbedtls or its own version? (I have zero lime / openfl knowledge)

The problem is cpp side rather than haxe side. Lime has its own mbedtls version which it has internally in its ndll, separate from hxcpp's mbedtls used by the haxe standard library. However, in static builds only one mbedtls can exist, so hxcpp's mbedtls gets overwritten by lime's during linking. This means that all haxe standard library calls end up using lime's mbedtls.

The problem that occurred previously was that lime updated its internal version to mbedtls 3 while hxcpp is still using mbedtls 2. So haxe standard library calls were forced to use mbedtls 3 which resulted in segmentation faults. Lime ended up reverting to mbedtls 2 for the time being. If hxcpp is updated to mbedtls 3 now, current versions of lime will interfere with (and possibly break) any Haxe standard library calls that use mbedtls.

This is a bit of a weird situation though so I'm not sure what's best to do here, and I agree that hxcpp shouldn't use eol software. This can't be solved with conditional compilation either, because the issue happens at link time. I'll try to investigate whether lime can simply use hxcpp's version of mbedtls.

[Simn]

I propose that we release hxcpp 4.3.3 as-is and then merge this PR afterwards. That way we can point to a specific hxcpp version that works with mbedtls2.

[tobil4sk]

I think it is useful to have this file containing only the flags that we have set differently from the default values. The default configuration is already included in include/mbedtls/mbedtls_config.h. Having the entire configuration here again makes it harder to keep track of what we actually need to set ourselves, and it makes it harder to update the config file when we update mbedtls in future.

[tobil4sk]

Ah, I see you've replaced MBEDTLS_USER_CONFIG_FILE with MBEDTLS_CONFIG_FILE which is why the entire config is required now. I'd still suggest that it's easier to keep track of defines we've changed if we have the default config and then our own small config file which just changes the values that we need.

[Aidan63]

Seems I misunderstood the migration docs. It only talked about using MBEDTLS_CONFIG_FILE so assumed that was the new config method. I've reverted back to MBEDTLS_USER_CONFIG_FILE.

[tobil4sk]

I propose that we release hxcpp 4.3.3 as-is and then merge this PR afterwards. That way we can point to a specific hxcpp version that works with mbedtls2.

This would be good. It would be helpful also if this release before mbedtls 3 includes some changes to make it easier for hxcpp's mbedtls (and other static libs) to be used externally. This would be the proper way to allow lime (and other projects) to avoid these types of linking issues, then it will make it less of a concern once the migration to mbedtls 3 occurs.

Also, minor note, but I think following Hugh's convention, the next hxcpp release is whatever the tag number is (e.g. 4.3.45).

[tobil4sk]

I ran into this issue when porting neko as well. It may make more sense to put this definition directly in threading_alt.h, where it is needed:
HaxeFoundation/neko@31e9db6#diff-c5bad8f9ad3067b73cb76e84d84c5fef6f46ad66600fa9f8356ecfc0240158fd

[Aidan63]

Shifting the lean and mean define to threading_alt for hxcpp doesn't do the trick. You'll still get redefinition errors.

[tobil4sk]

I tried this patch on my machine and hxcpp_ssl was compiling fine: tobil4sk@a51f140. It's not breaking the windows ci either: https://github.com/tobil4sk/hxcpp/actions/runs/9809645213/job/27088061829

The reason I would prefer to have it in threading_alt.h is that reducing the number of command line flags required to compile hxcpp's mbedtls makes it easier for lime to link against it, which would solve lime's mbedtls issues.

[Aidan63]

Just gave it another try and it works now. I can't remember where the error I was getting was coming from since I tried moving the lean and mean define around the place pretty early on in this branch. Probably some compiler cache invalidation issue.

[tobil4sk]

Include path has to be updated here as well now. I think eventually we should switch to using submodules and just have a mbedtls directory rather than mbedtls-x.x.x.

[Aidan63]

I definitely wouldn't want to see a change to submodules, they are hell on earth and it would be better if the git world forgot they existed. Switching to a sane alternative like subtrees would be good though.

[tobil4sk]

Thanks for bearing with all the xml changes. They should allow for lime to work around the mbedtls issue, so it should be possible to avoid the problem caused by hxcpp updating to mbedtls 3.