search

openfl / lime

A foundational Haxe framework for cross-platform development
https://lime.openfl.org/
MIT License
749 stars 359 forks source link

Downgrade to mbedtls 2 to avoid crash on ios #1767

Closed tobil4sk closed 3 months ago

tobil4sk commented 4 months ago

Lime was updated to use mbedtls 3.3.0 in https://github.com/openfl/lime/pull/1682.

When linking a project statically (for example for ios), this means that hxcpp is forced to use this version of mbedtls as well, which is a problem because hxcpp is still written for mbedtls 2. This can cause crashes and other issues due to incompatibility.

Ideally, hxcpp should be upgraded to use mbedtls 3, but this will probably require some work. In the meantime it can be worked around by downgrading to 2.28 in lime. This is still an upgrade from before (previous version was 2.6.0), and 2.28 is still supported until the end of 2024.

joshtynjala commented 3 months ago

It appears that we updated mbedtls to 3.3.0 for two reasons.

1) We initially ran into an issue where https stopped working on newer macOS versions. I noticed that mbedtls 3.1.0 was being used in the 8.2.0-Dev branch, and the issue did not exist there. So I tried updating to that version.

2) However, mbedtls 3.1.0 broke Android NDK r15c compatibility. The 8.2.0-Dev branch can use newer versions of the NDK, but the develop branch still relies specifically on r15c. mbedtls 3.3.0 fixed this Android NDK issue, so that's why we updated to 3.3.0. It appears CI passed for your pull request, so this shouldn't be an issue.

I'm willing to downgrade to a version of mbedtls 2.x, but if we do, we need to be sure that https works on macOS.

joshtynjala commented 3 months ago

I confirm that macOS https is working with mbedtls 2.28.7 in this PR, and as I mentioned previously, the build passes successfully, so NDK r15c seems to be working too. I'm going to merge it into develop.

@player-03 I'll let you decide what you want to do in the 8.2.0-Dev branch, since you did all of the submodule updates. If mbedtls 3.x is incompatible with hxcpp in some situations, we should probably stick with mbedtls 2.x until hxcpp updates to 3.x. Based on the mbedtls repo commits, it looks like mbedtls 2.28.7 was just released a couple of months ago, so they must be continuing to maintain 2.x at the same time as 3.x, at least for a while. So it seems that we wouldn't be downgrading to an obsolete version with security issues.

tobil4sk commented 3 months ago

It appears CI passed for your pull request, so this shouldn't be an issue.

Yes, the patch that fixed the android compilation problem in 3.3.0 was also backported to 2.28: https://github.com/Mbed-TLS/mbedtls/pull/6096

I'm going to merge it into develop.

Thanks!

Based on the mbedtls repo commits, it looks like mbedtls 2.28.7 was just released a couple of months ago, so they must be continuing to maintain 2.x at the same time as 3.x, at least for a while.

According to the release notes:

"Mbed TLS 2.28 is a long-time support branch. It will be supported with bug-fixes and security fixes until end of 2024."

player-03 commented 2 months ago

Finally got around to merging this into 8.2.0.

I had no particular reason to prefer 3.x over 2.28, other than that we eventually want to make the update. But it sounds like we have until the end of 2024.