diff --git a/.pylintrc b/.pylintrc index 36d8c286f..6b38b1d48 100644 --- a/.pylintrc +++ b/.pylintrc @@ -225,7 +225,7 @@ single-line-if-stmt=no no-space-check=trailing-comma # Maximum number of lines in a module -max-module-lines=1250 +max-module-lines=1300 # String used as indentation unit. This is usually " " (4 spaces) or "\t" (1 # tab). diff --git a/certbot/cli.py b/certbot/cli.py index dec7474f9..09dd71d13 100644 --- a/certbot/cli.py +++ b/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)") diff --git a/certbot/main.py b/certbot/main.py index 32dd69256..fe9115f49 100644 --- a/certbot/main.py +++ b/certbot/main.py @@ -779,11 +779,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. " + "Alternatively you can use both --key-path and --cert-path to install a " + "custom certificate.") +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.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. diff --git a/certbot/tests/main_test.py b/certbot/tests/main_test.py index b1d58542f..ba1fabae1 100644 --- a/certbot/tests/main_test.py +++ b/certbot/tests/main_test.py @@ -679,9 +679,68 @@ class MainTest(test_util.ConfigTestCase): # pylint: disable=too-many-public-met @mock.patch('certbot.main.plug_sel.record_chosen_plugins') @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']) - self.assertEqual(mock_pick_installer.call_count, 1) + with mock.patch("os.path.isfile", return_value=True): + self._call(['install', '--domains', 'foo.bar', '--cert-path', 'cert', + '--key-path', 'key', '--chain-path', 'chain']) + 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 + with mock.patch("os.path.isfile", return_value=True): + self._call(['install', '--cert-name', 'whatever']) + 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 + with mock.patch("os.path.isfile", return_value=True): + self._call(['install', '--cert-name', 'whatever', + '--key-path', '/tmp/overriding_key_path']) + 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/overriding_key_path") + mock_install.reset() + with mock.patch("os.path.isfile", return_value=True): + self._call(['install', '--cert-name', 'whatever', + '--cert-path', '/tmp/overriding_cert_path']) + call_config = mock_install.call_args[0][0] + self.assertEqual(call_config.cert_path, "/tmp/overriding_cert_path") + 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')