From 18660251b81b158ad6fea6efa3cafe198e1e727b Mon Sep 17 00:00:00 2001 From: jrconlin Date: Wed, 16 Mar 2016 15:56:18 -0700 Subject: [PATCH] bug: decode and process crypto-key header correctly The Crypto-Key header has a different format than what was first understood. A valid Crypto-Key header may look like `keyid="foo"; key1="data",key2="data"`. This patch specialized Crypto-Key parsing to a class. Closes #410 --- autopush/endpoint.py | 36 ++++++--------------- autopush/settings.py | 21 ++++++++++--- autopush/tests/test_endpoint.py | 56 ++++++++------------------------- autopush/utils.py | 43 ------------------------- 4 files changed, 39 insertions(+), 117 deletions(-) diff --git a/autopush/endpoint.py b/autopush/endpoint.py index 501fafd6..ecd81096 100644 --- a/autopush/endpoint.py +++ b/autopush/endpoint.py @@ -47,11 +47,11 @@ ) from autopush.exceptions import InvalidTokenException from autopush.router.interface import RouterException +from autopush.crypto_key import (CryptoKey, CryptoKeyException) from autopush.utils import ( generate_hash, validate_uaid, ErrorLogger, - parse_header, extract_jwt, ) @@ -311,7 +311,7 @@ def _invalid_auth(self, fail): log.msg("Invalid bearer token: " + message, **self._client_info) raise VapidAuthException("Invalid bearer token: " + message) - def _process_auth(self, result, keys): + def _process_auth(self, result): """Process the optional VAPID auth token. VAPID requires two headers to be present; @@ -324,22 +324,15 @@ def _process_auth(self, result, keys): # No auth present, so it's not a VAPID call. if not authorization: return result - if not keys: - raise VapidAuthException("Missing Crypto-Key") - if not isinstance(keys, dict): - raise VapidAuthException("Invalid Crypto-Key") - crypto_key = keys.get('p256ecdsa') - if not crypto_key: - raise VapidAuthException("Invalid bearer token: " - "improperly specified crypto-key") + public_key = result.get("public_key") try: (auth_type, token) = authorization.split(' ', 1) except ValueError: raise VapidAuthException("Invalid Authorization header") # if it's a bearer token containing what may be a JWT if auth_type.lower() == AUTH_SCHEME and '.' in token: - d = deferToThread(extract_jwt, token, crypto_key) - d.addCallback(self._store_auth, crypto_key, token, result) + d = deferToThread(extract_jwt, token, public_key) + d.addCallback(self._store_auth, public_key, token, result) d.addErrback(self._invalid_auth) return d # otherwise, it's not, so ignore the VAPID data. @@ -412,20 +405,13 @@ def put(self, api_ver="v0", token=None): """ api_ver = api_ver or "v0" self.start_time = time.time() - public_key = None - keys = {} - crypto_key = self.request.headers.get('crypto-key') - if crypto_key: - header_info = parse_header(crypto_key) - keys = header_info[-1] - if isinstance(keys, dict): - public_key = keys.get('p256ecdsa') + crypto_key_header = self.request.headers.get('crypto-key') d = deferToThread(self.ap_settings.parse_endpoint, token, api_ver, - public_key) - d.addCallback(self._process_auth, keys) + crypto_key_header) + d.addCallback(self._process_auth) d.addCallback(self._token_valid) d.addErrback(self._auth_err) d.addErrback(self._token_err) @@ -437,10 +423,8 @@ def put(self, api_ver="v0", token=None): ############################################################# def _token_valid(self, result): """Called after the token is decrypted successfully""" - if len(result) != 2: - raise InvalidTokenException("Wrong subscription token components") - - self.uaid, self.chid = result + self.uaid = result.get("uaid") + self.chid = result.get("chid") d = deferToThread(self.ap_settings.router.get_uaid, self.uaid) d.addCallback(self._uaid_lookup_results) d.addErrback(self._uaid_not_found_err) diff --git a/autopush/settings.py b/autopush/settings.py index 1a5a8bba..669cfe7e 100644 --- a/autopush/settings.py +++ b/autopush/settings.py @@ -39,6 +39,7 @@ ) from autopush.utils import canonical_url, resolve_ip from autopush.senderids import SENDERID_EXPRY, DEFAULT_BUCKET +from autopush.crypto_key import (CryptoKey, CryptoKeyException) VALID_V0_TOKEN = re.compile(r'[0-9A-Za-z-]{32,36}:[0-9A-Za-z-]{32,36}') @@ -280,26 +281,34 @@ def make_endpoint(self, uaid, chid, key=None): return root + 'v2/' + self.fernet.encrypt(base + sha256(key).digest()) - def parse_endpoint(self, token, version="v0", public_key=None): + def parse_endpoint(self, token, version="v0", ckey_header=None): """Parse an endpoint into component elements of UAID, CHID and optional key hash if v2 :param token: The obscured subscription data. :param version: This is the API version of the token. - :param public_key: the public key (from Encryption-Key: p256ecdsa=) + :param ckey_header: the Crypto-Key header bearing the public key + (from Crypto-Key: p256ecdsa=) :raises ValueError: In the case of a malformed endpoint. - :returns: a tuple containing the (UAID, CHID) + :returns: a dict containing (uaid=UAID, chid=CHID, public_key=KEY) """ token = self.fernet.decrypt(token.encode('utf8')) + public_key = None + if ckey_header: + try: + public_key = CryptoKey(ckey_header).get_label('p256ecdsa') + except CryptoKeyException: + raise InvalidTokenException("Invalid key data") if version == 'v0': if not VALID_V0_TOKEN.match(token): raise InvalidTokenException("Corrupted push token") - return tuple(token.split(':')) + items = token.split(':') + return dict(uaid=items[0], chid=items[1], public_key=public_key) if version == 'v1' and len(token) != 32: raise InvalidTokenException("Corrupted push token") if version == 'v2': @@ -310,4 +319,6 @@ def parse_endpoint(self, token, version="v0", public_key=None): if not constant_time.bytes_eq(sha256(public_key).digest(), token[32:]): raise InvalidTokenException("Key mismatch") - return (token[:16].encode('hex'), token[16:32].encode('hex')) + return dict(uaid=token[:16].encode('hex'), + chid=token[16:32].encode('hex'), + public_key=public_key) diff --git a/autopush/tests/test_endpoint.py b/autopush/tests/test_endpoint.py index 81eda332..e3c454ad 100644 --- a/autopush/tests/test_endpoint.py +++ b/autopush/tests/test_endpoint.py @@ -39,6 +39,7 @@ from autopush.router.interface import IRouter, RouterResponse from autopush.senderids import SenderIDs from autopush.utils import (generate_hash, decipher_public_key) +from autopush.crypto_key import (CryptoKeyException) mock_dynamodb2 = mock_dynamodb2() dummy_uaid = str(uuid.UUID("abad1dea00000000aabbccdd00000000")) @@ -231,12 +232,6 @@ def _check_error(self, code, errno, error): eq_(d.get("errno"), errno) eq_(d.get("error"), error) - def test_parse_header(self): - result = utils.parse_header('fee; fie="foe"; fum=""foobar="five""";') - eq_(result[0], 'fee') - eq_(result[1], {'fum': 'foobar="five"', 'fie': 'foe'}) - eq_([], utils.parse_header("")) - def test_uaid_lookup_results(self): fresult = dict(router_type="test") frouter = Mock(spec=Router) @@ -590,12 +585,7 @@ def handle_finish(result): self.endpoint.version, self.endpoint.data = 789, None - exc = self.assertRaises(InvalidTokenException, - self.endpoint._token_valid, - ['invalid']) - eq_(exc.message, "Wrong subscription token components") - - self.endpoint._token_valid(['123', dummy_chid]) + self.endpoint._token_valid(dict(uaid='123', chid=dummy_chid)) return self.finish_deferred def test_put_default_router(self): @@ -692,7 +682,7 @@ def test_put_invalid_vapid_crypto_header(self): def handle_finish(result): self.assertTrue(result) - self.endpoint.set_status.assert_called_with(401) + self.endpoint.set_status.assert_called_with(400) self.finish_deferred.addCallback(handle_finish) self.endpoint.put(None, dummy_uaid) @@ -746,29 +736,6 @@ def handle_finish(result): self.endpoint.put(None, dummy_uaid) return self.finish_deferred - def test_put_missing_vapid_crypto_header(self): - self.request_mock.headers["encryption"] = "ignored" - self.request_mock.headers["content-encoding"] = 'text' - self.request_mock.headers["authorization"] = "some auth" - self.request_mock.body = b' ' - self.fernet_mock.decrypt.return_value = dummy_token - self.router_mock.get_uaid.return_value = dict( - router_type="webpush", - router_data=dict(), - ) - self.wp_router_mock.route_notification.return_value = RouterResponse( - status_code=200, - router_data={}, - ) - - def handle_finish(result): - self.assertTrue(result) - self.endpoint.set_status.assert_called_with(401) - - self.finish_deferred.addCallback(handle_finish) - self.endpoint.put(None, dummy_uaid) - return self.finish_deferred - def test_post_webpush_with_headers_in_response(self): self.fernet_mock.decrypt.return_value = dummy_token self.endpoint.set_header = Mock() @@ -1262,12 +1229,13 @@ def test_parse_endpoint(self): chid_dec = chid_strip.decode('hex') v1_valid = uaid_dec + chid_dec pub_key = uuid.uuid4().hex + crypto_key = "p256ecdsa=" + pub_key v2_valid = sha256(pub_key).digest() v2_invalid = sha256(uuid.uuid4().hex).digest() # v0 good self.fernet_mock.decrypt.return_value = v0_valid tokens = self.settings.parse_endpoint('/valid') - eq_(tokens, (dummy_uaid, dummy_chid)) + eq_(tokens, dict(uaid=dummy_uaid, chid=dummy_chid, public_key=None)) # v0 bad self.fernet_mock.decrypt.return_value = v1_valid @@ -1284,22 +1252,24 @@ def test_parse_endpoint(self): self.fernet_mock.decrypt.return_value = v1_valid tokens = self.settings.parse_endpoint('valid', 'v1') - eq_(tokens, (uaid_strip, chid_strip)) + eq_(tokens, dict(uaid=uaid_strip, chid=chid_strip, public_key=None)) self.fernet_mock.decrypt.return_value = v1_valid + v2_valid - tokens = self.settings.parse_endpoint('valid', 'v2', pub_key) - eq_(tokens, (uaid_strip, chid_strip)) + tokens = self.settings.parse_endpoint('valid', 'v2', crypto_key) + eq_(tokens, + dict(uaid=uaid_strip, chid=chid_strip, public_key=pub_key)) self.fernet_mock.decrypt.return_value = v1_valid + "invalid" exc = self.assertRaises(InvalidTokenException, self.settings.parse_endpoint, - 'invalid', 'v2', pub_key) + 'invalid', 'v2', crypto_key) eq_(exc.message, "Corrupted push token") self.fernet_mock.decrypt.return_value = v1_valid + v2_valid exc = self.assertRaises(InvalidTokenException, self.settings.parse_endpoint, - 'invalid', 'v2', pub_key[:30]) + 'invalid', 'v2', + "p256ecdsa="+pub_key[:30]) eq_(exc.message, "Key mismatch") self.fernet_mock.decrypt.return_value = v1_valid + v2_invalid @@ -1311,7 +1281,7 @@ def test_parse_endpoint(self): self.fernet_mock.decrypt.return_value = v1_valid + v2_invalid exc = self.assertRaises(InvalidTokenException, self.settings.parse_endpoint, - 'invalid', 'v2', pub_key) + 'invalid', 'v2', crypto_key) eq_(exc.message, "Key mismatch") def test_make_endpoint(self): diff --git a/autopush/utils.py b/autopush/utils.py index d8d0b26d..2f03597f 100644 --- a/autopush/utils.py +++ b/autopush/utils.py @@ -58,49 +58,6 @@ def generate_hash(key, payload): return h.hexdigest() -def parse_header(header, major=";", minor="="): - """Convert a multi-component header line (e.g. "a;b=c;d=e;...") to - a list. - - For example, if the header content line is - - `"a;c=1;b;d=2+3=5"` - - and presuming default values for major and minor, then the - response would be: - - `['a', 'b', {'c': '1', 'd': '2+3=5'}]` - - items defined with values will always appear as a dictionary at the - end of the list. If no items are assigned values, then no dictionary - is appended. - - :param header: Header content line to parse. - :param major: Major item separator. - :param minor: Minor item separator. - - """ - vals = dict() - items = [] - if not header: - return items - for v in map(lambda x: x.strip().split(minor, 1), - header.split(major)): - try: - val = v[1] - # Trim quotes equally off of start and end - # because ""this is "quoted""" is a thing. - while val[0] == val[-1] == '"': - val = val[1:-1] - vals[v[0].lower()] = val - except IndexError: - if len(v[0]): - items.append(v[0].strip('"')) - if vals: - items.append(vals) - return items - - def fix_padding(string): """ Some JWT fields may strip the end padding from base64 strings """ if len(string) % 4: