certbot: add --preferred-chain (#8080)

* acme: add support for alternative cert. chains

* certbot: add --preferred-chain

* remove support for issuer SKI matching

* show --preferred-chain in "run" help

* warn if no chain matched and it's not a dry-run

* fix existing failing tests

* add unit, integration tests

* bump acme dependency to dev version

* simplify test to avoid py2.7 recursion bug

* add preferred_chain to STR_CONFIG_ITEMS

* reduce preferred_chain warning to info level

* acme: fix some docstrings in .messages

* certbot: fix docstring in crypto_util

* try to fix certbot-nginx acme dep problem

acme/acme/client.py

@@ -13,6 +13,7 @@ import josepy as jose
 import OpenSSL
 import requests
 from requests.adapters import HTTPAdapter
+from requests.utils import parse_header_links
 from requests_toolbelt.adapters.source import SourceAddressAdapter
 import six
 from six.moves import http_client
@@ -733,11 +734,13 @@ class ClientV2(ClientBase):
             raise errors.ValidationError(failed)
         return orderr.update(authorizations=responses)
 
-    def finalize_order(self, orderr, deadline):
+    def finalize_order(self, orderr, deadline, fetch_alternative_chains=False):
         """Finalize an order and obtain a certificate.
 
         :param messages.OrderResource orderr: order to finalize
         :param datetime.datetime deadline: when to stop polling and timeout
+        :param bool fetch_alternative_chains: whether to also fetch alternative
+            certificate chains
 
         :returns: finalized order
         :rtype: messages.OrderResource
@@ -754,8 +757,13 @@ class ClientV2(ClientBase):
             if body.error is not None:
                 raise errors.IssuanceError(body.error)
             if body.certificate is not None:
-                certificate_response = self._post_as_get(body.certificate).text
-                return orderr.update(body=body, fullchain_pem=certificate_response)
+                certificate_response = self._post_as_get(body.certificate)
+                orderr = orderr.update(body=body, fullchain_pem=certificate_response.text)
+                if fetch_alternative_chains:
+                    alt_chains_urls = self._get_links(certificate_response, 'alternate')
+                    alt_chains = [self._post_as_get(url).text for url in alt_chains_urls]
+                    orderr = orderr.update(alternative_fullchains_pem=alt_chains)
+                return orderr
         raise errors.TimeoutError()
 
     def revoke(self, cert, rsn):
@@ -785,6 +793,20 @@ class ClientV2(ClientBase):
         new_args = args[:1] + (None,) + args[1:]
         return self._post(*new_args, **kwargs)
 
+    def _get_links(self, response, relation_type):
+        """
+        Retrieves all Link URIs of relation_type from the response.
+        :param requests.Response response: The requests HTTP response.
+        :param str relation_type: The relation type to filter by.
+        """
+        # Can't use response.links directly because it drops multiple links
+        # of the same relation type, which is possible in RFC8555 responses.
+        if not 'Link' in response.headers:
+            return []
+        links = parse_header_links(response.headers['Link'])
+        return [l['url'] for l in links
+                if 'rel' in l and 'url' in l and l['rel'] == relation_type]
+
 
 class BackwardsCompatibleClientV2(object):
     """ACME client wrapper that tends towards V2-style calls, but
@@ -863,11 +885,13 @@ class BackwardsCompatibleClientV2(object):
             return messages.OrderResource(authorizations=authorizations, csr_pem=csr_pem)
         return self.client.new_order(csr_pem)
 
-    def finalize_order(self, orderr, deadline):
+    def finalize_order(self, orderr, deadline, fetch_alternative_chains=False):
         """Finalize an order and obtain a certificate.
 
         :param messages.OrderResource orderr: order to finalize
         :param datetime.datetime deadline: when to stop polling and timeout
+        :param bool fetch_alternative_chains: whether to also fetch alternative
+            certificate chains
 
         :returns: finalized order
         :rtype: messages.OrderResource
@@ -898,7 +922,7 @@ class BackwardsCompatibleClientV2(object):
             chain = crypto_util.dump_pyopenssl_chain(chain).decode()
 
             return orderr.update(fullchain_pem=(cert + chain))
-        return self.client.finalize_order(orderr, deadline)
+        return self.client.finalize_order(orderr, deadline, fetch_alternative_chains)
 
     def revoke(self, cert, rsn):
         """Revoke certificate.

acme/acme/messages.py

@@ -566,9 +566,11 @@ class Revocation(ResourceMixin, jose.JSONObjectWithFields):
 class Order(ResourceBody):
     """Order Resource Body.
 
-    :ivar list of .Identifier: List of identifiers for the certificate.
+    :ivar identifiers: List of identifiers for the certificate.
+    :vartype identifiers: `list` of `.Identifier`
     :ivar acme.messages.Status status:
-    :ivar list of str authorizations: URLs of authorizations.
+    :ivar authorizations: URLs of authorizations.
+    :vartype authorizations: `list` of `str`
     :ivar str certificate: URL to download certificate as a fullchain PEM.
     :ivar str finalize: URL to POST to to request issuance once all
         authorizations have "valid" status.
@@ -593,15 +595,20 @@ class OrderResource(ResourceWithURI):
 
     :ivar acme.messages.Order body:
     :ivar str csr_pem: The CSR this Order will be finalized with.
-    :ivar list of acme.messages.AuthorizationResource authorizations:
-        Fully-fetched AuthorizationResource objects.
+    :ivar authorizations: Fully-fetched AuthorizationResource objects.
+    :vartype authorizations: `list` of `acme.messages.AuthorizationResource`
     :ivar str fullchain_pem: The fetched contents of the certificate URL
         produced once the order was finalized, if it's present.
+    :ivar alternative_fullchains_pem: The fetched contents of alternative certificate
+        chain URLs produced once the order was finalized, if present and requested during
+        finalization.
+    :vartype alternative_fullchains_pem: `list` of `str`
     """
     body = jose.Field('body', decoder=Order.from_json)
     csr_pem = jose.Field('csr_pem', omitempty=True)
     authorizations = jose.Field('authorizations')
     fullchain_pem = jose.Field('fullchain_pem', omitempty=True)
+    alternative_fullchains_pem = jose.Field('alternative_fullchains_pem', omitempty=True)
 
 @Directory.register
 class NewOrder(Order):

acme/tests/client_test.py

@@ -263,7 +263,7 @@ class BackwardsCompatibleClientV2Test(ClientTestBase):
         with mock.patch('acme.client.ClientV2') as mock_client:
             client = self._init()
             client.finalize_order(mock_orderr, mock_deadline)
-            mock_client().finalize_order.assert_called_once_with(mock_orderr, mock_deadline)
+            mock_client().finalize_order.assert_called_once_with(mock_orderr, mock_deadline, False)
 
     def test_revoke(self):
         self.response.json.return_value = DIRECTORY_V1.to_json()
@@ -842,6 +842,32 @@ class ClientV2Test(ClientTestBase):
         deadline = datetime.datetime.now() - datetime.timedelta(seconds=60)
         self.assertRaises(errors.TimeoutError, self.client.finalize_order, self.orderr, deadline)
 
+    def test_finalize_order_alt_chains(self):
+        updated_order = self.order.update(
+            certificate='https://www.letsencrypt-demo.org/acme/cert/',
+        )
+        updated_orderr = self.orderr.update(body=updated_order,
+                                            fullchain_pem=CERT_SAN_PEM,
+                                            alternative_fullchains_pem=[CERT_SAN_PEM,
+                                                                        CERT_SAN_PEM])
+        self.response.json.return_value = updated_order.to_json()
+        self.response.text = CERT_SAN_PEM
+        self.response.headers['Link'] ='<https://example.com/acme/cert/1>;rel="alternate", ' + \
+            '<https://exaple.com/dir>;rel="index", ' + \
+            '<https://example.com/acme/cert/2>;title="foo";rel="alternate"'
+
+        deadline = datetime.datetime(9999, 9, 9)
+        resp = self.client.finalize_order(self.orderr, deadline, fetch_alternative_chains=True)
+        self.net.post.assert_any_call('https://example.com/acme/cert/1',
+                                      mock.ANY, acme_version=2, new_nonce_url=mock.ANY)
+        self.net.post.assert_any_call('https://example.com/acme/cert/2',
+                                      mock.ANY, acme_version=2, new_nonce_url=mock.ANY)
+        self.assertEqual(resp, updated_orderr)
+
+        del self.response.headers['Link']
+        resp = self.client.finalize_order(self.orderr, deadline, fetch_alternative_chains=True)
+        self.assertEqual(resp, updated_orderr.update(alternative_fullchains_pem=[]))
+
     def test_revoke(self):
         self.client.revoke(messages_test.CERT, self.rsn)
         self.net.post.assert_called_once_with(

certbot-ci/certbot_integration_tests/certbot_tests/test_main.py

@@ -9,6 +9,8 @@ import shutil
 import subprocess
 import time
 
+from cryptography.x509 import NameOID
+
 import pytest
 
 from certbot_integration_tests.certbot_tests import context as certbot_context
@@ -628,3 +630,31 @@ def test_dry_run_deactivate_authzs(context):
     context.certbot(args)
     with open(join(context.workspace, 'logs', 'letsencrypt.log'), 'r') as f:
         assert log_line in f.read(), 'Second order should have been recreated due to authz reuse'
+
+
+def test_preferred_chain(context):
+    """Test that --preferred-chain results in the correct chain.pem being produced"""
+    try:
+        issuers = misc.get_acme_issuers(context)
+    except NotImplementedError:
+        pytest.skip('This ACME server does not support alternative issuers.')
+
+    names = [i.issuer.get_attributes_for_oid(NameOID.COMMON_NAME)[0].value \
+             for i in issuers]
+
+    domain = context.get_domain('preferred-chain')
+    cert_path = join(context.config_dir, 'live', domain, 'chain.pem')
+    conf_path = join(context.config_dir, 'renewal', '{}.conf'.format(domain))
+
+    for (requested, expected) in [(n, n) for n in names] + [('nonexistent', names[0])]:
+        args = ['certonly', '--cert-name', domain, '-d', domain,
+                '--preferred-chain', requested, '--force-renewal']
+        context.certbot(args)
+
+        dumped = misc.read_certificate(cert_path)
+        assert 'Issuer: CN={}'.format(expected) in dumped, \
+               'Expected chain issuer to be {} when preferring {}'.format(expected, requested)
+
+        with open(conf_path, 'r') as f:
+            assert 'preferred_chain = {}'.format(requested) in f.read(), \
+                   'Expected preferred_chain to be set in renewal config'

certbot-ci/certbot_integration_tests/utils/acme_server.py

@@ -130,6 +130,7 @@ class ACMEServer(object):
         environ['PEBBLE_VA_NOSLEEP'] = '1'
         environ['PEBBLE_WFE_NONCEREJECT'] = '0'
         environ['PEBBLE_AUTHZREUSE'] = '100'
+        environ['PEBBLE_ALTERNATE_ROOTS'] = str(PEBBLE_ALTERNATE_ROOTS)
 
         self._launch_process(
             [pebble_path, '-config', pebble_config_path, '-dnsserver', '127.0.0.1:8053', '-strict'],

certbot-ci/certbot_integration_tests/utils/constants.py

@@ -7,3 +7,4 @@ BOULDER_V2_DIRECTORY_URL = 'http://localhost:4001/directory'
 PEBBLE_DIRECTORY_URL = 'https://localhost:14000/dir'
 PEBBLE_MANAGEMENT_URL = 'https://localhost:15000'
 MOCK_OCSP_SERVER_PORT = 4002
+PEBBLE_ALTERNATE_ROOTS = 2

certbot-ci/certbot_integration_tests/utils/misc.py

@@ -19,16 +19,30 @@ from cryptography.hazmat.primitives.asymmetric import ec
 from cryptography.hazmat.primitives.serialization import Encoding
 from cryptography.hazmat.primitives.serialization import NoEncryption
 from cryptography.hazmat.primitives.serialization import PrivateFormat
+from cryptography.x509 import load_pem_x509_certificate
 from OpenSSL import crypto
 import pkg_resources
 import requests
 from six.moves import SimpleHTTPServer
 from six.moves import socketserver
 
+from certbot_integration_tests.utils.constants import \
+     PEBBLE_ALTERNATE_ROOTS, PEBBLE_MANAGEMENT_URL
+
 RSA_KEY_TYPE = 'rsa'
 ECDSA_KEY_TYPE = 'ecdsa'
 
 
+def _suppress_x509_verification_warnings():
+    try:
+        import urllib3
+        urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)
+    except ImportError:
+        # Handle old versions of request with vendorized urllib3
+        from requests.packages.urllib3.exceptions import InsecureRequestWarning
+        requests.packages.urllib3.disable_warnings(InsecureRequestWarning)
+
+
 def check_until_timeout(url, attempts=30):
     """
     Wait and block until given url responds with status 200, or raise an exception
@@ -37,14 +51,7 @@ def check_until_timeout(url, attempts=30):
     :param int attempts: the number of times to try to connect to the URL
     :raise ValueError: exception raised if unable to reach the URL
     """
-    try:
-        import urllib3
-        urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning)
-    except ImportError:
-        # Handle old versions of request with vendorized urllib3
-        from requests.packages.urllib3.exceptions import InsecureRequestWarning
-        requests.packages.urllib3.disable_warnings(InsecureRequestWarning)
-
+    _suppress_x509_verification_warnings()
     for _ in range(attempts):
         time.sleep(1)
         try:
@@ -299,3 +306,23 @@ def echo(keyword, path=None):
                          .format(keyword))
     return '{0} -c "from __future__ import print_function; print(\'{1}\')"{2}'.format(
         os.path.basename(sys.executable), keyword, ' >> "{0}"'.format(path) if path else '')
+
+
+def get_acme_issuers(context):
+    """Gets the list of one or more issuer certificates from the ACME server used by the
+    context.
+    :param context: the testing context.
+    :return: the `list of x509.Certificate` representing the list of issuers.
+    """
+    # TODO: in fact, Boulder has alternate chains in config-next/, just not yet in config/.
+    if context.acme_server != "pebble":
+        raise NotImplementedError()
+
+    _suppress_x509_verification_warnings()
+
+    issuers = []
+    for i in range(PEBBLE_ALTERNATE_ROOTS + 1):
+        request = requests.get(PEBBLE_MANAGEMENT_URL + '/intermediates/{}'.format(i), verify=False)
+        issuers.append(load_pem_x509_certificate(request.content, default_backend()))
+
+    return issuers

certbot-nginx/local-oldest-requirements.txt

@@ -1,3 +1,3 @@
 # Remember to update setup.py to match the package versions below.
-acme[dev]==1.4.0
+-e acme[dev]
 -e certbot[dev]

certbot/CHANGELOG.md

@@ -11,6 +11,10 @@ Certbot adheres to [Semantic Versioning](https://semver.org/).
 * Make Certbot snap find externally snapped plugins
 * Function `certbot.compat.filesystem.umask` is a drop-in replacement for `os.umask`
   implementing umask for both UNIX and Windows systems.
+* Support for alternative certificate chains in the `acme` module.
+* Added `--preferred-chain <issuer CN>`. If a CA offers multiple certificate chains,
+  it may be  used to indicate to Certbot which chain should be preferred.
+  * e.g. `--preferred-chain "DST Root CA X3"`
 
 ### Changed
 

certbot/certbot/_internal/cli/__init__.py

@@ -360,6 +360,11 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False):
         default=flag_default("strict_permissions"),
         help="Require that all configuration files are owned by the current "
              "user; only needed if your config is somewhere unsafe like /tmp/")
+    helpful.add(
+        [None, "certonly", "renew", "run"],
+        "--preferred-chain", dest="preferred_chain",
+        default=flag_default("preferred_chain"), help=config_help("preferred_chain")
+    )
     helpful.add(
         ["manual", "standalone", "certonly", "renew"],
         "--preferred-challenges", dest="pref_challs",

certbot/certbot/_internal/client.py

@@ -288,8 +288,16 @@ class Client(object):
             orderr = self._get_order_and_authorizations(csr.data, best_effort=False)
 
         deadline = datetime.datetime.now() + datetime.timedelta(seconds=90)
-        orderr = self.acme.finalize_order(orderr, deadline)
-        cert, chain = crypto_util.cert_and_chain_from_fullchain(orderr.fullchain_pem)
+        get_alt_chains = self.config.preferred_chain is not None
+        orderr = self.acme.finalize_order(orderr, deadline,
+                                          fetch_alternative_chains=get_alt_chains)
+        fullchain = orderr.fullchain_pem
+        if get_alt_chains and orderr.alternative_fullchains_pem:
+            fullchain = crypto_util.find_chain_with_issuer([fullchain] + \
+                                                           orderr.alternative_fullchains_pem,
+                                                           self.config.preferred_chain,
+                                                           not self.config.dry_run)
+        cert, chain = crypto_util.cert_and_chain_from_fullchain(fullchain)
         return cert.encode(), chain.encode()
 
     def obtain_certificate(self, domains, old_keypath=None):

certbot/certbot/_internal/constants.py

@@ -63,6 +63,7 @@ CLI_DEFAULTS = dict(
     uir=None,
     staple=None,
     strict_permissions=False,
+    preferred_chain=None,
     pref_challs=[],
     validate_hooks=True,
     directory_hooks=True,

certbot/certbot/_internal/renewal.py

@@ -34,7 +34,8 @@ logger = logging.getLogger(__name__)
 # the renewal configuration process loses this information.
 STR_CONFIG_ITEMS = ["config_dir", "logs_dir", "work_dir", "user_agent",
                     "server", "account", "authenticator", "installer",
-                    "renew_hook", "pre_hook", "post_hook", "http01_address"]
+                    "renew_hook", "pre_hook", "post_hook", "http01_address",
+                    "preferred_chain"]
 INT_CONFIG_ITEMS = ["rsa_key_size", "http01_port"]
 BOOL_CONFIG_ITEMS = ["must_staple", "allow_subset_of_names", "reuse_key",
                      "autorenew"]

certbot/certbot/crypto_util.py

@@ -530,3 +530,33 @@ def get_serial_from_cert(cert_path):
         x509 = crypto.load_certificate(crypto.FILETYPE_PEM,
                                                            f.read())
     return x509.get_serial_number()
+
+
+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.
+
+    :param fullchains: The list of fullchains in PEM chain format.
+    :type fullchains: `list` of `str`
+    :param `str` issuer_cn: The exact Subject Common Name to match against any
+        issuer in the certificate chain.
+
+    :returns: The best-matching fullchain, PEM-encoded, or the first if none match.
+    :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
+
+    # Nothing matched, return whatever was first in the list.
+    if warn_on_no_match:
+        logger.info("Certbot has been configured to prefer certificate chains with "
+                    "issuer '%s', but no chain from the CA matched this issuer. Using "
+                    "the default certificate chain instead.", issuer_cn)
+    return fullchains[0]

certbot/certbot/interfaces.py

@@ -254,6 +254,12 @@ class IConfig(zope.interface.Interface):
         "If updates provided by installer enhancements when Certbot is being run"
         " 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."
+    )
+
 class IInstaller(IPlugin):
     """Generic Certbot Installer Interface.
 

certbot/certbot/tests/testdata/cert_intermediate_1.pem

@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDUDCCAjigAwIBAgIIYbnTKswm95swDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE
+AxMVUGViYmxlIFJvb3QgQ0EgNzcwNjgzMCAXDTIwMDYxOTIxMDY1NVoYDzIwNTAw
+NjE5MjAwNjU1WjAoMSYwJAYDVQQDEx1QZWJibGUgSW50ZXJtZWRpYXRlIENBIDI1
+OWJjMTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALFeijP7HyxcUMrk
+CuKgEqyaRmgjGj3JvPnm7RPJl0j6rgWe767eweP2dMJ8LnYHWZVDwmkysq1rb/kS
+HuWoqUW1gpqwo4+ARMAb0fhx79Ze7eNp5M44PRN9SenK3TZQYrNFzC5t0NzNQaq8
+ksv0R4kxi2VqfPuV6GCEYHI2t5z7249U+4PHEPSLEjq0TvFYp8aLVJ2cRLsspdXd
+ANQwbKCWz00j03CcK79LXR6ncNw1WqEoYxjzmagiAKBf9kJVawxeJF91A+kPH052
+hzGxhmMOU/VeWcl+zGeRR2cybyMF1qZ/tDX0n+J4q7FrcOjzJeZH8uxwOZ4N7Xn2
+lqavdTkCAwEAAaOBgzCBgDAOBgNVHQ8BAf8EBAMCAoQwHQYDVR0lBBYwFAYIKwYB
+BQUHAwEGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFMCE8MSR
+KqrGnv0Vd4iv8FCVtVmhMB8GA1UdIwQYMBaAFBg/Nkzipd7/vMs/f9VV11uPo2bS
+MA0GCSqGSIb3DQEBCwUAA4IBAQBISrMR9Fchj2u1FhxAr7eQsdM1Lus3B/eEJmcR
+KDO/tNzns5zsrcJL42wzDVQA61X+aVzZBSfb1oMbwHCpWBvj88avL/mJ1OgrC+VZ
+v7IDdKOwvXmFf0VVE5LLCPY+gfR65LVRkb0W9ZrZ9Oj0ke9A9ryPSNTYBvxE+Qar
+V5Jqe0YdG7IQBd4xFFnpWZyEPnQBW2Sl5oudt6LgJaQosMJdRLp4lL805G/lKPEt
+CKjWoaP6J4V1Ty+zwuWVdIpSUuIoWkZ7BeIIqVkTg7ieNj4eN2zddDxbTIquFD2K
+7rZ1691krTWTe8bkLx6wGaAfvLOk97Ywmdsot3nTVRbaVTXq
+-----END CERTIFICATE-----

certbot/certbot/tests/testdata/cert_intermediate_2.pem

@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDUDCCAjigAwIBAgIILrJTDiWZDFkwDQYJKoZIhvcNAQELBQAwIDEeMBwGA1UE
+AxMVUGViYmxlIFJvb3QgQ0EgMGNjNmYwMCAXDTIwMDYxOTIxMDY1NFoYDzIwNTAw
+NjE5MjAwNjU0WjAoMSYwJAYDVQQDEx1QZWJibGUgSW50ZXJtZWRpYXRlIENBIDI1
+OWJjMTCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoCggEBALFeijP7HyxcUMrk
+CuKgEqyaRmgjGj3JvPnm7RPJl0j6rgWe767eweP2dMJ8LnYHWZVDwmkysq1rb/kS
+HuWoqUW1gpqwo4+ARMAb0fhx79Ze7eNp5M44PRN9SenK3TZQYrNFzC5t0NzNQaq8
+ksv0R4kxi2VqfPuV6GCEYHI2t5z7249U+4PHEPSLEjq0TvFYp8aLVJ2cRLsspdXd
+ANQwbKCWz00j03CcK79LXR6ncNw1WqEoYxjzmagiAKBf9kJVawxeJF91A+kPH052
+hzGxhmMOU/VeWcl+zGeRR2cybyMF1qZ/tDX0n+J4q7FrcOjzJeZH8uxwOZ4N7Xn2
+lqavdTkCAwEAAaOBgzCBgDAOBgNVHQ8BAf8EBAMCAoQwHQYDVR0lBBYwFAYIKwYB
+BQUHAwEGCCsGAQUFBwMCMA8GA1UdEwEB/wQFMAMBAf8wHQYDVR0OBBYEFMCE8MSR
+KqrGnv0Vd4iv8FCVtVmhMB8GA1UdIwQYMBaAFE4E9PDqHbaTzFi8XEeTc/s/xEal
+MA0GCSqGSIb3DQEBCwUAA4IBAQCIEnLk9ZgIzwjew3ktKmZ/lDKl00EcIJ/or/GE
+IM3exvEzUYJotoCKdw/6d1mjfsJ1AkJUrY5WASEhXgQ9wP5/z08xwmCSVowSnHhg
+iKeQf6iNIqpxg7LGqOeSc32QXzeBMTsvQcqupZ7J6ptomiPopeAfYIMQWni9Ym/I
+P1ZjydR2petIFioPuFtByu1UhvIcF4LCJ7UWwhVNcrivf5VGaRUVac4IxGZ4Urw+
+W1xgT8pt5Rk1yCoqXsenllXRxdakbDDG9F0ZBWQNmvArYZJt1mdq1dBHT/JO/wKI
+aGF5PXa6k3YwhyvK4zTDcwPKmOBikL2FgzlohLYfETpTznW0
+-----END CERTIFICATE-----

certbot/certbot/tests/testdata/cert_leaf.pem

@@ -0,0 +1,20 @@
+-----BEGIN CERTIFICATE-----
+MIIDWTCCAkGgAwIBAgIIYWk/LgSO+zowDQYJKoZIhvcNAQELBQAwKDEmMCQGA1UE
+AxMdUGViYmxlIEludGVybWVkaWF0ZSBDQSAyNTliYzEwHhcNMjAwNjIxMDEyMTM3
+WhcNMjUwNjIxMDEyMTM3WjAWMRQwEgYDVQQDEwtleGFtcGxlLmNvbTCCASIwDQYJ
+KoZIhvcNAQEBBQADggEPADCCAQoCggEBAOABmyVITyczj6/R/1EWH6FGEfzJgOFi
+zUC1HweYouEnC3Pu4SV/qFyKC8FsVQAY5FYWV4rrexnMgAeCL7XBTFyvIYQ7FoCH
+5SeyUr3T1j/BAaFZtdb419d+BXLBOYigIu36pyRrH/3z0oxvv0brG7zXty/AOHTK
+/k5aE7e3iuUSPJqNoGIQi4NwtNw5Fwcpe4lOHRxNNp9kfjxKYzLgj9xaT6cw6r2T
+9QJ7wnU9C0/cs9h98xz+IhApyoVWlF4SE87dpG99NQ8M92od5xv1jSIF34AMQzww
+fAW3FIKr4onaMP6XHtiXnJFn4pSw+BryxPqsKz864Ritvokz/nJoylUCAwEAAaOB
+mDCBlTAOBgNVHQ8BAf8EBAMCBaAwHQYDVR0lBBYwFAYIKwYBBQUHAwEGCCsGAQUF
+BwMCMAwGA1UdEwEB/wQCMAAwHQYDVR0OBBYEFKG2yAaJtpPVStXakLLD5Jc7fcaN
+MB8GA1UdIwQYMBaAFMCE8MSRKqrGnv0Vd4iv8FCVtVmhMBYGA1UdEQQPMA2CC2V4
+YW1wbGUuY29tMA0GCSqGSIb3DQEBCwUAA4IBAQBde/j8lgRqzksphFVwfx310TaD
+4foS+15cq0OnTM/iVt1JtjyyGaRamgCcFXkm31IP8c9vnaReYGY/QawshK1NSLii
+W6mN/RqguQNoovp0xw3SkDb+lyDo0VuIyMrbwlbhP4P5uue+Shb1ZTy7QmfUeYuK
+OTjFI3j0oBiSKins3ryEFHIMvuXcaQ4F2uw6UOHBgwv5XbVV5B9l62jGbZnxrpc7
+SB9mzU8I5CJoK1uetwuADwlhDX8ITQ778unR7yIZOojyIMKWd9z9KtKDVSPK9I+d
+u0oYF4xG97nTks6Cqt2OUJizSY3ojgwK4U8w1795FCVUlq5Z58gpWf1BhA8a
+-----END CERTIFICATE-----

certbot/local-oldest-requirements.txt

@@ -1,2 +1,2 @@
 # Remember to update setup.py to match the package versions below.
-acme[dev]==1.4.0
+-e acme[dev]

certbot/setup.py

@@ -36,7 +36,7 @@ version = meta['version']
 # specified here to avoid masking the more specific request requirements in
 # acme. See https://github.com/pypa/pip/issues/988 for more info.
 install_requires = [
-    'acme>=1.4.0',
+    'acme>=1.6.0.dev0',
     # We technically need ConfigArgParse 0.10.0 for Python 2.6 support, but
     # saying so here causes a runtime error against our temporary fork of 0.9.3
     # in which we added 2.6 support (see #2243), so we relax the requirement.

certbot/tests/client_test.py

@@ -274,7 +274,8 @@ class ClientTest(ClientTestCommon):
             self.assertEqual(self.client.auth_handler.handle_authorizations.call_count, auth_count)
 
         self.acme.finalize_order.assert_called_once_with(
-            self.eg_order, mock.ANY)
+            self.eg_order, mock.ANY,
+            fetch_alternative_chains=self.config.preferred_chain is not None)
 
     @mock.patch("certbot._internal.client.crypto_util")
     @mock.patch("certbot._internal.client.logger")
@@ -293,9 +294,22 @@ class ClientTest(ClientTestCommon):
             self.client.obtain_certificate_from_csr(
                 test_csr,
                 orderr=orderr))
+        mock_crypto_util.find_chain_with_issuer.assert_not_called()
         # and that the cert was obtained correctly
         self._check_obtain_certificate()
 
+        # Test that --preferred-chain results in chain selection
+        self.config.preferred_chain = "some issuer"
+        self.assertEqual(
+            (mock.sentinel.cert, mock.sentinel.chain),
+            self.client.obtain_certificate_from_csr(
+                test_csr,
+                orderr=orderr))
+        mock_crypto_util.find_chain_with_issuer.assert_called_once_with(
+            [orderr.fullchain_pem] + orderr.alternative_fullchains_pem,
+            "some issuer", True)
+        self.config.preferred_chain = None
+
         # Test for orderr=None
         self.assertEqual(
             (mock.sentinel.cert, mock.sentinel.chain),

certbot/tests/crypto_util_test.py

@@ -27,6 +27,10 @@ SS_CERT = test_util.load_vector('cert_2048.pem')
 P256_KEY = test_util.load_vector('nistp256_key.pem')
 P256_CERT_PATH = test_util.vector_path('cert-nosans_nistp256.pem')
 P256_CERT = test_util.load_vector('cert-nosans_nistp256.pem')
+# CERT_LEAF is signed by CERT_ISSUER. CERT_ALT_ISSUER is a cross-sign of CERT_ISSUER.
+CERT_LEAF = test_util.load_vector('cert_leaf.pem')
+CERT_ISSUER = test_util.load_vector('cert_intermediate_1.pem')
+CERT_ALT_ISSUER = test_util.load_vector('cert_intermediate_2.pem')
 
 class InitSaveKeyTest(test_util.TempDirTestCase):
     """Tests for certbot.crypto_util.init_save_key."""
@@ -407,5 +411,42 @@ class CertAndChainFromFullchainTest(unittest.TestCase):
         self.assertRaises(errors.Error, cert_and_chain_from_fullchain, cert_pem)
 
 
+class FindChainWithIssuerTest(unittest.TestCase):
+    """Tests for certbot.crypto_util.find_chain_with_issuer"""
+
+    @classmethod
+    def _call(cls, fullchains, issuer_cn, **kwargs):
+        from certbot.crypto_util import find_chain_with_issuer
+        return find_chain_with_issuer(fullchains, issuer_cn, kwargs)
+
+    def _all_fullchains(self):
+        return [CERT_LEAF.decode() + CERT_ISSUER.decode(),
+                CERT_LEAF.decode() + CERT_ALT_ISSUER.decode()]
+
+    def test_positive_match(self):
+        """Correctly pick the chain based on the root's CN"""
+        fullchains = self._all_fullchains()
+        matched = self._call(fullchains, "Pebble Root CA 0cc6f0")
+        self.assertEqual(matched, fullchains[1])
+
+    @mock.patch('certbot.crypto_util.logger.info')
+    def test_no_match(self, mock_info):
+        fullchains = self._all_fullchains()
+        matched = self._call(fullchains, "non-existent issuer")
+        self.assertEqual(matched, fullchains[0])
+        mock_info.assert_not_called()
+
+    @mock.patch('certbot.crypto_util.logger.info')
+    def test_warning_on_no_match(self, mock_info):
+        fullchains = self._all_fullchains()
+        matched = self._call(fullchains, "non-existent issuer",
+                             warn_on_no_match=True)
+        self.assertEqual(matched, fullchains[0])
+        mock_info.assert_called_once_with("Certbot has been configured to prefer "
+            "certificate chains with issuer '%s', but no chain from the CA matched "
+            "this issuer. Using the default certificate chain instead.",
+            "non-existent issuer")
+
+
 if __name__ == '__main__':
     unittest.main()  # pragma: no cover