treewide: support NIX_SSL_CERT_FILE as an impureEnvVar#271161
Closed
timbertson wants to merge 2 commits intoNixOS:stagingfrom
Closed
treewide: support NIX_SSL_CERT_FILE as an impureEnvVar#271161timbertson wants to merge 2 commits intoNixOS:stagingfrom
timbertson wants to merge 2 commits intoNixOS:stagingfrom
Conversation
13 tasks
cfb4e0e to
bb84dbb
Compare
Contributor
|
I thought we should use the cert in fetchurl instead of using --insecure. See my PR #259179. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Contributor
Author
roberth
requested changes
Feb 8, 2024
Member
roberth
left a comment
There was a problem hiding this comment.
This looks quite sensible to me, but could you add some (very) basic documentation? We don't have any documentation for proxy support in the Nixpkgs manual yet.
bb84dbb to
6c15a86
Compare
Contributor
Author
|
Good idea, I've added a "Proxy usage" section in the nixpkgs fetchers chapter. |
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
This envvar is also added to lib.proxyImpureEnvVars since it's typically required for https proxies. This change also updates fetchgit and go module fetching to use this envvar. NIX_GIT_SSL_CAINFO is still supported for backwards compatibility in fetchgit.
Contributor
Author
|
Oh dear, I've pushed the wrong branch and it wants the world to review it. I'll open a new PR to avoid notification chaos 🤦 |
13 tasks
This file contains hidden or 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
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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
tl;dr this PR makes
NIX_SSL_CERT_FILEthe preferred way to control CA certificates throughout nix, by allowing it to be set as in impureEnvVar, including it in proxyImpureEnvVars, and ensuring fetchers support it.Nix (the executable) as well as much of its packaged software respects
NIX_SSL_CERT_FILEas the preferred way to specify a custom set of root certificates. Within derivations, this is typically done by the setup hook ofcacert.However, there's no way to inject this variable from outside (with impure env vars used in fetchers). This is required for most use cases of an an https proxy, as the proxy's own certificate will not likely be trusted by nix's builtin cacert package.
Functionality for injecting a custom trust store has been added to
fetchgit, but because the setuphook will always overrideNIX_SSL_CERT_FILE, a different envvar had to be used (NIX_GIT_SSL_CAINFO). I have an open PR to add this same customisation to the go module fetcher.However, it'd be preferable if we didn't invent a new envvar, and instead made
NIX_SSL_CERT_FILEthe single way to control this setting for both fetchers and nix itself.For this to work, we need two small changes:
cacertshould only set this variable if it's not already set, so that any impure version is not overwritten"NIX_SSL_CERT_FILE"toproxyImpureEnvVars(fetcher.nix). It's not strictly a proxy-only variable, but:Various fetchers current (and updated) behaviour is oulined below:
NIX_GIT_SSL_CAINFOas the impure overrideable version. This is still supported, but the standardNIX_SSL_CERT_FILEnow works and is preferred.--insecureto curl for a fixed-output derivation, so I guess it doesn't need modificationGIT_SSL_CAINFOto the value ofNIX_SSL_CERT_FILEusing whatever's provided by the user / setup hookLooking through other fetchers, it seems most of them either have their own schemes or delegate to fetchurl. So this isn't a wide-reaching code change, but I think it's important to support
NIX_SSL_CERT_FILEas an impureEnvVar, so that we don't force fetchers to invent their own envvars for this purpose.Things done
Since this is a mass-rebuild, I tested locally by only applying these changes to the version of
cacertused by fetchers. With these modifications I tested fetching over https (using custom certs) with:nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)