From 1bb62eed4dd97067ae35e922db3669701dc0a3fe Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 10 Sep 2015 22:35:44 -0400 Subject: [PATCH 01/33] Started crash recovery mechanism --- letsencrypt/error_handler.py | 46 ++++++++++++++++++++++++++++++++++++ letsencrypt/interfaces.py | 11 +++++++++ 2 files changed, 57 insertions(+) create mode 100644 letsencrypt/error_handler.py diff --git a/letsencrypt/error_handler.py b/letsencrypt/error_handler.py new file mode 100644 index 000000000..884c73927 --- /dev/null +++ b/letsencrypt/error_handler.py @@ -0,0 +1,46 @@ +"""Registers and calls cleanup functions in case of an error.""" +import os +import signal + + +_SIGNALS = [signal.SIGTERM] if os.name == "nt" else + [signal.SIGTERM, signal.SIGHUP, signal.SIGQUIT, + signal.SIGXCPU, signal.SIGXFSZ, signal.SIGPWR,] + + +class ErrorHandler(): + """Registers and calls cleanup functions in case of an error.""" + def __init__(self, func=None): + self.funcs = [] + if func: + self.funcs.append(func) + + def __enter__(self): + self.set_signal_handlers() + + def __exit__(self, exec_type, exec_value, traceback): + if exec_value is not None: + self.cleanup() + self.reset_signal_handlers() + + def register(self, func): + """Registers func to be called if an error occurs.""" + self.funcs.append(func) + + def cleanup(self): + """Calls all registered functions.""" + while self.funcs: + self.funcs.pop()() + + def set_signal_handlers(self): + for signal_type in _SIGNALS: + signal.signal(signal_type, self._signal_handler) + + def reset_signal_handlers(self): + for signal_type in _SIGNALS: + signal.signal(signal_type, signal.SIG_DFL) + + def _signal_handler(self, signum, frame): + self.cleanup() + signal.signal(signal_type, signal.SIG_DFL) + os.kill(os.getpid(), signum) diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index f330e28ce..653b5685b 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -322,6 +322,17 @@ class IInstaller(IPlugin): """ + def recovery_routine(self): + """Revert configuration to most recent finalized checkpoint. + + Remove all changes (temporary and permanent) that have not been + finalized. This is useful to protect against crashes and other + execution interruptions. + + :raises .errors.PluginError: If unable to recover the configuration + + """ + def view_config_changes(): """Display all of the LE config changes. From c025c17b5de7d5b328c8143ed9d3c32075d9df32 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 15 Sep 2015 22:48:36 -0700 Subject: [PATCH 02/33] auth use renewal --- letsencrypt/cli.py | 189 ++++++++++++++++++------------ letsencrypt/client.py | 10 +- letsencrypt/tests/renewer_test.py | 5 + 3 files changed, 121 insertions(+), 83 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 3a600d7f7..825cae775 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -196,8 +196,101 @@ def _find_duplicative_certs(domains, config, renew_config): return identical_names_cert, subset_names_cert +def _treat_as_renewal(config, domains): + """Determine whether or not the call should be treated as a renewal. + + :returns: RenewableCert or None if renewal shouldn't occur. + :rtype: :class:`.storage.RenewableCert` + + :raises .Error: If the user would like to rerun the client again. + + """ + renewal = False + + # Considering the possibility that the requested certificate is + # related to an existing certificate. (config.duplicate, which + # is set with --duplicate, skips all of this logic and forces any + # kind of certificate to be obtained with renewal = False.) + if not config.duplicate: + ident_names_cert, subset_names_cert = _find_duplicative_certs( + domains, config, configuration.RenewerConfiguration(config)) + # I am not sure whether that correctly reads the systemwide + # configuration file. + question = None + if ident_names_cert is not None: + question = ( + "You have an existing certificate that contains exactly the " + "same domains you requested (ref: {0})\n\nDo you want to " + "renew and replace this certificate with a newly-issued one?" + ).format(ident_names_cert.configfile.filename) + elif subset_names_cert is not None: + question = ( + "You have an existing certificate that contains a portion of " + "the domains you requested (ref: {0})\n\nIt contains these " + "names: {1}\n\nYou requested these names for the new " + "certificate: {2}.\n\nDo you want to replace this existing " + "certificate with the new certificate?" + ).format(subset_names_cert.configfile.filename, + ", ".join(subset_names_cert.names()), + ", ".join(domains)) + if question is None: + # We aren't in a duplicative-names situation at all, so we don't + # have to tell or ask the user anything about this. + pass + elif zope.component.getUtility(interfaces.IDisplay).yesno( + question, "Replace", "Cancel"): + renewal = True + else: + reporter_util = zope.component.getUtility(interfaces.IReporter) + reporter_util.add_message( + "To obtain a new certificate that {0} an existing certificate " + "in its domain-name coverage, you must use the --duplicate " + "option.\n\nFor example:\n\n{1} --duplicate {2}".format( + "duplicates" if ident_names_cert is not None else + "overlaps with", sys.argv[0], " ".join(sys.argv[1:])), + reporter_util.HIGH_PRIORITY) + raise errors.Error( + "BUser did not use proper CLI and would like " + "to reinvoke the client.") + + if renewal: + return ident_names_cert if ident_names_cert is not None else subset_names_cert + + return None + + +def auth_from_domains(le_client, config, domains, plugins): + """Authenticate and enroll certificate.""" + # Note: This can raise errors... caught above us though. + lineage = _treat_as_renewal(config, domains) + + if lineage is not None: + new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) + # TODO: Check whether it worked! + lineage.save_successor( + lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, new_certr.body), + new_key.pem, OpenSSL.crypto.dump_certificate( + OpenSSL.crypto.FILETYPE_PEM, new_chain)) + + lineage.update_all_links_to(lineage.latest_common_version()) + # TODO: Check return value of save_successor + # TODO: Also update lineage renewal config with any relevant + # configuration values from this attempt? - YES + else: + # TREAT AS NEW REQUEST + lineage = le_client.obtain_and_enroll_certificate( + domains, le_client.dv_auth, le_client.installer, plugins) + if not lineage: + raise errors.Error("Certificate could not be obtained") + + return lineage + +# TODO: Make run as close to auth + install as possible +# Possible difficulties: args.csr was hacked into auth def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-locals """Obtain a certificate and install.""" + # Begin authenticator and installer setup if args.configurator is not None and (args.installer is not None or args.authenticator is not None): return ("Either --configurator or --authenticator/--installer" @@ -216,88 +309,28 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo if installer is None or authenticator is None: return "Configurator could not be determined" + # End authenticator and installer setup domains = _find_domains(args, installer) - treat_as_renewal = False - - # Considering the possibility that the requested certificate is - # related to an existing certificate. (config.duplicate, which - # is set with --duplicate, skips all of this logic and forces any - # kind of certificate to be obtained with treat_as_renewal = False.) - if not config.duplicate: - identical_names_cert, subset_names_cert = _find_duplicative_certs( - domains, config, configuration.RenewerConfiguration(config)) - # I am not sure whether that correctly reads the systemwide - # configuration file. - question = None - if identical_names_cert is not None: - question = ( - "You have an existing certificate that contains exactly the " - "same domains you requested (ref: {0})\n\nDo you want to " - "renew and replace this certificate with a newly-issued one?" - ).format(identical_names_cert.configfile.filename) - elif subset_names_cert is not None: - question = ( - "You have an existing certificate that contains a portion of " - "the domains you requested (ref: {0})\n\nIt contains these " - "names: {1}\n\nYou requested these names for the new " - "certificate: {2}.\n\nDo you want to replace this existing " - "certificate with the new certificate?" - ).format(subset_names_cert.configfile.filename, - ", ".join(subset_names_cert.names()), - ", ".join(domains)) - if question is None: - # We aren't in a duplicative-names situation at all, so we don't - # have to tell or ask the user anything about this. - pass - elif zope.component.getUtility(interfaces.IDisplay).yesno( - question, "Replace", "Cancel"): - treat_as_renewal = True - else: - reporter_util = zope.component.getUtility(interfaces.IReporter) - reporter_util.add_message( - "To obtain a new certificate that {0} an existing certificate " - "in its domain-name coverage, you must use the --duplicate " - "option.\n\nFor example:\n\n{1} --duplicate {2}".format( - "duplicates" if identical_names_cert is not None else - "overlaps with", sys.argv[0], " ".join(sys.argv[1:])), - reporter_util.HIGH_PRIORITY) - return 1 - # Attempting to obtain the certificate # TODO: Handle errors from _init_le_client? le_client = _init_le_client(args, config, authenticator, installer) - if treat_as_renewal: - lineage = identical_names_cert if identical_names_cert is not None else subset_names_cert - # TODO: Use existing privkey instead of generating a new one - new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) - # TODO: Check whether it worked! - lineage.save_successor( - lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, new_certr.body), - new_key.pem, OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_PEM, new_chain)) - lineage.update_all_links_to(lineage.latest_common_version()) - # TODO: Check return value of save_successor - # TODO: Also update lineage renewal config with any relevant - # configuration values from this attempt? - le_client.deploy_certificate( - domains, lineage.privkey, lineage.cert, lineage.chain) - display_ops.success_renewal(domains) - else: - # TREAT AS NEW REQUEST - lineage = le_client.obtain_and_enroll_certificate( - domains, authenticator, installer, plugins) - if not lineage: - return "Certificate could not be obtained" - # TODO: This treats the key as changed even when it wasn't - # TODO: We also need to pass the fullchain (for Nginx) - le_client.deploy_certificate( - domains, lineage.privkey, lineage.cert, lineage.chain) - le_client.enhance_config(domains, args.redirect) + try: + lineage = auth_from_domains(le_client, config, domains, plugins) + except errors.Error as err: + return str(err) + + # TODO: We also need to pass the fullchain (for Nginx) + le_client.deploy_certificate( + domains, lineage.privkey, lineage.cert, lineage.chain) + le_client.enhance_config(domains, args.redirect) + + if lineage.available_versions("cert") == 1: display_ops.success_installation(domains) + else: + display_ops.success_renewal(domains) def auth(args, config, plugins): @@ -322,6 +355,7 @@ def auth(args, config, plugins): # TODO: Handle errors from _init_le_client? le_client = _init_le_client(args, config, authenticator, installer) + # This is a special case; cert and chain are simply saved if args.csr is not None: certr, chain = le_client.obtain_certificate_from_csr(le_util.CSR( file=args.csr[0], data=args.csr[1], form="der")) @@ -329,9 +363,10 @@ def auth(args, config, plugins): certr, chain, args.cert_path, args.chain_path) else: domains = _find_domains(args, installer) - if not le_client.obtain_and_enroll_certificate( - domains, authenticator, installer, plugins): - return "Certificate could not be obtained" + try: + auth_from_domains(le_client, config, domains, plugins) + except errors.Error as err: + return str(err) def install(args, config, plugins): diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 7f40fef5b..131b0b9f0 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -111,6 +111,8 @@ class Client(object): :ivar .AuthHandler auth_handler: Authorizations handler that will dispatch DV and Continuity challenges to appropriate authenticators (providing `.IAuthenticator` interface). + :ivar .IAuthenticator dv_auth: Prepared (`.IAuthenticator.prepare`) + authenticator that can solve the `.constants.DV_CHALLENGES`. :ivar .IInstaller installer: Installer. :ivar acme.client.Client acme: Optional ACME client API handle. You might already have one from `register`. @@ -118,14 +120,10 @@ class Client(object): """ def __init__(self, config, account_, dv_auth, installer, acme=None): - """Initialize a client. - - :param .IAuthenticator dv_auth: Prepared (`.IAuthenticator.prepare`) - authenticator that can solve the `.constants.DV_CHALLENGES`. - - """ + """Initialize a client.""" self.config = config self.account = account_ + self.dv_auth = dv_auth self.installer = installer # Initialize ACME if account is provided diff --git a/letsencrypt/tests/renewer_test.py b/letsencrypt/tests/renewer_test.py index abf7298b2..1235ee329 100644 --- a/letsencrypt/tests/renewer_test.py +++ b/letsencrypt/tests/renewer_test.py @@ -33,7 +33,12 @@ def fill_with_sample_data(rc_object): class BaseRenewableCertTest(unittest.TestCase): + """Base class for setting up Renewable Cert tests. + .. note:: It may be required to write out self.config for + your test. Check :class:`.cli_test.DuplicateCertTest` for an example. + + """ def setUp(self): from letsencrypt import storage self.tempdir = tempfile.mkdtemp() From 23edd48d5adc3de7fdcffc459bcd7eed9f9c0ceb Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 15 Sep 2015 23:34:00 -0700 Subject: [PATCH 03/33] minor fixes --- letsencrypt/cli.py | 9 ++++----- letsencrypt/tests/cli_test.py | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 35a17d0a7..11496b231 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -250,7 +250,7 @@ def _treat_as_renewal(config, domains): "overlaps with", sys.argv[0], " ".join(sys.argv[1:])), reporter_util.HIGH_PRIORITY) raise errors.Error( - "BUser did not use proper CLI and would like " + "User did not use proper CLI and would like " "to reinvoke the client.") if renewal: @@ -259,7 +259,7 @@ def _treat_as_renewal(config, domains): return None -def auth_from_domains(le_client, config, domains, plugins): +def _auth_from_domains(le_client, config, domains, plugins): """Authenticate and enroll certificate.""" # Note: This can raise errors... caught above us though. lineage = _treat_as_renewal(config, domains) @@ -312,12 +312,11 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo domains = _find_domains(args, installer) - # Attempting to obtain the certificate # TODO: Handle errors from _init_le_client? le_client = _init_le_client(args, config, authenticator, installer) try: - lineage = auth_from_domains(le_client, config, domains, plugins) + lineage = _auth_from_domains(le_client, config, domains, plugins) except errors.Error as err: return str(err) @@ -363,7 +362,7 @@ def auth(args, config, plugins): else: domains = _find_domains(args, installer) try: - auth_from_domains(le_client, config, domains, plugins) + _auth_from_domains(le_client, config, domains, plugins) except errors.Error as err: return str(err) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 2da1272fc..584e67feb 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -206,5 +206,5 @@ class DuplicativeCertsTest(renewer_test.BaseRenewableCertTest): self.assertEqual(result, (None, None)) -if __name__ == '__main__': +if __name__ == "__main__": unittest.main() # pragma: no cover From a0d67aeed7eb0a853ce9edebda62213adcdd81ee Mon Sep 17 00:00:00 2001 From: James Kasten Date: Wed, 16 Sep 2015 01:25:08 -0700 Subject: [PATCH 04/33] correct success message for 'run' --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 11496b231..040be2b03 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -325,7 +325,7 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo domains, lineage.privkey, lineage.cert, lineage.chain) le_client.enhance_config(domains, args.redirect) - if lineage.available_versions("cert") == 1: + if len(lineage.available_versions("cert")) == 1: display_ops.success_installation(domains) else: display_ops.success_renewal(domains) From 8b9a66d7ddcd8fbf6d6bfad2ec214ae6ece86bbf Mon Sep 17 00:00:00 2001 From: James Kasten Date: Wed, 16 Sep 2015 12:33:56 -0700 Subject: [PATCH 05/33] Make sure configs directory exists --- letsencrypt/cli.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 040be2b03..b4649a29a 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -172,6 +172,9 @@ def _find_duplicative_certs(domains, config, renew_config): identical_names_cert, subset_names_cert = None, None configs_dir = renew_config.renewal_configs_dir + # Verify the directory is there + le_util.make_or_verify_dir(configs_dir, mode=0o755, uid=os.geteuid()) + cli_config = configuration.RenewerConfiguration(config) for renewal_file in os.listdir(configs_dir): try: @@ -220,15 +223,15 @@ def _treat_as_renewal(config, domains): if ident_names_cert is not None: question = ( "You have an existing certificate that contains exactly the " - "same domains you requested (ref: {0})\n\nDo you want to " + "same domains you requested (ref: {0}){br}{br}Do you want to " "renew and replace this certificate with a newly-issued one?" ).format(ident_names_cert.configfile.filename) elif subset_names_cert is not None: question = ( "You have an existing certificate that contains a portion of " - "the domains you requested (ref: {0})\n\nIt contains these " - "names: {1}\n\nYou requested these names for the new " - "certificate: {2}.\n\nDo you want to replace this existing " + "the domains you requested (ref: {0}){br}{br}It contains these " + "names: {1}{br}{br}You requested these names for the new " + "certificate: {2}.{br}{br}Do you want to replace this existing " "certificate with the new certificate?" ).format(subset_names_cert.configfile.filename, ", ".join(subset_names_cert.names()), @@ -245,7 +248,7 @@ def _treat_as_renewal(config, domains): reporter_util.add_message( "To obtain a new certificate that {0} an existing certificate " "in its domain-name coverage, you must use the --duplicate " - "option.\n\nFor example:\n\n{1} --duplicate {2}".format( + "option.{br}{br}For example:{br}{br}{1} --duplicate {2}".format( "duplicates" if ident_names_cert is not None else "overlaps with", sys.argv[0], " ".join(sys.argv[1:])), reporter_util.HIGH_PRIORITY) @@ -285,7 +288,7 @@ def _auth_from_domains(le_client, config, domains, plugins): return lineage -# TODO: Make run as close to auth + install as possible +# TODO: Make run as close to auth + install as possible # Possible difficulties: args.csr was hacked into auth def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-locals """Obtain a certificate and install.""" From f582a85314f2b09d42c97a02846d77c5c62eea0c Mon Sep 17 00:00:00 2001 From: James Kasten Date: Wed, 16 Sep 2015 13:03:42 -0700 Subject: [PATCH 06/33] mock out make_or_verify --- letsencrypt/tests/cli_test.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 584e67feb..c38ece0e1 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -176,7 +176,8 @@ class DuplicativeCertsTest(renewer_test.BaseRenewableCertTest): def tearDown(self): shutil.rmtree(self.tempdir) - def test_find_duplicative_names(self): + @mock.patch("letsencrypt.le_util.make_or_verify_dir") + def test_find_duplicative_names(self, unused): from letsencrypt.cli import _find_duplicative_certs test_cert = test_util.load_vector("cert-san.pem") with open(self.test_rc.cert, "w") as f: From e8611d299ad490fbefffd039d90d1f2676fd6ebb Mon Sep 17 00:00:00 2001 From: James Kasten Date: Wed, 16 Sep 2015 13:23:46 -0700 Subject: [PATCH 07/33] Cleanup formatting issues --- letsencrypt/cli.py | 16 +++++++++++----- letsencrypt/tests/cli_test.py | 2 +- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index b4649a29a..88266aaeb 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -225,7 +225,7 @@ def _treat_as_renewal(config, domains): "You have an existing certificate that contains exactly the " "same domains you requested (ref: {0}){br}{br}Do you want to " "renew and replace this certificate with a newly-issued one?" - ).format(ident_names_cert.configfile.filename) + ).format(ident_names_cert.configfile.filename, br=os.linesep) elif subset_names_cert is not None: question = ( "You have an existing certificate that contains a portion of " @@ -235,7 +235,8 @@ def _treat_as_renewal(config, domains): "certificate with the new certificate?" ).format(subset_names_cert.configfile.filename, ", ".join(subset_names_cert.names()), - ", ".join(domains)) + ", ".join(domains), + br=os.linesep) if question is None: # We aren't in a duplicative-names situation at all, so we don't # have to tell or ask the user anything about this. @@ -250,7 +251,10 @@ def _treat_as_renewal(config, domains): "in its domain-name coverage, you must use the --duplicate " "option.{br}{br}For example:{br}{br}{1} --duplicate {2}".format( "duplicates" if ident_names_cert is not None else - "overlaps with", sys.argv[0], " ".join(sys.argv[1:])), + "overlaps with", + sys.argv[0], " ".join(sys.argv[1:]), + br=os.linesep + ), reporter_util.HIGH_PRIORITY) raise errors.Error( "User did not use proper CLI and would like " @@ -288,6 +292,7 @@ def _auth_from_domains(le_client, config, domains, plugins): return lineage + # TODO: Make run as close to auth + install as possible # Possible difficulties: args.csr was hacked into auth def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-locals @@ -708,7 +713,7 @@ def create_parser(plugins, args): # For now unfortunately this constant just needs to match the code below; # there isn't an elegant way to autogenerate it in time. -VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes",\ +VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes", "plugins"] @@ -862,7 +867,8 @@ def _handle_exception(exc_type, exc_value, trace, args): """ logger.debug( - "Exiting abnormally:\n%s", + "Exiting abnormally:%s%s", + os.linesep, "".join(traceback.format_exception(exc_type, exc_value, trace))) if issubclass(exc_type, Exception) and (args is None or not args.debug): diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index c38ece0e1..97725a4c7 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -177,7 +177,7 @@ class DuplicativeCertsTest(renewer_test.BaseRenewableCertTest): shutil.rmtree(self.tempdir) @mock.patch("letsencrypt.le_util.make_or_verify_dir") - def test_find_duplicative_names(self, unused): + def test_find_duplicative_names(self, unused): # pylint: disable=unused-argument from letsencrypt.cli import _find_duplicative_certs test_cert = test_util.load_vector("cert-san.pem") with open(self.test_rc.cert, "w") as f: From aa216a96d4ec2ede40dda8dfea81330669dca150 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Tue, 22 Sep 2015 18:24:22 -0700 Subject: [PATCH 08/33] Finished error_handler --- letsencrypt/error_handler.py | 51 +++++++++++++++---------- letsencrypt/tests/error_handler_test.py | 25 ++++++++++++ 2 files changed, 56 insertions(+), 20 deletions(-) create mode 100644 letsencrypt/tests/error_handler_test.py diff --git a/letsencrypt/error_handler.py b/letsencrypt/error_handler.py index 884c73927..b82f49b5a 100644 --- a/letsencrypt/error_handler.py +++ b/letsencrypt/error_handler.py @@ -3,44 +3,55 @@ import os import signal -_SIGNALS = [signal.SIGTERM] if os.name == "nt" else - [signal.SIGTERM, signal.SIGHUP, signal.SIGQUIT, - signal.SIGXCPU, signal.SIGXFSZ, signal.SIGPWR,] +_SIGNALS = ([signal.SIGTERM] if os.name == "nt" else + [signal.SIGTERM, signal.SIGHUP, signal.SIGQUIT, + signal.SIGXCPU, signal.SIGXFSZ, signal.SIGPWR]) -class ErrorHandler(): +class ErrorHandler(object): """Registers and calls cleanup functions in case of an error.""" def __init__(self, func=None): - self.funcs = [] - if func: - self.funcs.append(func) + self.funcs = [func] if func else [] + self.prev_handlers = {} def __enter__(self): self.set_signal_handlers() def __exit__(self, exec_type, exec_value, traceback): if exec_value is not None: - self.cleanup() + self.call_registered() self.reset_signal_handlers() def register(self, func): """Registers func to be called if an error occurs.""" self.funcs.append(func) - - def cleanup(self): - """Calls all registered functions.""" - while self.funcs: - self.funcs.pop()() + + def call_registered(self): + """Calls all functions in the order they were registered.""" + for func in self.funcs: + func() def set_signal_handlers(self): - for signal_type in _SIGNALS: - signal.signal(signal_type, self._signal_handler) + """Sets signal handlers for signals in _SIGNALS.""" + for signum in _SIGNALS: + prev_handler = signal.getsignal(signum) + # If prev_handler is None, the handler was set outside of Python + if prev_handler is not None: + self.prev_handlers[signum] = prev_handler + signal.signal(signum, self._signal_handler) def reset_signal_handlers(self): - for signal_type in _SIGNALS: - signal.signal(signal_type, signal.SIG_DFL) + """Resets signal handlers for signals in _SIGNALS.""" + for signum in self.prev_handlers: + signal.signal(signum, self.prev_handlers[signum]) + self.prev_handlers.clear() - def _signal_handler(self, signum, frame): - self.cleanup() - signal.signal(signal_type, signal.SIG_DFL) + def _signal_handler(self, signum, _): + """Calls registered functions and the previous signal handler. + + :param int signum: number of current signal + + """ + self.call_registered() + signal.signal(signum, self.prev_handlers[signum]) os.kill(os.getpid(), signum) diff --git a/letsencrypt/tests/error_handler_test.py b/letsencrypt/tests/error_handler_test.py new file mode 100644 index 000000000..6c6d02ec3 --- /dev/null +++ b/letsencrypt/tests/error_handler_test.py @@ -0,0 +1,25 @@ +"""Tests for letsencrypt.error_handler.""" +import unittest + +import mock + + +class ErrorHandlerTest(unittest.TestCase): + """Tests for letsencrypt.error_handler.""" + + def setUp(self): + from letsencrypt import error_handler + self.init_func = mock.MagicMock() + self.error_handler = error_handler.ErrorHandler(self.init_func) + + def test_context_manager(self): + try: + with self.error_handler: + raise ValueError + except ValueError: + pass + self.init_func.assert_called_once_with() + + +if __name__ == "__main__": + unittest.main() # pragma: no cover From 2b9f72fc29c2e3bf4b223f37f4e503037b82d548 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 23 Sep 2015 15:02:20 -0700 Subject: [PATCH 09/33] Finished basic crash recovery --- letsencrypt/auth_handler.py | 10 +++----- letsencrypt/client.py | 33 ++++++++++++++----------- letsencrypt/error_handler.py | 4 ++- letsencrypt/interfaces.py | 2 +- letsencrypt/tests/client_test.py | 33 +++++++++++++++++++++++++ letsencrypt/tests/error_handler_test.py | 27 +++++++++++++++++--- 6 files changed, 82 insertions(+), 27 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index 6498a5c19..a285825dc 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -11,6 +11,7 @@ from acme import messages from letsencrypt import achallenges from letsencrypt import constants from letsencrypt import errors +from letsencrypt import error_handler from letsencrypt import interfaces @@ -106,17 +107,12 @@ class AuthHandler(object): """Get Responses for challenges from authenticators.""" cont_resp = [] dv_resp = [] - try: + logger.info("Attempting to set up challenges.") + with error_handler.ErrorHandler(self._cleanup_challenges): if self.cont_c: cont_resp = self.cont_auth.perform(self.cont_c) if self.dv_c: dv_resp = self.dv_auth.perform(self.dv_c) - # This will catch both specific types of errors. - except errors.AuthorizationError: - logger.critical("Failure in setting up challenges.") - logger.info("Attempting to clean up outstanding challenges...") - self._cleanup_challenges() - raise assert len(cont_resp) == len(self.cont_c) assert len(dv_resp) == len(self.dv_c) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 60eaea5a1..3f1f4900b 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -18,6 +18,7 @@ from letsencrypt import constants from letsencrypt import continuity_auth from letsencrypt import crypto_util from letsencrypt import errors +from letsencrypt import error_handler from letsencrypt import interfaces from letsencrypt import le_util from letsencrypt import reverter @@ -364,16 +365,17 @@ class Client(object): chain_path = None if chain_path is None else os.path.abspath(chain_path) - for dom in domains: - # TODO: Provide a fullchain reference for installers like - # nginx that want it - self.installer.deploy_cert( - dom, os.path.abspath(cert_path), - os.path.abspath(privkey_path), chain_path) + with error_handler.ErrorHandler(self.installer.recovery_routine): + for dom in domains: + # TODO: Provide a fullchain reference for installers like + # nginx that want it + self.installer.deploy_cert( + dom, os.path.abspath(cert_path), + os.path.abspath(privkey_path), chain_path) - self.installer.save("Deployed Let's Encrypt Certificate") - # sites may have been enabled / final cleanup - self.installer.restart() + self.installer.save("Deployed Let's Encrypt Certificate") + # sites may have been enabled / final cleanup + self.installer.restart() def enhance_config(self, domains, redirect=None): """Enhance the configuration. @@ -399,6 +401,8 @@ class Client(object): if redirect is None: redirect = enhancements.ask("redirect") + # When support for more enhancements are added, the call to the + # plugin's `enhance` function should be wrapped by an ErrorHandler if redirect: self.redirect_to_ssl(domains) @@ -409,14 +413,13 @@ class Client(object): :type vhost: :class:`letsencrypt.interfaces.IInstaller` """ - for dom in domains: - try: + with error_handler.ErrorHandler(self.installer.recovery_routine): + for dom in domains: + logger.info("Attempting to perform redirect for %s", dom) self.installer.enhance(dom, "redirect") - except errors.PluginError: - logger.warn("Unable to perform redirect for %s", dom) - self.installer.save("Add Redirects") - self.installer.restart() + self.installer.save("Add Redirects") + self.installer.restart() def validate_key_csr(privkey, csr=None): diff --git a/letsencrypt/error_handler.py b/letsencrypt/error_handler.py index b82f49b5a..3fc948b54 100644 --- a/letsencrypt/error_handler.py +++ b/letsencrypt/error_handler.py @@ -11,8 +11,10 @@ _SIGNALS = ([signal.SIGTERM] if os.name == "nt" else class ErrorHandler(object): """Registers and calls cleanup functions in case of an error.""" def __init__(self, func=None): - self.funcs = [func] if func else [] + self.funcs = [] self.prev_handlers = {} + if func: + self.register(func) def __enter__(self): self.set_signal_handlers() diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index af145ab0a..a0d2eb97f 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -321,7 +321,7 @@ class IInstaller(IPlugin): """ - def recovery_routine(self): + def recovery_routine(): """Revert configuration to most recent finalized checkpoint. Remove all changes (temporary and permanent) that have not been diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 93fdf2cd3..0131d3c93 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -178,6 +178,39 @@ class ClientTest(unittest.TestCase): shutil.rmtree(tmp_path) + def test_deploy_certificate(self): + self.assertRaises(errors.Error, self.client.deploy_certificate, + ["foo.bar"], "key", "cert", "chain") + + installer = mock.MagicMock() + self.client.installer = installer + + self.client.deploy_certificate(["foo.bar"], "key", "cert", "chain") + installer.deploy_cert.assert_called_once_with( + "foo.bar", os.path.abspath("cert"), + os.path.abspath("key"), os.path.abspath("chain")) + self.assertTrue(installer.save.call_count == 1) + installer.restart.assert_called_once_with() + + @mock.patch("letsencrypt.client.enhancements") + def test_enhance_config(self, mock_enhancements): + self.assertRaises(errors.Error, + self.client.enhance_config, ["foo.bar"]) + + mock_enhancements.ask.return_value = True + installer = mock.MagicMock() + self.client.installer = installer + + self.client.enhance_config(["foo.bar"]) + installer.enhance.assert_called_once_with("foo.bar", "redirect") + self.assertTrue(installer.save.call_count == 1) + installer.restart.assert_called_once_with() + + installer.enhance.side_effect = errors.PluginError + self.assertRaises(errors.PluginError, + self.client.enhance_config, ["foo.bar"], True) + installer.recovery_routine.assert_called_once_with() + class RollbackTest(unittest.TestCase): """Tests for letsencrypt.client.rollback.""" diff --git a/letsencrypt/tests/error_handler_test.py b/letsencrypt/tests/error_handler_test.py index 6c6d02ec3..6927b32a0 100644 --- a/letsencrypt/tests/error_handler_test.py +++ b/letsencrypt/tests/error_handler_test.py @@ -1,25 +1,46 @@ """Tests for letsencrypt.error_handler.""" +import signal import unittest import mock +from letsencrypt import error_handler + class ErrorHandlerTest(unittest.TestCase): """Tests for letsencrypt.error_handler.""" def setUp(self): - from letsencrypt import error_handler self.init_func = mock.MagicMock() - self.error_handler = error_handler.ErrorHandler(self.init_func) + self.handler = error_handler.ErrorHandler(self.init_func) def test_context_manager(self): try: - with self.error_handler: + with self.handler: raise ValueError except ValueError: pass self.init_func.assert_called_once_with() + @mock.patch('letsencrypt.error_handler.os') + @mock.patch('letsencrypt.error_handler.signal') + def test_signal_handler(self, mock_signal, mock_os): + # pylint: disable=protected-access + mock_signal.getsignal.return_value = signal.SIG_DFL + self.handler.set_signal_handlers() + signal_handler = self.handler._signal_handler + for signum in error_handler._SIGNALS: + mock_signal.signal.assert_any_call(signum, signal_handler) + + signum = error_handler._SIGNALS[0] + signal_handler(signum, None) + self.init_func.assert_called_once_with() + mock_os.kill.assert_called_once_with(mock_os.getpid(), signum) + + self.handler.reset_signal_handlers() + for signum in error_handler._SIGNALS: + mock_signal.signal.assert_any_call(signum, signal.SIG_DFL) + if __name__ == "__main__": unittest.main() # pragma: no cover From 31e9519ef5af39550cd1d333d6c1ecd608f24221 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 23 Sep 2015 15:11:10 -0700 Subject: [PATCH 10/33] Updated null installer interface --- letsencrypt/plugins/null.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/letsencrypt/plugins/null.py b/letsencrypt/plugins/null.py index bc9565e5a..efe041cac 100644 --- a/letsencrypt/plugins/null.py +++ b/letsencrypt/plugins/null.py @@ -47,6 +47,9 @@ class Installer(common.Plugin): def rollback_checkpoints(self, rollback=1): pass # pragma: no cover + def recovery_routine(self): + pass # pragma: no cover + def view_config_changes(self): pass # pragma: no cover From fd0c51e48afef3fb618d5027d4420a921c00f9a3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Thu, 24 Sep 2015 16:23:40 -0700 Subject: [PATCH 11/33] Incorporated Kuba's feedback and better defined corner cases --- letsencrypt/auth_handler.py | 14 ++++--- letsencrypt/client.py | 7 +++- letsencrypt/error_handler.py | 55 +++++++++++++++++++++---- letsencrypt/tests/client_test.py | 4 +- letsencrypt/tests/error_handler_test.py | 19 ++++++--- 5 files changed, 77 insertions(+), 22 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index a285825dc..68aed510a 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -107,12 +107,16 @@ class AuthHandler(object): """Get Responses for challenges from authenticators.""" cont_resp = [] dv_resp = [] - logger.info("Attempting to set up challenges.") with error_handler.ErrorHandler(self._cleanup_challenges): - if self.cont_c: - cont_resp = self.cont_auth.perform(self.cont_c) - if self.dv_c: - dv_resp = self.dv_auth.perform(self.dv_c) + try: + if self.cont_c: + cont_resp = self.cont_auth.perform(self.cont_c) + if self.dv_c: + dv_resp = self.dv_auth.perform(self.dv_c) + except errors.AuthorizationError: + logger.critical("Failure in setting up challenges.") + logger.info("Attempting to clean up outstanding challenges...") + raise assert len(cont_resp) == len(self.cont_c) assert len(dv_resp) == len(self.dv_c) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 3f1f4900b..56d9b1fda 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -415,8 +415,11 @@ class Client(object): """ with error_handler.ErrorHandler(self.installer.recovery_routine): for dom in domains: - logger.info("Attempting to perform redirect for %s", dom) - self.installer.enhance(dom, "redirect") + try: + self.installer.enhance(dom, "redirect") + except errors.PluginError: + logger.warn("Unable to perform redirect for %s", dom) + raise self.installer.save("Add Redirects") self.installer.restart() diff --git a/letsencrypt/error_handler.py b/letsencrypt/error_handler.py index 3fc948b54..fedb66c0e 100644 --- a/letsencrypt/error_handler.py +++ b/letsencrypt/error_handler.py @@ -1,26 +1,58 @@ -"""Registers and calls cleanup functions in case of an error.""" +"""Registers functions to be called if an exception or signal occurs.""" +import logging import os import signal +import traceback +logger = logging.getLogger(__name__) + + +# _SIGNALS stores the signals that will be handled by the ErrorHandler. These +# signals were chosen as their default handler terminates the process and could +# potentially occur from inside Python. Signals such as SIGILL were not +# included as they could be a sign of something devious and we should terminate +# immediately. _SIGNALS = ([signal.SIGTERM] if os.name == "nt" else [signal.SIGTERM, signal.SIGHUP, signal.SIGQUIT, signal.SIGXCPU, signal.SIGXFSZ, signal.SIGPWR]) class ErrorHandler(object): - """Registers and calls cleanup functions in case of an error.""" + """Registers functions to be called if an exception or signal occurs. + + This class allows you to register functions that will be called when + an exception or signal is encountered. The class works best as a + context manager. For example: + + with ErrorHandler(cleanup_func): + do_something() + + If an exception is raised out of do_something, cleanup_func will be + called. The exception is not caught by the ErrorHandler. Similarly, + if a signal is encountered, cleanup_func is called followed by the + previously registered signal handler. + + Every registered function is attempted to be run to completion + exactly once. If a registered function raises an exception, it is + logged and the next function is called. If a (different) handled + signal occurs while calling a registered function, it is attempted + to be called again by the next signal handler. + + """ def __init__(self, func=None): self.funcs = [] self.prev_handlers = {} - if func: + if func is not None: self.register(func) def __enter__(self): self.set_signal_handlers() - def __exit__(self, exec_type, exec_value, traceback): + def __exit__(self, exec_type, exec_value, trace): if exec_value is not None: + logger.debug("Encountered exception:\n%s", "".join( + traceback.format_exception(exec_type, exec_value, trace))) self.call_registered() self.reset_signal_handlers() @@ -29,9 +61,15 @@ class ErrorHandler(object): self.funcs.append(func) def call_registered(self): - """Calls all functions in the order they were registered.""" - for func in self.funcs: - func() + """Calls all registered functions""" + logger.debug("Calling registered functions") + while self.funcs: + try: + self.funcs[-1]() + except Exception as error: # pylint: disable=broad-except + logger.error("Encountered exception during recovery") + logger.exception(error) + self.funcs.pop() def set_signal_handlers(self): """Sets signal handlers for signals in _SIGNALS.""" @@ -48,12 +86,13 @@ class ErrorHandler(object): signal.signal(signum, self.prev_handlers[signum]) self.prev_handlers.clear() - def _signal_handler(self, signum, _): + def _signal_handler(self, signum, unused_frame): """Calls registered functions and the previous signal handler. :param int signum: number of current signal """ + logger.debug("Singal %s encountered", signum) self.call_registered() signal.signal(signum, self.prev_handlers[signum]) os.kill(os.getpid(), signum) diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 0131d3c93..83cd54226 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -189,7 +189,7 @@ class ClientTest(unittest.TestCase): installer.deploy_cert.assert_called_once_with( "foo.bar", os.path.abspath("cert"), os.path.abspath("key"), os.path.abspath("chain")) - self.assertTrue(installer.save.call_count == 1) + self.assertEqual(installer.save.call_count, 1) installer.restart.assert_called_once_with() @mock.patch("letsencrypt.client.enhancements") @@ -203,7 +203,7 @@ class ClientTest(unittest.TestCase): self.client.enhance_config(["foo.bar"]) installer.enhance.assert_called_once_with("foo.bar", "redirect") - self.assertTrue(installer.save.call_count == 1) + self.assertEqual(installer.save.call_count, 1) installer.restart.assert_called_once_with() installer.enhance.side_effect = errors.PluginError diff --git a/letsencrypt/tests/error_handler_test.py b/letsencrypt/tests/error_handler_test.py index 6927b32a0..66acac930 100644 --- a/letsencrypt/tests/error_handler_test.py +++ b/letsencrypt/tests/error_handler_test.py @@ -4,15 +4,17 @@ import unittest import mock -from letsencrypt import error_handler - class ErrorHandlerTest(unittest.TestCase): """Tests for letsencrypt.error_handler.""" def setUp(self): + from letsencrypt import error_handler + self.init_func = mock.MagicMock() self.handler = error_handler.ErrorHandler(self.init_func) + # pylint: disable=protected-access + self.signals = error_handler._SIGNALS def test_context_manager(self): try: @@ -29,18 +31,25 @@ class ErrorHandlerTest(unittest.TestCase): mock_signal.getsignal.return_value = signal.SIG_DFL self.handler.set_signal_handlers() signal_handler = self.handler._signal_handler - for signum in error_handler._SIGNALS: + for signum in self.signals: mock_signal.signal.assert_any_call(signum, signal_handler) - signum = error_handler._SIGNALS[0] + signum = self.signals[0] signal_handler(signum, None) self.init_func.assert_called_once_with() mock_os.kill.assert_called_once_with(mock_os.getpid(), signum) self.handler.reset_signal_handlers() - for signum in error_handler._SIGNALS: + for signum in self.signals: mock_signal.signal.assert_any_call(signum, signal.SIG_DFL) + def test_bad_recovery(self): + bad_func = mock.MagicMock(side_effect=[ValueError]) + self.handler.register(bad_func) + self.handler.call_registered() + self.init_func.assert_called_once_with() + bad_func.assert_called_once_with() + if __name__ == "__main__": unittest.main() # pragma: no cover From fe810020c490d2c1bd8b32f806d28d80ad537ab1 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 25 Sep 2015 13:26:45 -0700 Subject: [PATCH 12/33] Made error logging entries red in the terminal --- letsencrypt/cli.py | 3 +- letsencrypt/colored_logging.py | 47 +++++++++++++++++++++++ letsencrypt/le_util.py | 4 ++ letsencrypt/reporter.py | 6 +-- letsencrypt/tests/colored_logging_test.py | 39 +++++++++++++++++++ 5 files changed, 95 insertions(+), 4 deletions(-) create mode 100644 letsencrypt/colored_logging.py create mode 100644 letsencrypt/tests/colored_logging_test.py diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e4787a849..7cb4a0458 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -24,6 +24,7 @@ from acme import jose import letsencrypt from letsencrypt import account +from letsencrypt import colored_logging from letsencrypt import configuration from letsencrypt import constants from letsencrypt import client @@ -786,7 +787,7 @@ def _setup_logging(args): level = -args.verbose_count * 10 fmt = "%(asctime)s:%(levelname)s:%(name)s:%(message)s" if args.text_mode: - handler = logging.StreamHandler() + handler = colored_logging.StreamHandler() handler.setFormatter(logging.Formatter(fmt)) else: handler = log.DialogHandler() diff --git a/letsencrypt/colored_logging.py b/letsencrypt/colored_logging.py new file mode 100644 index 000000000..89239e2c7 --- /dev/null +++ b/letsencrypt/colored_logging.py @@ -0,0 +1,47 @@ +"""A formatter and StreamHandler for colorizing logging output.""" +import logging +import sys + +from letsencrypt import le_util + + +class StreamHandler(logging.StreamHandler): + """Sends colored logging output to a stream. + + If the specified stream is not a tty, the class works like the + standard logging.StreamHandler. Default red_level is logging.WARNING. + + :ivar bool colored: True if output should be colored + :ivar bool red_level: The level at which to output + + """ + _RED = '\033[31m' + + def __init__(self, stream=None): + super(StreamHandler, self).__init__(stream) + self.colored = (sys.stderr.isatty() if stream is None else + stream.isatty()) + self.set_red_level(logging.WARNING) + + def format(self, record): + """Formats the string representation of record. + + :param logging.LogRecord record: Record to be formatted + + :returns: Formatted, string representation of record + :rtype: str + + """ + output = super(StreamHandler, self).format(record) + if self.colored and record.levelno >= self.red_level: + return ''.join((self._RED, output, le_util.ANSI_SGR_RESET)) + else: + return output + + def set_red_level(self, red_level): + """Sets the level necessary to display output in red. + + :param int red_level: Minimum log level for displaying red text + + """ + self.red_level = red_level diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index ffc7da190..74e03d8a1 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -18,6 +18,10 @@ Key = collections.namedtuple("Key", "file pem") CSR = collections.namedtuple("CSR", "file data form") +# ANSI escape code for resetting output format +ANSI_SGR_RESET = "\033[0m" + + def run_script(params): """Run the script with the given params. diff --git a/letsencrypt/reporter.py b/letsencrypt/reporter.py index 0c4a7b378..86413053e 100644 --- a/letsencrypt/reporter.py +++ b/letsencrypt/reporter.py @@ -9,6 +9,7 @@ import textwrap import zope.interface from letsencrypt import interfaces +from letsencrypt import le_util logger = logging.getLogger(__name__) @@ -30,7 +31,6 @@ class Reporter(object): LOW_PRIORITY = 2 """Low priority constant. See `add_message`.""" - _RESET = '\033[0m' _BOLD = '\033[1m' _msg_type = collections.namedtuple('ReporterMsg', 'priority text on_crash') @@ -87,7 +87,7 @@ class Reporter(object): msg = self.messages.get() if no_exception or msg.on_crash: if bold_on and msg.priority > self.HIGH_PRIORITY: - sys.stdout.write(self._RESET) + sys.stdout.write(le_util.ANSI_SGR_RESET) bold_on = False lines = msg.text.splitlines() print first_wrapper.fill(lines[0]) @@ -95,4 +95,4 @@ class Reporter(object): print "\n".join( next_wrapper.fill(line) for line in lines[1:]) if bold_on: - sys.stdout.write(self._RESET) + sys.stdout.write(le_util.ANSI_SGR_RESET) diff --git a/letsencrypt/tests/colored_logging_test.py b/letsencrypt/tests/colored_logging_test.py new file mode 100644 index 000000000..fc97b2a49 --- /dev/null +++ b/letsencrypt/tests/colored_logging_test.py @@ -0,0 +1,39 @@ +"""Tests for letsencrypt.colored_logging.""" +import logging +import StringIO +import unittest + +from letsencrypt import le_util + + +class StreamHandlerTest(unittest.TestCase): + + def setUp(self): + from letsencrypt import colored_logging + + self.stream = StringIO.StringIO() + self.stream.isatty = lambda: True + self.handler = colored_logging.StreamHandler(self.stream) + + self.logger = logging.getLogger() + self.logger.setLevel(logging.DEBUG) + self.logger.addHandler(self.handler) + + def test_format(self): + msg = 'I did a thing' + self.logger.debug(msg) + self.assertEqual(self.stream.getvalue(), '{0}\n'.format(msg)) + + def test_format_and_red_level(self): + msg = 'I did another thing' + self.handler.set_red_level(logging.DEBUG) + self.logger.debug(msg) + + # pylint: disable=protected-access + expected = '{0}{1}{2}\n'.format(self.handler._RED, msg, + le_util.ANSI_SGR_RESET) + self.assertEqual(self.stream.getvalue(), expected) + + +if __name__ == "__main__": + unittest.main() # pragma: no cover From 817aadae6abd2c8e7ed4c3d038ba7cfe18f93be6 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 25 Sep 2015 13:27:19 -0700 Subject: [PATCH 13/33] Fixed indentation --- letsencrypt/colored_logging.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/colored_logging.py b/letsencrypt/colored_logging.py index 89239e2c7..f5750870e 100644 --- a/letsencrypt/colored_logging.py +++ b/letsencrypt/colored_logging.py @@ -26,7 +26,7 @@ class StreamHandler(logging.StreamHandler): def format(self, record): """Formats the string representation of record. - :param logging.LogRecord record: Record to be formatted + :param logging.LogRecord record: Record to be formatted :returns: Formatted, string representation of record :rtype: str From cfe103b4edc5e8366cccb7e34e1a890fe8ad9bfc Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 25 Sep 2015 20:01:12 -0700 Subject: [PATCH 14/33] unify quotes --- letsencrypt/tests/configuration_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/letsencrypt/tests/configuration_test.py b/letsencrypt/tests/configuration_test.py index 498147c6d..9692f9479 100644 --- a/letsencrypt/tests/configuration_test.py +++ b/letsencrypt/tests/configuration_test.py @@ -59,9 +59,9 @@ class RenewerConfigurationTest(unittest.TestCase): @mock.patch('letsencrypt.configuration.constants') def test_dynamic_dirs(self, constants): - constants.ARCHIVE_DIR = "a" + constants.ARCHIVE_DIR = 'a' constants.LIVE_DIR = 'l' - constants.RENEWAL_CONFIGS_DIR = "renewal_configs" + constants.RENEWAL_CONFIGS_DIR = 'renewal_configs' constants.RENEWER_CONFIG_FILENAME = 'r.conf' self.assertEqual(self.config.archive_dir, '/tmp/config/a') From add23360a560891431013766717d4bbe2af34688 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 25 Sep 2015 20:04:34 -0700 Subject: [PATCH 15/33] Take away confirmation screen for testing --- letsencrypt/cli.py | 6 +++--- tests/integration/_common.sh | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 88266aaeb..6e2849466 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -241,8 +241,8 @@ def _treat_as_renewal(config, domains): # We aren't in a duplicative-names situation at all, so we don't # have to tell or ask the user anything about this. pass - elif zope.component.getUtility(interfaces.IDisplay).yesno( - question, "Replace", "Cancel"): + elif config.no_confirm or zope.component.getUtility( + interfaces.IDisplay).yesno(question, "Replace", "Cancel"): renewal = True else: reporter_util = zope.component.getUtility(interfaces.IReporter) @@ -661,7 +661,7 @@ def create_parser(plugins, args): help="show program's version number and exit") helpful.add( "automation", "--no-confirm", dest="no_confirm", action="store_true", - help="Turn off confirmation screens, currently used for --revoke") + help="Turn off confirmation screens, used for renewal screens") helpful.add( "automation", "--agree-eula", dest="eula", action="store_true", help="Agree to the Let's Encrypt Developer Preview EULA") diff --git a/tests/integration/_common.sh b/tests/integration/_common.sh index c8b142cf2..7897ff1b7 100755 --- a/tests/integration/_common.sh +++ b/tests/integration/_common.sh @@ -23,6 +23,7 @@ letsencrypt_test () { --agree-eula \ --agree-tos \ --email "" \ + --no-confirm \ --debug \ -vvvvvvv \ "$@" From 8bc260dd64fee5ca4c76b957d72c86d70350604e Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 25 Sep 2015 21:45:56 -0700 Subject: [PATCH 16/33] Fix crypto_util tests --- letsencrypt/tests/crypto_util_test.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/letsencrypt/tests/crypto_util_test.py b/letsencrypt/tests/crypto_util_test.py index b248ffd8a..b4d2aa394 100644 --- a/letsencrypt/tests/crypto_util_test.py +++ b/letsencrypt/tests/crypto_util_test.py @@ -6,7 +6,9 @@ import unittest import OpenSSL import mock +import zope.component +from letsencrypt import interfaces from letsencrypt.tests import test_util @@ -20,6 +22,8 @@ class InitSaveKeyTest(unittest.TestCase): """Tests for letsencrypt.crypto_util.init_save_key.""" def setUp(self): logging.disable(logging.CRITICAL) + zope.component.provideUtility( + mock.Mock(strict_permissions=True), interfaces.IConfig) self.key_dir = tempfile.mkdtemp('key_dir') def tearDown(self): @@ -48,6 +52,8 @@ class InitSaveCSRTest(unittest.TestCase): """Tests for letsencrypt.crypto_util.init_save_csr.""" def setUp(self): + zope.component.provideUtility( + mock.Mock(strict_permissions=True), interfaces.IConfig) self.csr_dir = tempfile.mkdtemp('csr_dir') def tearDown(self): From b72f451a1b5056be4d22a32f5bb75f744ff21a33 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 25 Sep 2015 22:26:32 -0700 Subject: [PATCH 17/33] rename certs directory to csr directory --- letsencrypt/client.py | 2 +- letsencrypt/configuration.py | 7 +++---- letsencrypt/constants.py | 4 ++-- letsencrypt/interfaces.py | 7 ++----- letsencrypt/tests/client_test.py | 2 +- letsencrypt/tests/configuration_test.py | 4 ++-- 6 files changed, 11 insertions(+), 15 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 60eaea5a1..7f035dc25 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -211,7 +211,7 @@ class Client(object): # Create CSR from names key = crypto_util.init_save_key( self.config.rsa_key_size, self.config.key_dir) - csr = crypto_util.init_save_csr(key, domains, self.config.cert_dir) + csr = crypto_util.init_save_csr(key, domains, self.config.csr_dir) return self._obtain_certificate(domains, csr) + (key, csr) diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 6f3ece9fd..bd1ba162a 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -18,8 +18,7 @@ class NamespaceConfig(object): paths defined in :py:mod:`letsencrypt.constants`: - `accounts_dir` - - `cert_dir` - - `cert_key_backup` + - `csr_dir` - `in_progress_dir` - `key_dir` - `renewer_config_file` @@ -54,8 +53,8 @@ class NamespaceConfig(object): return os.path.join(self.namespace.work_dir, constants.BACKUP_DIR) @property - def cert_dir(self): # pylint: disable=missing-docstring - return os.path.join(self.namespace.config_dir, constants.CERT_DIR) + def csr_dir(self): # pylint: disable=missing-docstring + return os.path.join(self.namespace.config_dir, constants.CSR_DIR) @property def cert_key_backup(self): # pylint: disable=missing-docstring diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 6c67ce445..0456d3253 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -68,8 +68,8 @@ ACCOUNTS_DIR = "accounts" BACKUP_DIR = "backups" """Directory (relative to `IConfig.work_dir`) where backups are kept.""" -CERT_DIR = "certs" -"""See `.IConfig.cert_dir`.""" +CSR_DIR = "csr" +"""See `.IConfig.csr_dir`.""" CERT_KEY_BACKUP_DIR = "keys-certs" """Directory where all certificates and keys are stored (relative to diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index 5db92b368..139e2e9f4 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -205,12 +205,9 @@ class IConfig(zope.interface.Interface): accounts_dir = zope.interface.Attribute( "Directory where all account information is stored.") backup_dir = zope.interface.Attribute("Configuration backups directory.") - cert_dir = zope.interface.Attribute( + csr_dir = zope.interface.Attribute( "Directory where newly generated Certificate Signing Requests " - "(CSRs) and certificates not enrolled in the renewer are saved.") - cert_key_backup = zope.interface.Attribute( - "Directory where all certificates and keys are stored. " - "Used for easy revocation.") + "(CSRs) are saved.") in_progress_dir = zope.interface.Attribute( "Directory used before a permanent checkpoint is finalized.") key_dir = zope.interface.Attribute("Keys storage.") diff --git a/letsencrypt/tests/client_test.py b/letsencrypt/tests/client_test.py index 93fdf2cd3..fe1cb1243 100644 --- a/letsencrypt/tests/client_test.py +++ b/letsencrypt/tests/client_test.py @@ -113,7 +113,7 @@ class ClientTest(unittest.TestCase): mock_crypto_util.init_save_key.assert_called_once_with( self.config.rsa_key_size, self.config.key_dir) mock_crypto_util.init_save_csr.assert_called_once_with( - mock.sentinel.key, domains, self.config.cert_dir) + mock.sentinel.key, domains, self.config.csr_dir) self._check_obtain_certificate() @mock.patch("letsencrypt.client.zope.component.getUtility") diff --git a/letsencrypt/tests/configuration_test.py b/letsencrypt/tests/configuration_test.py index 498147c6d..79f867be9 100644 --- a/letsencrypt/tests/configuration_test.py +++ b/letsencrypt/tests/configuration_test.py @@ -33,7 +33,7 @@ class NamespaceConfigTest(unittest.TestCase): constants.ACCOUNTS_DIR = 'acc' constants.BACKUP_DIR = 'backups' constants.CERT_KEY_BACKUP_DIR = 'c/' - constants.CERT_DIR = 'certs' + constants.CSR_DIR = 'csr' constants.IN_PROGRESS_DIR = '../p' constants.KEY_DIR = 'keys' constants.TEMP_CHECKPOINT_DIR = 't' @@ -41,7 +41,7 @@ class NamespaceConfigTest(unittest.TestCase): self.assertEqual( self.config.accounts_dir, '/tmp/config/acc/acme-server.org:443/new') self.assertEqual(self.config.backup_dir, '/tmp/foo/backups') - self.assertEqual(self.config.cert_dir, '/tmp/config/certs') + self.assertEqual(self.config.csr_dir, '/tmp/config/csr') self.assertEqual( self.config.cert_key_backup, '/tmp/foo/c/acme-server.org:443/new') self.assertEqual(self.config.in_progress_dir, '/tmp/foo/../p') From 022c5c3c243c321b7e2956876ebc95f8e2d0af75 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 25 Sep 2015 22:35:43 -0700 Subject: [PATCH 18/33] Remove revoker and associated code --- letsencrypt/configuration.py | 6 - letsencrypt/constants.py | 4 - letsencrypt/interfaces.py | 3 - letsencrypt/revoker.py | 560 ------------------------ letsencrypt/tests/configuration_test.py | 3 - letsencrypt/tests/revoker_test.py | 409 ----------------- 6 files changed, 985 deletions(-) delete mode 100644 letsencrypt/revoker.py delete mode 100644 letsencrypt/tests/revoker_test.py diff --git a/letsencrypt/configuration.py b/letsencrypt/configuration.py index 6f3ece9fd..ec8ddb14e 100644 --- a/letsencrypt/configuration.py +++ b/letsencrypt/configuration.py @@ -19,7 +19,6 @@ class NamespaceConfig(object): - `accounts_dir` - `cert_dir` - - `cert_key_backup` - `in_progress_dir` - `key_dir` - `renewer_config_file` @@ -57,11 +56,6 @@ class NamespaceConfig(object): def cert_dir(self): # pylint: disable=missing-docstring return os.path.join(self.namespace.config_dir, constants.CERT_DIR) - @property - def cert_key_backup(self): # pylint: disable=missing-docstring - return os.path.join(self.namespace.work_dir, - constants.CERT_KEY_BACKUP_DIR, self.server_path) - @property def in_progress_dir(self): # pylint: disable=missing-docstring return os.path.join(self.namespace.work_dir, constants.IN_PROGRESS_DIR) diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 6c67ce445..adca4ed02 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -71,10 +71,6 @@ BACKUP_DIR = "backups" CERT_DIR = "certs" """See `.IConfig.cert_dir`.""" -CERT_KEY_BACKUP_DIR = "keys-certs" -"""Directory where all certificates and keys are stored (relative to -`IConfig.work_dir`). Used for easy revocation.""" - IN_PROGRESS_DIR = "IN_PROGRESS" """Directory used before a permanent checkpoint is finalized (relative to `IConfig.work_dir`).""" diff --git a/letsencrypt/interfaces.py b/letsencrypt/interfaces.py index 5db92b368..345a0d779 100644 --- a/letsencrypt/interfaces.py +++ b/letsencrypt/interfaces.py @@ -208,9 +208,6 @@ class IConfig(zope.interface.Interface): cert_dir = zope.interface.Attribute( "Directory where newly generated Certificate Signing Requests " "(CSRs) and certificates not enrolled in the renewer are saved.") - cert_key_backup = zope.interface.Attribute( - "Directory where all certificates and keys are stored. " - "Used for easy revocation.") in_progress_dir = zope.interface.Attribute( "Directory used before a permanent checkpoint is finalized.") key_dir = zope.interface.Attribute("Keys storage.") diff --git a/letsencrypt/revoker.py b/letsencrypt/revoker.py deleted file mode 100644 index 32c6f003d..000000000 --- a/letsencrypt/revoker.py +++ /dev/null @@ -1,560 +0,0 @@ -"""Revoker module to enable LE revocations. - -The backend of this module would fit a database quite nicely, but in order to -minimize dependencies and maintain transparency, the class currently implements -its own storage system. The number of certs that will likely be stored on any -given client might not warrant requiring a database. - -""" -import collections -import csv -import logging -import os -import shutil -import tempfile - -import OpenSSL - -from acme import client as acme_client -from acme import crypto_util as acme_crypto_util -from acme.jose import util as jose_util - -from letsencrypt import crypto_util -from letsencrypt import errors -from letsencrypt import le_util - -from letsencrypt.display import util as display_util -from letsencrypt.display import revocation - - -logger = logging.getLogger(__name__) - - -class Revoker(object): - """A revocation class for LE. - - .. todo:: Add a method to specify your own certificate for revocation - CLI - - :ivar .acme.client.Client acme: ACME client - - :ivar installer: Installer object - :type installer: :class:`~letsencrypt.interfaces.IInstaller` - - :ivar config: Configuration. - :type config: :class:`~letsencrypt.interfaces.IConfig` - - :ivar bool no_confirm: Whether or not to ask for confirmation for revocation - - """ - def __init__(self, installer, config, no_confirm=False): - # XXX - self.acme = acme_client.Client(directory=None, key=None, alg=None) - - self.installer = installer - self.config = config - self.no_confirm = no_confirm - - le_util.make_or_verify_dir(config.cert_key_backup, 0o700, os.geteuid(), - self.config.strict_permissions) - - # TODO: Find a better solution for this... - self.list_path = os.path.join(config.cert_key_backup, "LIST") - # Make sure that the file is available for use for rest of class - open(self.list_path, "a").close() - - def revoke_from_key(self, authkey): - """Revoke all certificates under an authorized key. - - :param authkey: Authorized key used in previous transactions - :type authkey: :class:`letsencrypt.le_util.Key` - - """ - certs = [] - try: - clean_pem = OpenSSL.crypto.dump_privatekey( - OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.load_privatekey( - OpenSSL.crypto.FILETYPE_PEM, authkey.pem)) - except OpenSSL.crypto.Error as error: - logger.debug(error, exc_info=True) - raise errors.RevokerError( - "Invalid key file specified to revoke_from_key") - - with open(self.list_path, "rb") as csvfile: - csvreader = csv.reader(csvfile) - for row in csvreader: - # idx, cert, key - # Add all keys that match to marked list - # Note: The key can be different than the pub key found in the - # certificate. - _, b_k = self._row_to_backup(row) - try: - test_pem = OpenSSL.crypto.dump_privatekey( - OpenSSL.crypto.FILETYPE_PEM, OpenSSL.crypto.load_privatekey( - OpenSSL.crypto.FILETYPE_PEM, open(b_k).read())) - except OpenSSL.crypto.Error as error: - logger.debug(error, exc_info=True) - # This should never happen given the assumptions of the - # module. If it does, it is probably best to delete the - # the offending key/cert. For now... just raise an exception - raise errors.RevokerError("%s - backup file is corrupted.") - - if clean_pem == test_pem: - certs.append( - Cert.fromrow(row, self.config.cert_key_backup)) - if certs: - self._safe_revoke(certs) - else: - logger.info("No certificates using the authorized key were found.") - - def revoke_from_cert(self, cert_path): - """Revoke a certificate by specifying a file path. - - .. todo:: Add the ability to revoke the certificate even if the cert - is not stored locally. A path to the auth key will need to be - attained from the user. - - :param str cert_path: path to ACME certificate in pem form - - """ - # Locate the correct certificate (do not rely on filename) - cert_to_revoke = Cert(cert_path) - - with open(self.list_path, "rb") as csvfile: - csvreader = csv.reader(csvfile) - for row in csvreader: - cert = Cert.fromrow(row, self.config.cert_key_backup) - - if cert.get_der() == cert_to_revoke.get_der(): - self._safe_revoke([cert]) - return - - logger.info("Associated ACME certificate was not found.") - - def revoke_from_menu(self): - """List trusted Let's Encrypt certificates.""" - - csha1_vhlist = self._get_installed_locations() - certs = self._populate_saved_certs(csha1_vhlist) - - while True: - if certs: - code, selection = revocation.display_certs(certs) - - if code == display_util.OK: - revoked_certs = self._safe_revoke([certs[selection]]) - # Since we are currently only revoking one cert at a time... - if revoked_certs: - del certs[selection] - elif code == display_util.HELP: - revocation.more_info_cert(certs[selection]) - else: - return - else: - logger.info( - "There are not any trusted Let's Encrypt " - "certificates for this server.") - return - - def _populate_saved_certs(self, csha1_vhlist): - # pylint: disable=no-self-use - """Populate a list of all the saved certs. - - It is important to read from the file rather than the directory. - We assume that the LIST file is the master record and depending on - program crashes, this may differ from what is actually in the directory. - Namely, additional certs/keys may exist. There should never be any - certs/keys in the LIST that don't exist in the directory however. - - :param dict csha1_vhlist: map from cert sha1 fingerprints to a list - of it's installed location paths. - - """ - certs = [] - with open(self.list_path, "rb") as csvfile: - csvreader = csv.reader(csvfile) - # idx, orig_cert, orig_key - for row in csvreader: - cert = Cert.fromrow(row, self.config.cert_key_backup) - - # If we were able to find the cert installed... update status - cert.installed = csha1_vhlist.get(cert.get_fingerprint(), []) - - certs.append(cert) - - return certs - - def _get_installed_locations(self): - """Get installed locations of certificates. - - :returns: map from cert sha1 fingerprint to :class:`list` of vhosts - where the certificate is installed. - - """ - csha1_vhlist = {} - - if self.installer is None: - return csha1_vhlist - - for (cert_path, _, path) in self.installer.get_all_certs_keys(): - try: - with open(cert_path) as cert_file: - cert_data = cert_file.read() - except IOError: - continue - try: - cert_obj, _ = crypto_util.pyopenssl_load_certificate(cert_data) - except errors.Error: - continue - cert_sha1 = cert_obj.digest("sha1") - if cert_sha1 in csha1_vhlist: - csha1_vhlist[cert_sha1].append(path) - else: - csha1_vhlist[cert_sha1] = [path] - - return csha1_vhlist - - def _safe_revoke(self, certs): - """Confirm and revoke certificates. - - :param certs: certs intended to be revoked - :type certs: :class:`list` of :class:`letsencrypt.revoker.Cert` - - :returns: certs successfully revoked - :rtype: :class:`list` of :class:`letsencrypt.revoker.Cert` - - """ - success_list = [] - try: - for cert in certs: - if self.no_confirm or revocation.confirm_revocation(cert): - try: - self._acme_revoke(cert) - except errors.Error: - # TODO: Improve error handling when networking is set... - logger.error( - "Unable to revoke cert:%s%s", os.linesep, str(cert)) - success_list.append(cert) - revocation.success_revocation(cert) - finally: - if success_list: - self._remove_certs_keys(success_list) - - return success_list - - def _acme_revoke(self, cert): - """Revoke the certificate with the ACME server. - - :param cert: certificate to revoke - :type cert: :class:`letsencrypt.revoker.Cert` - - :returns: TODO - - """ - # XXX | pylint: disable=unused-variable - - # pylint: disable=protected-access - certificate = jose_util.ComparableX509(cert._cert) - try: - with open(cert.backup_key_path, "rU") as backup_key_file: - key = OpenSSL.crypto.load_privatekey( - OpenSSL.crypto.FILETYPE_PEM, backup_key_file.read()) - # If the key file doesn't exist... or is corrupted - except OpenSSL.crypto.Error as error: - logger.debug(error, exc_info=True) - raise errors.RevokerError( - "Corrupted backup key file: %s" % cert.backup_key_path) - - return self.acme.revoke(cert=None) # XXX - - def _remove_certs_keys(self, cert_list): # pylint: disable=no-self-use - """Remove certificate and key. - - :param list cert_list: Must contain certs, each is of type - :class:`letsencrypt.revoker.Cert` - - """ - # This must occur first, LIST is the official key - self._remove_certs_from_list(cert_list) - - # Remove files - for cert in cert_list: - os.remove(cert.backup_path) - os.remove(cert.backup_key_path) - - def _remove_certs_from_list(self, cert_list): # pylint: disable=no-self-use - """Remove a certificate from the LIST file. - - :param list cert_list: Must contain valid certs, each is of type - :class:`letsencrypt.revoker.Cert` - - """ - newfile_handle, list_path2 = tempfile.mkstemp(".tmp", "LIST") - idx = 0 - - with open(self.list_path, "rb") as orgfile: - csvreader = csv.reader(orgfile) - with os.fdopen(newfile_handle, "wb") as newfile: - csvwriter = csv.writer(newfile) - - for row in csvreader: - if idx >= len(cert_list) or row != cert_list[idx].get_row(): - csvwriter.writerow(row) - else: - idx += 1 - - # This should never happen... - if idx != len(cert_list): - raise errors.RevokerError( - "Did not find all cert_list items to remove from LIST") - - shutil.copy2(list_path2, self.list_path) - os.remove(list_path2) - - def _row_to_backup(self, row): - """Convenience function - - :param list row: csv file row 'idx', 'cert_path', 'key_path' - - :returns: tuple of the form ('backup_cert_path', 'backup_key_path') - :rtype: tuple - - """ - return (self._get_backup(self.config.cert_key_backup, row[0], row[1]), - self._get_backup(self.config.cert_key_backup, row[0], row[2])) - - @classmethod - def store_cert_key(cls, cert_path, key_path, config): - """Store certificate key. (Used to allow quick revocation) - - :param str cert_path: Path to a certificate file. - :param str key_path: Path to authorized key for certificate - - :ivar config: Configuration. - :type config: :class:`~letsencrypt.interfaces.IConfig` - - """ - list_path = os.path.join(config.cert_key_backup, "LIST") - le_util.make_or_verify_dir(config.cert_key_backup, 0o700, os.geteuid(), - config.strict_permissions) - - cls._catalog_files( - config.cert_key_backup, cert_path, key_path, list_path) - - @classmethod - def _catalog_files(cls, backup_dir, cert_path, key_path, list_path): - idx = 0 - if os.path.isfile(list_path): - with open(list_path, "r+b") as csvfile: - csvreader = csv.reader(csvfile) - - # Find the highest index in the file - for row in csvreader: - idx = int(row[0]) + 1 - csvwriter = csv.writer(csvfile) - # You must move the files before appending the row - cls._copy_files(backup_dir, idx, cert_path, key_path) - csvwriter.writerow([str(idx), cert_path, key_path]) - - else: - with open(list_path, "wb") as csvfile: - csvwriter = csv.writer(csvfile) - # You must move the files before appending the row - cls._copy_files(backup_dir, idx, cert_path, key_path) - csvwriter.writerow([str(idx), cert_path, key_path]) - - @classmethod - def _copy_files(cls, backup_dir, idx, cert_path, key_path): - """Copies the files into the backup dir appropriately.""" - shutil.copy2(cert_path, cls._get_backup(backup_dir, idx, cert_path)) - shutil.copy2(key_path, cls._get_backup(backup_dir, idx, key_path)) - - @classmethod - def _get_backup(cls, backup_dir, idx, orig_path): - """Returns the path to the backup.""" - return os.path.join( - backup_dir, "{name}_{idx}".format( - name=os.path.basename(orig_path), idx=str(idx))) - - -class Cert(object): - """Cert object used for Revocation convenience. - - :ivar _cert: Certificate - :type _cert: :class:`OpenSSL.crypto.X509` - - :ivar int idx: convenience index used for listing - :ivar orig: (`str` path - original certificate, `str` status) - :type orig: :class:`PathStatus` - :ivar orig_key: (`str` path - original auth key, `str` status) - :type orig_key: :class:`PathStatus` - :ivar str backup_path: backup filepath of the certificate - :ivar str backup_key_path: backup filepath of the authorized key - - :ivar list installed: `list` of `str` describing all locations the cert - is installed - - """ - PathStatus = collections.namedtuple("PathStatus", "path status") - """Convenience container to hold path and status info""" - - DELETED_MSG = "This file has been moved or deleted" - CHANGED_MSG = "This file has changed" - - def __init__(self, cert_path): - """Cert initialization - - :param str cert_filepath: Name of file containing certificate in - PEM format. - - """ - try: - with open(cert_path) as cert_file: - cert_data = cert_file.read() - except IOError: - raise errors.RevokerError( - "Error loading certificate: %s" % cert_path) - - try: - self._cert = OpenSSL.crypto.load_certificate( - OpenSSL.crypto.FILETYPE_PEM, cert_data) - except OpenSSL.crypto.Error: - raise errors.RevokerError( - "Error loading certificate: %s" % cert_path) - - self.idx = -1 - - self.orig = None - self.orig_key = None - self.backup_path = "" - self.backup_key_path = "" - - self.installed = ["Unknown"] - - @classmethod - def fromrow(cls, row, backup_dir): - # pylint: disable=protected-access - """Initialize Cert from a csv row.""" - idx = int(row[0]) - backup = Revoker._get_backup(backup_dir, idx, row[1]) - backup_key = Revoker._get_backup(backup_dir, idx, row[2]) - - obj = cls(backup) - obj.add_meta(idx, row[1], row[2], backup, backup_key) - return obj - - def get_row(self): - """Returns a list in CSV format. If meta data is available.""" - if self.orig is not None and self.orig_key is not None: - return [str(self.idx), self.orig.path, self.orig_key.path] - return None - - def add_meta(self, idx, orig, orig_key, backup, backup_key): - """Add meta data to cert - - :param int idx: convenience index for revoker - :param tuple orig: (`str` original certificate filepath, `str` status) - :param tuple orig_key: (`str` original auth key path, `str` status) - :param str backup: backup certificate filepath - :param str backup_key: backup key filepath - - """ - status = "" - key_status = "" - - # Verify original cert path - if not os.path.isfile(orig): - status = Cert.DELETED_MSG - else: - with open(orig) as orig_file: - orig_data = orig_file.read() - o_cert = OpenSSL.crypto.load_certificate( - OpenSSL.crypto.FILETYPE_PEM, orig_data) - if self.get_fingerprint() != o_cert.digest("sha1"): - status = Cert.CHANGED_MSG - - # Verify original key path - if not os.path.isfile(orig_key): - key_status = Cert.DELETED_MSG - else: - with open(orig_key, "r") as fd: - key_pem = fd.read() - with open(backup_key, "r") as fd: - backup_key_pem = fd.read() - if key_pem != backup_key_pem: - key_status = Cert.CHANGED_MSG - - self.idx = idx - self.orig = Cert.PathStatus(orig, status) - self.orig_key = Cert.PathStatus(orig_key, key_status) - self.backup_path = backup - self.backup_key_path = backup_key - - def get_cn(self): - """Get common name.""" - return self._cert.get_subject().CN - - def get_fingerprint(self): - """Get SHA1 fingerprint.""" - return self._cert.digest("sha1") - - def get_not_before(self): - """Get not_valid_before field.""" - return crypto_util.asn1_generalizedtime_to_dt( - self._cert.get_notBefore()) - - def get_not_after(self): - """Get not_valid_after field.""" - return crypto_util.asn1_generalizedtime_to_dt( - self._cert.get_notAfter()) - - def get_der(self): - """Get certificate in der format.""" - return OpenSSL.crypto.dump_certificate( - OpenSSL.crypto.FILETYPE_ASN1, self._cert) - - def get_pub_key(self): - """Get public key size. - - .. todo:: Support for ECC - - """ - return "RSA {0}".format(self._cert.get_pubkey().bits) - - def get_san(self): - """Get subject alternative name if available.""" - # pylint: disable=protected-access - return ", ".join(acme_crypto_util._pyopenssl_cert_or_req_san(self._cert)) - - def __str__(self): - text = [ - "Subject: %s" % crypto_util.pyopenssl_x509_name_as_text( - self._cert.get_subject()), - "SAN: %s" % self.get_san(), - "Issuer: %s" % crypto_util.pyopenssl_x509_name_as_text( - self._cert.get_issuer()), - "Public Key: %s" % self.get_pub_key(), - "Not Before: %s" % str(self.get_not_before()), - "Not After: %s" % str(self.get_not_after()), - "Serial Number: %s" % self._cert.get_serial_number(), - "SHA1: %s%s" % (self.get_fingerprint(), os.linesep), - "Installed: %s" % ", ".join(self.installed), - ] - - if self.orig is not None: - if self.orig.status == "": - text.append("Path: %s" % self.orig.path) - else: - text.append("Orig Path: %s (%s)" % self.orig) - if self.orig_key is not None: - if self.orig_key.status == "": - text.append("Auth Key Path: %s" % self.orig_key.path) - else: - text.append("Orig Auth Key Path: %s (%s)" % self.orig_key) - - text.append("") - return os.linesep.join(text) - - def pretty_print(self): - """Nicely frames a cert str""" - frame = "-" * (display_util.WIDTH - 4) + os.linesep - return "{frame}{cert}{frame}".format(frame=frame, cert=str(self)) diff --git a/letsencrypt/tests/configuration_test.py b/letsencrypt/tests/configuration_test.py index 498147c6d..110bfe223 100644 --- a/letsencrypt/tests/configuration_test.py +++ b/letsencrypt/tests/configuration_test.py @@ -32,7 +32,6 @@ class NamespaceConfigTest(unittest.TestCase): def test_dynamic_dirs(self, constants): constants.ACCOUNTS_DIR = 'acc' constants.BACKUP_DIR = 'backups' - constants.CERT_KEY_BACKUP_DIR = 'c/' constants.CERT_DIR = 'certs' constants.IN_PROGRESS_DIR = '../p' constants.KEY_DIR = 'keys' @@ -42,8 +41,6 @@ class NamespaceConfigTest(unittest.TestCase): self.config.accounts_dir, '/tmp/config/acc/acme-server.org:443/new') self.assertEqual(self.config.backup_dir, '/tmp/foo/backups') self.assertEqual(self.config.cert_dir, '/tmp/config/certs') - self.assertEqual( - self.config.cert_key_backup, '/tmp/foo/c/acme-server.org:443/new') self.assertEqual(self.config.in_progress_dir, '/tmp/foo/../p') self.assertEqual(self.config.key_dir, '/tmp/config/keys') self.assertEqual(self.config.temp_checkpoint_dir, '/tmp/foo/t') diff --git a/letsencrypt/tests/revoker_test.py b/letsencrypt/tests/revoker_test.py deleted file mode 100644 index 87dab4eb8..000000000 --- a/letsencrypt/tests/revoker_test.py +++ /dev/null @@ -1,409 +0,0 @@ -"""Test letsencrypt.revoker.""" -import csv -import os -import shutil -import tempfile -import unittest - -import mock -import OpenSSL - -from letsencrypt import errors -from letsencrypt import le_util -from letsencrypt.display import util as display_util - -from letsencrypt.tests import test_util - - -KEY = OpenSSL.crypto.load_privatekey( - OpenSSL.crypto.FILETYPE_PEM, test_util.load_vector("rsa512_key.pem")) - - -class RevokerBase(unittest.TestCase): # pylint: disable=too-few-public-methods - """Base Class for Revoker Tests.""" - def setUp(self): - self.paths, self.certs, self.key_path = create_revoker_certs() - - self.backup_dir = tempfile.mkdtemp("cert_backup") - self.mock_config = mock.MagicMock(cert_key_backup=self.backup_dir) - - self.list_path = os.path.join(self.backup_dir, "LIST") - - def _store_certs(self): - # pylint: disable=protected-access - from letsencrypt.revoker import Revoker - Revoker.store_cert_key(self.paths[0], self.key_path, self.mock_config) - Revoker.store_cert_key(self.paths[1], self.key_path, self.mock_config) - - # Set metadata - for i in xrange(2): - self.certs[i].add_meta( - i, self.paths[i], self.key_path, - Revoker._get_backup(self.backup_dir, i, self.paths[i]), - Revoker._get_backup(self.backup_dir, i, self.key_path)) - - def _get_rows(self): - with open(self.list_path, "rb") as csvfile: - return [row for row in csv.reader(csvfile)] - - def _write_rows(self, rows): - with open(self.list_path, "wb") as csvfile: - csvwriter = csv.writer(csvfile) - for row in rows: - csvwriter.writerow(row) - - -class RevokerTest(RevokerBase): - def setUp(self): - from letsencrypt.revoker import Revoker - super(RevokerTest, self).setUp() - - with open(self.key_path) as key_file: - self.key = le_util.Key(self.key_path, key_file.read()) - - self._store_certs() - - self.revoker = Revoker( - installer=mock.MagicMock(), config=self.mock_config) - - def tearDown(self): - shutil.rmtree(self.backup_dir) - - @mock.patch("acme.client.Client.revoke") - @mock.patch("letsencrypt.revoker.revocation") - def test_revoke_by_key_all(self, mock_display, mock_acme): - mock_display().confirm_revocation.return_value = True - - self.revoker.revoke_from_key(self.key) - self.assertEqual(self._get_rows(), []) - - # Check to make sure backups were eliminated - for i in xrange(2): - self.assertFalse(self._backups_exist(self.certs[i].get_row())) - - self.assertEqual(mock_acme.call_count, 2) - - @mock.patch("letsencrypt.revoker.OpenSSL.crypto.load_privatekey") - def test_revoke_by_invalid_keys(self, mock_load_privatekey): - mock_load_privatekey.side_effect = OpenSSL.crypto.Error - self.assertRaises( - errors.RevokerError, self.revoker.revoke_from_key, self.key) - - mock_load_privatekey.side_effect = [KEY, OpenSSL.crypto.Error] - self.assertRaises( - errors.RevokerError, self.revoker.revoke_from_key, self.key) - - @mock.patch("acme.client.Client.revoke") - @mock.patch("letsencrypt.revoker.revocation") - def test_revoke_by_wrong_key(self, mock_display, mock_acme): - mock_display().confirm_revocation.return_value = True - - key_path = test_util.vector_path("rsa256_key.pem") - - wrong_key = le_util.Key(key_path, open(key_path).read()) - self.revoker.revoke_from_key(wrong_key) - - # Nothing was removed - self.assertEqual(len(self._get_rows()), 2) - # No revocation went through - self.assertEqual(mock_acme.call_count, 0) - - @mock.patch("acme.client.Client.revoke") - @mock.patch("letsencrypt.revoker.revocation") - def test_revoke_by_cert(self, mock_display, mock_acme): - mock_display().confirm_revocation.return_value = True - - self.revoker.revoke_from_cert(self.paths[1]) - - row0 = self.certs[0].get_row() - row1 = self.certs[1].get_row() - - self.assertEqual(self._get_rows(), [row0]) - - self.assertTrue(self._backups_exist(row0)) - self.assertFalse(self._backups_exist(row1)) - - self.assertEqual(mock_acme.call_count, 1) - - @mock.patch("acme.client.Client.revoke") - @mock.patch("letsencrypt.revoker.revocation") - def test_revoke_by_cert_not_found(self, mock_display, mock_acme): - mock_display().confirm_revocation.return_value = True - - self.revoker.revoke_from_cert(self.paths[0]) - self.revoker.revoke_from_cert(self.paths[0]) - - row0 = self.certs[0].get_row() - row1 = self.certs[1].get_row() - - # Same check as last time... just reversed. - self.assertEqual(self._get_rows(), [row1]) - - self.assertTrue(self._backups_exist(row1)) - self.assertFalse(self._backups_exist(row0)) - - self.assertEqual(mock_acme.call_count, 1) - - @mock.patch("acme.client.Client.revoke") - @mock.patch("letsencrypt.revoker.revocation") - def test_revoke_by_menu(self, mock_display, mock_acme): - mock_display().confirm_revocation.return_value = True - mock_display.display_certs.side_effect = [ - (display_util.HELP, 0), - (display_util.OK, 0), - (display_util.CANCEL, -1), - ] - - self.revoker.revoke_from_menu() - - row0 = self.certs[0].get_row() - row1 = self.certs[1].get_row() - - self.assertEqual(self._get_rows(), [row1]) - - self.assertFalse(self._backups_exist(row0)) - self.assertTrue(self._backups_exist(row1)) - - self.assertEqual(mock_acme.call_count, 1) - self.assertEqual(mock_display.more_info_cert.call_count, 1) - - @mock.patch("letsencrypt.revoker.logger") - @mock.patch("acme.client.Client.revoke") - @mock.patch("letsencrypt.revoker.revocation") - def test_revoke_by_menu_delete_all(self, mock_display, mock_acme, mock_log): - mock_display().confirm_revocation.return_value = True - mock_display.display_certs.return_value = (display_util.OK, 0) - - self.revoker.revoke_from_menu() - - self.assertEqual(self._get_rows(), []) - - # Everything should be deleted... - for i in xrange(2): - self.assertFalse(self._backups_exist(self.certs[i].get_row())) - - self.assertEqual(mock_acme.call_count, 2) - # Info is called when there aren't any certs left... - self.assertTrue(mock_log.info.called) - - @mock.patch("letsencrypt.revoker.revocation") - @mock.patch("letsencrypt.revoker.Revoker._acme_revoke") - @mock.patch("letsencrypt.revoker.logger") - def test_safe_revoke_acme_fail(self, mock_log, mock_revoke, mock_display): - # pylint: disable=protected-access - mock_revoke.side_effect = errors.Error - mock_display().confirm_revocation.return_value = True - - self.revoker._safe_revoke(self.certs) - self.assertTrue(mock_log.error.called) - - @mock.patch("letsencrypt.revoker.OpenSSL.crypto.load_privatekey") - def test_acme_revoke_failure(self, mock_load_privatekey): - # pylint: disable=protected-access - mock_load_privatekey.side_effect = OpenSSL.crypto.Error - self.assertRaises( - errors.Error, self.revoker._acme_revoke, self.certs[0]) - - def test_remove_certs_from_list_bad_certs(self): - # pylint: disable=protected-access - from letsencrypt.revoker import Cert - - new_cert = Cert(self.paths[0]) - - # This isn't stored in the db - new_cert.idx = 10 - new_cert.backup_path = self.paths[0] - new_cert.backup_key_path = self.key_path - new_cert.orig = Cert.PathStatus("false path", "not here") - new_cert.orig_key = Cert.PathStatus("false path", "not here") - - self.assertRaises(errors.RevokerError, - self.revoker._remove_certs_from_list, [new_cert]) - - def _backups_exist(self, row): - # pylint: disable=protected-access - cert_path, key_path = self.revoker._row_to_backup(row) - return os.path.isfile(cert_path) and os.path.isfile(key_path) - - -class RevokerInstallerTest(RevokerBase): - def setUp(self): - super(RevokerInstallerTest, self).setUp() - - self.installs = [ - ["installation/path0a", "installation/path0b"], - ["installation/path1"], - ] - - self.certs_keys = [ - (self.paths[0], self.key_path, self.installs[0][0]), - (self.paths[0], self.key_path, self.installs[0][1]), - (self.paths[1], self.key_path, self.installs[1][0]), - ] - - self._store_certs() - - def _get_revoker(self, installer): - from letsencrypt.revoker import Revoker - return Revoker(installer, self.mock_config) - - def test_no_installer_get_installed_locations(self): - # pylint: disable=protected-access - revoker = self._get_revoker(None) - self.assertEqual(revoker._get_installed_locations(), {}) - - def test_get_installed_locations(self): - # pylint: disable=protected-access - mock_installer = mock.MagicMock() - mock_installer.get_all_certs_keys.return_value = self.certs_keys - - revoker = self._get_revoker(mock_installer) - sha_vh = revoker._get_installed_locations() - - self.assertEqual(len(sha_vh), 2) - for i, cert in enumerate(self.certs): - self.assertTrue(cert.get_fingerprint() in sha_vh) - self.assertEqual( - sha_vh[cert.get_fingerprint()], self.installs[i]) - - @mock.patch("letsencrypt.revoker.OpenSSL.crypto.load_certificate") - def test_get_installed_load_failure(self, mock_load_certificate): - mock_installer = mock.MagicMock() - mock_installer.get_all_certs_keys.return_value = self.certs_keys - - mock_load_certificate.side_effect = OpenSSL.crypto.Error - - revoker = self._get_revoker(mock_installer) - - # pylint: disable=protected-access - self.assertEqual(revoker._get_installed_locations(), {}) - - def test_get_installed_load_failure_open(self): - tmp = tempfile.mkdtemp() - mock_installer = mock.MagicMock() - mock_installer.get_all_certs_keys.return_value = [( - os.path.join(tmp, 'missing'), None, None)] - revoker = self._get_revoker(mock_installer) - # pylint: disable=protected-access - self.assertEqual(revoker._get_installed_locations(), {}) - os.rmdir(tmp) - - -class RevokerClassMethodsTest(RevokerBase): - def setUp(self): - super(RevokerClassMethodsTest, self).setUp() - self.mock_config = mock.MagicMock(cert_key_backup=self.backup_dir) - - def tearDown(self): - shutil.rmtree(self.backup_dir) - - def _call(self, cert_path, key_path): - from letsencrypt.revoker import Revoker - Revoker.store_cert_key(cert_path, key_path, self.mock_config) - - def test_store_two(self): - from letsencrypt.revoker import Revoker - self._call(self.paths[0], self.key_path) - self._call(self.paths[1], self.key_path) - - self.assertTrue(os.path.isfile(self.list_path)) - rows = self._get_rows() - - for i, row in enumerate(rows): - # pylint: disable=protected-access - self.assertTrue(os.path.isfile( - Revoker._get_backup(self.backup_dir, i, self.paths[i]))) - self.assertTrue(os.path.isfile( - Revoker._get_backup(self.backup_dir, i, self.key_path))) - self.assertEqual([str(i), self.paths[i], self.key_path], row) - - self.assertEqual(len(rows), 2) - - def test_store_one_mixed(self): - from letsencrypt.revoker import Revoker - self._write_rows( - [["5", "blank", "blank"], ["18", "dc", "dc"], ["21", "b", "b"]]) - self._call(self.paths[0], self.key_path) - - self.assertEqual( - self._get_rows()[3], ["22", self.paths[0], self.key_path]) - - # pylint: disable=protected-access - self.assertTrue(os.path.isfile( - Revoker._get_backup(self.backup_dir, 22, self.paths[0]))) - self.assertTrue(os.path.isfile( - Revoker._get_backup(self.backup_dir, 22, self.key_path))) - - -class CertTest(unittest.TestCase): - def setUp(self): - self.paths, self.certs, self.key_path = create_revoker_certs() - - def test_failed_load(self): - from letsencrypt.revoker import Cert - self.assertRaises(errors.RevokerError, Cert, self.key_path) - - def test_failed_load_open(self): - tmp = tempfile.mkdtemp() - from letsencrypt.revoker import Cert - self.assertRaises( - errors.RevokerError, Cert, os.path.join(tmp, 'missing')) - os.rmdir(tmp) - - def test_no_row(self): - self.assertEqual(self.certs[0].get_row(), None) - - def test_meta_moved_files(self): - from letsencrypt.revoker import Cert - fake_path = "/not/a/real/path/r72d3t6" - self.certs[0].add_meta( - 0, fake_path, fake_path, self.paths[0], self.key_path) - - self.assertEqual(self.certs[0].orig.status, Cert.DELETED_MSG) - self.assertEqual(self.certs[0].orig_key.status, Cert.DELETED_MSG) - - def test_meta_changed_files(self): - from letsencrypt.revoker import Cert - self.certs[0].add_meta( - 0, self.paths[1], self.paths[1], self.paths[0], self.key_path) - - self.assertEqual(self.certs[0].orig.status, Cert.CHANGED_MSG) - self.assertEqual(self.certs[0].orig_key.status, Cert.CHANGED_MSG) - - def test_meta_no_status(self): - self.certs[0].add_meta( - 0, self.paths[0], self.key_path, self.paths[0], self.key_path) - - self.assertEqual(self.certs[0].orig.status, "") - self.assertEqual(self.certs[0].orig_key.status, "") - - def test_print_meta(self): - """Just make sure there aren't any major errors.""" - self.certs[0].add_meta( - 0, self.paths[0], self.key_path, self.paths[0], self.key_path) - # Changed path and deleted file - self.certs[1].add_meta( - 1, self.paths[0], "/not/a/path", self.paths[1], self.key_path) - self.assertTrue(self.certs[0].pretty_print()) - self.assertTrue(self.certs[1].pretty_print()) - - def test_print_no_meta(self): - self.assertTrue(self.certs[0].pretty_print()) - self.assertTrue(self.certs[1].pretty_print()) - - -def create_revoker_certs(): - """Create a few revoker.Cert objects.""" - cert0_path = test_util.vector_path("cert.pem") - cert1_path = test_util.vector_path("cert-san.pem") - key_path = test_util.vector_path("rsa512_key.pem") - - from letsencrypt.revoker import Cert - cert0 = Cert(cert0_path) - cert1 = Cert(cert1_path) - - return [cert0_path, cert1_path], [cert0, cert1], key_path - - -if __name__ == "__main__": - unittest.main() # pragma: no cover From c1a959de4532b3ca5ae1787338b45b3bf85dc6af Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 25 Sep 2015 22:44:33 -0700 Subject: [PATCH 19/33] Remove Revocation display --- letsencrypt/display/revocation.py | 77 ---------------- letsencrypt/tests/display/revocation_test.py | 97 -------------------- 2 files changed, 174 deletions(-) delete mode 100644 letsencrypt/display/revocation.py delete mode 100644 letsencrypt/tests/display/revocation_test.py diff --git a/letsencrypt/display/revocation.py b/letsencrypt/display/revocation.py deleted file mode 100644 index 02a253676..000000000 --- a/letsencrypt/display/revocation.py +++ /dev/null @@ -1,77 +0,0 @@ -"""Revocation UI class.""" -import os - -import zope.component - -from letsencrypt import interfaces -from letsencrypt.display import util as display_util - -# Define a helper function to avoid verbose code -util = zope.component.getUtility # pylint: disable=invalid-name - - -def display_certs(certs): - """Display the certificates in a menu for revocation. - - :param list certs: each is a :class:`letsencrypt.revoker.Cert` - - :returns: tuple of the form (code, selection) where - code is a display exit code - selection is the user's int selection - :rtype: tuple - - """ - list_choices = [ - "%s | %s | %s" % ( - str(cert.get_cn().ljust(display_util.WIDTH - 39)), - cert.get_not_before().strftime("%m-%d-%y"), - "Installed" if cert.installed and cert.installed != ["Unknown"] - else "") for cert in certs - ] - - code, tag = util(interfaces.IDisplay).menu( - "Which certificates would you like to revoke?", - list_choices, help_label="More Info", ok_label="Revoke", - cancel_label="Exit") - - return code, tag - - -def confirm_revocation(cert): - """Confirm revocation screen. - - :param cert: certificate object - :type cert: :class: - - :returns: True if user would like to revoke, False otherwise - :rtype: bool - - """ - return util(interfaces.IDisplay).yesno( - "Are you sure you would like to revoke the following " - "certificate:{0}{cert}This action cannot be reversed!".format( - os.linesep, cert=cert.pretty_print())) - - -def more_info_cert(cert): - """Displays more info about the cert. - - :param dict cert: cert dict used throughout revoker.py - - """ - util(interfaces.IDisplay).notification( - "Certificate Information:{0}{1}".format( - os.linesep, cert.pretty_print()), - height=display_util.HEIGHT) - - -def success_revocation(cert): - """Display a success message. - - :param cert: cert that was revoked - :type cert: :class:`letsencrypt.revoker.Cert` - - """ - util(interfaces.IDisplay).notification( - "You have successfully revoked the certificate for " - "%s" % cert.get_cn()) diff --git a/letsencrypt/tests/display/revocation_test.py b/letsencrypt/tests/display/revocation_test.py deleted file mode 100644 index 6e9763006..000000000 --- a/letsencrypt/tests/display/revocation_test.py +++ /dev/null @@ -1,97 +0,0 @@ -"""Test :mod:`letsencrypt.display.revocation`.""" -import sys -import unittest - -import mock -import zope.component - -from letsencrypt.display import util as display_util - -from letsencrypt.tests import test_util - - -class DisplayCertsTest(unittest.TestCase): - def setUp(self): - from letsencrypt.revoker import Cert - self.cert0 = Cert(test_util.vector_path("cert.pem")) - self.cert1 = Cert(test_util.vector_path("cert-san.pem")) - - self.certs = [self.cert0, self.cert1] - - zope.component.provideUtility(display_util.FileDisplay(sys.stdout)) - - @classmethod - def _call(cls, certs): - from letsencrypt.display.revocation import display_certs - return display_certs(certs) - - @mock.patch("letsencrypt.display.revocation.util") - def test_revocation(self, mock_util): - mock_util().menu.return_value = (display_util.OK, 0) - - code, choice = self._call(self.certs) - - self.assertEqual(display_util.OK, code) - self.assertEqual(self.certs[choice], self.cert0) - - @mock.patch("letsencrypt.display.revocation.util") - def test_cancel(self, mock_util): - mock_util().menu.return_value = (display_util.CANCEL, -1) - - code, _ = self._call(self.certs) - self.assertEqual(display_util.CANCEL, code) - - -class MoreInfoCertTest(unittest.TestCase): - # pylint: disable=too-few-public-methods - @classmethod - def _call(cls, cert): - from letsencrypt.display.revocation import more_info_cert - more_info_cert(cert) - - @mock.patch("letsencrypt.display.revocation.util") - def test_more_info(self, mock_util): - self._call(mock.MagicMock()) - - self.assertEqual(mock_util().notification.call_count, 1) - - -class SuccessRevocationTest(unittest.TestCase): - def setUp(self): - from letsencrypt.revoker import Cert - self.cert = Cert(test_util.vector_path("cert.pem")) - - @classmethod - def _call(cls, cert): - from letsencrypt.display.revocation import success_revocation - success_revocation(cert) - - # Pretty trivial test... something is displayed... - @mock.patch("letsencrypt.display.revocation.util") - def test_success_revocation(self, mock_util): - self._call(self.cert) - - self.assertEqual(mock_util().notification.call_count, 1) - - -class ConfirmRevocationTest(unittest.TestCase): - def setUp(self): - from letsencrypt.revoker import Cert - self.cert = Cert(test_util.vector_path("cert.pem")) - - @classmethod - def _call(cls, cert): - from letsencrypt.display.revocation import confirm_revocation - return confirm_revocation(cert) - - @mock.patch("letsencrypt.display.revocation.util") - def test_confirm_revocation(self, mock_util): - mock_util().yesno.return_value = True - self.assertTrue(self._call(self.cert)) - - mock_util().yesno.return_value = False - self.assertFalse(self._call(self.cert)) - - -if __name__ == "__main__": - unittest.main() # pragma: no cover From f02653801df539df45518eb1887af876da984027 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 25 Sep 2015 22:54:15 -0700 Subject: [PATCH 20/33] Remove revocation from client --- letsencrypt/client.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 60eaea5a1..0eba8349d 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -21,7 +21,6 @@ from letsencrypt import errors from letsencrypt import interfaces from letsencrypt import le_util from letsencrypt import reverter -from letsencrypt import revoker from letsencrypt import storage from letsencrypt.display import ops as display_ops @@ -485,27 +484,6 @@ def rollback(default_installer, checkpoints, config, plugins): installer.restart() -def revoke(default_installer, config, plugins, no_confirm, cert, authkey): - """Revoke certificates. - - :param config: Configuration. - :type config: :class:`letsencrypt.interfaces.IConfig` - - """ - installer = display_ops.pick_installer( - config, default_installer, plugins, question="Which installer " - "should be used for certificate revocation?") - - revoc = revoker.Revoker(installer, config, no_confirm) - # Cert is most selective, so it is chosen first. - if cert is not None: - revoc.revoke_from_cert(cert[0]) - elif authkey is not None: - revoc.revoke_from_key(le_util.Key(authkey[0], authkey[1])) - else: - revoc.revoke_from_menu() - - def view_config_changes(config): """View checkpoints and associated configuration changes. From 514fc49e696d5d1e546184a1b8afeac87933c5a2 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 25 Sep 2015 22:57:39 -0700 Subject: [PATCH 21/33] lower coverage due to removing revoker :( --- tox.cover.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tox.cover.sh b/tox.cover.sh index edfd9b81a..aa5e3ed88 100755 --- a/tox.cover.sh +++ b/tox.cover.sh @@ -16,7 +16,7 @@ fi cover () { if [ "$1" = "letsencrypt" ]; then - min=97 + min=96 elif [ "$1" = "acme" ]; then min=100 elif [ "$1" = "letsencrypt_apache" ]; then From 98d49ae8bf7b686601efda423fd5875249451671 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Sat, 26 Sep 2015 01:34:09 -0700 Subject: [PATCH 22/33] Remove excessive error handling --- letsencrypt/cli.py | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index cd34708b9..11ee65734 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -323,10 +323,7 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo # TODO: Handle errors from _init_le_client? le_client = _init_le_client(args, config, authenticator, installer) - try: - lineage = _auth_from_domains(le_client, config, domains, plugins) - except errors.Error as err: - return str(err) + lineage = _auth_from_domains(le_client, config, domains, plugins) # TODO: We also need to pass the fullchain (for Nginx) le_client.deploy_certificate( @@ -369,10 +366,7 @@ def auth(args, config, plugins): certr, chain, args.cert_path, args.chain_path) else: domains = _find_domains(args, installer) - try: - _auth_from_domains(le_client, config, domains, plugins) - except errors.Error as err: - return str(err) + _auth_from_domains(le_client, config, domains, plugins) def install(args, config, plugins): From 81f0a973a3452e1581c186c15fc5db6ffb218607 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 26 Sep 2015 09:07:08 +0000 Subject: [PATCH 23/33] ManualAuthenticator -> Authenticator --- letsencrypt/plugins/manual.py | 4 ++-- letsencrypt/plugins/manual_test.py | 10 +++++----- setup.py | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 43d0ac055..2014c8c0e 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -23,7 +23,7 @@ from letsencrypt.plugins import common logger = logging.getLogger(__name__) -class ManualAuthenticator(common.Plugin): +class Authenticator(common.Plugin): """Manual Authenticator. .. todo:: Support for `~.challenges.DVSNI`. @@ -87,7 +87,7 @@ s.serve_forever()" """ """ def __init__(self, *args, **kwargs): - super(ManualAuthenticator, self).__init__(*args, **kwargs) + super(Authenticator, self).__init__(*args, **kwargs) self.template = (self.HTTP_TEMPLATE if self.config.no_simple_http_tls else self.HTTPS_TEMPLATE) self._root = (tempfile.mkdtemp() if self.conf("test-mode") diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index 6b9359db1..cfe47b833 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -17,22 +17,22 @@ from letsencrypt.tests import test_util KEY = jose.JWKRSA.load(test_util.load_vector("rsa512_key.pem")) -class ManualAuthenticatorTest(unittest.TestCase): - """Tests for letsencrypt.plugins.manual.ManualAuthenticator.""" +class AuthenticatorTest(unittest.TestCase): + """Tests for letsencrypt.plugins.manual.Authenticator.""" def setUp(self): - from letsencrypt.plugins.manual import ManualAuthenticator + from letsencrypt.plugins.manual import Authenticator self.config = mock.MagicMock( no_simple_http_tls=True, simple_http_port=4430, manual_test_mode=False) - self.auth = ManualAuthenticator(config=self.config, name="manual") + self.auth = Authenticator(config=self.config, name="manual") self.achalls = [achallenges.SimpleHTTP( challb=acme_util.SIMPLE_HTTP_P, domain="foo.com", account_key=KEY)] config_test_mode = mock.MagicMock( no_simple_http_tls=True, simple_http_port=4430, manual_test_mode=True) - self.auth_test_mode = ManualAuthenticator( + self.auth_test_mode = Authenticator( config=config_test_mode, name="manual") def test_more_info(self): diff --git a/setup.py b/setup.py index 6e1640e3e..ef7132edd 100644 --- a/setup.py +++ b/setup.py @@ -116,7 +116,7 @@ setup( 'letsencrypt-renewer = letsencrypt.renewer:main', ], 'letsencrypt.plugins': [ - 'manual = letsencrypt.plugins.manual:ManualAuthenticator', + 'manual = letsencrypt.plugins.manual:Authenticator', # TODO: null should probably not be presented to the user 'null = letsencrypt.plugins.null:Installer', 'standalone = letsencrypt.plugins.standalone.authenticator' From 08c0c4aebaa4dac9f8016f0399efe3c98472e532 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Thu, 24 Sep 2015 19:28:21 +0000 Subject: [PATCH 24/33] Explicit dependency on setuptools (pkg_resources). --- acme/setup.py | 1 + letsencrypt-apache/setup.py | 1 + letsencrypt-nginx/setup.py | 1 + letshelp-letsencrypt/setup.py | 4 +++- setup.py | 1 + 5 files changed, 7 insertions(+), 1 deletion(-) diff --git a/acme/setup.py b/acme/setup.py index 4cf215b40..60f97844b 100644 --- a/acme/setup.py +++ b/acme/setup.py @@ -16,6 +16,7 @@ install_requires = [ 'pyrfc3339', 'pytz', 'requests', + 'setuptools', # pkg_resources 'six', 'werkzeug', ] diff --git a/letsencrypt-apache/setup.py b/letsencrypt-apache/setup.py index 5ecb071c7..57d2f6b47 100644 --- a/letsencrypt-apache/setup.py +++ b/letsencrypt-apache/setup.py @@ -7,6 +7,7 @@ install_requires = [ 'letsencrypt', 'mock<1.1.0', # py26 'python-augeas', + 'setuptools', # pkg_resources 'zope.component', 'zope.interface', ] diff --git a/letsencrypt-nginx/setup.py b/letsencrypt-nginx/setup.py index 30dfa584f..b4ef69505 100644 --- a/letsencrypt-nginx/setup.py +++ b/letsencrypt-nginx/setup.py @@ -8,6 +8,7 @@ install_requires = [ 'mock<1.1.0', # py26 'PyOpenSSL', 'pyparsing>=1.5.5', # Python3 support; perhaps unnecessary? + 'setuptools', # pkg_resources 'zope.interface', ] diff --git a/letshelp-letsencrypt/setup.py b/letshelp-letsencrypt/setup.py index 6b89a6d09..5e7542411 100644 --- a/letshelp-letsencrypt/setup.py +++ b/letshelp-letsencrypt/setup.py @@ -4,7 +4,9 @@ from setuptools import setup from setuptools import find_packages -install_requires = [] +install_requires = [ + 'setuptools', # pkg_resources +] if sys.version_info < (2, 7): install_requires.append("mock<1.1.0") else: diff --git a/setup.py b/setup.py index 6e1640e3e..8f743d4da 100644 --- a/setup.py +++ b/setup.py @@ -42,6 +42,7 @@ install_requires = [ 'python2-pythondialog>=3.2.2rc1', # Debian squeeze support, cf. #280 'pytz', 'requests', + 'setuptools', 'zope.component', 'zope.interface', ] From d337865f484ddc9aa3dda8628d845685e2a20c5d Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 26 Sep 2015 10:54:24 +0000 Subject: [PATCH 25/33] add missing pkg_resources comment --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 8f743d4da..b43365a98 100644 --- a/setup.py +++ b/setup.py @@ -42,7 +42,7 @@ install_requires = [ 'python2-pythondialog>=3.2.2rc1', # Debian squeeze support, cf. #280 'pytz', 'requests', - 'setuptools', + 'setuptools', # pkg_resources 'zope.component', 'zope.interface', ] From 5128a0345ff613ebe151ee749275854741a0dc09 Mon Sep 17 00:00:00 2001 From: Jakub Warmuz Date: Sat, 26 Sep 2015 11:07:54 +0000 Subject: [PATCH 26/33] agree-tos in dev-cli.ini --- examples/dev-cli.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/dev-cli.ini b/examples/dev-cli.ini index 761bc58c9..085d4bfcc 100644 --- a/examples/dev-cli.ini +++ b/examples/dev-cli.ini @@ -9,6 +9,7 @@ domains = example.com text = True agree-eula = True +agree-tos = True debug = True # Unfortunately, it's not possible to specify "verbose" multiple times # (correspondingly to -vvvvvv) From 2015811a6c84682466005566afd795ea4c03f10f Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Sat, 26 Sep 2015 12:18:32 -0700 Subject: [PATCH 27/33] Incorporated Kuba's feedback --- letsencrypt/colored_logging.py | 15 ++++----------- letsencrypt/le_util.py | 7 ++++++- letsencrypt/reporter.py | 3 +-- letsencrypt/tests/colored_logging_test.py | 11 ++++++----- 4 files changed, 17 insertions(+), 19 deletions(-) diff --git a/letsencrypt/colored_logging.py b/letsencrypt/colored_logging.py index f5750870e..170da0b38 100644 --- a/letsencrypt/colored_logging.py +++ b/letsencrypt/colored_logging.py @@ -15,13 +15,12 @@ class StreamHandler(logging.StreamHandler): :ivar bool red_level: The level at which to output """ - _RED = '\033[31m' def __init__(self, stream=None): super(StreamHandler, self).__init__(stream) self.colored = (sys.stderr.isatty() if stream is None else stream.isatty()) - self.set_red_level(logging.WARNING) + self.red_level = logging.WARNING def format(self, record): """Formats the string representation of record. @@ -34,14 +33,8 @@ class StreamHandler(logging.StreamHandler): """ output = super(StreamHandler, self).format(record) if self.colored and record.levelno >= self.red_level: - return ''.join((self._RED, output, le_util.ANSI_SGR_RESET)) + return ''.join((le_util.ANSI_SGR_RED, + output, + le_util.ANSI_SGR_RESET)) else: return output - - def set_red_level(self, red_level): - """Sets the level necessary to display output in red. - - :param int red_level: Minimum log level for displaying red text - - """ - self.red_level = red_level diff --git a/letsencrypt/le_util.py b/letsencrypt/le_util.py index 74e03d8a1..5626902ef 100644 --- a/letsencrypt/le_util.py +++ b/letsencrypt/le_util.py @@ -18,7 +18,12 @@ Key = collections.namedtuple("Key", "file pem") CSR = collections.namedtuple("CSR", "file data form") -# ANSI escape code for resetting output format +# ANSI SGR escape codes +# Formats text as bold or with increased intensity +ANSI_SGR_BOLD = '\033[1m' +# Colors text red +ANSI_SGR_RED = "\033[31m" +# Resets output format ANSI_SGR_RESET = "\033[0m" diff --git a/letsencrypt/reporter.py b/letsencrypt/reporter.py index 86413053e..482305838 100644 --- a/letsencrypt/reporter.py +++ b/letsencrypt/reporter.py @@ -31,7 +31,6 @@ class Reporter(object): LOW_PRIORITY = 2 """Low priority constant. See `add_message`.""" - _BOLD = '\033[1m' _msg_type = collections.namedtuple('ReporterMsg', 'priority text on_crash') def __init__(self): @@ -76,7 +75,7 @@ class Reporter(object): no_exception = sys.exc_info()[0] is None bold_on = sys.stdout.isatty() if bold_on: - print self._BOLD + print le_util.ANSI_SGR_BOLD print 'IMPORTANT NOTES:' first_wrapper = textwrap.TextWrapper( initial_indent=' - ', subsequent_indent=(' ' * 3)) diff --git a/letsencrypt/tests/colored_logging_test.py b/letsencrypt/tests/colored_logging_test.py index fc97b2a49..5b49ec820 100644 --- a/letsencrypt/tests/colored_logging_test.py +++ b/letsencrypt/tests/colored_logging_test.py @@ -7,6 +7,7 @@ from letsencrypt import le_util class StreamHandlerTest(unittest.TestCase): + """Tests for letsencrypt.colored_logging.""" def setUp(self): from letsencrypt import colored_logging @@ -26,13 +27,13 @@ class StreamHandlerTest(unittest.TestCase): def test_format_and_red_level(self): msg = 'I did another thing' - self.handler.set_red_level(logging.DEBUG) + self.handler.red_level = logging.DEBUG self.logger.debug(msg) - # pylint: disable=protected-access - expected = '{0}{1}{2}\n'.format(self.handler._RED, msg, - le_util.ANSI_SGR_RESET) - self.assertEqual(self.stream.getvalue(), expected) + self.assertEqual(self.stream.getvalue(), + '{0}{1}{2}\n'.format(le_util.ANSI_SGR_RED, + msg, + le_util.ANSI_SGR_RESET)) if __name__ == "__main__": From 655c3c2a0eaa833577efdaa33a7c440fad7343b3 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Sat, 26 Sep 2015 15:44:57 -0700 Subject: [PATCH 28/33] Address comments --- letsencrypt/cli.py | 10 +++++----- letsencrypt/client.py | 16 +++++----------- 2 files changed, 10 insertions(+), 16 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 11ee65734..bd73c93d7 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -272,8 +272,10 @@ def _auth_from_domains(le_client, config, domains, plugins): lineage = _treat_as_renewal(config, domains) if lineage is not None: + # TODO: schoen wishes to reuse key - discussion + # https://github.com/letsencrypt/letsencrypt/pull/777/files#r40498574 new_certr, new_chain, new_key, _ = le_client.obtain_certificate(domains) - # TODO: Check whether it worked! + # TODO: Check whether it worked! <- or make sure errors are thrown (jdk) lineage.save_successor( lineage.latest_common_version(), OpenSSL.crypto.dump_certificate( OpenSSL.crypto.FILETYPE_PEM, new_certr.body), @@ -282,11 +284,10 @@ def _auth_from_domains(le_client, config, domains, plugins): lineage.update_all_links_to(lineage.latest_common_version()) # TODO: Check return value of save_successor # TODO: Also update lineage renewal config with any relevant - # configuration values from this attempt? - YES + # configuration values from this attempt? <- Absolutely (jdkasten) else: # TREAT AS NEW REQUEST - lineage = le_client.obtain_and_enroll_certificate( - domains, le_client.dv_auth, le_client.installer, plugins) + lineage = le_client.obtain_and_enroll_certificate(domains, plugins) if not lineage: raise errors.Error("Certificate could not be obtained") @@ -338,7 +339,6 @@ def run(args, config, plugins): # pylint: disable=too-many-branches,too-many-lo def auth(args, config, plugins): """Authenticate & obtain cert, but do not install it.""" - # XXX: Update for renewer / RenewableCert if args.domains is not None and args.csr is not None: # TODO: --csr could have a priority, when --domains is diff --git a/letsencrypt/client.py b/letsencrypt/client.py index 84ce9b7b2..39dd6ddfe 100644 --- a/letsencrypt/client.py +++ b/letsencrypt/client.py @@ -213,8 +213,7 @@ class Client(object): return self._obtain_certificate(domains, csr) + (key, csr) - def obtain_and_enroll_certificate( - self, domains, authenticator, installer, plugins): + def obtain_and_enroll_certificate(self, domains, plugins): """Obtain and enroll certificate. Get a new certificate for the specified domains using the specified @@ -222,12 +221,6 @@ class Client(object): containing it. :param list domains: Domains to request. - :param authenticator: The authenticator to use. - :type authenticator: :class:`letsencrypt.interfaces.IAuthenticator` - - :param installer: The installer to use. - :type installer: :class:`letsencrypt.interfaces.IInstaller` - :param plugins: A PluginsFactory object. :returns: A new :class:`letsencrypt.storage.RenewableCert` instance @@ -239,9 +232,10 @@ class Client(object): # TODO: remove this dirty hack self.config.namespace.authenticator = plugins.find_init( - authenticator).name - if installer is not None: - self.config.namespace.installer = plugins.find_init(installer).name + self.dv_auth).name + if self.installer is not None: + self.config.namespace.installer = plugins.find_init( + self.installer).name # XXX: We clearly need a more general and correct way of getting # options into the configobj for the RenewableCert instance. From 8dc345a3a0909612c88836c6f82b9290495c801c Mon Sep 17 00:00:00 2001 From: James Kasten Date: Sat, 26 Sep 2015 16:04:44 -0700 Subject: [PATCH 29/33] address naming conventions --- letsencrypt/cli.py | 7 ++++--- letsencrypt/tests/cli_test.py | 2 +- tests/integration/_common.sh | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index bd73c93d7..0804142f6 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -241,7 +241,7 @@ def _treat_as_renewal(config, domains): # We aren't in a duplicative-names situation at all, so we don't # have to tell or ask the user anything about this. pass - elif config.no_confirm or zope.component.getUtility( + elif config.renew_by_default or zope.component.getUtility( interfaces.IDisplay).yesno(question, "Replace", "Cancel"): renewal = True else: @@ -654,8 +654,9 @@ def create_parser(plugins, args): version="%(prog)s {0}".format(letsencrypt.__version__), help="show program's version number and exit") helpful.add( - "automation", "--no-confirm", dest="no_confirm", action="store_true", - help="Turn off confirmation screens, used for renewal screens") + "automation", "--renew-by-default", action="store_true", + help="Select renewal by default when domains are a superset of a " + "a previously attained cert") helpful.add( "automation", "--agree-eula", dest="eula", action="store_true", help="Agree to the Let's Encrypt Developer Preview EULA") diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 97725a4c7..2e9f3330c 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -177,7 +177,7 @@ class DuplicativeCertsTest(renewer_test.BaseRenewableCertTest): shutil.rmtree(self.tempdir) @mock.patch("letsencrypt.le_util.make_or_verify_dir") - def test_find_duplicative_names(self, unused): # pylint: disable=unused-argument + def test_find_duplicative_names(self, unused_makedir): from letsencrypt.cli import _find_duplicative_certs test_cert = test_util.load_vector("cert-san.pem") with open(self.test_rc.cert, "w") as f: diff --git a/tests/integration/_common.sh b/tests/integration/_common.sh index 7897ff1b7..fd60b9258 100755 --- a/tests/integration/_common.sh +++ b/tests/integration/_common.sh @@ -23,7 +23,7 @@ letsencrypt_test () { --agree-eula \ --agree-tos \ --email "" \ - --no-confirm \ + --renew-by-default \ --debug \ -vvvvvvv \ "$@" From 6f1b1570b13d9e41dceaca909ebf417469609ee7 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 26 Sep 2015 17:48:45 -0700 Subject: [PATCH 30/33] Revert "ManualAuthenticator -> Authenticator" This reverts commit 81f0a973a3452e1581c186c15fc5db6ffb218607. This was breaking the client. Not sure if/how it passed any tests? --- letsencrypt/plugins/manual.py | 4 ++-- letsencrypt/plugins/manual_test.py | 10 +++++----- setup.py | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 2014c8c0e..43d0ac055 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -23,7 +23,7 @@ from letsencrypt.plugins import common logger = logging.getLogger(__name__) -class Authenticator(common.Plugin): +class ManualAuthenticator(common.Plugin): """Manual Authenticator. .. todo:: Support for `~.challenges.DVSNI`. @@ -87,7 +87,7 @@ s.serve_forever()" """ """ def __init__(self, *args, **kwargs): - super(Authenticator, self).__init__(*args, **kwargs) + super(ManualAuthenticator, self).__init__(*args, **kwargs) self.template = (self.HTTP_TEMPLATE if self.config.no_simple_http_tls else self.HTTPS_TEMPLATE) self._root = (tempfile.mkdtemp() if self.conf("test-mode") diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index cfe47b833..6b9359db1 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -17,22 +17,22 @@ from letsencrypt.tests import test_util KEY = jose.JWKRSA.load(test_util.load_vector("rsa512_key.pem")) -class AuthenticatorTest(unittest.TestCase): - """Tests for letsencrypt.plugins.manual.Authenticator.""" +class ManualAuthenticatorTest(unittest.TestCase): + """Tests for letsencrypt.plugins.manual.ManualAuthenticator.""" def setUp(self): - from letsencrypt.plugins.manual import Authenticator + from letsencrypt.plugins.manual import ManualAuthenticator self.config = mock.MagicMock( no_simple_http_tls=True, simple_http_port=4430, manual_test_mode=False) - self.auth = Authenticator(config=self.config, name="manual") + self.auth = ManualAuthenticator(config=self.config, name="manual") self.achalls = [achallenges.SimpleHTTP( challb=acme_util.SIMPLE_HTTP_P, domain="foo.com", account_key=KEY)] config_test_mode = mock.MagicMock( no_simple_http_tls=True, simple_http_port=4430, manual_test_mode=True) - self.auth_test_mode = Authenticator( + self.auth_test_mode = ManualAuthenticator( config=config_test_mode, name="manual") def test_more_info(self): diff --git a/setup.py b/setup.py index c568d2872..b43365a98 100644 --- a/setup.py +++ b/setup.py @@ -117,7 +117,7 @@ setup( 'letsencrypt-renewer = letsencrypt.renewer:main', ], 'letsencrypt.plugins': [ - 'manual = letsencrypt.plugins.manual:Authenticator', + 'manual = letsencrypt.plugins.manual:ManualAuthenticator', # TODO: null should probably not be presented to the user 'null = letsencrypt.plugins.null:Installer', 'standalone = letsencrypt.plugins.standalone.authenticator' From 63e1c652e18f98850f529173494cfbbd0a2905df Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 26 Sep 2015 18:05:17 -0700 Subject: [PATCH 31/33] Undo damage from PEP8ification --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 7cb4a0458..81f8a8414 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -484,7 +484,7 @@ class HelpfulArgumentParser(object): help2 = self.prescan_for_flag("--help", self.help_topics) assert max(True, "a") == "a", "Gravity changed direction" help_arg = max(help1, help2) - if help_arg: + if help_arg == True: # just --help with no topic; avoid argparse altogether print USAGE sys.exit(0) From 405bc99235754b661224a18753bdb8a8ec3ff60d Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 26 Sep 2015 18:19:56 -0700 Subject: [PATCH 32/33] --help is effectively a verb for CLI purposes --- letsencrypt/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index 81f8a8414..a5968ec9c 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -679,7 +679,7 @@ def create_parser(plugins, args): # For now unfortunately this constant just needs to match the code below; # there isn't an elegant way to autogenerate it in time. VERBS = ["run", "auth", "install", "revoke", "rollback", "config_changes", - "plugins"] + "plugins", "--help"] def _create_subparsers(helpful): From e7cbdc4f9a0e021b385a8eb1869a21011d5e7840 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Sat, 26 Sep 2015 18:20:13 -0700 Subject: [PATCH 33/33] Revert reversion Revert "Revert "ManualAuthenticator -> Authenticator"" (commit required a pip reinstall but was not inherently broken) This reverts commit 6f1b1570b13d9e41dceaca909ebf417469609ee7. --- letsencrypt/plugins/manual.py | 4 ++-- letsencrypt/plugins/manual_test.py | 10 +++++----- setup.py | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/letsencrypt/plugins/manual.py b/letsencrypt/plugins/manual.py index 43d0ac055..2014c8c0e 100644 --- a/letsencrypt/plugins/manual.py +++ b/letsencrypt/plugins/manual.py @@ -23,7 +23,7 @@ from letsencrypt.plugins import common logger = logging.getLogger(__name__) -class ManualAuthenticator(common.Plugin): +class Authenticator(common.Plugin): """Manual Authenticator. .. todo:: Support for `~.challenges.DVSNI`. @@ -87,7 +87,7 @@ s.serve_forever()" """ """ def __init__(self, *args, **kwargs): - super(ManualAuthenticator, self).__init__(*args, **kwargs) + super(Authenticator, self).__init__(*args, **kwargs) self.template = (self.HTTP_TEMPLATE if self.config.no_simple_http_tls else self.HTTPS_TEMPLATE) self._root = (tempfile.mkdtemp() if self.conf("test-mode") diff --git a/letsencrypt/plugins/manual_test.py b/letsencrypt/plugins/manual_test.py index 6b9359db1..cfe47b833 100644 --- a/letsencrypt/plugins/manual_test.py +++ b/letsencrypt/plugins/manual_test.py @@ -17,22 +17,22 @@ from letsencrypt.tests import test_util KEY = jose.JWKRSA.load(test_util.load_vector("rsa512_key.pem")) -class ManualAuthenticatorTest(unittest.TestCase): - """Tests for letsencrypt.plugins.manual.ManualAuthenticator.""" +class AuthenticatorTest(unittest.TestCase): + """Tests for letsencrypt.plugins.manual.Authenticator.""" def setUp(self): - from letsencrypt.plugins.manual import ManualAuthenticator + from letsencrypt.plugins.manual import Authenticator self.config = mock.MagicMock( no_simple_http_tls=True, simple_http_port=4430, manual_test_mode=False) - self.auth = ManualAuthenticator(config=self.config, name="manual") + self.auth = Authenticator(config=self.config, name="manual") self.achalls = [achallenges.SimpleHTTP( challb=acme_util.SIMPLE_HTTP_P, domain="foo.com", account_key=KEY)] config_test_mode = mock.MagicMock( no_simple_http_tls=True, simple_http_port=4430, manual_test_mode=True) - self.auth_test_mode = ManualAuthenticator( + self.auth_test_mode = Authenticator( config=config_test_mode, name="manual") def test_more_info(self): diff --git a/setup.py b/setup.py index b43365a98..c568d2872 100644 --- a/setup.py +++ b/setup.py @@ -117,7 +117,7 @@ setup( 'letsencrypt-renewer = letsencrypt.renewer:main', ], 'letsencrypt.plugins': [ - 'manual = letsencrypt.plugins.manual:ManualAuthenticator', + 'manual = letsencrypt.plugins.manual:Authenticator', # TODO: null should probably not be presented to the user 'null = letsencrypt.plugins.null:Installer', 'standalone = letsencrypt.plugins.standalone.authenticator'