diff --git a/CHANGES.md b/CHANGES.md index e403a65d1..6fdd31daa 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,17 @@ +Synapse 0.34.1.1 (2019-01-11) +============================= + +This release fixes CVE-2019-5885 and is recommended for all users of Synapse 0.34.1. + +This release is compatible with Python 2.7 and 3.5+. Python 3.7 is fully supported. + +Bugfixes +-------- + +- Fix spontaneous logout on upgrade + ([\#4374](https://github.com/matrix-org/synapse/issues/4374)) + + Synapse 0.34.1 (2019-01-09) =========================== diff --git a/debian/changelog b/debian/changelog index 921f7021d..09bb0001e 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,3 +1,9 @@ +matrix-synapse-py3 (0.34.1.1) stable; urgency=high + + * New synapse release 0.34.1.1 + + -- Synapse Packaging team Thu, 10 Jan 2019 15:04:52 +0000 + matrix-synapse-py3 (0.34.1+1) stable; urgency=medium * Remove 'Breaks: matrix-synapse-ldap3'. (matrix-synapse-py3 includes diff --git a/synapse/__init__.py b/synapse/__init__.py index 2935238fa..d6a191ccc 100644 --- a/synapse/__init__.py +++ b/synapse/__init__.py @@ -27,4 +27,4 @@ try: except ImportError: pass -__version__ = "0.34.1" +__version__ = "0.34.1.1" diff --git a/synapse/api/auth.py b/synapse/api/auth.py index b8a9af715..ba1019b9b 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -300,20 +300,28 @@ class Auth(object): Raises: AuthError if no user by that token exists or the token is invalid. """ + + if rights == "access": + # first look in the database + r = yield self._look_up_user_by_access_token(token) + if r: + defer.returnValue(r) + + # otherwise it needs to be a valid macaroon try: user_id, guest = self._parse_and_validate_macaroon(token, rights) - except _InvalidMacaroonException: - # doesn't look like a macaroon: treat it as an opaque token which - # must be in the database. - # TODO: it would be nice to get rid of this, but apparently some - # people use access tokens which aren't macaroons - r = yield self._look_up_user_by_access_token(token) - defer.returnValue(r) - - try: user = UserID.from_string(user_id) - if guest: + if rights == "access": + if not guest: + # non-guest access tokens must be in the database + logger.warning("Unrecognised access token - not in store.") + raise AuthError( + self.TOKEN_NOT_FOUND_HTTP_STATUS, + "Unrecognised access token.", + errcode=Codes.UNKNOWN_TOKEN, + ) + # Guest access tokens are not stored in the database (there can # only be one access token per guest, anyway). # @@ -354,31 +362,15 @@ class Auth(object): "device_id": None, } else: - # This codepath exists for several reasons: - # * so that we can actually return a token ID, which is used - # in some parts of the schema (where we probably ought to - # use device IDs instead) - # * the only way we currently have to invalidate an - # access_token is by removing it from the database, so we - # have to check here that it is still in the db - # * some attributes (notably device_id) aren't stored in the - # macaroon. They probably should be. - # TODO: build the dictionary from the macaroon once the - # above are fixed - ret = yield self._look_up_user_by_access_token(token) - if ret["user"] != user: - logger.error( - "Macaroon user (%s) != DB user (%s)", - user, - ret["user"] - ) - raise AuthError( - self.TOKEN_NOT_FOUND_HTTP_STATUS, - "User mismatch in macaroon", - errcode=Codes.UNKNOWN_TOKEN - ) + raise RuntimeError("Unknown rights setting %s", rights) defer.returnValue(ret) - except (pymacaroons.exceptions.MacaroonException, TypeError, ValueError): + except ( + _InvalidMacaroonException, + pymacaroons.exceptions.MacaroonException, + TypeError, + ValueError, + ) as e: + logger.warning("Invalid macaroon in auth: %s %s", type(e), e) raise AuthError( self.TOKEN_NOT_FOUND_HTTP_STATUS, "Invalid macaroon passed.", errcode=Codes.UNKNOWN_TOKEN @@ -508,11 +500,8 @@ class Auth(object): def _look_up_user_by_access_token(self, token): ret = yield self.store.get_user_by_access_token(token) if not ret: - logger.warn("Unrecognised access token - not in store.") - raise AuthError( - self.TOKEN_NOT_FOUND_HTTP_STATUS, "Unrecognised access token.", - errcode=Codes.UNKNOWN_TOKEN - ) + defer.returnValue(None) + # we use ret.get() below because *lots* of unit tests stub out # get_user_by_access_token in a way where it only returns a couple of # the fields. diff --git a/synapse/config/key.py b/synapse/config/key.py index 53f48fe2d..3b11f0cfa 100644 --- a/synapse/config/key.py +++ b/synapse/config/key.py @@ -57,8 +57,8 @@ class KeyConfig(Config): # Unfortunately, there are people out there that don't have this # set. Lets just be "nice" and derive one from their secret key. logger.warn("Config is missing missing macaroon_secret_key") - seed = self.signing_key[0].seed - self.macaroon_secret_key = hashlib.sha256(seed) + seed = bytes(self.signing_key[0]) + self.macaroon_secret_key = hashlib.sha256(seed).digest() self.expire_access_token = config.get("expire_access_token", False) diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py index 69dc40428..d77f20e87 100644 --- a/tests/api/test_auth.py +++ b/tests/api/test_auth.py @@ -194,8 +194,6 @@ class AuthTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_user_from_macaroon(self): - # TODO(danielwh): Remove this mock when we remove the - # get_user_by_access_token fallback. self.store.get_user_by_access_token = Mock( return_value={"name": "@baldrick:matrix.org", "device_id": "device"} ) @@ -220,6 +218,7 @@ class AuthTestCase(unittest.TestCase): @defer.inlineCallbacks def test_get_guest_user_from_macaroon(self): self.store.get_user_by_id = Mock(return_value={"is_guest": True}) + self.store.get_user_by_access_token = Mock(return_value=None) user_id = "@baldrick:matrix.org" macaroon = pymacaroons.Macaroon( @@ -240,158 +239,6 @@ class AuthTestCase(unittest.TestCase): self.assertTrue(is_guest) self.store.get_user_by_id.assert_called_with(user_id) - @defer.inlineCallbacks - def test_get_user_from_macaroon_user_db_mismatch(self): - self.store.get_user_by_access_token = Mock( - return_value={"name": "@percy:matrix.org"} - ) - - user = "@baldrick:matrix.org" - macaroon = pymacaroons.Macaroon( - location=self.hs.config.server_name, - identifier="key", - key=self.hs.config.macaroon_secret_key, - ) - macaroon.add_first_party_caveat("gen = 1") - macaroon.add_first_party_caveat("type = access") - macaroon.add_first_party_caveat("user_id = %s" % (user,)) - with self.assertRaises(AuthError) as cm: - yield self.auth.get_user_by_access_token(macaroon.serialize()) - self.assertEqual(401, cm.exception.code) - self.assertIn("User mismatch", cm.exception.msg) - - @defer.inlineCallbacks - def test_get_user_from_macaroon_missing_caveat(self): - # TODO(danielwh): Remove this mock when we remove the - # get_user_by_access_token fallback. - self.store.get_user_by_access_token = Mock( - return_value={"name": "@baldrick:matrix.org"} - ) - - macaroon = pymacaroons.Macaroon( - location=self.hs.config.server_name, - identifier="key", - key=self.hs.config.macaroon_secret_key, - ) - macaroon.add_first_party_caveat("gen = 1") - macaroon.add_first_party_caveat("type = access") - - with self.assertRaises(AuthError) as cm: - yield self.auth.get_user_by_access_token(macaroon.serialize()) - self.assertEqual(401, cm.exception.code) - self.assertIn("No user caveat", cm.exception.msg) - - @defer.inlineCallbacks - def test_get_user_from_macaroon_wrong_key(self): - # TODO(danielwh): Remove this mock when we remove the - # get_user_by_access_token fallback. - self.store.get_user_by_access_token = Mock( - return_value={"name": "@baldrick:matrix.org"} - ) - - user = "@baldrick:matrix.org" - macaroon = pymacaroons.Macaroon( - location=self.hs.config.server_name, - identifier="key", - key=self.hs.config.macaroon_secret_key + "wrong", - ) - macaroon.add_first_party_caveat("gen = 1") - macaroon.add_first_party_caveat("type = access") - macaroon.add_first_party_caveat("user_id = %s" % (user,)) - - with self.assertRaises(AuthError) as cm: - yield self.auth.get_user_by_access_token(macaroon.serialize()) - self.assertEqual(401, cm.exception.code) - self.assertIn("Invalid macaroon", cm.exception.msg) - - @defer.inlineCallbacks - def test_get_user_from_macaroon_unknown_caveat(self): - # TODO(danielwh): Remove this mock when we remove the - # get_user_by_access_token fallback. - self.store.get_user_by_access_token = Mock( - return_value={"name": "@baldrick:matrix.org"} - ) - - user = "@baldrick:matrix.org" - macaroon = pymacaroons.Macaroon( - location=self.hs.config.server_name, - identifier="key", - key=self.hs.config.macaroon_secret_key, - ) - macaroon.add_first_party_caveat("gen = 1") - macaroon.add_first_party_caveat("type = access") - macaroon.add_first_party_caveat("user_id = %s" % (user,)) - macaroon.add_first_party_caveat("cunning > fox") - - with self.assertRaises(AuthError) as cm: - yield self.auth.get_user_by_access_token(macaroon.serialize()) - self.assertEqual(401, cm.exception.code) - self.assertIn("Invalid macaroon", cm.exception.msg) - - @defer.inlineCallbacks - def test_get_user_from_macaroon_expired(self): - # TODO(danielwh): Remove this mock when we remove the - # get_user_by_access_token fallback. - self.store.get_user_by_access_token = Mock( - return_value={"name": "@baldrick:matrix.org"} - ) - - self.store.get_user_by_access_token = Mock( - return_value={"name": "@baldrick:matrix.org"} - ) - - user = "@baldrick:matrix.org" - macaroon = pymacaroons.Macaroon( - location=self.hs.config.server_name, - identifier="key", - key=self.hs.config.macaroon_secret_key, - ) - macaroon.add_first_party_caveat("gen = 1") - macaroon.add_first_party_caveat("type = access") - macaroon.add_first_party_caveat("user_id = %s" % (user,)) - macaroon.add_first_party_caveat("time < -2000") # ms - - self.hs.clock.now = 5000 # seconds - self.hs.config.expire_access_token = True - # yield self.auth.get_user_by_access_token(macaroon.serialize()) - # TODO(daniel): Turn on the check that we validate expiration, when we - # validate expiration (and remove the above line, which will start - # throwing). - with self.assertRaises(AuthError) as cm: - yield self.auth.get_user_by_access_token(macaroon.serialize()) - self.assertEqual(401, cm.exception.code) - self.assertIn("Invalid macaroon", cm.exception.msg) - - @defer.inlineCallbacks - def test_get_user_from_macaroon_with_valid_duration(self): - # TODO(danielwh): Remove this mock when we remove the - # get_user_by_access_token fallback. - self.store.get_user_by_access_token = Mock( - return_value={"name": "@baldrick:matrix.org"} - ) - - self.store.get_user_by_access_token = Mock( - return_value={"name": "@baldrick:matrix.org"} - ) - - user_id = "@baldrick:matrix.org" - macaroon = pymacaroons.Macaroon( - location=self.hs.config.server_name, - identifier="key", - key=self.hs.config.macaroon_secret_key, - ) - macaroon.add_first_party_caveat("gen = 1") - macaroon.add_first_party_caveat("type = access") - macaroon.add_first_party_caveat("user_id = %s" % (user_id,)) - macaroon.add_first_party_caveat("time < 900000000") # ms - - self.hs.clock.now = 5000 # seconds - self.hs.config.expire_access_token = True - - user_info = yield self.auth.get_user_by_access_token(macaroon.serialize()) - user = user_info["user"] - self.assertEqual(UserID.from_string(user_id), user) - @defer.inlineCallbacks def test_cannot_use_regular_token_as_guest(self): USER_ID = "@percy:matrix.org"