1401350 - WebExtensions: webRequest.onAuthRequired does not fire for system requests

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, defect, P1)

Description

[Onur Taskin]

Attachment: Screen Shot 2017-09-19 at 18.19.02.png

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.113 Safari/537.36

Steps to reproduce:

Whilst migrating my Proxy add-on to the new WebExtensions API, I have discovered an incompatibility between the WebExtensions API and the Chrome Extensions API.

The webRequest listener onAuthRequired do not fire for system requests (isSystemPrincipal = true);

I have set a pacscript to proxy all requests.

I have tested the following code on background script:

browser.webRequest.onAuthRequired.addListener(onAsyncAuth, {urls: ["<all_urls>"]}, ["blocking"]);


Actual results:

In Firefox Nigthly, the system requests are proxied. But webRequest.onAuthRequired does not fire when those request get "407 Proxy Authentication Required" from the proxy server. Authentication prompt is shown to the user.


Expected results:

The system requests are proxied. The webRequest.onAuthRequired listener should fire if those request get 407 and we should be able to provide credentials before any Authentication prompt is displayed to the user just like with any other request.

This makes webRequest.onAuthRequired unreliable and on most cases useless since it fires on some requests which are proxied and some other requests it  shows Authentication prompt to the user.

Comment 1

[Bob Silverberg [:bsilverberg]]

This sounds like something that should be fixed. Shane, can you comment on how important you think this is, and whether it's something we want to flag for 57?

Comment 2

[Shane Caraveo (:mixedpuppy)]

Yeah, I'd already been thinking about this after looking into bug 1382822.  It's pretty broken behavior.

The use of webRequest for proxy auth is a problem since we don't allow system requests through.  There is bug 1360404 to allow the proxy script to return http auth as it can with socks auth.

A simpler fix might be that we bypass the system limitation for onAuthRequired if the statuscode is 407 and isproxy is set and the extension has the proxy permission.

Another option could be to simply add a browser.proxy.onAuthRequired function, similar to the webRequest version but different in  a couple key ways.  a) no system limitation, b) limited to 407 authentication from the proxies the addon registers.

Comment 3

[Shane Caraveo (:mixedpuppy)]

Attachment: WIP proxyAuth.diff

Comment 4

[Kris Maglione [:kmag]]

Comment on attachment 8910818 [details] [diff] [review]
WIP proxyAuth.diff

Review of attachment 8910818 [details] [diff] [review]:
-----------------------------------------------------------------

I think this is a good start, but we probably also need to change the canModify handler to only allow auth callbacks, and not other modifications, in this case.

Comment 5

[Shane Caraveo (:mixedpuppy)]

Attachment: WIP proxyAuth.diff v2

channel doesn't know about proxy auth, so I handled this in WebRequest.jsm.  Not sure if there is a way to set isProxy onto ChannelWrapper then have canModify see that.  Even so, we want to cut out early in this case.

Comment 6

[Shane Caraveo (:mixedpuppy)]

I think this is a big enough hit on ux for proxy addons, it should get done fast and uplifted.

Comment 7

[Kris Maglione [:kmag]]

Comment on attachment 8910953 [details] [diff] [review]
WIP proxyAuth.diff v2

Review of attachment 8910953 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/extensions/ext-webRequest.js
@@ +25,3 @@
>        // Prevent listening in on requests originating from system principal to
> +      // prevent tinkering with OCSP, app and addon updates, etc.  However,
> +      // proxy addons need to be able to provide auth for any request wo we

s/ wo/, so/

::: toolkit/modules/addons/WebRequest.jsm
@@ +880,5 @@
> +        }
> +
> +        // We allow proxy auth to cancel or handle authCredentials regardless of
> +        // canModify, but ensure we do nothing else.
> +        if (!channel.canModify) {

This needs to happen before `if (result.cancel)`

Comment 8

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1401350 fix proxy auth for system requests,

Review commit: https://reviewboard.mozilla.org/r/183410/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/183410/

Comment 9

[Kris Maglione [:kmag]]

Comment on attachment 8912031 [details]
Bug 1401350 fix proxy auth for system requests,

https://reviewboard.mozilla.org/r/183410/#review188854

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/6924f941096f
fix proxy auth for system requests, r=kmag

Comment 11

[Shane Caraveo (:mixedpuppy)]

dev-doc

This changes the behavior of WebRequest.OnAuthRequired for extensions that also have the proxy permission.  The combination will allow a proxy extension to handle proxy authorizations for all requests, even those that are system requests which are usually barred from handling in WebRequest.  So, an extension with permissions "webRequest", "webRequestBlocking", "proxy", "<all_urls>" that adds an OnAuthRequired listener will be able to return the authCredentials for proxy authorization, but not for normal web authorization.  The listener will not be able to cancel system requests or make any other modifications to any system requests.

Comment 12

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8912031 [details]
Bug 1401350 fix proxy auth for system requests,

Getting this on the radar early.

Approval Request Comment
[Feature/Bug causing the regression]: extension proxy
[User impact if declined]: proxy extensions will not be able to handle proxy authorizations for system requests, resulting in authorization dialogs.  e.g. a proxy extension that contains credentials that the user may not have direct access to.
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: low
[Why is the change risky/not risky?]: most of the code is the same, a little reorganization and bypassing a couple restrictions during proxy authorization.
[String changes made/needed]: none

Comment 13

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/6924f941096f

Comment 14

[Sylvestre Ledru [:Sylvestre]]

Shane, I am really sorry but I don't understand what means:
> [User impact if declined]: proxy extensions will not be able to handle proxy authorizations for system requests, resulting in authorization dialogs.  e.g. a proxy extension that contains credentials that the user may not have direct access to.

Can you explain me like I am 5 ? (to understand the current impact for users)

Comment 15

[Shane Caraveo (:mixedpuppy)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #14)
> Shane, I am really sorry but I don't understand what means:
> > [User impact if declined]: proxy extensions will not be able to handle proxy authorizations for system requests, resulting in authorization dialogs.  e.g. a proxy extension that contains credentials that the user may not have direct access to.
> 
> Can you explain me like I am 5 ? (to understand the current impact for users)

Assume this for a proxy extension:

- it configures a proxy that all requests are handled through
- that proxy server requires authorization
- the extension also handles that authorization

Prior to this patch, an extension could not handle proxy authorization for any request that was a system request (e.g. extension updates, blocklist updates, firefox upgrades, search bar suggestions, etc).  The user would end up getting the standard firefox authorization dialog instead.

Without this patch:  At best, the user knows the username/password and is able to enter it and continue and has merely experienced poor UX.  At worst, it is some custom authorization and there is no way for the user to continue, so those requests are essentially blocked, but the firefox auth dialog keeps appearing all the time.

Comment 16

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8912031 [details]
Bug 1401350 fix proxy auth for system requests,

OK, let's take it then.
Should be in 57b4

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/8654b5afc31d

Comment 18

[Andrei Vaida [:avaida]]

(In reply to Shane Caraveo (:mixedpuppy) from comment #12)
> [Is this code covered by automated tests?]: yes
> [Has the fix been verified in Nightly?]: not yet
> [Needs manual test from QE? If yes, steps to reproduce]: no

Setting qe-verify- based on Shane's assessment on manual testing needs and the fact that this fix has automated coverage.

Comment 19

[Will Bamberg [:wbamberg]]

I've added a bit here: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onAuthRequired#Proxy_authorization It's mostly taken from your comment 11, but in case I've introduced some error here, please let me know.

Comment 20

[Shane Caraveo (:mixedpuppy)]

Thanks!  lgtm