[RFC 0097] Unset read permission bit on /nix/store for other users #97
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,80 @@ | ||
| --- | ||
| feature: nix-store-perms | ||
| start-date: 2021-07-04 | ||
| author: Las Safin | ||
| co-authors: | ||
| shepherd-team: @kevincox @7c6f434c @edolstra | ||
| shepherd-leader: @edolstra | ||
| related-issues: | ||
| --- | ||
|
|
||
| # Summary | ||
| [summary]: #summary | ||
|
|
||
| - NixOS should have a module for configuring the permissions set for `/nix/store` on boot. | ||
| - Nix should not enforce the permissions used for `/nix/store`. | ||
| - The default permissions if the store doesn't exist should be 1735 when the store is made by Nix or the NixOS installer. | ||
| This means that the nixbld group can't `ls` the directory. | ||
|
|
||
| # Motivation | ||
| [motivation]: #motivation | ||
|
|
||
| Right now you can't set the permissions for `/nix/store`, since they'll be overwritten | ||
| by Nix anytime you use `nix`. | ||
|
|
||
| `chmod g-r /nix/store` is beneficial because the `nixbld` group doesn't actually | ||
| need to read the directory. It only needs to be able to write and "execute" it. | ||
| This, however, should be optional, since the user should be able to configure | ||
| the permissions however they want. | ||
|
|
||
| Some users might also want to do things like `chmod o-r /nix/store`, which | ||
| gives you the interesting property that you can not access paths you do not | ||
| already know of. | ||
| Do note that given that all processes can by default read `/proc/cmdline`, | ||
| `/run/current-system`, and many other places which reveal your | ||
| system's closure, making this permission change an insufficient solution for | ||
| security in many cases. This, however, is also entirely optional and is not | ||
| the default in any way. | ||
|
|
||
| # Detailed design | ||
L-as marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| [design]: #detailed-design | ||
|
|
||
| Where we previously would enforce the permissions, we now need to | ||
| only set them if there is no directory in the first place. | ||
| The same applies for `/nix/store/trash` and `/nix/store/.links`. | ||
|
|
||
| Specifically, we need to modify the following places (not exhaustive): | ||
| - [nixpkgs/nixos/modules/system/boot/stage-2-init.sh](https://github.com/NixOS/nixpkgs/blob/8284fc30c84ea47e63209d1a892aca1dfcd6bdf3/nixos/modules/system/boot/stage-2-init.sh#L62) | ||
| - [nix/scripts/install-multi-user.sh](https://github.com/NixOS/nix/blob/cf1d4299a8fa8906f62271dcd878018cef84cc30/scripts/install-multi-user.sh#L577) | ||
| - [nix/src/libstore/globals.hh](https://github.com/NixOS/nix/blob/ba8b39c13003c8ddafb6bec308997e09b9851c46/src/libstore/globals.hh#L278) | ||
| - [nix/src/libstore/build/local-derivation-goal.cc](https://github.com/NixOS/nix/blob/6182ae689826554d915b4ed72e07f7978dc1d13c/src/libstore/build/local-derivation-goal.cc#L641) | ||
| - [nix/src/libstore/local-store.cc](https://github.com/NixOS/nix/blob/0a535dd5ac93576f7152d786464e330ae3d46b50/src/libstore/local-store.cc#L181) | ||
|
|
||
| # Examples and Interactions | ||
| [examples-and-interactions]: #examples-and-interactions | ||
|
|
||
| You should be able to do something like the following: | ||
| ```nix | ||
| nix.store-perms = "xxxx"; | ||
| ``` | ||
|
|
||
| # Drawbacks | ||
L-as marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| [drawbacks]: #drawbacks | ||
|
|
||
| If a user on a non-NixOS platform mistakenly sets the permissions for `/nix/store` to | ||
| something undesirable, it won't be reverted by Nix automatically. | ||
|
Comment on lines
+64
to
+65
There was a problem hiding this comment. Why not restrict it to sensible or at least not terrible values? Seems quite feasible and the allow list could be extended if we discover a new use case. There was a problem hiding this comment. Because this «undesirable» includes the idea that most likely unfortunate mistakes are someon else's «just as planned» (I am not even sure 0000 is always useless; hmmm, now I am tempted to try once this is merged and trying is easy) |
||
|
|
||
| # Alternatives | ||
| [alternatives]: #alternatives | ||
|
|
||
| You could not do this and keep it as it is. | ||
|
|
||
| # Unresolved questions | ||
L-as marked this conversation as resolved.
Show resolved
Hide resolved
L-as marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| [unresolved]: #unresolved-questions | ||
|
|
||
| There doesn't seem to be any. | ||
|
|
||
| # Future work | ||
| [future]: #future-work | ||
|
|
||
| In the future we likely want to reduce the default permissions for `/nix/store` as much as possible. | ||
|
There was a problem hiding this comment. Sometimes I use tab completion on a store path; saves some mousing when I fail to copy a path completely, or sometimes typing three or four letters of the hash is easy enough. |
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that at least some filesystems are vulnerable to timing attacks, probing paths with
statto build prefixes of subdirectory paths until the full path is recovered; using thexbit to recoverrfor all paths, including ones that aren't running.Nonetheless, it's good for practical hermeticity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we hope that people either look at the list and start, like you, name extra leak vectors, or trust «insufficient for security» claim. It would be interesting to have a survey of path leak vectors, but this RFC is shorter than any useful survey like that…
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree. Any list we try to create in this RFC is likely incomplete so it is better to just state that as an assumption and accept it for the rationale here. If someone wants to prove that the list of leaks is empty (and expected to stay that way) they can propose a follow-up RFC to rely on that fact.