diff --git a/letsencrypt/client/auth_handler.py b/letsencrypt/client/auth_handler.py index 05f3722cf..136265aa6 100644 --- a/letsencrypt/client/auth_handler.py +++ b/letsencrypt/client/auth_handler.py @@ -315,24 +315,23 @@ class AuthHandler(object): # pylint: disable=too-many-instance-attributes def gen_challenge_path(challs, preferences, combinations): """Generate a plan to get authority over the identity. - .. todo:: Make sure that the challenges are feasible... - Example: Do you have the recovery key? + .. todo:: This can be possibly be rewritten to use resolved_combinations. - :param list challs: A list of challenges + :param tuple challs: A tuple of challenges (:class:`letsencrypt.acme.challenges.Challenge`) from :class:`letsencrypt.acme.messages.Challenge` server message to be fulfilled by the client in order to prove possession of the identifier. :param list preferences: List of challenge preferences for domain - (:class:`letsencrypt.acme.challenges.Challege` subclasses) + (:class:`letsencrypt.acme.challenges.Challenge` subclasses) - :param list combinations: A collection of sets of challenges from + :param tuple combinations: A collection of sets of challenges from :class:`letsencrypt.acme.messages.Challenge`, each of which would be sufficient to prove possession of the identifier. - :returns: List of indices from ``challenges``. - :rtype: list + :returns: tuple of indices from ``challenges``. + :rtype: tuple """ if combinations: @@ -349,29 +348,34 @@ def _find_smart_path(challs, preferences, combinations): """ chall_cost = {} - max_cost = 0 + max_cost = 1 for i, chall_cls in enumerate(preferences): chall_cost[chall_cls] = i max_cost += i + # max_cost is now equal to sum(indices) + 1 + best_combo = [] # Set above completing all of the available challenges - best_combo_cost = max_cost + 1 + best_combo_cost = max_cost combo_total = 0 for combo in combinations: for challenge_index in combo: combo_total += chall_cost.get(challs[ challenge_index].__class__, max_cost) + if combo_total < best_combo_cost: best_combo = combo best_combo_cost = combo_total - combo_total = 0 + + combo_total = 0 if not best_combo: - logging.fatal("Client does not support any combination of " - "challenges to satisfy ACME server") - sys.exit(22) + msg = ("Client does not support any combination of challenges that " + "will satisfy the CA.") + logging.fatal(msg) + raise errors.LetsEncryptAuthHandlerError(msg) return best_combo @@ -387,13 +391,14 @@ def _find_dumb_path(challs, preferences): assert len(preferences) == len(set(preferences)) path = [] - satisfied = set() + # This cannot be a set() because POP challenge is not currently hashable + satisfied = [] for pref_c in preferences: for i, offered_chall in enumerate(challs): if (isinstance(offered_chall, pref_c) and is_preferred(offered_chall, satisfied)): path.append(i) - satisfied.add(offered_chall) + satisfied.append(offered_chall) return path diff --git a/letsencrypt/client/tests/acme_util.py b/letsencrypt/client/tests/acme_util.py index aba839f8c..1b121e49f 100644 --- a/letsencrypt/client/tests/acme_util.py +++ b/letsencrypt/client/tests/acme_util.py @@ -27,19 +27,19 @@ POP = challenges.ProofOfPossession( alg="RS256", nonce="xD\xf9\xb9\xdbU\xed\xaa\x17\xf1y|\x81\x88\x99 ", hints=challenges.ProofOfPossession.Hints( jwk=jose.JWKRSA(key=KEY.publickey()), - cert_fingerprints=[ + cert_fingerprints=( "93416768eb85e33adc4277f4c9acd63e7418fcfe", "16d95b7b63f1972b980b14c20291f3c0d1855d95", "48b46570d9fc6358108af43ad1649484def0debf" - ], - certs=[], # TODO - subject_key_identifiers=["d0083162dcc4c8a23ecb8aecbd86120e56fd24e5"], - serial_numbers=[34234239832, 23993939911, 17], - issuers=[ + ), + certs=(), # TODO + subject_key_identifiers=("d0083162dcc4c8a23ecb8aecbd86120e56fd24e5"), + serial_numbers=(34234239832, 23993939911, 17), + issuers=( "C=US, O=SuperT LLC, CN=SuperTrustworthy Public CA", "O=LessTrustworthy CA Inc, CN=LessTrustworthy But StillSecure", - ], - authorized_for=["www.example.com", "example.net"], + ), + authorized_for=("www.example.com", "example.net"), ) ) @@ -61,6 +61,6 @@ def gen_combos(challs): else: renewal_chall.append(i) - # Gen combos for 1 of each type - return [[i, j] for i in xrange(len(dv_chall)) - for j in xrange(len(renewal_chall))] + # Gen combos for 1 of each type, lowest index first (makes testing easier) + return tuple((i, j) if i < j else (j, i) + for i in dv_chall for j in renewal_chall) diff --git a/letsencrypt/client/tests/auth_handler_test.py b/letsencrypt/client/tests/auth_handler_test.py index 478d4c0ac..6150899de 100644 --- a/letsencrypt/client/tests/auth_handler_test.py +++ b/letsencrypt/client/tests/auth_handler_test.py @@ -513,6 +513,78 @@ class PathSatisfiedTest(unittest.TestCase): self.assertFalse(self.handler._path_satisfied(dom[i])) +class GenChallengePathTest(unittest.TestCase): + """Tests for letsencrypt.client.auth_handler.gen_challenge_path. + + .. todo:: Add more tests for dumb_path... depending on what we want to do. + + """ + def setUp(self): + logging.disable(logging.fatal) + + def tearDown(self): + logging.disable(logging.NOTSET) + + @classmethod + def _call(cls, challs, preferences, combinations): + from letsencrypt.client.auth_handler import gen_challenge_path + return gen_challenge_path(challs, preferences, combinations) + + def test_common_case(self): + """Given DVSNI and SimpleHTTPS with appropriate combos.""" + challs = (acme_util.DVSNI, acme_util.SIMPLE_HTTPS) + prefs = [challenges.DVSNI] + combos = ((0,), (1,)) + + # Smart then trivial dumb path test + self.assertEqual(self._call(challs, prefs, combos), (0,)) + self.assertTrue(self._call(challs, prefs, None)) + # Rearrange order... + self.assertEqual(self._call(challs[::-1], prefs, combos), (1,)) + self.assertTrue(self._call(challs[::-1], prefs, None)) + + def test_common_case_with_continuity(self): + challs = (acme_util.RECOVERY_TOKEN, + acme_util.RECOVERY_CONTACT, + acme_util.DVSNI, + acme_util.SIMPLE_HTTPS) + prefs = [challenges.RecoveryToken, challenges.DVSNI] + combos = acme_util.gen_combos(challs) + self.assertEqual(self._call(challs, prefs, combos), (0, 2)) + + # dumb_path() trivial test + self.assertTrue(self._call(challs, prefs, None)) + + def test_full_client_server(self): + challs = (acme_util.RECOVERY_TOKEN, + acme_util.RECOVERY_CONTACT, + acme_util.POP, + acme_util.DVSNI, + acme_util.SIMPLE_HTTPS, + acme_util.DNS) + # Typical webserver client that can do everything except DNS + # Attempted to make the order realistic + prefs = [challenges.RecoveryToken, + challenges.ProofOfPossession, + challenges.SimpleHTTPS, + challenges.DVSNI, + challenges.RecoveryContact] + combos = acme_util.gen_combos(challs) + self.assertEqual(self._call(challs, prefs, combos), (0, 4)) + + # Dumb path trivial test + self.assertTrue(self._call(challs, prefs, None)) + + def test_not_supported(self): + challs = (acme_util.POP, acme_util.DVSNI) + prefs = [challenges.DVSNI] + combos = ((0, 1),) + + self.assertRaises(errors.LetsEncryptAuthHandlerError, + self._call, + challs, prefs, combos) + + class MutuallyExclusiveTest(unittest.TestCase): """Tests for letsencrypt.client.auth_handler.mutually_exclusive.""" diff --git a/tox.ini b/tox.ini index bb5ac1bb7..fe9da1865 100644 --- a/tox.ini +++ b/tox.ini @@ -19,7 +19,7 @@ setenv = basepython = python2.7 commands = pip install -e .[testing] - python setup.py nosetests --with-coverage --cover-min-percentage=86 + python setup.py nosetests --with-coverage --cover-min-percentage=87 [testenv:lint] # recent versions of pylint do not support Python 2.6 (#97, #187)