Use rustls for Swift to support TLS 1.3

[csett86]

Currently Element X iOS does not support TLS 1.3, this PR shall fix that.

Explanation:

There is an official recommendation from Apple, that boils down to the following if you use cross-platform code with sockets (as we do with the rust sdk):

To use TLS in that case [BSD Sockets], add your own TLS implementation.

Don’t use Secure Transport for your TLS implementation. It’s been deprecated since 2019
and doesn’t support TLS 1.3. If you have existing code that uses Secure Transport, make
a plan to migrate off it.

Modern TLS implementations including TLS 1.3 on macOS are only available as a builtin via the Apple-specific URLSession / Network framework APIs, so APIs where you feed in an URL and get the response back. They are not available in combination with a generic sockets-based cross-platform code.

With that in mind, there is currently no hope that rust-native-tls would support TLS 1.3 in the forseeable future as there is simply no native TLS implementation in current macOS/iOS that could be used by rust-native-tls.

See https://developer.apple.com/documentation/technotes/tn3151-choosing-the-right-networking-api#TLS-best-practices

Fixes: element-hq/element-x-ios#786

• Public API changes documented in changelogs (optional)

Signed-off-by: Christoph Settgast csett86_git@quicksands.de

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9bf48ef) 83.75% compared to head (75e8fd8) 83.73%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3113      +/-   ##
==========================================
- Coverage   83.75%   83.73%   -0.03%     
==========================================
  Files         229      229              
  Lines       23654    23655       +1     
==========================================
- Hits        19812    19807       -5     
- Misses       3842     3848       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[jplatte]

Oh no, since some packages use native-tls by default and this one now enables rustls-tls on linux, but both can't be enabled at the same time, some CI jobs are failing.

Do you mind me pushing a few changes to your PR to attempt to fix this, or do you want to revert the PR to its previous state? (in that case I'd create a follow-up PR)

[csett86]

Feel free to push a few changes to fix it forward, please go ahead

[jplatte]

I have pushed a change that I think works, but I realized that the change I suggested also breaks local development setups if they don't have some configuration like what I added to contrib/ide/vscode for Visual Studio Code.

IMO the right fix for that is to make rustls-tls and native-tls not mutually exclusive, since they aren't in the underlying library either. I forgot why they currently are mutually exlusive in the SDK. If this was implemented, all the uses of --no-default-features --features __ci_or_development could simply become --all-features, I think.

[csett86]

Thanks for helping @jplatte as this is getting out of my comfort zone quickly.

Regarding both backends: From what I read in https://docs.rs/reqwest/latest/reqwest/tls/index.html and seanmonstar/reqwest#378, the features are not mutually exclusive, but if they are both enabled, native-tls is chosen. So my guess is that this exclusivity is coded here so that you can be sure which one of the tls engines it is and can influence it when using the SDK. But thats just my guess at the moment...

It was also introduced as mutually exclusive directly from the beginning: #89

[jplatte]

Looks like everything works with the features no longer being mutually exclusive. But I will wait with merging until there's some feedback from @Hywan or somebody else from the team.

I also had some extra local changes to use --all-features and remove the docsrs feature on one crate, but accidentally deleted them. But makes sense for that to be a separate PR anyways.

[pixlwave]

Please see element-hq/element-x-ios#786 (comment) :)

[poljar]

Please see element-hq/element-x-ios#786 (comment) :)

Alright now that we have support for user-installed certs in EXA, can we reconsider this PR?

Do some certs get ignored on iOS as well if we use rustls? If so, can we work around this like we did with Android until rusttls-platform-verifier becomes a reality?

[jplatte]

If so, can we work around this like we did with Android until rusttls-platform-verifier becomes a reality?

What's not real about it now? 😅

[poljar]

If so, can we work around this like we did with Android until rusttls-platform-verifier becomes a reality?

What's not real about it now? 😅

As far as I could tell you can't enable it on reqwest yet and we're stuck on a old reqwest version because of a bug, so even adding ways to enable it on reqwest won't help us until we unblocked on the version bump.

[poljar]

Alright now that we have support for user-installed certs in element-hq/element-x-android#2392, can we reconsider this PR?

@pixlwave any update on this? I believe you wanted to check if everything continues to work with this patch enabled.

[pixlwave]

@pixlwave any update on this? I believe you wanted to check if everything continues to work with this patch enabled.

Ah sorry I replied in the room and not on the issue 🤦‍♂️

Trying this PR did indeed reveal that custom CAs added to the system don't worth with rustls like they do with native-tls.

I was unable to find a way on iOS to get all of the user installed certificates from the keychain like we do for Android. SecTrustCopyAnchorCertificates and SecTrustSettingsCopyCertificates are both macOS only, SecTrustCopyCustomAnchorCertificates can only be performed against a certificate you want to verify and all the docs point to evaluating trust for a given certificate rather than handing that process over to another library. When you look at rusttls-platform-verifier this is exactly what they're doing, using a SecTrust object from the system.

[poljar]

Alright, @csett86, sorry but we can't enable rustls until we're able to use the platform-verifier on iOS.

Closing this for now.

[csett86]

For future reference, this is the issue tracking adding the platform verifier to reqwest: seanmonstar/reqwest#2159