Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check the status code from the CONNECT response when the proxy is HTTP and the target is HTTPS. #61

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

BogdanCln
Copy link

Issue

When the target is an HTTPS endpoint and the proxy server uses HTTP, global-agent does not parse the response coming from the proxy server on the CONNECT method.

As a result, it causes a confusing low-level exception (write EPROTO 281473125421184:error:1408F10B:SSL routines:ssl3_get_record:wrong version number) whenever the proxy server responds with an error status code (for example 403 if proxy has access control and denies the request, or 407 when the credentials are wrong).

It is easy to replicate this issue with a proxy server that has authentication or access filters. Unfortunately, anyproxy, the module used in the automated tests doesn't provide any of these out of the box, letting the users implement them via 'Rule module interface'.

Therefore, to replicate the issue locally, I used a simple squid docker container and a script based on global-agent:

Replicating the issue

I. Set up a proxy server with squid:

  1. Create a new empty folder and navigate there (I suggest an empty folder since it will be mounted to the docker container)

For example, I used:

mkdir -p ~/dev/global-proxy-issue-test && cd ~/dev/global-proxy-issue-test

  1. Prepare a squid config that enables basic authentication with username user and password password:

    2.1. Create the hashed basic authentication file:

    echo "user:\$apr1\$rCCYyoxE\$aZmgzc9iF/uQgPl9Lx1291" >> squid_password

    2.2. Create the squid.conf configuration file:

    echo "auth_param basic program /usr/lib/squid/basic_ncsa_auth /etc/squid/squid_password
    auth_param basic realm Proxy Basic Authentication
    acl usersquid proxy_auth REQUIRED
    http_access allow usersquid
    http_access deny all
    http_port 9998" >> squid.conf
    
  2. Run the squid docker container:

docker run -d -p 9998:9998 -v $(pwd):/etc/squid ubuntu/squid

The proxy server is now running at http://user:password@127.0.0.1:9998/

II. Set up a simple test file with global-agent

npm i global-agent

touch script.js

Paste the following script into script.js (global-agent is the only external dependency, but testing with got or request-promise will have the same result, regardless of Node.js version):

(notice that the correct proxy credentials are user:password, but first we will try with wrong credentials in order to receive 407 from proxy server)

const { bootstrap } = require('global-agent');

const https = require('https');

async function task() {
    process.env.GLOBAL_AGENT_ENVIRONMENT_VARIABLE_NAMESPACE = '';
    process.env.HTTP_PROXY = 'http://WRONGUSER:WRONGPASSWORD@127.0.0.1:9998';
    process.env.HTTPS_PROXY = 'http://WRONGUSER:WRONGPASSWORD@127.0.0.1:9998';
    process.env.NO_PROXY = '';

    bootstrap();

    try {
        await doRequest();
    } catch (e) {
        console.error(e);
    }
}

async function doRequest() {
    const options = {
        host: 'httpbin.org',
        path: '/status/200'
    };

    return new Promise((resolve, reject) => {
        const callback = function (res) {
            let str = '';

            res.on('data', function (chunk) {
                str += chunk;
            });

            res.on('end', function () {
                console.log(str);
                resolve(str);
            });
        };

        const req = https.request(options, callback).end();
        req.on('error', reject);
    });
}

void task();

III. Run the script and observe initial output:

$ node script.js
Error: write EPROTO 8669745856:error:1408F10B:SSL routines:ssl3_get_record:wrong version number:../deps/openssl/openssl/ssl/record/ssl3_record.c:332:

    at WriteWrap.onWriteComplete [as oncomplete] (internal/stream_base_commons.js:94:16) {
  errno: -100,
  code: 'EPROTO',
  syscall: 'write'
}

The error is caused by the fact that the socket used for the CONNECT method is upgraded to TLS even if the response from the proxy server is not 200. In this case, the squid proxy server responds with HTTP/1.1 407 Proxy Authentication Required to the CONNECT request.

This can be inspected with:

curl -v -x "http://WRONGUSER:WRONGPASSWORD@127.0.0.1:9998" http://httpbin.org/headers

IV. Modify the script with the correct credentials and the request will now work:

const { bootstrap } = require('global-agent');

const https = require('https');

async function task() {
    process.env.GLOBAL_AGENT_ENVIRONMENT_VARIABLE_NAMESPACE = '';
    process.env.HTTP_PROXY = 'http://user:password@127.0.0.1:9998';
    process.env.HTTPS_PROXY = 'http://user:password@127.0.0.1:9998';
    process.env.NO_PROXY = '';

    bootstrap();

    try {
        await doRequest();
    } catch (e) {
        console.error(e);
    }
}

async function doRequest() {
    const options = {
        host: 'httpbin.org',
        path: '/status/200'
    };

    return new Promise((resolve, reject) => {
        const callback = function (res) {
            let str = '';

            res.on('data', function (chunk) {
                str += chunk;
            });

            res.on('end', function () {
                console.log(str);
                resolve(str);
            });
        };

        const req = https.request(options, callback).end();
        req.on('error', reject);
    });
}

void task();

Proposed change

My proposed change is to check the status code from the CONNECT response when the proxy is HTTP and the target is HTTPS. If the status code is >= 400, emit an error with the status line and don't go any further.

Since proxies with HTTPS as protocal are not allowed by parseProxyUrl(),it should be safe to assume that the response is plain text and parse it.

Bogdan Calina added 2 commits January 8, 2023 23:52
@NPhMKgbDNy1M
Copy link

I have the same problem!

utc8 pushed a commit to utc8/global-agent that referenced this pull request May 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants