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

nix-universal-prefetch: init at 0.2.0 #53436

Merged
merged 1 commit into from Jan 23, 2019

Conversation

samueldr
Copy link
Member

@samueldr samueldr commented Jan 5, 2019

Upstream URL: https://github.com/samueldr/nix-universal-prefetch

Motivation for this change

Let's hopefully have users use this!

This software is intended to resolve once and for all the question "but I don't want to TOFU?" (which isn't even a question!) when end-users want to get the hash to use with a specific fetcher.

For anyone confused by TOFU, one of the quick replies on IRC is:

To get a sha256 hash of a new source, you can use the Trust On First Use model: use probably-wrong hash (for example: 0000000000000000000000000000000000000000000000000000) then replace it with the correct hash Nix expected.

This, in addition, could be used with automated scripts to reduce hand-holding when the fetchers aren't simple underneath.

Things done
  • ✔️ Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • Built on platform(s)
    • ✔️ NixOS
    • ⬜ macOS
    • ⬜ other Linux distributions
  • ⬜ Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • ✔️ Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • ✔️ Tested execution of all binary files (usually in ./result/bin/)
  • ⬜ Determined the impact on package closure size (by running nix path-info -S before and after)
  • ✔️ Assured whether relevant documentation is up to date
  • ✔️ Fits CONTRIBUTING.md.

Additionally, I ate my own dogfood when making this PR, and did not TOFU the hash, but instead ran:

nix-universal-prefetch fetchFromGitHub --owner samueldr --repo nix-universal-prefetch --rev v0.1.0

license = licenses.mit;
maintainers = with maintainers; [ samueldr ];
platforms = platforms.linux ++ platforms.darwin;
inherit version;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, why put the version in the meta? It's already an attribute on the derivation.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Frankly, you'd have to ask me past July, when I added nix-top as I copied its derivation and removed what wasn't required, while not really looking at meta. I must have had a brain fart when implementing that.

@bhipple
Copy link
Contributor

bhipple commented Jan 5, 2019

This looks neat! Another use-case you may not have considered, besides re-verifying TOFU hashes: when nix already has the src in the store it doesn't need to hit the internet. Some users may be running nix daemons on machines that don't have internet access for security/isolation/auditing purposes, which means users need to setup a network proxy and semi-manually prefetch external sources when upgrading; having a general tool to handle all the various ways of prefetching is 👍

@Mic92 Mic92 mentioned this pull request Jan 13, 2019
10 tasks
@samueldr samueldr changed the title nix-universal-prefetch: init at 0.1.0 nix-universal-prefetch: init at 0.2.0 Jan 15, 2019
@samueldr
Copy link
Member Author

The update (0.2.0) adds support for nix 2.2's new error message. It also supports builtins.fetchurl which has a different message.

All this is now tested, though not tested in this build as the test is made using nix.

@danbst
Copy link
Contributor

danbst commented Jan 20, 2019

Another case when universal prefetcher would be better then url prefetcher e36b4d6

@danbst danbst merged commit 6418e34 into NixOS:master Jan 23, 2019
@samueldr samueldr deleted the nix-universal-prefetch branch January 23, 2019 18:36
@danbst
Copy link
Contributor

danbst commented Jan 23, 2019

Though, to be honest, it is not true:

Additionally, I ate my own dogfood when making this PR, and did not TOFU the hash, but instead ran:

This prefetcher is TOFU, but hidden from eyes. What would be extra awesome, if in next iteration:

  • some support for checking TLS certificates is added to nixpkgs (I can think of a configuration parameter, which when passed to nixpkgs makes fetch* check certificates). Requested by @7c6f434c
  • this prefetcher then applies that config by default when prefetching things. When certificates check fail, it should fail as well and point to MITM attack page in docs
  • then rename this to nix-prefetch
  • autocomplete!

Then we can make this an "officially blessed" prefetcher and mention in docs.

@samueldr
Copy link
Member Author

You're right, not not-TOFU since it's TOFU, but automated for you, but isn't nix-prefetch TOFU in that sense (in the current implementation)?

And right again, this would need the implementation details of further checks to be made in nixpkgs, since there is no introspection possible here as to making further checks.

samueldr/nix-universal-prefetch#1 is a tracking issue if you want to reference it in the future.


As for autocomplete, there's an annoying hard problem here: there is no way to introspect deep enough to show all arguments. Let's use fetchFromGitHub as an example, all of its arguments are discoverable, but it will pass-through some arguments to either fetchurl or fetchgit, depending on some conditions. What could be done, though, is to supplement the list of discovered arguments with a list of known arguments, either through making "dumb" passthrough arguments inside nixpkgs, or inside nix-universal-prefetch. (Implementation inside nixpkgs is probably preferrable.)


And thanks for the interest :).

@msteen
Copy link
Contributor

msteen commented Jan 24, 2019

I have been working on my own idea for nix-prefetch: https://github.com/msteen/nix-prefetch. I think I have managed to improve upon all aspects of nix-universal-prefetch and have still some more ideas on how to improve it further.

@danbst Could you give me more information about that TLS support, I like to implement your suggestion. Also with regards to auto complete, I have not implemented it yet, but I do have all the information available to make it work, even considering the problem mentioned by @samueldr. The only limitation being is that for fetchers (mine supports both pointing to fetchers and extending existing fetcher calls, like package sources), I would need to keep a list of valid bogus values, or else risk producing errors.

@danbst
Copy link
Contributor

danbst commented Jan 24, 2019

@msteen it is basically set here: https://github.com/NixOS/nix/blob/800cd55ab74fffe9a1d212c2b8caf4024ebd98b6/src/libstore/download.hh#L16

Pretty impressed, that there are now 2 implementations of same concept!

@7c6f434c
Copy link
Member

@msteen re: TLS: all (if I remember correctly; in any case, most) of the Nixpkgs fetchers intended for direct use in packages do not verify certificates; the assumption is that if hash is given, why bother how the content was obtained — and size leaks do indeed kill all or almost all privacy when downloading tarballs (size leaks can even damage privacy of encrypted VoIP, and popular tarballs on a single server are known to attackers in advance).

With TOFU that means zero MITM protection.

The main benefit is probably that an old Nixpkgs checkout without fresh certificates can still build something.

I think, given the popularity of manual TOFU which is not going to disappear overnight, we should convert all the fetchers to opt-out certificate checking: the default is to check, but there are both a fetcher parameter and a global option to skip verification (the first is useful for packages with known-non-trivial setups, like special-purposes CA certificates; the second is useful in case Nixpkgs checkout is too old)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants