Skip to content

Commit

Permalink
Merge pull request #11693 from NixOS/backport-11610-to-2.19-maintenance
Browse files Browse the repository at this point in the history
Backport #11610 to 2.19 maintenance
  • Loading branch information
roberth authored Oct 21, 2024
2 parents 211c806 + 9965a29 commit 52166fd
Show file tree
Hide file tree
Showing 7 changed files with 105 additions and 34 deletions.
34 changes: 24 additions & 10 deletions src/libstore/build/local-derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1740,13 +1740,20 @@ void LocalDerivationGoal::runChild()

bool setUser = true;

/* Make the contents of netrc available to builtin:fetchurl
(which may run under a different uid and/or in a sandbox). */
/* Make the contents of netrc and the CA certificate bundle
available to builtin:fetchurl (which may run under a
different uid and/or in a sandbox). */
std::string netrcData;
try {
if (drv->isBuiltin() && drv->builder == "builtin:fetchurl")
netrcData = readFile(settings.netrcFile);
} catch (SysError &) { }
std::string caFileData;
if (drv->isBuiltin() && drv->builder == "builtin:fetchurl") {
try {
netrcData = readFile(settings.netrcFile);
} catch (SysError &) { }

try {
caFileData = readFile(settings.caFile);
} catch (SysError &) { }
}

#if __linux__
if (useChroot) {
Expand Down Expand Up @@ -1848,11 +1855,18 @@ void LocalDerivationGoal::runChild()
if (pathExists(path))
ss.push_back(path);

if (settings.caFile != "")
pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", settings.caFile, true);
if (settings.caFile != "" && pathExists(settings.caFile)) {
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
Expand Down Expand Up @@ -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")
Expand Down
4 changes: 3 additions & 1 deletion src/libstore/builtins.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}
8 changes: 7 additions & 1 deletion src/libstore/builtins/fetchurl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);
Expand Down
9 changes: 7 additions & 2 deletions src/libstore/filetransfer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)",
request.verb(), request.uri, curl_easy_strerror(code), code);
"unable to %s '%s': %s (%d) %s",
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
Expand Down
17 changes: 16 additions & 1 deletion tests/functional/linux-sandbox.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
55 changes: 39 additions & 16 deletions tests/functional/symlink-derivation.nix
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,45 @@ let
'';
};
in
mkDerivation {
name = "depends-on-symlink";
buildCommand = ''
(
set -x
{
depends_on_symlink = mkDerivation {
name = "depends-on-symlink";
buildCommand = ''
(
set -x
# `foo_symlink` should be a symlink pointing to `foo_in_store`
[[ -L ${foo_symlink} ]]
[[ $(readlink ${foo_symlink}) == ${foo_in_store} ]]
# `symlink_to_not_in_store` should be a symlink pointing to `./.`, which
# is not available in the sandbox
[[ -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 ]]
# `foo_symlink` should be a symlink pointing to `foo_in_store`
[[ -L ${foo_symlink} ]]
[[ $(readlink ${foo_symlink}) == ${foo_in_store} ]]
# /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) ]]
)
# `symlink_to_not_in_store` should be a symlink pointing to `./.`, which
# is not available in the sandbox
[[ -L ${symlink_to_not_in_store} ]]
[[ $(readlink ${symlink_to_not_in_store}) == ${builtins.toString ./.} ]]
(! ls ${symlink_to_not_in_store}/)
)
echo "Success!" > $out
'';
touch $out
'';
};
}
12 changes: 9 additions & 3 deletions tests/nixos/fetchurl.nix
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
# Test whether builtin:fetchurl properly performs TLS certificate
# checks on HTTPS servers.

{ lib, config, pkgs, ... }:
{ pkgs, ... }:

let

Expand All @@ -25,7 +25,7 @@ in
name = "nss-preload";

nodes = {
machine = { lib, pkgs, ... }: {
machine = { pkgs, ... }: {
services.nginx = {
enable = true;

Expand Down Expand Up @@ -60,19 +60,25 @@ 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=\"; }'")
# Fetching from a server with an untrusted cert should fail.
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=\"; }'")
'';
}

0 comments on commit 52166fd

Please sign in to comment.