--preferred-chain: only match root name (#8596)

* --preferred-chain: only match root name

Currently, when certbot is given the `--preferred-chain='Some Name'`
flag, it iterates through all alternate chains offered by the ACME
server until it finds any certificate which has `'Some Name'` as its
Issuer Common Name. Unfortunately, this means that if the desired
alternate chain is a strict subset of any earlier chain (e.g. the
default chain is 'EE <-- Int <-- Root1 <-- Root2', but the desired
chain is 'EE <-- Int <-- Root1'), there is no name which can be
provided by the user which will allow the client to select the desired
chain.

This change makes it so that the `find_chain_with_issuer` logic only
cares about the Issuer Common Name found in the last certificate in
each chain. In the example above, the user would then be able to get
their desired chain by specifying `--preferred-chain='Root1'`: although
that name appears in the default chain, it does not appear in the
highest certificate of that chain.

This change is technically backwards-incompatible. However, the only
advice that has been given to users of certbot (and the only usecase
that we believe has existed so far) involved setting the flag to a
value that is the name of a root, not an intermediate, so we don't
expect any real-world configurations or use-cases to be broken.

Fixes #8577

* Update interfaces.py

AUTHORS.md

@@ -1,6 +1,7 @@
 Authors
 =======
 
+* [Aaron Gable](https://github.com/aarongable)
 * [Aaron Zirbes](https://github.com/aaronzirbes)
 * Aaron Zuehlke
 * Ada Lovelace

certbot/CHANGELOG.md

@@ -10,7 +10,10 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 
 ### Changed
 
-*
+* The `--preferred-chain` flag now only checks the Issuer Common Name of the
+  topmost (closest to the root) certificate in the chain, instead of checking
+  every certificate in the chain.
+  See [#8577](https://github.com/certbot/certbot/issues/8577).
 
 ### Fixed
 

certbot/certbot/crypto_util.py

@@ -573,8 +573,9 @@ def get_serial_from_cert(cert_path):
 
 
 def find_chain_with_issuer(fullchains, issuer_cn, warn_on_no_match=False):
-    """Chooses the first certificate chain from fullchains which contains an
-    Issuer Subject Common Name matching issuer_cn.
+    """Chooses the first certificate chain from fullchains whose topmost
+    intermediate has an Issuer Common Name matching issuer_cn (in other words
+    the first chain which chains to a root whose name matches issuer_cn).
 
     :param fullchains: The list of fullchains in PEM chain format.
     :type fullchains: `list` of `str`
@@ -585,14 +586,11 @@ def find_chain_with_issuer(fullchains, issuer_cn, warn_on_no_match=False):
     :rtype: `str`
     """
     for chain in fullchains:
-        certs = [x509.load_pem_x509_certificate(cert, default_backend()) \
-                 for cert in CERT_PEM_REGEX.findall(chain.encode())]
-        # Iterate the fullchain beginning from the leaf. For each certificate encountered,
-        # match against Issuer Subject CN.
-        for cert in certs:
-            cert_issuer_cn = cert.issuer.get_attributes_for_oid(x509.NameOID.COMMON_NAME)
-            if cert_issuer_cn and cert_issuer_cn[0].value == issuer_cn:
-                return chain
+        certs = CERT_PEM_REGEX.findall(chain.encode())
+        top_cert = x509.load_pem_x509_certificate(certs[-1], default_backend())
+        top_issuer_cn = top_cert.issuer.get_attributes_for_oid(x509.NameOID.COMMON_NAME)
+        if top_issuer_cn and top_issuer_cn[0].value == issuer_cn:
+            return chain
 
     # Nothing matched, return whatever was first in the list.
     if warn_on_no_match:

certbot/certbot/interfaces.py

@@ -262,9 +262,9 @@ class IConfig(zope.interface.Interface):
         " with \"renew\" verb should be disabled.")
 
     preferred_chain = zope.interface.Attribute(
-        "If the CA offers multiple certificate chains, prefer the chain with "
-        "an issuer matching this Subject Common Name. If no match, the default "
-        "offered chain will be used."
+        "If the CA offers multiple certificate chains, prefer the chain whose "
+        "topmost certificate was issued from this Subject Common Name. "
+        "If no match, the default offered chain will be used."
     )
 
 

certbot/tests/crypto_util_test.py

@@ -473,6 +473,19 @@ class FindChainWithIssuerTest(unittest.TestCase):
         matched = self._call(fullchains, "Pebble Root CA 0cc6f0")
         self.assertEqual(matched, fullchains[1])
 
+    @mock.patch('certbot.crypto_util.logger.info')
+    def test_intermediate_match(self, mock_info):
+        """Don't pick a chain where only an intermediate matches"""
+        fullchains = self._all_fullchains()
+        # Make the second chain actually only contain "Pebble Root CA 0cc6f0"
+        # as an intermediate, not as the root. This wouldn't be a valid chain
+        # (the CERT_ISSUER cert didn't issue the CERT_ALT_ISSUER cert), but the
+        # function under test here doesn't care about that.
+        fullchains[1] = fullchains[1] + CERT_ISSUER.decode()
+        matched = self._call(fullchains, "Pebble Root CA 0cc6f0")
+        self.assertEqual(matched, fullchains[0])
+        mock_info.assert_not_called()
+
     @mock.patch('certbot.crypto_util.logger.info')
     def test_no_match(self, mock_info):
         fullchains = self._all_fullchains()