Open Bug 1794464 Opened 2 years ago Updated 2 years ago

Allow HTTP authentication in proxy.onRequest

Categories

(Core :: Networking, enhancement, P3)

enhancement

Tracking

()

People

(Reporter: eros_uk, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [necko-triaged])

Preamble

The bug was filed subsequent to a discussion with Valentin in #necko:mozilla.org and instruction to file a bug under https://bugzilla.mozilla.org/enter_bug.cgi?product=Core&component=Networking

API was restricted due to the request in D29825:

can you restrict this only for https proxies?
Why exactly should this be limited to https proxies only?
Because we limit any new stuff to https only.

Background

Reference: Bug 1545420 & D29825

Originally, proxy.onRequest was created to process proxy forwarding and authenticating.

  • The API can handle HTTP/HTTPS/SOCKS4/SOCKS5 proxy forwarding
  • The API can also provide authentication for SOCKS5 (unique to Firefox), as well as HTTPS proxies

At the moment, proxying using proxy.onRequest involves:

  • Forwarding to SOCKS4 proxies (no authentication as it is not supported)
  • Forwarding & authenticating withing the same process to SOCKS5 proxies
  • Forwarding & authenticating withing the same process to HTTPS proxies as basic authentication header e.g. Proxy-Authorization: Basic YWxhZGRpbjpvcGVuc2VzYW1l
  • Forwarding to HTTP proxies

As a result, in case of HTTP proxies, forwarding and authenticating can not be performed within the same process and additional listeners (and permissions) are needed.

webRequest.onAuthRequired

Proxy authorization
If an extension has the "webRequest", "webRequestBlocking", "proxy", and "<all_urls>" permissions, then it will be able to use onAuthRequired to supply credentials for proxy authorization (but not for normal web authorization).

browser.webRequest.onAuthRequired.addListener() 
browser.webRequest.onCompleted.addListener()
browser.webRequest.onErrorOccurred.addListener()

Performance Comparison:

  • Forwarding to proxy with Proxy-Authorization headers in one process (as is the case with SOCKS5 & HTTPS)
    ----- vs -----
  • Forwarding to the proxy (HTTP)
  • Adding a listeners to wait for HTTP 407
  • Replying to the HTTP 407 after retrieving user/pass

If HTTP authentication header was also permitted in proxy.onRequest, then there would not be any need to resort to a separate API (actually 3 APIs) simply to handle sending authentication for HTTP proxies.

In fact, the other APIs eventually end up sending the authentication as Proxy-Authorization: Basic YWxhZGRpbjpvcGVuc2VzYW1l header anyway.

Currently, proxy.onRequest disables setting proxyAuthorizationHeader for HTTP in
https://searchfox.org/mozilla-central/source/toolkit/components/extensions/ProxyChannelFilter.jsm#173

  proxyAuthorizationHeader(proxyData) {
    let { proxyAuthorizationHeader, type } = proxyData;
    if (proxyAuthorizationHeader === undefined) {
      return;
    }
    if (typeof proxyAuthorizationHeader !== "string") {
      throw new ExtensionError(
        `ProxyInfoData: Invalid proxy server authorization header: "${proxyAuthorizationHeader}"`
      );
    }
    if (type !== "https") {
      throw new ExtensionError(
        `ProxyInfoData: ProxyAuthorizationHeader requires type "https"`
      );
    }
  },

Request

Above limit was requested in D29825. If there are no underlying issues, then the removal of lines 183-187 should enhance the proxy.onRequest and improve the overall performance by processing everything within the same process.

Component: Networking → Request Handling
Product: Core → WebExtensions

Other notes on API

The followings are not part of the above request and are added only as additional material on the API (that can be looked into while on the subject):

  • Is passing proxyDNS necessary for HTTP/HTTPS?
  • Username/password is not supported in SOCKS4. Is it necessary to pass it for SCOSK4?

https://searchfox.org/mozilla-central/source/toolkit/components/extensions/ProxyChannelFilter.jsm#141

  proxyDNS(proxyData) {
    let { proxyDNS, type } = proxyData;
    if (proxyDNS !== undefined) {
      if (typeof proxyDNS !== "boolean") {
        throw new ExtensionError(
          `ProxyInfoData: Invalid proxyDNS value: "${proxyDNS}"`
        );
      }
      if (
        proxyDNS &&
        type !== PROXY_TYPES.SOCKS &&
        type !== PROXY_TYPES.SOCKS4
      ) {
        throw new ExtensionError(
          `ProxyInfoData: proxyDNS can only be true for SOCKS proxy servers`
        );
      }
    }
  },

https://searchfox.org/mozilla-central/source/toolkit/components/extensions/ProxyChannelFilter.jsm#237

    if (type === PROXY_TYPES.SOCKS || type === PROXY_TYPES.SOCKS4) {
      proxyInfo = lazy.ProxyService.newProxyInfoWithAuth(
        type,
        host,
        port,
        username,
        password,
        proxyAuthorizationHeader,
        connectionIsolationKey,
        proxyDNS ? TRANSPARENT_PROXY_RESOLVES_HOST : 0,
        failoverTimeout ? failoverTimeout : PROXY_TIMEOUT_SEC,
        failoverProxy
      );
    } else {
      proxyInfo = lazy.ProxyService.newProxyInfo(
        type,
        host,
        port,
        proxyAuthorizationHeader,
        connectionIsolationKey,
        proxyDNS ? TRANSPARENT_PROXY_RESOLVES_HOST : 0,
        failoverTimeout ? failoverTimeout : PROXY_TIMEOUT_SEC,
        failoverProxy
      );
    }

This is a limitation in the networking code, not extensions, per https://phabricator.services.mozilla.com/D29825#881054

Component: Request Handling → Networking
Product: WebExtensions → Core

(In reply to Shane Caraveo (:mixedpuppy) from comment #2)

This is a limitation in the networking code, not extensions, per https://phabricator.services.mozilla.com/D29825#881054

My apologies not seeing that.
I've put this in our internal queue to get this fixed soon.

Severity: -- → N/A
Priority: -- → P2
Whiteboard: [necko-triaged][necko-priority-review]
Blocks: necko-proxy
Priority: P2 → P3
Whiteboard: [necko-triaged][necko-priority-review] → [necko-triaged]
You need to log in before you can comment on or make changes to this bug.