diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index c63d8c8d4..ffbd70ced 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -57,7 +57,7 @@ class AuthHandler(object): def get_authorizations(self, domains, best_effort=False): """Retrieve all authorizations for challenges. - :param set domains: Domains for authorization + :param list domains: Domains for authorization :param bool best_effort: Whether or not all authorizations are required (this is useful in renewal) @@ -480,6 +480,9 @@ def is_preferred(offered_challb, satisfied, return True +_ACME_PREFIX = "urn:acme:error:" + + _ERROR_HELP_COMMON = ( "To fix these errors, please make sure that your domain name was entered " "correctly and the DNS A record(s) for that domain contain(s) the " @@ -490,7 +493,9 @@ _ERROR_HELP = { "connection": _ERROR_HELP_COMMON + " Additionally, please check that your computer " "has a publicly routable IP address and that no firewalls are preventing " - "the server from communicating with the client.", + "the server from communicating with the client. If you're using the " + "webroot plugin, you should also verify that you are serving files " + "from the webroot path you provided.", "dnssec": _ERROR_HELP_COMMON + " Additionally, if you have DNSSEC enabled for " "your domain, please ensure that the signature is valid.", @@ -540,11 +545,13 @@ def _generate_failed_chall_msg(failed_achalls): """ typ = failed_achalls[0].error.typ + if typ.startswith(_ACME_PREFIX): + typ = typ[len(_ACME_PREFIX):] msg = ["The following errors were reported by the server:"] for achall in failed_achalls: msg.append("\n\nDomain: %s\nType: %s\nDetail: %s" % ( - achall.domain, achall.error.typ, achall.error.detail)) + achall.domain, typ, achall.error.detail)) if typ in _ERROR_HELP: msg.append("\n\n") diff --git a/letsencrypt/cli.py b/letsencrypt/cli.py index e01275153..c9c58ea3b 100644 --- a/letsencrypt/cli.py +++ b/letsencrypt/cli.py @@ -850,10 +850,10 @@ def _reconstitute(config, full_path): try: domains = [le_util.enforce_domain_sanity(x) for x in renewal_candidate.names()] - except (UnicodeError, ValueError): + except errors.ConfigurationError as error: logger.warning("Renewal configuration file %s references a cert " - "that mentions a domain name that we regarded as " - "invalid. Skipping.", full_path) + "that contains an invalid domain name. The problem " + "was: %s. Skipping.", full_path, error) return None setattr(config.namespace, "domains", domains) diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index 5b4c2bfc7..5a6199ca3 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -437,9 +437,12 @@ class ReportFailedChallsTest(unittest.TestCase): "chall": acme_util.HTTP01, "uri": "uri", "status": messages.STATUS_INVALID, - "error": messages.Error(typ="tls", detail="detail"), + "error": messages.Error(typ="urn:acme:error:tls", detail="detail"), } + # Prevent future regressions if the error type changes + self.assertTrue(kwargs["error"].description is not None) + self.http01 = achallenges.KeyAuthorizationAnnotatedChallenge( # pylint: disable=star-args challb=messages.ChallengeBody(**kwargs), diff --git a/letsencrypt/tests/cli_test.py b/letsencrypt/tests/cli_test.py index 1a86fb99b..2d36a9d21 100644 --- a/letsencrypt/tests/cli_test.py +++ b/letsencrypt/tests/cli_test.py @@ -599,19 +599,101 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods print "Logs:" print lf.read() - - def test_renewal_verb(self): + def test_renew_verb(self): with open(test_util.vector_path('sample-renewal.conf')) as src: # put the correct path for cert.pem, chain.pem etc in the renewal conf renewal_conf = src.read().replace("MAGICDIR", test_util.vector_path()) rd = os.path.join(self.config_dir, "renewal") - os.makedirs(rd) + if not os.path.exists(rd): + os.makedirs(rd) rc = os.path.join(rd, "sample-renewal.conf") with open(rc, "w") as dest: dest.write(renewal_conf) args = ["renew", "--dry-run", "-tvv"] self._test_renewal_common(True, [], args=args, renew=True) + def test_renew_verb_empty_config(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'empty.conf'), 'w'): + pass # leave the file empty + self.test_renew_verb() + + def _make_dummy_renewal_config(self): + renewer_configs_dir = os.path.join(self.config_dir, 'renewal') + os.makedirs(renewer_configs_dir) + with open(os.path.join(renewer_configs_dir, 'test.conf'), 'w') as f: + f.write("My contents don't matter") + + def _test_renew_common(self, renewalparams=None, + names=None, assert_oc_called=None): + self._make_dummy_renewal_config() + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + if renewalparams is not None: + mock_lineage.configuration = {'renewalparams': renewalparams} + if names is not None: + mock_lineage.names.return_value = names + mock_rc.return_value = mock_lineage + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + self._test_renewal_common(True, None, + args=['renew'], renew=False) + if assert_oc_called is not None: + if assert_oc_called: + self.assertTrue(mock_obtain_cert.called) + else: + self.assertFalse(mock_obtain_cert.called) + + def test_renew_no_renewalparams(self): + self._test_renew_common(assert_oc_called=False) + + def test_renew_no_authenticator(self): + self._test_renew_common(renewalparams={}, assert_oc_called=False) + + def test_renew_with_bad_int(self): + renewalparams = {'authenticator': 'webroot', + 'rsa_key_size': 'over 9000'} + self._test_renew_common(renewalparams=renewalparams, + assert_oc_called=False) + + def test_renew_with_bad_domain(self): + renewalparams = {'authenticator': 'webroot'} + names = ['*.example.com'] + self._test_renew_common(renewalparams=renewalparams, + names=names, assert_oc_called=False) + + def test_renew_plugin_config_restoration(self): + renewalparams = {'authenticator': 'webroot', + 'webroot_path': 'None', + 'webroot_imaginary_flag': '42'} + self._test_renew_common(renewalparams=renewalparams, + assert_oc_called=True) + + def test_renew_reconstitute_error(self): + # pylint: disable=protected-access + with mock.patch('letsencrypt.cli._reconstitute') as mock_reconstitute: + mock_reconstitute.side_effect = Exception + self._test_renew_common(assert_oc_called=False) + + def test_renew_obtain_cert_error(self): + self._make_dummy_renewal_config() + with mock.patch('letsencrypt.storage.RenewableCert') as mock_rc: + mock_lineage = mock.MagicMock() + mock_rc.return_value = mock_lineage + mock_lineage.configuration = { + 'renewalparams': {'authenticator': 'webroot'}} + with mock.patch('letsencrypt.cli.obtain_cert') as mock_obtain_cert: + mock_obtain_cert.side_effect = Exception + self._test_renewal_common(True, None, + args=['renew'], renew=False) + + def test_renew_with_bad_cli_args(self): + self.assertRaises(errors.Error, self._test_renewal_common, True, None, + args='renew -d example.com'.split(), renew=False) + self.assertRaises(errors.Error, self._test_renewal_common, True, None, + args='renew --csr {0}'.format(CSR).split(), + renew=False) + @mock.patch('letsencrypt.cli.zope.component.getUtility') @mock.patch('letsencrypt.cli._treat_as_renewal') @mock.patch('letsencrypt.cli._init_le_client')