Forgejo: Can't use SECRET_URIs

stfnx commented 10 months ago

Describe the bug

I tried to provide the secrets with sops-nix to have them survive a re-installation. For that I've set the following options in services.forgejo.settings:

SECRET_KEY_URI (security cfg block)
INTERNAL_TOKEN_URI (security cfg block)
JWT_SECRET_URI (oauth2 cfg block)
LFS_JWT_SECRET_URI (server cfg block)

Nonetheless the secrets will get generated by the nixos module here and SECRET_KEY etc. will get added to app.ini, which leads to the following issue:

forgejo forgejo-pre-start[3707]: 2023/10/22 21:03:28 .../setting/security.go:48:loadSecret() [F] Cannot specify both SECRET_KEY_URI and SECRET_KEY
forgejo systemd[1]: forgejo.service: Control process exited, code=exited, status=1/FAILURE
forgejo systemd[1]: forgejo.service: Failed with result 'exit-code'.
forgejo systemd[1]: Failed to start Forgejo (Beyond coding. We forge.).

Then I thought I could override all the secrets with an empty value (lib.mkForce "";). This kind of works - the settings in app.ini will end up without a value. But still the service won't start:

forgejo forgejo-pre-start[6833]: 2023/10/22 21:41:48 ...s/setting/setting.go:111:LoadCommonSettings() [F] Unable to load settings from config: error saving JWT Secret for custom config: failed to save "/var/lib/forgejo/custom/conf/app.ini": open /var/lib/forgejo/custom/conf/app.ini: permission denied
forgejo systemd[1]: forgejo.service: Control process exited, code=exited, status=1/FAILURE
forgejo systemd[1]: forgejo.service: Failed with result 'exit-code'.
forgejo systemd[1]: Failed to start Forgejo (Beyond coding. We forge.).

Here I don't fully understand whats going on. Seems like Forgejo tries to write to app.ini (which obviously isn't possible, because it is read-only). But why does it try to write JWT secret to app.ini?

Steps To Reproduce

Steps to reproduce the behavior:

Set the 4 above mentioned secret URIs in services.forgejo.settings

Expected behavior

It should be possible to provide your own secrets with the URI options. Then the module shouldn't generate them and shouldn't add the secret options (without URI) to app.ini. But this still doesn't solve the "write JWT secret to app.ini" issue...

Notify maintainers

@bendlas @emilylange

Metadata

Note: I'm using Forgejo package from unstable (v1.20.5-0) and the forgejo.nix module from unstable aswell, even though I'm running on nixpkgs 23.05...

- system: `"x86_64-linux"`
- host os: `Linux 6.1.58, NixOS, 23.05 (Stoat), 23.05.20231018.80c1aab`
- multi-user?: `yes`
- sandbox: `yes`
- version: `nix-env (Nix) 2.13.5`
- nixpkgs: `/nix/store/843msk4m4k285za9qy9nhw28pf30za1j-source`

bendlas commented 10 months ago

Hi, good point about being able to use the *_URI variants of secret options, after https://github.com/go-gitea/gitea/issues/25034

As you already noted,this may be at odds with the "write to app.ini" behavior, that gitea displays. About that:

we may actually want to look at accomodating that behavior, instead of trying to avoid: https://github.com/go-gitea/gitea/issues/25377#issuecomment-1609757289 But would be good to get confirmation from upstream first.
that behavior may still cause secret leakage, even if using file based secrets: https://github.com/go-gitea/gitea/issues/19856#issuecomment-1595701236

stfnx commented 10 months ago

Not really solved but still all of those issues are closed. How should we progress now?

Gitea requires that the "app.ini" should be writable by itself, it's not well documented but many functions depend on this fact.

This seems to be a problem, since the Nix store will never be writeable, right? While copying the file to the destination would work, it will be stateful, which we typically don't want in Nix-land.

Anything I can do to bring this forward?

bendlas commented 10 months ago

This seems to be a problem, since the Nix store will never be writeable, right?

It won't be, and cases like this are the exact reason for that. Better to break than to get back into the quagmire.

Last time this broke, we made it so that the app.ini is generated with everything, so that gitea wouldn't try to add to it. This worked, but is brittle. Again, if that quote is true and we should be expecting forgejo to write to app.ini, then it would be better to find a solution, where e.g. ExecStartPre copies over a pristine version before each run.

Keep in mind though, that all of these issues were about gitea, where forgejo is its own product, that will likely become more independent in the future.

So what I would do is to hop over on https://matrix.to/#/#forgejo:matrix.org and see what the forgejo community has to say on that issue.

But unless this turns out to be a priority (which I wouldn't expect, TBH), it's probably best IMO to do the stateful thing, for now. Thoughts @emilylange?

stfnx commented 10 months ago

it's probably best IMO to do the stateful thing, for now.

You mean using the automatically generated secrets instead of providing my own?

bendlas commented 10 months ago

no, I mean to keep app.ini in a writable location (/var/lib/forgejo?) and overwrite it with the latest generated version in ExecStartPre

this is only tangentially related to the secrets issue here, but probably should be solved in preparation, as you noted

emilylange commented 10 months ago

We should probably use the _URI versions in our module instead of the current placeholders with replace-secret. lib.mkDefault-ing would allow users to easily override it, without lib.mkForce and whatnot. I wasn't aware of those _URI settings, thanks for telling me! :)

Please keep in mind, that your /var/lib/forgejo is full of state, and the auto-generated secrets, are within that folder (by default), and even get automatically backed up if you enable services.forgejo.dump.enable (though those dumps might be annoying to restore).

Additionally, this also reminded me that I should really upstream my use of forgejo's contrib/environment-to-ini binary into our nixpkgs module. It is the one that is used within the official container/docker image, to allow the configuration via environment variables.

I use that in my config to provide secret for things like service.HCAPTCHA_SECRET and to redact things like mailer.HOST. For that, I use a single EnvironmentFile passed to systemd. But this could also be extended and used with systemd-creds instead.

emilylange commented 3 months ago

Hi, depending on how you use nix-sops, your issue could be resolved using the #312523.

It's still not really intended to be used that way (lib.mkForce) but it should work as long as the secret is readable by forgejo/cfg.user.

The secrets are read by systemd (root), but the sidecar bootstrap service runs as forgejo and wants to check whether those automatically bootstrapped secret exist first (to bootstrap them if necessary).

We could fix that though. If there is need.

But as mentioned one of the commit messages: Overriding the bootstrapped secrets is now "more possible than before".

That secret bootstrap unit could be adjusted to run as root without any issues.

I personally still don't get why you would want to override those automatically bootstrapped secrets, since they are state in /var/lib/forgejo just like your git repositories, uploads, etc., but oh well.

Let me know what you think :)

NixOS / nixpkgs