Fix multi-cert chains in renewer

letsencrypt/client.py

@@ -264,19 +264,11 @@ class Client(object):
         lineage = storage.RenewableCert.new_lineage(
             domains[0], OpenSSL.crypto.dump_certificate(
                 OpenSSL.crypto.FILETYPE_PEM, certr.body),
-            key.pem, self._dump_chain(chain), params, config, cli_config)
+            key.pem, crypto_util.dump_pyopenssl_chain(chain),
+            params, config, cli_config)
         self._report_renewal_status(lineage)
         return lineage
 
-    @staticmethod
-    def _dump_chain(chain, filetype=OpenSSL.crypto.FILETYPE_PEM):
-        # assumes that OpenSSL.crypto.dump_certificate includes ending
-        # newline character; XXX: returns empty string when no chain
-        # is available, which shuts up RenewableCert, but might not be
-        # the best solution...
-        return "".join(OpenSSL.crypto.dump_certificate(
-            filetype, cert) for cert in chain)
-
     def _report_renewal_status(self, cert):
         # pylint: disable=no-self-use
         """Informs the user about automatic renewal and deployment.
@@ -342,7 +334,7 @@ class Client(object):
             chain_file, act_chain_path = le_util.unique_file(
                 chain_path, 0o644)
             # TODO: Except
-            chain_pem = self._dump_chain(chain_cert)
+            chain_pem = crypto_util.dump_pyopenssl_chain(chain_cert)
             try:
                 chain_file.write(chain_pem)
             finally:

letsencrypt/crypto_util.py

@@ -11,6 +11,7 @@ import os
 import OpenSSL
 
 from acme import crypto_util as acme_crypto_util
+from acme import jose
 
 from letsencrypt import errors
 from letsencrypt import le_util
@@ -269,3 +270,24 @@ def asn1_generalizedtime_to_dt(timestamp):
 def pyopenssl_x509_name_as_text(x509name):
     """Convert `OpenSSL.crypto.X509Name` to text."""
     return "/".join("{0}={1}" for key, value in x509name.get_components())
+
+
+def dump_pyopenssl_chain(chain, filetype=OpenSSL.crypto.FILETYPE_PEM):
+    """Dump certificate chain into a bundle.
+
+    :param list chain: List of `OpenSSL.crypto.X509` (or wrapped in
+        `acme.jose.ComparableX509`).
+
+    """
+    # XXX: returns empty string when no chain is available, which
+    # shuts up RenewableCert, but might not be the best solution...
+
+    def _dump_cert(cert):
+        if isinstance(cert, jose.ComparableX509):
+            # pylint: disable=protected-access
+            cert = cert._wrapped
+        return OpenSSL.crypto.dump_certificate(filetype, cert)
+
+    # assumes that OpenSSL.crypto.dump_certificate includes ending
+    # newline character
+    return "".join(_dump_cert(cert) for cert in chain)

letsencrypt/renewer.py

@@ -85,7 +85,7 @@ def renew(cert, old_version):
     with open(cert.version("cert", old_version)) as f:
         sans = crypto_util.get_sans_from_cert(f.read())
     new_certr, new_chain, new_key, _ = le_client.obtain_certificate(sans)
-    if new_chain is not None:
+    if new_chain:
         # XXX: Assumes that there was no key change.  We need logic
         #      for figuring out whether there was or not.  Probably
         #      best is to have obtain_certificate return None for
@@ -94,8 +94,7 @@ def renew(cert, old_version):
         return cert.save_successor(
             old_version, OpenSSL.crypto.dump_certificate(
                 OpenSSL.crypto.FILETYPE_PEM, new_certr.body),
-            new_key.pem, OpenSSL.crypto.dump_certificate(
+            new_key.pem, crypto_util.dump_pyopenssl_chain(new_chain))
-                OpenSSL.crypto.FILETYPE_PEM, new_chain))
         # TODO: Notify results
     else:
         # TODO: Notify negative results

letsencrypt/tests/renewer_test.py

@@ -596,7 +596,7 @@ class RenewableCertTests(unittest.TestCase):
         mock_client = mock.MagicMock()
         # pylint: disable=star-args
         mock_client.obtain_certificate.return_value = (
-            mock.MagicMock(body=CERT), CERT, mock.Mock(pem="key"),
+            mock.MagicMock(body=CERT), [CERT], mock.Mock(pem="key"),
             mock.sentinel.csr)
         mock_c.return_value = mock_client
         self.assertEqual(2, renewer.renew(self.test_rc, 1))
@@ -604,7 +604,7 @@ class RenewableCertTests(unittest.TestCase):
         #       have been made to the mock functions here.
         mock_acc_storage().load.assert_called_once_with(account_id="abcde")
         mock_client.obtain_certificate.return_value = (
-            mock.sentinel.certr, None, mock.sentinel.key, mock.sentinel.csr)
+            mock.sentinel.certr, [], mock.sentinel.key, mock.sentinel.csr)
         # This should fail because the renewal itself appears to fail
         self.assertFalse(renewer.renew(self.test_rc, 1))