From 21efc26ed22b402e416215ed8fe8646509c51bd4 Mon Sep 17 00:00:00 2001 From: Evan Cordell Date: Tue, 8 Jan 2019 10:38:02 -0500 Subject: [PATCH] Store discharge macaroons on the verifier, and don't allow them to be used more than once. This prevents an infinite recursion on a discharge that requires itself. This is a breaking change to the python verifier API, but not to the macaroon protocol API. fixes #40 --- .../caveat_delegates/base_third_party.py | 1 - pymacaroons/caveat_delegates/third_party.py | 24 +++++++++++-------- pymacaroons/verifier.py | 22 +++++++---------- tests/functional_tests/functional_tests.py | 16 +++++++++---- tests/functional_tests/serialization_tests.py | 3 +-- 5 files changed, 36 insertions(+), 30 deletions(-) diff --git a/pymacaroons/caveat_delegates/base_third_party.py b/pymacaroons/caveat_delegates/base_third_party.py index d4c9376..b97ea88 100644 --- a/pymacaroons/caveat_delegates/base_third_party.py +++ b/pymacaroons/caveat_delegates/base_third_party.py @@ -31,7 +31,6 @@ def verify_third_party_caveat(self, caveat, root, macaroon, - discharge_macaroons, signature): pass diff --git a/pymacaroons/caveat_delegates/third_party.py b/pymacaroons/caveat_delegates/third_party.py index 7dfc552..604d4ee 100644 --- a/pymacaroons/caveat_delegates/third_party.py +++ b/pymacaroons/caveat_delegates/third_party.py @@ -54,26 +54,33 @@ def add_third_party_caveat(self, class ThirdPartyCaveatVerifierDelegate(BaseThirdPartyCaveatVerifierDelegate): - def __init__(self, *args, **kwargs): + def __init__(self, discharge_macaroons=None, *args, **kwargs): super(ThirdPartyCaveatVerifierDelegate, self).__init__(*args, **kwargs) + if discharge_macaroons: + self.discharge_macaroons = {m.identifier_bytes: m for m in discharge_macaroons} def verify_third_party_caveat(self, verifier, caveat, root, macaroon, - discharge_macaroons, signature): - caveat_macaroon = self._caveat_macaroon(caveat, discharge_macaroons) + caveat_macaroon = self._caveat_macaroon(caveat) caveat_key = self._extract_caveat_key(signature, caveat) + discharge = self.discharge_macaroons[caveat.caveat_id_bytes] + del self.discharge_macaroons[caveat.caveat_id_bytes] + caveat_met = verifier.verify_discharge( root, caveat_macaroon, caveat_key, - discharge_macaroons=discharge_macaroons ) - + + # if the caveat wasn't successfully discharged, restore the discharge macaroon to the available set + if not caveat_met: + self.discharge_macaroons[caveat.caveat_id_bytes] = discharge + return caveat_met def update_signature(self, signature, caveat): @@ -85,11 +92,8 @@ def update_signature(self, signature, caveat): ) ) - def _caveat_macaroon(self, caveat, discharge_macaroons): - # TODO: index discharge macaroons by identifier - caveat_macaroon = next( - (m for m in discharge_macaroons - if m.identifier_bytes == caveat.caveat_id_bytes), None) + def _caveat_macaroon(self, caveat): + caveat_macaroon = self.discharge_macaroons.get(caveat.caveat_id_bytes, None) if not caveat_macaroon: raise MacaroonUnmetCaveatException( diff --git a/pymacaroons/verifier.py b/pymacaroons/verifier.py index b3475ff..0b47b30 100644 --- a/pymacaroons/verifier.py +++ b/pymacaroons/verifier.py @@ -22,7 +22,7 @@ class Verifier(object): - def __init__(self): + def __init__(self, discharge_macaroons=None): self.predicates = [] self.callbacks = [self.verify_exact] self.calculated_signature = None @@ -30,7 +30,7 @@ def __init__(self): FirstPartyCaveatVerifierDelegate() ) self.third_party_caveat_verifier_delegate = ( - ThirdPartyCaveatVerifierDelegate() + ThirdPartyCaveatVerifierDelegate(discharge_macaroons=discharge_macaroons) ) def satisfy_exact(self, predicate): @@ -46,22 +46,21 @@ def satisfy_general(self, func): def verify_exact(self, predicate): return predicate in self.predicates - def verify(self, macaroon, key, discharge_macaroons=None): + def verify(self, macaroon, key): key = generate_derived_key(convert_to_bytes(key)) return self.verify_discharge( macaroon, macaroon, - key, - discharge_macaroons + key, ) - def verify_discharge(self, root, discharge, key, discharge_macaroons=None): + def verify_discharge(self, root, discharge, key): calculated_signature = hmac_digest( key, discharge.identifier_bytes ) calculated_signature = self._verify_caveats( - root, discharge, discharge_macaroons, calculated_signature + root, discharge, calculated_signature ) if root != discharge: @@ -78,18 +77,16 @@ def verify_discharge(self, root, discharge, key, discharge_macaroons=None): return True - def _verify_caveats(self, root, macaroon, discharge_macaroons, signature): + def _verify_caveats(self, root, macaroon, signature): for caveat in macaroon.caveats: if self._caveat_met(root, caveat, macaroon, - discharge_macaroons, signature): signature = self._update_signature(caveat, signature) return signature - def _caveat_met(self, root, caveat, macaroon, - discharge_macaroons, signature): + def _caveat_met(self, root, caveat, macaroon, signature): if caveat.first_party(): return ( self @@ -101,8 +98,7 @@ def _caveat_met(self, root, caveat, macaroon, self .third_party_caveat_verifier_delegate .verify_third_party_caveat( - self, caveat, root, macaroon, - discharge_macaroons, signature, + self, caveat, root, macaroon, signature, ) ) diff --git a/tests/functional_tests/functional_tests.py b/tests/functional_tests/functional_tests.py index aa7a819..3807760 100644 --- a/tests/functional_tests/functional_tests.py +++ b/tests/functional_tests/functional_tests.py @@ -398,14 +398,13 @@ def test_verify_third_party_caveats(self): discharge.add_first_party_caveat('time < 2015-01-01T00:00') protected = m.prepare_for_request(discharge) - v = Verifier() + v = Verifier(discharge_macaroons=[protected]) v.satisfy_exact('account = 3735928559') v.satisfy_exact('time < 2015-01-01T00:00') verified = v.verify( m, 'this is a different super-secret key; \ -never use the same secret twice', - discharge_macaroons=[protected] +never use the same secret twice' ) assert_true(verified) @@ -425,7 +424,7 @@ def test_verify_third_party_caveats_multi_level(self): discharge1 = root.prepare_for_request(discharge1) discharge2 = root.prepare_for_request(discharge2) - verified = Verifier().verify(root, "root-key", [discharge1, discharge2]) + verified = Verifier(discharge_macaroons=[discharge1, discharge2]).verify(root, "root-key") assert_true(verified) @patch('nacl.secret.random') @@ -452,3 +451,12 @@ def test_inspect(self, rand_nonce): 'vid AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA68NYajhiFuHnKGSNcVhkAwgbs0VZ0yK2o+q0Aq9+bONkXw7ky7HAuhCLO9hhaMMc\n' 'cl http://auth.mybank/\n' 'signature 7a9289bfbb92d725f748bbcb4f3e04e56b7021513ebeed8411bfba10a16a662e')) + + @raises(MacaroonUnmetCaveatException) + def test_mutual_discharge(self): + m1 = Macaroon(location="", identifier="root-id", key="root-key") + m1.add_third_party_caveat("bob", "bob-caveat-root-key", "bob-is-great") + m2 = Macaroon(location="bob", identifier="bob-is-great", key="bob-caveat-root-key") + m2.add_third_party_caveat("charlie", "bob-caveat-root-key", "bob-is-great") + m2 = m1.prepare_for_request(m2) + Verifier(discharge_macaroons=[m2]).verify(m1, "root-key") diff --git a/tests/functional_tests/serialization_tests.py b/tests/functional_tests/serialization_tests.py index 6bff9f9..4ddd749 100644 --- a/tests/functional_tests/serialization_tests.py +++ b/tests/functional_tests/serialization_tests.py @@ -69,11 +69,10 @@ def assert_macaroon(m, discharge, version): assert_equal(m.location, 'my location') assert_equal(m.version, version) assert_equal(m.identifier_bytes, b'my identifier') - v = Verifier() + v = Verifier(discharge_macaroons=[discharge]) v.satisfy_exact('fp caveat') verified = v.verify( m, "my secret key", - discharge_macaroons=[discharge], ) assert_true(verified)