diff --git a/certbot/client.py b/certbot/client.py index 0159d3946..ba31f8760 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -24,6 +24,7 @@ from certbot import interfaces from certbot import le_util from certbot import reverter from certbot import storage +from certbot import cli from certbot.display import ops as display_ops from certbot.display import enhancements @@ -317,23 +318,30 @@ class Client(object): cert_pem = OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, certr.body.wrapped) - cert_file, act_cert_path = le_util.unique_file(cert_path, 0o644) + + cert_file, abs_cert_path = _open_pem_file('cert_path', cert_path) + try: cert_file.write(cert_pem) finally: cert_file.close() logger.info("Server issued certificate; certificate written to %s", - act_cert_path) + abs_cert_path) - cert_chain_abspath = None - fullchain_abspath = None - if chain_cert: + if not chain_cert: + return abs_cert_path, None, None + else: chain_pem = crypto_util.dump_pyopenssl_chain(chain_cert) - cert_chain_abspath = _save_chain(chain_pem, chain_path) - fullchain_abspath = _save_chain(cert_pem + chain_pem, - fullchain_path) - return os.path.abspath(act_cert_path), cert_chain_abspath, fullchain_abspath + chain_file, abs_chain_path =\ + _open_pem_file('chain_path', chain_path) + fullchain_file, abs_fullchain_path =\ + _open_pem_file('fullchain_path', fullchain_path) + + _save_chain(chain_pem, chain_file) + _save_chain(cert_pem + chain_pem, fullchain_file) + + return abs_cert_path, abs_chain_path, abs_fullchain_path def deploy_certificate(self, domains, privkey_path, cert_path, chain_path, fullchain_path): @@ -565,24 +573,35 @@ def view_config_changes(config, num=None): rev.recovery_routine() rev.view_config_changes(num) +def _open_pem_file(cli_arg_path, pem_path): + """Open a pem file. -def _save_chain(chain_pem, chain_path): + If cli_arg_path was set by the client, open that. + Otherwise, uniquify the file path. + + :param str cli_arg_path: the cli arg name, e.g. cert_path + :param str pem_path: the pem file path to open + + :returns: a tuple of file object and its absolute file path + + """ + if cli.set_by_cli(cli_arg_path): + return le_util.safe_open(pem_path, chmod=0o644),\ + os.path.abspath(pem_path) + else: + uniq = le_util.unique_file(pem_path, 0o644) + return uniq[0], os.path.abspath(uniq[1]) + +def _save_chain(chain_pem, chain_file): """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 + :param str chain_file: chain file object """ - 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) + logger.info("Cert chain written to %s", chain_file.name) diff --git a/certbot/le_util.py b/certbot/le_util.py index 1e5997d92..020050f6d 100644 --- a/certbot/le_util.py +++ b/certbot/le_util.py @@ -154,7 +154,8 @@ def _unique_file(path, filename_pat, count, mode): while True: current_path = os.path.join(path, filename_pat(count)) try: - return safe_open(current_path, chmod=mode), current_path + return safe_open(current_path, chmod=mode),\ + os.path.abspath(current_path) except OSError as err: # "File exists," is okay, try a different name. if err.errno != errno.EEXIST: diff --git a/certbot/tests/client_test.py b/certbot/tests/client_test.py index b7195a777..8490efd9f 100644 --- a/certbot/tests/client_test.py +++ b/certbot/tests/client_test.py @@ -203,7 +203,9 @@ class ClientTest(unittest.TestCase): mock.sentinel.key, domains, self.config.csr_dir) self._check_obtain_certificate() - def test_save_certificate(self): + @mock.patch("certbot.cli.helpful_parser") + def test_save_certificate(self, mock_parser): + # pylint: disable=too-many-locals certs = ["matching_cert.pem", "cert.pem", "cert-san.pem"] tmp_path = tempfile.mkdtemp() os.chmod(tmp_path, 0o755) # TODO: really?? @@ -214,6 +216,10 @@ class ClientTest(unittest.TestCase): 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") + mock_parser.verb = "certonly" + mock_parser.args = ["--cert-path", candidate_cert_path, + "--chain-path", candidate_chain_path, + "--fullchain-path", candidate_fullchain_path] cert_path, chain_path, fullchain_path = self.client.save_certificate( certr, chain_cert, candidate_cert_path, candidate_chain_path, diff --git a/tests/boulder-integration.sh b/tests/boulder-integration.sh index 201343525..323ea004b 100755 --- a/tests/boulder-integration.sh +++ b/tests/boulder-integration.sh @@ -43,8 +43,8 @@ export CSR_PATH="${root}/csr.der" KEY_PATH="${root}/key.pem" \ common auth --csr "$CSR_PATH" \ --cert-path "${root}/csr/cert.pem" \ --chain-path "${root}/csr/chain.pem" -openssl x509 -in "${root}/csr/0000_cert.pem" -text -openssl x509 -in "${root}/csr/0000_chain.pem" -text +openssl x509 -in "${root}/csr/cert.pem" -text +openssl x509 -in "${root}/csr/chain.pem" -text common --domains le3.wtf install \ --cert-path "${root}/csr/cert.pem" \