Merge remote-tracking branch 'origin/master' into randomsleep

This commit is contained in:
Seth Schoen
2018-10-16 05:58:39 -07:00
9 changed files with 50 additions and 170 deletions
+2
View File
@@ -10,6 +10,8 @@ Certbot adheres to [Semantic Versioning](http://semver.org/).
terminal) now randomly sleep 1-480 seconds before beginning work in
order to spread out load spikes on the server side.
* `revoke` accepts `--cert-name`, and doesn't accept both `--cert-name` and `--cert-path`.
### Changed
*
+1
View File
@@ -102,6 +102,7 @@ class NginxHttp01(common.ChallengePerformer):
config = [self._make_or_mod_server_block(achall) for achall in self.achalls]
config = [x for x in config if x is not None]
config = nginxparser.UnspacedList(config)
logger.debug("Generated server block:\n%s", str(config))
self.configurator.reverter.register_file_creation(
True, self.challenge_conf)
@@ -141,6 +141,8 @@ class NginxTlsSni01(common.TLSSNI01):
with open(self.challenge_conf, "w") as new_conf:
nginxparser.dump(config, new_conf)
logger.debug("Generated server block:\n%s", str(config))
def _make_server_block(self, achall, addrs):
"""Creates a server block for a challenge.
@@ -35,6 +35,7 @@ test_deployment_and_rollback() {
}
export default_server="default_server"
nginx -v
reload_nginx
certbot_test_nginx --domains nginx.wtf run
test_deployment_and_rollback nginx.wtf
+5 -4
View File
@@ -96,7 +96,7 @@ obtain, install, and renew certificates:
manage certificates:
certificates Display information about certificates you have from Certbot
revoke Revoke a certificate (supply --cert-path)
revoke Revoke a certificate (supply --cert-path or --cert-name)
delete Delete a certificate
manage your account with Let's Encrypt:
@@ -387,9 +387,10 @@ VERB_HELP = [
"usage": "\n\n certbot delete --cert-name CERTNAME\n\n"
}),
("revoke", {
"short": "Revoke a certificate specified with --cert-path",
"short": "Revoke a certificate specified with --cert-path or --cert-name",
"opts": "Options for revocation of certificates",
"usage": "\n\n certbot revoke --cert-path /path/to/fullchain.pem [options]\n\n"
"usage": "\n\n certbot revoke [--cert-path /path/to/fullchain.pem | "
"--cert-name example.com] [options]\n\n"
}),
("register", {
"short": "Register for account with Let's Encrypt / other ACME server",
@@ -1333,7 +1334,7 @@ def _paths_parser(helpful):
add(sections, "--cert-path", type=os.path.abspath,
default=flag_default("auth_cert_path"), help=cph)
elif verb == "revoke":
add(sections, "--cert-path", type=read_file, required=True, help=cph)
add(sections, "--cert-path", type=read_file, required=False, help=cph)
else:
add(sections, "--cert-path", type=os.path.abspath, help=cph)
+13 -44
View File
@@ -534,8 +534,7 @@ def _determine_account(config):
def _delete_if_appropriate(config): # pylint: disable=too-many-locals,too-many-branches
"""Does the user want to delete their now-revoked certs? If run in non-interactive mode,
deleting happens automatically, unless if both `--cert-name` and `--cert-path` were
specified with conflicting values.
deleting happens automatically.
:param config: parsed command line arguments
:type config: interfaces.IConfig
@@ -559,50 +558,13 @@ def _delete_if_appropriate(config): # pylint: disable=too-many-locals,too-many-b
reporter_util.add_message("Not deleting revoked certs.", reporter_util.LOW_PRIORITY)
return
if not (config.certname or config.cert_path):
raise errors.Error('At least one of --cert-path or --cert-name must be specified.')
# config.cert_path must have been set
# config.certname may have been set
assert config.cert_path
if config.certname and config.cert_path:
# first, check if certname and cert_path imply the same certs
implied_cert_name = cert_manager.cert_path_to_lineage(config)
if implied_cert_name != config.certname:
cert_path_implied_cert_name = cert_manager.cert_path_to_lineage(config)
cert_path_implied_conf = storage.renewal_file_for_certname(config,
cert_path_implied_cert_name)
cert_path_cert = storage.RenewableCert(cert_path_implied_conf, config)
cert_path_info = cert_manager.human_readable_cert_info(config, cert_path_cert,
skip_filter_checks=True)
cert_name_implied_conf = storage.renewal_file_for_certname(config, config.certname)
cert_name_cert = storage.RenewableCert(cert_name_implied_conf, config)
cert_name_info = cert_manager.human_readable_cert_info(config, cert_name_cert)
msg = ("You specified conflicting values for --cert-path and --cert-name. "
"Which did you mean to select?")
choices = [cert_path_info, cert_name_info]
try:
code, index = display.menu(msg,
choices, ok_label="Select", force_interactive=True)
except errors.MissingCommandlineFlag:
error_msg = ('To run in non-interactive mode, you must either specify only one of '
'--cert-path or --cert-name, or both must point to the same certificate lineages.')
raise errors.Error(error_msg)
if code != display_util.OK or not index in range(0, len(choices)):
raise errors.Error("User ended interaction.")
if index == 0:
config.certname = cert_path_implied_cert_name
else:
config.cert_path = storage.cert_path_for_cert_name(config, config.certname)
elif config.cert_path:
if not config.certname:
config.certname = cert_manager.cert_path_to_lineage(config)
else: # if only config.certname was specified
config.cert_path = storage.cert_path_for_cert_name(config, config.certname)
# don't delete if the archive_dir is used by some other lineage
archive_dir = storage.full_archive_path(
configobj.ConfigObj(storage.renewal_file_for_certname(config, config.certname)),
@@ -1068,6 +1030,14 @@ def revoke(config, unused_plugins): # TODO: coop with renewal config
"""
# For user-agent construction
config.installer = config.authenticator = None
if config.cert_path is None and config.certname:
config.cert_path = storage.cert_path_for_cert_name(config, config.certname)
elif not config.cert_path or (config.cert_path and config.certname):
# intentionally not supporting --cert-path & --cert-name together,
# to avoid dealing with mismatched values
raise errors.Error("Error! Exactly one of --cert-path or --cert-name must be specified!")
if config.key_path is not None: # revocation by cert key
logger.debug("Revoking %s using cert key %s",
config.cert_path[0], config.key_path[0])
@@ -1080,7 +1050,6 @@ def revoke(config, unused_plugins): # TODO: coop with renewal config
acme = client.acme_from_config_key(config, acc.key, acc.regr)
cert = crypto_util.pyopenssl_load_certificate(config.cert_path[1])[0]
logger.debug("Reason code for revocation: %s", config.reason)
try:
acme.revoke(jose.ComparableX509(cert), config.reason)
_delete_if_appropriate(config)
+21 -107
View File
@@ -242,6 +242,8 @@ class RevokeTest(test_util.TempDirTestCase):
shutil.copy(CERT_PATH, self.tempdir)
self.tmp_cert_path = os.path.abspath(os.path.join(self.tempdir,
'cert_512.pem'))
with open(self.tmp_cert_path, 'r') as f:
self.tmp_cert = (self.tmp_cert_path, f.read())
self.patches = [
mock.patch('acme.client.BackwardsCompatibleClientV2'),
@@ -271,9 +273,10 @@ class RevokeTest(test_util.TempDirTestCase):
for patch in self.patches:
patch.stop()
def _call(self, extra_args=""):
args = 'revoke --cert-path={0} ' + extra_args
args = args.format(self.tmp_cert_path).split()
def _call(self, args=None):
if not args:
args = 'revoke --cert-path={0} '
args = args.format(self.tmp_cert_path).split()
plugins = disco.PluginsRegistry.find_all()
config = configuration.NamespaceConfig(
cli.prepare_and_parse_args(plugins, args))
@@ -289,12 +292,25 @@ class RevokeTest(test_util.TempDirTestCase):
mock_revoke = mock_acme_client.BackwardsCompatibleClientV2().revoke
expected = []
for reason, code in constants.REVOCATION_REASONS.items():
self._call("--reason " + reason)
args = 'revoke --cert-path={0} --reason {1}'.format(self.tmp_cert_path, reason).split()
self._call(args)
expected.append(mock.call(mock.ANY, code))
self._call("--reason " + reason.upper())
args = 'revoke --cert-path={0} --reason {1}'.format(self.tmp_cert_path,
reason.upper()).split()
self._call(args)
expected.append(mock.call(mock.ANY, code))
self.assertEqual(expected, mock_revoke.call_args_list)
@mock.patch('certbot.main._delete_if_appropriate')
@mock.patch('certbot.storage.cert_path_for_cert_name')
def test_revoke_by_certname(self, mock_cert_path_for_cert_name,
mock_delete_if_appropriate):
args = 'revoke --cert-name=example.com'.split()
mock_cert_path_for_cert_name.return_value = self.tmp_cert
mock_delete_if_appropriate.return_value = False
self._call(args)
self.mock_success_revoke.assert_called_once_with(self.tmp_cert_path)
@mock.patch('certbot.main._delete_if_appropriate')
def test_revocation_success(self, mock_delete_if_appropriate):
self._call()
@@ -361,25 +377,6 @@ class DeleteIfAppropriateTest(test_util.ConfigTestCase):
self._call(config)
mock_delete.assert_not_called()
# pylint: disable=too-many-arguments
@mock.patch('certbot.storage.renewal_file_for_certname')
@mock.patch('certbot.cert_manager.match_and_check_overlaps')
@mock.patch('certbot.storage.full_archive_path')
@mock.patch('certbot.cert_manager.delete')
@mock.patch('certbot.storage.cert_path_for_cert_name')
@test_util.patch_get_utility()
def test_cert_name_only(self, mock_get_utility,
mock_cert_path_for_cert_name, mock_delete, mock_archive,
mock_overlapping_archive_dirs, mock_renewal_file_for_certname):
# pylint: disable = unused-argument
config = self.config
config.certname = "example.com"
config.cert_path = ""
mock_cert_path_for_cert_name.return_value = "/some/reasonable/path"
mock_overlapping_archive_dirs.return_value = False
self._call(config)
self.assertEqual(mock_delete.call_count, 1)
# pylint: disable=too-many-arguments
@mock.patch('certbot.storage.renewal_file_for_certname')
@mock.patch('certbot.cert_manager.match_and_check_overlaps')
@@ -442,89 +439,6 @@ class DeleteIfAppropriateTest(test_util.ConfigTestCase):
self.assertEqual(mock_delete.call_count, 1)
self.assertFalse(mock_get_utility().yesno.called)
# pylint: disable=too-many-arguments
@mock.patch('certbot.storage.renewal_file_for_certname')
@mock.patch('certbot.cert_manager.match_and_check_overlaps')
@mock.patch('certbot.storage.full_archive_path')
@mock.patch('certbot.cert_manager.delete')
@mock.patch('certbot.cert_manager.cert_path_to_lineage')
@test_util.patch_get_utility()
def test_certname_and_cert_path_match(self, mock_get_utility,
mock_cert_path_to_lineage, mock_delete, mock_archive,
mock_overlapping_archive_dirs, mock_renewal_file_for_certname):
# pylint: disable = unused-argument
config = self.config
config.certname = "example.com"
config.cert_path = "/some/reasonable/path"
mock_cert_path_to_lineage.return_value = config.certname
mock_overlapping_archive_dirs.return_value = False
self._call(config)
self.assertEqual(mock_delete.call_count, 1)
# pylint: disable=too-many-arguments
@mock.patch('certbot.cert_manager.match_and_check_overlaps')
@mock.patch('certbot.storage.full_archive_path')
@mock.patch('certbot.cert_manager.delete')
@mock.patch('certbot.cert_manager.human_readable_cert_info')
@mock.patch('certbot.storage.RenewableCert')
@mock.patch('certbot.storage.renewal_file_for_certname')
@mock.patch('certbot.cert_manager.cert_path_to_lineage')
@test_util.patch_get_utility()
def test_certname_and_cert_path_mismatch(self, mock_get_utility,
mock_cert_path_to_lineage, mock_renewal_file_for_certname,
mock_RenewableCert, mock_human_readable_cert_info,
mock_delete, mock_archive, mock_overlapping_archive_dirs):
# pylint: disable=unused-argument
config = self.config
config.certname = "example.com"
config.cert_path = "/some/reasonable/path"
mock_cert_path_to_lineage = "something else"
mock_RenewableCert.return_value = mock.Mock()
mock_human_readable_cert_info.return_value = ""
mock_overlapping_archive_dirs.return_value = False
from certbot.display import util as display_util
util_mock = mock_get_utility()
util_mock.menu.return_value = (display_util.OK, 0)
self._call(config)
self.assertEqual(mock_delete.call_count, 1)
# pylint: disable=too-many-arguments
@mock.patch('certbot.cert_manager.match_and_check_overlaps')
@mock.patch('certbot.storage.full_archive_path')
@mock.patch('certbot.cert_manager.delete')
@mock.patch('certbot.cert_manager.human_readable_cert_info')
@mock.patch('certbot.storage.RenewableCert')
@mock.patch('certbot.storage.renewal_file_for_certname')
@mock.patch('certbot.cert_manager.cert_path_to_lineage')
@test_util.patch_get_utility()
def test_noninteractive_certname_cert_path_mismatch(self, mock_get_utility,
mock_cert_path_to_lineage, mock_renewal_file_for_certname,
mock_RenewableCert, mock_human_readable_cert_info,
mock_delete, mock_archive, mock_overlapping_archive_dirs):
# pylint: disable=unused-argument
config = self.config
config.certname = "example.com"
config.cert_path = "/some/reasonable/path"
mock_cert_path_to_lineage.return_value = "some-reasonable-path.com"
mock_RenewableCert.return_value = mock.Mock()
mock_human_readable_cert_info.return_value = ""
mock_overlapping_archive_dirs.return_value = False
# Test for non-interactive mode
util_mock = mock_get_utility()
util_mock.menu.side_effect = errors.MissingCommandlineFlag("Oh no.")
self.assertRaises(errors.Error, self._call, config)
mock_delete.assert_not_called()
@mock.patch('certbot.cert_manager.delete')
@test_util.patch_get_utility()
def test_no_certname_or_cert_path(self, mock_get_utility, mock_delete):
# pylint: disable=unused-argument
config = self.config
config.certname = None
config.cert_path = None
self.assertRaises(errors.Error, self._call, config)
mock_delete.assert_not_called()
class DetermineAccountTest(test_util.ConfigTestCase):
"""Tests for certbot.main._determine_account."""
+1 -1
View File
@@ -905,7 +905,7 @@ Lock Files
When processing a validation Certbot writes a number of lock files on your system
to prevent multiple instances from overwriting each other's changes. This means
that be default two instances of Certbot will not be able to run in parallel.
that by default two instances of Certbot will not be able to run in parallel.
Since the directories used by Certbot are configurable, Certbot
will write a lock file for all of the directories it uses. This include Certbot's
+4 -14
View File
@@ -440,25 +440,15 @@ for subdomain in $subdomains; do
fi
done
# Test that revocation raises correct error if --cert-name and --cert-path don't match
# Test that revocation raises correct error when both --cert-name and --cert-path specified
common --domains le1.wtf
common --domains le2.wtf
out=$(common revoke --cert-path "$root/conf/live/le1.wtf/fullchain.pem" --cert-name "le2.wtf" 2>&1) || true
if ! echo $out | grep "or both must point to the same certificate lineages."; then
echo "Non-interactive revoking with mismatched --cert-name and --cert-path "
out=$(common revoke --cert-path "$root/conf/live/le1.wtf/fullchain.pem" --cert-name "le1.wtf" 2>&1) || true
if ! echo $out | grep "Exactly one of --cert-path or --cert-name must be specified"; then
echo "Non-interactive revoking with both --cert-name and --cert-path "
echo "did not raise the correct error!"
exit 1
fi
# Revoking by matching --cert-name and --cert-path deletes
common --domains le1.wtf
common revoke --cert-path "$root/conf/live/le1.wtf/fullchain.pem" --cert-name "le1.wtf"
out=$(common certificates)
if echo $out | grep "le1.wtf"; then
echo "Cert le1.wtf should've been deleted! Was revoked via matching --cert-path & --cert-name"
exit 1
fi
# Test that revocation doesn't delete if multiple lineages share an archive dir
common --domains le1.wtf
common --domains le2.wtf