Skip to content

treewide: support NIX_SSL_CERT_FILE as an impureEnvVar#271161

Closed
timbertson wants to merge 2 commits intoNixOS:stagingfrom
timbertson:https_proxy
Closed

treewide: support NIX_SSL_CERT_FILE as an impureEnvVar#271161
timbertson wants to merge 2 commits intoNixOS:stagingfrom
timbertson:https_proxy

Conversation

@timbertson
Copy link
Contributor

@timbertson timbertson commented Nov 30, 2023

Description of changes

tl;dr this PR makes NIX_SSL_CERT_FILE the 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_FILE as the preferred way to specify a custom set of root certificates. Within derivations, this is typically done by the setup hook of cacert.

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 override NIX_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_FILE the single way to control this setting for both fetchers and nix itself.

For this to work, we need two small changes:

  • cacert should only set this variable if it's not already set, so that any impure version is not overwritten
    • this is in a setuphook so it's a mass rebuild
  • add "NIX_SSL_CERT_FILE" to proxyImpureEnvVars (fetcher.nix). It's not strictly a proxy-only variable, but:
    • this needs to be overridden for all HTTPS proxy use cases, and most things are https these days. Currently fetcher support for custom certs is patchy, because they're all doing their own thing (see below notes)
    • using the certificates the system has explicitly set for nix (if any) is inline with user expectations even outside proxy setups, and this only affects fixed-output derivations anyway

Various fetchers current (and updated) behaviour is oulined below:

  • fetchgit: previously added explicit support for NIX_GIT_SSL_CAINFO as the impure overrideable version. This is still supported, but the standard NIX_SSL_CERT_FILE now works and is preferred.
  • fetchurl: passes --insecure to curl for a fixed-output derivation, so I guess it doesn't need modification
  • fetchgomodule: previously no support for custom certs for git dependencies, now sets GIT_SSL_CAINFO to the value of NIX_SSL_CERT_FILE using whatever's provided by the user / setup hook

Looking 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_FILE as 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 cacert used by fetchers. With these modifications I tested fetching over https (using custom certs) with:

  • fetchurl
  • fetchgit (over https)
  • go modules fetch (git https)
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@github-actions github-actions bot added 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 6.topic: lib The Nixpkgs function library labels Nov 30, 2023
@linsui
Copy link
Contributor

linsui commented Nov 30, 2023

I thought we should use the cert in fetchurl instead of using --insecure. See my PR #259179.

@ofborg ofborg bot added 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches. labels Nov 30, 2023
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/3252

@timbertson
Copy link
Contributor Author

Pinging a couple folks who've been involved in this area previously, I'd appreciate any reviews you have time for 🙏
@vcunat @roberth

Copy link
Member

@roberth roberth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can be done now.

@github-actions github-actions bot added the 8.has: documentation This PR adds or changes documentation label Feb 9, 2024
@timbertson
Copy link
Contributor Author

Good idea, I've added a "Proxy usage" section in the nixpkgs fetchers chapter.

@timbertson timbertson requested a review from roberth February 22, 2024 10:10
@nixos-discourse
Copy link

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Apr 5, 2024
@timbertson timbertson requested a review from ulrikstrid as a code owner April 11, 2024 08:22
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.
@timbertson
Copy link
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 🤦

@timbertson timbertson closed this Apr 11, 2024
@github-actions github-actions bot removed 6.topic: python Python is a high-level, general-purpose programming language. 6.topic: nixos Issues or PRs affecting NixOS modules, or package usability issues specific to NixOS 6.topic: GNOME GNOME desktop environment and its underlying platform 6.topic: haskell General-purpose, statically typed, purely functional programming language 6.topic: qt/kde Object-oriented framework for GUI creation 6.topic: kernel The Linux kernel 8.has: changelog This PR adds or changes release notes 8.has: module (update) This PR changes an existing module in `nixos/` 6.topic: policy discussion Discuss policies to work in and around Nixpkgs 6.topic: vim Advanced text editor 6.topic: erlang General-purpose, concurrent, functional high-level programming language 6.topic: ocaml OCaml is a general-purpose, high-level, multi-paradigm programming language. 6.topic: xfce The Xfce Desktop Environment 6.topic: nodejs Node.js is a free, open-source, cross-platform JavaScript runtime environment 6.topic: pantheon The Pantheon desktop environment 6.topic: cinnamon Desktop environment 6.topic: module system About "NixOS" module system internals 6.topic: systemd Software suite that provides an array of system components for Linux operating systems. 6.topic: Lumina DE The Lumina Desktop Environment 6.topic: Enlightenment DE The Enlightenment Desktop Environment 6.topic: mate The MATE Desktop Environment 6.topic: vscode A free and versatile code editor that supports almost every major programming language. 6.topic: jupyter Interactive computing tooling: kernels, notebook, jupyterlab labels Apr 11, 2024
@NixOS NixOS locked as too heated and limited conversation to collaborators Apr 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

2.status: merge conflict This PR has merge conflicts with the target branch 6.topic: fetch Fetchers (e.g. fetchgit, fetchsvn, ...) 6.topic: golang Go is a high-level general purpose programming language that is statically typed and compiled. 6.topic: lib The Nixpkgs function library 8.has: documentation This PR adds or changes documentation 10.rebuild-darwin: 501+ This PR causes many rebuilds on Darwin and should normally target the staging branches. 10.rebuild-darwin: 5001+ This PR causes many rebuilds on Darwin and must target the staging branches. 10.rebuild-darwin-stdenv This PR causes stdenv to rebuild on Darwin and must target a staging branch. 10.rebuild-linux: 501+ This PR causes many rebuilds on Linux and should normally target the staging branches. 10.rebuild-linux: 5001+ This PR causes many rebuilds on Linux and must target the staging branches.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants