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

Downgrade to mbedtls 2 to avoid crash on ios #1767

Merged
merged 1 commit into from
Mar 11, 2024

Conversation

tobil4sk
Copy link
Member

Lime was updated to use mbedtls 3.3.0 in #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.

On ios, we build statically linked executables. This means that if lime uses mbedtls 3, hxcpp is also forced to use it which can cause crashes and other problems, as hxcpp is currently written for 2.28
@joshtynjala
Copy link
Member

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
Copy link
Member

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.

@joshtynjala joshtynjala merged commit fc6b905 into openfl:develop Mar 11, 2024
23 checks passed
@tobil4sk
Copy link
Member Author

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: Mbed-TLS/mbedtls#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."

@tobil4sk tobil4sk deleted the fix/hxcpp-mbedtls-conflict branch March 11, 2024 21:49
@player-03
Copy link
Contributor

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.

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.

3 participants