From 3402b650cdce318e42acd4dc34f42423cafef587 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 7 Jun 2023 15:06:12 +0200 Subject: [PATCH 1/4] Add a generic check for rev attribute mismatches --- src/libfetchers/fetchers.cc | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 91db3a9eb..2860c1ceb 100644 --- a/src/libfetchers/fetchers.cc +++ b/src/libfetchers/fetchers.cc @@ -159,6 +159,12 @@ std::pair Input::fetch(ref store) const input.to_string(), *prevLastModified); } + if (auto prevRev = getRev()) { + if (input.getRev() != prevRev) + throw Error("'rev' attribute mismatch in input '%s', expected %s", + input.to_string(), prevRev->gitRev()); + } + if (auto prevRevCount = getRevCount()) { if (input.getRevCount() != prevRevCount) throw Error("'revCount' attribute mismatch in input '%s', expected %d", From 1ad3328c5efea041990fa82e6ad24ae2b4e81c24 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 7 Jun 2023 14:26:30 +0200 Subject: [PATCH 2/4] Allow tarball URLs to redirect to a lockable immutable URL Previously, for tarball flakes, we recorded the original URL of the tarball flake, rather than the URL to which it ultimately redirects. Thus, a flake URL like http://example.org/patchelf-latest.tar that redirects to http://example.org/patchelf-.tar was not really usable. We couldn't record the redirected URL, because sites like GitHub redirect to CDN URLs that we can't rely on to be stable. So now we use the redirected URL only if the server returns the `x-nix-is-immutable` or `x-amz-meta-nix-is-immutable` headers in its response. --- flake.nix | 2 + src/libcmd/common-eval-args.cc | 2 +- src/libexpr/parser.y | 2 +- src/libexpr/primops/fetchTree.cc | 2 +- src/libfetchers/attrs.hh | 1 + src/libfetchers/fetchers.hh | 10 +++- src/libfetchers/github.cc | 10 ++-- src/libfetchers/tarball.cc | 68 +++++++++++++++++++------- src/libstore/filetransfer.cc | 22 +++++++-- src/libstore/filetransfer.hh | 4 ++ tests/nixos/tarball-flakes.nix | 84 ++++++++++++++++++++++++++++++++ 11 files changed, 177 insertions(+), 30 deletions(-) create mode 100644 tests/nixos/tarball-flakes.nix diff --git a/flake.nix b/flake.nix index a4ee80b32..bdbf54169 100644 --- a/flake.nix +++ b/flake.nix @@ -590,6 +590,8 @@ tests.sourcehutFlakes = runNixOSTestFor "x86_64-linux" ./tests/nixos/sourcehut-flakes.nix; + tests.tarballFlakes = runNixOSTestFor "x86_64-linux" ./tests/nixos/tarball-flakes.nix; + tests.containers = runNixOSTestFor "x86_64-linux" ./tests/nixos/containers/containers.nix; tests.setuid = lib.genAttrs diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index ff3abd534..7f97364a1 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -165,7 +165,7 @@ SourcePath lookupFileArg(EvalState & state, std::string_view s) { if (EvalSettings::isPseudoUrl(s)) { auto storePath = fetchers::downloadTarball( - state.store, EvalSettings::resolvePseudoUrl(s), "source", false).first.storePath; + state.store, EvalSettings::resolvePseudoUrl(s), "source", false).tree.storePath; return state.rootPath(CanonPath(state.store->toRealPath(storePath))); } diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index 4d981712a..3b545fd84 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -793,7 +793,7 @@ std::pair EvalState::resolveSearchPathElem(const SearchPathEl if (EvalSettings::isPseudoUrl(elem.second)) { try { auto storePath = fetchers::downloadTarball( - store, EvalSettings::resolvePseudoUrl(elem.second), "source", false).first.storePath; + store, EvalSettings::resolvePseudoUrl(elem.second), "source", false).tree.storePath; res = { true, store->toRealPath(storePath) }; } catch (FileTransferError & e) { logWarning({ diff --git a/src/libexpr/primops/fetchTree.cc b/src/libexpr/primops/fetchTree.cc index fe880aaa8..b34a46fa0 100644 --- a/src/libexpr/primops/fetchTree.cc +++ b/src/libexpr/primops/fetchTree.cc @@ -262,7 +262,7 @@ static void fetch(EvalState & state, const PosIdx pos, Value * * args, Value & v // https://github.com/NixOS/nix/issues/4313 auto storePath = unpack - ? fetchers::downloadTarball(state.store, *url, name, (bool) expectedHash).first.storePath + ? fetchers::downloadTarball(state.store, *url, name, (bool) expectedHash).tree.storePath : fetchers::downloadFile(state.store, *url, name, (bool) expectedHash).storePath; if (expectedHash) { diff --git a/src/libfetchers/attrs.hh b/src/libfetchers/attrs.hh index 1a14bb023..9f885a793 100644 --- a/src/libfetchers/attrs.hh +++ b/src/libfetchers/attrs.hh @@ -2,6 +2,7 @@ ///@file #include "types.hh" +#include "hash.hh" #include diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 498ad7e4d..d0738f619 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -158,6 +158,7 @@ struct DownloadFileResult StorePath storePath; std::string etag; std::string effectiveUrl; + std::optional immutableUrl; }; DownloadFileResult downloadFile( @@ -167,7 +168,14 @@ DownloadFileResult downloadFile( bool locked, const Headers & headers = {}); -std::pair downloadTarball( +struct DownloadTarballResult +{ + Tree tree; + time_t lastModified; + std::optional immutableUrl; +}; + +DownloadTarballResult downloadTarball( ref store, const std::string & url, const std::string & name, diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 6c1d573ce..80598e7f8 100644 --- a/src/libfetchers/github.cc +++ b/src/libfetchers/github.cc @@ -207,21 +207,21 @@ struct GitArchiveInputScheme : InputScheme auto url = getDownloadUrl(input); - auto [tree, lastModified] = downloadTarball(store, url.url, input.getName(), true, url.headers); + auto result = downloadTarball(store, url.url, input.getName(), true, url.headers); - input.attrs.insert_or_assign("lastModified", uint64_t(lastModified)); + input.attrs.insert_or_assign("lastModified", uint64_t(result.lastModified)); getCache()->add( store, lockedAttrs, { {"rev", rev->gitRev()}, - {"lastModified", uint64_t(lastModified)} + {"lastModified", uint64_t(result.lastModified)} }, - tree.storePath, + result.tree.storePath, true); - return {std::move(tree.storePath), input}; + return {result.tree.storePath, input}; } }; diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index 96fe5faca..e42aca6db 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -32,7 +32,8 @@ DownloadFileResult downloadFile( return { .storePath = std::move(cached->storePath), .etag = getStrAttr(cached->infoAttrs, "etag"), - .effectiveUrl = getStrAttr(cached->infoAttrs, "url") + .effectiveUrl = getStrAttr(cached->infoAttrs, "url"), + .immutableUrl = maybeGetStrAttr(cached->infoAttrs, "immutableUrl"), }; }; @@ -55,12 +56,14 @@ DownloadFileResult downloadFile( } // FIXME: write to temporary file. - Attrs infoAttrs({ {"etag", res.etag}, {"url", res.effectiveUri}, }); + if (res.immutableUrl) + infoAttrs.emplace("immutableUrl", *res.immutableUrl); + std::optional storePath; if (res.cached) { @@ -111,10 +114,11 @@ DownloadFileResult downloadFile( .storePath = std::move(*storePath), .etag = res.etag, .effectiveUrl = res.effectiveUri, + .immutableUrl = res.immutableUrl, }; } -std::pair downloadTarball( +DownloadTarballResult downloadTarball( ref store, const std::string & url, const std::string & name, @@ -131,8 +135,9 @@ std::pair downloadTarball( if (cached && !cached->expired) return { - Tree { .actualPath = store->toRealPath(cached->storePath), .storePath = std::move(cached->storePath) }, - getIntAttr(cached->infoAttrs, "lastModified") + .tree = Tree { .actualPath = store->toRealPath(cached->storePath), .storePath = std::move(cached->storePath) }, + .lastModified = (time_t) getIntAttr(cached->infoAttrs, "lastModified"), + .immutableUrl = maybeGetStrAttr(cached->infoAttrs, "immutableUrl"), }; auto res = downloadFile(store, url, name, locked, headers); @@ -160,6 +165,9 @@ std::pair downloadTarball( {"etag", res.etag}, }); + if (res.immutableUrl) + infoAttrs.emplace("immutableUrl", *res.immutableUrl); + getCache()->add( store, inAttrs, @@ -168,8 +176,9 @@ std::pair downloadTarball( locked); return { - Tree { .actualPath = store->toRealPath(*unpackedStorePath), .storePath = std::move(*unpackedStorePath) }, - lastModified, + .tree = Tree { .actualPath = store->toRealPath(*unpackedStorePath), .storePath = std::move(*unpackedStorePath) }, + .lastModified = lastModified, + .immutableUrl = res.immutableUrl, }; } @@ -189,21 +198,33 @@ struct CurlInputScheme : InputScheme virtual bool isValidURL(const ParsedURL & url) const = 0; - std::optional inputFromURL(const ParsedURL & url) const override + std::optional inputFromURL(const ParsedURL & _url) const override { - if (!isValidURL(url)) + if (!isValidURL(_url)) return std::nullopt; Input input; - auto urlWithoutApplicationScheme = url; - urlWithoutApplicationScheme.scheme = parseUrlScheme(url.scheme).transport; + auto url = _url; + + url.scheme = parseUrlScheme(url.scheme).transport; - input.attrs.insert_or_assign("type", inputType()); - input.attrs.insert_or_assign("url", urlWithoutApplicationScheme.to_string()); auto narHash = url.query.find("narHash"); if (narHash != url.query.end()) input.attrs.insert_or_assign("narHash", narHash->second); + + if (auto i = get(url.query, "rev")) + input.attrs.insert_or_assign("rev", *i); + + if (auto i = get(url.query, "revCount")) + if (auto n = string2Int(*i)) + input.attrs.insert_or_assign("revCount", *n); + + url.query.erase("rev"); + url.query.erase("revCount"); + + input.attrs.insert_or_assign("type", inputType()); + input.attrs.insert_or_assign("url", url.to_string()); return input; } @@ -212,7 +233,8 @@ struct CurlInputScheme : InputScheme auto type = maybeGetStrAttr(attrs, "type"); if (type != inputType()) return {}; - std::set allowedNames = {"type", "url", "narHash", "name", "unpack"}; + // FIXME: some of these only apply to TarballInputScheme. + std::set allowedNames = {"type", "url", "narHash", "name", "unpack", "rev", "revCount"}; for (auto & [name, value] : attrs) if (!allowedNames.count(name)) throw Error("unsupported %s input attribute '%s'", *type, name); @@ -275,10 +297,22 @@ struct TarballInputScheme : CurlInputScheme : hasTarballExtension(url.path)); } - std::pair fetch(ref store, const Input & input) override + std::pair fetch(ref store, const Input & _input) override { - auto tree = downloadTarball(store, getStrAttr(input.attrs, "url"), input.getName(), false).first; - return {std::move(tree.storePath), input}; + Input input(_input); + auto url = getStrAttr(input.attrs, "url"); + auto result = downloadTarball(store, url, input.getName(), false); + + if (result.immutableUrl) { + auto immutableInput = Input::fromURL(*result.immutableUrl); + // FIXME: would be nice to support arbitrary flakerefs + // here, e.g. git flakes. + if (immutableInput.getType() != "tarball") + throw Error("tarball 'Link' headers that redirect to non-tarball URLs are not supported"); + input = immutableInput; + } + + return {result.tree.storePath, std::move(input)}; } }; diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 2346accbe..38b691279 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -186,9 +186,9 @@ struct curlFileTransfer : public FileTransfer size_t realSize = size * nmemb; std::string line((char *) contents, realSize); printMsg(lvlVomit, "got header for '%s': %s", request.uri, trim(line)); + static std::regex statusLine("HTTP/[^ ]+ +[0-9]+(.*)", std::regex::extended | std::regex::icase); - std::smatch match; - if (std::regex_match(line, match, statusLine)) { + if (std::smatch match; std::regex_match(line, match, statusLine)) { result.etag = ""; result.data.clear(); result.bodySize = 0; @@ -196,9 +196,11 @@ struct curlFileTransfer : public FileTransfer acceptRanges = false; encoding = ""; } else { + auto i = line.find(':'); if (i != std::string::npos) { std::string name = toLower(trim(line.substr(0, i))); + if (name == "etag") { result.etag = trim(line.substr(i + 1)); /* Hack to work around a GitHub bug: it sends @@ -212,10 +214,22 @@ struct curlFileTransfer : public FileTransfer debug("shutting down on 200 HTTP response with expected ETag"); return 0; } - } else if (name == "content-encoding") + } + + else if (name == "content-encoding") encoding = trim(line.substr(i + 1)); + else if (name == "accept-ranges" && toLower(trim(line.substr(i + 1))) == "bytes") acceptRanges = true; + + else if (name == "link" || name == "x-amz-meta-link") { + auto value = trim(line.substr(i + 1)); + static std::regex linkRegex("<([^>]*)>; rel=\"immutable\"", std::regex::extended | std::regex::icase); + if (std::smatch match; std::regex_match(value, match, linkRegex)) + result.immutableUrl = match.str(1); + else + debug("got invalid link header '%s'", value); + } } } return realSize; @@ -345,7 +359,7 @@ struct curlFileTransfer : public FileTransfer { auto httpStatus = getHTTPStatus(); - char * effectiveUriCStr; + char * effectiveUriCStr = nullptr; curl_easy_getinfo(req, CURLINFO_EFFECTIVE_URL, &effectiveUriCStr); if (effectiveUriCStr) result.effectiveUri = effectiveUriCStr; diff --git a/src/libstore/filetransfer.hh b/src/libstore/filetransfer.hh index 378c6ff78..a3b0dde1f 100644 --- a/src/libstore/filetransfer.hh +++ b/src/libstore/filetransfer.hh @@ -80,6 +80,10 @@ struct FileTransferResult std::string effectiveUri; std::string data; uint64_t bodySize = 0; + /* An "immutable" URL for this resource (i.e. one whose contents + will never change), as returned by the `Link: ; + rel="immutable"` header. */ + std::optional immutableUrl; }; class Store; diff --git a/tests/nixos/tarball-flakes.nix b/tests/nixos/tarball-flakes.nix new file mode 100644 index 000000000..1d43a5d04 --- /dev/null +++ b/tests/nixos/tarball-flakes.nix @@ -0,0 +1,84 @@ +{ lib, config, nixpkgs, ... }: + +let + pkgs = config.nodes.machine.nixpkgs.pkgs; + + root = pkgs.runCommand "nixpkgs-flake" {} + '' + mkdir -p $out/stable + + set -x + dir=nixpkgs-${nixpkgs.shortRev} + cp -prd ${nixpkgs} $dir + # Set the correct timestamp in the tarball. + find $dir -print0 | xargs -0 touch -t ${builtins.substring 0 12 nixpkgs.lastModifiedDate}.${builtins.substring 12 2 nixpkgs.lastModifiedDate} -- + tar cfz $out/stable/${nixpkgs.rev}.tar.gz $dir --hard-dereference + + echo 'Redirect "/latest.tar.gz" "/stable/${nixpkgs.rev}.tar.gz"' > $out/.htaccess + + echo 'Header set Link "; rel=\"immutable\""' > $out/stable/.htaccess + ''; +in + +{ + name = "tarball-flakes"; + + nodes = + { + machine = + { config, pkgs, ... }: + { networking.firewall.allowedTCPPorts = [ 80 ]; + + services.httpd.enable = true; + services.httpd.adminAddr = "foo@example.org"; + services.httpd.extraConfig = '' + ErrorLog syslog:local6 + ''; + services.httpd.virtualHosts."localhost" = + { servedDirs = + [ { urlPath = "/"; + dir = root; + } + ]; + }; + + virtualisation.writableStore = true; + virtualisation.diskSize = 2048; + virtualisation.additionalPaths = [ pkgs.hello pkgs.fuse ]; + virtualisation.memorySize = 4096; + nix.settings.substituters = lib.mkForce [ ]; + nix.extraOptions = "experimental-features = nix-command flakes"; + }; + }; + + testScript = { nodes }: '' + # fmt: off + import json + + start_all() + + machine.wait_for_unit("httpd.service") + + out = machine.succeed("nix flake metadata --json http://localhost/latest.tar.gz") + print(out) + info = json.loads(out) + + # Check that we got redirected to the immutable URL. + assert info["locked"]["url"] == "http://localhost/stable/${nixpkgs.rev}.tar.gz" + + # Check that we got the rev and revCount attributes. + assert info["revision"] == "${nixpkgs.rev}" + assert info["revCount"] == 1234 + + # Check that fetching with rev/revCount/narHash succeeds. + machine.succeed("nix flake metadata --json http://localhost/latest.tar.gz?rev=" + info["revision"]) + machine.succeed("nix flake metadata --json http://localhost/latest.tar.gz?revCount=" + str(info["revCount"])) + machine.succeed("nix flake metadata --json http://localhost/latest.tar.gz?narHash=" + info["locked"]["narHash"]) + + # Check that fetching fails if we provide incorrect attributes. + machine.fail("nix flake metadata --json http://localhost/latest.tar.gz?rev=493300eb13ae6fb387fbd47bf54a85915acc31c0") + machine.fail("nix flake metadata --json http://localhost/latest.tar.gz?revCount=789") + machine.fail("nix flake metadata --json http://localhost/latest.tar.gz?narHash=sha256-tbudgBSg+bHWHiHnlteNzN8TUvI80ygS9IULh4rklEw=") + ''; + +} From cab03fb7794f6e20e02dc1460a78201ab3955c18 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 16 Jun 2023 15:58:42 +0200 Subject: [PATCH 3/4] Add docs --- doc/manual/src/SUMMARY.md.in | 2 ++ doc/manual/src/protocols/protocols.md | 4 +++ doc/manual/src/protocols/tarball-fetcher.md | 40 +++++++++++++++++++++ 3 files changed, 46 insertions(+) create mode 100644 doc/manual/src/protocols/protocols.md create mode 100644 doc/manual/src/protocols/tarball-fetcher.md diff --git a/doc/manual/src/SUMMARY.md.in b/doc/manual/src/SUMMARY.md.in index 69c721b57..cfbb38be8 100644 --- a/doc/manual/src/SUMMARY.md.in +++ b/doc/manual/src/SUMMARY.md.in @@ -98,6 +98,8 @@ - [Channels](command-ref/files/channels.md) - [Default Nix expression](command-ref/files/default-nix-expression.md) - [Architecture](architecture/architecture.md) +- [Protocols](protocols/protocols.md) + - [Serving Tarball Flakes](protocols/tarball-fetcher.md) - [Glossary](glossary.md) - [Contributing](contributing/contributing.md) - [Hacking](contributing/hacking.md) diff --git a/doc/manual/src/protocols/protocols.md b/doc/manual/src/protocols/protocols.md new file mode 100644 index 000000000..d6bf1d809 --- /dev/null +++ b/doc/manual/src/protocols/protocols.md @@ -0,0 +1,4 @@ +# Protocols + +This chapter documents various developer-facing interfaces provided by +Nix. diff --git a/doc/manual/src/protocols/tarball-fetcher.md b/doc/manual/src/protocols/tarball-fetcher.md new file mode 100644 index 000000000..1c6c92000 --- /dev/null +++ b/doc/manual/src/protocols/tarball-fetcher.md @@ -0,0 +1,40 @@ +# Serving Tarball Flakes + +Tarball flakes are served as regular tarballs via HTTP or the file +system (for `file://` URLs). + +An HTTP server can return an "immutable" flakeref appropriate for lock +files. This allows users to specify a tarball flake input in +`flake.nix` that requests the latest version of a flake +(e.g. `https://example.org/hello/latest.tar.gz`), while `flake.lock` +will record a URL whose contents will not change +(e.g. `https://example.org/hello/.tar.gz`). To do so, the +server must return a `Link` header with the `rel` attribute set to +`immutable`, as follows: + +``` +Link: ; rel="immutable" +``` + +(Note the required `<` and `>` characters around *flakeref*.) + +*flakeref* must be a tarball flakeref. It can contain flake attributes +such as `narHash`, `rev` and `revCount`. If `narHash` is included, its +value must be the NAR hash of the unpacked tarball (as computed via +`nix hash path`). Nix checks the contents of the returned tarball +against the `narHash` attribute. The `rev` and `revCount` attributes +are useful when the tarball flake is a mirror of a fetcher type that +has those attributes, such as Git or GitHub. They are not checked by +Nix. + +``` +Link: ; rel="immutable" +``` + +(The linebreaks in this example are for clarity and must not be included in the actual response.) + +For tarball flakes, the value of the `lastModified` flake attribute is +defined as the timestamp of the newest file inside the tarball. From b1ed9b4b0cc037a8b14dcc37fd32f6a5a16e7ba3 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 16 Jun 2023 16:48:37 +0200 Subject: [PATCH 4/4] Apply suggestions from code review Co-authored-by: Robert Hensing --- doc/manual/src/protocols/tarball-fetcher.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/doc/manual/src/protocols/tarball-fetcher.md b/doc/manual/src/protocols/tarball-fetcher.md index 1c6c92000..0d3212303 100644 --- a/doc/manual/src/protocols/tarball-fetcher.md +++ b/doc/manual/src/protocols/tarball-fetcher.md @@ -1,15 +1,17 @@ -# Serving Tarball Flakes +# Lockable HTTP Tarball Protocol -Tarball flakes are served as regular tarballs via HTTP or the file -system (for `file://` URLs). +Tarball flakes can be served as regular tarballs via HTTP or the file +system (for `file://` URLs). Unless the server implements the Lockable +HTTP Tarball protocol, it is the responsibility of the user to make sure that +the URL always produces the same tarball contents. -An HTTP server can return an "immutable" flakeref appropriate for lock +An HTTP server can return an "immutable" HTTP URL appropriate for lock files. This allows users to specify a tarball flake input in `flake.nix` that requests the latest version of a flake (e.g. `https://example.org/hello/latest.tar.gz`), while `flake.lock` will record a URL whose contents will not change (e.g. `https://example.org/hello/.tar.gz`). To do so, the -server must return a `Link` header with the `rel` attribute set to +server must return an [HTTP `Link` header](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Link) with the `rel` attribute set to `immutable`, as follows: ```