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.

Sign up for GitHub

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

Jump to bottom

{apache,caddy,nginx}: not "before" ACME certs using DNS validation #336412

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

{apache,caddy,nginx}: not "before" ACME certs using DNS validation #336412

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ee13a92
nixos/apache: not "before" ACME certs using DNS validation
ThinkChaos Aug 21, 2024
fa20be9
nixos/caddy: not "before" ACME certs using DNS validation
ThinkChaos Aug 21, 2024
5354cc5
nixos/nginx: not "before" ACME certs using DNS validation
ThinkChaos Aug 21, 2024
1796dae
nixos/web-servers: assert ACME cert access via service user and groups
ThinkChaos Aug 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 20 additions & 3 deletions nixos/modules/security/acme/mk-cert-ownership-assertion.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,21 @@
{ cert, group, groups, user }: {
assertion = cert.group == group || builtins.any (u: u == user) groups.${cert.group}.members;
message = "Group for certificate ${cert.domain} must be ${group}, or user ${user} must be a member of group ${cert.group}";
lib:

{ cert, groups, services }:
let
catSep = builtins.concatStringsSep;

svcGroups = svc:
(lib.optional (svc.serviceConfig ? Group) svc.serviceConfig.Group)
++ (svc.serviceConfig.SupplementaryGroups or [ ]);
in
{
assertion = builtins.all (svc:
svc.serviceConfig.User or "root" == "root"
|| builtins.elem svc.serviceConfig.User groups.${cert.group}.members
|| builtins.elem cert.group (svcGroups svc)
) services;

message = "Certificate ${cert.domain} (group=${cert.group}) must be readable by service(s) ${
catSep ", " (map (svc: "${svc.name} (user=${svc.serviceConfig.User} groups=${catSep " " (svcGroups svc)})") services)
}";
}
21 changes: 11 additions & 10 deletions nixos/modules/services/web-servers/apache-httpd/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ let
certName = if hostOpts.useACMEHost != null then hostOpts.useACMEHost else hostOpts.hostName;
}) (filter (hostOpts: hostOpts.enableACME || hostOpts.useACMEHost != null) vhosts);

dependentCertNames = unique (map (hostOpts: hostOpts.certName) acmeEnabledVhosts);
vhostCertNames = unique (map (hostOpts: hostOpts.certName) acmeEnabledVhosts);
dependentCertNames = filter (cert: certs.${cert}.dnsProvider == null) vhostCertNames; # those that might depend on the HTTP server

mkListenInfo = hostOpts:
if hostOpts.listen != [] then
Expand Down Expand Up @@ -371,7 +372,7 @@ let
echo "$options" >> $out
'';

mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix;
mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix lib;
in


Expand Down Expand Up @@ -641,10 +642,10 @@ in
'';
}
] ++ map (name: mkCertOwnershipAssertion {
inherit (cfg) group user;
cert = config.security.acme.certs.${name};
groups = config.users.groups;
}) dependentCertNames;
services = [ config.systemd.services.httpd ] ++ lib.optional (vhostCertNames != []) config.systemd.services.httpd-config-reload;
}) vhostCertNames;

warnings =
mapAttrsToList (name: hostOpts: ''
Expand Down Expand Up @@ -747,8 +748,8 @@ in
systemd.services.httpd = {
description = "Apache HTTPD";
wantedBy = [ "multi-user.target" ];
wants = concatLists (map (certName: [ "acme-finished-${certName}.target" ]) dependentCertNames);
after = [ "network.target" ] ++ map (certName: "acme-selfsigned-${certName}.service") dependentCertNames;
wants = concatLists (map (certName: [ "acme-finished-${certName}.target" ]) vhostCertNames);
after = [ "network.target" ] ++ map (certName: "acme-selfsigned-${certName}.service") vhostCertNames;
before = map (certName: "acme-${certName}.service") dependentCertNames;
restartTriggers = [ cfg.configFile ];

Expand Down Expand Up @@ -789,9 +790,9 @@ in
# which allows the acme-finished-$cert.target to signify the successful updating
# of certs end-to-end.
systemd.services.httpd-config-reload = let
sslServices = map (certName: "acme-${certName}.service") dependentCertNames;
sslTargets = map (certName: "acme-finished-${certName}.target") dependentCertNames;
in mkIf (sslServices != []) {
sslServices = map (certName: "acme-${certName}.service") vhostCertNames;
sslTargets = map (certName: "acme-finished-${certName}.target") vhostCertNames;
in mkIf (vhostCertNames != []) {
wantedBy = sslServices ++ [ "multi-user.target" ];
# Before the finished targets, after the renew services.
# This service might be needed for HTTP-01 challenges, but we only want to confirm
Expand All @@ -801,7 +802,7 @@ in
restartTriggers = [ cfg.configFile ];
# Block reloading if not all certs exist yet.
# Happens when config changes add new vhosts/certs.
unitConfig.ConditionPathExists = map (certName: certs.${certName}.directory + "/fullchain.pem") dependentCertNames;
unitConfig.ConditionPathExists = map (certName: certs.${certName}.directory + "/fullchain.pem") vhostCertNames;
serviceConfig = {
Type = "oneshot";
TimeoutSec = 60;
Expand Down
23 changes: 12 additions & 11 deletions nixos/modules/services/web-servers/caddy/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,11 @@ with lib;
let
cfg = config.services.caddy;

certs = config.security.acme.certs;
virtualHosts = attrValues cfg.virtualHosts;
acmeVHosts = filter (hostOpts: hostOpts.useACMEHost != null) virtualHosts;
acmeEnabledVhosts = filter (hostOpts: hostOpts.useACMEHost != null) virtualHosts;
vhostCertNames = unique (map (hostOpts: hostOpts.useACMEHost) acmeEnabledVhosts);
dependentCertNames = filter (cert: certs.${cert}.dnsProvider == null) vhostCertNames; # those that might depend on the HTTP server

mkVHostConf = hostOpts:
let
Expand Down Expand Up @@ -51,9 +54,7 @@ let

configPath = "/etc/${etcConfigFile}";

acmeHosts = unique (catAttrs "useACMEHost" acmeVHosts);

mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix;
mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix lib;
in
{
imports = [
Expand Down Expand Up @@ -329,10 +330,10 @@ in
message = "To specify an adapter other than 'caddyfile' please provide your own configuration via `services.caddy.configFile`";
}
] ++ map (name: mkCertOwnershipAssertion {
inherit (cfg) group user;
cert = config.security.acme.certs.${name};
groups = config.users.groups;
}) acmeHosts;
services = [ config.systemd.services.caddy ];
}) vhostCertNames;

services.caddy.globalConfig = ''
${optionalString (cfg.email != null) "email ${cfg.email}"}
Expand All @@ -348,9 +349,9 @@ in

systemd.packages = [ cfg.package ];
systemd.services.caddy = {
wants = map (hostOpts: "acme-finished-${hostOpts.useACMEHost}.target") acmeVHosts;
after = map (hostOpts: "acme-selfsigned-${hostOpts.useACMEHost}.service") acmeVHosts;
before = map (hostOpts: "acme-${hostOpts.useACMEHost}.service") acmeVHosts;
wants = map (certName: "acme-finished-${certName}.target") vhostCertNames;
after = map (certName: "acme-selfsigned-${certName}.service") vhostCertNames;
before = map (certName: "acme-${certName}.service") dependentCertNames;

wantedBy = [ "multi-user.target" ];
startLimitIntervalSec = 14400;
Expand Down Expand Up @@ -396,10 +397,10 @@ in

security.acme.certs =
let
certCfg = map (useACMEHost: nameValuePair useACMEHost {
certCfg = map (certName: nameValuePair certName {
group = mkDefault cfg.group;
reloadServices = [ "caddy.service" ];
}) acmeHosts;
}) vhostCertNames;
in
listToAttrs certCfg;

Expand Down
21 changes: 11 additions & 10 deletions nixos/modules/services/web-servers/nginx/default.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ let
inherit (config.security.acme) certs;
vhostsConfigs = mapAttrsToList (vhostName: vhostConfig: vhostConfig) virtualHosts;
acmeEnabledVhosts = filter (vhostConfig: vhostConfig.enableACME || vhostConfig.useACMEHost != null) vhostsConfigs;
dependentCertNames = unique (map (hostOpts: hostOpts.certName) acmeEnabledVhosts);
vhostCertNames = unique (map (hostOpts: hostOpts.certName) acmeEnabledVhosts);
dependentCertNames = filter (cert: certs.${cert}.dnsProvider == null) vhostCertNames; # those that might depend on the HTTP server
virtualHosts = mapAttrs (vhostName: vhostConfig:
let
serverName = if vhostConfig.serverName != null
Expand Down Expand Up @@ -474,7 +475,7 @@ let
'') authDef)
);

mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix;
mkCertOwnershipAssertion = import ../../../security/acme/mk-cert-ownership-assertion.nix lib;

oldHTTP2 = (versionOlder cfg.package.version "1.25.1" && !(cfg.package.pname == "angie" || cfg.package.pname == "angieQuic"));
in
Expand Down Expand Up @@ -1204,10 +1205,10 @@ in
'';
}
] ++ map (name: mkCertOwnershipAssertion {
inherit (cfg) group user;
cert = config.security.acme.certs.${name};
groups = config.users.groups;
}) dependentCertNames;
services = [ config.systemd.services.nginx ] ++ lib.optional (cfg.enableReload || vhostCertNames != []) config.systemd.services.nginx-config-reload;
}) vhostCertNames;

services.nginx.additionalModules = optional cfg.recommendedBrotliSettings pkgs.nginxModules.brotli
++ lib.optional cfg.recommendedZstdSettings pkgs.nginxModules.zstd;
Expand All @@ -1230,8 +1231,8 @@ in
systemd.services.nginx = {
description = "Nginx Web Server";
wantedBy = [ "multi-user.target" ];
wants = concatLists (map (certName: [ "acme-finished-${certName}.target" ]) dependentCertNames);
after = [ "network.target" ] ++ map (certName: "acme-selfsigned-${certName}.service") dependentCertNames;
wants = concatLists (map (certName: [ "acme-finished-${certName}.target" ]) vhostCertNames);
after = [ "network.target" ] ++ map (certName: "acme-selfsigned-${certName}.service") vhostCertNames;
# Nginx needs to be started in order to be able to request certificates
# (it's hosting the acme challenge after all)
# This fixes https://github.com/NixOS/nixpkgs/issues/81842
Expand Down Expand Up @@ -1310,9 +1311,9 @@ in
# which allows the acme-finished-$cert.target to signify the successful updating
# of certs end-to-end.
systemd.services.nginx-config-reload = let
sslServices = map (certName: "acme-${certName}.service") dependentCertNames;
sslTargets = map (certName: "acme-finished-${certName}.target") dependentCertNames;
in mkIf (cfg.enableReload || sslServices != []) {
sslServices = map (certName: "acme-${certName}.service") vhostCertNames;
sslTargets = map (certName: "acme-finished-${certName}.target") vhostCertNames;
in mkIf (cfg.enableReload || vhostCertNames != []) {
wants = optionals cfg.enableReload [ "nginx.service" ];
wantedBy = sslServices ++ [ "multi-user.target" ];
# Before the finished targets, after the renew services.
Expand All @@ -1323,7 +1324,7 @@ in
restartTriggers = optionals cfg.enableReload [ configFile ];
# Block reloading if not all certs exist yet.
# Happens when config changes add new vhosts/certs.
unitConfig.ConditionPathExists = optionals (sslServices != []) (map (certName: certs.${certName}.directory + "/fullchain.pem") dependentCertNames);
unitConfig.ConditionPathExists = optionals (sslServices != []) (map (certName: certs.${certName}.directory + "/fullchain.pem") vhostCertNames);
serviceConfig = {
Type = "oneshot";
TimeoutSec = 60;
Expand Down