Merge pull request #11693 from NixOS/backport-11610-to-2.19-maintenance

Backport #11610 to 2.19 maintenance

src/libstore/build/local-derivation-goal.cc

@@ -1740,14 +1740,21 @@ void LocalDerivationGoal::runChild()
 
         bool setUser = true;
 
-        /* Make the contents of netrc available to builtin:fetchurl
+        /* Make the contents of netrc and the CA certificate bundle
-           (which may run under a different uid and/or in a sandbox). */
+           available to builtin:fetchurl (which may run under a
+           different uid and/or in a sandbox). */
         std::string netrcData;
+        std::string caFileData;
+        if (drv->isBuiltin() && drv->builder == "builtin:fetchurl") {
            try {
-            if (drv->isBuiltin() && drv->builder == "builtin:fetchurl")
                netrcData = readFile(settings.netrcFile);
            } catch (SysError &) { }
 
+           try {
+               caFileData = readFile(settings.caFile);
+           } catch (SysError &) { }
+        }
+
 #if __linux__
         if (useChroot) {
 
@@ -1848,11 +1855,18 @@ void LocalDerivationGoal::runChild()
                     if (pathExists(path))
                         ss.push_back(path);
 
-                if (settings.caFile != "")
+                if (settings.caFile != "" && pathExists(settings.caFile)) {
-                    pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", settings.caFile, true);
+                    Path caFile = settings.caFile;
+                    pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", canonPath(caFile, true), true);
+                }
             }
 
-            for (auto & i : ss) pathsInChroot.emplace(i, i);
+            for (auto & i : ss) {
+                // For backwards-compatibiliy, resolve all the symlinks in the
+                // chroot paths
+                auto canonicalPath = canonPath(i, true);
+                pathsInChroot.emplace(i, canonicalPath);
+            }
 
             /* Bind-mount all the directories from the "host"
                filesystem that we want in the chroot
@@ -2174,7 +2188,7 @@ void LocalDerivationGoal::runChild()
                     e.second = rewriteStrings(e.second, inputRewrites);
 
                 if (drv->builder == "builtin:fetchurl")
-                    builtinFetchurl(drv2, netrcData);
+                    builtinFetchurl(drv2, netrcData, caFileData);
                 else if (drv->builder == "builtin:buildenv")
                     builtinBuildenv(drv2);
                 else if (drv->builder == "builtin:unpack-channel")

src/libstore/builtins.hh

@@ -6,7 +6,9 @@
 namespace nix {
 
 // TODO: make pluggable.
-void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData);
+void builtinFetchurl(const BasicDerivation & drv,
+    const std::string & netrcData,
+    const std::string & caFileData);
 void builtinUnpackChannel(const BasicDerivation & drv);
 
 }

src/libstore/builtins/fetchurl.cc

@@ -6,7 +6,10 @@
 
 namespace nix {
 
-void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData)
+void builtinFetchurl(
+    const BasicDerivation & drv,
+    const std::string & netrcData,
+    const std::string & caFileData)
 {
     /* Make the host's netrc data available. Too bad curl requires
        this to be stored in a file. It would be nice if we could just
@@ -16,6 +19,9 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData)
         writeFile(settings.netrcFile, netrcData, 0600);
     }
 
+    settings.caFile = "ca-certificates.crt";
+    writeFile(settings.caFile, caFileData, 0600);
+
     auto getAttr = [&](const std::string & name) {
         auto i = drv.env.find(name);
         if (i == drv.env.end()) throw Error("attribute '%s' missing", name);

src/libstore/filetransfer.cc

@@ -50,6 +50,8 @@ struct curlFileTransfer : public FileTransfer
         bool done = false; // whether either the success or failure function has been called
         Callback<FileTransferResult> callback;
         CURL * req = 0;
+        // buffer to accompany the `req` above
+        char errbuf[CURL_ERROR_SIZE];
         bool active = false; // whether the handle has been added to the multi object
         std::string statusMsg;
 
@@ -352,6 +354,9 @@ struct curlFileTransfer : public FileTransfer
             if (writtenToSink)
                 curl_easy_setopt(req, CURLOPT_RESUME_FROM_LARGE, writtenToSink);
 
+            curl_easy_setopt(req, CURLOPT_ERRORBUFFER, errbuf);
+            errbuf[0] = 0;
+
             result.data.clear();
             result.bodySize = 0;
         }
@@ -465,8 +470,8 @@ struct curlFileTransfer : public FileTransfer
                         code == CURLE_OK ? "" : fmt(" (curl error: %s)", curl_easy_strerror(code)))
                     : FileTransferError(err,
                         std::move(response),
-                        "unable to %s '%s': %s (%d)",
+                        "unable to %s '%s': %s (%d) %s",
-                        request.verb(), request.uri, curl_easy_strerror(code), code);
+                        request.verb(), request.uri, curl_easy_strerror(code), code, errbuf);
 
                 /* If this is a transient error, then maybe retry the
                    download after a while. If we're writing to a

tests/functional/linux-sandbox.sh

@@ -60,7 +60,13 @@ testCert () {
 
 nocert=$TEST_ROOT/no-cert-file.pem
 cert=$TEST_ROOT/some-cert-file.pem
+symlinkcert=$TEST_ROOT/symlink-cert-file.pem
+transitivesymlinkcert=$TEST_ROOT/transitive-symlink-cert-file.pem
+symlinkDir=$TEST_ROOT/symlink-dir
 echo -n "CERT_CONTENT" > $cert
+ln -s $cert $symlinkcert
+ln -s $symlinkcert $transitivesymlinkcert
+ln -s $TEST_ROOT $symlinkDir
 
 # No cert in sandbox when not a fixed-output derivation
 testCert missing normal       "$cert"
@@ -74,5 +80,14 @@ testCert missing fixed-output "$nocert"
 # Cert in sandbox when ssl-cert-file is set to an existing file
 testCert present fixed-output "$cert"
 
+# Cert in sandbox when ssl-cert-file is set to a (potentially transitive) symlink to an existing file
+testCert present fixed-output "$symlinkcert"
+testCert present fixed-output "$transitivesymlinkcert"
+
 # Symlinks should be added in the sandbox directly and not followed
-nix-sandbox-build symlink-derivation.nix
+nix-sandbox-build symlink-derivation.nix -A depends_on_symlink
+nix-sandbox-build symlink-derivation.nix -A test_sandbox_paths \
+    --option extra-sandbox-paths "/file=$cert" \
+    --option extra-sandbox-paths "/dir=$TEST_ROOT" \
+    --option extra-sandbox-paths "/symlinkDir=$symlinkDir" \
+    --option extra-sandbox-paths "/symlink=$symlinkcert"

tests/functional/symlink-derivation.nix

@@ -15,7 +15,8 @@ let
     '';
   };
 in
-mkDerivation {
+{
+  depends_on_symlink = mkDerivation {
     name = "depends-on-symlink";
     buildCommand = ''
       (
@@ -30,7 +31,29 @@ mkDerivation {
         [[ -L ${symlink_to_not_in_store} ]]
         [[ $(readlink ${symlink_to_not_in_store}) == ${builtins.toString ./.} ]]
         (! ls ${symlink_to_not_in_store}/)
+
+        # Native paths
       )
       echo "Success!" > $out
     '';
+  };
+
+  test_sandbox_paths = mkDerivation {
+    # Depends on the caller to set a bunch of `--sandbox-path` arguments
+    name = "test-sandbox-paths";
+    buildCommand = ''
+      (
+        set -x
+        [[ -f /file ]]
+        [[ -d /dir ]]
+
+        # /symlink and /symlinkDir should be available as raw symlinks
+        # (pointing to files outside of the sandbox)
+        [[ -L /symlink ]] && [[ ! -e $(readlink /symlink) ]]
+        [[ -L /symlinkDir ]] && [[ ! -e $(readlink /symlinkDir) ]]
+      )
+
+      touch $out
+    '';
+  };
 }

tests/nixos/fetchurl.nix

@@ -1,7 +1,7 @@
 # Test whether builtin:fetchurl properly performs TLS certificate
 # checks on HTTPS servers.
 
-{ lib, config, pkgs, ... }:
+{ pkgs, ... }:
 
 let
 
@@ -25,7 +25,7 @@ in
   name = "nss-preload";
 
   nodes = {
-    machine = { lib, pkgs, ... }: {
+    machine = { pkgs, ... }: {
       services.nginx = {
         enable = true;
 
@@ -60,13 +60,16 @@ in
     };
   };
 
-  testScript = { nodes, ... }: ''
+  testScript = ''
     machine.wait_for_unit("nginx")
     machine.wait_for_open_port(443)
 
     out = machine.succeed("curl https://good/index.html")
     assert out == "hello world\n"
 
+    out = machine.succeed("cat ${badCert}/cert.pem > /tmp/cafile.pem; curl --cacert /tmp/cafile.pem https://bad/index.html")
+    assert out == "foobar\n"
+
     # Fetching from a server with a trusted cert should work.
     machine.succeed("nix build --no-substitute --expr 'import <nix/fetchurl.nix> { url = \"https://good/index.html\"; hash = \"sha256-qUiQTy8PR5uPgZdpSzAYSw0u0cHNKh7A+4XSmaGSpEc=\"; }'")
 
@@ -74,5 +77,8 @@ in
     err = machine.fail("nix build --no-substitute --expr 'import <nix/fetchurl.nix> { url = \"https://bad/index.html\"; hash = \"sha256-rsBwZF/lPuOzdjBZN2E08FjMM3JHyXit0Xi2zN+wAZ8=\"; }' 2>&1")
     print(err)
     assert "SSL certificate problem: self-signed certificate" in err
+
+    # Fetching from a server with a trusted cert should work via environment variable override.
+    machine.succeed("NIX_SSL_CERT_FILE=/tmp/cafile.pem nix build --no-substitute --expr 'import <nix/fetchurl.nix> { url = \"https://bad/index.html\"; hash = \"sha256-rsBwZF/lPuOzdjBZN2E08FjMM3JHyXit0Xi2zN+wAZ8=\"; }'")
   '';
 }