fix https_proxy problem for IO::Socket::SSL and Net::SSL, includes extensive test (Part#1) #52

Merged
merged 1 commit into from Nov 20, 2013

Conversation

Projects
None yet
4 participants

noxxi commented Nov 9, 2013

Hi,
this change fixes the behavior of https_proxy.
Current behavior: send unencrypted requests with https:// URLs to specified proxy
Correct behavior: create encrypted tunnel to proxy and send requests within
This is part#1 of the change, the rest is in lwp-protocol-https

Regards,
Steffen Ullrich (maintainer IO::Socket::SSL)

Steffen Ullrich correct behavior for https_proxy,
e.g. don't send plain https:// requests to proxy, but instead establish
CONNECT tunnel and then send requests inside tunnel.
This change does together with a change in LWP::Protocol::https.
The change supports LWP::Protocol::https with the default
IO::Socket::SSL backend, but also with Net::SSL. Also:
- proxy authorization is supported (http://user:pass@host:port as proxy
  URL, Net::SSL still needs special HTTPS_PROXY_* environemt variables,
  as before)
- CONNECT request does not need to be the first request inside the
  tunnel (not with Net::SSL)
- conn_cache is read and written inside request(), instead of writing in
  request() and reading in _new_socket(). If a https tunnel is
  established the cache_key no longer depends only on proxy host,port
  but also on the tunnel endpoint
- CONNECT is a proxy request and must always use Proxy-Authorization,
  not Authorization header
cb80c2d

dod38fr commented Nov 16, 2013

Hello

In short, this patch works well to connect to https://github.com through a proxy with hostname verification.

In more details, here are the tests I performed. I've slightly edited the program output for brevity.

I started with the following simple program:

use LWP::UserAgent; 
my $ua = LWP::UserAgent->new(); 
my $req = HTTP::Request->new('GET','https://github.com/libwww-perl'); 
my $res = $ua->request($req); 
print $res->dump(), "\n";

Direct connection with vanilla libwww-perl fails:

500 Can't verify SSL peers without knowing which Certificate Authorities to trust
Content-Type: text/plain
Client-Date: Wed, 20 Nov 2013 19:45:09 GMT
Client-Warning: Internal response

Direct connection with patched libwww-perl and lwp-protocol-https fails the same way.

Once the ca file is specified, direct connections works in both cases:

$ PERL_LWP_SSL_CA_FILE=/etc/ssl/certs/ca-certificates.crt perl -I libwww-perl/lib/ -I lwp-protocol-https/lib/ test-proxy.pl | grep Peer
Client-Peer: 192.30.252.129:443

Now with a proxy (tinyproxy on localhost). The test program is now:

use LWP::UserAgent; 
my $ua = LWP::UserAgent->new(); 
$ua->proxy('https' , 'http://127.0.0.1:8888') ;
my $req = HTTP::Request->new('GET','https://github.com/libwww-perl'); 
my $res = $ua->request($req); 
print $res->dump(), "\n";

Vanilla libwww-perl fails (with or without ca file):

$ perl test-proxy.pl 
HTTP/1.1 301 Moved Permanently
Via: 1.1 tinyproxy (tinyproxy/1.8.3)
Location: https://github.com/libwww-perl

Patched liwwww-perl without ca file fails with the same error as above, which is normal.

Patched liwwww-perl with ca file works (YEAH :-D):

$ PERL_LWP_SSL_CA_FILE=/etc/ssl/certs/ca-certificates.crt perl -I libwww-perl/lib/ -I lwp-protocol-https/lib/ test-proxy.pl 
HTTP/1.1 200 OK
Cache-Control: private, max-age=0, must-revalidate
Connection: close
Date: Sat, 16 Nov 2013 16:27:26 GMT
ETag: "d5156fbbd63f60cce60c6b275ca54e63"
Server: GitHub.com
Vary: Accept-Encoding
Content-Length: 17355
Content-Type: text/html; charset=utf-8
Client-Date: Sat, 16 Nov 2013 16:27:26 GMT
Client-Peer: 127.0.0.1:8888
Client-Response-Num: 1

Now using https_proxy env variable:

use LWP::UserAgent; 
my $ua = LWP::UserAgent->new(); 
$ua->env_proxy;
my $req = HTTP::Request->new('GET','https://github.com/libwww-perl'); 
my $res = $ua->request($req); 
print $res->dump(), "\n";

The proxy connection works ( :-D ):

$ https_proxy=http://localhost:8888 PERL_LWP_SSL_CA_FILE=/etc/ssl/certs/ca-certificates.crt perl -I libwww-perl/lib/ -I lwp-protocol-https/lib/ test-proxy.pl 
HTTP/1.1 200 OK
Cache-Control: private, max-age=0, must-revalidate
Connection: close
Date: Sat, 16 Nov 2013 16:35:16 GMT
ETag: "f6d5aeb24e6f297c0506a6373bf014ea"
Server: GitHub.com
Vary: Accept-Encoding
Content-Length: 17355
Content-Type: text/html; charset=utf-8
Client-Date: Sat, 16 Nov 2013 16:35:16 GMT
Client-Peer: 127.0.0.1:8888
Client-Response-Num: 1

Many thanks to @noxxi for these patches. I hope they'll soon be merged.

Hope this helps

noxxi commented Nov 17, 2013

Hi Dominique,
thanks for testing.

In short, this patch works well to connect to https://github.com through a proxy with hostname verification. On the other hand, once the patch is applied, https connection fail if no ca file is provided. This is probably more secure, but may break existing programs. Perhaps a warning should be issued instead of an error.

$ perl test-proxy.pl |grep Client-Peer
...
$ perl -I libwww-perl/lib/ -I lwp-protocol-https/lib/ test-proxy.pl 

You compare Debians version of LWP::Protocol::https against sock LWP::Protocol::https with my patch. But, Debian has patched LWP::Protocol::https to provide a useful default CA path. If you apply Debians patch on top of mine you no longer need to specify a CA file.
So this patch should not make any programs fail which worked before, because it does not change anything regarding verification of certificates. Only programs which depend on the broken proxy behavior of LWP will fail.

Regards,
Steffen

dod38fr commented Nov 20, 2013

@noxxi : yes. You're right. Thanks for the clarification.

dod38fr commented Nov 20, 2013

Following @noxxi 's comments, I've updated the test report. @gisle , this patch looks good to me. What do you think ?

All the best

@gisle gisle merged commit cb80c2d into libwww-perl:master Nov 20, 2013

Owner

gisle commented Nov 20, 2013

Seems sensible to me. I've merged the patch.

dod38fr commented Nov 21, 2013

@gisle PR libwww-perl/lwp-protocol-https#7 also needs to be merged for this fix to be complete

@goneri goneri added a commit to fusioninventory/fusioninventory-agent that referenced this pull request Dec 25, 2013

@goneri @goneri goneri + goneri test: fix the HTTPS over proxy test
The test was failing when libwww-perl really support HTTPS over proxy.
See: libwww-perl/libwww-perl#52
fe68a18

@goneri goneri added a commit to fusioninventory/fusioninventory-agent that referenced this pull request Dec 25, 2013

@goneri @goneri goneri + goneri test: fix the HTTPS over proxy test
The test was failing when libwww-perl really support HTTPS over
proxy.
See: libwww-perl/libwww-perl#52
591645c

@goneri goneri added a commit to fusioninventory/fusioninventory-agent that referenced this pull request Dec 26, 2013

@goneri @goneri goneri + goneri test: fix the HTTPS over proxy test
The test was failing when libwww-perl really support HTTPS over
proxy.
See: libwww-perl/libwww-perl#52
1e9e1d5

Any chance this should just be '$response->is_success or die $response;'? That way the proper HTTP status can be propagated through to the callers. In my case, I'm looking for the 407 so proxy authentication can get triggered.

thoke commented on cb80c2d May 6, 2014

Now that this is in v6.06, I'm doing some testing with it. I'm finding issues with two things:

  1. Some of the $response->is_success or die situations are resulting in propagation of status 500 plus the underlying real HTTP status (407 proxy auth in my case).
  2. This will only work with basic authentication to the proxy and only if it's specified on the proxy url.

Do you have any desire to extend this to work with provided credentials? e.g. $ua->credentials(...) Or can share any pointers so I can look into it further?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment