Bug 175378

Summary: [Soup] Cannot access HTTPS sites using a HTTP proxy that requires authentication
Product: WebKit Reporter: Paul van Tilburg <paul>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: berto, bugs-noreply, buildbot, cgarcia, danw, gustavo, mcatanzaro, svillar
Priority: P2    
Version: WebKit Local Build   
Hardware: PC   
OS: Linux   
See Also: https://bugzilla.gnome.org/show_bug.cgi?id=786147
Attachments:
Description Flags
Source of the mini browser
none
Source of the test proxy
none
Patch
svillar: review+
Patch for landing none

Description Paul van Tilburg 2017-08-09 05:45:41 PDT
When configuring a HTTP(S) proxy in GNOME, the user cannot access HTTPS sites if the proxy requires authentication.

For example, to reproduce:

* Configure a test proxy that requires authentication on http://localhost:8080/.
* Open a HTTP website in Epiphany or with a small test browser; an authentication dialog is shown, and the page is retrieved through the test proxy and shown.
* Then, open a HTTPS website; no authentication dialog is shown and Epiphany shows a file download with the error "Proxy Authentication Required".

Closer inspection of the test proxy logs and the code of our small test browser that when CONNECT verb is used on the HTTP proxy, no "authentication" signal is emitted on the WebKitWebView.
The signal _is_ emitted for the GET verb and it can successfully 

This is on Ubuntu (16.04) Xenial, libsoup version 2.25.2, libwebkit2gtk-4.0 version 2.16.6.

(As far as we are aware everything works with libwebkitgtk-3.0 2.4.11.)
Comment 1 Paul van Tilburg 2017-08-09 07:18:09 PDT
I have tested some more and added load changed/failed callbacks.
The reported load change events are from WEBKIT_LOAD_STARTED directly to WEBKIT_LOAD_FINISHED, the load failed callback gets the error message "Frame load was interrupted".
Comment 2 Michael Catanzaro 2017-08-09 08:44:59 PDT
(In reply to Paul van Tilburg from comment #0)
> * Configure a test proxy that requires authentication on
> http://localhost:8080/.

Could you post some instructions or a small script for configuring this?

> This is on Ubuntu (16.04) Xenial, libsoup version 2.25.2, libwebkit2gtk-4.0
> version 2.16.6.

I guess you mean libsoup 2.52.2.
Comment 3 Paul van Tilburg 2017-08-10 01:16:28 PDT
Created attachment 317795 [details]
Source of the mini browser
Comment 4 Paul van Tilburg 2017-08-10 01:17:06 PDT
Created attachment 317796 [details]
Source of the test proxy
Comment 5 Paul van Tilburg 2017-08-10 01:17:12 PDT
I have attached a small Ruby script that we use to run a test proxy on localhost:8080 that requires the hardcoded basic authentication credenials user/password.  However, also some Squid proxy would do.

To configure the proxy, I guess export http_proxy=http://localhost:8080 would do.

To test I am using a modified version of the mini browser example that adds an authentication hook with the hardcoded credentials.  I have also attached the source.

The mini browser source has a commented-out load URI line for http://www.nu.nl. When building and running with that line enabled, the proxy logs:

[2017-08-10 10:10:09] ERROR Basic Proxy Realm: no credentials in the request.
[2017-08-10 10:10:09] ERROR WEBrick::HTTPStatus::ProxyAuthenticationRequired
IP_ADDRESS - - [10/Aug/2017:10:10:09 CEST] "GET http://www.nu.nl/ HTTP/1.1" 407 342
- -> http://www.nu.nl/
[2017-08-10 10:10:09] INFO  Basic Proxy Realm: user: authentication succeeded.
IP_ADDRESS - user [10/Aug/2017:10:10:09 CEST] "GET http://www.nu.nl/ HTTP/1.1" 304 0
- -> http://www.nu.nl/

and the mini browser logs:
** Message: Authentication callback is called for www.nu.nl:80!

When using the load URI line for https://google.com, the proxy logs:

[2017-08-10 10:10:24] ERROR Basic Proxy Realm: no credentials in the request.
[2017-08-10 10:10:24] ERROR WEBrick::HTTPStatus::ProxyAuthenticationRequired
IP_ADDRESS - - [10/Aug/2017:10:10:24 CEST] "CONNECT google.com:443 HTTP/1.1" 407 347
- -> google.com:443

and the mini browser logs nothing (in particular not that the callback is called).
Comment 6 Carlos Garcia Campos 2017-08-10 08:40:07 PDT
Thanks for the detailed instructions, it helped a lot. There are two problems here:

 1- We are failing to detect the response MIME type when using https + proxy, since it's a main resource load, we end up trying to download it. I'm not sure if we can fix it, the server is not providing the content type header and we fail to sniff because we only talked to the proxy.

 2- In the authenticateCallback we receive from libsoup we return early if the given soup message is not our resource soup message. This is wrong in the case of htts + proxy because libsoup creates a tunnel internally and the soup message it uses for the authentication is the tunnel one, not ours.

I've fixed 2 and authentication works, but if you cancel the auth dialog, we end up trying to download it again, because of 1.
Comment 7 Carlos Garcia Campos 2017-08-10 09:03:07 PDT
Created attachment 317812 [details]
Patch
Comment 8 Build Bot 2017-08-10 09:05:55 PDT
Attachment 317812 [details] did not pass style-queue:


ERROR: Source/WebCore/platform/network/soup/AuthenticationChallengeSoup.cpp:75:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Sergio Villar Senin 2017-08-10 10:02:40 PDT
Comment on attachment 317812 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=317812&action=review

Can't we come up with a test case?

> Source/WebCore/platform/network/soup/AuthenticationChallengeSoup.cpp:84
> +        && a.soupAuth() == b.soupAuth();

One line?
Comment 10 Michael Catanzaro 2017-08-10 10:31:26 PDT
We do have proxy tests in TestWebKitAPI. Not sure if we have HTTP auth tests, but if so, a combination test would be wonderful.
Comment 11 Carlos Garcia Campos 2017-08-10 23:26:47 PDT
(In reply to Michael Catanzaro from comment #10)
> We do have proxy tests in TestWebKitAPI. Not sure if we have HTTP auth
> tests, but if so, a combination test would be wonderful.

Well, we have proxy settings api tests. What we do is that we run another soup server in a different port and set its url as the http proxy and then we check that requests reach the proxy instead of the server, but we don't really implement the proxy. We have auth tests too, but I'm not sure it's that easy to combine both. I'll give it a try.
Comment 12 Carlos Garcia Campos 2017-08-11 02:55:31 PDT
Created attachment 317922 [details]
Patch for landing
Comment 13 Build Bot 2017-08-11 03:03:40 PDT
Attachment 317922 [details] did not pass style-queue:


ERROR: Tools/TestWebKitAPI/Tests/WebKitGLib/TestAuthentication.cpp:269:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Tools/TestWebKitAPI/Tests/WebKitGLib/TestAuthentication.cpp:290:  Extra space before ( in function call  [whitespace/parens] [4]
ERROR: Source/WebCore/platform/network/soup/AuthenticationChallengeSoup.cpp:75:  Code inside a namespace should not be indented.  [whitespace/indent] [4]
Total errors found: 3 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 14 Carlos Garcia Campos 2017-08-11 03:06:22 PDT
Committed r220583: <http://trac.webkit.org/changeset/220583>
Comment 15 Michael Catanzaro 2017-08-11 08:14:00 PDT
As you noted, this is only half-fixed. Will you open another bug?
Comment 16 Carlos Garcia Campos 2017-08-12 00:19:47 PDT
(In reply to Michael Catanzaro from comment #15)
> As you noted, this is only half-fixed. Will you open another bug?

What other half do you mean? That we try a download when you cancel the dialog? or that auth dialog refers to the requested url instead of the proxy? The former is not actually a bug, there's little we can do if we don't have a content-type header and we didn't reach the actual contents to sniff them, chromium shows a white page in that case, and we start a download because MiniBrowser has that policy, in webkit itself the policy in that case is ignore. And the latter is actually a libsoup bug, see https://bugzilla.gnome.org/show_bug.cgi?id=786147
Comment 17 Michael Catanzaro 2017-08-12 08:54:37 PDT
I was trying to ask about https://bugzilla.gnome.org/show_bug.cgi?id=786147. Thanks!