nodejs: use system ca certificate store #257513
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
SuperSandro2000
approved these changes
Sep 27, 2023
JulienMalka
approved these changes
Sep 27, 2023
delroth
added
the
12.approvals: 2
hexchen
approved these changes
Sep 27, 2023
hexchen
added
12.approvals: 3+
|
This could theoretically go straight to master |
|
The contributing guidelines currently give 500+ rebuilds as the rule of thumb threshold for mass rebuilds that should go to staging, so I based this PR on staging. I could rebase onto master if that's okay despite being a mass rebuild? |
|
It's only a mass rebuild if labels are red but it really does not matter and someone should just hit merge. |
vifino
approved these changes
Sep 27, 2023
marsam
approved these changes
Sep 28, 2023
This was referenced Nov 20, 2023
This was referenced Nov 21, 2023
13 tasks
2 tasks
phaer
added a commit
to phaer/nixpkgs
that referenced
this pull request
Dec 15, 2023
We are currently using an impure method to build a derivation for playwrights browsers on darwin: We just call `playwright install` during build-time. But this recently began to fail, with an error that the azure certificate would be self-signed, while it was not. This is presumably due to NixOS#257513 where we started to use the systems CA store for nodejs instead of the bundled one, so we add cacert to fix it. Maybe the whole impure method of downloading browsers on darwin should be reconsidered in favor of following the same approach as on Linux, but that would probably have the drawback that we'd loose safari as one of the browsers.
13 tasks
github-actions bot
pushed a commit
that referenced
this pull request
Dec 18, 2023
We are currently using an impure method to build a derivation for playwrights browsers on darwin: We just call `playwright install` during build-time. But this recently began to fail, with an error that the azure certificate would be self-signed, while it was not. This is presumably due to #257513 where we started to use the systems CA store for nodejs instead of the bundled one, so we add cacert to fix it. Maybe the whole impure method of downloading browsers on darwin should be reconsidered in favor of following the same approach as on Linux, but that would probably have the drawback that we'd loose safari as one of the browsers. (cherry picked from commit bb74ebb)
Lainera
pushed a commit
to Lainera/nixpkgs
that referenced
this pull request
Dec 20, 2023
We are currently using an impure method to build a derivation for playwrights browsers on darwin: We just call `playwright install` during build-time. But this recently began to fail, with an error that the azure certificate would be self-signed, while it was not. This is presumably due to NixOS#257513 where we started to use the systems CA store for nodejs instead of the bundled one, so we add cacert to fix it. Maybe the whole impure method of downloading browsers on darwin should be reconsidered in favor of following the same approach as on Linux, but that would probably have the drawback that we'd loose safari as one of the browsers.
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description of changes
NodeJS has a hardcoded list of trusted certificate authorities built-in which it uses instead of the system CA certificates. (see nodejs/node#4175). Additional CA certificates can be added using the
NODE_EXTRA_CA_CERTSenvironment variable, but there's no way to 1) allow for certificate revocations and 2) add trusted CAs (for example a self-signed CA for custom infrastructure) to all nodejs processes systemwide.To ignore the built-in list of CAs and always use only the system default CAs, nodejs has to be compiled with the
--openssl-use-def-ca-storeflag which this PR adds. One example of other distros using this is Debian.I've tested this change by running this snippet in the nodejs repl:
const https=require('node:https'); https.get('https://local.url.example.com')where the url in question is served with a self-signed certificate added insecurity.pki.certificateFiles. This throws the error messageUncaught Error: self-signed certificate in certificate chainwhen run with the currentnodejspackage but works with this configuration set.Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)