Honor npm configuration for CA bundles

[heikkipora]

Pass ‘ca’ and ‘cafile’ values from npm config options to request() when downloading binaries to honor a user-specified SSL CA. Allows using node-pre-gyp on environments with custom CAs.

[heikkipora]

Sigh.. retire.js is broken at the moment so "npm install" fails for node-pre-gyp. See RetireJS/retire.js#141

[heikkipora]

Retire.js also incorrectly flags cli-1.0.1 as vulnerable, so posted a PR for that as well to get your build to pass 😄 RetireJS/retire.js#143

[springmeyer]

Thank you for the contribution @heikkipora. I have a few questions:

1. This looks different than what node-gyp does (nodejs/node-gyp@8c4b0ff). I think the node-gyp impl looks more robust. Can you take a look at reimplementing this based on node-gyp's method? I think that gyp.opts.cafile will be == to opts.cafile in node-pre-gyp.

2. We'll need tests for this functionality. Can you think of a way to test this in the unit tests?

[heikkipora]

1. I originally took a look at that impl and thought that readCAFile() is practically just waste as people don't generally store random stuff in their PEM files 😄 - and reading a PEM directly without parsing their content is common practice (see e.g. https://github.com/request/request#tlsssl-protocol).

All of gyp.opts.* are currently not merged to opts.* so opts.cafile won't be available automatically. I modified my commit so that gyp.opts.ca and gyp.opts.cafile are passed to download() via opts - which seems a bit cleaner approach to accessing them from process.env

2. I cannot figure out any meaningful way to test this with a unit test. On the other hand it's not very critical nor complex functionality so it may be OK to let it live without one (writing a complicated test setup just for this doesn't have a good ROI in my opinion).

What do you think ?

[springmeyer]

Looks great @heikkipora, thank you for that revision. I notice that something is broken with appveyor and it appears to be in old tests that are no longer maintained. I just removed those in 35b10c3. Would you mind rebasing against master and then we'll see if your commit passes on both travis and appveyor okay?

[heikkipora]

The PR has now been rebased against master. However, there's still one (rather ironic) failure on the AppVeyor node 4.6.1/x86 environment where node-gyp tries to read a PEM CA file with undefined name 😃 . Do you have any clue why it does that or how to make that work?

[springmeyer]

@heikkipora - thanks for the rebase. About to head to sleep here (PST) but will plan to merge in the morning. As far as that bizarre appveyor test - no, mysterious (#209) and unrelated to your PR (though I hoped your PR might solve).

[heikkipora]

Ok!

[springmeyer]

Merged, will be included in v0.6.32. Thanks @heikkipora!