Merge pull request #3009 from sagi/fix_cert_path

certonly subcommand - If cert_path provided - do not uniquify  it

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)

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:

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,

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" \