From f7a09e5e44b25d5631898f771022ac94df4334ae Mon Sep 17 00:00:00 2001 From: Mathieu Leduc-Hamel Date: Sun, 7 Aug 2016 10:51:25 -0400 Subject: [PATCH 01/36] Add dns-01 challenge support to the Manual plugin --- certbot/plugins/manual.py | 126 ++++++++++++++++++++--------- certbot/plugins/manual_test.py | 32 +++++--- certbot/plugins/standalone.py | 38 ++------- certbot/plugins/standalone_test.py | 28 ------- certbot/plugins/util.py | 36 +++++++++ certbot/plugins/util_test.py | 38 +++++++++ certbot/tests/acme_util.py | 8 +- certbot/tests/auth_handler_test.py | 8 +- certbot/tests/errors_test.py | 4 +- certbot/util.py | 21 +++++ 10 files changed, 221 insertions(+), 118 deletions(-) diff --git a/certbot/plugins/manual.py b/certbot/plugins/manual.py index 6c7b822ab..608484031 100644 --- a/certbot/plugins/manual.py +++ b/certbot/plugins/manual.py @@ -4,26 +4,30 @@ import logging import pipes import shutil import signal -import socket import subprocess import sys import tempfile -import time import six import zope.component import zope.interface +from functools import partial + from acme import challenges from certbot import errors from certbot import interfaces -from certbot.plugins import common +from certbot.plugins import common, util +from certbot.util import busy_wait logger = logging.getLogger(__name__) +SUPPORTED_CHALLENGES = [challenges.HTTP01, challenges.DNS01] + + @zope.interface.implementer(interfaces.IAuthenticator) @zope.interface.provider(interfaces.IPluginFactory) class Authenticator(common.Plugin): @@ -41,7 +45,16 @@ class Authenticator(common.Plugin): description = "Manually configure an HTTP server" - MESSAGE_TEMPLATE = """\ + MESSAGE_TEMPLATE = { + "dns-01": """\ +Make sure your dns configuration content the following key before continuing: + +{validation} + +if you didn't, make sure to add the validation as TXT record into your domain +configuration. +""", + "http-01": """\ Make sure your web server displays the following content at {uri} before continuing: @@ -51,7 +64,7 @@ If you don't have HTTP server configured, you can run the following command on the target server (as root): {command} -""" +"""} # a disclaimer about your current IP being transmitted to Let's Encrypt's servers. IP_DISCLAIMER = """\ @@ -86,10 +99,23 @@ s.serve_forever()" """ @classmethod def add_parser_arguments(cls, add): + validator = partial(util.supported_challenges_validator, + supported=SUPPORTED_CHALLENGES) + add("test-mode", action="store_true", help="Test mode. Executes the manual command in subprocess.") add("public-ip-logging-ok", action="store_true", help="Automatically allows public IP logging.") + add("supported-challenges", + help="Supported challenges. Preferred in the order they are listed.", + type=validator, + default="http-01") + + @property + def supported_challenges(self): + """Challenges supported by this plugin.""" + return [challenges.Challenge.TYPES[name] for name in + self.conf("supported-challenges").split(",")] def prepare(self): # pylint: disable=missing-docstring,no-self-use if self.config.noninteractive_mode and not self.conf("test-mode"): @@ -97,38 +123,35 @@ s.serve_forever()" """ def more_info(self): # pylint: disable=missing-docstring,no-self-use return ("This plugin requires user's manual intervention in setting " - "up an HTTP server for solving http-01 challenges and thus " + "up an HTTP server when solving http-01 challenges and thus " "does not need to be run as a privileged process. " "Alternatively shows instructions on how to use Python's " - "built-in HTTP server.") + "built-in HTTP server." + "When solving dns-01 challenges, it simply needs to wait for " + "the proper configuration of the domain's dns") def get_chall_pref(self, domain): # pylint: disable=missing-docstring,no-self-use,unused-argument - return [challenges.HTTP01] + return self.supported_challenges - def perform(self, achalls): # pylint: disable=missing-docstring + def perform(self, achalls): + # pylint: disable=missing-docstring + mapping = {"http-01": self._perform_http01_challenge, + "dns-01": self._perform_dns01_challenge} responses = [] # TODO: group achalls by the same socket.gethostbyname(_ex) # and prompt only once per server (one "echo -n" per domain) for achall in achalls: - responses.append(self._perform_single(achall)) + responses.append(mapping[achall.typ](achall)) return responses - @classmethod - def _test_mode_busy_wait(cls, port): - while True: - time.sleep(1) - sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - try: - sock.connect(("localhost", port)) - except socket.error: # pragma: no cover - pass - else: - break - finally: - sock.close() + def cleanup(self, achalls): + # pylint: disable=missing-docstring + for achall in achalls: + if isinstance(achall.chall, challenges.HTTP01): + self._cleanup_http01_challenge(achall) - def _perform_single(self, achall): + def _perform_http01_challenge(self, achall): # same path for each challenge response would be easier for # users, but will not work if multiple domains point at the # same server: default command doesn't support virtual hosts @@ -161,7 +184,8 @@ s.serve_forever()" """ logger.debug("Manual command running as PID %s.", self._httpd.pid) # give it some time to bootstrap, before we try to verify # (cert generation in case of simpleHttpS might take time) - self._test_mode_busy_wait(port) + busy_wait(port) + if self._httpd.poll() is not None: raise errors.Error("Couldn't execute manual command") else: @@ -171,10 +195,14 @@ s.serve_forever()" """ cli_flag="--manual-public-ip-logging-ok"): raise errors.PluginError("Must agree to IP logging to proceed") - self._notify_and_wait(self.MESSAGE_TEMPLATE.format( - validation=validation, response=response, + message = self._get_message(achall) + + self._notify_and_wait(message.format( + validation=validation, + response=response, uri=achall.chall.uri(achall.domain), - command=command)) + command=command + )) if not response.simple_verify( achall.chall, achall.domain, @@ -183,15 +211,30 @@ s.serve_forever()" """ return response - def _notify_and_wait(self, message): # pylint: disable=no-self-use - # TODO: IDisplay wraps messages, breaking the command - #answer = zope.component.getUtility(interfaces.IDisplay).notification( - # message=message, height=25, pause=True) - sys.stdout.write(message) - six.moves.input("Press ENTER to continue") + def _perform_dns01_challenge(self, achall): + response, validation = achall.response_and_validation() + if not self.conf("test-mode"): + if not self.conf("public-ip-logging-ok"): + if not zope.component.getUtility(interfaces.IDisplay).yesno( + self.IP_DISCLAIMER, "Yes", "No", + cli_flag="--manual-public-ip-logging-ok"): + raise errors.PluginError("Must agree to IP logging to proceed") - def cleanup(self, achalls): - # pylint: disable=missing-docstring,no-self-use,unused-argument + message = self._get_message(achall) + formated_message = message.format(validation=validation, + response=response) + + self._notify_and_wait(formated_message) + + if not response.simple_verify( + achall.chall, achall.domain, + achall.account_key.public_key()): + logger.warning("Self-verify of challenge failed.") + + return response + + def _cleanup_http01_challenge(self, achall): + # pylint: disable=missing-docstring,unused-argument if self.conf("test-mode"): assert self._httpd is not None, ( "cleanup() must be called after perform()") @@ -202,3 +245,14 @@ s.serve_forever()" """ logger.debug("Manual command process already terminated " "with %s code", self._httpd.returncode) shutil.rmtree(self._root) + + def _notify_and_wait(self, message): # pylint: disable=no-self-use + # TODO: IDisplay wraps messages, breaking the command + #answer = zope.component.getUtility(interfaces.IDisplay).notification( + # message=message, height=25, pause=True) + sys.stdout.write(message) + six.moves.input("Press ENTER to continue") + + def _get_message(self, achall): + # pylint: disable=missing-docstring,no-self-use,unused-argument + return self.MESSAGE_TEMPLATE.get(achall.chall.typ, "") diff --git a/certbot/plugins/manual_test.py b/certbot/plugins/manual_test.py index dd0905049..0686a24d7 100644 --- a/certbot/plugins/manual_test.py +++ b/certbot/plugins/manual_test.py @@ -24,10 +24,16 @@ class AuthenticatorTest(unittest.TestCase): from certbot.plugins.manual import Authenticator self.config = mock.MagicMock( http01_port=8080, manual_test_mode=False, - manual_public_ip_logging_ok=False, noninteractive_mode=True) + manual_public_ip_logging_ok=False, noninteractive_mode=True, + standalone_supported_challenges="dns-01,http-01") self.auth = Authenticator(config=self.config, name="manual") - self.achalls = [achallenges.KeyAuthorizationAnnotatedChallenge( - challb=acme_util.HTTP01_P, domain="foo.com", account_key=KEY)] + + self.http01 = achallenges.KeyAuthorizationAnnotatedChallenge( + challb=acme_util.HTTP01_P, domain="foo.com", account_key=KEY) + self.dns01 = achallenges.KeyAuthorizationAnnotatedChallenge( + challb=acme_util.DNS01_P, domain="foo.com", account_key=KEY) + + self.achalls = [self.http01, self.dns01] config_test_mode = mock.MagicMock( http01_port=8080, manual_test_mode=True, noninteractive_mode=True) @@ -56,19 +62,21 @@ class AuthenticatorTest(unittest.TestCase): mock_verify.return_value = True mock_interaction().yesno.return_value = True - resp = self.achalls[0].response(KEY) - self.assertEqual([resp], self.auth.perform(self.achalls)) - self.assertEqual(1, mock_raw_input.call_count) + resp_http = self.http01.response(KEY) + resp_dns = self.dns01.response(KEY) + + self.assertEqual([resp_http, resp_dns], self.auth.perform(self.achalls)) + self.assertEqual(2, mock_raw_input.call_count) mock_verify.assert_called_with( - self.achalls[0].challb.chall, "foo.com", KEY.public_key(), 8080) + self.http01.challb.chall, "foo.com", KEY.public_key(), 8080) message = mock_stdout.write.mock_calls[0][1][0] - self.assertTrue(self.achalls[0].chall.encode("token") in message) + self.assertTrue(self.http01.chall.encode("token") in message) mock_verify.return_value = False with mock.patch("certbot.plugins.manual.logger") as mock_logger: self.auth.perform(self.achalls) - mock_logger.warning.assert_called_once_with(mock.ANY) + self.assertEqual(2, mock_logger.warning.call_count) @mock.patch("certbot.plugins.manual.zope.component.getUtility") @mock.patch("certbot.plugins.manual.Authenticator._notify_and_wait") @@ -82,10 +90,10 @@ class AuthenticatorTest(unittest.TestCase): @mock.patch("certbot.plugins.manual.subprocess.Popen", autospec=True) def test_perform_test_command_oserror(self, mock_popen): mock_popen.side_effect = OSError - self.assertEqual([False], self.auth_test_mode.perform(self.achalls)) + self.assertEqual([False], self.auth_test_mode.perform([self.http01])) - @mock.patch("certbot.plugins.manual.socket.socket") - @mock.patch("certbot.plugins.manual.time.sleep", autospec=True) + @mock.patch("certbot.util.socket.socket") + @mock.patch("certbot.util.time.sleep", autospec=True) @mock.patch("certbot.plugins.manual.subprocess.Popen", autospec=True) def test_perform_test_command_run_failure( self, mock_popen, unused_mock_sleep, unused_mock_socket): diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index 97aca351a..bf38ced27 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -1,5 +1,4 @@ """Standalone Authenticator.""" -import argparse import collections import logging import socket @@ -9,6 +8,8 @@ import OpenSSL import six import zope.interface +from functools import partial + from acme import challenges from acme import standalone as acme_standalone @@ -113,36 +114,6 @@ class ServerManager(object): SUPPORTED_CHALLENGES = [challenges.TLSSNI01, challenges.HTTP01] -def supported_challenges_validator(data): - """Supported challenges validator for the `argparse`. - - It should be passed as `type` argument to `add_argument`. - - """ - challs = data.split(",") - - # tls-sni-01 was dvsni during private beta - if "dvsni" in challs: - logger.info("Updating legacy standalone_supported_challenges value") - challs = [challenges.TLSSNI01.typ if chall == "dvsni" else chall - for chall in challs] - data = ",".join(challs) - - unrecognized = [name for name in challs - if name not in challenges.Challenge.TYPES] - if unrecognized: - raise argparse.ArgumentTypeError( - "Unrecognized challenges: {0}".format(", ".join(unrecognized))) - - choices = set(chall.typ for chall in SUPPORTED_CHALLENGES) - if not set(challs).issubset(choices): - raise argparse.ArgumentTypeError( - "Plugin does not support the following (valid) " - "challenges: {0}".format(", ".join(set(challs) - choices))) - - return data - - @zope.interface.implementer(interfaces.IAuthenticator) @zope.interface.provider(interfaces.IPluginFactory) class Authenticator(common.Plugin): @@ -176,9 +147,12 @@ class Authenticator(common.Plugin): @classmethod def add_parser_arguments(cls, add): + validator = partial(util.supported_challenges_validator, + supported=SUPPORTED_CHALLENGES) + add("supported-challenges", help="Supported challenges. Preferred in the order they are listed.", - type=supported_challenges_validator, + type=validator, default=",".join(chall.typ for chall in SUPPORTED_CHALLENGES)) @property diff --git a/certbot/plugins/standalone_test.py b/certbot/plugins/standalone_test.py index eb6631732..914332307 100644 --- a/certbot/plugins/standalone_test.py +++ b/certbot/plugins/standalone_test.py @@ -1,5 +1,4 @@ """Tests for certbot.plugins.standalone.""" -import argparse import socket import unittest @@ -64,33 +63,6 @@ class ServerManagerTest(unittest.TestCase): self.assertEqual(self.mgr.running(), {}) -class SupportedChallengesValidatorTest(unittest.TestCase): - """Tests for plugins.standalone.supported_challenges_validator.""" - - def _call(self, data): - from certbot.plugins.standalone import ( - supported_challenges_validator) - return supported_challenges_validator(data) - - def test_correct(self): - self.assertEqual("tls-sni-01", self._call("tls-sni-01")) - self.assertEqual("http-01", self._call("http-01")) - self.assertEqual("tls-sni-01,http-01", self._call("tls-sni-01,http-01")) - self.assertEqual("http-01,tls-sni-01", self._call("http-01,tls-sni-01")) - - def test_unrecognized(self): - assert "foo" not in challenges.Challenge.TYPES - self.assertRaises(argparse.ArgumentTypeError, self._call, "foo") - - def test_not_subset(self): - self.assertRaises(argparse.ArgumentTypeError, self._call, "dns") - - def test_dvsni(self): - self.assertEqual("tls-sni-01", self._call("dvsni")) - self.assertEqual("http-01,tls-sni-01", self._call("http-01,dvsni")) - self.assertEqual("tls-sni-01,http-01", self._call("dvsni,http-01")) - - class AuthenticatorTest(unittest.TestCase): """Tests for certbot.plugins.standalone.Authenticator.""" diff --git a/certbot/plugins/util.py b/certbot/plugins/util.py index b97ca1afd..5a8789c79 100644 --- a/certbot/plugins/util.py +++ b/certbot/plugins/util.py @@ -1,10 +1,13 @@ """Plugin utilities.""" +import argparse import logging import os import socket import zope.component +from acme import challenges + from certbot import interfaces from certbot import util @@ -154,3 +157,36 @@ def already_listening_psutil(port, renewer=False): # name (AccessDenied). pass return False + + +def supported_challenges_validator(data, supported=None): + """Supported challenges validator for the `argparse`. + + It should be passed as `type` argument to `add_argument`. + + :param str data: input value representing the supported_challenges + :returns: original value if valid + """ + challs = data.split(",") + supported = supported or [] + + # tls-sni-01 was dvsni during private beta + if "dvsni" in challs: + logger.info("Updating legacy standalone_supported_challenges value") + challs = [challenges.TLSSNI01.typ if chall == "dvsni" else chall + for chall in challs] + data = ",".join(challs) + + unrecognized = [name for name in challs + if name not in challenges.Challenge.TYPES] + if unrecognized: + raise argparse.ArgumentTypeError( + "Unrecognized challenges: {0}".format(", ".join(unrecognized))) + + choices = set(chall.typ for chall in supported) + if not set(challs).issubset(choices): + raise argparse.ArgumentTypeError( + "Plugin does not support the following (valid) " + "challenges: {0}".format(", ".join(set(challs) - choices))) + + return data diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index e1a064fb3..798d7780e 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -1,4 +1,5 @@ """Tests for certbot.plugins.util.""" +import argparse import os import unittest import sys @@ -181,5 +182,42 @@ class AlreadyListeningTestPsutil(unittest.TestCase): mock_net.side_effect = psutil.AccessDenied("") self.assertFalse(self._call(12345)) + +class SupportedChallengesValidatorTest(unittest.TestCase): + """Tests for plugins.standalone.supported_challenges_validator.""" + + def _call(self, data): + from certbot.plugins.util import supported_challenges_validator + from acme import challenges + + supported = [challenges.HTTP01, challenges.DNS01, challenges.TLSSNI01] + + return supported_challenges_validator(data, supported=supported) + + def test_correct(self): + self.assertEqual("tls-sni-01", self._call("tls-sni-01")) + self.assertEqual("http-01", self._call("http-01")) + self.assertEqual("tls-sni-01,http-01", self._call("tls-sni-01,http-01")) + self.assertEqual("http-01,tls-sni-01", self._call("http-01,tls-sni-01")) + + def test_unrecognized(self): + from acme import challenges + + assert "foo" not in challenges.Challenge.TYPES + self.assertRaises(argparse.ArgumentTypeError, self._call, "foo") + + def test_not_subset(self): + self.assertRaises(argparse.ArgumentTypeError, self._call, "dns") + + def test_dvsni(self): + self.assertEqual("tls-sni-01", self._call("dvsni")) + self.assertEqual("http-01,tls-sni-01", self._call("http-01,dvsni")) + self.assertEqual("tls-sni-01,http-01", self._call("dvsni,http-01")) + + def test_dns01(self): + self.assertEqual("dns-01", self._call("dns-01")) + self.assertEqual("http-01,dns-01", self._call("http-01,dns-01")) + self.assertEqual("dns-01,http-01", self._call("dns-01,http-01")) + if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot/tests/acme_util.py b/certbot/tests/acme_util.py index 4f6e86cc7..de64dfef9 100644 --- a/certbot/tests/acme_util.py +++ b/certbot/tests/acme_util.py @@ -17,9 +17,9 @@ HTTP01 = challenges.HTTP01( token=b"evaGxfADs6pSRb2LAv9IZf17Dt3juxGJ+PCt92wr+oA") TLSSNI01 = challenges.TLSSNI01( token=jose.b64decode(b"evaGxfADs6pSRb2LAv9IZf17Dt3juxGJyPCt92wrDoA")) -DNS = challenges.DNS(token=b"17817c66b60ce2e4012dfad92657527a") +DNS01 = challenges.DNS01(token=b"17817c66b60ce2e4012dfad92657527a") -CHALLENGES = [HTTP01, TLSSNI01, DNS] +CHALLENGES = [HTTP01, TLSSNI01, DNS01] def gen_combos(challbs): @@ -45,9 +45,9 @@ def chall_to_challb(chall, status): # pylint: disable=redefined-outer-name # Pending ChallengeBody objects TLSSNI01_P = chall_to_challb(TLSSNI01, messages.STATUS_PENDING) HTTP01_P = chall_to_challb(HTTP01, messages.STATUS_PENDING) -DNS_P = chall_to_challb(DNS, messages.STATUS_PENDING) +DNS01_P = chall_to_challb(DNS01, messages.STATUS_PENDING) -CHALLENGES_P = [HTTP01_P, TLSSNI01_P, DNS_P] +CHALLENGES_P = [HTTP01_P, TLSSNI01_P, DNS01_P] def gen_authzr(authz_status, domain, challs, statuses, combos=True): diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index fce130f7c..fcc22f41c 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -111,7 +111,7 @@ class GetAuthorizationsTest(unittest.TestCase): mock_poll.side_effect = self._validate_all self.mock_auth.get_chall_pref.return_value.append(challenges.HTTP01) - self.mock_auth.get_chall_pref.return_value.append(challenges.DNS) + self.mock_auth.get_chall_pref.return_value.append(challenges.DNS01) authzr = self.handler.get_authorizations(["0"]) @@ -125,7 +125,7 @@ class GetAuthorizationsTest(unittest.TestCase): self.assertEqual(self.mock_auth.cleanup.call_count, 1) # Test if list first element is TLSSNI01, use typ because it is an achall for achall in self.mock_auth.cleanup.call_args[0][0]: - self.assertTrue(achall.typ in ["tls-sni-01", "http-01", "dns"]) + self.assertTrue(achall.typ in ["tls-sni-01", "http-01", "dns-01"]) # Length of authorizations list self.assertEqual(len(authzr), 1) @@ -240,7 +240,7 @@ class PollChallengesTest(unittest.TestCase): from certbot.auth_handler import challb_to_achall self.mock_net.poll.side_effect = self._mock_poll_solve_one_valid self.chall_update[self.doms[0]].append( - challb_to_achall(acme_util.DNS_P, "key", self.doms[0])) + challb_to_achall(acme_util.DNS01_P, "key", self.doms[0])) self.assertRaises( errors.AuthorizationError, self.handler._poll_challenges, self.chall_update, False) @@ -342,7 +342,7 @@ class GenChallengePathTest(unittest.TestCase): self.assertTrue(self._call(challbs[::-1], prefs, None)) def test_not_supported(self): - challbs = (acme_util.DNS_P, acme_util.TLSSNI01_P) + challbs = (acme_util.DNS01_P, acme_util.TLSSNI01_P) prefs = [challenges.TLSSNI01] combos = ((0, 1),) diff --git a/certbot/tests/errors_test.py b/certbot/tests/errors_test.py index 67611ed45..f35a5ea08 100644 --- a/certbot/tests/errors_test.py +++ b/certbot/tests/errors_test.py @@ -16,12 +16,12 @@ class FaiiledChallengesTest(unittest.TestCase): from certbot.errors import FailedChallenges self.error = FailedChallenges(set([achallenges.DNS( domain="example.com", challb=messages.ChallengeBody( - chall=acme_util.DNS, uri=None, + chall=acme_util.DNS01, uri=None, error=messages.Error(typ="tls", detail="detail")))])) def test_str(self): self.assertTrue(str(self.error).startswith( - "Failed authorization procedure. example.com (dns): tls")) + "Failed authorization procedure. example.com (dns-01): tls")) class StandaloneBindErrorTest(unittest.TestCase): diff --git a/certbot/util.py b/certbot/util.py index e78ae664c..e5d671ce1 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -14,6 +14,7 @@ import socket import stat import subprocess import sys +import time import configargparse @@ -474,3 +475,23 @@ def get_strict_version(normalized): # strict version ending with "a" and a number designates a pre-release # pylint: disable=no-member return distutils.version.StrictVersion(normalized.replace(".dev", "a")) + + +def busy_wait(port, host="localhost"): + """Artificialy wait a fixed amount of time on a specific host and port + + :param str port: port of the connection + :param str host: hostname of the connection, "localhost" if None + + """ + while True: + time.sleep(1) + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + try: + sock.connect((host, port)) + except socket.error: # pragma: no cover + pass + else: + break + finally: + sock.close() From fff459e6f53a7a03122fd0af699e2a8e5b1c6aec Mon Sep 17 00:00:00 2001 From: Mathieu Leduc-Hamel Date: Sat, 13 Aug 2016 11:09:17 -0400 Subject: [PATCH 02/36] Factored out the ip logging permission functionalities --- certbot/plugins/manual.py | 48 +++++++++++++++++++-------------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/certbot/plugins/manual.py b/certbot/plugins/manual.py index 608484031..b3cb2bec3 100644 --- a/certbot/plugins/manual.py +++ b/certbot/plugins/manual.py @@ -47,12 +47,11 @@ class Authenticator(common.Plugin): MESSAGE_TEMPLATE = { "dns-01": """\ -Make sure your dns configuration content the following key before continuing: +To prove control of the domain {domain}, please deploy a DNS TXT record with the following value: {validation} -if you didn't, make sure to add the validation as TXT record into your domain -configuration. +Once this is deployed, """, "http-01": """\ Make sure your web server displays the following content at @@ -189,20 +188,14 @@ s.serve_forever()" """ if self._httpd.poll() is not None: raise errors.Error("Couldn't execute manual command") else: - if not self.conf("public-ip-logging-ok"): - if not zope.component.getUtility(interfaces.IDisplay).yesno( - self.IP_DISCLAIMER, "Yes", "No", - cli_flag="--manual-public-ip-logging-ok"): - raise errors.PluginError("Must agree to IP logging to proceed") - message = self._get_message(achall) + uri = achall.chall.uri(achall.domain) + formated_message = message.format(validation=validation, + response=response, + uri=uri, + command=command) - self._notify_and_wait(message.format( - validation=validation, - response=response, - uri=achall.chall.uri(achall.domain), - command=command - )) + self._ip_logging_permission(response, formated_message) if not response.simple_verify( achall.chall, achall.domain, @@ -214,17 +207,11 @@ s.serve_forever()" """ def _perform_dns01_challenge(self, achall): response, validation = achall.response_and_validation() if not self.conf("test-mode"): - if not self.conf("public-ip-logging-ok"): - if not zope.component.getUtility(interfaces.IDisplay).yesno( - self.IP_DISCLAIMER, "Yes", "No", - cli_flag="--manual-public-ip-logging-ok"): - raise errors.PluginError("Must agree to IP logging to proceed") - message = self._get_message(achall) formated_message = message.format(validation=validation, + domain=achall.domain, response=response) - - self._notify_and_wait(formated_message) + self._ip_logging_permission(response, formated_message) if not response.simple_verify( achall.chall, achall.domain, @@ -246,13 +233,26 @@ s.serve_forever()" """ "with %s code", self._httpd.returncode) shutil.rmtree(self._root) - def _notify_and_wait(self, message): # pylint: disable=no-self-use + def _notify_and_wait(self, message): + # pylint: disable=no-self-use # TODO: IDisplay wraps messages, breaking the command #answer = zope.component.getUtility(interfaces.IDisplay).notification( # message=message, height=25, pause=True) sys.stdout.write(message) six.moves.input("Press ENTER to continue") + def _ip_logging_permission(self, response, formated_message): + # pylint: disable=missing-docstring + if not self.conf("public-ip-logging-ok"): + if not zope.component.getUtility(interfaces.IDisplay).yesno( + self.IP_DISCLAIMER, "Yes", "No", + cli_flag="--manual-public-ip-logging-ok"): + raise errors.PluginError("Must agree to IP logging to proceed") + else: + self.config.namespace.manual_public_ip_logging_ok = True + + self._notify_and_wait(formated_message) + def _get_message(self, achall): # pylint: disable=missing-docstring,no-self-use,unused-argument return self.MESSAGE_TEMPLATE.get(achall.chall.typ, "") From 075e66630030ea7f2a33afbe3127e8b12d5e5bc1 Mon Sep 17 00:00:00 2001 From: Mathieu Leduc-Hamel Date: Thu, 18 Aug 2016 09:54:30 -0400 Subject: [PATCH 03/36] Fixes broken tests --- certbot/plugins/manual.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/certbot/plugins/manual.py b/certbot/plugins/manual.py index b3cb2bec3..a42d3d2ce 100644 --- a/certbot/plugins/manual.py +++ b/certbot/plugins/manual.py @@ -195,7 +195,7 @@ s.serve_forever()" """ uri=uri, command=command) - self._ip_logging_permission(response, formated_message) + self._ip_logging_permission(formated_message) if not response.simple_verify( achall.chall, achall.domain, @@ -211,7 +211,7 @@ s.serve_forever()" """ formated_message = message.format(validation=validation, domain=achall.domain, response=response) - self._ip_logging_permission(response, formated_message) + self._ip_logging_permission(formated_message) if not response.simple_verify( achall.chall, achall.domain, @@ -241,7 +241,7 @@ s.serve_forever()" """ sys.stdout.write(message) six.moves.input("Press ENTER to continue") - def _ip_logging_permission(self, response, formated_message): + def _ip_logging_permission(self, formated_message): # pylint: disable=missing-docstring if not self.conf("public-ip-logging-ok"): if not zope.component.getUtility(interfaces.IDisplay).yesno( From 9958a7fc1cb78a72823288f0978ea1459548bc8c Mon Sep 17 00:00:00 2001 From: Mathieu Leduc-Hamel Date: Thu, 18 Aug 2016 14:55:03 -0400 Subject: [PATCH 04/36] Handle missing dnspython by displaying a warning message --- acme/acme/challenges.py | 4 ++-- acme/acme/errors.py | 4 ++++ certbot/plugins/manual.py | 11 +++++++++-- certbot/plugins/manual_test.py | 14 ++++++++++++++ 4 files changed, 29 insertions(+), 4 deletions(-) diff --git a/acme/acme/challenges.py b/acme/acme/challenges.py index 6242c376c..4ebd37bf9 100644 --- a/acme/acme/challenges.py +++ b/acme/acme/challenges.py @@ -234,8 +234,8 @@ class DNS01Response(KeyAuthorizationChallengeResponse): try: from acme import dns_resolver except ImportError: # pragma: no cover - raise errors.Error("Local validation for 'dns-01' challenges " - "requires 'dnspython'") + raise errors.DependencyError("Local validation for 'dns-01' " + "challenges requires 'dnspython'") txt_records = dns_resolver.txt_records_for_name(validation_domain_name) exists = validation in txt_records if not exists: diff --git a/acme/acme/errors.py b/acme/acme/errors.py index 70894a808..7446b60fc 100644 --- a/acme/acme/errors.py +++ b/acme/acme/errors.py @@ -6,6 +6,10 @@ class Error(Exception): """Generic ACME error.""" +class DependencyError(Error): + """Dependency error""" + + class SchemaValidationError(jose_errors.DeserializationError): """JSON schema ACME object validation error.""" diff --git a/certbot/plugins/manual.py b/certbot/plugins/manual.py index a42d3d2ce..a7ba571d7 100644 --- a/certbot/plugins/manual.py +++ b/certbot/plugins/manual.py @@ -15,6 +15,7 @@ import zope.interface from functools import partial from acme import challenges +from acme import errors as acme_errors from certbot import errors from certbot import interfaces @@ -213,9 +214,15 @@ s.serve_forever()" """ response=response) self._ip_logging_permission(formated_message) - if not response.simple_verify( + try: + verification_status = response.simple_verify( achall.chall, achall.domain, - achall.account_key.public_key()): + achall.account_key.public_key()) + except acme_errors.DependencyError: + verification_status = False + logger.warning("Dns challenge requires `dnspython`") + + if not verification_status: logger.warning("Self-verify of challenge failed.") return response diff --git a/certbot/plugins/manual_test.py b/certbot/plugins/manual_test.py index 0686a24d7..0179c2932 100644 --- a/certbot/plugins/manual_test.py +++ b/certbot/plugins/manual_test.py @@ -5,6 +5,7 @@ import unittest import mock from acme import challenges +from acme import errors as acme_errors from acme import jose from certbot import achallenges @@ -78,6 +79,19 @@ class AuthenticatorTest(unittest.TestCase): self.auth.perform(self.achalls) self.assertEqual(2, mock_logger.warning.call_count) + @mock.patch("certbot.plugins.manual.zope.component.getUtility") + @mock.patch("acme.challenges.DNS01Response.simple_verify") + @mock.patch("six.moves.input") + def test_perform_missing_dependency(self, mock_raw_input, mock_verify, mock_interaction): + mock_interaction().yesno.return_value = True + mock_verify.side_effect = acme_errors.DependencyError() + + with mock.patch("certbot.plugins.manual.logger") as mock_logger: + self.auth.perform([self.dns01]) + self.assertEqual(2, mock_logger.warning.call_count) + + mock_raw_input.assert_called_once_with("Press ENTER to continue") + @mock.patch("certbot.plugins.manual.zope.component.getUtility") @mock.patch("certbot.plugins.manual.Authenticator._notify_and_wait") def test_disagree_with_ip_logging(self, mock_notify, mock_interaction): From 51afe06ff7a51777408489305d85260700a3ed59 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Wed, 24 Aug 2016 16:18:23 -0700 Subject: [PATCH 05/36] in progress change --- certbot/auth_handler.py | 12 +++++++- certbot/cli.py | 40 +++++++++++++++++++++++++ certbot/client.py | 2 +- certbot/plugins/standalone.py | 47 +++++++++--------------------- certbot/plugins/standalone_test.py | 32 +++++++------------- certbot/tests/auth_handler_test.py | 6 ++-- certbot/tests/cli_test.py | 18 ++++++++++++ 7 files changed, 96 insertions(+), 61 deletions(-) diff --git a/certbot/auth_handler.py b/certbot/auth_handler.py index a94734572..dcde3f9a7 100644 --- a/certbot/auth_handler.py +++ b/certbot/auth_handler.py @@ -33,14 +33,16 @@ class AuthHandler(object): and values are :class:`acme.messages.AuthorizationResource` :ivar list achalls: DV challenges in the form of :class:`certbot.achallenges.AnnotatedChallenge` + :ivar list pref_challs: A list of user specified preferred challenges """ - def __init__(self, auth, acme, account): + def __init__(self, auth, acme, account, pref_challs): self.auth = auth self.acme = acme self.account = account self.authzr = dict() + self.pref_challs = pref_challs # List must be used to keep responses straight. self.achalls = [] @@ -246,6 +248,14 @@ class AuthHandler(object): """ # Make sure to make a copy... chall_prefs = [] + plugin_pref = self.auth.get_chall_pref(domain) + if self.pref_challs: + out = [pref for pref in self.pref_challs if pref in plugin_pref] + if out: + return out + else: + raise errors.AuthorizationError( + "None of the selected challenges are supported by the selected plugins") chall_prefs.extend(self.auth.get_chall_pref(domain)) return chall_prefs diff --git a/certbot/cli.py b/certbot/cli.py index 46ff74cd0..a0cd9b173 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -13,6 +13,8 @@ import six import certbot +from acme import challenges + from certbot import constants from certbot import crypto_util from certbot import errors @@ -844,6 +846,13 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis "security", "--strict-permissions", action="store_true", help="Require that all configuration files are owned by the current " "user; only needed if your config is somewhere unsafe like /tmp/") + helpful.add( + "security", "--preferred-challenges", dest="pref_chall", + action=_PrefChallAction, default=[], + help="Specify which challenges you'd prefer to use. If any of those " + "challenges are valid for your authenticator they will be used. " + "Otherwise Certbot will not attempt authorization. The first " + "challenge listed that is supported by the plugin will be used.") helpful.add( "renew", "--pre-hook", help="Command to be run in a shell before obtaining any certificates." @@ -1032,3 +1041,34 @@ def add_domains(args_or_config, domains): args_or_config.domains.append(domain) return validated_domains + +class _PrefChallAction(argparse.Action): + """Action class for parsing preferred challenges.""" + + def __call__(self, parser, namespace, pref_chall, option_string=None): + """Just wrap add_pref_challs in argparseese.""" + _ = add_pref_challs(namespace, pref_chall) + +def add_pref_challs(namespace, pref_challs): + """Parses and validates user specified challenge types. + + Adds challenges (in order) to the configuration object. + + :param namespace: parsed command line arguments + :type namespace: argparse.Namespace or + configuration.NamespaceConfig + :param str pref_challs: one or more comma separated challenge types + + :returns: Challenge objects which match the validated string inputs + :rtype: `list` + """ + challs = pref_challs.split(",") + unrecognized = [name for name in challs if name not in challenges.Challenge.TYPES] + if unrecognized: + raise argparse.ArgumentTypeError( + "Unrecognized challenges: {0}".format(", ".join(unrecognized))) + + out = [challenges.Challenge.TYPES[name] for name in challs] + print(namespace) + namespace.pref_chall.extend(out) + return out diff --git a/certbot/client.py b/certbot/client.py index 119fb0947..d80a67bad 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -186,7 +186,7 @@ class Client(object): if auth is not None: self.auth_handler = auth_handler.AuthHandler( - auth, self.acme, self.account) + auth, self.acme, self.account, self.config.pref_chall) else: self.auth_handler = None diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index 97aca351a..1fe79e8dd 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -3,6 +3,7 @@ import argparse import collections import logging import socket +import sys import threading import OpenSSL @@ -12,6 +13,7 @@ import zope.interface from acme import challenges from acme import standalone as acme_standalone +from certbot import cli from certbot import errors from certbot import interfaces @@ -110,38 +112,17 @@ class ServerManager(object): in six.iteritems(self._instances)) -SUPPORTED_CHALLENGES = [challenges.TLSSNI01, challenges.HTTP01] -def supported_challenges_validator(data): - """Supported challenges validator for the `argparse`. - - It should be passed as `type` argument to `add_argument`. - - """ - challs = data.split(",") - - # tls-sni-01 was dvsni during private beta - if "dvsni" in challs: - logger.info("Updating legacy standalone_supported_challenges value") - challs = [challenges.TLSSNI01.typ if chall == "dvsni" else chall - for chall in challs] - data = ",".join(challs) - - unrecognized = [name for name in challs - if name not in challenges.Challenge.TYPES] - if unrecognized: - raise argparse.ArgumentTypeError( - "Unrecognized challenges: {0}".format(", ".join(unrecognized))) - - choices = set(chall.typ for chall in SUPPORTED_CHALLENGES) - if not set(challs).issubset(choices): - raise argparse.ArgumentTypeError( - "Plugin does not support the following (valid) " - "challenges: {0}".format(", ".join(set(challs) - choices))) - - return data +class supported_challenges_wrapper(argparse.Action): + """Wrapper for the depricated supported challenges flag""" + def __call__(self, parser, namespace, pref_chall, option_string=None): + # print deprecation warning + sys.stderr.write("WARNING: The standalone specific supported challenges flag is depricated") + sys.stderr.write("\nPlease use the --preferred-challenges flag instead.\n") + #call cli version - move namespace back into it + _ = cli.add_pref_challs(namespace, pref_chall) @zope.interface.implementer(interfaces.IAuthenticator) @zope.interface.provider(interfaces.IPluginFactory) @@ -178,14 +159,12 @@ class Authenticator(common.Plugin): def add_parser_arguments(cls, add): add("supported-challenges", help="Supported challenges. Preferred in the order they are listed.", - type=supported_challenges_validator, - default=",".join(chall.typ for chall in SUPPORTED_CHALLENGES)) + action=supported_challenges_wrapper, dest="pref_chall") @property def supported_challenges(self): """Challenges supported by this plugin.""" - return [challenges.Challenge.TYPES[name] for name in - self.conf("supported-challenges").split(",")] + return self.config.pref_chall @property def _necessary_ports(self): @@ -208,7 +187,7 @@ class Authenticator(common.Plugin): def get_chall_pref(self, domain): # pylint: disable=unused-argument,missing-docstring - return self.supported_challenges + return [challenges.TLSSNI01, challenges.HTTP01] def perform(self, achalls): # pylint: disable=missing-docstring renewer = self.config.verb == "renew" diff --git a/certbot/plugins/standalone_test.py b/certbot/plugins/standalone_test.py index eb6631732..b77ec0554 100644 --- a/certbot/plugins/standalone_test.py +++ b/certbot/plugins/standalone_test.py @@ -67,29 +67,17 @@ class ServerManagerTest(unittest.TestCase): class SupportedChallengesValidatorTest(unittest.TestCase): """Tests for plugins.standalone.supported_challenges_validator.""" - def _call(self, data): - from certbot.plugins.standalone import ( - supported_challenges_validator) - return supported_challenges_validator(data) - - def test_correct(self): - self.assertEqual("tls-sni-01", self._call("tls-sni-01")) - self.assertEqual("http-01", self._call("http-01")) - self.assertEqual("tls-sni-01,http-01", self._call("tls-sni-01,http-01")) - self.assertEqual("http-01,tls-sni-01", self._call("http-01,tls-sni-01")) - - def test_unrecognized(self): - assert "foo" not in challenges.Challenge.TYPES - self.assertRaises(argparse.ArgumentTypeError, self._call, "foo") - - def test_not_subset(self): - self.assertRaises(argparse.ArgumentTypeError, self._call, "dns") - - def test_dvsni(self): - self.assertEqual("tls-sni-01", self._call("dvsni")) - self.assertEqual("http-01,tls-sni-01", self._call("http-01,dvsni")) - self.assertEqual("tls-sni-01,http-01", self._call("dvsni,http-01")) + def setUp(self): + self.parser = argparse.ArgumentParser() + from certbot.plugins import standalone + standalone.Authenticator.add_parser_arguments(self.parser.add_argument) + def test_standalone_flag(self): + config = self.parser.parse_args(["--supported_challenges", "http-01"]) + http = challenges.Challenge.TYPES["http-01"] + tls = challenges.Challenge.TYPES["tls-sni-01"] + print config + self.assertEqual(config.pref_chall, [tls, http]) class AuthenticatorTest(unittest.TestCase): """Tests for certbot.plugins.standalone.Authenticator.""" diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index fce130f7c..e6e2445d9 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -24,7 +24,7 @@ class ChallengeFactoryTest(unittest.TestCase): from certbot.auth_handler import AuthHandler # Account is mocked... - self.handler = AuthHandler(None, None, mock.Mock(key="mock_key")) + self.handler = AuthHandler(None, None, mock.Mock(key="mock_key"), []) self.dom = "test" self.handler.authzr[self.dom] = acme_util.gen_authzr( @@ -74,7 +74,7 @@ class GetAuthorizationsTest(unittest.TestCase): self.mock_net = mock.MagicMock(spec=acme_client.Client) self.handler = AuthHandler( - self.mock_auth, self.mock_net, self.mock_account) + self.mock_auth, self.mock_net, self.mock_account, []) logging.disable(logging.CRITICAL) @@ -189,7 +189,7 @@ class PollChallengesTest(unittest.TestCase): # Account and network are mocked... self.mock_net = mock.MagicMock() self.handler = AuthHandler( - None, self.mock_net, mock.Mock(key="mock_key")) + None, self.mock_net, mock.Mock(key="mock_key"), []) self.doms = ["0", "1", "2"] self.handler.authzr[self.doms[0]] = acme_util.gen_authzr( diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 2c6e32705..3fe330b96 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -1035,6 +1035,24 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods namespace = parse(short_args) self.assertTrue(namespace.text_mode) + #TODO massage this to work in cli + def test_correct(self): + self.assertEqual("tls-sni-01", self._call("tls-sni-01")) + self.assertEqual("http-01", self._call("http-01")) + self.assertEqual("tls-sni-01,http-01", self._call("tls-sni-01,http-01")) + self.assertEqual("http-01,tls-sni-01", self._call("http-01,tls-sni-01")) + + def test_unrecognized(self): + assert "foo" not in challenges.Challenge.TYPES + self.assertRaises(argparse.ArgumentTypeError, self._call, "foo") + + def test_not_subset(self): + self.assertRaises(argparse.ArgumentTypeError, self._call, "dns") + + def test_dvsni(self): + self.assertEqual("tls-sni-01", self._call("dvsni")) + self.assertEqual("http-01,tls-sni-01", self._call("http-01,dvsni")) + self.assertEqual("tls-sni-01,http-01", self._call("dvsni,http-01")) class DetermineAccountTest(unittest.TestCase): """Tests for certbot.cli._determine_account.""" From d3bdf9772070d733c313c451215193567ce4e2c7 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 24 Aug 2016 16:39:41 -0700 Subject: [PATCH 06/36] fix standalone flag test --- certbot/plugins/standalone.py | 2 +- certbot/plugins/standalone_test.py | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index 1fe79e8dd..b239029ba 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -159,7 +159,7 @@ class Authenticator(common.Plugin): def add_parser_arguments(cls, add): add("supported-challenges", help="Supported challenges. Preferred in the order they are listed.", - action=supported_challenges_wrapper, dest="pref_chall") + action=supported_challenges_wrapper, default= [], dest="pref_chall") @property def supported_challenges(self): diff --git a/certbot/plugins/standalone_test.py b/certbot/plugins/standalone_test.py index b77ec0554..24b28c6eb 100644 --- a/certbot/plugins/standalone_test.py +++ b/certbot/plugins/standalone_test.py @@ -14,6 +14,8 @@ from certbot import achallenges from certbot import errors from certbot import interfaces +from certbot.plugins import disco + from certbot.tests import acme_util from certbot.tests import test_util @@ -69,14 +71,15 @@ class SupportedChallengesValidatorTest(unittest.TestCase): def setUp(self): self.parser = argparse.ArgumentParser() - from certbot.plugins import standalone - standalone.Authenticator.add_parser_arguments(self.parser.add_argument) + name = "standalone" + disco.PluginsRegistry.find_all()[name].plugin_cls.inject_parser_options( + self.parser, name) def test_standalone_flag(self): - config = self.parser.parse_args(["--supported_challenges", "http-01"]) + config = self.parser.parse_args(["--standalone-supported-challenges", + "tls-sni-01,http-01"]) http = challenges.Challenge.TYPES["http-01"] tls = challenges.Challenge.TYPES["tls-sni-01"] - print config self.assertEqual(config.pref_chall, [tls, http]) class AuthenticatorTest(unittest.TestCase): From 5b112cef7a6d52610d0ae5462cb5011e581bff50 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Thu, 25 Aug 2016 16:58:23 -0700 Subject: [PATCH 07/36] revert standalone --- certbot/plugins/standalone.py | 47 +++++++++++++++++++++--------- certbot/plugins/standalone_test.py | 35 +++++++++++++--------- certbot/tests/cli_test.py | 17 ----------- 3 files changed, 56 insertions(+), 43 deletions(-) diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index b239029ba..97aca351a 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -3,7 +3,6 @@ import argparse import collections import logging import socket -import sys import threading import OpenSSL @@ -13,7 +12,6 @@ import zope.interface from acme import challenges from acme import standalone as acme_standalone -from certbot import cli from certbot import errors from certbot import interfaces @@ -112,17 +110,38 @@ class ServerManager(object): in six.iteritems(self._instances)) +SUPPORTED_CHALLENGES = [challenges.TLSSNI01, challenges.HTTP01] -class supported_challenges_wrapper(argparse.Action): - """Wrapper for the depricated supported challenges flag""" +def supported_challenges_validator(data): + """Supported challenges validator for the `argparse`. + + It should be passed as `type` argument to `add_argument`. + + """ + challs = data.split(",") + + # tls-sni-01 was dvsni during private beta + if "dvsni" in challs: + logger.info("Updating legacy standalone_supported_challenges value") + challs = [challenges.TLSSNI01.typ if chall == "dvsni" else chall + for chall in challs] + data = ",".join(challs) + + unrecognized = [name for name in challs + if name not in challenges.Challenge.TYPES] + if unrecognized: + raise argparse.ArgumentTypeError( + "Unrecognized challenges: {0}".format(", ".join(unrecognized))) + + choices = set(chall.typ for chall in SUPPORTED_CHALLENGES) + if not set(challs).issubset(choices): + raise argparse.ArgumentTypeError( + "Plugin does not support the following (valid) " + "challenges: {0}".format(", ".join(set(challs) - choices))) + + return data - def __call__(self, parser, namespace, pref_chall, option_string=None): - # print deprecation warning - sys.stderr.write("WARNING: The standalone specific supported challenges flag is depricated") - sys.stderr.write("\nPlease use the --preferred-challenges flag instead.\n") - #call cli version - move namespace back into it - _ = cli.add_pref_challs(namespace, pref_chall) @zope.interface.implementer(interfaces.IAuthenticator) @zope.interface.provider(interfaces.IPluginFactory) @@ -159,12 +178,14 @@ class Authenticator(common.Plugin): def add_parser_arguments(cls, add): add("supported-challenges", help="Supported challenges. Preferred in the order they are listed.", - action=supported_challenges_wrapper, default= [], dest="pref_chall") + type=supported_challenges_validator, + default=",".join(chall.typ for chall in SUPPORTED_CHALLENGES)) @property def supported_challenges(self): """Challenges supported by this plugin.""" - return self.config.pref_chall + return [challenges.Challenge.TYPES[name] for name in + self.conf("supported-challenges").split(",")] @property def _necessary_ports(self): @@ -187,7 +208,7 @@ class Authenticator(common.Plugin): def get_chall_pref(self, domain): # pylint: disable=unused-argument,missing-docstring - return [challenges.TLSSNI01, challenges.HTTP01] + return self.supported_challenges def perform(self, achalls): # pylint: disable=missing-docstring renewer = self.config.verb == "renew" diff --git a/certbot/plugins/standalone_test.py b/certbot/plugins/standalone_test.py index 24b28c6eb..eb6631732 100644 --- a/certbot/plugins/standalone_test.py +++ b/certbot/plugins/standalone_test.py @@ -14,8 +14,6 @@ from certbot import achallenges from certbot import errors from certbot import interfaces -from certbot.plugins import disco - from certbot.tests import acme_util from certbot.tests import test_util @@ -69,18 +67,29 @@ class ServerManagerTest(unittest.TestCase): class SupportedChallengesValidatorTest(unittest.TestCase): """Tests for plugins.standalone.supported_challenges_validator.""" - def setUp(self): - self.parser = argparse.ArgumentParser() - name = "standalone" - disco.PluginsRegistry.find_all()[name].plugin_cls.inject_parser_options( - self.parser, name) + def _call(self, data): + from certbot.plugins.standalone import ( + supported_challenges_validator) + return supported_challenges_validator(data) + + def test_correct(self): + self.assertEqual("tls-sni-01", self._call("tls-sni-01")) + self.assertEqual("http-01", self._call("http-01")) + self.assertEqual("tls-sni-01,http-01", self._call("tls-sni-01,http-01")) + self.assertEqual("http-01,tls-sni-01", self._call("http-01,tls-sni-01")) + + def test_unrecognized(self): + assert "foo" not in challenges.Challenge.TYPES + self.assertRaises(argparse.ArgumentTypeError, self._call, "foo") + + def test_not_subset(self): + self.assertRaises(argparse.ArgumentTypeError, self._call, "dns") + + def test_dvsni(self): + self.assertEqual("tls-sni-01", self._call("dvsni")) + self.assertEqual("http-01,tls-sni-01", self._call("http-01,dvsni")) + self.assertEqual("tls-sni-01,http-01", self._call("dvsni,http-01")) - def test_standalone_flag(self): - config = self.parser.parse_args(["--standalone-supported-challenges", - "tls-sni-01,http-01"]) - http = challenges.Challenge.TYPES["http-01"] - tls = challenges.Challenge.TYPES["tls-sni-01"] - self.assertEqual(config.pref_chall, [tls, http]) class AuthenticatorTest(unittest.TestCase): """Tests for certbot.plugins.standalone.Authenticator.""" diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 3fe330b96..5a8845c4c 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -1036,23 +1036,6 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods self.assertTrue(namespace.text_mode) #TODO massage this to work in cli - def test_correct(self): - self.assertEqual("tls-sni-01", self._call("tls-sni-01")) - self.assertEqual("http-01", self._call("http-01")) - self.assertEqual("tls-sni-01,http-01", self._call("tls-sni-01,http-01")) - self.assertEqual("http-01,tls-sni-01", self._call("http-01,tls-sni-01")) - - def test_unrecognized(self): - assert "foo" not in challenges.Challenge.TYPES - self.assertRaises(argparse.ArgumentTypeError, self._call, "foo") - - def test_not_subset(self): - self.assertRaises(argparse.ArgumentTypeError, self._call, "dns") - - def test_dvsni(self): - self.assertEqual("tls-sni-01", self._call("dvsni")) - self.assertEqual("http-01,tls-sni-01", self._call("http-01,dvsni")) - self.assertEqual("tls-sni-01,http-01", self._call("dvsni,http-01")) class DetermineAccountTest(unittest.TestCase): """Tests for certbot.cli._determine_account.""" From d8031f16bd85aa4d2f1a04edf1b816a3bd5c2d47 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Thu, 25 Aug 2016 17:45:13 -0700 Subject: [PATCH 08/36] add deprication --- certbot/plugins/standalone.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index 97aca351a..c00f30052 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -3,6 +3,7 @@ import argparse import collections import logging import socket +import sys import threading import OpenSSL @@ -119,6 +120,8 @@ def supported_challenges_validator(data): It should be passed as `type` argument to `add_argument`. """ + sys.stderr.write("WARNING: The standalone specific supported challenges flag is depricated") + sys.stderr.write("\nPlease use the --preferred-challenges flag instead.\n") challs = data.split(",") # tls-sni-01 was dvsni during private beta @@ -177,7 +180,7 @@ class Authenticator(common.Plugin): @classmethod def add_parser_arguments(cls, add): add("supported-challenges", - help="Supported challenges. Preferred in the order they are listed.", + help=argparse.SUPPRESS, type=supported_challenges_validator, default=",".join(chall.typ for chall in SUPPORTED_CHALLENGES)) From 4814f043303a566d36e1368ce30b20bc8bc40458 Mon Sep 17 00:00:00 2001 From: Noah Swartz Date: Fri, 26 Aug 2016 13:11:45 -0700 Subject: [PATCH 09/36] remove old todo --- certbot/tests/cli_test.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 5a8845c4c..d011be957 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -1035,8 +1035,6 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods namespace = parse(short_args) self.assertTrue(namespace.text_mode) - #TODO massage this to work in cli - class DetermineAccountTest(unittest.TestCase): """Tests for certbot.cli._determine_account.""" From 17d54a5f6ada2ad8b43f3e2d540ce8f199cdd24c Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 11:59:07 -0700 Subject: [PATCH 10/36] make docstring more explicit --- certbot/auth_handler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/certbot/auth_handler.py b/certbot/auth_handler.py index dcde3f9a7..e9b4d66c8 100644 --- a/certbot/auth_handler.py +++ b/certbot/auth_handler.py @@ -33,7 +33,9 @@ class AuthHandler(object): and values are :class:`acme.messages.AuthorizationResource` :ivar list achalls: DV challenges in the form of :class:`certbot.achallenges.AnnotatedChallenge` - :ivar list pref_challs: A list of user specified preferred challenges + :ivar list pref_challs: sorted user specified preferred challenges + in the form of subclasses of :class:`acme.challenges.Challenge` + with the most preferred challenge listed first """ def __init__(self, auth, acme, account, pref_challs): From d4f81f825c72e6930e72c9e648612daa3dcc0ebc Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 12:18:57 -0700 Subject: [PATCH 11/36] minor _get_chall_pref cleanup --- certbot/auth_handler.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/certbot/auth_handler.py b/certbot/auth_handler.py index e9b4d66c8..cc8beb463 100644 --- a/certbot/auth_handler.py +++ b/certbot/auth_handler.py @@ -248,17 +248,18 @@ class AuthHandler(object): :param str domain: domain for which you are requesting preferences """ - # Make sure to make a copy... chall_prefs = [] + # Make sure to make a copy... plugin_pref = self.auth.get_chall_pref(domain) if self.pref_challs: - out = [pref for pref in self.pref_challs if pref in plugin_pref] - if out: - return out - else: - raise errors.AuthorizationError( - "None of the selected challenges are supported by the selected plugins") - chall_prefs.extend(self.auth.get_chall_pref(domain)) + chall_prefs.extend(pref for pref in self.pref_challs + if pref in plugin_pref) + if chall_prefs: + return chall_prefs + raise errors.AuthorizationError( + "None of the preferred challenges " + "are supported by the selected plugin") + chall_prefs.extend(plugin_pref) return chall_prefs def _cleanup_challenges(self, achall_list=None): From 1560fd46805571032e0ba77814121423bd27b7d3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 12:41:55 -0700 Subject: [PATCH 12/36] cleanup pref_challs help, use consistent naming, and simplify parsing --- certbot/cli.py | 53 ++++++++++++++++------------------------------- certbot/client.py | 2 +- 2 files changed, 19 insertions(+), 36 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index a0cd9b173..c236041ce 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -11,10 +11,10 @@ import sys import configargparse import six -import certbot - from acme import challenges +import certbot + from certbot import constants from certbot import crypto_util from certbot import errors @@ -847,12 +847,13 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis help="Require that all configuration files are owned by the current " "user; only needed if your config is somewhere unsafe like /tmp/") helpful.add( - "security", "--preferred-challenges", dest="pref_chall", - action=_PrefChallAction, default=[], - help="Specify which challenges you'd prefer to use. If any of those " - "challenges are valid for your authenticator they will be used. " - "Otherwise Certbot will not attempt authorization. The first " - "challenge listed that is supported by the plugin will be used.") + ["certonly", "renew", "run"], "--preferred-challenges", + dest="pref_challs", action=_PrefChallAction, default=[], + help="A sorted, comma delimited list of the preferred challenge to " + "use during authorization with the most preferred challenge " + "listed first (e.g. tls-sni-01,http-01). If none of the " + "preferred challenges can be used by the selected plugin to " + "satisfy the CA, authorization is not attempted.") helpful.add( "renew", "--pre-hook", help="Command to be run in a shell before obtaining any certificates." @@ -1045,30 +1046,12 @@ def add_domains(args_or_config, domains): class _PrefChallAction(argparse.Action): """Action class for parsing preferred challenges.""" - def __call__(self, parser, namespace, pref_chall, option_string=None): - """Just wrap add_pref_challs in argparseese.""" - _ = add_pref_challs(namespace, pref_chall) - -def add_pref_challs(namespace, pref_challs): - """Parses and validates user specified challenge types. - - Adds challenges (in order) to the configuration object. - - :param namespace: parsed command line arguments - :type namespace: argparse.Namespace or - configuration.NamespaceConfig - :param str pref_challs: one or more comma separated challenge types - - :returns: Challenge objects which match the validated string inputs - :rtype: `list` - """ - challs = pref_challs.split(",") - unrecognized = [name for name in challs if name not in challenges.Challenge.TYPES] - if unrecognized: - raise argparse.ArgumentTypeError( - "Unrecognized challenges: {0}".format(", ".join(unrecognized))) - - out = [challenges.Challenge.TYPES[name] for name in challs] - print(namespace) - namespace.pref_chall.extend(out) - return out + def __call__(self, parser, namespace, pref_challs, option_string=None): + challs = pref_challs.split(",") + unrecognized = ", ".join(name for name in challs + if name not in challenges.Challenge.TYPES) + if unrecognized: + raise argparse.ArgumentTypeError( + "Unrecognized challenges: {0}".format(unrecognized)) + namespace.pref_challs.extend(challenges.Challenge.TYPES[name] + for name in challs) diff --git a/certbot/client.py b/certbot/client.py index 66e90bb1f..44eb67e48 100644 --- a/certbot/client.py +++ b/certbot/client.py @@ -192,7 +192,7 @@ class Client(object): if auth is not None: self.auth_handler = auth_handler.AuthHandler( - auth, self.acme, self.account, self.config.pref_chall) + auth, self.acme, self.account, self.config.pref_challs) else: self.auth_handler = None From c0d060a8f1e45d23048638851735ed4a394fbebd Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 12:44:20 -0700 Subject: [PATCH 13/36] fix typo and long line --- certbot/plugins/standalone.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index c00f30052..f3cd4b49d 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -120,8 +120,10 @@ def supported_challenges_validator(data): It should be passed as `type` argument to `add_argument`. """ - sys.stderr.write("WARNING: The standalone specific supported challenges flag is depricated") - sys.stderr.write("\nPlease use the --preferred-challenges flag instead.\n") + sys.stderr.write( + "WARNING: The standalone specific " + "supported challenges flag is deprecated\n") + sys.stderr.write("Please use the --preferred-challenges flag instead.\n") challs = data.split(",") # tls-sni-01 was dvsni during private beta From 74677946373d38a7e1720b70c45332d958cde0b3 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 12:46:41 -0700 Subject: [PATCH 14/36] readd newline to prevent pep8 errors --- certbot/tests/cli_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index d011be957..2c6e32705 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -1035,6 +1035,7 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods namespace = parse(short_args) self.assertTrue(namespace.text_mode) + class DetermineAccountTest(unittest.TestCase): """Tests for certbot.cli._determine_account.""" From 24cc03d1f6e3d3fb5a89aa489b0bd2f71fc80684 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 13:32:19 -0700 Subject: [PATCH 15/36] only print deprecation warning when --standalone-supported-challenges is set --- certbot/plugins/standalone.py | 10 ++++++---- certbot/plugins/standalone_test.py | 17 ++++++++++++++++- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/certbot/plugins/standalone.py b/certbot/plugins/standalone.py index f3cd4b49d..0195b2726 100644 --- a/certbot/plugins/standalone.py +++ b/certbot/plugins/standalone.py @@ -13,6 +13,7 @@ import zope.interface from acme import challenges from acme import standalone as acme_standalone +from certbot import cli from certbot import errors from certbot import interfaces @@ -120,10 +121,11 @@ def supported_challenges_validator(data): It should be passed as `type` argument to `add_argument`. """ - sys.stderr.write( - "WARNING: The standalone specific " - "supported challenges flag is deprecated\n") - sys.stderr.write("Please use the --preferred-challenges flag instead.\n") + if cli.set_by_cli("standalone_supported_challenges"): + sys.stderr.write( + "WARNING: The standalone specific " + "supported challenges flag is deprecated.\n" + "Please use the --preferred-challenges flag instead.\n") challs = data.split(",") # tls-sni-01 was dvsni during private beta diff --git a/certbot/plugins/standalone_test.py b/certbot/plugins/standalone_test.py index eb6631732..1dfa3950a 100644 --- a/certbot/plugins/standalone_test.py +++ b/certbot/plugins/standalone_test.py @@ -67,10 +67,25 @@ class ServerManagerTest(unittest.TestCase): class SupportedChallengesValidatorTest(unittest.TestCase): """Tests for plugins.standalone.supported_challenges_validator.""" + def setUp(self): + self.set_by_cli_patch = mock.patch( + "certbot.plugins.standalone.cli.set_by_cli") + self.stderr_patch = mock.patch("certbot.plugins.standalone.sys.stderr") + + self.set_by_cli_patch.start().return_value = True + self.stderr = self.stderr_patch.start() + + def tearDown(self): + self.set_by_cli_patch.stop() + self.stderr_patch.stop() + def _call(self, data): from certbot.plugins.standalone import ( supported_challenges_validator) - return supported_challenges_validator(data) + return_value = supported_challenges_validator(data) + self.assertTrue(self.stderr.write.called) # pylint: disable=no-member + self.stderr.write.reset_mock() # pylint: disable=no-member + return return_value def test_correct(self): self.assertEqual("tls-sni-01", self._call("tls-sni-01")) From 606636766e85469bd7268a12f24a39797a4c02aa Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 14:13:52 -0700 Subject: [PATCH 16/36] Test preferred challenges not supported --- certbot/tests/auth_handler_test.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index e6e2445d9..4e2db2712 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -167,6 +167,13 @@ class GetAuthorizationsTest(unittest.TestCase): def test_no_domains(self): self.assertRaises(errors.AuthorizationError, self.handler.get_authorizations, []) + def test_preferred_challenges_not_supported(self): + self.mock_net.request_domain_challenges.side_effect = functools.partial( + gen_dom_authzr, challs=acme_util.CHALLENGES) + self.handler.pref_challs.append(challenges.HTTP01) + self.assertRaises( + errors.AuthorizationError, self.handler.get_authorizations, ["0"]) + def _validate_all(self, unused_1, unused_2): for dom in six.iterkeys(self.handler.authzr): azr = self.handler.authzr[dom] From 0f575cf80d48b66fbbd620364d51a03489e1dc3e Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 14:34:03 -0700 Subject: [PATCH 17/36] test that pref_challs is respected --- certbot/tests/auth_handler_test.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index 4e2db2712..bb1fbc912 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -167,6 +167,22 @@ class GetAuthorizationsTest(unittest.TestCase): def test_no_domains(self): self.assertRaises(errors.AuthorizationError, self.handler.get_authorizations, []) + @mock.patch("certbot.auth_handler.AuthHandler._poll_challenges") + def test_preferred_challenge_choice(self, mock_poll): + self.mock_net.request_domain_challenges.side_effect = functools.partial( + gen_dom_authzr, challs=acme_util.CHALLENGES) + + mock_poll.side_effect = self._validate_all + self.mock_auth.get_chall_pref.return_value.append(challenges.HTTP01) + + self.handler.pref_challs.extend((challenges.HTTP01, challenges.DNS,)) + + self.handler.get_authorizations(["0"]) + + self.assertEqual(self.mock_auth.cleanup.call_count, 1) + self.assertEqual( + self.mock_auth.cleanup.call_args[0][0][0].typ, "http-01") + def test_preferred_challenges_not_supported(self): self.mock_net.request_domain_challenges.side_effect = functools.partial( gen_dom_authzr, challs=acme_util.CHALLENGES) From 322546b8d132ad2a040606975f49b1d80e2c9f1b Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 15:44:31 -0700 Subject: [PATCH 18/36] make manual use --preferred-challenges flag --- certbot/plugins/manual.py | 41 ++++++++++++++++------------------ certbot/plugins/manual_test.py | 6 ++--- certbot/plugins/util.py | 36 ----------------------------- certbot/plugins/util_test.py | 38 ------------------------------- certbot/util.py | 21 ----------------- 5 files changed, 22 insertions(+), 120 deletions(-) diff --git a/certbot/plugins/manual.py b/certbot/plugins/manual.py index a7ba571d7..6fe8cd597 100644 --- a/certbot/plugins/manual.py +++ b/certbot/plugins/manual.py @@ -4,31 +4,27 @@ import logging import pipes import shutil import signal +import socket import subprocess import sys import tempfile +import time import six import zope.component import zope.interface -from functools import partial - from acme import challenges from acme import errors as acme_errors from certbot import errors from certbot import interfaces -from certbot.plugins import common, util -from certbot.util import busy_wait +from certbot.plugins import common logger = logging.getLogger(__name__) -SUPPORTED_CHALLENGES = [challenges.HTTP01, challenges.DNS01] - - @zope.interface.implementer(interfaces.IAuthenticator) @zope.interface.provider(interfaces.IPluginFactory) class Authenticator(common.Plugin): @@ -99,23 +95,10 @@ s.serve_forever()" """ @classmethod def add_parser_arguments(cls, add): - validator = partial(util.supported_challenges_validator, - supported=SUPPORTED_CHALLENGES) - add("test-mode", action="store_true", help="Test mode. Executes the manual command in subprocess.") add("public-ip-logging-ok", action="store_true", help="Automatically allows public IP logging.") - add("supported-challenges", - help="Supported challenges. Preferred in the order they are listed.", - type=validator, - default="http-01") - - @property - def supported_challenges(self): - """Challenges supported by this plugin.""" - return [challenges.Challenge.TYPES[name] for name in - self.conf("supported-challenges").split(",")] def prepare(self): # pylint: disable=missing-docstring,no-self-use if self.config.noninteractive_mode and not self.conf("test-mode"): @@ -132,7 +115,7 @@ s.serve_forever()" """ def get_chall_pref(self, domain): # pylint: disable=missing-docstring,no-self-use,unused-argument - return self.supported_challenges + return [challenges.HTTP01, challenges.DNS01] def perform(self, achalls): # pylint: disable=missing-docstring @@ -145,6 +128,20 @@ s.serve_forever()" """ responses.append(mapping[achall.typ](achall)) return responses + @classmethod + def _test_mode_busy_wait(cls, port): + while True: + time.sleep(1) + sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + try: + sock.connect(("localhost", port)) + except socket.error: # pragma: no cover + pass + else: + break + finally: + sock.close() + def cleanup(self, achalls): # pylint: disable=missing-docstring for achall in achalls: @@ -184,7 +181,7 @@ s.serve_forever()" """ logger.debug("Manual command running as PID %s.", self._httpd.pid) # give it some time to bootstrap, before we try to verify # (cert generation in case of simpleHttpS might take time) - busy_wait(port) + self._test_mode_busy_wait(port) if self._httpd.poll() is not None: raise errors.Error("Couldn't execute manual command") diff --git a/certbot/plugins/manual_test.py b/certbot/plugins/manual_test.py index 0179c2932..2fb679a36 100644 --- a/certbot/plugins/manual_test.py +++ b/certbot/plugins/manual_test.py @@ -32,7 +32,7 @@ class AuthenticatorTest(unittest.TestCase): self.http01 = achallenges.KeyAuthorizationAnnotatedChallenge( challb=acme_util.HTTP01_P, domain="foo.com", account_key=KEY) self.dns01 = achallenges.KeyAuthorizationAnnotatedChallenge( - challb=acme_util.DNS01_P, domain="foo.com", account_key=KEY) + challb=acme_util.DNS01_P, domain="foo.com", account_key=KEY) self.achalls = [self.http01, self.dns01] @@ -106,8 +106,8 @@ class AuthenticatorTest(unittest.TestCase): mock_popen.side_effect = OSError self.assertEqual([False], self.auth_test_mode.perform([self.http01])) - @mock.patch("certbot.util.socket.socket") - @mock.patch("certbot.util.time.sleep", autospec=True) + @mock.patch("certbot.plugins.manual.socket.socket") + @mock.patch("certbot.plugins.manual.time.sleep", autospec=True) @mock.patch("certbot.plugins.manual.subprocess.Popen", autospec=True) def test_perform_test_command_run_failure( self, mock_popen, unused_mock_sleep, unused_mock_socket): diff --git a/certbot/plugins/util.py b/certbot/plugins/util.py index 5a8789c79..b97ca1afd 100644 --- a/certbot/plugins/util.py +++ b/certbot/plugins/util.py @@ -1,13 +1,10 @@ """Plugin utilities.""" -import argparse import logging import os import socket import zope.component -from acme import challenges - from certbot import interfaces from certbot import util @@ -157,36 +154,3 @@ def already_listening_psutil(port, renewer=False): # name (AccessDenied). pass return False - - -def supported_challenges_validator(data, supported=None): - """Supported challenges validator for the `argparse`. - - It should be passed as `type` argument to `add_argument`. - - :param str data: input value representing the supported_challenges - :returns: original value if valid - """ - challs = data.split(",") - supported = supported or [] - - # tls-sni-01 was dvsni during private beta - if "dvsni" in challs: - logger.info("Updating legacy standalone_supported_challenges value") - challs = [challenges.TLSSNI01.typ if chall == "dvsni" else chall - for chall in challs] - data = ",".join(challs) - - unrecognized = [name for name in challs - if name not in challenges.Challenge.TYPES] - if unrecognized: - raise argparse.ArgumentTypeError( - "Unrecognized challenges: {0}".format(", ".join(unrecognized))) - - choices = set(chall.typ for chall in supported) - if not set(challs).issubset(choices): - raise argparse.ArgumentTypeError( - "Plugin does not support the following (valid) " - "challenges: {0}".format(", ".join(set(challs) - choices))) - - return data diff --git a/certbot/plugins/util_test.py b/certbot/plugins/util_test.py index 200c3d9c5..27ede6533 100644 --- a/certbot/plugins/util_test.py +++ b/certbot/plugins/util_test.py @@ -1,5 +1,4 @@ """Tests for certbot.plugins.util.""" -import argparse import os import unittest import sys @@ -212,42 +211,5 @@ class AlreadyListeningTestPsutil(unittest.TestCase): mock_net.side_effect = psutil.AccessDenied("") self.assertFalse(self._call(12345)) - -class SupportedChallengesValidatorTest(unittest.TestCase): - """Tests for plugins.standalone.supported_challenges_validator.""" - - def _call(self, data): - from certbot.plugins.util import supported_challenges_validator - from acme import challenges - - supported = [challenges.HTTP01, challenges.DNS01, challenges.TLSSNI01] - - return supported_challenges_validator(data, supported=supported) - - def test_correct(self): - self.assertEqual("tls-sni-01", self._call("tls-sni-01")) - self.assertEqual("http-01", self._call("http-01")) - self.assertEqual("tls-sni-01,http-01", self._call("tls-sni-01,http-01")) - self.assertEqual("http-01,tls-sni-01", self._call("http-01,tls-sni-01")) - - def test_unrecognized(self): - from acme import challenges - - assert "foo" not in challenges.Challenge.TYPES - self.assertRaises(argparse.ArgumentTypeError, self._call, "foo") - - def test_not_subset(self): - self.assertRaises(argparse.ArgumentTypeError, self._call, "dns") - - def test_dvsni(self): - self.assertEqual("tls-sni-01", self._call("dvsni")) - self.assertEqual("http-01,tls-sni-01", self._call("http-01,dvsni")) - self.assertEqual("tls-sni-01,http-01", self._call("dvsni,http-01")) - - def test_dns01(self): - self.assertEqual("dns-01", self._call("dns-01")) - self.assertEqual("http-01,dns-01", self._call("http-01,dns-01")) - self.assertEqual("dns-01,http-01", self._call("dns-01,http-01")) - if __name__ == "__main__": unittest.main() # pragma: no cover diff --git a/certbot/util.py b/certbot/util.py index e5d671ce1..e78ae664c 100644 --- a/certbot/util.py +++ b/certbot/util.py @@ -14,7 +14,6 @@ import socket import stat import subprocess import sys -import time import configargparse @@ -475,23 +474,3 @@ def get_strict_version(normalized): # strict version ending with "a" and a number designates a pre-release # pylint: disable=no-member return distutils.version.StrictVersion(normalized.replace(".dev", "a")) - - -def busy_wait(port, host="localhost"): - """Artificialy wait a fixed amount of time on a specific host and port - - :param str port: port of the connection - :param str host: hostname of the connection, "localhost" if None - - """ - while True: - time.sleep(1) - sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - try: - sock.connect((host, port)) - except socket.error: # pragma: no cover - pass - else: - break - finally: - sock.close() From b73bdcd51831f91b500dc392a724d51d53321c37 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 15:48:17 -0700 Subject: [PATCH 19/36] Use DNS01 not DNS --- certbot/tests/auth_handler_test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/tests/auth_handler_test.py b/certbot/tests/auth_handler_test.py index b37e7cd51..84c3e16fa 100644 --- a/certbot/tests/auth_handler_test.py +++ b/certbot/tests/auth_handler_test.py @@ -175,7 +175,7 @@ class GetAuthorizationsTest(unittest.TestCase): mock_poll.side_effect = self._validate_all self.mock_auth.get_chall_pref.return_value.append(challenges.HTTP01) - self.handler.pref_challs.extend((challenges.HTTP01, challenges.DNS,)) + self.handler.pref_challs.extend((challenges.HTTP01, challenges.DNS01,)) self.handler.get_authorizations(["0"]) From 26467d4233307f6999b371297cbee8e74a53badf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 15:57:03 -0700 Subject: [PATCH 20/36] update manual plugin info --- certbot/plugins/manual.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/certbot/plugins/manual.py b/certbot/plugins/manual.py index 6fe8cd597..eeefe20e5 100644 --- a/certbot/plugins/manual.py +++ b/certbot/plugins/manual.py @@ -106,12 +106,14 @@ s.serve_forever()" """ def more_info(self): # pylint: disable=missing-docstring,no-self-use return ("This plugin requires user's manual intervention in setting " - "up an HTTP server when solving http-01 challenges and thus " - "does not need to be run as a privileged process. " - "Alternatively shows instructions on how to use Python's " - "built-in HTTP server." - "When solving dns-01 challenges, it simply needs to wait for " - "the proper configuration of the domain's dns") + "up challenges to prove control of a domain and does not need " + "to be run as a privileged process. When solving " + "http-01 challenges, the user is responsible for setting up " + "an HTTP server. Alternatively, instructions are shown on how " + "to use Python's built-in HTTP server. The user is " + "responsible for configuration of a domain's DNS when solving " + "dns-01 challenges. The type of challenges used can be " + "controlled through the --preferred-challenges flag.") def get_chall_pref(self, domain): # pylint: disable=missing-docstring,no-self-use,unused-argument From b854d10795ee24cd8b3a14175616b1397093ff58 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 16:18:23 -0700 Subject: [PATCH 21/36] reduce number of ip_logging_permission checks --- certbot/plugins/manual.py | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/certbot/plugins/manual.py b/certbot/plugins/manual.py index eeefe20e5..ae263e0a3 100644 --- a/certbot/plugins/manual.py +++ b/certbot/plugins/manual.py @@ -121,6 +121,7 @@ s.serve_forever()" """ def perform(self, achalls): # pylint: disable=missing-docstring + self._get_ip_logging_permission() mapping = {"http-01": self._perform_http01_challenge, "dns-01": self._perform_dns01_challenge} responses = [] @@ -188,14 +189,12 @@ s.serve_forever()" """ if self._httpd.poll() is not None: raise errors.Error("Couldn't execute manual command") else: - message = self._get_message(achall) - uri = achall.chall.uri(achall.domain) - formated_message = message.format(validation=validation, - response=response, - uri=uri, - command=command) - - self._ip_logging_permission(formated_message) + self._notify_and_wait( + self._get_message(achall).format( + validation=validation, + response=response, + uri=achall.chall.uri(achall.domain), + command=command)) if not response.simple_verify( achall.chall, achall.domain, @@ -207,11 +206,11 @@ s.serve_forever()" """ def _perform_dns01_challenge(self, achall): response, validation = achall.response_and_validation() if not self.conf("test-mode"): - message = self._get_message(achall) - formated_message = message.format(validation=validation, - domain=achall.domain, - response=response) - self._ip_logging_permission(formated_message) + self._notify_and_wait( + self._get_message(achall).format( + validation=validation, + domain=achall.domain, + response=response)) try: verification_status = response.simple_verify( @@ -247,7 +246,7 @@ s.serve_forever()" """ sys.stdout.write(message) six.moves.input("Press ENTER to continue") - def _ip_logging_permission(self, formated_message): + def _get_ip_logging_permission(self): # pylint: disable=missing-docstring if not self.conf("public-ip-logging-ok"): if not zope.component.getUtility(interfaces.IDisplay).yesno( @@ -257,8 +256,6 @@ s.serve_forever()" """ else: self.config.namespace.manual_public_ip_logging_ok = True - self._notify_and_wait(formated_message) - def _get_message(self, achall): # pylint: disable=missing-docstring,no-self-use,unused-argument return self.MESSAGE_TEMPLATE.get(achall.chall.typ, "") From 55e86983e793e8510b013809cfdb8dd257111928 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 16:30:17 -0700 Subject: [PATCH 22/36] fix test mock after moving getUtility call --- certbot/plugins/manual_test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/certbot/plugins/manual_test.py b/certbot/plugins/manual_test.py index 2fb679a36..7626f607d 100644 --- a/certbot/plugins/manual_test.py +++ b/certbot/plugins/manual_test.py @@ -52,7 +52,9 @@ class AuthenticatorTest(unittest.TestCase): self.assertTrue(all(issubclass(pref, challenges.Challenge) for pref in self.auth.get_chall_pref("foo.com"))) - def test_perform_empty(self): + @mock.patch("certbot.plugins.manual.zope.component.getUtility") + def test_perform_empty(self, mock_interaction): + mock_interaction().yesno.return_value = True self.assertEqual([], self.auth.perform([])) @mock.patch("certbot.plugins.manual.zope.component.getUtility") From 456f3527cd6d6330c658f320d17840618d481c66 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 16:31:58 -0700 Subject: [PATCH 23/36] Only log one warning when missing dnspython --- certbot/plugins/manual.py | 10 +++++----- certbot/plugins/manual_test.py | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/certbot/plugins/manual.py b/certbot/plugins/manual.py index ae263e0a3..7ae45a8b5 100644 --- a/certbot/plugins/manual.py +++ b/certbot/plugins/manual.py @@ -217,11 +217,11 @@ s.serve_forever()" """ achall.chall, achall.domain, achall.account_key.public_key()) except acme_errors.DependencyError: - verification_status = False - logger.warning("Dns challenge requires `dnspython`") - - if not verification_status: - logger.warning("Self-verify of challenge failed.") + logger.warning("Self verification requires optional " + "dependency `dnspython` to be installed.") + else: + if not verification_status: + logger.warning("Self-verify of challenge failed.") return response diff --git a/certbot/plugins/manual_test.py b/certbot/plugins/manual_test.py index 7626f607d..e55abbeb5 100644 --- a/certbot/plugins/manual_test.py +++ b/certbot/plugins/manual_test.py @@ -90,7 +90,7 @@ class AuthenticatorTest(unittest.TestCase): with mock.patch("certbot.plugins.manual.logger") as mock_logger: self.auth.perform([self.dns01]) - self.assertEqual(2, mock_logger.warning.call_count) + self.assertEqual(1, mock_logger.warning.call_count) mock_raw_input.assert_called_once_with("Press ENTER to continue") From eb88e7a577de03cd5bc45753ab8f66d0104399ba Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 16:34:43 -0700 Subject: [PATCH 24/36] Remove standalone_supported_challenges value from manual test --- certbot/plugins/manual_test.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/certbot/plugins/manual_test.py b/certbot/plugins/manual_test.py index e55abbeb5..25107e4b4 100644 --- a/certbot/plugins/manual_test.py +++ b/certbot/plugins/manual_test.py @@ -25,8 +25,7 @@ class AuthenticatorTest(unittest.TestCase): from certbot.plugins.manual import Authenticator self.config = mock.MagicMock( http01_port=8080, manual_test_mode=False, - manual_public_ip_logging_ok=False, noninteractive_mode=True, - standalone_supported_challenges="dns-01,http-01") + manual_public_ip_logging_ok=False, noninteractive_mode=True) self.auth = Authenticator(config=self.config, name="manual") self.http01 = achallenges.KeyAuthorizationAnnotatedChallenge( From 5d8127177cbcafc72304d2d265bec82811f55e1b Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 16:48:31 -0700 Subject: [PATCH 25/36] correct challenge domain name --- certbot/plugins/manual.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/certbot/plugins/manual.py b/certbot/plugins/manual.py index 7ae45a8b5..9025dc9cb 100644 --- a/certbot/plugins/manual.py +++ b/certbot/plugins/manual.py @@ -44,7 +44,8 @@ class Authenticator(common.Plugin): MESSAGE_TEMPLATE = { "dns-01": """\ -To prove control of the domain {domain}, please deploy a DNS TXT record with the following value: +Please deploy a DNS TXT record under the name +{domain} with the following value: {validation} @@ -209,7 +210,7 @@ s.serve_forever()" """ self._notify_and_wait( self._get_message(achall).format( validation=validation, - domain=achall.domain, + domain=achall.validation_domain_name(achall.domain), response=response)) try: From 021313acd12a4449b40c496d6dbe5ebcd33117c0 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 17:08:46 -0700 Subject: [PATCH 26/36] make port flags more visible with better help --- certbot/cli.py | 5 +++-- certbot/interfaces.py | 9 ++++++--- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index c236041ce..942fc8ac2 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -790,11 +790,12 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis help=config_help("no_verify_ssl"), default=flag_default("no_verify_ssl")) helpful.add( - "testing", "--tls-sni-01-port", type=int, + ["certonly", "renew", "run"], "--tls-sni-01-port", type=int, default=flag_default("tls_sni_01_port"), help=config_help("tls_sni_01_port")) helpful.add( - "testing", "--http-01-port", type=int, dest="http01_port", + ["certonly", "renew", "run"], "--http-01-port", type=int, + dest="http01_port", default=flag_default("http01_port"), help=config_help("http01_port")) helpful.add( "testing", "--break-my-certs", action="store_true", diff --git a/certbot/interfaces.py b/certbot/interfaces.py index d4b391378..42a952f10 100644 --- a/certbot/interfaces.py +++ b/certbot/interfaces.py @@ -229,11 +229,14 @@ class IConfig(zope.interface.Interface): no_verify_ssl = zope.interface.Attribute( "Disable verification of the ACME server's certificate.") tls_sni_01_port = zope.interface.Attribute( - "Port number to perform tls-sni-01 challenge. " - "Boulder in testing mode defaults to 5001.") + "Port used during tls-sni-01 challenge. " + "This only affects the port Certbot listens on. " + "A conforming ACME server will still attempt to connect on port 443.") http01_port = zope.interface.Attribute( - "Port used in the SimpleHttp challenge.") + "Port used in the http-01 challenge." + "This only affects the port Certbot listens on. " + "A conforming ACME server will still attempt to connect on port 80.") class IInstaller(IPlugin): From a8b2880963f0165585b0e30250709c450baac184 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Mon, 29 Aug 2016 18:45:16 -0700 Subject: [PATCH 27/36] fix ip logging prompt for manual-test-mode --- certbot/plugins/manual.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/plugins/manual.py b/certbot/plugins/manual.py index 9025dc9cb..2ef49d7f4 100644 --- a/certbot/plugins/manual.py +++ b/certbot/plugins/manual.py @@ -249,7 +249,7 @@ s.serve_forever()" """ def _get_ip_logging_permission(self): # pylint: disable=missing-docstring - if not self.conf("public-ip-logging-ok"): + if not (self.conf("test-mode") or self.conf("public-ip-logging-ok")): if not zope.component.getUtility(interfaces.IDisplay).yesno( self.IP_DISCLAIMER, "Yes", "No", cli_flag="--manual-public-ip-logging-ok"): From a18a8f051d8d32bc92bc24fb6f41a30f3a89be3b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 21 Sep 2016 15:01:01 -0700 Subject: [PATCH 28/36] Improve documentation for --preferred-challenges --- certbot/cli.py | 10 ++++----- docs/using.rst | 55 ++++++++++++++++++++++++++++---------------------- 2 files changed, 36 insertions(+), 29 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 942fc8ac2..0fbf6eefb 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -794,7 +794,7 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis default=flag_default("tls_sni_01_port"), help=config_help("tls_sni_01_port")) helpful.add( - ["certonly", "renew", "run"], "--http-01-port", type=int, + ["certonly", "renew", "run", "manual"], "--http-01-port", type=int, dest="http01_port", default=flag_default("http01_port"), help=config_help("http01_port")) helpful.add( @@ -848,13 +848,13 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis help="Require that all configuration files are owned by the current " "user; only needed if your config is somewhere unsafe like /tmp/") helpful.add( - ["certonly", "renew", "run"], "--preferred-challenges", + ["manual", "certonly", "renew", "run"], "--preferred-challenges", dest="pref_challs", action=_PrefChallAction, default=[], help="A sorted, comma delimited list of the preferred challenge to " "use during authorization with the most preferred challenge " - "listed first (e.g. tls-sni-01,http-01). If none of the " - "preferred challenges can be used by the selected plugin to " - "satisfy the CA, authorization is not attempted.") + 'listed first. Eg, "dns-01" or "tls-sni-01,http-01,dns-01").' + ' Not all plugins support all challenges. See ' + 'https://certbot.eff.org/docs/using.html#plugins for details.') helpful.add( "renew", "--pre-hook", help="Command to be run in a shell before obtaining any certificates." diff --git a/docs/using.rst b/docs/using.rst index 20b6cc5c7..972c7248f 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -60,7 +60,7 @@ an alternate method fo install ``certbot``. Certbot-Auto ^^^^^^^^^^^^ -The ``certbot-auto`` wrapper script installs Certbot, obtaining some dependencies +The ``certbot-auto`` wrapper script installs Certbot, obtaining some dependencies from your web server OS and putting others in a python virtual environment. You can download and run it as follows:: @@ -77,8 +77,8 @@ download and run it as follows:: The ``certbot-auto`` command updates to the latest client release automatically. Since ``certbot-auto`` is a wrapper to ``certbot``, it accepts exactly -the same command line flags and arguments. For more information, see -`Certbot command-line options `_. +the same command line flags and arguments. For more information, see +`Certbot command-line options `_. Running with Docker ^^^^^^^^^^^^^^^^^^^ @@ -88,8 +88,8 @@ certificate. However, this mode of operation is unable to install certificates or configure your webserver, because our installer plugins cannot reach your webserver from inside the Docker container. -Most users should use the operating system packages (see instructions at -certbot.eff.org_) or, as a fallback, ``certbot-auto``. You should only +Most users should use the operating system packages (see instructions at +certbot.eff.org_) or, as a fallback, ``certbot-auto``. You should only use Docker if you are sure you know what you are doing and have a good reason to do so. @@ -113,12 +113,12 @@ to, `install Docker`_, then issue the following command: quay.io/letsencrypt/letsencrypt:latest certonly Running Certbot with the ``certonly`` command will obtain a certificate and place it in the directory -``/etc/letsencrypt/live`` on your system. Because Certonly cannot install the certificate from +``/etc/letsencrypt/live`` on your system. Because Certonly cannot install the certificate from within Docker, you must install the certificate manually according to the procedure recommended by the provider of your webserver. -For more information about the layout -of the ``/etc/letsencrypt`` directory, see :ref:`where-certs`. +For more information about the layout +of the ``/etc/letsencrypt`` directory, see :ref:`where-certs`. .. _Docker: https://docker.com .. _`install Docker`: https://docs.docker.com/userguide/ @@ -242,8 +242,8 @@ whole process is described in the :doc:`contributing`. .. _plugins: -Getting certificates -==================== +Getting certificates (and chosing plugins) +========================================== The Certbot client supports a number of different "plugins" that can be used to obtain and/or install certificates. @@ -252,34 +252,41 @@ Plugins that can obtain a cert are called "authenticators" and can be used with the "certonly" command. This will carry out the steps needed to validate that you control the domain(s) you are requesting a cert for, obtain a cert for the specified domain(s), and place it in the ``/etc/letsencrypt`` directory on your -machine - without editing any of your server's configuration files to serve the +machine - without editing any of your server's configuration files to serve the obtained certificate. If you specify multiple domains to authenticate, they will all be listed in a single certificate. To obtain multiple seperate certificates you will need to run Certbot multiple times. -Plugins that can install a cert are called "installers" and can be used with the +Plugins that can install a cert are called "installers" and can be used with the "install" command. These plugins can modify your webserver's configuration to -serve your website over HTTPS using certificates obtained by certbot. +serve your website over HTTPS using certificates obtained by certbot. Plugins that do both can be used with the "certbot run" command, which is the default when no command is specified. The "run" subcommand can also be used to specify a combination of distinct authenticator and installer plugins. -=========== ==== ==== =============================================================== -Plugin Auth Inst Notes -=========== ==== ==== =============================================================== -apache_ Y Y Automates obtaining and installing a cert with Apache 2.4 on +=========== ==== ==== =============================================================== ============================= +Plugin Auth Inst Notes Challenge types (and port) +=========== ==== ==== =============================================================== ============================= +apache_ Y Y Automates obtaining and installing a cert with Apache 2.4 on ``tls-sni-01`` (443) Debian-based distributions with ``libaugeas0`` 1.0+. -webroot_ Y N Obtains a cert by writing to the webroot directory of an +webroot_ Y N Obtains a cert by writing to the webroot directory of an ``http-01`` (80) already running webserver. -standalone_ Y N Uses a "standalone" webserver to obtain a cert. Requires - port 80 or 443 to be available. This is useful on systems +standalone_ Y N Uses a "standalone" webserver to obtain a cert. Requires ``http-01`` (80) or + port 80 or 443 to be available. This is useful on systems ``tls-sni-01`` (443) with no webserver, or when direct integration with the local webserver is not supported or not desired. -manual_ Y N Helps you obtain a cert by giving you instructions to perform - domain validation yourself. -nginx_ Y Y Very experimental and not included in certbot-auto_. -=========== ==== ==== =============================================================== +manual_ Y N Helps you obtain a cert by giving you instructions to perform ``http-01`` (80) or + domain validation yourself. ``dns-01`` (53) +nginx_ Y Y Very experimental and not included in certbot-auto_. ``tls-sni-01`` (443) +=========== ==== ==== =============================================================== ============================= + +Under the hood, plugins use one of several "Challenge Types" to prove you control a domain. +The options are ``http-01`` (which uses port 80), ``tls-sni-01`` (port 443) and ``dns-01`` +(requring configuration of a DNS server on port 53, thought that's often not +the same machine as your webserver). A few plugins support more than one +challenge type, in which case you can choose it with +``--preferred-challenges``. There are also many third-party-plugins_ available. Below we describe in more detail the circumstances in which each plugin can be used, and how to use it. From 741a57c976f53f36e38a91c252186f5098e941df Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 21 Sep 2016 15:02:03 -0700 Subject: [PATCH 29/36] Accept --preferred-challenges "dns-01, http-01" --- certbot/cli.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/certbot/cli.py b/certbot/cli.py index 0fbf6eefb..4d91a1904 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -1048,7 +1048,7 @@ class _PrefChallAction(argparse.Action): """Action class for parsing preferred challenges.""" def __call__(self, parser, namespace, pref_challs, option_string=None): - challs = pref_challs.split(",") + challs = [c.strip() for c in pref_challs.split(",")] unrecognized = ", ".join(name for name in challs if name not in challenges.Challenge.TYPES) if unrecognized: From 9219617b0c5f7628f2cfff2561ac2ad32251b3e1 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 21 Sep 2016 15:20:06 -0700 Subject: [PATCH 30/36] Nicer table formatting --- docs/using.rst | 30 +++++++++++++++++------------- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/docs/using.rst b/docs/using.rst index 972c7248f..e5c6d43bb 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -268,29 +268,33 @@ a combination of distinct authenticator and installer plugins. =========== ==== ==== =============================================================== ============================= Plugin Auth Inst Notes Challenge types (and port) =========== ==== ==== =============================================================== ============================= -apache_ Y Y Automates obtaining and installing a cert with Apache 2.4 on ``tls-sni-01`` (443) - Debian-based distributions with ``libaugeas0`` 1.0+. -webroot_ Y N Obtains a cert by writing to the webroot directory of an ``http-01`` (80) - already running webserver. -standalone_ Y N Uses a "standalone" webserver to obtain a cert. Requires ``http-01`` (80) or - port 80 or 443 to be available. This is useful on systems ``tls-sni-01`` (443) - with no webserver, or when direct integration with the local - webserver is not supported or not desired. -manual_ Y N Helps you obtain a cert by giving you instructions to perform ``http-01`` (80) or - domain validation yourself. ``dns-01`` (53) -nginx_ Y Y Very experimental and not included in certbot-auto_. ``tls-sni-01`` (443) +apache_ Y Y | Automates obtaining and installing a cert with Apache 2.4 on tls-sni-01_ (443) + | Debian-based distributions with ``libaugeas0`` 1.0+. +webroot_ Y N | Obtains a cert by writing to the webroot directory of an http-01_ (80) + | already running webserver. +standalone_ Y N | Uses a "standalone" webserver to obtain a cert. Requires http-01_ (80) or + | port 80 or 443 to be available. This is useful on systems tls-sni-01_ (443) + | with no webserver, or when direct integration with the local + | webserver is not supported or not desired. +manual_ Y N | Helps you obtain a cert by giving you instructions to perform http-01_ (80) or + | domain validation yourself. dns-01_ (53) +nginx_ Y Y | Very experimental and not included in certbot-auto_. tls-sni-01_ (443) =========== ==== ==== =============================================================== ============================= Under the hood, plugins use one of several "Challenge Types" to prove you control a domain. -The options are ``http-01`` (which uses port 80), ``tls-sni-01`` (port 443) and ``dns-01`` +The options are http-01_ (which uses port 80), tls-sni-01_ (port 443) and dns-01_ (requring configuration of a DNS server on port 53, thought that's often not the same machine as your webserver). A few plugins support more than one -challenge type, in which case you can choose it with +challenge type, in which case you can choose one with ``--preferred-challenges``. There are also many third-party-plugins_ available. Below we describe in more detail the circumstances in which each plugin can be used, and how to use it. +.. _tls-sni-01: https://tools.ietf.org/html/draft-ietf-acme-acme-03#section-7.3 +.. _http-01: https://tools.ietf.org/html/draft-ietf-acme-acme-03#page-40 +.. _dns-01: https://tools.ietf.org/html/draft-ietf-acme-acme-03#section-7.4 + Apache ------ From 6c066ef10cffa58c75b60f0827f4c9231993b898 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 21 Sep 2016 15:23:42 -0700 Subject: [PATCH 31/36] Better section link --- docs/using.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/using.rst b/docs/using.rst index e5c6d43bb..8e9524634 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -292,7 +292,7 @@ There are also many third-party-plugins_ available. Below we describe in more de the circumstances in which each plugin can be used, and how to use it. .. _tls-sni-01: https://tools.ietf.org/html/draft-ietf-acme-acme-03#section-7.3 -.. _http-01: https://tools.ietf.org/html/draft-ietf-acme-acme-03#page-40 +.. _http-01: https://tools.ietf.org/html/draft-ietf-acme-acme-03#section-7.2 .. _dns-01: https://tools.ietf.org/html/draft-ietf-acme-acme-03#section-7.4 Apache From 107a3e6aa93f19de3e3b117f6fd11a8ab049caab Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 21 Sep 2016 16:17:31 -0700 Subject: [PATCH 32/36] Allow & document --preferred-challenges dns,http --- certbot/cli.py | 8 ++++++-- docs/using.rst | 13 +++++++------ 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 4d91a1904..5fbafa51e 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -852,9 +852,11 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis dest="pref_challs", action=_PrefChallAction, default=[], help="A sorted, comma delimited list of the preferred challenge to " "use during authorization with the most preferred challenge " - 'listed first. Eg, "dns-01" or "tls-sni-01,http-01,dns-01").' + 'listed first. Eg, "dns" or "tls-sni-01,http,dns").' ' Not all plugins support all challenges. See ' - 'https://certbot.eff.org/docs/using.html#plugins for details.') + 'https://certbot.eff.org/docs/using.html#plugins for details.' + ' Challenges are versioned, but if you pick "http" rather than' + ' "http-01", Certbot will select the latest version automatically.' ) helpful.add( "renew", "--pre-hook", help="Command to be run in a shell before obtaining any certificates." @@ -1048,7 +1050,9 @@ class _PrefChallAction(argparse.Action): """Action class for parsing preferred challenges.""" def __call__(self, parser, namespace, pref_challs, option_string=None): + aliases = {"dns": "dns-01", "http": "http-01", "tls-sni": "tls-sni-01"} challs = [c.strip() for c in pref_challs.split(",")] + challs = [aliases[c] if c in aliases else c for c in challs] unrecognized = ", ".join(name for name in challs if name not in challenges.Challenge.TYPES) if unrecognized: diff --git a/docs/using.rst b/docs/using.rst index 8e9524634..18dca071a 100644 --- a/docs/using.rst +++ b/docs/using.rst @@ -281,16 +281,17 @@ manual_ Y N | Helps you obtain a cert by giving you instructions to pe nginx_ Y Y | Very experimental and not included in certbot-auto_. tls-sni-01_ (443) =========== ==== ==== =============================================================== ============================= -Under the hood, plugins use one of several "Challenge Types" to prove you control a domain. -The options are http-01_ (which uses port 80), tls-sni-01_ (port 443) and dns-01_ -(requring configuration of a DNS server on port 53, thought that's often not -the same machine as your webserver). A few plugins support more than one -challenge type, in which case you can choose one with -``--preferred-challenges``. +Under the hood, plugins use one of several ACME protocol "Challenges_" to +prove you control a domain. The options are http-01_ (which uses port 80), +tls-sni-01_ (port 443) and dns-01_ (requring configuration of a DNS server on +port 53, thought that's often not the same machine as your webserver). A few +plugins support more than one challenge type, in which case you can choose one +with ``--preferred-challenges``. There are also many third-party-plugins_ available. Below we describe in more detail the circumstances in which each plugin can be used, and how to use it. +.. _Challenges: https://tools.ietf.org/html/draft-ietf-acme-acme-03#section-7 .. _tls-sni-01: https://tools.ietf.org/html/draft-ietf-acme-acme-03#section-7.3 .. _http-01: https://tools.ietf.org/html/draft-ietf-acme-acme-03#section-7.2 .. _dns-01: https://tools.ietf.org/html/draft-ietf-acme-acme-03#section-7.4 From f6c605cd15344e579a18e2aebd8f192ea4c5b43b Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 21 Sep 2016 16:43:35 -0700 Subject: [PATCH 33/36] Add tests for the --preferred-challenges cli parser --- certbot/tests/cli_test.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/certbot/tests/cli_test.py b/certbot/tests/cli_test.py index 2c6e32705..fdfb9dcc8 100644 --- a/certbot/tests/cli_test.py +++ b/certbot/tests/cli_test.py @@ -423,6 +423,18 @@ class CLITest(unittest.TestCase): # pylint: disable=too-many-public-methods namespace = parse(long_args) self.assertEqual(namespace.domains, ['example.com', 'another.net']) + def test_preferred_challenges(self): + from acme.challenges import HTTP01, TLSSNI01, DNS01 + parse = self._get_argument_parser() + + short_args = ['--preferred-challenges', 'http, tls-sni-01, dns'] + namespace = parse(short_args) + + self.assertEqual(namespace.pref_challs, [HTTP01, TLSSNI01, DNS01]) + + short_args = ['--preferred-challenges', 'jumping-over-the-moon'] + self.assertRaises(argparse.ArgumentTypeError, parse, short_args) + def test_server_flag(self): parse = self._get_argument_parser() namespace = parse('--server example.com'.split()) From e28017a1108d674ce834d254bf20765c5096ca86 Mon Sep 17 00:00:00 2001 From: Peter Eckersley Date: Wed, 21 Sep 2016 16:45:42 -0700 Subject: [PATCH 34/36] Lint --- certbot/cli.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 5fbafa51e..81f7819e2 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -855,8 +855,8 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis 'listed first. Eg, "dns" or "tls-sni-01,http,dns").' ' Not all plugins support all challenges. See ' 'https://certbot.eff.org/docs/using.html#plugins for details.' - ' Challenges are versioned, but if you pick "http" rather than' - ' "http-01", Certbot will select the latest version automatically.' ) + ' ACME Challenges are versioned, but if you pick "http" rather than' + ' "http-01", Certbot will select the latest version automatically.') helpful.add( "renew", "--pre-hook", help="Command to be run in a shell before obtaining any certificates." From 0165269c052965091b12edddc240b66fae87aedf Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 21 Sep 2016 17:37:47 -0700 Subject: [PATCH 35/36] denit cli.py --- certbot/cli.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 81f7819e2..78ac04b06 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -848,15 +848,16 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis help="Require that all configuration files are owned by the current " "user; only needed if your config is somewhere unsafe like /tmp/") helpful.add( - ["manual", "certonly", "renew", "run"], "--preferred-challenges", - dest="pref_challs", action=_PrefChallAction, default=[], - help="A sorted, comma delimited list of the preferred challenge to " - "use during authorization with the most preferred challenge " - 'listed first. Eg, "dns" or "tls-sni-01,http,dns").' - ' Not all plugins support all challenges. See ' + ["manual", "standalone", "certonly", "renew", "run"], + "--preferred-challenges", dest="pref_challs", + action=_PrefChallAction, default=[], + help='A sorted, comma delimited list of the preferred challenge to ' + 'use during authorization with the most preferred challenge ' + 'listed first (Eg, "dns" or "tls-sni-01,http,dns").' + 'Not all plugins support all challenges. See ' 'https://certbot.eff.org/docs/using.html#plugins for details.' - ' ACME Challenges are versioned, but if you pick "http" rather than' - ' "http-01", Certbot will select the latest version automatically.') + 'ACME Challenges are versioned, but if you pick "http" rather than' + '"http-01", Certbot will select the latest version automatically.') helpful.add( "renew", "--pre-hook", help="Command to be run in a shell before obtaining any certificates." From 74ac006f14959aebdf77a0b81182ebdc85042531 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Wed, 21 Sep 2016 17:48:17 -0700 Subject: [PATCH 36/36] fix spacing --- certbot/cli.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/certbot/cli.py b/certbot/cli.py index 78ac04b06..83697d8da 100644 --- a/certbot/cli.py +++ b/certbot/cli.py @@ -853,11 +853,12 @@ def prepare_and_parse_args(plugins, args, detect_defaults=False): # pylint: dis action=_PrefChallAction, default=[], help='A sorted, comma delimited list of the preferred challenge to ' 'use during authorization with the most preferred challenge ' - 'listed first (Eg, "dns" or "tls-sni-01,http,dns").' + 'listed first (Eg, "dns" or "tls-sni-01,http,dns"). ' 'Not all plugins support all challenges. See ' - 'https://certbot.eff.org/docs/using.html#plugins for details.' - 'ACME Challenges are versioned, but if you pick "http" rather than' - '"http-01", Certbot will select the latest version automatically.') + 'https://certbot.eff.org/docs/using.html#plugins for details. ' + 'ACME Challenges are versioned, but if you pick "http" rather ' + 'than "http-01", Certbot will select the latest version ' + 'automatically.') helpful.add( "renew", "--pre-hook", help="Command to be run in a shell before obtaining any certificates."