Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Backport #11610 to 2.20 maintenance #11686

Draft
wants to merge 6 commits into
base: 2.20-maintenance
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -1732,13 +1732,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 (SystemError &) { }
std::string caFileData;
if (drv->isBuiltin() && drv->builder == "builtin:fetchurl") {
try {
netrcData = readFile(settings.netrcFile);
} catch (SystemError &) { }

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

#if __linux__
if (useChroot) {
Expand Down Expand Up @@ -1840,11 +1847,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 @@ -2166,7 +2180,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 out = get(drv.outputs, "out");
if (!out)
throw Error("'builtin:fetchurl' requires an 'out' output");
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=\"; }'")
'';
}
Loading