From f1eacc74615d3e3f00837dd54066dbeb1ab02226 Mon Sep 17 00:00:00 2001 From: Brad Warren Date: Fri, 26 Feb 2016 15:32:11 -0800 Subject: [PATCH] simplify code when no combos in authzr --- letsencrypt/auth_handler.py | 59 ++++++-------------- letsencrypt/constants.py | 5 -- letsencrypt/tests/acme_util.py | 11 +++- letsencrypt/tests/auth_handler_test.py | 76 +------------------------- 4 files changed, 27 insertions(+), 124 deletions(-) diff --git a/letsencrypt/auth_handler.py b/letsencrypt/auth_handler.py index f142346cc..bf37c3c92 100644 --- a/letsencrypt/auth_handler.py +++ b/letsencrypt/auth_handler.py @@ -9,7 +9,6 @@ from acme import challenges from acme import messages from letsencrypt import achallenges -from letsencrypt import constants from letsencrypt import errors from letsencrypt import error_handler from letsencrypt import interfaces @@ -396,10 +395,7 @@ def _find_smart_path(challbs, preferences, combinations): combo_total = 0 if not best_combo: - msg = ("Client does not support any combination of challenges that " - "will satisfy the CA.") - logger.fatal(msg) - raise errors.AuthorizationError(msg) + _report_no_chall_path() return best_combo @@ -408,48 +404,29 @@ def _find_dumb_path(challbs, preferences): """Find challenge path without server hints. Should be called if the combinations hint is not included by the - server. This function returns the best path that does not contain - multiple mutually exclusive challenges. + server. This function either returns a path containing all + challenges provided by the CA or raises an exception. """ - assert len(preferences) == len(set(preferences)) - path = [] - satisfied = set() - for pref_c in preferences: - for i, offered_challb in enumerate(challbs): - if (isinstance(offered_challb.chall, pref_c) and - is_preferred(offered_challb, satisfied)): - path.append(i) - satisfied.add(offered_challb) + for i, challb in enumerate(challbs): + # supported is set to True if the challenge type is supported + supported = next((True for pref_c in preferences + if isinstance(challb.chall, pref_c)), False) + if supported: + path.append(i) + else: + _report_no_chall_path() + return path -def mutually_exclusive(obj1, obj2, groups, different=False): - """Are two objects mutually exclusive?""" - for group in groups: - obj1_present = False - obj2_present = False - - for obj_cls in group: - obj1_present |= isinstance(obj1, obj_cls) - obj2_present |= isinstance(obj2, obj_cls) - - if obj1_present and obj2_present and ( - not different or not isinstance(obj1, obj2.__class__)): - return False - return True - - -def is_preferred(offered_challb, satisfied, - exclusive_groups=constants.EXCLUSIVE_CHALLENGES): - """Return whether or not the challenge is preferred in path.""" - for challb in satisfied: - if not mutually_exclusive( - offered_challb.chall, challb.chall, exclusive_groups, - different=True): - return False - return True +def _report_no_chall_path(): + """Logs and raises an error that no satisfiable chall path exists.""" + msg = ("Client with the currently selected authenticator does not support " + "any combination of challenges that will satisfy the CA.") + logger.fatal(msg) + raise errors.AuthorizationError(msg) _ACME_PREFIX = "urn:acme:error:" diff --git a/letsencrypt/constants.py b/letsencrypt/constants.py index 402f5e9a1..f8ef1e845 100644 --- a/letsencrypt/constants.py +++ b/letsencrypt/constants.py @@ -44,11 +44,6 @@ RENEWER_DEFAULTS = dict( """Defaults for renewer script.""" -EXCLUSIVE_CHALLENGES = frozenset([frozenset([ - challenges.TLSSNI01, challenges.HTTP01])]) -"""Mutually exclusive challenges.""" - - ENHANCEMENTS = ["redirect", "http-header", "ocsp-stapling", "spdy"] """List of possible :class:`letsencrypt.interfaces.IInstaller` enhancements. diff --git a/letsencrypt/tests/acme_util.py b/letsencrypt/tests/acme_util.py index 6b07b840f..71ebb471d 100644 --- a/letsencrypt/tests/acme_util.py +++ b/letsencrypt/tests/acme_util.py @@ -59,9 +59,14 @@ def gen_combos(challbs): else: cont_chall.append(i) - # 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 cont_chall) + if cont_chall: + # Gen combos for 1 of each type, lowest + # index included first (makes testing easier) + return tuple((i, j) if i < j else (j, i) + for i in dv_chall for j in cont_chall) + else: + # completing a single DV chall satisfies the CA + return tuple((i,) for i in dv_chall) def chall_to_challb(chall, status): # pylint: disable=redefined-outer-name diff --git a/letsencrypt/tests/auth_handler_test.py b/letsencrypt/tests/auth_handler_test.py index e0b7958f4..f4c09a5ba 100644 --- a/letsencrypt/tests/auth_handler_test.py +++ b/letsencrypt/tests/auth_handler_test.py @@ -299,7 +299,7 @@ class GenChallengePathTest(unittest.TestCase): def test_common_case(self): """Given TLSSNI01 and HTTP01 with appropriate combos.""" challbs = (acme_util.TLSSNI01_P, acme_util.HTTP01_P) - prefs = [challenges.TLSSNI01] + prefs = [challenges.TLSSNI01, challenges.HTTP01] combos = ((0,), (1,)) # Smart then trivial dumb path test @@ -318,9 +318,6 @@ class GenChallengePathTest(unittest.TestCase): combos = acme_util.gen_combos(challbs) self.assertEqual(self._call(challbs, prefs, combos), (0, 2)) - # dumb_path() trivial test - self.assertTrue(self._call(challbs, prefs, None)) - def test_full_cont_server(self): challbs = (acme_util.RECOVERY_CONTACT_P, acme_util.POP_P, @@ -336,9 +333,6 @@ class GenChallengePathTest(unittest.TestCase): combos = acme_util.gen_combos(challbs) self.assertEqual(self._call(challbs, prefs, combos), (1, 3)) - # Dumb path trivial test - self.assertTrue(self._call(challbs, prefs, None)) - def test_not_supported(self): challbs = (acme_util.POP_P, acme_util.TLSSNI01_P) prefs = [challenges.TLSSNI01] @@ -348,74 +342,6 @@ class GenChallengePathTest(unittest.TestCase): errors.AuthorizationError, self._call, challbs, prefs, combos) -class MutuallyExclusiveTest(unittest.TestCase): - """Tests for letsencrypt.auth_handler.mutually_exclusive.""" - - # pylint: disable=missing-docstring,too-few-public-methods - class A(object): - pass - - class B(object): - pass - - class C(object): - pass - - class D(C): - pass - - @classmethod - def _call(cls, chall1, chall2, different=False): - from letsencrypt.auth_handler import mutually_exclusive - return mutually_exclusive(chall1, chall2, groups=frozenset([ - frozenset([cls.A, cls.B]), frozenset([cls.A, cls.C]), - ]), different=different) - - def test_group_members(self): - self.assertFalse(self._call(self.A(), self.B())) - self.assertFalse(self._call(self.A(), self.C())) - - def test_cross_group(self): - self.assertTrue(self._call(self.B(), self.C())) - - def test_same_type(self): - self.assertFalse(self._call(self.A(), self.A(), different=False)) - self.assertTrue(self._call(self.A(), self.A(), different=True)) - - # in particular... - obj = self.A() - self.assertFalse(self._call(obj, obj, different=False)) - self.assertTrue(self._call(obj, obj, different=True)) - - def test_subclass(self): - self.assertFalse(self._call(self.A(), self.D())) - self.assertFalse(self._call(self.D(), self.A())) - - -class IsPreferredTest(unittest.TestCase): - """Tests for letsencrypt.auth_handler.is_preferred.""" - - @classmethod - def _call(cls, chall, satisfied): - from letsencrypt.auth_handler import is_preferred - return is_preferred(chall, satisfied, exclusive_groups=frozenset([ - frozenset([challenges.TLSSNI01, challenges.HTTP01]), - frozenset([challenges.DNS, challenges.HTTP01]), - ])) - - def test_empty_satisfied(self): - self.assertTrue(self._call(acme_util.DNS_P, frozenset())) - - def test_mutually_exclusvie(self): - self.assertFalse( - self._call( - acme_util.TLSSNI01_P, frozenset([acme_util.HTTP01_P]))) - - def test_mutually_exclusive_same_type(self): - self.assertTrue( - self._call(acme_util.TLSSNI01_P, frozenset([acme_util.TLSSNI01_P]))) - - class ReportFailedChallsTest(unittest.TestCase): """Tests for letsencrypt.auth_handler._report_failed_challs.""" # pylint: disable=protected-access