Add support for HTTPS over HTTP on Node.js #5037

ppati000 · 2022-10-06T18:10:36Z

So far, Axios did not support HTTPS requests over HTTP proxies.

This pull request implements of CONNECT requests to establish a tunnel through the proxy. The implementation works with both Connection: close and Connection: keep-alive, so that multiple HTTPS requests can be sent after one CONNECT. Only Node.js is supported for now.

This pull request should fix the following issues, which still seem to be unresolved:

Feedback is highly appreciated!

Caveats

If the upstream server closes the connection first (e.g., when keep-alive is disabled), the proxy may send an RST packet to the client. Here's what I believe is happening after some Wireshark debugging:

After sending the response, the upstream server sends TLS close_notify and FIN=1.
Proxy relays the packet to the client.
Proxy sends FIN reply to upstream server on its own (closing its connection with the upstream server)
Client sends TLS close_notify packet back to the proxy
Proxy does not understand TLS and has already closed the connection to the upstream server => sends RST to client.
Node.js triggers ECONNRESET although the HTTPS response has already been received (res.complete === true).

My mitigation for this issue was to ignore ECONNRESET errors if res.complete === true. This mitigation does not work with follow-redirects, so redirect support is disabled for HTTPS over HTTP.

Nevon · 2022-10-10T15:07:40Z

lib/adapters/http.js

@@ -47,6 +48,56 @@ function dispatchBeforeRedirect(options) {
  }
 }

+/**
+ * Provides an HTTP tunnel socket via an HTTP CONNECT request.
+ * The tunnel socket is used for sending HTTPS requests via an HTTP proxy.


HTTP requests can be made via an HTTP proxy using the CONNECT method as well. Shouldn't this only create the TlsSocket in case the destination is using HTTPS, and otherwise return the original socket that was used to make the CONNECT request?

EDIT: Oh I see further down that this is only used when the target is HTTPS. So for an HTTP proxy with a target that's not using TLS, it assumes it's not gonna use the connect method, which is quite reasonable for an HTTP client.

@Nevon exactly, CONNECT is only used if the destination uses TLS. I figured it would not make that much sense for non-TLS destinations because of the extra round trip.

ppati000 · 2022-10-11T12:19:50Z

Will probably get back to this PR after #5097, which will cause some conflicts.

mikaelfs · 2022-11-11T01:28:20Z

Any plan or target release version to merge this upstream @ppati000 ? We also happen to stumble on the same https-over-http-proxy issue. It would be great to have this fix in the official release.

ferranmuntada · 2022-11-11T08:06:29Z

We have encountered this problem in our project, do you know when it will be added? As soon as possible please!

jasonsaayman · 2022-11-25T13:52:25Z

@ppati000 please can you let me know if you will be able to complete this PR? The other PR you mentioned has been merged.

test/unit/adapters/http.js

+      proxy.listen(4000, async () => {
+        try {
+          const httpsAgent = new https.Agent({
+            rejectUnauthorized: false,


ppati000 · 2022-12-14T15:06:13Z

@mikaelfs @ferranmuntada @jasonsaayman Hey everyone, thanks for your interest :) Will look into this pull request again over the holidays.

foreverest · 2023-01-19T07:36:19Z

@ppati000 could you share latest news on this please? Really interested in having this asap.

adriencarbonaro · 2023-01-25T18:00:23Z

+1

dudizi · 2023-01-25T19:36:17Z

Great job on the PR, @ppati000. It appears that this issue is widely spread, as many 3rd party SDKs use axios. Is there a target release version for a fix so it's just a matter of updating the version dependency?

mbargiel · 2023-01-31T22:47:37Z

lib/adapters/http.js

    }

+    options._httpsOverHttp = proxy.protocol === "http:" && options.protocol === "https:";


What about https over https? You still need a tunnel in that case because you need end-to-end encryption with the intended recipient.

Just for reference, here's how I fixed HTTP CONNECT support for secure HTTPS proxying. My solution relies on the underlying agents (I mentioned it here but I found it a bit too hacky and it required introducing an unmaintained dependency, which I wasn't comfortable with).

You could also check out my fork directly for ideas : mbargiel/axios@e6f9026...feature/fix-proxy/http-connect

The nice thing with agents is that you don't have to worry about problems with follow-redirects: the tunnel is established in a lower abstraction layer, and once Axios has the socket, it's established properly to the end server. Redirects work over the tunnel.

mikaelfs · 2023-02-01T02:19:04Z

@mbargiel The nice thing with agents is that you don't have to worry about problems with follow-redirects: the tunnel is established in a lower abstraction layer, and once Axios has the socket, it's established properly to the end server. Redirects work over the tunnel.

This is actually how we resolved the proxy issue. With custom http/https agent, https-over-http proxy or https-over-https proxy is properly handled by the agent and is independent of proxy handling by axios. If time permits, I will share some write-up explaining our method.

mbargiel · 2023-02-01T14:59:43Z

@mikaelfs This is actually how we resolved the proxy issue. With custom http/https agent, https-over-http proxy or https-over-https proxy is properly handled by the agent and is independent of proxy handling by axios. If time permits, I will share some write-up explaining our method.

Did you use any other library for that? Our own solution was to use https-proxy-agent, but it's a somewhat overly complicated and unmaintained package that isn't self-contained (so, more dependencies...). We were considering keeping the exact same pattern but changing the agent class to hpagent.

I was just hoping to have time to actually open an Axios PR.

frzsombor · 2023-04-23T19:52:07Z

I'm really not the "is there any update?" comment guy, but this looks like the one last problem with proxies in axios, so would be awesome to get this merged if everything seems ready. Sooo.. is there any update on this? :)

Thore1954 · 2023-07-15T19:48:40Z

This PR seems dead. I created #5781 which also works with HTTPS proxies.

Patrick Petrovic and others added 2 commits October 6, 2022 18:25

Add basic support for HTTPS over HTTP on Node.js

ff8f411

Merge branch 'v1.x' into https-over-http

83ee32a

ppati000 mentioned this pull request Oct 10, 2022

Proxy support does not work properly with CONNECT on https requests #4531

Open

Nevon reviewed Oct 10, 2022

View reviewed changes

greenmaid mentioned this pull request Oct 18, 2022

Axios proxy via cli options firecow/gitlab-ci-local#630

Open

Merge branch 'v1.x' into https-over-http

b09f549

jasonsaayman added the follow-up label Nov 25, 2022

github-advanced-security bot found potential problems Nov 25, 2022

View reviewed changes

test/unit/adapters/http.js

proxy.listen(4000, async () => {

try {

const httpsAgent = new https.Agent({

rejectUnauthorized: false,

Check failure

Code scanning / CodeQL

Disabling certificate validation

Disabling certificate validation is strongly discouraged.

mbargiel reviewed Jan 31, 2023

View reviewed changes

Thore1954 mentioned this pull request Jul 15, 2023

feat(http): enable ssl tunneling through proxy #5781

Closed

ppati000 closed this Aug 7, 2023

code2933 mentioned this pull request Oct 20, 2023

fix: Update command does not work with proxy tldr-pages/tldr-node-client#423

Open

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for HTTPS over HTTP on Node.js #5037

Add support for HTTPS over HTTP on Node.js #5037

ppati000 commented Oct 6, 2022

Nevon Oct 10, 2022 •

edited

ppati000 Oct 11, 2022

ppati000 commented Oct 11, 2022

mikaelfs commented Nov 11, 2022

ferranmuntada commented Nov 11, 2022

jasonsaayman commented Nov 25, 2022

ppati000 commented Dec 14, 2022

foreverest commented Jan 19, 2023

adriencarbonaro commented Jan 25, 2023

dudizi commented Jan 25, 2023

mbargiel Jan 31, 2023 •

edited

mbargiel Jan 31, 2023

mikaelfs commented Feb 1, 2023

mbargiel commented Feb 1, 2023

frzsombor commented Apr 23, 2023 •

edited

Thore1954 commented Jul 15, 2023

		}

		options._httpsOverHttp = proxy.protocol === "http:" && options.protocol === "https:";

Add support for HTTPS over HTTP on Node.js #5037

Add support for HTTPS over HTTP on Node.js #5037

Conversation

ppati000 commented Oct 6, 2022

Caveats

Nevon Oct 10, 2022 • edited

Choose a reason for hiding this comment

ppati000 Oct 11, 2022

Choose a reason for hiding this comment

ppati000 commented Oct 11, 2022

mikaelfs commented Nov 11, 2022

ferranmuntada commented Nov 11, 2022

jasonsaayman commented Nov 25, 2022

ppati000 commented Dec 14, 2022

foreverest commented Jan 19, 2023

adriencarbonaro commented Jan 25, 2023

dudizi commented Jan 25, 2023

mbargiel Jan 31, 2023 • edited

Choose a reason for hiding this comment

mbargiel Jan 31, 2023

Choose a reason for hiding this comment

mikaelfs commented Feb 1, 2023

mbargiel commented Feb 1, 2023

frzsombor commented Apr 23, 2023 • edited

Thore1954 commented Jul 15, 2023

Nevon Oct 10, 2022 •

edited

mbargiel Jan 31, 2023 •

edited

frzsombor commented Apr 23, 2023 •

edited