Last Comment Bug 378637 - Add support for connecting to HTTP proxy over HTTPS
: Add support for connecting to HTTP proxy over HTTPS
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Networking: HTTP (show other bugs)
: unspecified
: All All
-- enhancement with 28 votes (vote)
: mozilla33
Assigned To: Patrick McManus [:mcmanus]
:
: Patrick McManus [:mcmanus]
Mentors:
: 709106 764358 (view as bug list)
Depends on: 715905 1012853 1013623 1014250 1022268 1029163 1040323 1045640 1079484 1083930 1085329 1110707 1133177 1136140 1293328
Blocks: 646452 209312 1012825 1012827 1014589 1037082
  Show dependency treegraph
 
Reported: 2007-04-24 11:36 PDT by Dan Harkless
Modified: 2017-03-18 14:50 PDT (History)
43 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
disabled
fixed
fixed
33+
+


Attachments
[PATCH 01/12] add leak detection macros to http classes (4.51 KB, patch)
2014-04-29 09:34 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 02/12] change some stack nscstrings to spdystream* autocstring (3.47 KB, patch)
2014-04-29 09:35 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 03/12] fix http style: comments, whitespace, formatters (37.98 KB, patch)
2014-04-29 09:36 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 04/12] nshttpconnection - more proactive failed to connect (12.40 KB, patch)
2014-04-29 09:36 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 05/12] move Spdy*::EnsureBuffer to nsHttp so non spdy code can (23.36 KB, patch)
2014-04-29 09:36 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 06/12] nsAHttpTransaction::QueryHttpTransaction (2.77 KB, patch)
2014-04-29 09:36 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 07/12] newspdysession() no longer takes first transaction (19.83 KB, patch)
2014-04-29 09:36 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 08/12] add https proxy info type (8.31 KB, patch)
2014-04-29 09:37 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 09/12] https proxy info added to connection info and reqeuest (22.70 KB, patch)
2014-04-29 09:37 PDT, Patrick McManus [:mcmanus]
no flags Details | Diff | Splinter Review
[PATCH 09/12] https proxy info added to connection info and reqeuest (22.70 KB, patch)
2014-04-29 09:37 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 10/12] nsAHttpTransaction::ConnectionInfo (13.18 KB, patch)
2014-04-29 09:37 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 11/12] static nsHttpHandler::makeconnectstring instead of (8.16 KB, patch)
2014-04-29 09:37 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
[PATCH 12/12] https proxying (185.25 KB, patch)
2014-04-29 09:37 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
patch 13 (26.62 KB, patch)
2014-05-14 12:15 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
patch 14 - https proxying definitions for spdy 3.1 and http2 (58.67 KB, patch)
2014-05-14 12:16 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
patch 15 - its ok to clear a connection in a nshttppipeline (1.00 KB, patch)
2014-05-14 12:17 PDT, Patrick McManus [:mcmanus]
hurley: review+
Details | Diff | Splinter Review
BO (1.33 KB, patch)
2014-07-28 12:02 PDT, Patrick McManus [:mcmanus]
mcmanus: review+
lmandel: approval‑mozilla‑beta-
Details | Diff | Splinter Review

Description User image Dan Harkless 2007-04-24 11:36:58 PDT
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.8.1.3) Gecko/20070309 Firefox/2.0.0.3

I wanted to set up a remote SSL- and password-protected HTTP proxy on my server so that when I'm on unfriendly networks (like the schoool where my girlfriend works) I can use it to byppass URL / content filtering.  Squid appeared to do what I needed, providing an https_port option that had parameters to point to my site's SSL certificate, but when I tried to point Firefox at the proxy, they wouldn't talk.  Apparently Firefox and the other Mozilla browsers have no way to talk to an HTTP proxy over HTTPS.

The only workaround I can see is to have Squid just listen on the usual unencrypted HTTP port 3128 and use SSH port-forwarding to securely tunnel my proxy authentication and requests over the Internet.  This solution has issues if I'm using someone else's machine without (a port-forwarding-capable) SSH, etc.  It would be nice if the browser could support securely talking to the proxy itself, since the HTTP proxies appear to support HTTPS connections to them (I think this can be set up with Apache mod_proxy as well).

Reproducible: Always
Comment 1 User image Dan Harkless 2007-04-24 12:14:18 PDT
Oh, I should add that desire for this feature is not limited to just personal remote proxies.  The corporation I work at has recently gotten strict about requiring credentials to be encrypted when sent over our (large) WAN, and new web applications that don't have support for HTTPS are getting denied deployment.
Comment 2 User image Andris Atteka 2011-05-24 19:18:50 PDT
Chrome can do this: http://www.chromium.org/developers/design-documents/secure-web-proxy

They also have introduced a new PAC file directive to specify an HTTPS proxy (e.g. "HTTPS proxy.com:443")
Comment 3 User image Honza Bambas (:mayhemer) 2011-11-06 13:51:25 PST
Bug 573043 is related.  We must make sure that when both or just one of the proxy and the server are using NTLM authentication with Extended Protection enabled/enforced we correctly send the EP data to both.
Comment 4 User image Al Billings [:abillings] 2012-05-23 13:23:41 PDT
Is anyone interested in taking this on?
Comment 5 User image Patrick McManus [:mcmanus] 2012-05-23 14:04:22 PDT
(In reply to Al Billings [:abillings] from comment #4)
> Is anyone interested in taking this on?

I was until honza said ntlm :)
Comment 6 User image Al Billings [:abillings] 2012-05-23 15:09:01 PDT
The NTLM work is done though, I thought.
Comment 7 User image Jo Hermans 2012-06-13 08:50:34 PDT
*** Bug 764358 has been marked as a duplicate of this bug. ***
Comment 8 User image sunyucong 2012-07-16 15:42:59 PDT
with the SPDY support in this should have been a easy one, right? Please don't block on features such as NTLM support, let's just get a basic HTTPS proxy support first.
Comment 9 User image Curt Wetzel 2012-08-29 16:25:07 PDT
This is a pretty important capability for any organization using a forward proxy. Without it web traffic can be sniffed from the local network.

With the sophistication of malware, this is opens an unacceptable security hole for any organization using a standard proxy. Although proxy use is uncommon, here in the US it is used by many public libraries and schools to comply with COPA (the Child Online Protection Act). 

Any login or access to secure website is at risk of being compromised. 

Allowing an HTTPs connection between FireFox and the proxy would greatly reduce the threat.

Considering FireFox can connect to destination websites using HTTPs it seems that allowing a secure connection to the proxy server would be reasonably straight forward.
Comment 10 User image otacon 2013-04-27 17:54:26 PDT
This is a pretty big issue.

We wanted to authenticate our Proxy against Active Directory using LDAP.

We DON'T want our users to send they internal account Data, also used for VPN connection, unencrypted over the network.
Comment 11 User image naegelin 2013-11-02 17:18:43 PDT
So with all the recent wiretapping news does anyone else feel like this should float back up to the priority queue =) . I'd be willing to throw down some sponsorship $$ to get this working
Comment 12 User image Patrick McManus [:mcmanus] 2014-04-29 09:34:29 PDT
Created attachment 8414556 [details] [diff] [review]
[PATCH 01/12] add leak detection macros to http classes

From 0089e06dda86afd004c31e9f5eeb098428c51061 Mon Sep 17 00:00:00 2001
---
 netwerk/protocol/http/nsHttpConnectionMgr.cpp | 5 ++---
 netwerk/protocol/http/nsHttpConnectionMgr.h   | 2 +-
 netwerk/protocol/http/nsHttpRequestHead.cpp   | 6 ++++++
 netwerk/protocol/http/nsHttpRequestHead.h     | 1 +
 4 files changed, 10 insertions(+), 4 deletions(-)
Comment 13 User image Patrick McManus [:mcmanus] 2014-04-29 09:35:14 PDT
Created attachment 8414558 [details] [diff] [review]
[PATCH 02/12] change some stack nscstrings to spdystream* autocstring

From 21a282b4b1e5dddc251ab7db51d2cc0bb29d6401 Mon Sep 17 00:00:00 2001
---
 netwerk/protocol/http/Http2Stream.cpp  | 4 ++--
 netwerk/protocol/http/SpdyStream3.cpp  | 4 ++--
 netwerk/protocol/http/SpdyStream31.cpp | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)
Comment 14 User image Patrick McManus [:mcmanus] 2014-04-29 09:36:23 PDT
Created attachment 8414560 [details] [diff] [review]
[PATCH 03/12] fix http style: comments, whitespace, formatters

From 78f6022131455068ab29cbcafe2ed3120c944f00 Mon Sep 17 00:00:00 2001
---
 netwerk/protocol/http/Http2Session.cpp        |  4 +-
 netwerk/protocol/http/Http2Stream.cpp         | 56 ++++++++++----------
 netwerk/protocol/http/SpdySession3.cpp        | 52 +++++++++----------
 netwerk/protocol/http/SpdySession31.cpp       | 64 +++++++++++------------
 netwerk/protocol/http/SpdySession31.h         |  6 +--
 netwerk/protocol/http/SpdyStream3.cpp         | 73 +++++++++++++--------------
 netwerk/protocol/http/SpdyStream31.cpp        | 65 ++++++++++++------------
 netwerk/protocol/http/nsHttpConnection.cpp    |  2 +-
 netwerk/protocol/http/nsHttpConnection.h      | 20 ++++----
 netwerk/protocol/http/nsHttpConnectionMgr.cpp | 22 ++++----
 10 files changed, 182 insertions(+), 182 deletions(-)
Comment 15 User image Patrick McManus [:mcmanus] 2014-04-29 09:36:31 PDT
Created attachment 8414561 [details] [diff] [review]
[PATCH 04/12] nshttpconnection - more proactive failed to connect

From 0ca9d0e556e7a032abcb77aa144f6d6a4f982b1d Mon Sep 17 00:00:00 2001
 detection
---
 netwerk/protocol/http/nsHttpConnection.cpp    | 26 +++++++++++++++++++++++-
 netwerk/protocol/http/nsHttpConnection.h      |  5 +++--
 netwerk/protocol/http/nsHttpConnectionMgr.cpp | 29 +++++++++++++++++++--------
 netwerk/protocol/http/nsHttpConnectionMgr.h   |  5 +++++
 4 files changed, 54 insertions(+), 11 deletions(-)
Comment 16 User image Patrick McManus [:mcmanus] 2014-04-29 09:36:40 PDT
Created attachment 8414563 [details] [diff] [review]
[PATCH 05/12] move Spdy*::EnsureBuffer to nsHttp so non spdy code can

From 96ba7732ebece73cfc4bb7cfbf473abe7de886ec Mon Sep 17 00:00:00 2001
 share
---
 netwerk/protocol/http/Http2Push.cpp     |  6 ++----
 netwerk/protocol/http/Http2Session.cpp  | 28 ---------------------------
 netwerk/protocol/http/Http2Session.h    |  3 ---
 netwerk/protocol/http/Http2Stream.cpp   | 18 ++++++-----------
 netwerk/protocol/http/SpdyPush3.cpp     |  6 ++----
 netwerk/protocol/http/SpdyPush31.cpp    |  6 ++----
 netwerk/protocol/http/SpdySession3.cpp  | 34 ---------------------------------
 netwerk/protocol/http/SpdySession3.h    |  4 ----
 netwerk/protocol/http/SpdySession31.cpp | 34 ---------------------------------
 netwerk/protocol/http/SpdySession31.h   |  4 ----
 netwerk/protocol/http/SpdyStream3.cpp   | 18 ++++++-----------
 netwerk/protocol/http/SpdyStream31.cpp  | 18 ++++++-----------
 netwerk/protocol/http/nsHttp.cpp        | 34 +++++++++++++++++++++++++++++++++
 netwerk/protocol/http/nsHttp.h          |  5 +++++
 14 files changed, 63 insertions(+), 155 deletions(-)
Comment 17 User image Patrick McManus [:mcmanus] 2014-04-29 09:36:49 PDT
Created attachment 8414564 [details] [diff] [review]
[PATCH 06/12] nsAHttpTransaction::QueryHttpTransaction

From 77f2b2f795e9e780b0d8180127c51d5406adac87 Mon Sep 17 00:00:00 2001
---
 netwerk/protocol/http/nsAHttpTransaction.h | 5 +++++
 netwerk/protocol/http/nsHttpTransaction.h  | 2 ++
 2 files changed, 7 insertions(+)
Comment 18 User image Patrick McManus [:mcmanus] 2014-04-29 09:36:57 PDT
Created attachment 8414565 [details] [diff] [review]
[PATCH 07/12] newspdysession() no longer takes first transaction

From fc10147320d9cb4f1e3b0ecdf66c96d570e251cf Mon Sep 17 00:00:00 2001
---
 netwerk/protocol/http/ASpdySession.cpp     | 19 ++++++------
 netwerk/protocol/http/ASpdySession.h       |  5 +---
 netwerk/protocol/http/Http2Session.cpp     | 46 +++++++++++++++++-------------
 netwerk/protocol/http/Http2Session.h       |  2 +-
 netwerk/protocol/http/SpdySession3.cpp     | 22 ++++++++------
 netwerk/protocol/http/SpdySession3.h       |  2 +-
 netwerk/protocol/http/SpdySession31.cpp    | 22 ++++++++------
 netwerk/protocol/http/SpdySession31.h      |  2 +-
 netwerk/protocol/http/nsHttpConnection.cpp | 28 ++++++++----------
 9 files changed, 78 insertions(+), 70 deletions(-)
Comment 19 User image Patrick McManus [:mcmanus] 2014-04-29 09:37:02 PDT
https://tbpl.mozilla.org/?tree=Try&rev=51ab0a3f99ee
Comment 20 User image Patrick McManus [:mcmanus] 2014-04-29 09:37:06 PDT
Created attachment 8414566 [details] [diff] [review]
[PATCH 08/12] add https proxy info type

From 7b4412954c763e70fb2cd70a9bacd8c46a483201 Mon Sep 17 00:00:00 2001
---
 netwerk/base/src/ProxyAutoConfig.h          |  2 +-
 netwerk/base/src/nsProtocolProxyService.cpp | 27 +++++++++++++++++++++------
 netwerk/base/src/nsProxyInfo.cpp            |  7 +++++++
 netwerk/base/src/nsProxyInfo.h              |  1 +
 4 files changed, 30 insertions(+), 7 deletions(-)
Comment 21 User image Patrick McManus [:mcmanus] 2014-04-29 09:37:15 PDT
Created attachment 8414567 [details] [diff] [review]
[PATCH 09/12] https proxy info added to connection info and reqeuest

From 6203c5e9ee033757cb8ded299cd8e2b298fa3e55 Mon Sep 17 00:00:00 2001
 head
---
 netwerk/protocol/http/nsHttpChannel.cpp        | 11 ++++++-----
 netwerk/protocol/http/nsHttpConnection.cpp     | 12 ++++++------
 netwerk/protocol/http/nsHttpConnectionInfo.cpp | 25 +++++++++++++++++-------
 netwerk/protocol/http/nsHttpConnectionInfo.h   | 27 +++++++++++++++++++-------
 netwerk/protocol/http/nsHttpConnectionMgr.cpp  | 24 ++++++++++++-----------
 netwerk/protocol/http/nsHttpRequestHead.cpp    |  1 +
 netwerk/protocol/http/nsHttpRequestHead.h      |  4 ++++
 7 files changed, 68 insertions(+), 36 deletions(-)
Comment 22 User image Patrick McManus [:mcmanus] 2014-04-29 09:37:22 PDT
Created attachment 8414568 [details] [diff] [review]
[PATCH 09/12] https proxy info added to connection info and reqeuest

From 6203c5e9ee033757cb8ded299cd8e2b298fa3e55 Mon Sep 17 00:00:00 2001
 head
---
 netwerk/protocol/http/nsHttpChannel.cpp        | 11 ++++++-----
 netwerk/protocol/http/nsHttpConnection.cpp     | 12 ++++++------
 netwerk/protocol/http/nsHttpConnectionInfo.cpp | 25 +++++++++++++++++-------
 netwerk/protocol/http/nsHttpConnectionInfo.h   | 27 +++++++++++++++++++-------
 netwerk/protocol/http/nsHttpConnectionMgr.cpp  | 24 ++++++++++++-----------
 netwerk/protocol/http/nsHttpRequestHead.cpp    |  1 +
 netwerk/protocol/http/nsHttpRequestHead.h      |  4 ++++
 7 files changed, 68 insertions(+), 36 deletions(-)
Comment 23 User image Patrick McManus [:mcmanus] 2014-04-29 09:37:35 PDT
Created attachment 8414569 [details] [diff] [review]
[PATCH 10/12] nsAHttpTransaction::ConnectionInfo

From e64b1060afcabdabc1e82ec74b5d7041ed9ea40f Mon Sep 17 00:00:00 2001
---
 netwerk/protocol/http/Http2Push.cpp           | 13 +++++++++++++
 netwerk/protocol/http/Http2Session.cpp        |  8 ++++++++
 netwerk/protocol/http/NullHttpTransaction.cpp |  6 ++++++
 netwerk/protocol/http/NullHttpTransaction.h   |  2 --
 netwerk/protocol/http/SpdyPush3.cpp           | 13 +++++++++++++
 netwerk/protocol/http/SpdyPush31.cpp          | 13 +++++++++++++
 netwerk/protocol/http/SpdySession3.cpp        |  8 ++++++++
 netwerk/protocol/http/SpdySession31.cpp       |  8 ++++++++
 netwerk/protocol/http/nsAHttpTransaction.h    |  5 +++++
 netwerk/protocol/http/nsHttpPipeline.cpp      | 10 ++++++++++
 netwerk/protocol/http/nsHttpTransaction.cpp   |  6 ++++++
 netwerk/protocol/http/nsHttpTransaction.h     |  1 -
 12 files changed, 90 insertions(+), 3 deletions(-)
Comment 24 User image Patrick McManus [:mcmanus] 2014-04-29 09:37:43 PDT
Created attachment 8414570 [details] [diff] [review]
[PATCH 11/12] static nsHttpHandler::makeconnectstring instead of

From 15d4b4167472ea71fe8f03b1778b26b83eae6500 Mon Sep 17 00:00:00 2001
 member of nsHttpConnection
---
 netwerk/base/public/nsNetUtil.h            |  2 +-
 netwerk/protocol/http/nsHttpConnection.cpp | 66 ++++++++++++++++++------------
 netwerk/protocol/http/nsHttpConnection.h   |  4 ++
 netwerk/protocol/http/nsHttpHandler.cpp    |  2 +-
 netwerk/protocol/http/nsHttpHandler.h      |  2 +-
 5 files changed, 46 insertions(+), 30 deletions(-)
Comment 25 User image Patrick McManus [:mcmanus] 2014-04-29 09:37:52 PDT
Created attachment 8414571 [details] [diff] [review]
[PATCH 12/12] https proxying

From cc55b16e9d2580ae374f088b4e060ad211cd1144 Mon Sep 17 00:00:00 2001
---
 netwerk/base/src/nsSocketTransport2.cpp        |   36 +-
 netwerk/base/src/nsSocketTransport2.h          |    2 +
 netwerk/protocol/http/ASpdySession.cpp         |   10 +-
 netwerk/protocol/http/ASpdySession.h           |    8 +-
 netwerk/protocol/http/Http2Session.cpp         |   21 +-
 netwerk/protocol/http/Http2Session.h           |    3 +-
 netwerk/protocol/http/NullHttpTransaction.cpp  |    2 +-
 netwerk/protocol/http/NullHttpTransaction.h    |    4 +-
 netwerk/protocol/http/SpdySession3.cpp         |  158 ++-
 netwerk/protocol/http/SpdySession3.h           |   13 +-
 netwerk/protocol/http/SpdySession31.cpp        |   21 +-
 netwerk/protocol/http/SpdySession31.h          |    3 +-
 netwerk/protocol/http/SpdyStream3.cpp          |  127 ++-
 netwerk/protocol/http/SpdyStream3.h            |   23 +-
 netwerk/protocol/http/SpdyStream31.cpp         |    4 +-
 netwerk/protocol/http/TunnelUtils.cpp          | 1326 ++++++++++++++++++++++++
 netwerk/protocol/http/TunnelUtils.h            |  216 ++++
 netwerk/protocol/http/moz.build                |    1 +
 netwerk/protocol/http/nsAHttpConnection.h      |    9 +-
 netwerk/protocol/http/nsAHttpTransaction.h     |   20 +-
 netwerk/protocol/http/nsHttpAtomList.h         |    1 +
 netwerk/protocol/http/nsHttpConnection.cpp     |  455 +++++---
 netwerk/protocol/http/nsHttpConnection.h       |   25 +-
 netwerk/protocol/http/nsHttpConnectionInfo.cpp |   26 +-
 netwerk/protocol/http/nsHttpConnectionInfo.h   |   12 +-
 netwerk/protocol/http/nsHttpConnectionMgr.cpp  |  317 ++++--
 netwerk/protocol/http/nsHttpConnectionMgr.h    |   15 +-
 netwerk/protocol/http/nsHttpHandler.h          |    1 +
 netwerk/protocol/http/nsHttpTransaction.cpp    |    2 +
 netwerk/protocol/http/nsHttpTransaction.h      |    4 +
 30 files changed, 2534 insertions(+), 331 deletions(-)
 create mode 100644 netwerk/protocol/http/TunnelUtils.cpp
 create mode 100644 netwerk/protocol/http/TunnelUtils.h
Comment 26 User image Patrick McManus [:mcmanus] 2014-04-29 09:55:47 PDT
nick - 

first, sorry.

second, I split a bunch of stuff out to make it easier. but most of it still ended up in patch 12.

third, maybe a vidyo session is in order to make it easier?

fourth, I can do a github clone if it would make it easier

fifth - at least I can go do those http2 reviews now :)
Comment 27 User image Patrick McManus [:mcmanus] 2014-04-29 11:36:34 PDT
fyi- this is a convenient way to setup the config

function FindProxyForURL(url, host) {
return "HTTPS localhost:2443;"
}
Comment 28 User image Patrick McManus [:mcmanus] 2014-04-29 12:00:19 PDT
the tlsproxy01 branch of the git clone
https://github.com/mcmanus/gecko-dev

has these patches

https://github.com/mcmanus/gecko-dev/tree/tlsproxy01
Comment 29 User image Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2014-04-29 18:59:11 PDT
Comment on attachment 8414560 [details] [diff] [review]
[PATCH 03/12] fix http style: comments, whitespace, formatters

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

::: netwerk/protocol/http/SpdySession31.cpp
@@ +48,5 @@
>  SpdySession31::SpdySession31(nsAHttpTransaction *aHttpTransaction,
>                               nsISocketTransport *aSocketTransport,
>                               int32_t firstPriority)
>    : mSocketTransport(aSocketTransport),
> +    mSegmentReader(nullptr),

Do you want to put commas at the beginning of the line, like in the other blocks in this patch like this?
Comment 30 User image Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2014-04-29 19:54:12 PDT
Comment on attachment 8414563 [details] [diff] [review]
[PATCH 05/12] move Spdy*::EnsureBuffer to nsHttp so non spdy code can

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

Hooray, less duplication of code!
Comment 31 User image Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2014-04-30 10:41:03 PDT
Comment on attachment 8414568 [details] [diff] [review]
[PATCH 09/12] https proxy info added to connection info and reqeuest

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

::: netwerk/protocol/http/nsHttpConnectionInfo.h
@@ +78,4 @@
>      bool UsingProxy();
>  
> +    // Returns true when proxying over HTTP or HTTPS
> +    bool UsingHttpProxy() const { return mUsingHttpProxy | mUsingHttpsProxy; }

|| instead of |
Comment 32 User image Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2014-05-06 08:44:17 PDT
Patrick - just to keep you not so in the dark, first round of review is incoming. At this point, I just have to copy my notes into splinter, and then we can get the ball moving forward on this again.
Comment 33 User image Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2014-05-06 12:02:59 PDT
Comment on attachment 8414571 [details] [diff] [review]
[PATCH 12/12] https proxying

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

OK, here we go!

First - oh my, is this ugly (but in a GOOD way!)
Second - thanks for the gigantic comment at the top of TunnelUtils.h - it made things much easier to understand. We should write this up in more prose and put it on a wiki somewhere (same with spdy and http/2, since they follow a similar pattern of indirection mania).
Third - I'm adding Honza here to get a second pair of eyes. Something this large I don't want to r+ and then have it blow up :)
Fourth - see the rest of my comments below. Most of the really important ones are bigger blobs of text, so they should be easy to pick out. Doesn't mean the others are unimportant, though :)

::: netwerk/base/src/nsSocketTransport2.cpp
@@ +832,5 @@
> +        if (proxyType && (proxyInfo->IsHTTP() ||
> +                          proxyInfo->IsHTTPS() ||
> +                          proxyInfo->IsDirect() ||
> +                          !strcmp(proxyType, "unknown"))) {
> +        proxyType = nullptr;

indentation

::: netwerk/protocol/http/ASpdySession.cpp
@@ +21,5 @@
>  #include "Http2Push.h"
>  #include "SpdySession3.h"
>  #include "SpdySession31.h"
>  #include "Http2Session.h"
> +#include <algorithm>

This doesn't seem used by any of the new code (though maybe it was used in old code that's not showing up in splinter?) Remove it if not used anywhere.

::: netwerk/protocol/http/TunnelUtils.cpp
@@ +155,5 @@
> +    }
> +
> +    mEncryptedTextUsed -= amt;
> +    if (mEncryptedTextUsed) {
> +      memmove (mEncryptedText, mEncryptedText + amt, mEncryptedTextUsed);

Extra space before the paren here (this seems to be a common occurrence with memmove and memcpy, so I'm not going to call them all out).

@@ +1317,5 @@
> +
> +} // namespace mozilla::net
> +} // namespace mozilla
> +
> +#if 0

Get rid of this block, and file followups.

::: netwerk/protocol/http/TunnelUtils.h
@@ +43,5 @@
> +
> +now pop the stack back up to SpdyConnectTransaction::WriteSegment
> +The data has been read and is stored in mInputData
> +
> +  mTunnelConn::OnInputStreamRead(mTunnelStreamIn)

s/OnInputStreamRead/OnInputStreamReady/

@@ +46,5 @@
> +
> +  mTunnelConn::OnInputStreamRead(mTunnelStreamIn)
> +  mTunnelConn::OnSocketReadable()
> +  TLSFilterTransaction::WriteSegment()
> +  RealHttpTrans::WriteSegment()

s/RealHttpTrans/nsHttpTransaction/ (and mark that this is the real trans in parens, like you do with sockets above)

@@ +120,5 @@
> +  void Cleanup();
> +  int32_t FilterWrite(const char *aBuf, int32_t aAmount);
> +  int32_t FilterRead(char *aBuf, int32_t aAmount);
> +
> +  static PRStatus sGetPeerName(PRFileDesc *fd, PRNetAddr*addr);

Not totally crazy about naming these sBlahBlah. It's good to know they're static, but at first glance, I thought they were variables. Maybe use capital S here? Up to you, really.

::: netwerk/protocol/http/nsHttpConnection.cpp
@@ +12,5 @@
>  #define LOG(args) LOG5(args)
>  #undef LOG_ENABLED
>  #define LOG_ENABLED() LOG5_ENABLED()
>  
> +#include <algorithm>

Just like in ASpdySession.cpp - doesn't seem used (but maybe I missed something). If unused, kill it.

@@ +302,4 @@
>          // By writing 0 bytes to the socket the SSL handshake machine is
>          // pushed forward.
>          uint32_t count = 0;
> +        // write to wrong socket :)

Clarify this comment so we know what's up with writing to the wrong socket.

@@ +1541,5 @@
> +                if (mTLSFilter) {
> +                    LOG(("  blocked tunnel (handshake?)\n"));
> +                    mTLSFilter->NudgeTunnel(this, this, this);
> +                }
> +                else {

Move this up by the preceding closing brace, to match other changes.

@@ +1835,4 @@
>          return rv;
>      }
>  
> +

Extra blank line.

::: netwerk/protocol/http/nsHttpConnection.h
@@ +189,5 @@
>      static nsresult MakeConnectString(nsAHttpTransaction *trans,
>                                        nsHttpRequestHead *request,
>                                        nsACString &result);
> +    void    SetupSecondaryTLS();
> +    void    SetInHttp2Tunnel(bool arg);

I'm not wild about using Http2 here, but Spdy pretty much everywhere else (unless I've missed something and this really is http/2 specific). Can we settle on one (most likely Spdy, since that's what we're already using everywhere else)?

::: netwerk/protocol/http/nsHttpConnectionMgr.cpp
@@ +1072,5 @@
>                  alreadyHalfOpen = true;
>                  break;
>              }
>          }
> + 

Extra whitespace.

@@ +2380,5 @@
>      nsConnectionEntry *ent = LookupConnectionEntry(conn->ConnectionInfo(),
>                                                     conn, nullptr);
> +    if (!ent) {
> +        // this can happen if the connection is made outside of the
> +        // connection manager an is being "reclaimed" for use with

s/an/and/

@@ +2447,2 @@
>      }
>      else {

Move this up by the preceding closing brace, since you've done it elsewhere.

::: netwerk/protocol/http/nsHttpTransaction.h
@@ +102,5 @@
>  
>      bool      ProxyConnectFailed() { return mProxyConnectFailed; }
>  
> +    void SetDontRouteViaWildCard(bool var) { mDontRouteViaWildCard = var; }
> +    bool DontRouteViaWildCard() { return mDontRouteViaWildCard; }

I'm not wild (ha!) about calling it DontRouteViaWildCard - in some ways it sounds like setting this to true means it wouldn't go over the tunnel (since the tunnel is like a wild card for all your connections). At least that's how I read it at first. It makes more sense now that I've read the whole patch, but it might be good to at least comment here (and perhaps other key points, such as where you set don't route via wild card to true) that it doesn't mean the transaction won't go over the tunnel, just that it's not responsible for setting up the tunnel.
Comment 34 User image Honza Bambas (:mayhemer) 2014-05-07 08:56:36 PDT
I'll start next week..  This is.. HUGE!
Comment 35 User image Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2014-05-07 09:11:41 PDT
Comment on attachment 8414571 [details] [diff] [review]
[PATCH 12/12] https proxying

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

So somehow, my intention to r+ and also r? honza turned into clearing r? and r?'ing honza. Let's fix that.
Comment 36 User image Jason Duell [:jduell] (needinfo me) 2014-05-07 14:42:15 PDT
Comment on attachment 8414571 [details] [diff] [review]
[PATCH 12/12] https proxying

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

Talked to Nick--given how busy Honza is and how thoroughly Nick reviewed the code here already, Nick's +r is enough.
Comment 37 User image Patrick McManus [:mcmanus] 2014-05-08 06:51:30 PDT
(In reply to Nicholas Hurley [:hurley] from comment #31)
> Comment on attachment 8414568 [details] [diff] [review]
> [PATCH 09/12] https proxy info added to connection info and reqeuest
> 
> Review of attachment 8414568 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: netwerk/protocol/http/nsHttpConnectionInfo.h
> @@ +78,4 @@
> >      bool UsingProxy();
> >  
> > +    // Returns true when proxying over HTTP or HTTPS
> > +    bool UsingHttpProxy() const { return mUsingHttpProxy | mUsingHttpsProxy; }
> 
> || instead of |

thank you!
Comment 38 User image Patrick McManus [:mcmanus] 2014-05-09 04:56:29 PDT
https://tbpl.mozilla.org/?tree=Try&rev=64e4cf18d045
Comment 40 User image [Out of Office Until 24-April] Ryan VanderMeulen [:RyanVM] 2014-05-09 12:26:26 PDT
Backed out for Android crashes.
https://hg.mozilla.org/integration/mozilla-inbound/rev/2bafee610170

https://tbpl.mozilla.org/php/getParsedLog.php?id=39377377&tree=Mozilla-Inbound
Comment 41 User image Patrick McManus [:mcmanus] 2014-05-09 12:34:08 PDT
given the below it seems likely this test will show the error on desktop with debug and pipelining on..

Android 4.0 Panda mozilla-inbound debug test mochitest-4 on 2014-05-09 11:47:26 PDT for push 2a607cddc4cb

PROCESS-CRASH | /tests/content/svg/content/test/test_onerror.xhtml | application crashed [@ mozilla::net::nsHttpPipeline::SetConnection(mozilla::net::nsAHttpConnection*)]
05-09 11:56:34.953 F/MOZ_Assert( 2253): Assertion failure: !mConnection (already have a connection), at /builds/slave/m-in-and-d-0000000000000000000/build/netwerk/protocol/http/nsHttpPipeline.cpp:411
Return code: 1
05-09 11:56:34.953 F/MOZ_Assert( 2253): Assertion failure: !mConnection (already have a connection), at /builds/slave/m-in-and-d-0000000000000000000/build/netwerk/protocol/http/nsHttpPipeline.cpp:411

11:57:29     INFO -   0  libxul.so!mozilla::net::nsHttpPipeline::SetConnection(mozilla::net::nsAHttpConnection*) [nsHttpPipeline.cpp:2a607cddc4cb : 411 + 0x14]
11:57:29     INFO -       r4 = 0x00000000    r5 = 0x68cbb280    r6 = 0x68bf2f40    r7 = 0x804b000d
11:57:29     INFO -       r8 = 0x68cdb9d0    r9 = 0x00000023   r10 = 0x68cdb9d0    fp = 0x5be58260
11:57:29     INFO -       sp = 0x661fab90    lr = 0x62040d3f    pc = 0x62041052
11:57:29     INFO -      Found by: given as instruction pointer in context
11:57:29     INFO -   1  libxul.so!mozilla::net::nsHttpConnectionMgr::DispatchAbstractTransaction(mozilla::net::nsHttpConnectionMgr::nsConnectionEntry*, mozilla::net::nsAHttpTransaction*, unsigned int, mozilla::net::nsHttpConnection*, int) [nsHttpConnectionMgr.cpp:2a607cddc4cb : 1866 + 0x5]
11:57:29     INFO -       r4 = 0x661faba8    r5 = 0x00000000    r6 = 0x68bf2f40    r7 = 0x804b000d
11:57:29     INFO -       r8 = 0x68cdb9d0    r9 = 0x00000023   r10 = 0x68cdb9d0    fp = 0x5be58260
11:57:29     INFO -       sp = 0x661faba0    pc = 0x6203b903
11:57:29     INFO -      Found by: call frame info
11:57:29     INFO -   2  libxul.so!mozilla::net::nsHttpConnectionMgr::DispatchTransaction(mozilla::net::nsHttpConnectionMgr::nsConnectionEntry*, mozilla::net::nsHttpTransaction*, mozilla::net::nsHttpConnection*) [nsHttpConnectionMgr.cpp:2a607cddc4cb : 1797 + 0xf]
11:57:29     INFO -       r4 = 0x5be54940    r5 = 0x00000023    r6 = 0x66381340    r7 = 0x0000000b
11:57:29     INFO -       r8 = 0x68bf2f40    r9 = 0x00000000   r10 = 0x68cdb9d0    fp = 0x5be58260
11:57:29     INFO -       sp = 0x661fabd0    pc = 0x6203ba63
11:57:29     INFO -      Found by: call frame info
11:57:29     INFO -   3  libxul.so!mozilla::net::nsHttpConnectionMgr::nsHalfOpenSocket::OnOutputStreamReady(nsIAsyncOutputStream*) [nsHttpConnectionMgr.cpp:2a607cddc4cb : 3155 + 0xb]
11:57:29     INFO -       r4 = 0x68bf2fa0    r5 = 0x63f96b30    r6 = 0x661fac30    r7 = 0x00000000
11:57:29     INFO -       r8 = 0x00000000    r9 = 0x68bf2fc0   r10 = 0x00000000    fp = 0x5be58260
11:57:29     INFO -       sp = 0x661fac00    pc = 0x6203e1cd
11:57:29     INFO -      Found by: call frame info
Comment 42 User image [Out of Office Until 24-April] Ryan VanderMeulen [:RyanVM] 2014-05-09 12:34:47 PDT
So far, the occurrences have all been Android 4.0 debug mochitest-4, FWIW.
Comment 43 User image [Out of Office Until 24-April] Ryan VanderMeulen [:RyanVM] 2014-05-09 13:07:20 PDT
And B2G debug.
https://tbpl.mozilla.org/php/getParsedLog.php?id=39381502&tree=Mozilla-Inbound
Comment 44 User image Patrick McManus [:mcmanus] 2014-05-12 12:10:59 PDT
I think patch 4 has uncovered a latent bug..

connection manager does transaction->setconnection(foo)
if (failed(foo->activate())) {
 transaction->setconnection(nullptr);
 destroy (foo)
}

and that's right enough.. nshttppipeline::setconnection(c)
says assert (!mConnection) which makes sense because you don't want a double connection, but doesn't take into consideration the case where c == nullptr where mConnection is being freed.

patch 4 is the reason the test coverage has shifted to show this bug.. a connection failure now shows up in the activate() path at an earlier time than before.

oh that's the theory. let's see what try says :)
Comment 45 User image Patrick McManus [:mcmanus] 2014-05-14 12:15:02 PDT
Created attachment 8422640 [details] [diff] [review]
patch 13

I'm going to present the update as interdiffs to the previous series for ease of review.

This patch tries to ease some of the object heirarchy magic that was going on with weak pointer and do_queryObject in patch 12. If I combined it with patch 12 it would actually simplify that patch, but I think that makes for a harder incremental review.

It also sets the KA Caps flag on a transaction that has forced the creation of a connect tunnel before that transaction is given back to the connection manager for dispatch (in order to use the new tunnel - which is now technically an unused persistent connection.) Given that the transaction honored a no-KA setting on its first trip through the CM in order to create the tunnel, this seems ok.

The Filter transaction now takes the default read/write handlers at ctor time (though they can be updated with individual read/write segment calls). Again - the odd nature of TLS deciding to read when it has only been called for write necessitates it.
Comment 46 User image Patrick McManus [:mcmanus] 2014-05-14 12:16:13 PDT
Created attachment 8422642 [details] [diff] [review]
patch 14 - https proxying definitions for spdy 3.1 and http2

support for http/2 and spdy3.1 that was omitted last time
Comment 47 User image Patrick McManus [:mcmanus] 2014-05-14 12:17:33 PDT
Created attachment 8422643 [details] [diff] [review]
patch 15 - its ok to clear a connection in a nshttppipeline

see comment 44
Comment 48 User image Patrick McManus [:mcmanus] 2014-05-14 12:21:15 PDT
https://tbpl.mozilla.org/?tree=Try&rev=75d2ed5d813c
Comment 49 User image Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2014-05-14 17:06:39 PDT
Comment on attachment 8422640 [details] [diff] [review]
patch 13

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

Looks pretty good, One question below, r=me contingent on an answer :)

::: netwerk/protocol/http/SpdyStream3.cpp
@@ +1558,5 @@
>  
>  void
>  SpdyStream3::MapStreamToHttpConnection()
>  {
> +  nsRefPtr<SpdyConnectTransaction>qiTrans(mTransaction->QuerySpdyConnectTransaction());

nit: space before qiTrans

::: netwerk/protocol/http/TunnelUtils.cpp
@@ -333,5 @@
>    }
>  
>    mSegmentWriter = aWriter;
>    nsresult rv = mTransaction->WriteSegments(this, aCount, outCountWritten);
> -  mSegmentWriter = nullptr;

Was this deletion intentional, given we still set mSegmentWriter above? It seems like this is (or at least could be) right, given the new way of getting mSegment{Writer,Reader} in the ctor, but I want to double-check to make sure it wasn't an oversight.

Also, assuming this is right, what about a similar change for mSegmentReader in ReadSegments?
Comment 50 User image Patrick McManus [:mcmanus] 2014-05-15 06:52:40 PDT
(In reply to Nicholas Hurley [:hurley] from comment #49)

> 
> Also, assuming this is right, what about a similar change for mSegmentReader
> in ReadSegments?

yes, it was intentional now that segmentreader/segmentwriter can be used in ways other than the corresponding readsegment/writesegment stack. (i.e. ssl reads when asked to write).

I'll add a similar change for ReadSegments() - good suggestion.
Comment 51 User image Nicholas Hurley [:nwgh][:hurley] (also hurley@todesschaf.org) 2014-05-15 11:53:30 PDT
Comment on attachment 8422642 [details] [diff] [review]
patch 14 - https proxying definitions for spdy 3.1 and http2

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

::: netwerk/protocol/http/Http2Stream.cpp
@@ +1195,5 @@
> +
> +void
> +Http2Stream::MapStreamToHttpConnection()
> +{
> +  nsRefPtr<SpdyConnectTransaction>qiTrans(mTransaction->QuerySpdyConnectTransaction());

nit: space before qiTrans

::: netwerk/protocol/http/Http2Stream.h
@@ +88,5 @@
>  
>    void UpdateTransportSendEvents(uint32_t count);
>    void UpdateTransportReadEvents(uint32_t count);
>  
> +  // NS_ERROR_ABORT terminates stream, other failure terminates session

random commentary: ugh. not entirely wild about this, but worse things have happened, and we have special-case failure modes all over the place already. One more won't hurt, right?

::: netwerk/protocol/http/SpdyStream31.cpp
@@ +1582,5 @@
> +
> +void
> +SpdyStream31::MapStreamToHttpConnection()
> +{
> +  nsRefPtr<SpdyConnectTransaction>qiTrans(mTransaction->QuerySpdyConnectTransaction());

nit: space before qiTrans
Comment 54 User image Patrick McManus [:mcmanus] 2014-07-28 12:02:22 PDT
Created attachment 8463535 [details] [diff] [review]
BO

Approval Request Comment
[Feature/regressing bug #]: this one
[User impact if declined]: new feature that has reliability bugs related only to use of the feature (i.e. it is not generally destabilizing)
[Describe test coverage new/current, TBPL]: backout
[Risks and why]: minimal
[String/UUID change made/needed]: none

I'm going to ask that we disable this on FF32 via flipping an existing pref and instead target FF33. There is no UI - this is just new syntax in the PAC file format that won't be supported yet.

FF32 and FF33 are missing the related fixes for 1040930, 1040323, and 1043402. The first is on FF34 (and will be uplifted to 33) the latter two haven't landed at all yet. They're all expected to be small and suitable for FF33 but FF32 is too aggressive at this stage.

If it goes worse than planned we have plenty of time to disable on FF33, but this feature is eagerly awaited to give parity with Google's Compressing Data Proxy and it is receiving positive feedback.
Comment 55 User image Lawrence Mandel [:lmandel] (use needinfo) 2014-07-28 16:59:07 PDT
Comment on attachment 8463535 [details] [diff] [review]
BO

OK. We'll back this out as per your suggestion. Can you please file a separate bug for the backout as that is much easier to track.
Comment 56 User image Patrick McManus [:mcmanus] 2014-07-29 07:50:41 PDT
(In reply to Lawrence Mandel [:lmandel] from comment #55)
> Comment on attachment 8463535 [details] [diff] [review]
> BO
> 
> OK. We'll back this out as per your suggestion. Can you please file a
> separate bug for the backout as that is much easier to track.

https://bugzilla.mozilla.org/show_bug.cgi?id=1045640
Comment 57 User image Jason Haar 2014-08-09 00:23:58 PDT
Not quite sure where else to comment on this (the links in here fly off to other closed tickets or real dev ones), but if you're going to add proxy support via TLS, then can you make sure client certs are supported too? I'm thinking there are large organizations that already run their own PKI and "everyone" already has client certs installed, so allowing them to use a central/corporate Internet-based proxy server would be a great feature. Obviously it would have to support some form of authentication, and client certs is infinitely preferable to username/password, because infosec best practice is not to expose internal authentication to the Internet. ie they wouldn't like using Active Directory/etc, but a strong authentication model like client certs would be brilliant

Hopefully this wouldn't be too difficult. I've always thought Mozilla were the best at client cert support, being using it for 10+ years via firefox AND thunderbird ;-)
Comment 58 User image Sylvestre Ledru [:sylvestre] 2014-08-28 05:57:46 PDT
Already existing in the release notes with "Connect to HTTP proxy over HTTPS" as wording.
I removed it from the 32 release notes and added it to 33 aurora & beta.
Comment 59 User image Richard Soderberg [:atoll] 2015-01-07 03:05:12 PST
*** Bug 709106 has been marked as a duplicate of this bug. ***
Comment 60 User image Eric H. Jung 2015-01-10 07:43:47 PST
(In reply to Jason Haar from comment #57)
> Not quite sure where else to comment on this (the links in here fly off to
> other closed tickets or real dev ones), but if you're going to add proxy
> support via TLS, then can you make sure client certs are supported too? I'm
> thinking there are large organizations that already run their own PKI and
> "everyone" already has client certs installed, so allowing them to use a
> central/corporate Internet-based proxy server would be a great feature.
> Obviously it would have to support some form of authentication, and client
> certs is infinitely preferable to username/password

I think you should file a new bug for this.
Comment 61 User image Jeremy Akers 2015-03-11 06:54:43 PDT
I've been watching this bug/issue since 2013 and was quite pleased when I saw that it was being addressed.

However I've been trying to find documentation on how to actually use this new feature to connect to proxies via HTTPS and I can't seem to find it.  My Google searches keep taking me to either bugs like this or old documents that say it's not supported.

I've tried setting it up in the UI as well as in a proxy.pac file.  I'm sure I'm doing something wrong but I can't find the "manual" for how to do it so I thought maybe someone watching this bug would know where I could find the documentation for this feature?
Comment 62 User image Xander D Harkness 2015-03-11 07:04:50 PDT
@Jeremy Akers

I wrote a very quick and rough how to set up and connect to squid over SSL here:
https://ssl.harkness.se/proxy-ssl/
Comment 63 User image toddy.sun 2015-03-12 00:04:37 PDT
@Xander D Harkness,

I tried the same as you instructed, but without any luck. The ssl proxy does not work for me.

I’m using squid-3.4.10, configured below:
http_port 8080
https_port 8443 cert=/path/to/cert.pem

When I set my chrome to connect to port 8080, it works. but when I used 8443 the https_port, it does not. The error is below:

A proxy server is a server that acts as an intermediary between your computer and other servers. Right now, your system is configured to use a proxy, but Google Chrome can’t connect to it.
If you use a proxy server…
Check your proxy settings or contact your network administrator to make sure the proxy server is working. If you don’t believe you should be using a proxy server: Go to the Chrome menu > Settings > Show advanced settings… > Change proxy settings… and make sure your configuration is set to “no proxy” or “direct.”
Error code: ERR_PROXY_CONNECTION_FAILED

Do you have any ideas about why this is happening?

Is there any other steps I need to do to allow the browser trust the cert from proxy server?

Thanks.
Comment 64 User image toddy.sun 2015-03-12 19:56:28 PDT
BTW, when I'm using firefox 36, the browser keeps asking me "This Connection is Untrusted" and let me "add exception". but after I added exception, it redirects to the same page, like a infinite loop.
Comment 65 User image Patrick McManus [:mcmanus] 2015-03-13 07:10:29 PDT
toddy.sun - it sounds like your proxy has an invalid cert according to your trust list. you've got at least 3 choices

1] add CA signing cert used to create the proxy cert to your root store.. there are lots of tutorials for this on the web (http://www.herongyang.com/Cryptography/Web-Browser-Firefox-Import-CA-Certificate.html)

2] get yourself a cert firefox considers valid by default. startssl.com will do that for you for free.

3] you can do a TOFU exception for the proxy case, but you have to do it a little differently.
 a] turn off the proxy use in firefox.
 b] put https://PROXYNAME:PROXYPORT/ in the location bar (use the same name and port number as you have configured in the PAC - you can't use ip addresses or default ports.. you can't use ip addresses because they can't be verified by the PKI and exceptions are stored per port.
 c] override the cert warning and perm. store the exception. The response you get will be meaningless as you are now addressing the proxy port as if it was an endpoint
 d] turn the proxy back on and use it.

hth
Comment 66 User image toddy.sun 2015-03-15 19:24:39 PDT
Thank you Patrick for the instruction, that solves my issue.
Comment 67 User image herrernst 2015-03-19 05:41:36 PDT
Thanks, this is really cool. Is it possible already to set an such an proxy in settings dialog or in about:config? I am using a PAC file at the moment. Haven't found any more information about it.
Comment 68 User image Patrick McManus [:mcmanus] 2015-03-19 11:43:23 PDT
network.proxy.autoconfigurl = data:text/plain,function FindProxyForURL(u,h){return HTTPS host:port;}
Comment 69 User image herrernst 2015-03-19 11:46:46 PDT
Thanks Patrick, nice way to use a PAC without having to host it somewhere. (You can get rid of another 3 extra characters, the unused params "u,h".)

Still, a nice setting would be preferable.
Comment 70 User image Eric H. Jung 2017-01-21 08:10:30 PST
This is now supported natively in the FoxyProxy addon. No PAC required. See screenshot here: http://i.imgur.com/TGIEk8c.jpg

If the proxy server's certificate is not valid, you are prompted to accept the certificate into the Fx store.

Be sure you are using FoxyProxy Standard 4.6.1 or FoxyProxy Basic 3.6.1 (or higher). They are submitted to AMO and awaiting approval.
Comment 71 User image Erick Zite 2017-03-16 13:09:06 PDT
BTW, when I'm using firefox 36, the browser keeps asking me "This Connection is Untrusted" and let me "add exception". but after I added exception, it redirects to the same page, like a infinite loop.


:cool: https://logoreg.com/favicon.png
Comment 72 User image Patrick McManus [:mcmanus] 2017-03-16 14:04:57 PDT
1]get your proxy a cert from LE

or

2a] momentarily disable the proxy
2b] put https://PROXYIP:PORT/ in the location bar
2c] get cert warning and store the exception for it
2d] re-enable the proxy

basically what is happening is that you are storing exceptions per origin rather than for the proxy.
Comment 73 User image Eric H. Jung 2017-03-18 14:50:46 PDT
(In reply to Patrick McManus [:mcmanus] from comment #72)
> 1]get your proxy a cert from LE
> 
> or
> 
> 2a] momentarily disable the proxy
> 2b] put https://PROXYIP:PORT/ in the location bar
> 2c] get cert warning and store the exception for it
> 2d] re-enable the proxy
> 
> basically what is happening is that you are storing exceptions per origin
> rather than for the proxy.

FYI, you don't need to do any of this if you use the FoxyProxy extension 4.6.1. You'll get prompted to store the cert when you setup the proxy for the first time, and that's all.

Note You need to log in before you can comment on or make changes to this bug.