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

Provide options for storing secrets outside the Nix store #24288

Open
basvandijk opened this issue Mar 24, 2017 · 65 comments
Open

Provide options for storing secrets outside the Nix store #24288

basvandijk opened this issue Mar 24, 2017 · 65 comments

Comments

@basvandijk
Copy link
Member

@basvandijk basvandijk commented Mar 24, 2017

Introduction

Dear module authors and maintainers,

We currently have many modules that force users to store their secrets in the world-readble Nix store. This is bad for security. We should give users the option of specifying their secrets in individual files which can be stored outside the Nix store with suitable ownership and permissions. Users could then also use nixops to manage their secret files.

There's still the convenient but unsafe option of storing the secret file in the Nix store using pkgs.writeTextFile. If NixOS/nix#8 gets resolved these files can be encrypted / made private. Also see: NixOS/rfcs#5.

Proposal

The list below contains all the options that force a secret being stored in the Nix store. I propose the following:

  1. Each option should get a warning in the documentation of the form: "Warning: this secret is stored in the world-readable Nix store!"

  2. Each option should get an alternative passwordFile option.

  3. For backwards compatibility the passwordFile option should get a default based on the password option:

{ config = {
    passwordFile = mkDefault (toString (pkgs.writeTextFile {
      name = "password-file";
      text = cfg.password;
    }));
  };
}
  1. Some upstream programs don't support setting a password using a file. In that case an issue should be created in the upstream issue-tracker asking for that feature. (See namecoin/namecoin-core#148 for example). A URL to the issue should be placed in the list below and in the documentation of the password option so that it's easier to track when it gets resolved.

  2. If after some time (lets use September 2017 for now) the upstream developers have not provided the feature to specify the password by file, the NixOS module should be changed such that the config file that contains the password is written to /run before the service starts up. So something like the following:

{
  systemd.services.my-service = {  
    preStart = ''
      cat > /run/my-service/config << EOF
      ...
      password = $(cat "${cfg.passwordFile}")
      ...
      EOF
    '';
    script = "${pkgs.myService}/bin/my-service --config=/run/my-service/config";
  };
}
  1. Lets use this issue for planning and to track progress. Please mention in the comments if you have provided a passwordFile option for one of the options below. Then I check the box to indicate it has been resolved. See PR #24146 for reference.

  2. If we make sure the new options are backwards compatible we could consider cherry-picking them onto release-17.03 making sure users get these security fixes ASAP.

Secret options

This list was compiled by running the following in <nixpkgs> and manually inspecting and processing the result:

find . -type f -exec grep --color -nH -i -E -e '(secret|pass|key)' {} +`
@ip1981
Copy link
Contributor

@ip1981 ip1981 commented Mar 24, 2017

FYI, check out how I handle secrets throughout all the apps here: https://github.com/ip1981/nixsap/tree/master/modules/apps

Especially, with Jenkins :)

@teh
Copy link
Contributor

@teh teh commented Mar 24, 2017

I suspect @fpletz is now the maintainer for gitlab (sorry, I see you'r name is in this list a lot!)

@edolstra
Copy link
Member

@edolstra edolstra commented Mar 24, 2017

Wow, I didn't know the situation was this bad. All of these options should be removed really. It's almost as if people don't realize that the Nix store is world readable...

@jml
Copy link
Contributor

@jml jml commented Mar 24, 2017

@basvandijk — any plan for preventing future changes adding secrets to the Nix store, beyond eternal vigilance?

@Ekleog
Copy link
Member

@Ekleog Ekleog commented Mar 24, 2017

Hmm, I didn't re-read NixOS/nix#8 completely, but it seemed to me last time that @edolstra 's solution using encryption in the store and decryption at startup time was working and there was mostly bikeshedding about encryption vs ACLs?

This would be much easier than trying to patch every single upstream program that does not accept password files, especially given that some may not be willing to do it as it adds quite a bit of complexity. Wouldn't it?

@rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Mar 24, 2017

Besides being quite inconvenient, storing passwords/keys in a file with restricted access outside the store, may not solve the problem: they could end up in a systemd environment file or a unit file if you need to pass those as an command line argument.
Also expecting everyone upstream to comply seems incredibly too optimistic for me.

@basvandijk
Copy link
Member Author

@basvandijk basvandijk commented Mar 24, 2017

any plan for preventing future changes adding secrets to the Nix store, beyond eternal vigilance?

@jml we could add something to the PULL_REQUEST_TEMPLATE.md instructing contributors to use passwordFile instead of password options.

@basvandijk
Copy link
Member Author

@basvandijk basvandijk commented Mar 24, 2017

All of these options should be removed really...

@edolstra we could do that eventually but to ease the transition we should first provide a backwards compatible passwordFile option, then in a next release we could start throwing warnings when users use the password option and finally in a third release we can remove the password options.

@basvandijk
Copy link
Member Author

@basvandijk basvandijk commented Mar 24, 2017

@Ekleog regarding NixOS/nix#8, even if we have the ability to encrypt files in the Nix store I think it would be best to only encrypt files that should be encrypted. Currently we have big config files that somewhere contain a password. It would nicer if the config file remains unencrypted because then it can be shared and it makes debugging easier. Only the password needs to be encrypted. So having passwords in individual files would still be desirable.

@edolstra
Copy link
Member

@edolstra edolstra commented Mar 24, 2017

@basvandijk The encryption stuff allows you to encrypt only the "secret" parts of a configuration file. See edolstra@4c82120#diff-6c3fcb531890fdce200531b9ac69e4f8R14 for an example.

@rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Mar 24, 2017

@basvandijk Sometimes even the password stored in the configuration file needs to be readable. dnschain, for example, parses namecoin.conf to connect to the rpc server.

@basvandijk
Copy link
Member Author

@basvandijk basvandijk commented Mar 24, 2017

@rnhmjoj lets see how upstream reacts to a request for a rpcpasswordFile parameter...

@basvandijk
Copy link
Member Author

@basvandijk basvandijk commented Mar 24, 2017

The encryption stuff allows you to encrypt only the "secret" parts of a configuration file.

@edolstra that's great! What needs to be done to get this merged into Nix?

@edolstra
Copy link
Member

@edolstra edolstra commented Mar 24, 2017

Well, it's not clear whether this is the way to go. @kevincox listed the issues here: NixOS/nix#8 (comment)

@basvandijk
Copy link
Member Author

@basvandijk basvandijk commented Mar 24, 2017

@rnhmjoj regarding namecoind, would cookie based authentication work?

@basvandijk
Copy link
Member Author

@basvandijk basvandijk commented Mar 24, 2017

Besides being quite inconvenient, storing passwords/keys in a file with restricted access outside the store, may not solve the problem: they could end up in a systemd environment file or a unit file if you need to pass those as an command line argument.

@rnhmjoj regarding secrets in systemd unit files, we can always create a wrapper script that cats the password file and passes that on to the original script. I do something similar here.

@basvandijk basvandijk changed the title Provide options of storing secrets outside the Nix store Provide options for storing secrets outside the Nix store Mar 24, 2017
@mbrgm
Copy link
Member

@mbrgm mbrgm commented Mar 24, 2017

@basvandijk Regarding upstream changes for password file options: I think some cat/sed trickery, maybe creating a config file in /run from a template should also do the job for most cases where upstream doesn't have or doesn't want to add a password file, shouldn't it?

@basvandijk
Copy link
Member Author

@basvandijk basvandijk commented Mar 25, 2017

@mbrgm sure and we should do that in case upstream doesn't provide a password file option.

@rasendubi
Copy link
Member

@rasendubi rasendubi commented Mar 25, 2017

@basvandijk I would not tick the package until the PR is merged.

@rnhmjoj
Copy link
Contributor

@rnhmjoj rnhmjoj commented Mar 25, 2017

@basvandijk That seems a valid alternative for authenticating to namecoind however dnschain does not support it, so it would break the service. Anyway, thank you for opening the issue.

@basvandijk
Copy link
Member Author

@basvandijk basvandijk commented Mar 25, 2017

I would not tick the package until the PR is merged.

@rasendubi makes sense. I've unticked the wordpress checkbox.

@edwtjo
Copy link
Member

@edwtjo edwtjo commented Mar 25, 2017

Well for aiccu; SixXS is closing down its IPv6 tunnel in June so it doesn't seem worth the effort to create a patch for aiccu to support password files. Lets just remove the service in 0606.

@zimbatm
Copy link
Member

@zimbatm zimbatm commented Mar 26, 2017

Nix encryption is going to take a while to get there given nix's release history speed. I don't think it should be a blocker for trying alternative implementations.

On the nixpkgs side, the protection would be based on unix file ACL. How about treating secrets like any other state? We could introduce a "mkState" interface that defines any kind of state reference on the filesystem.

let
  postgresState = mkState {
    type = "directory";
    name = "postgres";
    owner = "postgres";
    group = "postgres";
    mode = "0700";
    mustExist = false;
    # run a command if it's missing
    onMissing = "pg_init";
  };
  assert (toString postgresState) == "/var/lib/postgres";
  # mkSecret is a specialization of mkState with a default dir to /run/keys/${name}, mode = 0700 and mustExist = true  
  nginxSecret = mkSecret {
    owner = "nginx";
  };

This would translate to (1) some activation script actions like creating the state dir (2) systemd service to initialize and/or check the secret, which can then be used as a dependency for other services.

I know it's still pretty vague but hopefully enough to convey the idea.

@CMCDragonkai
Copy link
Member

@CMCDragonkai CMCDragonkai commented Jul 13, 2018

@zimbatm About your last comment about nix 2.0, can you provide an example of how you use that to manage secrets from a private nix store?

@ob7
Copy link

@ob7 ob7 commented Jul 13, 2018

So if someone puts a password in plain text in a .nix file, that password gets uploaded to somewhere online that could be accessed by anyone?

@srhb
Copy link
Contributor

@srhb srhb commented Jul 13, 2018

@ob7 "World readable" in this sense means that any user on your system can read the file. It does not mean that files included in the nix store are somehow uploaded elsewhere.

@aanderse
Copy link
Contributor

@aanderse aanderse commented Sep 6, 2018

@basvandijk You can mark redmine as complete as of 18.09 as a passwordFile option has been added.

@aanderse
Copy link
Contributor

@aanderse aanderse commented Feb 10, 2019

@basvandijk the owncloud module was removed so you can tick that off.

@gbiscuolo
Copy link

@gbiscuolo gbiscuolo commented Nov 16, 2019

This is a related topic I started on Discourse: In configuration.nix can I read a value from a file?

@globin
Copy link
Member

@globin globin commented Nov 16, 2019

Also see NixOS/rfcs#59

@FRidh FRidh added this to the 20.03 milestone Dec 30, 2019
@wucke13
Copy link
Contributor

@wucke13 wucke13 commented Jan 13, 2020

services.mattermost.localDatabasePassword needs adjustment too.

@Infinisil
Copy link
Member

@Infinisil Infinisil commented Feb 9, 2020

Related is my PR which adds types.secretPath: #78640

@rnhmjoj rnhmjoj mentioned this issue Mar 23, 2020
3 of 10 tasks complete
@aanderse aanderse mentioned this issue Apr 25, 2020
3 of 10 tasks complete
@liamdiprose
Copy link

@liamdiprose liamdiprose commented Jun 2, 2020

List Updates
These options now exist:

  • services.grafana.database.passwordFile
  • services.grafana.security.secretKeyFile
  • services.grafana.security.adminPasswordFile

These options need an associated passwordFile option:

  • services.grafana.provision.datasources.*.password
  • services.grafana.provision.datasources.*.basicAuthPassword
@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Jun 26, 2020

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

https://discourse.nixos.org/t/any-default-config-for-reproducible-configurations/5268/17

@wucke13
Copy link
Contributor

@wucke13 wucke13 commented Aug 6, 2020

services.mattermost.localDatabasePassword is not possible via file as well.

@doronbehar
Copy link
Contributor

@doronbehar doronbehar commented Sep 11, 2020

I think we should add to this list the mpd module, which now gets the ability to store it's password "<pwd>@permissions" entry outside the store, at #95599 .

chkno added a commit to chkno/nixpkgs that referenced this issue Sep 21, 2020
…ix store

With type.path, both of these usages are permitted:

    key = ./foo-client-key.pem;           # Bad!  Puts secret in store.
    key = "/var/lib/foo/client-key.pem";  # OK

Disallow the bad usage using types.strMatching "/.*" instead of types.path.

The server version of this probably ought to be changed also, but that
would be a breaking change.  We can change the client key type because
this commit and the commit that introduces the client key option are
in the same PR and will be merged atomically.  We keep this a separate
commit to provide a succinct concrete example of the problems described
in NixOS#24288 .
@felschr
Copy link
Contributor

@felschr felschr commented Oct 22, 2020

I just noticed that virtualisation.oci-containers.containers.<name>.environment doesn't allow scripting:
https://github.com/NixOS/nixpkgs/blob/master/nixos/modules/virtualisation/oci-containers.nix#L242

Container environment variables are frequently used for secrets, too, thus I think we should provide an easy way to include them.
I see a few different ways to solve this:

  • stop escaping the value of the environment variables and thus allow for shell scripting (potentially breaking current configs)
  • add an environmentFiles option that reads variables from files, similar to the passwordFile options
  • add an option or separate config to opt-in to shell scripting in environment variables
@volth
Copy link
Contributor

@volth volth commented Oct 22, 2020

We have to link here https://github.com/Mic92/sops-nix as an elegant solution for secrets in Nix

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

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.