Merge pull request #1077 from letsencrypt/fullchain-doc

Point the user to fullchain.pem, not cert.pem

letsencrypt/cli.py

@@ -267,16 +267,31 @@ def _treat_as_renewal(config, domains):
     return None
 
 
-def _report_new_cert(cert_path):
-    """Reports the creation of a new certificate to the user."""
+def _report_new_cert(cert_path, fullchain_path):
+    """Reports the creation of a new certificate to the user.
+
+    :param str cert_path: path to cert
+    :param str fullchain_path: path to full chain
+
+    """
     expiry = crypto_util.notAfter(cert_path).date()
     reporter_util = zope.component.getUtility(interfaces.IReporter)
-    reporter_util.add_message("Congratulations! Your certificate has been "
-                              "saved at {0} and will expire on {1}. To obtain "
-                              "a new version of the certificate in the "
-                              "future, simply run Let's Encrypt again.".format(
-                                  cert_path, expiry),
-                              reporter_util.MEDIUM_PRIORITY)
+    if fullchain_path:
+        # Print the path to fullchain.pem because that's what modern webservers
+        # (Nginx and Apache2.4) will want.
+        and_chain = "and chain have"
+        path = fullchain_path
+    else:
+        # Unless we're in .csr mode and there really isn't one
+        and_chain = "has "
+        path = cert_path
+    # XXX Perhaps one day we could detect the presence of known old webservers
+    # and say something more informative here.
+    msg = ("Congratulations! Your certificate {0} been saved at {1}."
+           " Your cert will expire on {2}. To obtain a new version of the "
+           "certificate in the future, simply run Let's Encrypt again."
+           .format(and_chain, path, expiry))
+    reporter_util.add_message(msg, reporter_util.MEDIUM_PRIORITY)
 
 
 def _auth_from_domains(le_client, config, domains, plugins):
@@ -304,16 +319,16 @@ def _auth_from_domains(le_client, config, domains, plugins):
         if not lineage:
             raise Error("Certificate could not be obtained")
 
-    _report_new_cert(lineage.cert)
+    _report_new_cert(lineage.cert, lineage.fullchain)
 
     return lineage
 
 
 def set_configurator(previously, now):
     """
     Setting configurators multiple ways is okay, as long as they all agree
-    :param string previously: previously identified request for the installer/authenticator
-    :param string requested: the request currently being processed
+    :param str previously: previously identified request for the installer/authenticator
+    :param str requested: the request currently being processed
     """
     if now is None:
         # we're not actually setting anything
@@ -329,8 +344,8 @@ def diagnose_configurator_problem(cfg_type, requested, plugins):
     """
     Raise the most helpful error message about a plugin being unavailable
 
-    :param string cfg_type: either "installer" or "authenticator"
-    :param string requested: the plugin that was requested
+    :param str cfg_type: either "installer" or "authenticator"
+    :param str requested: the plugin that was requested
     :param PluginRegistry plugins: available plugins
 
     :raises error.PluginSelectionError: if there was a problem
@@ -455,9 +470,9 @@ def auth(args, config, plugins):
     if args.csr is not None:
         certr, chain = le_client.obtain_certificate_from_csr(le_util.CSR(
             file=args.csr[0], data=args.csr[1], form="der"))
-        cert_path, _ = le_client.save_certificate(
-            certr, chain, args.cert_path, args.chain_path)
-        _report_new_cert(cert_path)
+        cert_path, _, cert_fullchain = le_client.save_certificate(
+            certr, chain, args.cert_path, args.chain_path, args.fullchain_path)
+        _report_new_cert(cert_path, cert_fullchain)
     else:
         domains = _find_domains(args, installer)
         _auth_from_domains(le_client, config, domains, plugins)

letsencrypt/client.py

@@ -258,8 +258,8 @@ def obtain_and_enroll_certificate(self, domains, plugins):
             params, config, cli_config)
         return lineage
 
-    def save_certificate(self, certr, chain_cert, cert_path, chain_path):
-        # pylint: disable=no-self-use
+    def save_certificate(self, certr, chain_cert,
+                         cert_path, chain_path, fullchain_path):
         """Saves the certificate received from the ACME server.
 
         :param certr: ACME "certificate" resource.
@@ -268,47 +268,39 @@ def save_certificate(self, certr, chain_cert, cert_path, chain_path):
         :param list chain_cert:
         :param str cert_path: Candidate path to a certificate.
         :param str chain_path: Candidate path to a certificate chain.
+        :param str fullchain_path: Candidate path to a full cert chain.
 
-        :returns: cert_path, chain_path (absolute paths to the actual files)
+        :returns: cert_path, chain_path, and fullchain_path as absolute
+            paths to the actual files
         :rtype: `tuple` of `str`
 
         :raises IOError: If unable to find room to write the cert files
 
         """
-        for path in cert_path, chain_path:
+        for path in cert_path, chain_path, fullchain_path:
             le_util.make_or_verify_dir(
                 os.path.dirname(path), 0o755, os.geteuid(),
                 self.config.strict_permissions)
 
-        # try finally close
-        cert_chain_abspath = None
-        cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644)
-        # TODO: Except
         cert_pem = OpenSSL.crypto.dump_certificate(
             OpenSSL.crypto.FILETYPE_PEM, certr.body)
+        cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644)
         try:
             cert_file.write(cert_pem)
         finally:
             cert_file.close()
         logger.info("Server issued certificate; certificate written to %s",
                     act_cert_path)
 
+        cert_chain_abspath = None
+        fullchain_abspath = None
         if chain_cert:
-            chain_file, act_chain_path = le_util.unique_file(
-                chain_path, 0o644)
-            # TODO: Except
             chain_pem = crypto_util.dump_pyopenssl_chain(chain_cert)
-            try:
-                chain_file.write(chain_pem)
-            finally:
-                chain_file.close()
+            cert_chain_abspath = _save_chain(chain_pem, chain_path)
+            fullchain_abspath = _save_chain(cert_pem + chain_pem,
+                                            fullchain_path)
 
-            logger.info("Cert chain written to %s", act_chain_path)
-
-            # This expects a valid chain file
-            cert_chain_abspath = os.path.abspath(act_chain_path)
-
-        return os.path.abspath(act_cert_path), cert_chain_abspath
+        return os.path.abspath(act_cert_path), cert_chain_abspath, fullchain_abspath
 
     def deploy_certificate(self, domains, privkey_path,
                            cert_path, chain_path, fullchain_path):
@@ -465,3 +457,25 @@ def view_config_changes(config):
     rev = reverter.Reverter(config)
     rev.recovery_routine()
     rev.view_config_changes()
+
+
+def _save_chain(chain_pem, chain_path):
+    """Saves chain_pem at a unique path based on chain_path.
+
+    :param str chain_pem: certificate chain in PEM format
+    :param str chain_path: candidate path for the cert chain
+
+    :returns: absolute path to saved cert chain
+    :rtype: str
+
+    """
+    chain_file, act_chain_path = le_util.unique_file(chain_path, 0o644)
+    try:
+        chain_file.write(chain_pem)
+    finally:
+        chain_file.close()
+
+    logger.info("Cert chain written to %s", act_chain_path)
+
+    # This expects a valid chain file
+    return os.path.abspath(act_chain_path)

letsencrypt/tests/cli_test.py

@@ -149,7 +149,7 @@ def test_auth_new_request_success(self, mock_get_utility, mock_notAfter):
         date = '1970-01-01'
         mock_notAfter().date.return_value = date
 
-        mock_lineage = mock.MagicMock(cert=cert_path)
+        mock_lineage = mock.MagicMock(cert=cert_path, fullchain=cert_path)
         mock_client = mock.MagicMock()
         mock_client.obtain_and_enroll_certificate.return_value = mock_lineage
         self._auth_new_request_common(mock_client)
@@ -177,9 +177,10 @@ def _auth_new_request_common(self, mock_client):
     @mock.patch('letsencrypt.cli._treat_as_renewal')
     @mock.patch('letsencrypt.cli._init_le_client')
     def test_auth_renewal(self, mock_init, mock_renewal, mock_get_utility):
-        cert_path = '/etc/letsencrypt/live/foo.bar'
+        cert_path = '/etc/letsencrypt/live/foo.bar/cert.pem'
+        chain_path = '/etc/letsencrypt/live/foo.bar/fullchain.pem'
 
-        mock_lineage = mock.MagicMock(cert=cert_path)
+        mock_lineage = mock.MagicMock(cert=cert_path, fullchain=chain_path)
         mock_cert = mock.MagicMock(body='body')
         mock_key = mock.MagicMock(pem='pem_key')
         mock_renewal.return_value = mock_lineage
@@ -195,31 +196,32 @@ def test_auth_renewal(self, mock_init, mock_renewal, mock_get_utility):
         mock_lineage.update_all_links_to.assert_called_once_with(
             mock_lineage.latest_common_version())
         self.assertTrue(
-            cert_path in mock_get_utility().add_message.call_args[0][0])
+            chain_path in mock_get_utility().add_message.call_args[0][0])
 
     @mock.patch('letsencrypt.crypto_util.notAfter')
     @mock.patch('letsencrypt.cli.display_ops.pick_installer')
     @mock.patch('letsencrypt.cli.zope.component.getUtility')
     @mock.patch('letsencrypt.cli._init_le_client')
     def test_auth_csr(self, mock_init, mock_get_utility,
                       mock_pick_installer, mock_notAfter):
-        cert_path = '/etc/letsencrypt/live/foo.bar'
+        cert_path = '/etc/letsencrypt/live/blahcert.pem'
         date = '1970-01-01'
         mock_notAfter().date.return_value = date
 
         mock_client = mock.MagicMock()
         mock_client.obtain_certificate_from_csr.return_value = ('certr',
                                                                 'chain')
-        mock_client.save_certificate.return_value = cert_path, None
+        mock_client.save_certificate.return_value = cert_path, None, None
         mock_init.return_value = mock_client
 
         installer = 'installer'
         self._call(
             ['-a', 'standalone', '-i', installer, 'auth', '--csr', CSR,
-             '--cert-path', cert_path, '--chain-path', '/'])
+             '--cert-path', cert_path, '--fullchain-path', '/',
+             '--chain-path', '/'])
         self.assertEqual(mock_pick_installer.call_args[0][1], installer)
         mock_client.save_certificate.assert_called_once_with(
-            'certr', 'chain', cert_path, '/')
+            'certr', 'chain', cert_path, '/', '/')
         self.assertTrue(
             cert_path in mock_get_utility().add_message.call_args[0][0])
         self.assertTrue(

letsencrypt/tests/client_test.py

@@ -120,18 +120,22 @@ def test_save_certificate(self):
         os.chmod(tmp_path, 0o755)  # TODO: really??
 
         certr = mock.MagicMock(body=test_util.load_cert(certs[0]))
-        cert1 = test_util.load_cert(certs[1])
-        cert2 = test_util.load_cert(certs[2])
+        chain_cert = [test_util.load_cert(certs[1]),
+                      test_util.load_cert(certs[2])]
         candidate_cert_path = os.path.join(tmp_path, "certs", "cert.pem")
         candidate_chain_path = os.path.join(tmp_path, "chains", "chain.pem")
+        candidate_fullchain_path = os.path.join(tmp_path, "chains", "fullchain.pem")
 
-        cert_path, chain_path = self.client.save_certificate(
-            certr, [cert1, cert2], candidate_cert_path, candidate_chain_path)
+        cert_path, chain_path, fullchain_path = self.client.save_certificate(
+            certr, chain_cert, candidate_cert_path, candidate_chain_path,
+            candidate_fullchain_path)
 
         self.assertEqual(os.path.dirname(cert_path),
                          os.path.dirname(candidate_cert_path))
         self.assertEqual(os.path.dirname(chain_path),
                          os.path.dirname(candidate_chain_path))
+        self.assertEqual(os.path.dirname(fullchain_path),
+                         os.path.dirname(candidate_fullchain_path))
 
         with open(cert_path, "r") as cert_file:
             cert_contents = cert_file.read()