aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEelco Dolstra <edolstra@gmail.com>2024-09-23 15:09:44 +0200
committerJade Lovelace <lix@jade.fyi>2024-09-25 18:40:58 -0700
commitc1631b0a39d34267345b41214f1f5e8a77d98cd2 (patch)
tree7345d79929d07ceeb56c857f5388a769f0bf228c
parent8a6b84df147f5f38bae710fac9ec085d9d4e8ded (diff)
[security] builtin:fetchurl: Enable TLS verification
This is better for privacy and to avoid leaking netrc credentials in a MITM attack, but also the assumption that we check the hash no longer holds in some cases (in particular for impure derivations). Partially reverts https://github.com/NixOS/nix/commit/5db358d4d78aea7204a8f22c5bf2a309267ee038. (cherry picked from commit c04bc17a5a0fdcb725a11ef6541f94730112e7b6) (cherry picked from commit f2f47fa725fc87bfb536de171a2ea81f2789c9fb) (cherry picked from commit 7b39cd631e0d3c3d238015c6f450c59bbc9cbc5b) Upstream-PR: https://github.com/NixOS/nix/pull/11585 Change-Id: Ia973420f6098113da05a594d48394ce1fe41fbb9
-rw-r--r--doc/manual/rl-next/verify-tls.md10
-rw-r--r--src/libstore/builtins/fetchurl.cc3
-rw-r--r--tests/nixos/default.nix2
-rw-r--r--tests/nixos/fetchurl.nix78
4 files changed, 90 insertions, 3 deletions
diff --git a/doc/manual/rl-next/verify-tls.md b/doc/manual/rl-next/verify-tls.md
new file mode 100644
index 000000000..608f3347e
--- /dev/null
+++ b/doc/manual/rl-next/verify-tls.md
@@ -0,0 +1,10 @@
+---
+synopsis: "`<nix/fetchurl.nix>` now uses TLS verification"
+category: Fixes
+prs: [11585]
+credits: edolstra
+---
+
+Previously `<nix/fetchurl.nix>` did not do TLS verification. This was because the Nix sandbox in the past did not have access to TLS certificates, and Nix checks the hash of the fetched file anyway. However, this can expose authentication data from `netrc` and URLs to man-in-the-middle attackers. In addition, Nix now in some cases (such as when using impure derivations) does *not* check the hash. Therefore we have now enabled TLS verification. This means that downloads by `<nix/fetchurl.nix>` will now fail if you're fetching from a HTTPS server that does not have a valid certificate.
+
+`<nix/fetchurl.nix>` is also known as the builtin derivation builder `builtin:fetchurl`. It's not to be confused with the evaluation-time function `builtins.fetchurl`, which was not affected by this issue.
diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc
index 062ecdc14..3fb769fe6 100644
--- a/src/libstore/builtins/fetchurl.cc
+++ b/src/libstore/builtins/fetchurl.cc
@@ -33,10 +33,7 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData)
auto fetch = [&](const std::string & url) {
- /* No need to do TLS verification, because we check the hash of
- the result anyway. */
FileTransferRequest request(url);
- request.verifyTLS = false;
auto raw = fileTransfer->download(std::move(request));
auto decompressor = makeDecompressionSource(
diff --git a/tests/nixos/default.nix b/tests/nixos/default.nix
index 20e66f6c1..2d6eaed16 100644
--- a/tests/nixos/default.nix
+++ b/tests/nixos/default.nix
@@ -157,4 +157,6 @@ in
coredumps = runNixOSTestFor "x86_64-linux" ./coredumps;
io_uring = runNixOSTestFor "x86_64-linux" ./io_uring;
+
+ fetchurl = runNixOSTestFor "x86_64-linux" ./fetchurl.nix;
}
diff --git a/tests/nixos/fetchurl.nix b/tests/nixos/fetchurl.nix
new file mode 100644
index 000000000..63c639c31
--- /dev/null
+++ b/tests/nixos/fetchurl.nix
@@ -0,0 +1,78 @@
+# Test whether builtin:fetchurl properly performs TLS certificate
+# checks on HTTPS servers.
+
+{ lib, config, pkgs, ... }:
+
+let
+
+ makeTlsCert = name: pkgs.runCommand name {
+ nativeBuildInputs = with pkgs; [ openssl ];
+ } ''
+ mkdir -p $out
+ openssl req -x509 \
+ -subj '/CN=${name}/' -days 49710 \
+ -addext 'subjectAltName = DNS:${name}' \
+ -keyout "$out/key.pem" -newkey ed25519 \
+ -out "$out/cert.pem" -noenc
+ '';
+
+ goodCert = makeTlsCert "good";
+ badCert = makeTlsCert "bad";
+
+in
+
+{
+ name = "fetchurl";
+
+ nodes = {
+ machine = { lib, pkgs, ... }: {
+ services.nginx = {
+ enable = true;
+
+ virtualHosts."good" = {
+ addSSL = true;
+ sslCertificate = "${goodCert}/cert.pem";
+ sslCertificateKey = "${goodCert}/key.pem";
+ root = pkgs.runCommand "nginx-root" {} ''
+ mkdir "$out"
+ echo 'hello world' > "$out/index.html"
+ '';
+ };
+
+ virtualHosts."bad" = {
+ addSSL = true;
+ sslCertificate = "${badCert}/cert.pem";
+ sslCertificateKey = "${badCert}/key.pem";
+ root = pkgs.runCommand "nginx-root" {} ''
+ mkdir "$out"
+ echo 'foobar' > "$out/index.html"
+ '';
+ };
+ };
+
+ security.pki.certificateFiles = [ "${goodCert}/cert.pem" ];
+
+ networking.hosts."127.0.0.1" = [ "good" "bad" ];
+
+ virtualisation.writableStore = true;
+
+ nix.settings.experimental-features = "nix-command";
+ };
+ };
+
+ testScript = { nodes, ... }: ''
+ machine.wait_for_unit("nginx")
+ machine.wait_for_open_port(443)
+
+ out = machine.succeed("curl https://good/index.html")
+ assert out == "hello world\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 or "SSL peer certificate or SSH remote key was not OK" in err
+ '';
+}