This repository has been archived by the owner on May 10, 2019. It is now read-only.
lib/primary.js: Pass https.globalAgent.options.ca to https.get #4059
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Otherwise
$ ./scripts/check_primary_support my.net /path/to/my/ca.pem
fails with:
error: my.net is not a browserid primary: Error: SELF_SIGNED_CERT_IN_CHAIN
The user-specified certificate authority path was added in #3185
(3848755, Fix check_primary_support to read responses, 2013-04-03), but
because agent is set to false in the https.get call, the CA I'd specified was
not being used.
With this commit, I pass the globalAgent's ca through to https.get
1, which wraps https.request 2, which accepts the ca option (among others)
from tls.connect 3. Once it has the ca option, https.get verifies my
self-signed CA just fine (with Node v0.10.21).
I don't understand the httpProxy bit deciding between http.get and https.get,
or how key-verification works in that case. It's possible that this patch
needs to be adjusted to handle that case, but http.get
4, which wraps http.request 5 does not support the TLS options
(not much of a surprise ;).
I also don't understand why we set agent to false in the https.get call.
0e39dc6 (Explicitly set rejectUnauthorized to false when using
https.request(), 2013-08-08 [6]) mentions Node 0.8 needing both
"rejectUnauthorized: true" and "agent: false" to reject invalid certificated.
Maybe we're far enough past Node 0.8 that we can remove the "agent: false"
part of this pair? The "agent: false" bit dates back to the initial
primary.js addition in cc186ea (an abstraction around primary support checking
and a command line script to test primary support, 2011-12-19), so I'm not
clear on exactly what it's accomplishing.
There are also a bunch of other https.get calls in the codebase. I'm not far
enough into my local Persona install to have a fully successful
check_primary_support, so it's possible that some of the remaining calls also
need the globalAgent CA data to get through check_primary_support with a
self-signed CA.
[6]: It really does say "set rejectUnauthorized to false" in the
commit summary. Oops :p. The rest of the commit correctly sets
rejectUnauthorized to true.