Robust wildcard certificate naming #3326

src/common/core/letsencrypt/jobs/certbot-new.py

@@ -89,6 +89,11 @@
 LOGGER_CERTBOT = getLogger("LETS-ENCRYPT.NEW.CERTBOT")
 
 ZEROSSL_API_KEY_HASHES_PATH = DATA_PATH.joinpath("renewal", ".bw-zerossl-api-key-hashes.json")
+# Wildcard certs are stored under a distinct cert_name so they don't collide with
+# non-wildcard certs for the same base (e.g. hosts.example.com). This keeps the
+# filesystem layout explicit and makes UI deletion/debugging easier. "_wildcard_"
+# is not a valid hostname, so there will not be any overlaps.
+WILDCARD_CERT_NAME_PREFIX = "_wildcard_."
 MERGE_LOCK = Lock()
 RUNNING_LOCK = Lock()
 RUNNING_CERTBOT = 0
@@ -590,13 +595,16 @@ def build_service_entries(service: str) -> Dict[str, Dict[str, Union[str, bool,
 
     entries: Dict[str, Dict[str, Union[str, bool, int, Dict[str, str]]]] = {}
     if base_config["wildcard"]:
+        # Wildcard mode is "one cert per wildcard scope" (base). We intentionally store it under
+        # _wildcard_.<base> so it doesn't collide with a non-wildcard cert for <base>.
         wildcard_groups = extract_wildcard_groups(list(unique_names))
         if not wildcard_groups and base_config["activated"]:
             LOGGER.warning(f"[Service: {service}] No valid wildcard groups found, skipping generation.")
         for base, names in wildcard_groups.items():
             config = base_config.copy()
             config["server_names"] = ",".join(names)
-            entries[base] = config
+            cert_name = f"{WILDCARD_CERT_NAME_PREFIX}{base}"
+            entries[cert_name] = config
         return entries
 
     config = base_config.copy()
@@ -606,13 +614,18 @@ def build_service_entries(service: str) -> Dict[str, Dict[str, Union[str, bool,
 
 
 def _determine_wildcard_bases(labels_list: List[List[str]]) -> Set[str]:
+    """
+    Determine the base domain(s) for wildcard cert scope. For *.hosts.example.com the base
+    is hosts.example.com (cert name and coverage); for *.example.com the base is example.com.
+    """
     if not labels_list:
         return set()
 
     if len(labels_list) == 1:
         labels = labels_list[0]
-        if len(labels) > 2:
-            return {".".join(labels[1:])}
+        # Base = full cleaned domain.
+        # This fixes the common pitfall where "*.hosts.example.com" would otherwise incorrectly
+        # produce a base of "example.com" (which does not cover *.hosts.example.com).
         return {".".join(labels)}
 
     min_len = min(len(labels) for labels in labels_list)
@@ -625,17 +638,17 @@ def _determine_wildcard_bases(labels_list: List[List[str]]) -> Set[str]:
         else:
             break
 
-    if len(common_suffix) >= 2 and len(common_suffix) >= (min_len - 1):
-        return {".".join(common_suffix)}
-
-    bases: Set[str] = set()
-    for labels in labels_list:
-        if len(labels) > 2:
-            bases.add(".".join(labels[1:]))
-        else:
-            bases.add(".".join(labels))
+    # One base only when all entries are the same scope (e.g. *.example.com + example.com)
+    common_base = ".".join(common_suffix)
+    if len(common_suffix) >= 2 and len(common_suffix) >= (min_len - 1) and all(
+        ".".join(labels) == common_base for labels in labels_list
+    ):
+        return {common_base}
 
-    return bases
+    # Different scopes (e.g. *.hosts.example.com vs *.api.example.com) must not be merged into
+    # a broader "*.example.com" cert, because that would be a different scope than requested.
+    # We therefore emit one base per scope.
+    return {".".join(labels) for labels in labels_list}
 
 
 def certbot_delete(service: str, cmd_env: Dict[str, str] = None) -> int:

src/common/core/letsencrypt/letsencrypt.lua

@@ -35,6 +35,32 @@ local lower = string.lower
 local gsub = string.gsub
 
 -- Mirror certbot-new wildcard grouping so certificate identifiers stay in sync.
+-- Cert name prefix for wildcard certs (renewal/live/archive dirs), must match certbot-new.py.
+-- "_wildcard_" is not a valid hostname, so there will not be any overlaps.
+local WILDCARD_CERT_NAME_PREFIX = "_wildcard_."
+local LETSENCRYPT_LIVE = "/var/cache/bunkerweb/letsencrypt/etc/live/"
+
+-- Try reading wildcard cert files for a wildcard base.
+-- Certbot may suffix the cert_name based on key type/profile (-ecdsa/-rsa), so we attempt
+-- the unsuffixed dir first for backward compatibility, then the suffixed variants.
+local function read_wildcard_cert_files(base)
+	local cert_dir_base = WILDCARD_CERT_NAME_PREFIX .. base
+	local suffixes = { "", "-ecdsa", "-rsa" }
+	local last_err
+	for _, suffix in ipairs(suffixes) do
+		local cert_dir = cert_dir_base .. suffix
+		local check, data = read_files({
+			LETSENCRYPT_LIVE .. cert_dir .. "/fullchain.pem",
+			LETSENCRYPT_LIVE .. cert_dir .. "/privkey.pem",
+		})
+		if check then
+			return true, data
+		end
+		last_err = data
+	end
+	return false, last_err
+end
+
 local function sanitize_domain_labels(domain)
 	if not domain or domain == "" then
 		return nil
@@ -60,9 +86,7 @@ local function determine_wildcard_bases(labels_list)
 	end
 	if count == 1 then
 		local labels = labels_list[1]
-		if #labels > 2 then
-			return { table.concat(labels, ".", 2) }
-		end
+		-- Base = full cleaned domain so *.hosts.example.com -> hosts.example.com
 		return { table.concat(labels, ".") }
 	end
 	local min_len = #labels_list[1]
@@ -86,18 +110,26 @@ local function determine_wildcard_bases(labels_list)
 		end
 		insert(common_suffix, 1, label)
 	end
+	-- One base only when all entries are the same scope (e.g. *.example.com + example.com)
+	local common_base = table.concat(common_suffix, ".")
 	if #common_suffix >= 2 and #common_suffix >= (min_len - 1) then
-		return { table.concat(common_suffix, ".") }
+		local all_same = true
+		for _, labels in ipairs(labels_list) do
+			if table.concat(labels, ".") ~= common_base then
+				all_same = false
+				break
+			end
+		end
+		if all_same then
+			return { common_base }
+		end
 	end
+	-- Different scopes (e.g. *.hosts.example.com vs *.api.example.com) must not be merged;
+	-- emit one base per scope.
 	local bases = {}
 	local seen = {}
 	for _, labels in ipairs(labels_list) do
-		local base
-		if #labels > 2 then
-			base = table.concat(labels, ".", 2)
-		else
-			base = table.concat(labels, ".")
-		end
+		local base = table.concat(labels, ".")
 		if base ~= "" and not seen[base] then
 			seen[base] = true
 			insert(bases, base)
@@ -263,12 +295,9 @@ function letsencrypt:init()
 							data = self.internalstore:get("plugin_letsencrypt_" .. base, true)
 							if not data then
 								local check
-								check, data = read_files({
-									"/var/cache/bunkerweb/letsencrypt/etc/live/" .. base .. "/fullchain.pem",
-									"/var/cache/bunkerweb/letsencrypt/etc/live/" .. base .. "/privkey.pem",
-								})
+								check, data = read_wildcard_cert_files(base)
 								if not check then
-									self.logger:log(ERR, "error while reading files : " .. data)
+									self.logger:log(ERR, "error while reading wildcard cert files : " .. tostring(data))
 									ret_ok = false
 									ret_err = "error reading files"
 								else
@@ -357,12 +386,9 @@ function letsencrypt:init()
 					local data = self.internalstore:get("plugin_letsencrypt_" .. base, true)
 					if not data then
 						local check
-						check, data = read_files({
-							"/var/cache/bunkerweb/letsencrypt/etc/live/" .. base .. "/fullchain.pem",
-							"/var/cache/bunkerweb/letsencrypt/etc/live/" .. base .. "/privkey.pem",
-						})
+						check, data = read_wildcard_cert_files(base)
 						if not check then
-							self.logger:log(ERR, "error while reading files : " .. data)
+							self.logger:log(ERR, "error while reading wildcard cert files : " .. tostring(data))
 							ret_ok = false
 							ret_err = "error reading files"
 						else