From c025c17b5de7d5b328c8143ed9d3c32075d9df32 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Tue, 15 Sep 2015 22:48:36 -0700 Subject: [PATCH 01/12] 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 02/12] 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 03/12] 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 04/12] 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 05/12] 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 06/12] 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 cfe103b4edc5e8366cccb7e34e1a890fe8ad9bfc Mon Sep 17 00:00:00 2001 From: James Kasten Date: Fri, 25 Sep 2015 20:01:12 -0700 Subject: [PATCH 07/12] 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 08/12] 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 09/12] 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 98d49ae8bf7b686601efda423fd5875249451671 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Sat, 26 Sep 2015 01:34:09 -0700 Subject: [PATCH 10/12] 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 655c3c2a0eaa833577efdaa33a7c440fad7343b3 Mon Sep 17 00:00:00 2001 From: James Kasten Date: Sat, 26 Sep 2015 15:44:57 -0700 Subject: [PATCH 11/12] 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 12/12] 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 \ "$@"