HTTP client incorrectly passes IP to SSL_set_tlsext_host_name

[helgoboss]

#52118 fixes the call to X509_VERIFY_PARAM_set1_host (it uses X509_VERIFY_PARAM_set1_ip_asc instead, if we have an IP address). But it still calls SSL_set_tlsext_host_name, even if we have an IP address. This seems wrong. In fact, OpenSSL should reject this already on the client side, but because of this OpenSSL bug it doesn't. Instead, the server will be confronted with the invalid value. And depending on its standard conformance it will reject the request or accept it. Mine rejects it.

The SSL_set_tlsext_host_name problem was brought up and fixed already in PR #52707, but @mount33 couldn't seem to reproduce the practical issue later, therefore the PR was closed without merging.

This is not just of theoretical nature. I'm affected by this issue when trying to use TLS with an IP address. Here's what happens when I send a HTTP request using TLS with the host being an IP address (192.1678.178.182):

1. My server (using Rustls, apparently a quite strict TLS implementation) detects the error during the handshake and rejects the request: [2024-03-05T10:26:43Z WARN rustls::msgs::handshake] Illegal SNI hostname received "192.168.178.182". Because Dart has fed SSL_set_tlsext_host_name with an IP address. Here is the code in Rustls which rejects the hostname.
2. Flutter in return gives me below stacktrace.

I/flutter (23684): Connecting to URI https://192.168.178.182:39443...
E/flutter (23684): [ERROR:flutter/runtime/dart_vm_initializer.cc(41)] Unhandled Exception: HandshakeException: Handshake error in client (OS Error: 
E/flutter (23684): 	TLSV1_ALERT_DECODE_ERROR(tls_record.cc:592))
E/flutter (23684): #0      _SecureFilterImpl._handshake (dart:io-patch/secure_socket_patch.dart:99:46)
E/flutter (23684): #1      _SecureFilterImpl.handshake (dart:io-patch/secure_socket_patch.dart:143:25)
E/flutter (23684): #2      _RawSecureSocket._secureHandshake (dart:io/secure_socket.dart:920:54)
E/flutter (23684): #3      _RawSecureSocket._closeHandler (dart:io/secure_socket.dart:913:15)
E/flutter (23684): #4      _RawSecureSocket._eventDispatcher (dart:io/secure_socket.dart:856:9)
E/flutter (23684): #5      _RootZone.runUnaryGuarded (dart:async/zone.dart:1594:10)
E/flutter (23684): #6      _BufferingStreamSubscription._sendData (dart:async/stream_impl.dart:339:11)
E/flutter (23684): #7      _BufferingStreamSubscription._add (dart:async/stream_impl.dart:271:7)
E/flutter (23684): #8      _SyncStreamControllerDispatch._sendData (dart:async/stream_controller.dart:784:19)
E/flutter (23684): #9      _StreamController._add (dart:async/stream_controller.dart:658:7)
E/flutter (23684): #10     _StreamController.add (dart:async/stream_controller.dart:606:5)
E/flutter (23684): #11     new _RawSocket.<anonymous closure> (dart:io-patch/socket_patch.dart:1943:35)
E/flutter (23684): #12     _NativeSocket.issueReadEvent.issue (dart:io-patch/socket_patch.dart:1372:18)
E/flutter (23684): #13     _microtaskLoop (dart:async/schedule_microtask.dart:40:21)
E/flutter (23684): #14     _startMicrotaskLoop (dart:async/schedule_microtask.dart:49:5)

Originally posted by @helgoboss in dart-lang/sdk#49183 (comment)

[CJBuchel]

@helgoboss did you find a workaround for this?
I've currently got the same issue with a flutter front end and rust backend. Using TLS with the web compiled version works fine, but on android it provides an error TLSV1_ALERT_DECODE_ERROR when trying to do it's handshake.

And likewise, the normal Illegal SNI hostname received "xxx" rust side.

[helgoboss]

@CJBuchel No, but I hope that the new Rustls release 0.23.5 provides a workaround, see changelog bullet point about SNI. Of course, this is only a workaround for those who have the Rust backend under their control. Ideally, this would be fixed on Dart/Flutter side.

[CJBuchel]

Damn, this is quite problematic.
I'm building something currently which requires some form of encryption, I previously went with custom to avoid TLS for various reasons. And instead went with RSA keys, but it was slow and clunky to serialise and deserialise. So I resorted to the normal TLS method which is more supported and generally a safer option, even with self signed on local networks.

Didn't realise this would become the "new" issue when switching. Weighing up my options now, whats harder... trying to figure out how to clone my own copy of rustls in my project and just bypass it for the time being in warp and hyper (which I don't even think I can do with how tightly integrated it is). Or somehow fix it flutter side.

But if I recall from the comment you linked, this is part of the dart-sdk right? not even http package which I can do a quick fix for and keep locally

😢

[helgoboss]

Cargo makes it very easy to patch or replace dependencies, see here. I'll try this today or tomorrow and report back whether it fixes the issue for me.

[CJBuchel]

Oh my god, I didn't even know patch was a thing. Man... rust seems to always save the day.
If you can that would be awesome, otherwise don't stress. I'll also try to play around as well likely tomorrow or the day after to see if I can cobble something up.

[helgoboss]

Just to let you know, I couldn't apply this workaround because I'm forced to use the 0.21.x line of rustls (the workaround is on the 0.23.x line and that has multiple breaking changes compared to 0.21.x).

[djc]

(@helgoboss why are you forced to use 0.21.x?)

[helgoboss]

It's complicated ;) I use both tonic (gRPC server) and axum-server (HTTP server) in one application. tonic still uses Axum 0.6, so I'm stuck with Axum 0.6 for the time being. Axum 0.6 uses rustls 0.21.x.

[CJBuchel]

Damn, I was going to switch and use Tonic for gRPC a while ago instead, because it looked so interesting. But I was turned away as it doesn't have the best web support yet. But I suppose another reason now is the breaking changes in rustls 0.23.

I'll see about patching my project with warp/hyper for 0.23. Hopefully it can work, otherwise I'm going to need another option for encryption :(

[CJBuchel]

Patch works by the way.
Had to fork warp and update it's tokio-rustls version from 0.25 to 0.26 which includes the newer 0.23.5 version of rustls. And then make a patch in my project for it, but it works like a dream now.

I can only hope that this gets a proper fix Dart/Flutter side eventually. Otherwise people will just have to rely on patching it, or waiting a bit for libraries to have the updated rustls version. Which even if it does work, really shouldn't be relied on.

[cpu]

@helgoboss I've backported the invalid SNI tolerance change to the 0.21.x release stream and cut a 0.21.12 you should be able to use. Hope this helps.

[helgoboss]

@cpu Wow, thank you so much! I didn't expect this at all, what a nice surprise. I'm going to try this ASAP. ❤️

[helgoboss]

@cpu I can confirm it works now. Amazing support from the Rust community in the Dart issue tracker :) Thanks.

cargo update -p rustls did the job. It didn't even need a patch entry in Cargo.toml.