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

NODE_EXTRA_CA_CERTS cannot be set in code or relative #20432

Closed
sheerun opened this issue Apr 30, 2018 · 13 comments
Closed

NODE_EXTRA_CA_CERTS cannot be set in code or relative #20432

sheerun opened this issue Apr 30, 2018 · 13 comments
Labels
doc Issues and PRs related to the documentations. feature request Issues that request new features to be added to Node.js.

Comments

@sheerun
Copy link

sheerun commented Apr 30, 2018

  • Version:
    v9.11.1
  • Platform:
    Darwin sheerun.dev 17.4.0 Darwin Kernel Version 17.4.0: Sun Dec 17 09:19:54 PST 2017; root:xnu-4570.41.2~1/RELEASE_X86_64 x86_64 i386 MacBookPro12,1 Darwin

When I start my server with

NODE_EXTRA_CA_CERTS=/Users/sheerun/Source/Ada/search/.certs/ca.crt bin/start

and in code make request to http server that serves with given certificate, all is good

But when I set it at the beginning of bin/start as so:

process.env.NODE_EXTRA_CA_CERTS = "/Users/sheerun/Source/Ada/search/.certs/ca.crt"

then node complains that there's "self signed certificate in certificate chain".

Particularly NODE_EXTRA_CA_CERTS doesn't work when I use dotenv package and set NODE_EXTRA_CA_CERTS in .env file.

Also, it seems NODE_EXTRA_CA_CERTS doesn't allow relative path, just absolute.

I think both of these issues should be addressed or at least documented with reasons why.

@bnoordhuis bnoordhuis added the doc Issues and PRs related to the documentations. label Apr 30, 2018
@bnoordhuis
Copy link
Member

Pull request welcome.

process.env.NODE_EXTRA_CA_CERTS

Not reliable because the extra certificates are loaded only once.

it seems NODE_EXTRA_CA_CERTS doesn't allow relative path, just absolute.

Relative paths should work.

@sheerun
Copy link
Author

sheerun commented Apr 30, 2018

Not reliable because the extra certificates are loaded only once.

It would be nice if they were loaded only once but lazily, not before any code is executed in node. This way packages like dotenv would work because the recommendation is to run them at the very beginning. Possibly it could also speedup startup of node a bit.

@sheerun
Copy link
Author

sheerun commented Apr 30, 2018

Also in #20434 you say that it is loaded lazily but it doesn't seem so unless node makes http requests I don't know about before first line of code is executed. Probably it is instantiated when some not yet started http server is instantiated, but I think it would be better to load certificates when it's actually needed (server starts, http request, tls function is executed).

Ideally you'd expose a function for reloading these..

Also my use case: setting up development environment with https self-signed certificate so Facebook integrations are working properly locally.

@sheerun
Copy link
Author

sheerun commented Apr 30, 2018

Or maybe a function .addExtraCaCert() plus .freezeCaCerts() for security, just throwing ideas.

@bnoordhuis
Copy link
Member

You should chime in on #20434 but keep in mind that NODE_EXTRA_CA_CERTS is to let system administrators add extra certificates without touching code. It's explicitly not Yet Another API for adding certificates programmatically.

@bnoordhuis
Copy link
Member

Or maybe a function .addExtraCaCert() plus .freezeCaCerts() for security, just throwing ideas.

That might be acceptable. Can you file a new issue?

oyyd added a commit to oyyd/node that referenced this issue Oct 19, 2018
`NODE_EXTRA_CA_CERTS` is not intended to be used to set the paths
of extra certificates and this approach to setting is not reliable.
This commit makes node load extra certificates at startup instead
of first use.

Fixes: nodejs#20434
Refs: nodejs#20432
refack pushed a commit to oyyd/node that referenced this issue Oct 20, 2018
This commit makes node load extra certificates at startup instead
of first use.

PR-URL: nodejs#23354
Fixes: nodejs#20434
Refs: nodejs#20432
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@cinderblock
Copy link

This seems to be possible. However I have not found any documentation of this.

One other mention of this method: #4464 (comment)

Example:

const tls = require('tls');
const secureContext = tls.createSecureContext();
// https://letsencrypt.org/certs/lets-encrypt-x3-cross-signed.pem.txt
secureContext.context.addCACert(`-----BEGIN CERTIFICATE-----
MIIEkjCCA3qgAwIBAgIQCgFBQgAAAVOFc2oLheynCDANBgkqhkiG9w0BAQsFADA/
MSQwIgYDVQQKExtEaWdpdGFsIFNpZ25hdHVyZSBUcnVzdCBDby4xFzAVBgNVBAMT
DkRTVCBSb290IENBIFgzMB4XDTE2MDMxNzE2NDA0NloXDTIxMDMxNzE2NDA0Nlow
SjELMAkGA1UEBhMCVVMxFjAUBgNVBAoTDUxldCdzIEVuY3J5cHQxIzAhBgNVBAMT
GkxldCdzIEVuY3J5cHQgQXV0aG9yaXR5IFgzMIIBIjANBgkqhkiG9w0BAQEFAAOC
AQ8AMIIBCgKCAQEAnNMM8FrlLke3cl03g7NoYzDq1zUmGSXhvb418XCSL7e4S0EF
q6meNQhY7LEqxGiHC6PjdeTm86dicbp5gWAf15Gan/PQeGdxyGkOlZHP/uaZ6WA8
SMx+yk13EiSdRxta67nsHjcAHJyse6cF6s5K671B5TaYucv9bTyWaN8jKkKQDIZ0
Z8h/pZq4UmEUEz9l6YKHy9v6Dlb2honzhT+Xhq+w3Brvaw2VFn3EK6BlspkENnWA
a6xK8xuQSXgvopZPKiAlKQTGdMDQMc2PMTiVFrqoM7hD8bEfwzB/onkxEz0tNvjj
/PIzark5McWvxI0NHWQWM6r6hCm21AvA2H3DkwIDAQABo4IBfTCCAXkwEgYDVR0T
AQH/BAgwBgEB/wIBADAOBgNVHQ8BAf8EBAMCAYYwfwYIKwYBBQUHAQEEczBxMDIG
CCsGAQUFBzABhiZodHRwOi8vaXNyZy50cnVzdGlkLm9jc3AuaWRlbnRydXN0LmNv
bTA7BggrBgEFBQcwAoYvaHR0cDovL2FwcHMuaWRlbnRydXN0LmNvbS9yb290cy9k
c3Ryb290Y2F4My5wN2MwHwYDVR0jBBgwFoAUxKexpHsscfrb4UuQdf/EFWCFiRAw
VAYDVR0gBE0wSzAIBgZngQwBAgEwPwYLKwYBBAGC3xMBAQEwMDAuBggrBgEFBQcC
ARYiaHR0cDovL2Nwcy5yb290LXgxLmxldHNlbmNyeXB0Lm9yZzA8BgNVHR8ENTAz
MDGgL6AthitodHRwOi8vY3JsLmlkZW50cnVzdC5jb20vRFNUUk9PVENBWDNDUkwu
Y3JsMB0GA1UdDgQWBBSoSmpjBH3duubRObemRWXv86jsoTANBgkqhkiG9w0BAQsF
AAOCAQEA3TPXEfNjWDjdGBX7CVW+dla5cEilaUcne8IkCJLxWh9KEik3JHRRHGJo
uM2VcGfl96S8TihRzZvoroed6ti6WqEBmtzw3Wodatg+VyOeph4EYpr/1wXKtx8/
wApIvJSwtmVi4MFU5aMqrSDE6ea73Mj2tcMyo5jMd6jmeWUHK8so/joWUoHOUgwu
X4Po1QYz+3dszkDqMp4fklxBwXRsW10KXzPMTZ+sOPAveyxindmjkW8lGy+QsRlG
PfZ+G6Z6h7mjem0Y+iWlkYcV4PIWL1iwBi8saCbGS5jN2p8M+X+Q7UNKEkROb3N6
KOqkqm57TH2H3eDJAkSnh6/DNFu0Qg==
-----END CERTIFICATE-----`);

const sock = tls.connect(443, 'host', {secureContext});

@sam-github
Copy link
Contributor

Sorry, @cinderblock I don't understand your comment, or how it relates to NODE_EXTRA_CA_CERTS. Could you repost this question in http://github.com/nodejs/help, or as an independent issue if your believe there is a bug?

@cinderblock
Copy link

@sam-github You are correct that the titular question asks specifically about the NODE_EXTRA_CA_CERTS environment variable and setting that programmatically during runtime.

I however suspect the main thing in question is to set extra ca certificates in code/during runtime that don't override the built-in CA certificates. The method I've suggested does exactly what I believe the OP wanted without the method that the OP assumed was the correct way of solving their problem.

@sheerun
Copy link
Author

sheerun commented Nov 26, 2018

Reiterating my use case is:

setting up development environment with https self-signed certificate so Facebook integrations are working properly locally.

A programmatic way of doing so is better than setting ENV variable, so it would be OK for my use case. I'd rather use official, documented way of doing so though, so @cinderblock has a point that it should be documented.

@cinderblock
Copy link

@sheerun Yeah, I'm rather surprised that this API is not documented.

On the node.js tls documentation page, it simply says:

The tls.createSecureContext() method creates a credentials object.

But what is a "credentials object"? I have not found any documentation on it.

It looks to be a real API for a number of reasons:

@sam-github sam-github added the feature request Issues that request new features to be added to Node.js. label Mar 25, 2019
@sam-github
Copy link
Contributor

A number of independent issues are discussed here:

  1. runtime setting of env variable, see discussion in Consistent environment variables #26213
  2. relative path, it should be supported, please post reproduction if you find it to not be
  3. confusion about what a credentials object is, see doc: remove reference to "credentials object" #26908

sam-github added a commit to sam-github/node that referenced this issue Mar 25, 2019
The reference is confusing because the object is actually of class
SecureContext. There is no object with class "credentials".

See: nodejs#20432 (comment)
sam-github added a commit that referenced this issue Mar 28, 2019
The reference is confusing because the object is actually of class
SecureContext. There is no object with class "credentials".

See: #20432 (comment)

PR-URL: #26908
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos pushed a commit that referenced this issue Mar 28, 2019
The reference is confusing because the object is actually of class
SecureContext. There is no object with class "credentials".

See: #20432 (comment)

PR-URL: #26908
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos pushed a commit that referenced this issue Mar 29, 2019
The reference is confusing because the object is actually of class
SecureContext. There is no object with class "credentials".

See: #20432 (comment)

PR-URL: #26908
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
targos pushed a commit that referenced this issue Mar 30, 2019
The reference is confusing because the object is actually of class
SecureContext. There is no object with class "credentials".

See: #20432 (comment)

PR-URL: #26908
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
BethGriggs pushed a commit that referenced this issue Apr 5, 2019
The reference is confusing because the object is actually of class
SecureContext. There is no object with class "credentials".

See: #20432 (comment)

PR-URL: #26908
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
ebickle added a commit to ebickle/node that referenced this issue Mar 5, 2020
Modifies the tls.rootCertificates property so that it is settable. PEM-formatted certificates set to the property are loaded into a new Node.js root certificate store that is used for all subsequent TLS requests and peer certificate validation.

Allows full programmatic control of a Node.js process' root certificate store.

Fixes: nodejs#20432
Refs: nodejs#27079
ebickle added a commit to ebickle/node that referenced this issue Mar 17, 2020
Modifies the tls.rootCertificates property so that it is settable. PEM-formatted certificates set to the property are loaded into a new Node.js root certificate store that is used for all subsequent TLS requests and peer certificate validation.

Allows full programmatic control of a Node.js process' root certificate store.

Fixes: nodejs#20432
Refs: nodejs#27079

Changed root_cert_store to a smart pointer

Added support for empty root certificates

Added comment
@nickwesselman
Copy link

This is a real bummer. Just implemented dotenv assuming that it would allow us to populate NODE_EXTRA_CA_CERTS and this ended up being decidedly not the case.

jasnell added a commit to jasnell/node that referenced this issue Mar 1, 2021
@jasnell jasnell closed this as completed in 9c1274a Mar 5, 2021
danielleadams pushed a commit that referenced this issue Mar 16, 2021
Fixes: #20432

PR-URL: #37562
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue May 1, 2021
Fixes: #20432

PR-URL: #37562
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
ebickle added a commit to ebickle/node that referenced this issue Sep 5, 2022
Store loaded NODE_EXTRA_CA_CERTS into root_certs_vector, allowing
them to be added to secure contexts when NewRootCertStore() is
called.

When NODE_EXTRA_CA_CERTS is specified, the root certificates
(both bundled and extra) will no longer be preloaded at startup.
This improves Node.js startup time and makes the behavior of
NODE_EXTRA_CA_CERTS consistent with the default behavior when
NODE_EXTRA_CA_CERTS is omitted.

The original reason NODE_EXTRA_CA_CERTS were loaded at startup
(issues nodejs#20432, nodejs#20434) was to prevent the environment variable from
being changed at runtime. This change preserves the runtime consistency
without actually having to load the certs at startup.

Fixes: nodejs#32010
Refs: nodejs#40524
Refs: nodejs#23354
ebickle added a commit to ebickle/node that referenced this issue Sep 6, 2022
Store loaded NODE_EXTRA_CA_CERTS into root_certs_vector, allowing
them to be added to secure contexts when NewRootCertStore() is
called.

When NODE_EXTRA_CA_CERTS is specified, the root certificates
(both bundled and extra) will no longer be preloaded at startup.
This improves Node.js startup time and makes the behavior of
NODE_EXTRA_CA_CERTS consistent with the default behavior when
NODE_EXTRA_CA_CERTS is omitted.

The original reason NODE_EXTRA_CA_CERTS were loaded at startup
(issues nodejs#20432, nodejs#20434) was to prevent the environment variable from
being changed at runtime. This change preserves the runtime consistency
without actually having to load the certs at startup.

Fixes: nodejs#32010
Refs: nodejs#40524
Refs: nodejs#23354
ebickle added a commit to ebickle/node that referenced this issue Jul 26, 2024
Store loaded NODE_EXTRA_CA_CERTS into root_certs_vector, allowing
them to be added to secure contexts when NewRootCertStore() is
called.

When NODE_EXTRA_CA_CERTS is specified, the root certificates
(both bundled and extra) will no longer be preloaded at startup.
This improves Node.js startup time and makes the behavior of
NODE_EXTRA_CA_CERTS consistent with the default behavior when
NODE_EXTRA_CA_CERTS is omitted.

The original reason NODE_EXTRA_CA_CERTS were loaded at startup
(issues nodejs#20432, nodejs#20434) was to prevent the environment variable from
being changed at runtime. This change preserves the runtime consistency
without actually having to load the certs at startup.

Fixes: nodejs#32010
Refs: nodejs#40524
Refs: nodejs#23354
pimterry pushed a commit that referenced this issue Jul 30, 2024
Store loaded NODE_EXTRA_CA_CERTS into root_certs_vector, allowing
them to be added to secure contexts when NewRootCertStore() is
called, rather than losing them when unrelated options are provided.

When NODE_EXTRA_CA_CERTS is specified, the root certificates
(both bundled and extra) will no longer be preloaded at startup.
This improves Node.js startup time and makes the behavior of
NODE_EXTRA_CA_CERTS consistent with the default behavior when
NODE_EXTRA_CA_CERTS is omitted.

The original reason NODE_EXTRA_CA_CERTS were loaded at startup
(issues #20432, #20434) was to prevent the environment variable from
being changed at runtime. This change preserves the runtime consistency
without actually having to load the certs at startup.

Fixes: #32010
Refs: #40524
Refs: #23354
PR-URL: #44529
Reviewed-By: Tim Perry <pimterry@gmail.com>
targos pushed a commit that referenced this issue Aug 14, 2024
Store loaded NODE_EXTRA_CA_CERTS into root_certs_vector, allowing
them to be added to secure contexts when NewRootCertStore() is
called, rather than losing them when unrelated options are provided.

When NODE_EXTRA_CA_CERTS is specified, the root certificates
(both bundled and extra) will no longer be preloaded at startup.
This improves Node.js startup time and makes the behavior of
NODE_EXTRA_CA_CERTS consistent with the default behavior when
NODE_EXTRA_CA_CERTS is omitted.

The original reason NODE_EXTRA_CA_CERTS were loaded at startup
(issues #20432, #20434) was to prevent the environment variable from
being changed at runtime. This change preserves the runtime consistency
without actually having to load the certs at startup.

Fixes: #32010
Refs: #40524
Refs: #23354
PR-URL: #44529
Reviewed-By: Tim Perry <pimterry@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. feature request Issues that request new features to be added to Node.js.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants