From f7baa65db75b32db49894296646a1b9b74115482 Mon Sep 17 00:00:00 2001
From: Lin Jian <me@linj.tech>
Date: Sun, 26 Jun 2022 13:19:10 +0800
Subject: [PATCH] nixos/caddy: improve security about acme certs

Before this patch, the caddy process has acme in its supplementary
group because of the SupplementaryGroups in its service config, which
may give it more permission than needed, is inconsistent with the
documentation of services.caddy.virtualHosts.<name>.useACMEHost and is
redundant since we have mkCertOwnershipAssertion in assertions.

This patch fixes these problems by defaulting the group of needed
certs to caddy, which is what other web servers like nginx do and
deleting SupplementaryGroups config.
---
 nixos/modules/services/web-servers/caddy/default.nix      | 8 +++++---
 .../modules/services/web-servers/caddy/vhost-options.nix  | 4 +---
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/nixos/modules/services/web-servers/caddy/default.nix b/nixos/modules/services/web-servers/caddy/default.nix
index 2b8c6f2e308..bcf90e30fe8 100644
--- a/nixos/modules/services/web-servers/caddy/default.nix
+++ b/nixos/modules/services/web-servers/caddy/default.nix
@@ -308,7 +308,6 @@ in
         StateDirectory = mkIf (cfg.dataDir == "/var/lib/caddy") [ "caddy" ];
         LogsDirectory = mkIf (cfg.logDir == "/var/log/caddy") [ "caddy" ];
         Restart = "on-abnormal";
-        SupplementaryGroups = mkIf (length acmeVHosts != 0) [ "acme" ];
 
         # TODO: attempt to upstream these options
         NoNewPrivileges = true;
@@ -331,9 +330,12 @@ in
 
     security.acme.certs =
       let
-        reloads = map (useACMEHost: nameValuePair useACMEHost { reloadServices = [ "caddy.service" ]; }) acmeHosts;
+        certCfg = map (useACMEHost: nameValuePair useACMEHost {
+          group = mkDefault cfg.group;
+          reloadServices = [ "caddy.service" ];
+        }) acmeHosts;
       in
-        listToAttrs reloads;
+        listToAttrs certCfg;
 
   };
 }
diff --git a/nixos/modules/services/web-servers/caddy/vhost-options.nix b/nixos/modules/services/web-servers/caddy/vhost-options.nix
index f240ec605c2..668f17ea340 100644
--- a/nixos/modules/services/web-servers/caddy/vhost-options.nix
+++ b/nixos/modules/services/web-servers/caddy/vhost-options.nix
@@ -40,9 +40,7 @@ in
 
         <emphasis>Note that this option does not create any certificates, nor
         does it add subdomains to existing ones – you will need to create them
-        manually using <xref linkend="opt-security.acme.certs"/>. Additionally,
-        you should probably add the <literal>caddy</literal> user to the
-        <literal>acme</literal> group to grant access to the certificates.</emphasis>
+        manually using <xref linkend="opt-security.acme.certs"/>.</emphasis>
       '';
     };