www/caddy: Fix setup.sh script interaction with files and directories in caddy storage (#4911)

* www/caddy: Fix setup.sh script interaction with files and directories in caddy storage

This fixes multiple things:
- When running as www:www user, the interaction with the admin socket could fail, now we do not touch /var/run/caddy and let it be handled by the permissions set in the rc.d script
- When restarting/reloading caddy, permissions and ownerships would be changed every time, possibly breaking the storage if caddy writes at the same time
- The custom certificates are now stored outside the scope of the caddy storage, ensuring caddy has atomic write guarantee on /var/db/caddy/data...

* Fix some review comments

* add changelog

www/caddy/pkg-descr

@@ -9,8 +9,9 @@ Plugin Changelog
 2.0.3
 
 Add: Tabulator groupBy of domain and subdomain (opnsense/plugins/pull/4909)
-Fix: Tabulator 'Data Load Response Blocked' warning
 Cleanup: Grid HTML and style
+Fix: Tabulator 'Data Load Response Blocked' warning
+Fix: setup.sh interaction with caddy storage and permissions (opnsense/plugins/pull/4911)
 
 2.0.2
 

www/caddy/src/opnsense/scripts/OPNsense/Caddy/caddy_certs.php

@@ -43,7 +43,7 @@ $writeFileIfChanged = function ($filePath, $content) {
     }
 };
 
-$tempDir = '/var/db/caddy/data/caddy/certificates/temp/';
+$tempDir = '/usr/local/etc/caddy/certificates';
 
 // leaf certificate chain
 $certificateRefs = [];

www/caddy/src/opnsense/scripts/OPNsense/Caddy/setup.sh

@@ -1,7 +1,7 @@
 #!/bin/sh
 
 #
-# Copyright (c) 2023-2024 Cedrik Pischem
+# Copyright (c) 2023-2025 Cedrik Pischem
 # All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without modification,
@@ -26,46 +26,52 @@
 # POSSIBILITY OF SUCH DAMAGE.
 #
 
-# The directories are created as root:www with rwx permissions for both,
-# so the user can change in the GUI if caddy runs as root or www
-# If only ports 1024 and above are used, caddy can run as www user and group.
+# Detect configured caddy_user/group (defaults)
+. /etc/rc.conf.d/caddy
+CADDY_USER="${caddy_user:-root}"
+CADDY_GROUP="${caddy_group:-wheel}"
+
+# Canary to detect root->www switch (disable superuser) permission issues
+# The storage instance will always exist, it's a good assumption
+CANARY="/var/db/caddy/data/caddy/instance.uuid"
 
 # Define directories
 CADDY_CONF_DIR="/usr/local/etc/caddy"
 CADDY_DATA_DIR="/var/db/caddy"
 CADDY_LOG_DIR="/var/log/caddy"
-CADDY_RUN_DIR="/var/run/caddy"
 CADDY_CONF_CUSTOM_DIR="${CADDY_CONF_DIR}/caddy.d"
-CADDY_DATA_CUSTOM_DIR="${CADDY_DATA_DIR}/data/caddy/certificates/temp"
+CADDY_CONF_CERT_DIR="${CADDY_CONF_DIR}/certificates"
 CADDY_LOG_CUSTOM_DIR="${CADDY_LOG_DIR}/access"
 
-mkdir -p "${CADDY_CONF_DIR}"
-mkdir -p "${CADDY_DATA_DIR}"
-mkdir -p "${CADDY_LOG_DIR}"
-mkdir -p "${CADDY_RUN_DIR}"
-mkdir -p "${CADDY_CONF_CUSTOM_DIR}"
-mkdir -p "${CADDY_DATA_CUSTOM_DIR}"
-mkdir -p "${CADDY_LOG_CUSTOM_DIR}"
+mkdir -p "${CADDY_CONF_DIR}" \
+         "${CADDY_DATA_DIR}" \
+         "${CADDY_LOG_DIR}" \
+         "${CADDY_CONF_CUSTOM_DIR}" \
+         "${CADDY_CONF_CERT_DIR}" \
+         "${CADDY_LOG_CUSTOM_DIR}"
 
-chown -R root:www "${CADDY_CONF_DIR}"
-chown -R root:www "${CADDY_DATA_DIR}"
-chown -R root:www "${CADDY_LOG_DIR}"
-chown -R root:www "${CADDY_RUN_DIR}"
+# Format and overwrite the Caddyfile
+( cd "${CADDY_CONF_DIR}" && /usr/local/bin/caddy fmt --overwrite )
 
-# Directories need execute permissions to be accessible
-find "${CADDY_CONF_DIR}" -type d -exec chmod 770 {} +
-find "${CADDY_DATA_DIR}" -type d -exec chmod 770 {} +
-find "${CADDY_LOG_DIR}" -type d -exec chmod 770 {} +
-find "${CADDY_RUN_DIR}" -type d -exec chmod 770 {} +
-
-# Files can have read/write permissions
-find "${CADDY_CONF_DIR}" -type f -exec chmod 660 {} +
-find "${CADDY_DATA_DIR}" -type f -exec chmod 660 {} +
-find "${CADDY_LOG_DIR}" -type f -exec chmod 660 {} +
-find "${CADDY_RUN_DIR}" -type f -exec chmod 660 {} +
-
-# Format and overwrite the Caddyfile, this makes whitespace control in jinja2 unnecessary
-(cd "${CADDY_CONF_DIR}" && /usr/local/bin/caddy fmt --overwrite)
-
-# Write custom certs from the OPNsense Trust Store to CADDY_DATA_CUSTOM_DIR
+# Write custom certs from the OPNsense Trust Store
 /usr/local/opnsense/scripts/OPNsense/Caddy/caddy_certs.php
+
+# Ownership decision based on current service user/group, otherwise skip
+EXPECTED_USER="$CADDY_USER"
+EXPECTED_GROUP="$CADDY_GROUP"
+
+if [ -f "$CANARY" ]; then
+    CANARY_USER="$(stat -f '%Su' "$CANARY")"
+    CANARY_GROUP="$(stat -f '%Sg' "$CANARY")"
+
+    if [ "$CANARY_USER" = "$EXPECTED_USER" -a "$CANARY_GROUP" = "$EXPECTED_GROUP" ]; then
+        exit 0
+    fi
+fi
+
+# Use detected service user/group, only migrate ownership
+# We only interact with the storage in this specific edge case, in all other cases caddy must have atomic write guarantee
+chown -R "${CADDY_USER}:${CADDY_GROUP}" "${CADDY_CONF_DIR}" \
+                                        "${CADDY_DATA_DIR}" \
+                                        "${CADDY_LOG_DIR}" \
+                                        "${CADDY_CONF_CERT_DIR}"

www/caddy/src/opnsense/service/templates/OPNsense/Caddy/Caddyfile

@@ -312,7 +312,7 @@ http://{{ domain }} {
     tlsDnsPropagationResolvers=""
 ) %}
     {% if customCert or (dnsChallenge == "1" and dnsProvider) or clientAuthTrustPool %}
-        tls {% if customCert %}/var/db/caddy/data/caddy/certificates/temp/{{ customCert }}.pem /var/db/caddy/data/caddy/certificates/temp/{{ customCert }}.key{% endif %} {
+        tls {% if customCert %}/usr/local/etc/caddy/certificates/{{ customCert }}.pem /usr/local/etc/caddy/certificates/{{ customCert }}.key{% endif %} {
             {% if not customCert and (dnsChallenge == "1" and dnsProvider) %}
             issuer acme {
                 dns {{ dnsProvider }} {{ dnsApiKey }}
@@ -334,7 +334,7 @@ http://{{ domain }} {
             {% if clientAuthTrustPool %}
                 client_auth {
                     {% for ca in clientAuthTrustPool.split(',') %}
-                    trust_pool file /var/db/caddy/data/caddy/certificates/temp/{{ ca.strip() }}.pem
+                    trust_pool file /usr/local/etc/caddy/certificates/{{ ca.strip() }}.pem
                     {% endfor %}
                     {% if clientAuthMode %}
                     mode {{ clientAuthMode }}
@@ -508,7 +508,7 @@ http://{{ domain }} {
                             tls_insecure_skip_verify
                         {% endif %}
                         {% if handle.HttpTlsTrustedCaCerts %}
-                            tls_trust_pool file /var/db/caddy/data/caddy/certificates/temp/{{ handle.HttpTlsTrustedCaCerts }}.pem
+                            tls_trust_pool file /usr/local/etc/caddy/certificates/{{ handle.HttpTlsTrustedCaCerts }}.pem
                         {% endif %}
                         {% if handle.HttpTlsServerName %}
                             tls_server_name {{ handle.HttpTlsServerName }}

www/caddy/src/opnsense/service/templates/OPNsense/Caddy/includeLayer4

@@ -99,7 +99,7 @@
                 {% if layer4.FromOpenvpnStaticKey %}
                     group_key_direction {{ direction }}
                     {% for key_uuid in key_list %}
-                    group_key_file /var/db/caddy/data/caddy/certificates/temp/{{ key_uuid.strip() }}.key
+                    group_key_file /usr/local/etc/caddy/certificates/{{ key_uuid.strip() }}.key
                     {% endfor %}
                 {% endif %}
                 }
@@ -111,7 +111,7 @@
                     modes crypt
                 {% if layer4.FromOpenvpnStaticKey %}
                     {% for key_uuid in key_list %}
-                    group_key_file /var/db/caddy/data/caddy/certificates/temp/{{ key_uuid.strip() }}.key
+                    group_key_file /usr/local/etc/caddy/certificates/{{ key_uuid.strip() }}.key
                     {% endfor %}
                 {% endif %}
                 }
@@ -121,7 +121,7 @@
                     modes crypt2
                     {% if layer4.FromOpenvpnStaticKey %}
                         {% for key_uuid in key_list %}
-                    client_key_file /var/db/caddy/data/caddy/certificates/temp/{{ key_uuid.strip() }}.key
+                    client_key_file /usr/local/etc/caddy/certificates/{{ key_uuid.strip() }}.key
                         {% endfor %}
                     {% endif %}
                 }
@@ -133,7 +133,7 @@
                     modes crypt2
                     {% if layer4.FromOpenvpnStaticKey %}
                         {% for key_uuid in key_list %}
-                    server_key_file /var/db/caddy/data/caddy/certificates/temp/{{ key_uuid.strip() }}.key
+                    server_key_file /usr/local/etc/caddy/certificates/{{ key_uuid.strip() }}.key
                         {% endfor %}
                     {% endif %}
                 }