Fix ssl upgrade for regular host names #77
Conversation
Due to a buggy string match, SSL_hostname is always undef: matching ":" in host_port, which always has a colon between the host and the port. This leads to "certificate verify failed" openssl error (observed along with a proxy negotiating TLVv1.3). I suspect this is openssl trying to compare the hostname (which we do not pass) in the server certificate CN / SAN. Fixes regression introduced in v6.12: 6e9101b Making it possible to use IPv6 in https call...
|
I think regarding to comments @noxxi we need to keep the same logic. Maybe we can try to detect whether it has any double colons ( |
|
IMHO only host() should be used to check if it is an IP address, not host_port(). Checking for |
|
Ok, I see, let's make it |
|
If you add this, we can hopefully get a complete test run. diff --git a/Changes b/Changes
index 45cb4cf..d5af736 100644
--- a/Changes
+++ b/Changes
@@ -1,6 +1,7 @@
Release history for LWP-Protocol-https
{{$NEXT}}
+ - Fix ssl upgrade for regular host names (GH#77) (Axel Burri)
6.12 2024-01-22 17:51:31Z
- Enable MultiHomed for IO::Socket::SSL (GH#61) (ℕicolas ℝ.) |
| @@ -98,7 +98,7 @@ if ( $Net::HTTPS::SSL_SOCKET_CLASS->can('start_SSL')) { | |||
| my ($self,$sock,$url) = @_; | |||
| # SNI should be passed there only if it is not an IP address. | |||
| # Details: https://github.com/libwww-perl/libwww-perl/issues/449#issuecomment-1896175509 | |||
| my $host = $url->host_port() =~ m/:|^[\d.]+$/s ? undef : $url->host(); | |||
| my $host = $url->host() =~ m/:|^[\d.]+$/s ? undef : $url->host(); | |||
There was a problem hiding this comment.
Is it possible to add a regression test for this?
There was a problem hiding this comment.
Sadly I don't have much time for this right now. Also I don't know all the details on how the interaction with openssl works exactly, and in what situations this code is called (all I know is that it's used when talking to proxies, but I'm sure there's more to it).
I will keep this in the back of my head and try to come up with some tests in a quiet minute.
There was a problem hiding this comment.
FWIW, we've seen the same bug in Debian (in connection with an https_proxy), and the above change fixes it both for the reporter in his real enviroment and for me while testing.
That's https://bugs.debian.org/1061660 (a bit long-winded as it took me some time to figure out what the problem or the difference in my environment was -- the proxy).
While I don't claim to understand what's going on here on a deeper level and therefore can't offer any assistance in coming up with tests, I can at least confirm both the problem and that this proposed changes makes it better :)
Due to a buggy string match, SSL_hostname is always undef: matching ":" in host_port, which always has a colon between the host and the port. This leads to "certificate verify failed" openssl error (observed along with a proxy negotiating TLVv1.3).
I suspect this is openssl trying to compare the hostname (which we do not pass) in the server certificate CN / SAN.
Fixes regression introduced in v6.12:
6e9101b Making it possible to use IPv6 in https call...