validate lineage name (#9644)

Fixes #6127.

* Added lineage name validity check

* Verify lineage name validity before obtaining certificate

* Added linage name limitation to cli help

* Update documentation on certificate name

* Added lineage name validation to changelog

* Use filepath seperators to determine lineagename validity

* Add unittest for private choose_lineagename method

Co-authored-by: Brad Warren <bmw@users.noreply.github.com>

certbot/CHANGELOG.md

@@ -10,7 +10,8 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Changed
 
-*
+* Lineage name validity is performed for new lineages. `--cert-name` may no longer contain
+  filepath separators (i.e. `/` or `\`, depending on the platform).
 
 ### Fixed
 

certbot/certbot/_internal/cli/__init__.py

@@ -139,6 +139,8 @@ def prepare_and_parse_args(plugins: plugins_disco.PluginsRegistry, args: List[st
         metavar="CERTNAME", default=flag_default("certname"),
         help="Certificate name to apply. This name is used by Certbot for housekeeping "
              "and in file paths; it doesn't affect the content of the certificate itself. "
+             "Certificate name cannot contain filepath separators (i.e. '/' or '\\', depending "
+             "on the platform). "
              "To see certificate names, run 'certbot certificates'. "
              "When creating a new certificate, specifies the new certificate's name. "
              "(default: the first provided domain or the name of an existing "

certbot/certbot/_internal/client.py

@@ -513,6 +513,7 @@ class Client:
             referred to the enrolled cert lineage, or None if doing a successful dry run.
 
         """
+        new_name = self._choose_lineagename(domains, certname)
         cert, chain, key, _ = self.obtain_certificate(domains)
 
         if (self.config.config_dir != constants.CLI_DEFAULTS["config_dir"] or
@@ -521,8 +522,6 @@ class Client:
                 "Non-standard path(s), might not work with crontab installed "
                 "by your operating system package manager")
 
-        new_name = self._choose_lineagename(domains, certname)
-
         if self.config.dry_run:
             logger.debug("Dry run: Skipping creating new lineage for %s", new_name)
             return None
@@ -559,13 +558,41 @@ class Client:
         :returns: lineage name that should be used
         :rtype: str
 
+        :raises errors.Error: If the chosen lineage name is invalid.
+
         """
+        # Remember chosen name for new lineage
+        lineagename = None
         if certname:
-            return certname
+            lineagename = certname
         elif util.is_wildcard_domain(domains[0]):
             # Don't make files and directories starting with *.
-            return domains[0][2:]
-        return domains[0]
+            lineagename = domains[0][2:]
+        else:
+            lineagename = domains[0]
+        # Verify whether chosen lineage is valid
+        if self._is_valid_lineagename(lineagename):
+            return lineagename
+        else:
+            raise errors.Error(
+                "The provided certname cannot be used as a lineage name because it contains "
+                "an illegal character (i.e. filepath separator)." if certname else
+                "Cannot use domain name as lineage name because it contains an illegal "
+                "character (i.e. filepath separator). Specify an explicit lineage name "
+                "with --cert-name.")
+
+    def _is_valid_lineagename(self, name: str) -> bool:
+        """Determines whether the provided name is a valid lineagename. A lineagename
+        is invalid when it contains filepath separators.
+
+        :param name: the lineage name to determine validity for
+        :type name: `str`
+
+        :returns: Whether the provided string constitutes a valid lineage name.
+        :rtype: bool
+
+        """
+        return os.path.sep not in name
 
     def save_certificate(self, cert_pem: bytes, chain_pem: bytes,
                          cert_path: str, chain_path: str, fullchain_path: str

certbot/certbot/_internal/tests/client_test.py

@@ -815,6 +815,25 @@ class ClientTest(ClientTestCommon):
         installer.rollback_checkpoints.assert_called_once_with()
         assert installer.restart.call_count == 1
 
+    def test_choose_lineage_name(self):
+        sep = os.path.sep
+        invalid_domains = [f"exam{sep}ple.com"]
+        valid_domains = ["example.com"]
+        invalid_certname = f"foo{sep}.bar"
+        valid_certname = "foo.bar"
+        invalid_wildcard_domain = [f"*.exam{sep}ple.com"]
+        # Verify errors are raised when invalid lineagename is chosen.
+        with pytest.raises(errors.Error):
+            self.client._choose_lineagename(invalid_domains, None)
+        with pytest.raises(errors.Error):
+            self.client._choose_lineagename(invalid_domains, invalid_certname)
+        with pytest.raises(errors.Error):
+            self.client._choose_lineagename(valid_domains, invalid_certname)
+        with pytest.raises(errors.Error):
+            self.client._choose_lineagename(invalid_wildcard_domain, None)
+        # Verify no error is raised when invalid domain is overriden by valid certname.
+        self.client._choose_lineagename(invalid_domains, valid_certname)
+
 
 class EnhanceConfigTest(ClientTestCommon):
     """Tests for certbot._internal.client.Client.enhance_config."""

certbot/docs/using.rst

@@ -374,7 +374,9 @@ This returns information in the following format::
 
 ``Certificate Name`` shows the name of the certificate. Pass this name
 using the ``--cert-name`` flag to specify a particular certificate for the ``run``,
-``certonly``, ``certificates``, ``renew``, and ``delete`` commands. Example::
+``certonly``, ``certificates``, ``renew``, and ``delete`` commands. The certificate
+name cannot contain filepath separators (i.e. '/' or '\\', depending on the platform).
+Example::
 
   certbot certonly --cert-name example.com