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
nixos/wrappers: make mount have the +s bit. #95444
Conversation
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/how-to-make-a-derivations-executables-have-the-s-permission/8555/6 |
Hello, there was some previous discussion on this at #9848. I still support doing this, it would be useful, maybe behind a option for those that don't want setuid, and/or a warning if |
Wow this has been talked over a lot! I still don't understand the security concern against the mere assertion you wanted to suggest in #9998 . Do you know how much time I wasted on trying to find this out? For the protocol, I can't use udisks as I'm not interested in mounting a device. I want to @edolstra you are the boss. I'd like to save future users at least the time I spent debugging this - If you are against even a mere assertion such as in https://github.com/NixOS/nixpkgs/pull/9998/files : any (fs: [] != intersectLists (splitString "," fs.options)
[ "owner" "user" "users" ])
fileSystems
-> all (x: elem x config.security.setuidPrograms)
[ "mount" "umount" ];
message = ''
The fstab "owner", "user", and "users" options require "mount" and "umount"
to be setuid. Please add the following to your system configuration:
security.setuidPrograms = [ "mount" "umount" ];
''; How about only mentioning this in the diff --git i/nixos/modules/tasks/filesystems.nix w/nixos/modules/tasks/filesystems.nix
index 0ade74b957a..796d8e8220f 100644
--- i/nixos/modules/tasks/filesystems.nix
+++ w/nixos/modules/tasks/filesystems.nix
@@ -52,9 +52,16 @@ let
options = mkOption {
default = [ "defaults" ];
example = [ "data=journal" ];
- description = "Options used to mount the file system.";
- type = types.listOf nonEmptyStr;
- };
+ description = ''
+ Options used to mount the file system - the . Note that using either of the
+ options <literal>users</literal>, <literal>user</literal>,
+ <literal>owner</literal> will require you to make `mount` have the +s
+ permission, which is a known security risk. See <link
+ xlink:href="https://www.linuxquestions.org/questions/slackware-14/must-be-superuser-to-use-mount-fstab-is-correct-however-144932/"
+ >this thread</link> and discussion at <link
+ xlink:href="https://github.com/NixOS/nixpkgs/issues/9848">#9848</link>
+ '';
+ type = types.listOf nonEmptyStr; };
}; |
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.
udisks
is proposed is an alternative tool to mount things in #9848, but it's more tailored towards allowing users to mount block devices ad-hoc (which aren't in /etc/fstab
anyway).
There's still usecases for the users
option in /etc/fstab
(which requires the suid bit via the wrapper) - while it's not particularily nice, it's better than silently keeping things broken, so 👍 for me on this PR.
38afcae
to
8190075
Compare
Thanks @flokli. The last push adds |
Thanks! Can you extend one of the VM tests to check this?
|
Sure. I tested both the use case of expecting a |
8190075
to
d03da65
Compare
|
Who is black? |
4c9b8d8
to
d2b247c
Compare
d2b247c
to
975fe78
Compare
The code formatter - try running the test after the latest version of this PR.
|
For NixOS#95444 Co-authored-by: Florian Klink <flokli@flokli.de>
Oh I see - it's due to the now longer subtest descriptions. Now it's fixed. |
975fe78
to
2519e54
Compare
For NixOS#95444 Co-authored-by: Florian Klink <flokli@flokli.de>
See
https://discourse.nixos.org/t/how-to-make-a-derivations-executables-have-the-s-permission/8555
and:
https://www.linuxquestions.org/questions/slackware-14/must-be-superuser-to-use-mount-fstab-is-correct-however-144932/
Motivation for this change
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)