networking.defaultMailServer.authPassFile: allow types.path#29298
networking.defaultMailServer.authPassFile: allow types.path#29298Ma27 wants to merge 1 commit intoNixOS:masterfrom
Conversation
|
@Ma27, thanks for your PR! By analyzing the history of the files in this pull request, we identified @edolstra, @abbradar and @basvandijk to be potential reviewers. |
nixos/modules/programs/ssmtp.nix
Outdated
There was a problem hiding this comment.
Won't this result in /run/keys/ssmtp-autpass being copied to /nix/store/... at build time? If I understand that option correctly, that would be very undesirable. (You want the file not to be copied to the a world readable Nix store path.)
There was a problem hiding this comment.
the types.path allows relative paths and does some validation to ensure that the path actually exists while evaluating the expressions.
Furthermore the configuration file accepts a file path which comes directly from the configuration if authPassFile is given.
There was a problem hiding this comment.
I believe what @bjornfor means is that nix might copy the named file into the store while evaluating the example value, whereas when it is quoted it is treated as a string literal.
There was a problem hiding this comment.
Exactly what I meant. Also, if this reasoning is correct, using types.str ensures that one cannot define the option with a value that allows nix to copy the file into the store at eval/build time. So it acts as a safeguard against accidentally leaking the file into the store. (Please correct me if I'm wrong.)
There was a problem hiding this comment.
Per https://github.com/NixOS/nixpkgs/blob/master/lib/types.nix#L167, types.path is just a str that must begin with a forward slash.
I think it's generally hard to enforce stuff not being copied into the store via module options because nix probably will have evaluated the value (and hence copied stuff into the store) before it gets a chance to do the type-check. So if we added, say, types.keyPath with the proviso that the value could not point into the store, nix might still copy the file into the Store despite module eval ultimately failing type-check.
There was a problem hiding this comment.
Then again, perhaps I'm thinking about this the wrong way ...
There was a problem hiding this comment.
you're right, it seems as Nix the origin of a path file into the nix store which means that this change might result in a security vulnerability -> closing for now...
6f34d77 to
78ec175
Compare
78ec175 to
da15b5f
Compare
Motivation for this change
types.pathis semantically correct when providing file path values.However
networking.defaultMailServer.authPassFileonly allows strings and null ATM.Things done
build-use-sandboxinnix.confon non-NixOS)nix-shell -p nox --run "nox-review wip"./result/bin/)