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

Support private files in the Nix store #8

Open
edolstra opened this issue Apr 23, 2012 · 76 comments
Open

Support private files in the Nix store #8

edolstra opened this issue Apr 23, 2012 · 76 comments
Labels
feature Feature request or proposal significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc.

Comments

@edolstra
Copy link
Member

Sometimes it's desirable to have files in the Nix store that are not world-readable, such as configuration files containing passwords. This could be implemented as follows:

  • Private files are marked as such by giving them a certain magic prefix to the name, e.g. p:. When these are registered, Nix should make them readable only to root (permission 0700) and the calling user (using an ACL). (The Nix daemon knows the uid of the caller.)
  • The permissions on a derivation should be propagated to the output paths. Also, private derivations should be run with a umask of 0077.
  • If another user attempts to register an already valid private file, then its uid should be added to the ACL of the file. If it's a derivation, then the uid should also be added to the ACLs of any valid outputs.
  • Store operations that read files from the store (such as exportPath()) should check that the caller is in the ACL of the file.

So, for instance, if the root user builds a NixOS configuration containing some private files, then those files are only readable as root. If Alice builds the same configuration, then those files will be readable by root and Alice. If Alice then uses Charon to copy the configuration to a remote machine (i.e. nix-copy-closure --to root@remote ...), then the files on the remote machine will only be readable by root.

This model only allows files that are private to the instantiating user and root. So it doesn't support specifying a file that is readable only to the sshd user. But that's probably not necessary.

@ghost ghost assigned edolstra Apr 23, 2012
@shlevy
Copy link
Member

shlevy commented May 10, 2012

A few questions:

  • Does this potentially break nixos-rebuild build as non-root?
  • Is there any way to tell as a user that I've attempted to register an already-valid file? For example, if root and I both use the same password file for a particular program, will I be able to tell when I add that file to the store that root has the same one?
  • Would there be a reasonable way for a NixOS-managed service to know how to find per-user private files?

@edolstra
Copy link
Member Author

  • It shouldn't break non-root nixos-rebuilds since any private files would be owned by (or accessible to) the calling user.
  • If a user attempts to register an already-valid file, he would be added to the ACL to the file. So he could conclude from this that root needed the same file.
  • What kind of service, and why would it need to do that?

@viric
Copy link
Member

viric commented May 31, 2012

What about the build users? Should they be in the ACLs?

@chaoflow
Copy link
Member

Being able to tell that somebody else generated the same private file sounds like a no-go for me.

What do you think about including the user the file is private to, i.e. the one who created it, to be included in the hashing (for example an empty file with the user's name in the output)? So even if root, Alice and Charon all want a file with the same content, they would end up with 3 different files.

@shlevy
Copy link
Member

shlevy commented Jul 26, 2012

The problem with including the user in the hashing is you may eventually share stores, but it's better than nothing. Maybe a wrapper function in nixpkgs that hashed in a GUID read from some personal file? So I have ~/.nixpkgs/guid and when I run pkgs.lib.privateFile 'mypass' "passw0rd" it creates a file whose first line is passw0rd and the second line is the guid?

@edolstra
Copy link
Member Author

Why is this a problem? If the secret stored in the file is sufficiently secure (i.e. random), then this won't happen. I mean, if your login password is "foobar", then other people may be able to find out. So just don't do that.

@shlevy
Copy link
Member

shlevy commented Jul 26, 2012

Eh, it's not really a problem, at least not for nix itself. The security-paranoid folk can just make sure all of their private files have some unique identifier included.

@mornfall
Copy link

Unless I am mistaken, private files in the store are sensitive to offline dictionary attacks, since you can learn the hash and you have a procedure to check your guesses. It is also conceivable, that a rainbow attack could be mounted against a particular version of nixpkgs. It would be advisable that any such private files contain significant salting, regardless of the "learn a secret by random collision" problem. A hypothetical lib.privateFile could then enforce that salting by taking a template string and substituting in the salt (which makes it possible to hide that salt in comments or other such convenient places).

@viric
Copy link
Member

viric commented Mar 31, 2013

Yes, the sysadmin could care storing the salt somewhere only readable by the root user.

@domenkozar
Copy link
Member

There is a use case for pkgs.bacula that bacula user needs access to a configuration file with passwords (so non-root).

@MarcWeber
Copy link
Contributor

Alternative quick & dirty solution which may be useful for some use cases: https://github.com/MarcWeber/nix/blob/experimental/write-file-hashed/.topmsg

@domenkozar
Copy link
Member

Better link to see @MarcWeber diff: https://github.com/MarcWeber/nix/compare/experimental;write-file-hashed

What's the current workaround for this if a config file includes a password?

@MarcWeber
Copy link
Contributor

My version allows storing passwords in config, but instead of writing it to the store a path to a file containing it will be written to the store. The containing file can be read only by root. The complicated piece is that you have to write code which has to be run as root (eg when preparing config files for services) - but its very easy to review (eg in contrast to patching nix store allowing paths to be readable by some users only requires to think about many issues such as copying store paths - preserving privileges thereby etc). State is "I don't use my patch but it should be working".

@edolstra edolstra added bug and removed bug labels Feb 26, 2014
@domenkozar
Copy link
Member

What about a function with signature:

pkgs.lib.privateFile "my.conf" "user" "secret content";

And only user would have read permissions to that file? That solves the use cases I've been seeing on daily basis. Many services require to specify an user and thus we can pass that user to privateFile function.

No idea how that would work with nix-copy-closure, but we could just preserve the uid/gid.

@edolstra
Copy link
Member Author

One problem with that is that user may not exist yet, since it might be created at activation time.

However, for the most common case (root), it would be fine. And if the service needs to be readable by a non-root service, you could always copy it to some private location in /run in the service's preStart script.

@domenkozar
Copy link
Member

Now that our users require an uid, we could use that instead of the username. That would work then right?

aszlig added a commit to aszlig/nixops that referenced this issue Jun 24, 2014
This however only implements setting permissions if "storeKeysOnMachine" is
set to false right now, because if the value is set to true the keys are
symlinked from the store and we actually have to find a way to control
permisions on it, which for the store is only possible if NixOS/nix#8 is
implemented.

Also, this ensures that the key filename is properly escaped.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
aszlig added a commit to aszlig/nixops that referenced this issue Jul 4, 2014
This however only implements setting permissions if "storeKeysOnMachine" is
set to false right now, because if the value is set to true the keys are
symlinked from the store and we actually have to find a way to control
permisions on it, which for the store is only possible if NixOS/nix#8 is
implemented.

Also, this ensures that the key filename is properly escaped.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
@nbp
Copy link
Member

nbp commented Oct 18, 2014

@edolstra , with the test cases I made in #329, I am confident that the current proposal is secure enough to handle secret files for the owner of the nix store. When would you be able to review it?

This branch does not use ACL, it only remove the read/execute access for the group and others. Still, the implementation is made such as we can extend it with ACL later. I think it would be good to have a first iteration of NixOS with this branch before adding ACL.

atsukotakahashi pushed a commit to atsukotakahashi/ops that referenced this issue Nov 1, 2014
This however only implements setting permissions if "storeKeysOnMachine" is
set to false right now, because if the value is set to true the keys are
symlinked from the store and we actually have to find a way to control
permisions on it, which for the store is only possible if NixOS/nix#8 is
implemented.

Also, this ensures that the key filename is properly escaped.

Signed-off-by: aszlig <aszlig@redmoonstudios.org>
@wmertens
Copy link
Contributor

wmertens commented Jan 5, 2015

I just thought of another use-case of @MarcWeber's patch: setuid binaries.

As a reminder, MarcWeber/nix@master...experimental/write-file-hashed shows how nix can write content with special permissions outside of the store. It's a very minimal patch compared to #329. (what it is missing is that the hash needs to include the permissions and ownership as well for security)

Using this, you can either sideload secret files, generated them from an expression or have them be copied on package install from a specific directory under /nix-support.

Files can be secrets or simply regular files that need special permissions, like setuid binaries.

@edolstra @nbp?

@domenkozar
Copy link
Member

How can I help make this happen? NixOS is getting used more and more and this may lead to bad PR in the future since it's indeed a big issue (the biggest?).

@lucabrunox
Copy link
Contributor

Personally I overcome this problem by providing files as strings instead of a nix store path. Yes that means the configuration is not deployable/reproducible, but perhaps I don't want my passwords to be deployable/reproducible?

@wmertens
Copy link
Contributor

@lethalman can you give an example? I don't understand what you're doing when providing a private file as a string.

@stale
Copy link

stale bot commented Jan 3, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jan 3, 2022
@pinpox
Copy link
Member

pinpox commented Jan 4, 2022

Any progress on this in the last 10 years since it was opened?

@stale stale bot removed the stale label Jan 4, 2022
@wmertens
Copy link
Contributor

wmertens commented Jan 6, 2022

@pinpox well there's no real consensus, there's more discussion in NixOS/rfcs#5, but unless someone comes up with a compelling generic approach, the current way is userspace approaches like sops-nix and agenix.

It would be better to close this issue I think.

@wmertens
Copy link
Contributor

wmertens commented Jan 6, 2022

oh and actually a nixos-only approach via systemd seems to be working, see NixOS/nixpkgs#102397

nixos+systemd-only is not really a problem since that is where most of the solutions are required and ACL management would require multi-user anyway.

@edolstra
Copy link
Member Author

edolstra commented Jan 6, 2022

I'm currently working on ACL support (described here).

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/idiomatic-nixos/17079/2

@stale
Copy link

stale bot commented Jul 11, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the stale label Jul 11, 2022
@winny-
Copy link

winny- commented Jul 11, 2022

.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/how-to-effectively-manage-secrets/25793/14

@azahi
Copy link
Member

azahi commented Mar 17, 2023

Related RFC: NixOS/rfcs/pull/143

@roberth roberth added the significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc. label Jun 2, 2023
ryantm pushed a commit to ryantm/nix that referenced this issue Sep 10, 2023
…tests

Implement deduplication and add more test cases
zolodev pushed a commit to zolodev/nix that referenced this issue Jan 1, 2024
@edolstra edolstra removed their assignment Apr 26, 2024
tebowy pushed a commit to tebowy/nix that referenced this issue Jul 11, 2024
this was caused by the use of std::chrono::duration::max() which gets
multiplied by some ratio to calculate nanoseconds to wait. then, it
explodes because that is a signed integer overflow. this was definitely
a bug.

error below:

/nix/store/fdiknsmnnczx6brsbppyljcs9hqckawk-gcc-12.3.0/include/c++/12.3.0/bits/chrono.h:225:38: runtime error: signed integer overflow: 9223372036854775807 * 1000000 cannot be represented in type 'long'
    #0 0x736d376b2b69 in std::chrono::duration<long, std::ratio<1l, 1000000000l>> std::chrono::__duration_cast_impl<std::chrono:
:duration<long, std::ratio<1l, 1000000000l>>, std::ratio<1000000l, 1l>, long, false, true>::__cast<long, std::ratio<1l, 1000l>>(
std::chrono::duration<long, std::ratio<1l, 1000l>> const&) /nix/store/fdiknsmnnczx6brsbppyljcs9hqckawk-gcc-12.3.0/include/c++/12
.3.0/bits/chrono.h:225:38
    NixOS#1 0x736d376b2b69 in std::enable_if<__is_duration<std::chrono::duration<long, std::ratio<1l, 1000000000l>>>::value, std::chr
ono::duration<long, std::ratio<1l, 1000000000l>>>::type std::chrono::duration_cast<std::chrono::duration<long, std::ratio<1l, 10
00000000l>>, long, std::ratio<1l, 1000l>>(std::chrono::duration<long, std::ratio<1l, 1000l>> const&) /nix/store/fdiknsmnnczx6brs
bppyljcs9hqckawk-gcc-12.3.0/include/c++/12.3.0/bits/chrono.h:270:9
    NixOS#2 0x736d376b2b69 in std::enable_if<__is_duration<std::chrono::duration<long, std::ratio<1l, 1000000000l>>>::value, std::chr
ono::duration<long, std::ratio<1l, 1000000000l>>>::type std::chrono::ceil<std::chrono::duration<long, std::ratio<1l, 1000000000l
>>, long, std::ratio<1l, 1000l>>(std::chrono::duration<long, std::ratio<1l, 1000l>> const&) /nix/store/fdiknsmnnczx6brsbppyljcs9
hqckawk-gcc-12.3.0/include/c++/12.3.0/bits/chrono.h:386:14
    NixOS#3 0x736d376b2b69 in std::cv_status std::condition_variable::wait_for<long, std::ratio<1l, 1000l>>(std::unique_lock<std::mut
ex>&, std::chrono::duration<long, std::ratio<1l, 1000l>> const&) /nix/store/fdiknsmnnczx6brsbppyljcs9hqckawk-gcc-12.3.0/include/
c++/12.3.0/condition_variable:164:6
    NixOS#4 0x736d376b1ee9 in std::cv_status nix::Sync<nix::ProgressBar::State, std::mutex>::Lock::wait_for<long, std::ratio<1l, 1000
l>>(std::condition_variable&, std::chrono::duration<long, std::ratio<1l, 1000l>> const&) /home/jade/lix/lix/src/libutil/sync.hh:
65:23
    NixOS#5 0x736d376b1ee9 in nix::ProgressBar::ProgressBar(bool)::'lambda'()::operator()() const /home/jade/lix/lix/src/libmain/prog
ress-bar.cc:99:27
    NixOS#6 0x736d36de25c2 in execute_native_thread_routine (/nix/store/a3zlvnswi1p8cg7i9w4lpnvaankc7dxx-gcc-12.3.0-lib/lib/libstdc++
.so.6+0xe05c2)
    NixOS#7 0x736d36b6b0e3 in start_thread (/nix/store/1zy01hjzwvvia6h9dq5xar88v77fgh9x-glibc-2.38-44/lib/libc.so.6+0x8b0e3) (BuildId
: 287831bffdbdde0ec25dbd021d12bdfc0ab9f5ff)
    NixOS#8 0x736d36bed5e3 in __clone (/nix/store/1zy01hjzwvvia6h9dq5xar88v77fgh9x-glibc-2.38-44/lib/libc.so.6+0x10d5e3) (BuildId: 28
7831bffdbdde0ec25dbd021d12bdfc0ab9f5ff)

SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior /nix/store/fdiknsmnnczx6brsbppyljcs9hqckawk-gcc-12.3.0/include/c++/12.3.
0/bits/chrono.h:225:38 in

Change-Id: Ia0303242cdfd5d49385ae9e99718d709625a4633
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request or proposal significant Novel ideas, large API changes, notable refactorings, issues with RFC potential, etc.
Projects
None yet
Development

No branches or pull requests