Unsigned derivations uploaded to S3

[JackKelly-Bellroy]

Describe the bug

On Nix 2.9.2 when signing/uploading store paths in CI using post-build-hook as suggested by the manual, unsigned derivations get uploaded to S3.

Steps To Reproduce

1. Set up a post-build-hook as per the manual:

   #!/bin/sh
   
   set -eu
   set -f # disable globbing
   export IFS=' '
   
   echo "Signing paths" \$OUT_PATHS
   $(which nix) store sign --key-file /etc/nix/bellroy-nix-cache.private \$OUT_PATHS
   echo "Uploading paths" \$OUT_PATHS
   exec $(which nix) copy --to 's3://bellroy-nix-cache' \$OUT_PATHS

2. Build a bunch of stuff
3. On a machine with a fresh store (e.g., new CI instance, nix store gc, ...),
4. See a lot of warning: the substitute for '/nix/store/whatever' from 's3://cache-bucket' is not signed by any of the keys in 'trusted-public-keys'

Expected behavior

All derivations uploaded to S3 are signed. I have confirmed that some of the derivations that make it to S3 are unsigned by using nix path-info --json --sigs --store s3://cache-bucket /nix/store/whatever | jq.

Additional context

Nix 2.7 seems to work fine; Nix 2.8 was affected by #6446 which passed no paths at all to the post build hook. I suspect that leaves might be getting signed but not intermediate derivations; I'm not sure how to verify this. Should I be passing -r to nix store sign? I can see little difference in how nix store sign is implemented between versions 2.7.0 and 2.9.2.

This makes me suspect the way the post-build-hook is invoked. On Nix 2.7.0:

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

Lines 881 to 889 in ffe155a

	StorePathSet outputPaths;
	for (auto & [_, path] : finalOutputs)
	outputPaths.insert(path);
	runPostBuildHook(
	worker.store,
	*logger,
	drvPath,
	outputPaths
	);

On Nix 2.9.2:

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

Lines 903 to 915 in f5070e2

	/* Compute the FS closure of the outputs and register them as
	being valid. */
	auto builtOutputs = registerOutputs();
	StorePathSet outputPaths;
	for (auto & [_, output] : builtOutputs)
	outputPaths.insert(output.outPath);
	runPostBuildHook(
	worker.store,
	*logger,
	drvPath,
	outputPaths
	);

But I don't know enough about Nix internals to know whether or not it's safe to go back to using finalOutputs here.

[domenkozar]

We're seeing similar errors on Cachix side while I know they for sure get signed: cachix/cachix#428

Maybe client validation has changed somehow?

[JackKelly-Bellroy]

I think both. I am maybe 60% sure I saw derivations with no signature (I checked using nix path-info, but I wonder whether or not I remembered to pass --sigs), but a colleague using Nix 2.9.2 also reports seeing this warning on derivations that are definitely signed. The warning went away after switching back to Nix 2.7.0.

[domenkozar]

cc @edolstra, I think this is quite a serious regression on the Nix side.

[JackKelly-Bellroy]

I had work create a second S3 bucket for testing, and re-ran CI against this bucket with Nix 2.10.3. After pulling down *.narinfo from the new bucket and running nix path-info --sigs --json --store s3://new-test-bucket over each output path, I can now confirm newer versions of Nix are uploading unsigned store paths to S3 when called with the traditional post-build-hook. What's interesting is that the unsigned paths has a couple of things in common: they're the data outputs of cross-compiled haskell libraries from haskell.nix. The fact that they're cross-compiled might be a quirk of the way the CI is set up, but it's interesting that it's non-out outputs that aren't getting signed by the post-build-hook.

@domenkozar your client-side issue is probably real but different to this?

[thufschmitt]

I can now confirm newer versions of Nix are uploading unsigned store paths to S3 when called with the traditional post-build-hook.

Do you mean that the derivations are properly signed locally, but loose their signature when copied to the binary cache, or are they not signed at all?

[teto]

I havent investigated but I think I've witnessed something similar too. Is the cache mechanism tested in nix releases ?

[thufschmitt]

Is the cache mechanism tested in nix releases ?

It is, of course, but you know, tests…

[JackKelly-Bellroy]

I can now confirm newer versions of Nix are uploading unsigned store paths to S3 when called with the traditional post-build-hook.

Do you mean that the derivations are properly signed locally, but loose their signature when copied to the binary cache, or are they not signed at all?

I did not check this, but since the post-build-hook in the manual is what's calling both nix store sign and nix copy, I assume the output paths are not getting signed locally. It also seems more likely to me that we'd have a bug where certain paths aren't getting signed, versus a bug where signed paths are losing their signature when copied.

[thufschmitt]

That's what I'd assume too, but I would like to get a confirmation just to be sure (and I couldn't reproduce it yet, so I can't check it myself)

[thufschmitt]

Oh actually it seems that I can reproduce it trivially and that not all the outputs are passed to the hook. Which obviously means that they aren't all signed. But an output that didn't get signed might still be copied because the copy is recursive (so if you copy something that depends on it, it'll be copied along the way)

[thufschmitt]

More precisely, what seems to happen (didn't check the code yet) is that the only outputs that Nix passes to the hook are the ones that Nix needs for the build (because something depends on it). The other ones (which are mostly a side-effect of the build at that point) aren't passed (and so aren't signed).
And if you later build something else that depends on these previously-unneeded outputs, they still won't pass through the hook (since they are already built) and so they still won't be signed – but they'll be sent to the binary cache because they are part of the closure of the new thing you're building

[thufschmitt]

A quick workaround might be to pass --recursive to nix store sign, although that might be too much because that'll sign the whole closure with your key (including for example stuff that comes from cache.nixos.org and that you maybe don't want to trust with your own key)

[thufschmitt]

didn't check the code yet

Ok I did, and it seems to be the case indeed as a side-effect of fe5509d : the list of paths passed to the hook is computed from the result of registerOutputs() which used to contain all the built paths but now only contains the “wanted” ones

[nrdxp]

@JackKelly-Bellroy, you can also set the nix.conf value secret-key-files and Nix will automatically sign any locally built paths with the given key, ensuring that if they are built, they are signed. This is what I do in CI to avoid having to explicitly sign in my post-build-hook.

[JackKelly-Bellroy]

@nrdxp Thanks for the suggestion - man nix.conf agrees with you. I'm not game to test it until #6572 is fixed, which seems to be causing a fair amount of grief for people triggering builds in CI.

[nghamilton]

Along the lines of what @nrdxp suggested, adding the undocumented secret-key parameter to the S3 URL seems to do the job for me (e.g. nix copy --to s3://ninebyte-nix-cache?region=ap-southeast-2\&secret-key=<(echo $NIX_CACHE_PRIVATE_KEY) $OUT_PATHS). It seems all packages uploaded to S3 are now signed, as compared to when I previously used nix sign and had some without a signature.

[fricklerhandwerk]

Discussed in Nix team meeting 2023-01-06:

• @thufschmitt: think there is a rather clear fix, just needs someone to implement it
  • agreement, we want this fixed
• @Ericson2314: we should sign more things by default, would be better for a team member to draft a design
• assinged to @tomberek for a draft

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-01-06-nix-team-meeting-minutes-21/24573/1

[tomberek]

There are several ways to sign things. Let's look at the current situation:

• nix store sign: sign the specified paths and outputs only
• nix store sign -r: sign the specified paths and outputs recursively
• nix copy --to STORE_URI?secret-key=/path: sign everything that will be uploaded
• secret-key-files: a nix.conf option which will sign everything built
• post-build-hook: the hook is called for each build, but is notably missing outputs not necessary for the final output.

(If anything here is wrong, please point it out and it will be updated)

non-regrettable steps

1. post-build-hook should be called with all built outputs
2. clarify and improve the Nix manual recommend post-build-hook to reflect expected behavior

some complications

1. New syntax drv^* to refer to all outputs, otherwise it is easy to only sign the first output.
2. There are different commands and paradigms to sign various parts of the closures the user is interested in.

needs discussion/understanding

1. is improving the post-build-hook documentation and a dedicated "Nix and Signatures" section enough?
2. What signature invariants do users expect?
3. What is nix copy --secret-key-files vs nix copy --to URL?secret-key=/path?
4. Should a store be able to define policy in "nix-cache-info" that protects against mistakes?
   • local?
   • remote?

Signing more things by default

Users don't sign things because it can be confusing to get the settings right, frustrating to distribute keys safely, and managing signatures themselves is hard. A solution in this space requires a few things:

1. Visibility into signatures and what current state is of a closure
2. Allow users to express policy for a store and be warned/stopped from violating that policy.
3. Better warnings and errors to allow users to produce the outcome they want.

Proposal for discussion

A store should be able to advertise a set of policies that must be true. This makes it easier for clients to check and verify those policies and provide better error messages. Example interactions:

nix copy --to s3://somewhere some-output
Error: The remote store "s3://somewhere" policy requires paths with at least 1 signature.
Use: `nix copy --to s3://somewhere?secret-key=/some-path some-output` to sign paths being uploaded

nix copy --to s3://somewhere some-output
Error: The remote store "s3://somewhere" policy requires paths signed by one of the following.
key-1:asdfasdfasdfas
key-2:asdfasdfasdfasdf
key-3:asdfasdfasdffsd

nix build --store /path-to-store
Error: The local store "/path-to-store" must have all contents signed and cannot build paths that are not signed.
Check your secret-key-files setting in nix.conf.

[endgame]

2. clarify and improve the Nix manual recommend post-build-hook to reflect expected behavior

Already done: #7408 changes the manual to recommend secret-key-files

[nixos-discourse]

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-09-04-nix-team-meeting-minutes-85/32608/1