!MthpOIxqJhTgrMNxDS:nixos.org

NixOS ACME / LetsEncrypt

106 Members
Another day, another cert renewal46 Servers

Load older messages


SenderMessageTime
5 Mar 2022
@m1cr0man:m1cr0man.comm1cr0man

Heyo 👋 Sorry got distracted and forgot to reply earlier. Heading off but I'll read any replies tomorrow.

Yeah this is interesting. SupplementalGroups certainly would raise false alarms with the assertion the way it is. When you say make the change across the board, what are you thinking of doing?

I'm also thinking that depending on your plans here that assuming the cert's group is acme wouldn't be sufficient and you'd want to rely on config.security.acme.certs.<name>.group in dependant services.

00:43:44
@winterqt:nixos.devWinter (she/her)

When you say make the change across the board, what are you thinking of doing?

Migrating all web servers that we support to use it instead of the assertions, ideally.

02:48:20
@m1cr0man:m1cr0man.comm1cr0man

Yeah honestly I think that would be a good idea :) There will be some things to note however.

Firstly, we have weak values for group set on a cert used by nginx/httpd (example:
https://github.com/m1cr0man/nixpkgs/blob/674cfc91c7432662fc8ab96a6d17819f5517ddb8/nixos/modules/services/web-servers/nginx/default.nix#L967). It _might_ be necessary to check that the user/group for the web server isn't already in the cert's group, however knowing Systemd if you specify SupplementalGroups the user already has it'll probably no-op and be grand.

Secondly there was in the past some concern raised around granting acme group to other services because it would grant that service access to more certs than you may want. You might get some backlash in that regard. In reality, this is hard to operate around and for wildcard certs you're likely to only have 1 cert shared across multiple services anyway.

Lastly there was still some cases where people/services wanted root as the owner and before the useRoot option was added to acme, LoadCredential was the only solution here: https://github.com/NixOS/nixpkgs/pull/123261 (WOW just noticed this hasn't been merged). I bring this up because LoadCredential would also be a valid solution instead of SupplementalGroups, but because credentials are not re-read from disk when they change which is bad for ACME usage, I don't think it's preferred.

14:45:35
@m1cr0man:m1cr0man.comm1cr0manPoint 2 is really why your assertion was acceptable in the first place. We're letting users know that the permissions are incorrect and they have to decide how to solve it, rather than us just blanket-granting access to certs which may or may not be what the user expects14:46:52
@winterqt:nixos.devWinter (she/her)

Riiight, completely forgot about that.

I think the best thing to do here is to revisit how the Caddy module operates in this regard -- so removing the blanket "acme" group addition. (Since I'm not sure the best way to do this, would it be appropriate to open an issue to discuss it with the module maintainer?)

17:36:06
@m1cr0man:m1cr0man.comm1cr0manYeah that's probably best, and so that it's on record on Github too17:44:17
@winterqt:nixos.devWinter (she/her)
In reply to @m1cr0man:m1cr0man.com
Yeah that's probably best, and so that it's on record on Github too
Would it be appropriate to label the issue as a bug?
19:34:00
@winterqt:nixos.devWinter (she/her)(don't wanna open an issue with no label idk)19:34:07
@winterqt:nixos.devWinter (she/her)i think so19:34:22
@m1cr0man:m1cr0man.comm1cr0manhah uh idk what label to use honestly 😅 I think it's more just discussion atm, nothing is wrong per se19:34:33
@m1cr0man:m1cr0man.comm1cr0man * hah uh idk what label to use honestly 😅 I think it's more just discussion/suggestion atm, nothing is wrong per se19:34:40
@winterqt:nixos.devWinter (she/her)true19:34:43
@winterqt:nixos.devWinter (she/her)yeah ill do no label19:34:53
@winterqt:nixos.devWinter (she/her) m1cr0man: am i just blind, or is the group option for not defined in certOpts? 19:51:09
@winterqt:nixos.devWinter (she/her) * m1cr0man: am i just blind, or is the group option not defined in certOpts? 19:51:14
@m1cr0man:m1cr0man.comm1cr0manit's defined in the inheritableModule thing19:51:23
@winterqt:nixos.devWinter (she/her)oh19:51:53
@winterqt:nixos.devWinter (she/her)
      group = mkOption {
        type = types.str;
        inherit (defaultAndText "group" "acme") default defaultText;
        description = "Group running the ACME client.";
      };

i feel like this description is inaccurate?

19:52:02
@winterqt:nixos.devWinter (she/her)oh nevermind19:52:24
@winterqt:nixos.devWinter (she/her)guess its not19:52:27
@winterqt:nixos.devWinter (she/her)
        # Group might change between runs, re-apply it
        chown '${user}:${data.group}' certificates/*

hm

19:52:57
@m1cr0man:m1cr0man.comm1cr0manyeah that's 100% necessary19:53:11
@m1cr0man:m1cr0man.comm1cr0manran into it myself and covered by the test suite19:53:19
@winterqt:nixos.devWinter (she/her)so is that if the certificate doest have to be renewed, but the group changed?19:53:33
@winterqt:nixos.devWinter (she/her) * so is that for if the certificate doesn't have to be renewed, but the group changed?19:53:41
@m1cr0man:m1cr0man.comm1cr0manthat description might be a bit misleading I agree. It shuold maybe indicate that group will own the certs19:53:41
@m1cr0man:m1cr0man.comm1cr0manyeah exacrly19:53:46
@winterqt:nixos.devWinter (she/her)got it19:53:48
@m1cr0man:m1cr0man.comm1cr0man * yeah exactly19:53:49
@winterqt:nixos.devWinter (she/her)

Secondly there was in the past some concern raised around granting acme group to other services because it would grant that service access to more certs than you may want. You might get some backlash in that regard. In reality, this is hard to operate around and for wildcard certs you're likely to only have 1 cert shared across multiple services anyway.

so the thing about this point is that, like, if you set a specific group for a cert (that isn't acme), then its not like granting the acme group will give you access to those...

19:54:47

Show newer messages


Back to Room ListRoom Version: 6