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

Doesn't seem to pass options to tls.connect #89

Closed
Janpot opened this issue Dec 17, 2019 · 14 comments
Closed

Doesn't seem to pass options to tls.connect #89

Janpot opened this issue Dec 17, 2019 · 14 comments

Comments

@Janpot
Copy link

Janpot commented Dec 17, 2019

as per documentation https://www.npmjs.com/package/https-proxy-agent#new-httpsproxyagentobject-options

  • Any other options given are passed to the net.connect()/tls.connect() functions.

But when I try this, it doesn't seem to pass ca or rejectUnauthorized when upgrading connection. I can get around it by monkey patching the agent callback:

const HttpsProxyAgent = require('https-proxy-agent');
const fetch = require('node-fetch')

const extraOpts = Symbol('extra agent opts');

class PatchedHttpsProxyAgent extends HttpsProxyAgent {
  constructor (opts) {
    super(opts);
    this[extraOpts] = opts;
  }

  callback (req, opts, fn) {
    return super.callback(req, { ...this[extraOpts], ...opts }, fn);
  }
}

async function main () {
  const res = await fetch('https://my-url', {
    agent: new PatchedHttpsProxyAgent({
      protocol: 'http:',
      host: 'my-proxy',
      rejectUnauthorized: false
    })
  });
  console.log(res.status);
}

main();

I feel like this could be solved by changing this line https://github.com/TooTallNate/node-https-proxy-agent/blob/176d4b4fb20e229cf6cd1008f06bf97833fd725f/index.js#L151

to

sock = tls.connect({ ...proxy, ...opts });

so that all input options are passed when upgrading.

What do you think?

@bjowes
Copy link

bjowes commented Feb 1, 2020

I see this problem too - need to be able to pass the rejectUnauthorized option.

@kadler15
Copy link

kadler15 commented Feb 3, 2020

See #94. Those options will be passed through by https-proxy-agent@2.2.4, but not by the current version. I'm not sure how @TooTallNate wants to support this going forward.

If you were using Node's https.request directly, it would pass your rejectUnauthorized and ca values to the agent, but it looks like node-fetch only accepts basic request args, and recommends using a custom agent for any advanced options.

For now, I'd recommend downgrading to https-proxy-agent@2.2.4 or using your monkey patch.

@arantes555
Copy link

I am having the same problem too : I am stuck on 2.2.4 because I need to pass options to tls.connect.

I would be willing to try fixing it, if you would accept a PR @TooTallNate ?

@f0zi
Copy link

f0zi commented Jul 30, 2020

Same here. Please support checkServerIdentity, thanks!

Edit: I have to add that unlike others here I can't use v2.2.4 because the agent-base version v4.3.0 that it depends on patches https.request in an incompatible way. But I can't use anything later either as I need the checkServerIdentity option.

@BlacCello
Copy link

The hotfix works also for version 5.0.0 if I remove the "fn"-Paramter of the callback. Would be great to have this fixed, since this is a common setting to have mutual tls combined with the requirement of a (corporate) proxy.

@maslakov
Copy link

maslakov commented Sep 7, 2020

Hotfix works for me as well.
Here is my variant fo axios

import { HttpsProxyAgent, HttpsProxyAgentOptions } from 'https-proxy-agent';
import { ClientRequest, RequestOptions } from 'agent-base';
import { Socket } from 'net';

export class PatchedHttpsProxyAgent extends HttpsProxyAgent {
    private ca: any;

    constructor(opts: HttpsProxyAgentOptions) {
        super(opts);
        this.ca = opts.ca;
    }

    async callback(req: ClientRequest, opts: RequestOptions): Promise<Socket> {
        return super.callback(req, Object.assign(opts, { ca: this.ca }));
    }
}

Usage:

import axios from 'axios';
import crtfile from '../cert/intern.certificat.net.crt';
import { PatchedHttpsProxyAgent } from '../modules/proxy';

const agent = new PatchedHttpsProxyAgent({ ...url.parse(PROXY), ...{ca: crtfile} });
const axiosProxyConfig = {
        proxy: false,
        httpsAgent: agent
    }
const client = axios.create(axiosProxyConfig);

@cdesch
Copy link

cdesch commented Mar 2, 2021

@maslakov did you create a pull request for this?

@dscalzi
Copy link

dscalzi commented Jun 14, 2021

Hotfix works for me as well.
Here is my variant fo axios

import { HttpsProxyAgent, HttpsProxyAgentOptions } from 'https-proxy-agent';
import { ClientRequest, RequestOptions } from 'agent-base';
import { Socket } from 'net';

export class PatchedHttpsProxyAgent extends HttpsProxyAgent {
    private ca: any;

    constructor(opts: HttpsProxyAgentOptions) {
        super(opts);
        this.ca = opts.ca;
    }

    async callback(req: ClientRequest, opts: RequestOptions): Promise<Socket> {
        return super.callback(req, Object.assign(opts, { ca: this.ca }));
    }
}

Usage:

import axios from 'axios';
import crtfile from '../cert/intern.certificat.net.crt';
import { PatchedHttpsProxyAgent } from '../modules/proxy';

const agent = new PatchedHttpsProxyAgent({ ...url.parse(PROXY), ...{ca: crtfile} });
const axiosProxyConfig = {
        proxy: false,
        httpsAgent: agent
    }
const client = axios.create(axiosProxyConfig);

This worked for me, thanks. Shame this library doesnt just support setting any of the Agent options out of the box. Extremely frustrating.

mook-as added a commit to mook-as/rd that referenced this issue Jul 28, 2021
This adds support for use the system proxy configuration (by asking the
embedded Chrome to resolve the proxy configuration).  The idea to ask
Chrome was from the electron-proxy-agent package; however, it had
significant issues on supporting system CA certificates, and the result
ended up being a complete rewrite.

We need the wrapper classes for HttpsProxyAgent and SocksProxyAgent so that
we can pass the CA options down to the eventual tls.connect() call.  This
is due to TooTallNate/proxy-agents#89

Signed-off-by: Mark Yen <mark.yen@suse.com>
mook-as added a commit to mook-as/rd that referenced this issue Jul 28, 2021
This adds support for use the system proxy configuration (by asking the
embedded Chrome to resolve the proxy configuration).  The idea to ask
Chrome was from the electron-proxy-agent package; however, it had
significant issues on supporting system CA certificates, and the result
ended up being a complete rewrite.

We need the wrapper classes for HttpsProxyAgent and SocksProxyAgent so that
we can pass the CA options down to the eventual tls.connect() call.  This
is due to TooTallNate/proxy-agents#89

Signed-off-by: Mark Yen <mark.yen@suse.com>
mook-as added a commit to mook-as/rd that referenced this issue Aug 6, 2021
This adds support for use the system proxy configuration (by asking the
embedded Chrome to resolve the proxy configuration).  The idea to ask
Chrome was from the electron-proxy-agent package; however, it had
significant issues on supporting system CA certificates, and the result
ended up being a complete rewrite.

We need the wrapper classes for HttpsProxyAgent and SocksProxyAgent so that
we can pass the CA options down to the eventual tls.connect() call.  This
is due to TooTallNate/proxy-agents#89

Signed-off-by: Mark Yen <mark.yen@suse.com>
@GokulDharumar
Copy link

I am trying to use the hotfix with the callback. node-fetch and pfx file. but the hotfix is not working... can some one please help me out?

@tex0l
Copy link

tex0l commented Nov 10, 2022

I found out that it doesn't work anymore since v5 because HttpsProxyAgent is actually not a proper class, it is a function that returns an instance of the class, therefore it cannot be extended anymore through ES6 the extends keyword. See https://github.com/TooTallNate/node-https-proxy-agent/blob/d0d80cc0482f20495aa8595f802e1a9f3b1b3409/src/index.ts#L8

The workaround I found is to import the class directly from 'https-proxy-agent/dist/agent'.

The following workaround works on my end with v5.0.1:

import url from 'url'
import HttpsProxyAgent from 'https-proxy-agent/dist/agent'
const extraOpts = Symbol('extra agent opts')

export class PatchedHttpsProxyAgent extends HttpsProxyAgent {
  constructor (opts) {
    super(opts)

    this[extraOpts] = opts
  }

  callback (req, opts) {
    return super.callback(req, { ...this[extraOpts], ...opts })
  }
}

const proxyOptions = url.parse('localhost:9090')
const agent = new PatchedHttpsProxyAgent({ checkServerIdentity: (host, cert) => { /* my tls pinning */ }, ...proxyOptions })

@dscalzi
Copy link

dscalzi commented Nov 14, 2022

@tex0l I would recommend using https://github.com/delvedor/hpagent over relying on a non reliable hack with this one.

@TooTallNate
Copy link
Owner

This module has gone through a large refactor and modernization. I am closing this issue as a bit of house cleaning. If you feel that this issue still exists in the latest release, feel free to open a new issue.

@shrkbait-hpe
Copy link

Anyone trying @maslakov 's solution in v6+ (6.1.0 as of this writing), you need to rename callback to connect.

  async connect (req, opts) {
    return super.connect(req, Object.assign(opts, { ca: this.ca }))
  }

@TooTallNate , my use case for this is using Postman's built in proxy feature https://learning.postman.com/docs/getting-started/proxy/. It uses a self-signed cert and I need to include the contents of ~/Library/Application\ Support/Postman/proxy/postman-proxy-ca.crt as the value of 'ca'.

@iAlex97
Copy link

iAlex97 commented Oct 14, 2023

The constructor changed a little bit with leter versions (7.0.2 as of writing) and this issue still exists. Building upon the great answers before, here's an updated version:

import { HttpsProxyAgent, HttpsProxyAgentOptions } from 'https-proxy-agent';

type PatchedHttpsProxyAgentOptions<T> = HttpsProxyAgentOptions<T> & {
  ca: string;
};

export class PatchedHttpsProxyAgent<Uri extends string> extends HttpsProxyAgent<Uri> {
  private readonly ca: any;

  constructor(proxy: Uri | URL, opts?: PatchedHttpsProxyAgentOptions<Uri>) {
    super(proxy, opts);
    this.ca = opts.ca;
  }

  async connect(req, opts) {
    return super.connect(req, Object.assign(opts, { ca: this.ca }));
  }
}

Note: PatchedHttpsProxyAgentOptions might not be needed, but since I use both https-proxy-agent and http-proxy-agent there seems to be typing conflict, since both libraries depend on agent-base

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

No branches or pull requests