diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 1b396b0b8..5163f2868 100644 --- a/letsencrypt/cli.py +++ b/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,7 +319,7 @@ 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 @@ -312,8 +327,8 @@ def _auth_from_domains(le_client, config, domains, plugins): 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) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 732bdcf03..3a6d90472 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -258,8 +258,8 @@ class Client(object): 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,24 +268,23 @@ class Client(object): :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: @@ -293,22 +292,15 @@ class Client(object): 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) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 9d9164f24..8e9172055 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -149,7 +149,7 @@ class CLITest(unittest.TestCase): 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 @@ class CLITest(unittest.TestCase): @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,7 +196,7 @@ class CLITest(unittest.TestCase): 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') @@ -203,23 +204,24 @@ class CLITest(unittest.TestCase): @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( diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 3f7b84a64..2efe11108 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -120,18 +120,22 @@ class ClientTest(unittest.TestCase): 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()