NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
17.47k stars 13.66k forks source link

paperless: provide a way to set `PAPERLESS_SECRET_KEY` safely #261085

Closed matrss closed 10 months ago

matrss commented 11 months ago

Describe the bug

The paperless module does not provide a way to safely set PAPERLESS_SECRET_KEY (i.e. in a way that does not end up in the nix store, at least I couldn't think of any). Since paperless also does not support reading the key from a file something like sops-nix can't be used.

To make matters worse, paperless defaults to a hard-coded and publicly known secret key (as documented in https://docs.paperless-ngx.com/configuration/#PAPERLESS_SECRET_KEY). That means the only options are to set the value directly in the nix config and have it exposed in the nix store or use the default, which makes paperless insecure to open to the internet.

Since the documentation of the NixOS module does not mention the PAPERLESS_SECRET_KEY at all I'd imagine there might be a few unknowingly unsafe instances out there.

This is my understanding from reading the docs. If this is not an issue because of something I missed please correct me.

Steps To Reproduce

Steps to reproduce the behavior:

  1. setup paperless using the module
  2. set PAPERLESS_SECRET_KEY
  3. observe that it gets put into the systemd file directly

Expected behavior

Having some way to specify this secret safely.

Additional context

The best fix would be if paperless was able to read the secret from a file. There is a request to add that feature in https://github.com/paperless-ngx/paperless-ngx/discussions/614. Until then, it might be useful to address this issue in the NixOS module, if possible.

Notify maintainers

@erikarvstedt @Flakebi

matrss commented 11 months ago

Now that I thought about this a bit more I think I could use systemd's EnvironmentFile to load the secret safely (from sops-nix). That the "path of least resistance" is rather unsafe is still suboptimal though, in my opinion.

mweinelt commented 11 months ago

We could fail in prestart, if there is no secret key in the environment.


diff --git a/nixos/modules/services/misc/paperless.nix b/nixos/modules/services/misc/paperless.nix
index 9b8bd62809c5..fb5a427bbc79 100644
--- a/nixos/modules/services/misc/paperless.nix
+++ b/nixos/modules/services/misc/paperless.nix
@@ -252,6 +252,11 @@ in

           echo ${pkg.version} > "$versionFile"
         fi
+
+        if [ -z "$PAPERLESS_SECRET_KEY" ]; then
+          echo "Refusing to start without PAPERLESS_SECRET_KEY. See https://docs.paperless-ngx.com/configuration/#PAPERLESS_SECRET_KEY."
+          exit 1
+        fi
       ''
       + optionalString (cfg.passwordFile != null) ''
         export PAPERLESS_ADMIN_USER="''${PAPERLESS_ADMIN_USER:-admin}"
fabian-thomas commented 11 months ago

Why not provide a secretKeyFile attribute similar to services.paperless.passwordFile?

ctheune commented 11 months ago

The way that I handle this typically is to create a file on disk during the first startup and then use an exported environment variable every time.

fabian-thomas commented 11 months ago

Note: I started a discussion upstream about this, since I feel like paperless should enforce a randomized secret in any case (even when in local network). See https://github.com/paperless-ngx/paperless-ngx/discussions/4386.

matrss commented 11 months ago

Why not provide a secretKeyFile attribute similar to services.paperless.passwordFile?

I support this. It could be a mandatory option (or at least show a warning if unset) that explains the issue in its description. As far as I understand it, it is more or less safe to change the secret key even for existing instances. This would just invalidate everything that is signed using the secret key, so e.g. existing user sessions get logged out.

mweinelt commented 11 months ago

The way that I handle this typically is to create a file on disk during the first startup and then use an exported environment variable every time.

This is the solution, that should be implemented. It fixes the situation for everyone and makes it an implementation detail, not another pointless setting.

FliegendeWurst commented 11 months ago

I think it is possible to set PAPERLESS_SECRET_KEY_FILE (see https://github.com/paperless-ngx/paperless-ngx/pull/1034) if our package uses the docker-entrypoint.sh.

matrss commented 11 months ago

I think it is possible to set PAPERLESS_SECRET_KEY_FILE (see paperless-ngx/paperless-ngx#1034) if our package uses the docker-entrypoint.sh.

I did not double check, but I strongly assume that the nix package is not using the docker entrypoint script. paperless-ngx is a "standard" buildPythonApplication without any reference to docker. See: https://github.com/NixOS/nixpkgs/blob/ca012a02bf8327be9e488546faecae5e05d7d749/pkgs/applications/office/paperless-ngx/default.nix#L88