1388289 - When using a proxy, the details.challenger.host provided to the webRequest.onAuthRequired listener contain the host of the requested URL instead of the proxy hostname

Status: RESOLVED FIXED

(WebExtensions :: Request Handling, defect, P3)

Description

[shark0der]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.78 Safari/537.36

Steps to reproduce:

Context: VPN (proxy) extension.

1. I've registered a proxy using `browser.proxy.registerProxyScript` https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/proxy/register (latest docs changed without notice regarding `registerProxyScript`).
2. My script's FindProxyForURL returns `HTTPS my.proxy.tld:443`
3. I've setup a listener for `webRequest.onAuthRequired` like this: `browser.webRequest.onAuthRequired.addListener(onAuthCallback, { urls: ['<all_urls>'] }, ['blocking']);`

In the implementation of `onAuthCallback` I am checking whether the challenger is `my.proxy.tld` in order to prevent credential leak/theft to/by some unknown proxy. Here are the relevant lines in my function:

```
    let challengerAllowed = false;

      try {
        const host = proxyConfig.rules.singleProxy.host;
        const hostRegex = host.replace('.', '\\.');
        const challengerRegex = new RegExp('^' + hostRegex + '$', 'i');
        challengerAllowed = challengerRegex.test(details.challenger.host);
      } catch (e) {
        console.log('onAuthCallback error caught:', e.message);
      }

    return details.isProxy && challengerAllowed
      ? { authCredentials: credentials }
      : {};
```
4. Make an ajax call to a `my.connectioncheck.tld` in order to trigger the authentication process.



Actual results:

challengerAllowed is false because `details.challenger.host` contains `my.connectioncheck.tld` instead of `my.proxy.tld` even though `details.isProxy` si true.


Expected results:

`details.challenger.host` should be `my.proxy.tld`.

Comment 1

[Eric Jung [:ericjung]]

> When using a proxy, the details.challenger.host provided to the webRequest.onAuthRequired listener contain the host of the
> requested URL instead of the proxy hostname

This is the documented behavior and is working as designed. From https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onAuthRequired#details

 >Warning: Unlike chrome, Firefox will return the requested host instead of the proxy requesting the authentication, even if isProxy is true.

Comment 2

[Shane Caraveo (:mixedpuppy)]

Attachment: Bug 1388289 fix challenger info for proxy auth requests,

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

Comment 3

[Shane Caraveo (:mixedpuppy)]

(In reply to Eric Jung [:ericjung] from comment #1)

>  >Warning: Unlike chrome, Firefox will return the requested host instead of
> the proxy requesting the authentication, even if isProxy is true.

I don't know why it was documented like that, this is a bug.

Comment 4

[Shane Caraveo (:mixedpuppy)]

Comment on attachment 8898589 [details]
Bug 1388289 fix challenger info for proxy auth requests,

Eric, can you take a look and see if you feel there is any other issue?

Comment 5

[shark0der]

(In reply to Shane Caraveo (:mixedpuppy) from comment #3)
> I don't know why it was documented like that, this is a bug.

The warning in the documentation was added by me, after an hour of frustration on figuring out why the code that works on chrome doesn't work in firefox, and debugging it step by step until finding the culprit. Now I realize this was a bad idea and that at least I've should've mentioned it is actually a bug (not just a warning).

Comment 6

[Eric Jung [:ericjung]]

> Eric, can you take a look and see if you feel there is any other issue?

Looks good. However, for completeness, I would suggest providing the rest of the info in nsIProxyInfo. Otherwise, we'll be back here again someday:

* type (http, https, socks, etc)
* username
* password
* failoverTimeout
proxyDNS (true/false) in the flags variable

Comment 7

[Shane Caraveo (:mixedpuppy)]

Is it ok to only have that in onAuthRequired?

Comment 8

[Eric Jung [:ericjung]]

> Is it ok to only have that in onAuthRequired?

I don't see why not. What is your concern? Certainly |type| should be there.

Comment 9

[Shane Caraveo (:mixedpuppy)]

(In reply to Eric Jung [:ericjung] from comment #8)
> > Is it ok to only have that in onAuthRequired?
> 
> I don't see why not. What is your concern? Certainly |type| should be there.

No concern.  It's easy to quickly add it for onAuthRequired, more effort (not sure how much) to add it everywhere.  If onAuth is sufficient I'll do it in this patch.

Comment 10

[Eric Jung [:ericjung]]

> If onAuth is sufficient I'll do it in this patch.

It is for this bug. I recall a privacy-related addon that wanted to know if a proxy was used to load a resource (or will be used--can't remember which). That would imply putting this info in webRequest.onBeforeRequest() and/or webRequest.onCompleted(). But that should be a different bug not relevant here.

Comment 11

[Eric Jung [:ericjung]]

> I recall a privacy-related addon that wanted to know if a proxy was used to load a resource

bug 1373646

Comment 12

[Kris Maglione [:kmag]]

Comment on attachment 8898589 [details]
Bug 1388289 fix challenger info for proxy auth requests,

https://reviewboard.mozilla.org/r/169968/#review178204

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5d04b895717f
fix challenger info for proxy auth requests, r=kmag

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/5d04b895717f

Comment 15

[Will Bamberg [:wbamberg]]

I've added a note describing this old bug in Firefox: https://developer.mozilla.org/en-US/Add-ons/WebExtensions/API/webRequest/onAuthRequired#Browser_compatibility.

Let me know if you need anything else.

Comment 16

[Shane Caraveo (:mixedpuppy)]

2. Before version 57, on a proxied connection, the challenger.host property contains the hostname for the requested URL rather than the hostname for the proxy.

I think that is sightly incorrect.

change: on a proxied connection
to: when authenticating to a proxy a proxy server

ie. challenger should always be that of the server requesting authentication, but didn't work correctly for proxy authentication.

Comment 17

[Will Bamberg [:wbamberg]]

Thanks Shane, I've updated the note.

Comment 18

[CosminB]

Is manual testing required on this bug? If yes, please provide some STR and the proper extension(if required) or set the “qe verify-“ flag.

Thanks!