Also use chain_path from lineage if not defined on CLI

certbot/cli.py

@@ -1278,14 +1278,13 @@ def _paths_parser(helpful):
     elif verb == "revoke":
         add(section, "--cert-path", type=read_file, required=True, help=cph)
     else:
-        add(section, "--cert-path", type=os.path.abspath,
-            help=cph, required=(verb == "install"))
+        add(section, "--cert-path", type=os.path.abspath, help=cph)
 
     section = "paths"
     if verb in ("install", "revoke"):
         section = verb
     # revoke --key-path reads a file, install --key-path takes a string
-    add(section, "--key-path", required=(verb == "install"),
+    add(section, "--key-path",
         type=((verb == "revoke" and read_file) or os.path.abspath),
         help="Path to private key for certificate installation "
              "or revocation (if account key is missing)")

certbot/main.py

@@ -1,4 +1,5 @@
 """Certbot main entry point."""
+# pylint: disable=too-many-lines
 from __future__ import print_function
 import functools
 import logging.handlers
@@ -779,11 +780,45 @@ def install(config, plugins):
     except errors.PluginSelectionError as e:
         return str(e)
 
-    domains, _ = _find_domains_or_certname(config, installer)
-    le_client = _init_le_client(config, authenticator=None, installer=installer)
-    _install_cert(config, le_client, domains)
+    # If cert-path is defined, populate missing (ie. not overridden) values.
+    # Unfortunately this can't be done in argument parser, as certificate
+    # manager needs the access to renewal directory paths
+    if config.certname:
+        config = _populate_from_certname(config)
+    if config.key_path and config.cert_path:
+        _check_certificate_and_key(config)
+        domains, _ = _find_domains_or_certname(config, installer)
+        le_client = _init_le_client(config, authenticator=None, installer=installer)
+        _install_cert(config, le_client, domains)
+    else:
+        raise errors.ConfigurationError("Path to certificate or key was not defined. "
+            "If your certificate is managed by Certbot, please use --cert-name "
+            "to define which certificate you would like to install.")
 
+def _populate_from_certname(config):
+    """Helper function for install to populate missing config values from lineage
+    defined by --cert-name."""
 
+    lineage = cert_manager.lineage_for_certname(config, config.certname)
+    if not lineage:
+        return config
+    if not config.key_path:
+        config.namespace.key_path = lineage.key_path
+    if not config.cert_path:
+        config.namespace.cert_path = lineage.cert_path
+    if not config.chain_path:
+        config.namespace.chain_path = lineage.chain_path
+    if not config.fullchain_path:
+        config.namespace.fullchain_path = lineage.fullchain_path
+    return config
+
+def _check_certificate_and_key(config):
+    if not os.path.isfile(os.path.realpath(config.cert_path)):
+        raise errors.ConfigurationError("Error while reading certificate from path "
+                                       "{0}".format(config.cert_path))
+    if not os.path.isfile(os.path.realpath(config.key_path)):
+        raise errors.ConfigurationError("Error while reading private key from path "
+                                       "{0}".format(config.key_path))
 def plugins_cmd(config, plugins):
     """List server software plugins.
 

certbot/tests/main_test.py

@@ -592,11 +592,30 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
 
         super(MainTest, self).tearDown()
 
-    def _call(self, args, stdout=None):
-        "Run the cli with output streams and actual client mocked out"
-        with mock.patch('certbot.main.client') as client:
-            ret, stdout, stderr = self._call_no_clientmock(args, stdout)
-            return ret, stdout, stderr, client
+    def _call(self, args, stdout=None, mockisfile=False):
+        """Run the cli with output streams, actual client and optionally
+        os.path.isfile() mocked out"""
+
+        if mockisfile:
+            orig_open = os.path.isfile
+            def mock_isfile(fn, *args, **kwargs):
+                """Mock os.path.isfile()"""
+                if (fn.endswith("cert") or
+                    fn.endswith("chain") or
+                    fn.endswith("privkey")):
+                    return True
+                else:
+                    return orig_open(fn, *args, **kwargs)
+
+            with mock.patch("os.path.isfile") as mock_if:
+                mock_if.side_effect = mock_isfile
+                with mock.patch('certbot.main.client') as client:
+                    ret, stdout, stderr = self._call_no_clientmock(args, stdout)
+                    return ret, stdout, stderr, client
+        else:
+            with mock.patch('certbot.main.client') as client:
+                ret, stdout, stderr = self._call_no_clientmock(args, stdout)
+                return ret, stdout, stderr, client
 
     def _call_no_clientmock(self, args, stdout=None):
         "Run the client with output streams mocked out"
@@ -680,9 +699,68 @@ class MainTest(test_util.ConfigTestCase):  # pylint: disable=too-many-public-met
     @mock.patch('certbot.main.plug_sel.pick_installer')
     def test_installer_selection(self, mock_pick_installer, _rec):
         self._call(['install', '--domains', 'foo.bar', '--cert-path', 'cert',
-                    '--key-path', 'key', '--chain-path', 'chain'])
+                    '--key-path', 'privkey', '--chain-path', 'chain'], mockisfile=True)
         self.assertEqual(mock_pick_installer.call_count, 1)
 
+    @mock.patch('certbot.main._install_cert')
+    @mock.patch('certbot.main.plug_sel.record_chosen_plugins')
+    @mock.patch('certbot.main.plug_sel.pick_installer')
+    def test_installer_certname(self, _inst, _rec, mock_install):
+        mock_lineage = mock.MagicMock(cert_path="/tmp/cert", chain_path="/tmp/chain",
+                                      fullchain_path="/tmp/chain",
+                                      key_path="/tmp/privkey")
+
+        with mock.patch("certbot.cert_manager.lineage_for_certname") as mock_getlin:
+            mock_getlin.return_value = mock_lineage
+            self._call(['install', '--cert-name', 'whatever'], mockisfile=True)
+            call_config = mock_install.call_args[0][0]
+            self.assertEqual(call_config.cert_path, "/tmp/cert")
+            self.assertEqual(call_config.fullchain_path, "/tmp/chain")
+            self.assertEqual(call_config.key_path, "/tmp/privkey")
+
+    @mock.patch('certbot.main._install_cert')
+    @mock.patch('certbot.main.plug_sel.record_chosen_plugins')
+    @mock.patch('certbot.main.plug_sel.pick_installer')
+    def test_installer_param_override(self, _inst, _rec, mock_install):
+        mock_lineage = mock.MagicMock(cert_path="/tmp/cert", chain_path="/tmp/chain",
+                                      fullchain_path="/tmp/chain",
+                                      key_path="/tmp/privkey")
+        with mock.patch("certbot.cert_manager.lineage_for_certname") as mock_getlin:
+            mock_getlin.return_value = mock_lineage
+            self._call(['install', '--cert-name', 'whatever',
+                        '--key-path', '/tmp/overriding_privkey'], mockisfile=True)
+            call_config = mock_install.call_args[0][0]
+            self.assertEqual(call_config.cert_path, "/tmp/cert")
+            self.assertEqual(call_config.fullchain_path, "/tmp/chain")
+            self.assertEqual(call_config.chain_path, "/tmp/chain")
+            self.assertEqual(call_config.key_path, "/tmp/overriding_privkey")
+
+            mock_install.reset()
+
+            self._call(['install', '--cert-name', 'whatever',
+                        '--cert-path', '/tmp/overriding_cert'], mockisfile=True)
+            call_config = mock_install.call_args[0][0]
+            self.assertEqual(call_config.cert_path, "/tmp/overriding_cert")
+            self.assertEqual(call_config.fullchain_path, "/tmp/chain")
+            self.assertEqual(call_config.key_path, "/tmp/privkey")
+
+    @mock.patch('certbot.main.plug_sel.record_chosen_plugins')
+    @mock.patch('certbot.main.plug_sel.pick_installer')
+    def test_installer_param_error(self, _inst, _rec):
+        self.assertRaises(errors.ConfigurationError,
+                          self._call,
+                          ['install', '--key-path', '/tmp/key_path'])
+        self.assertRaises(errors.ConfigurationError,
+                          self._call,
+                          ['install', '--cert-path', '/tmp/key_path'])
+        self.assertRaises(errors.ConfigurationError,
+                          self._call,
+                          ['install'])
+        self.assertRaises(errors.ConfigurationError,
+                          self._call,
+                          ['install', '--cert-name', 'notfound',
+                           '--key-path', 'invalid'])
+
     @mock.patch('certbot.main._report_new_cert')
     @mock.patch('certbot.util.exe_exists')
     def test_configurator_selection(self, mock_exe_exists, unused_report):

tests/boulder-integration.sh

@@ -251,7 +251,7 @@ openssl x509 -in "${root}/csr/chain.pem" -text
 
 common --domains le3.wtf install \
        --cert-path "${root}/csr/cert.pem" \
-       --key-path "${root}/csr/key.pem"
+       --key-path "${root}/key.pem"
 
 CheckCertCount() {
     CERTCOUNT=`ls "${root}/conf/archive/$1/cert"* | wc -l`