fix(ingress): give wildcard certificates their own certbot lineage

custom-domain/dstack-ingress/scripts/certman.py

@@ -36,6 +36,17 @@ def renewal_due(cert: x509.Certificate, now: datetime) -> bool:
     return now >= not_before + (lifetime / 2 if lifetime < timedelta(days=10) else lifetime * 2 / 3)
 
 
+def _cert_names(path: str) -> Optional[List[str]]:
+    """The DNS names a certificate file is for, or None if it cannot be read."""
+    try:
+        with open(path, "rb") as fh:
+            cert = x509.load_pem_x509_certificate(fh.read())
+        return cert.extensions.get_extension_for_class(
+            x509.SubjectAlternativeName).value.get_values_for_type(x509.DNSName)
+    except (OSError, ValueError, x509.ExtensionNotFound):
+        return None
+
+
 def staging_enabled() -> bool:
     """Whether to use Let's Encrypt staging.
 
@@ -219,9 +230,10 @@ def _build_certbot_command(self, action: str, domain: str, email: str,
             # For `renew`, certbot reuses the authenticator + hooks saved in the
             # renewal config from the initial `certonly`, so we don't re-specify
             # them here (and must not fall back to the DNS plugin).
+            if "--csr" not in extra_args:
+                base_cmd.extend(["--cert-name", self._lineage_name(domain)])
             if action == "renew":
-                base_cmd.extend(["--cert-name", self._lineage_name(domain),
-                                 "--no-random-sleep-on-renew"])
+                base_cmd.append("--no-random-sleep-on-renew")
             # Explicit on `renew` too: certbot would otherwise take the server
             # saved in the lineage, whatever the configuration says now.
             base_cmd.extend(["--server", acme_server()])
@@ -263,16 +275,18 @@ def _build_certbot_command(self, action: str, domain: str, email: str,
             else:
                 base_cmd.append("--register-unsafely-without-email")
             base_cmd.extend(["-d", domain])
-        if action == "renew":
+        if "--csr" not in extra_args:
             # Without this, `certbot renew` renews *every* lineage in
             # /etc/letsencrypt/renewal. run_pass calls this once per domain, so
             # N domains meant N passes over all N lineages, and the result was
-            # then reported against whichever domain happened to ask.
-            #
+            # then reported against whichever domain happened to ask. On
+            # `certonly` it picks the lineage (see _lineage_name); --csr
+            # issues outside any lineage.
+            base_cmd.extend(["--cert-name", self._lineage_name(domain)])
+        if action == "renew":
             # Without a TTY, certbot also sleeps up to 8 minutes before a due
             # renewal, which overruns _certbot_timeout and gets the run killed.
-            base_cmd.extend(["--cert-name", self._lineage_name(domain),
-                             "--no-random-sleep-on-renew"])
+            base_cmd.append("--no-random-sleep-on-renew")
         base_cmd.extend(["--server", acme_server()])
 
         if getattr(self.provider, 'CERTBOT_PROPAGATION_SECONDS'):
@@ -401,10 +415,11 @@ def _clear_legacy_live(self, domain: str) -> None:
         to create the lineage over them left an empty renewal config, which
         would push the next lineage to <name>-0001.
         """
-        conf = self._renewal_conf_path(domain)
+        base = domain[2:] if domain.startswith("*.") else domain
+        conf = os.path.join(LETSENCRYPT_DIR, "renewal", f"{base}.conf")
         if os.path.isfile(conf) and os.path.getsize(conf) == 0:
             os.remove(conf)
-        live = self._live_dir(domain)
+        live = os.path.join(LETSENCRYPT_DIR, "live", base)
         privkey = os.path.join(live, "privkey.pem")
         if not os.path.isfile(privkey) or os.path.islink(privkey):
             return
@@ -512,8 +527,28 @@ def certificate_exists(self, domain: str) -> bool:
 
     @staticmethod
     def _lineage_name(domain: str) -> str:
-        """certbot stores a wildcard lineage under the bare name."""
-        return domain[2:] if domain.startswith("*.") else domain
+        """The lineage holding exactly this domain, else the name to create.
+
+        Left to itself certbot named a wildcard's lineage after the bare name,
+        the same as the base domain's, so with both in DOMAINS one became
+        <name>-0001. Existing lineages are therefore found by content. A new
+        wildcard lineage is _wildcard.<name>: an underscore cannot appear in a
+        certificate host name, so it never collides with a domain.
+        """
+        base = domain[2:] if domain.startswith("*.") else domain
+        new = f"_wildcard.{base}" if domain.startswith("*.") else domain
+        renewal = os.path.join(LETSENCRYPT_DIR, "renewal")
+        pattern = re.compile(rf"(_wildcard\.)?{re.escape(base)}(-\d{{4}})?")
+        existing = sorted(os.path.basename(p)[:-len(".conf")]
+                          for p in glob.glob(os.path.join(renewal, "*.conf")))
+        for name in [new] + [n for n in existing if pattern.fullmatch(n)]:
+            if _cert_names(os.path.join(LETSENCRYPT_DIR, "live", name, "cert.pem")) == [domain]:
+                return name
+        name, n = new, 0
+        while name in existing:
+            n += 1
+            name = f"{new}-{n:04d}"
+        return name
 
     @classmethod
     def _live_dir(cls, domain: str) -> str:

custom-domain/dstack-ingress/scripts/tests/test_certman.py

@@ -141,14 +141,6 @@ def test_a_failed_write_leaves_the_original_intact(self):
             os.chmod(self.dir, 0o700)
         self.assertEqual(self.read(), original)
 
-    def test_wildcard_uses_the_bare_lineage_name(self):
-        self.assertEqual(
-            certman.CertManager._lineage_name("*.example.com"), "example.com"
-        )
-        self.assertEqual(
-            certman.CertManager._lineage_name("example.com"), "example.com"
-        )
-
     def test_wildcard_has_its_own_deterministic_dir(self):
         with mock.patch.dict(os.environ, {"DETERMINISTIC_TLS_KEY": "true"}):
             self.assertEqual(certman.CertManager.cert_dir("*.example.com"),
@@ -191,6 +183,10 @@ def setUp(self):
         self._saved = {v: os.environ.get(v) for v in self.VARS}
         for v in self.VARS:
             os.environ.pop(v, None)
+        self.root = tempfile.mkdtemp()
+        self.addCleanup(shutil.rmtree, self.root)
+        self.addCleanup(setattr, certman, "LETSENCRYPT_DIR", certman.LETSENCRYPT_DIR)
+        certman.LETSENCRYPT_DIR = self.root
 
     def tearDown(self):
         for v, old in self._saved.items():
@@ -213,15 +209,38 @@ def test_renew_is_scoped_to_one_lineage(self):
         self.assertIn("--cert-name", cmd)
         self.assertEqual(cmd[cmd.index("--cert-name") + 1], "a.example.com")
 
-    def test_renew_uses_the_bare_name_for_a_wildcard(self):
+    def test_renew_uses_the_wildcard_lineage(self):
         cmd = _command_manager()._build_certbot_command("renew", "*.example.com", "")
-        self.assertEqual(cmd[cmd.index("--cert-name") + 1], "example.com")
-
-    def test_certonly_is_scoped_by_d_not_cert_name(self):
+        self.assertEqual(cmd[cmd.index("--cert-name") + 1], "_wildcard.example.com")
+
+    def test_certonly_names_the_lineage(self):
+        """certbot's own pick for a wildcard is the base domain's name."""
+        for delegation in ("", "deleg.example.net"):
+            os.environ["DELEGATION_ZONE"] = delegation
+            cmd = _command_manager()._build_certbot_command(
+                "certonly", "*.example.com", "")
+            self.assertEqual(cmd[cmd.index("--cert-name") + 1], "_wildcard.example.com")
+            self.assertEqual(cmd[cmd.index("-d") + 1], "*.example.com")
+
+    def test_csr_issuance_has_no_lineage(self):
+        os.environ["DETERMINISTIC_TLS_KEY"] = "true"
         cmd = _command_manager()._build_certbot_command(
-            "certonly", "a.example.com", "")
+            "certonly", "*.example.com", "", ("--csr", "x.csr"))
         self.assertNotIn("--cert-name", cmd)
-        self.assertIn("-d", cmd)
+
+    def test_replacing_keeps_the_lineage(self):
+        mgr = _command_manager()
+        mgr.install_plugin = lambda: True
+        mgr.provider.validate_credentials = lambda: True
+        mgr.setup_credentials = lambda: True
+        cmds = []
+        with mock.patch.object(certman.subprocess, "run",
+                               lambda cmd, **kw: cmds.append(cmd) or mock.Mock(returncode=0)):
+            self.assertTrue(mgr.obtain_certificate("*.x.com", "", replace=True))
+        cmd = cmds[0]
+        self.assertIn("--force-renewal", cmd)
+        self.assertIn("--break-my-certs", cmd)
+        self.assertEqual(cmd[cmd.index("--cert-name") + 1], "_wildcard.x.com")
 
     def test_delegation_renew_is_scoped_too(self):
         os.environ["DELEGATION_ZONE"] = "deleg.example.net"
@@ -313,6 +332,68 @@ def test_delegation_renew_skips_the_random_sleep(self):
         self.assertIn("--no-random-sleep-on-renew", cmd)
 
 
+def _cert_pem(name, key):
+    from datetime import datetime, timedelta, timezone
+    from cryptography import x509
+    from cryptography.hazmat.primitives import hashes, serialization
+    from cryptography.x509.oid import NameOID
+
+    now = datetime.now(timezone.utc)
+    subject = x509.Name([x509.NameAttribute(NameOID.COMMON_NAME, name)])
+    return (x509.CertificateBuilder()
+            .subject_name(subject).issuer_name(subject)
+            .public_key(key.public_key()).serial_number(1)
+            .not_valid_before(now).not_valid_after(now + timedelta(days=90))
+            .add_extension(x509.SubjectAlternativeName([x509.DNSName(name)]), critical=False)
+            .sign(key, hashes.SHA256())
+            .public_bytes(serialization.Encoding.PEM))
+
+
+class LineageNameTest(EnvTestCase):
+    """Existing lineages are found by content; new ones get a name of their own."""
+
+    def setUp(self):
+        super().setUp()
+        from cryptography.hazmat.primitives.asymmetric import ec
+
+        self.key = ec.generate_private_key(ec.SECP256R1())
+        self.mgr = object.__new__(certman.CertManager)
+
+    def lineage(self, lineage, name):
+        os.makedirs(f"{self.root}/live/{lineage}")
+        os.makedirs(f"{self.root}/renewal", exist_ok=True)
+        with open(f"{self.root}/live/{lineage}/cert.pem", "wb") as fh:
+            fh.write(_cert_pem(name, self.key))
+        open(f"{self.root}/renewal/{lineage}.conf", "w").close()
+
+    def test_new_wildcard_lineage_is_apart_from_the_base(self):
+        self.assertEqual(self.mgr._lineage_name("*.x.com"), "_wildcard.x.com")
+        self.assertEqual(self.mgr._lineage_name("x.com"), "x.com")
+
+    def test_lineages_certbot_named_itself_are_found(self):
+        self.lineage("x.com", "*.x.com")
+        self.lineage("x.com-0001", "x.com")
+        self.assertEqual(self.mgr._lineage_name("*.x.com"), "x.com")
+        self.assertEqual(self.mgr._lineage_name("x.com"), "x.com-0001")
+
+    def test_a_name_taken_by_another_certificate_is_not_reused(self):
+        self.lineage("x.com", "*.x.com")
+        self.assertEqual(self.mgr._lineage_name("x.com"), "x.com-0001")
+
+    def test_legacy_wildcard_files_under_the_bare_name_move_to_the_wildcards(self):
+        os.environ["DETERMINISTIC_TLS_KEY"] = "true"
+        live = f"{self.root}/live/x.com"
+        os.makedirs(live)
+        for item in ("cert", "fullchain"):
+            with open(f"{live}/{item}.pem", "wb") as fh:
+                fh.write(_cert_pem("*.x.com", self.key))
+        with open(f"{live}/privkey.pem", "wb") as fh:
+            fh.write(certman.deterministic_key.key_pem(self.key))
+        self.mgr._clear_legacy_live("*.x.com")
+        self.assertFalse(os.path.exists(live))
+        self.assertTrue(os.path.exists(f"{self.root}/deterministic/_wildcard.x.com/fullchain.pem"))
+
+
 class CertbotTimeoutTest(EnvTestCase):
     """A fixed 300s cap could not fit every provider's own propagation wait."""