From 53ace904b2b8e2444bc518b2498947c869c8c13b Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 5 Dec 2017 01:29:25 +0000 Subject: total WIP skeleton for /room_keys API --- synapse/rest/client/v2_alpha/room_keys.py | 56 +++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 synapse/rest/client/v2_alpha/room_keys.py (limited to 'synapse/rest') diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py new file mode 100644 index 0000000000..9b93001919 --- /dev/null +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -0,0 +1,56 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 New Vector Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import logging + +from twisted.internet import defer + +from synapse.api.errors import SynapseError +from synapse.http.servlet import ( + RestServlet, parse_json_object_from_request, parse_integer +) +from synapse.http.servlet import parse_string +from synapse.types import StreamToken +from ._base import client_v2_patterns + +logger = logging.getLogger(__name__) + + +class RoomKeysUploadServlet(RestServlet): + PATTERNS = client_v2_patterns("/room_keys/keys(/(?P[^/]+))?(/(?P[^/]+))?$") + + def __init__(self, hs): + """ + Args: + hs (synapse.server.HomeServer): server + """ + super(RoomKeysUploadServlet, self).__init__() + self.auth = hs.get_auth() + self.e2e_room_keys_handler = hs.get_e2e_room_keys_handler() + + @defer.inlineCallbacks + def on_POST(self, request, room_id, session_id): + requester = yield self.auth.get_user_by_req(request, allow_guest=True) + user_id = requester.user.to_string() + body = parse_json_object_from_request(request) + + result = yield self.e2e_room_keys_handler.upload_room_keys( + user_id, version, body + ) + defer.returnValue((200, result)) + + +def register_servlets(hs, http_server): + RoomKeysUploadServlet(hs).register(http_server) -- cgit 1.4.1 From 0bc4627a731d0edd437905a5b07e85421c7553d8 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 5 Dec 2017 17:54:48 +0000 Subject: interim WIP checkin; doesn't build yet --- synapse/handlers/e2e_room_keys.py | 46 +++++++++++++++++++++---------- synapse/rest/client/v2_alpha/room_keys.py | 37 ++++++++++++++++++++++--- synapse/storage/e2e_room_keys.py | 20 ++++++++++++++ 3 files changed, 84 insertions(+), 19 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index 78c838a829..15e3beb5ed 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -20,8 +20,7 @@ from canonicaljson import encode_canonical_json from twisted.internet import defer from synapse.api.errors import SynapseError, CodeMessageException -from synapse.types import get_domain_from_id -from synapse.util.logcontext import preserve_fn, make_deferred_yieldable +from synapse.util.async import Linearizer from synapse.util.retryutils import NotRetryingDestination logger = logging.getLogger(__name__) @@ -30,31 +29,48 @@ logger = logging.getLogger(__name__) class E2eRoomKeysHandler(object): def __init__(self, hs): self.store = hs.get_datastore() + self._upload_linearizer = async.Linearizer("upload_room_keys_lock") @defer.inlineCallbacks def get_room_keys(self, user_id, version, room_id, session_id): results = yield self.store.get_e2e_room_keys(user_id, version, room_id, session_id) defer.returnValue(results) + @defer.inlineCallbacks + def delete_room_keys(self, user_id, version, room_id, session_id): + yield self.store.delete_e2e_room_keys(user_id, version, room_id, session_id) + @defer.inlineCallbacks def upload_room_keys(self, user_id, version, room_keys): # TODO: Validate the JSON to make sure it has the right keys. - # go through the room_keys - for room_id in room_keys['rooms']: - for session_id in room_keys['rooms'][room_id]['sessions']: - session = room_keys['rooms'][room_id]['sessions'][session_id] - - # get a lock + # XXX: perhaps we should use a finer grained lock here? + with (yield self._upload_linearizer.queue(user_id): - # get the room_key for this particular row - yield self.store.get_e2e_room_key() + # go through the room_keys + for room_id in room_keys['rooms']: + for session_id in room_keys['rooms'][room_id]['sessions']: + room_key = room_keys['rooms'][room_id]['sessions'][session_id] - # check whether we merge or not - if() + # get the room_key for this particular row + current_room_key = yield self.store.get_e2e_room_key( + user_id, version, room_id, session_id + ) - # if so, we set it - yield self.store.set_e2e_room_key() + # check whether we merge or not. spelling it out with if/elifs rather than + # lots of booleans for legibility. + replace = False + if current_room_key: + if room_key['is_verified'] and not current_room_key['is_verified']: + replace = True + elif room_key['first_message_index'] < current_room_key['first_message_index']: + replace = True + elif room_key['forwarded_count'] < room_key['forwarded_count']: + replace = True - # release the lock + # if so, we set the new room_key + if replace: + yield self.store.set_e2e_room_key( + user_id, version, room_id, session_id, room_key + ) diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 9b93001919..7291018a48 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -28,7 +28,7 @@ from ._base import client_v2_patterns logger = logging.getLogger(__name__) -class RoomKeysUploadServlet(RestServlet): +class RoomKeysServlet(RestServlet): PATTERNS = client_v2_patterns("/room_keys/keys(/(?P[^/]+))?(/(?P[^/]+))?$") def __init__(self, hs): @@ -41,16 +41,45 @@ class RoomKeysUploadServlet(RestServlet): self.e2e_room_keys_handler = hs.get_e2e_room_keys_handler() @defer.inlineCallbacks - def on_POST(self, request, room_id, session_id): - requester = yield self.auth.get_user_by_req(request, allow_guest=True) + def on_PUT(self, request, room_id, session_id): + requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() body = parse_json_object_from_request(request) + version = request.args.get("version", None) + + if session_id: + body = { "sessions": { session_id : body } } + + if room_id: + body = { "rooms": { room_id : body } } result = yield self.e2e_room_keys_handler.upload_room_keys( user_id, version, body ) defer.returnValue((200, result)) + @defer.inlineCallbacks + def on_GET(self, request, room_id, session_id): + requester = yield self.auth.get_user_by_req(request, allow_guest=False) + user_id = requester.user.to_string() + version = request.args.get("version", None) + + room_keys = yield self.e2e_room_keys_handler.get_room_keys( + user_id, version, room_id, session_id + ) + defer.returnValue((200, room_keys)) + + @defer.inlineCallbacks + def on_DELETE(self, request, room_id, session_id): + requester = yield self.auth.get_user_by_req(request, allow_guest=False) + user_id = requester.user.to_string() + version = request.args.get("version", None) + + yield self.e2e_room_keys_handler.delete_room_keys( + user_id, version, room_id, session_id + ) + defer.returnValue((200, {})) + def register_servlets(hs, http_server): - RoomKeysUploadServlet(hs).register(http_server) + RoomKeysServlet(hs).register(http_server) diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index 9f6d47e1b6..903dc083f8 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + from twisted.internet import defer from synapse.util.caches.descriptors import cached @@ -77,6 +78,9 @@ class EndToEndRoomKeyStore(SQLBaseStore): ) + # XXX: this isn't currently used and isn't tested anywhere + # it could be used in future for bulk-uploading new versions of room_keys + # for a user or something though. def set_e2e_room_keys(self, user_id, version, room_keys): def _set_e2e_room_keys_txn(txn): @@ -131,3 +135,19 @@ class EndToEndRoomKeyStore(SQLBaseStore): sessions = {} sessions['rooms'][roomId]['sessions'][session_id] = row for row in rows; defer.returnValue(sessions); + + @defer.inlineCallbacks + def delete_e2e_room_keys(self, user_id, version, room_id, session_id): + + keyvalues={ + "user_id": user_id, + "version": version, + } + if room_id: keyvalues['room_id'] = room_id + if session_id: keyvalues['session_id'] = session_id + + yield self._simple_delete( + table="e2e_room_keys", + keyvalues=keyvalues, + desc="delete_e2e_room_keys", + ) -- cgit 1.4.1 From 6b8c07abc293bd222051e769550753bc0fd6f667 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 5 Dec 2017 21:44:25 +0000 Subject: make it work and fix pep8 --- synapse/handlers/e2e_room_keys.py | 69 +++++++++------ synapse/rest/__init__.py | 2 + synapse/rest/client/v2_alpha/room_keys.py | 33 ++++--- synapse/server.py | 5 ++ synapse/storage/__init__.py | 2 + synapse/storage/e2e_room_keys.py | 103 +++++++++++++--------- synapse/storage/schema/delta/46/e2e_room_keys.sql | 2 +- 7 files changed, 134 insertions(+), 82 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index 15e3beb5ed..93f4ad5194 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -13,15 +13,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -import ujson as json import logging -from canonicaljson import encode_canonical_json from twisted.internet import defer -from synapse.api.errors import SynapseError, CodeMessageException +from synapse.api.errors import StoreError from synapse.util.async import Linearizer -from synapse.util.retryutils import NotRetryingDestination logger = logging.getLogger(__name__) @@ -29,11 +26,13 @@ logger = logging.getLogger(__name__) class E2eRoomKeysHandler(object): def __init__(self, hs): self.store = hs.get_datastore() - self._upload_linearizer = async.Linearizer("upload_room_keys_lock") + self._upload_linearizer = Linearizer("upload_room_keys_lock") @defer.inlineCallbacks def get_room_keys(self, user_id, version, room_id, session_id): - results = yield self.store.get_e2e_room_keys(user_id, version, room_id, session_id) + results = yield self.store.get_e2e_room_keys( + user_id, version, room_id, session_id + ) defer.returnValue(results) @defer.inlineCallbacks @@ -46,31 +45,49 @@ class E2eRoomKeysHandler(object): # TODO: Validate the JSON to make sure it has the right keys. # XXX: perhaps we should use a finer grained lock here? - with (yield self._upload_linearizer.queue(user_id): + with (yield self._upload_linearizer.queue(user_id)): # go through the room_keys for room_id in room_keys['rooms']: for session_id in room_keys['rooms'][room_id]['sessions']: room_key = room_keys['rooms'][room_id]['sessions'][session_id] - # get the room_key for this particular row - current_room_key = yield self.store.get_e2e_room_key( - user_id, version, room_id, session_id + yield self._upload_room_key( + user_id, version, room_id, session_id, room_key ) - # check whether we merge or not. spelling it out with if/elifs rather than - # lots of booleans for legibility. - replace = False - if current_room_key: - if room_key['is_verified'] and not current_room_key['is_verified']: - replace = True - elif room_key['first_message_index'] < current_room_key['first_message_index']: - replace = True - elif room_key['forwarded_count'] < room_key['forwarded_count']: - replace = True - - # if so, we set the new room_key - if replace: - yield self.store.set_e2e_room_key( - user_id, version, room_id, session_id, room_key - ) + @defer.inlineCallbacks + def _upload_room_key(self, user_id, version, room_id, session_id, room_key): + # get the room_key for this particular row + current_room_key = None + try: + current_room_key = yield self.store.get_e2e_room_key( + user_id, version, room_id, session_id + ) + except StoreError as e: + if e.code == 404: + pass + else: + raise + + # check whether we merge or not. spelling it out with if/elifs rather + # than lots of booleans for legibility. + upsert = True + if current_room_key: + if room_key['is_verified'] and not current_room_key['is_verified']: + pass + elif ( + room_key['first_message_index'] < + current_room_key['first_message_index'] + ): + pass + elif room_key['forwarded_count'] < room_key['forwarded_count']: + pass + else: + upsert = False + + # if so, we set the new room_key + if upsert: + yield self.store.set_e2e_room_key( + user_id, version, room_id, session_id, room_key + ) diff --git a/synapse/rest/__init__.py b/synapse/rest/__init__.py index 3418f06fd6..4856822a5d 100644 --- a/synapse/rest/__init__.py +++ b/synapse/rest/__init__.py @@ -46,6 +46,7 @@ from synapse.rest.client.v2_alpha import ( receipts, register, report_event, + room_keys, sendtodevice, sync, tags, @@ -102,6 +103,7 @@ class ClientRestResource(JsonResource): auth.register_servlets(hs, client_resource) receipts.register_servlets(hs, client_resource) read_marker.register_servlets(hs, client_resource) + room_keys.register_servlets(hs, client_resource) keys.register_servlets(hs, client_resource) tokenrefresh.register_servlets(hs, client_resource) tags.register_servlets(hs, client_resource) diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 7291018a48..010aed98f9 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -17,26 +17,25 @@ import logging from twisted.internet import defer -from synapse.api.errors import SynapseError from synapse.http.servlet import ( - RestServlet, parse_json_object_from_request, parse_integer + RestServlet, parse_json_object_from_request ) -from synapse.http.servlet import parse_string -from synapse.types import StreamToken from ._base import client_v2_patterns logger = logging.getLogger(__name__) class RoomKeysServlet(RestServlet): - PATTERNS = client_v2_patterns("/room_keys/keys(/(?P[^/]+))?(/(?P[^/]+))?$") + PATTERNS = client_v2_patterns( + "/room_keys/keys(/(?P[^/]+))?(/(?P[^/]+))?$" + ) def __init__(self, hs): """ Args: hs (synapse.server.HomeServer): server """ - super(RoomKeysUploadServlet, self).__init__() + super(RoomKeysServlet, self).__init__() self.auth = hs.get_auth() self.e2e_room_keys_handler = hs.get_e2e_room_keys_handler() @@ -45,24 +44,32 @@ class RoomKeysServlet(RestServlet): requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() body = parse_json_object_from_request(request) - version = request.args.get("version", None) + version = request.args.get("version")[0] if session_id: - body = { "sessions": { session_id : body } } + body = { + "sessions": { + session_id: body + } + } if room_id: - body = { "rooms": { room_id : body } } + body = { + "rooms": { + room_id: body + } + } - result = yield self.e2e_room_keys_handler.upload_room_keys( + yield self.e2e_room_keys_handler.upload_room_keys( user_id, version, body ) - defer.returnValue((200, result)) + defer.returnValue((200, {})) @defer.inlineCallbacks def on_GET(self, request, room_id, session_id): requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() - version = request.args.get("version", None) + version = request.args.get("version")[0] room_keys = yield self.e2e_room_keys_handler.get_room_keys( user_id, version, room_id, session_id @@ -73,7 +80,7 @@ class RoomKeysServlet(RestServlet): def on_DELETE(self, request, room_id, session_id): requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() - version = request.args.get("version", None) + version = request.args.get("version")[0] yield self.e2e_room_keys_handler.delete_room_keys( user_id, version, room_id, session_id diff --git a/synapse/server.py b/synapse/server.py index 140be9ebe8..706cb1361f 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -49,6 +49,7 @@ from synapse.handlers.deactivate_account import DeactivateAccountHandler from synapse.handlers.device import DeviceHandler from synapse.handlers.devicemessage import DeviceMessageHandler from synapse.handlers.e2e_keys import E2eKeysHandler +from synapse.handlers.e2e_room_keys import E2eRoomKeysHandler from synapse.handlers.events import EventHandler, EventStreamHandler from synapse.handlers.groups_local import GroupsLocalHandler from synapse.handlers.initial_sync import InitialSyncHandler @@ -127,6 +128,7 @@ class HomeServer(object): 'auth_handler', 'device_handler', 'e2e_keys_handler', + 'e2e_room_keys_handler', 'event_handler', 'event_stream_handler', 'initial_sync_handler', @@ -288,6 +290,9 @@ class HomeServer(object): def build_e2e_keys_handler(self): return E2eKeysHandler(self) + def build_e2e_room_keys_handler(self): + return E2eRoomKeysHandler(self) + def build_application_service_api(self): return ApplicationServiceApi(self) diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index 134e4a80f1..69cb28268a 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -30,6 +30,7 @@ from .appservice import ApplicationServiceStore, ApplicationServiceTransactionSt from .client_ips import ClientIpStore from .deviceinbox import DeviceInboxStore from .directory import DirectoryStore +from .e2e_room_keys import EndToEndRoomKeyStore from .end_to_end_keys import EndToEndKeyStore from .engines import PostgresEngine from .event_federation import EventFederationStore @@ -76,6 +77,7 @@ class DataStore(RoomMemberStore, RoomStore, ApplicationServiceTransactionStore, ReceiptsStore, EndToEndKeyStore, + EndToEndRoomKeyStore, SearchStore, TagsStore, AccountDataStore, diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index 903dc083f8..5982710bd5 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -15,11 +15,6 @@ from twisted.internet import defer -from synapse.util.caches.descriptors import cached - -from canonicaljson import encode_canonical_json -import ujson as json - from ._base import SQLBaseStore @@ -45,29 +40,27 @@ class EndToEndRoomKeyStore(SQLBaseStore): desc="get_e2e_room_key", ) - defer.returnValue(row); + defer.returnValue(row) def set_e2e_room_key(self, user_id, version, room_id, session_id, room_key): def _set_e2e_room_key_txn(txn): - self._simple_upsert( + self._simple_upsert_txn( txn, table="e2e_room_keys", keyvalues={ "user_id": user_id, "room_id": room_id, - "session_id": session_id, - } - values=[ - { - "version": version, - "first_message_index": room_key['first_message_index'], - "forwarded_count": room_key['forwarded_count'], - "is_verified": room_key['is_verified'], - "session_data": room_key['session_data'], - } - ], + "session_id": session_id, + }, + values={ + "version": version, + "first_message_index": room_key['first_message_index'], + "forwarded_count": room_key['forwarded_count'], + "is_verified": room_key['is_verified'], + "session_data": room_key['session_data'], + }, lock=False, ) @@ -77,7 +70,6 @@ class EndToEndRoomKeyStore(SQLBaseStore): "set_e2e_room_key", _set_e2e_room_key_txn ) - # XXX: this isn't currently used and isn't tested anywhere # it could be used in future for bulk-uploading new versions of room_keys # for a user or something though. @@ -85,23 +77,27 @@ class EndToEndRoomKeyStore(SQLBaseStore): def _set_e2e_room_keys_txn(txn): + values = [] + for room_id in room_keys['rooms']: + for session_id in room_keys['rooms'][room_id]['sessions']: + session = room_keys['rooms'][room_id]['sessions'][session_id] + values.append( + { + "user_id": user_id, + "room_id": room_id, + "session_id": session_id, + "version": version, + "first_message_index": session['first_message_index'], + "forwarded_count": session['forwarded_count'], + "is_verified": session['is_verified'], + "session_data": session['session_data'], + } + ) + self._simple_insert_many_txn( txn, table="e2e_room_keys", - values=[ - { - "user_id": user_id, - "room_id": room_id, - "session_id": session_id, - "version": version, - "first_message_index": room_keys['rooms'][room_id]['sessions'][session_id]['first_message_index'], - "forwarded_count": room_keys['rooms'][room_id]['sessions'][session_id]['forwarded_count'], - "is_verified": room_keys['rooms'][room_id]['sessions'][session_id]['is_verified'], - "session_data": room_keys['rooms'][room_id]['sessions'][session_id]['session_data'], - } - for session_id in room_keys['rooms'][room_id]['sessions'] - for room_id in room_keys['rooms'] - ] + values=values ) return True @@ -113,17 +109,22 @@ class EndToEndRoomKeyStore(SQLBaseStore): @defer.inlineCallbacks def get_e2e_room_keys(self, user_id, version, room_id, session_id): - keyvalues={ + keyvalues = { "user_id": user_id, "version": version, } - if room_id: keyvalues['room_id'] = room_id - if session_id: keyvalues['session_id'] = session_id + if room_id: + keyvalues['room_id'] = room_id + if session_id: + keyvalues['session_id'] = session_id rows = yield self._simple_select_list( table="e2e_room_keys", keyvalues=keyvalues, retcols=( + "user_id", + "room_id", + "session_id", "first_message_index", "forwarded_count", "is_verified", @@ -132,19 +133,37 @@ class EndToEndRoomKeyStore(SQLBaseStore): desc="get_e2e_room_keys", ) - sessions = {} - sessions['rooms'][roomId]['sessions'][session_id] = row for row in rows; - defer.returnValue(sessions); + # perlesque autovivification from https://stackoverflow.com/a/19829714/6764493 + class AutoVivification(dict): + def __getitem__(self, item): + try: + return dict.__getitem__(self, item) + except KeyError: + value = self[item] = type(self)() + return value + + sessions = AutoVivification() + for row in rows: + sessions['rooms'][row['room_id']]['sessions'][row['session_id']] = { + "first_message_index": row["first_message_index"], + "forwarded_count": row["forwarded_count"], + "is_verified": row["is_verified"], + "session_data": row["session_data"], + } + + defer.returnValue(sessions) @defer.inlineCallbacks def delete_e2e_room_keys(self, user_id, version, room_id, session_id): - keyvalues={ + keyvalues = { "user_id": user_id, "version": version, } - if room_id: keyvalues['room_id'] = room_id - if session_id: keyvalues['session_id'] = session_id + if room_id: + keyvalues['room_id'] = room_id + if session_id: + keyvalues['session_id'] = session_id yield self._simple_delete( table="e2e_room_keys", diff --git a/synapse/storage/schema/delta/46/e2e_room_keys.sql b/synapse/storage/schema/delta/46/e2e_room_keys.sql index 51b826e8b3..6b344c5ad7 100644 --- a/synapse/storage/schema/delta/46/e2e_room_keys.sql +++ b/synapse/storage/schema/delta/46/e2e_room_keys.sql @@ -29,7 +29,7 @@ CREATE UNIQUE INDEX e2e_room_keys_user_idx ON e2e_room_keys(user_id); CREATE UNIQUE INDEX e2e_room_keys_room_idx ON e2e_room_keys(room_id); CREATE UNIQUE INDEX e2e_room_keys_session_idx ON e2e_room_keys(session_id); --- the versioning metadata about versions of users' encrypted e2e session backups +-- the metadata for each generation of encrypted e2e session backups CREATE TABLE e2e_room_key_versions ( user_id TEXT NOT NULL, version INT NOT NULL, -- cgit 1.4.1 From cf1e2000f623afea8f3afb58e4a7659288c45777 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Tue, 5 Dec 2017 23:06:43 +0000 Subject: document the API --- synapse/rest/client/v2_alpha/room_keys.py | 133 ++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) (limited to 'synapse/rest') diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 010aed98f9..be82eccb2b 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -41,6 +41,79 @@ class RoomKeysServlet(RestServlet): @defer.inlineCallbacks def on_PUT(self, request, room_id, session_id): + """ + Uploads one or more encrypted E2E room keys for backup purposes. + room_id: the ID of the room the keys are for (optional) + session_id: the ID for the E2E room keys for the room (optional) + version: the version of the user's backup which this data is for. + the version must already have been created via the /change_secret API. + + Each session has: + * first_message_index: a numeric index indicating the oldest message + encrypted by this session. + * forwarded_count: how many times the uploading client claims this key + has been shared (forwarded) + * is_verified: whether the client that uploaded the keys claims they + were sent by a device which they've verified + * session_data: base64-encrypted data describing the session. + + Returns 200 OK on success with body {} + + The API is designed to be otherwise agnostic to the room_key encryption + algorithm being used. Sessions are merged with existing ones in the + backup using the heuristics: + * is_verified sessions always win over unverified sessions + * older first_message_index always win over newer sessions + * lower forwarded_count always wins over higher forwarded_count + + We trust the clients not to lie and corrupt their own backups. + + POST /room_keys/keys/!abc:matrix.org/c0ff33?version=1 HTTP/1.1 + Content-Type: application/json + + { + "first_message_index": 1, + "forwarded_count": 1, + "is_verified": false, + "session_data": "SSBBTSBBIEZJU0gK" + } + + Or... + + POST /room_keys/keys/!abc:matrix.org?version=1 HTTP/1.1 + Content-Type: application/json + + { + "sessions": { + "c0ff33": { + "first_message_index": 1, + "forwarded_count": 1, + "is_verified": false, + "session_data": "SSBBTSBBIEZJU0gK" + } + } + } + + Or... + + POST /room_keys/keys?version=1 HTTP/1.1 + Content-Type: application/json + + { + "rooms": { + "!abc:matrix.org": { + "sessions": { + "c0ff33": { + "first_message_index": 1, + "forwarded_count": 1, + "is_verified": false, + "session_data": "SSBBTSBBIEZJU0gK" + } + } + } + } + } + """ requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() body = parse_json_object_from_request(request) @@ -67,6 +140,57 @@ class RoomKeysServlet(RestServlet): @defer.inlineCallbacks def on_GET(self, request, room_id, session_id): + """ + Retrieves one or more encrypted E2E room keys for backup purposes. + Symmetric with the PUT version of the API. + + room_id: the ID of the room to retrieve the keys for (optional) + session_id: the ID for the E2E room keys to retrieve the keys for (optional) + version: the version of the user's backup which this data is for. + the version must already have been created via the /change_secret API. + + Returns as follows: + + GET /room_keys/keys/!abc:matrix.org/c0ff33?version=1 HTTP/1.1 + { + "first_message_index": 1, + "forwarded_count": 1, + "is_verified": false, + "session_data": "SSBBTSBBIEZJU0gK" + } + + Or... + + GET /room_keys/keys/!abc:matrix.org?version=1 HTTP/1.1 + { + "sessions": { + "c0ff33": { + "first_message_index": 1, + "forwarded_count": 1, + "is_verified": false, + "session_data": "SSBBTSBBIEZJU0gK" + } + } + } + + Or... + + GET /room_keys/keys?version=1 HTTP/1.1 + { + "rooms": { + "!abc:matrix.org": { + "sessions": { + "c0ff33": { + "first_message_index": 1, + "forwarded_count": 1, + "is_verified": false, + "session_data": "SSBBTSBBIEZJU0gK" + } + } + } + } + } + """ requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() version = request.args.get("version")[0] @@ -78,6 +202,15 @@ class RoomKeysServlet(RestServlet): @defer.inlineCallbacks def on_DELETE(self, request, room_id, session_id): + """ + Deletes one or more encrypted E2E room keys for a user for backup purposes. + + room_id: the ID of the room whose keys to delete (optional) + session_id: the ID for the E2E session to delete (optional) + version: the version of the user's backup which this data is for. + the version must already have been created via the /change_secret API. + """ + requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() version = request.args.get("version")[0] -- cgit 1.4.1 From 8ae64b270f7742abfe4cf0b8d140c81993464ea4 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 6 Dec 2017 01:02:57 +0000 Subject: implement /room_keys/version too (untested) --- synapse/api/errors.py | 25 ++++++++++ synapse/handlers/e2e_room_keys.py | 47 +++++++++++++++++-- synapse/rest/client/v2_alpha/room_keys.py | 47 +++++++++++++++++++ synapse/storage/e2e_room_keys.py | 56 +++++++++++++++++++++++ synapse/storage/schema/delta/46/e2e_room_keys.sql | 2 +- 5 files changed, 171 insertions(+), 6 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/api/errors.py b/synapse/api/errors.py index b41d595059..8c97e91ba1 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -56,6 +56,7 @@ class Codes(object): CONSENT_NOT_GIVEN = "M_CONSENT_NOT_GIVEN" CANNOT_LEAVE_SERVER_NOTICE_ROOM = "M_CANNOT_LEAVE_SERVER_NOTICE_ROOM" MAU_LIMIT_EXCEEDED = "M_MAU_LIMIT_EXCEEDED" + WRONG_ROOM_KEYS_VERSION = "M_WRONG_ROOM_KEYS_VERSION" class CodeMessageException(RuntimeError): @@ -285,6 +286,30 @@ class LimitExceededError(SynapseError): ) +class RoomKeysVersionError(SynapseError): + """A client has tried to upload to a non-current version of the room_keys store + """ + def __init__(self, code=403, msg="Wrong room_keys version", current_version=None, + errcode=Codes.WRONG_ROOM_KEYS_VERSION): + super(RoomKeysVersionError, self).__init__(code, msg, errcode) + self.current_version = current_version + + def error_dict(self): + return cs_error( + self.msg, + self.errcode, + current_version=self.current_version, + ) + + +def cs_exception(exception): + if isinstance(exception, CodeMessageException): + return exception.error_dict() + else: + logger.error("Unknown exception type: %s", type(exception)) + return {} + + def cs_error(msg, code=Codes.UNKNOWN, **kwargs): """ Utility method for constructing an error response for client-server interactions. diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index 93f4ad5194..4333ca610c 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -17,7 +17,7 @@ import logging from twisted.internet import defer -from synapse.api.errors import StoreError +from synapse.api.errors import StoreError, SynapseError, RoomKeysVersionError from synapse.util.async import Linearizer logger = logging.getLogger(__name__) @@ -30,10 +30,13 @@ class E2eRoomKeysHandler(object): @defer.inlineCallbacks def get_room_keys(self, user_id, version, room_id, session_id): - results = yield self.store.get_e2e_room_keys( - user_id, version, room_id, session_id - ) - defer.returnValue(results) + # we deliberately take the lock to get keys so that changing the version + # works atomically + with (yield self._upload_linearizer.queue(user_id)): + results = yield self.store.get_e2e_room_keys( + user_id, version, room_id, session_id + ) + defer.returnValue(results) @defer.inlineCallbacks def delete_room_keys(self, user_id, version, room_id, session_id): @@ -44,6 +47,16 @@ class E2eRoomKeysHandler(object): # TODO: Validate the JSON to make sure it has the right keys. + # Check that the version we're trying to upload is the current version + try: + version_info = yield self.get_version_info(user_id, version) + except StoreError as e: + if e.code == 404: + raise SynapseError(404, "Version '%d' not found" % (version,)) + + if version_info.version != version: + raise RoomKeysVersionError(current_version=version_info.version) + # XXX: perhaps we should use a finer grained lock here? with (yield self._upload_linearizer.queue(user_id)): @@ -91,3 +104,27 @@ class E2eRoomKeysHandler(object): yield self.store.set_e2e_room_key( user_id, version, room_id, session_id, room_key ) + + @defer.inlineCallbacks + def create_version(self, user_id, version, version_info): + + # TODO: Validate the JSON to make sure it has the right keys. + + # lock everyone out until we've switched version + with (yield self._upload_linearizer.queue(user_id)): + yield self.store.create_version( + user_id, version, version_info + ) + + @defer.inlineCallbacks + def get_version_info(self, user_id, version): + with (yield self._upload_linearizer.queue(user_id)): + results = yield self.store.get_e2e_room_key_version( + user_id, version + ) + defer.returnValue(results) + + @defer.inlineCallbacks + def delete_version(self, user_id, version): + with (yield self._upload_linearizer.queue(user_id)): + yield self.store.delete_e2e_room_key_version(user_id, version) diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index be82eccb2b..4d76e1d824 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -221,5 +221,52 @@ class RoomKeysServlet(RestServlet): defer.returnValue((200, {})) +class RoomKeysVersionServlet(RestServlet): + PATTERNS = client_v2_patterns( + "/room_keys/version(/(?P[^/]+))?$" + ) + + def __init__(self, hs): + """ + Args: + hs (synapse.server.HomeServer): server + """ + super(RoomKeysVersionServlet, self).__init__() + self.auth = hs.get_auth() + self.e2e_room_keys_handler = hs.get_e2e_room_keys_handler() + + @defer.inlineCallbacks + def on_POST(self, request, version): + requester = yield self.auth.get_user_by_req(request, allow_guest=False) + user_id = requester.user.to_string() + info = parse_json_object_from_request(request) + + new_version = yield self.e2e_room_keys_handler.create_version( + user_id, version, info + ) + defer.returnValue((200, {"version": new_version})) + + @defer.inlineCallbacks + def on_GET(self, request, version): + requester = yield self.auth.get_user_by_req(request, allow_guest=False) + user_id = requester.user.to_string() + + info = yield self.e2e_room_keys_handler.get_version_info( + user_id, version + ) + defer.returnValue((200, info)) + + @defer.inlineCallbacks + def on_DELETE(self, request, version): + requester = yield self.auth.get_user_by_req(request, allow_guest=False) + user_id = requester.user.to_string() + + yield self.e2e_room_keys_handler.delete_version( + user_id, version + ) + defer.returnValue((200, {})) + + def register_servlets(hs, http_server): RoomKeysServlet(hs).register(http_server) + RoomKeysVersionServlet(hs).register(http_server) diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index 5982710bd5..994878acf6 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -170,3 +170,59 @@ class EndToEndRoomKeyStore(SQLBaseStore): keyvalues=keyvalues, desc="delete_e2e_room_keys", ) + + @defer.inlineCallbacks + def get_e2e_room_key_version(self, user_id, version): + + row = yield self._simple_select_one( + table="e2e_room_key_versions", + keyvalues={ + "user_id": user_id, + "version": version, + }, + retcols=( + "user_id", + "version", + "algorithm", + "auth_data", + ), + desc="get_e2e_room_key_version_info", + ) + + defer.returnValue(row) + + def create_e2e_room_key_version(self, user_id, version, info): + + def _create_e2e_room_key_version_txn(txn): + + self._simple_insert_txn( + txn, + table="e2e_room_key_versions", + values={ + "user_id": user_id, + "version": version, + "algorithm": info["algorithm"], + "auth_data": info["auth_data"], + }, + lock=False, + ) + + return True + + return self.runInteraction( + "create_e2e_room_key_version_txn", _create_e2e_room_key_version_txn + ) + + @defer.inlineCallbacks + def delete_e2e_room_key_version(self, user_id, version): + + keyvalues = { + "user_id": user_id, + "version": version, + } + + yield self._simple_delete( + table="e2e_room_key_versions", + keyvalues=keyvalues, + desc="delete_e2e_room_key_version", + ) diff --git a/synapse/storage/schema/delta/46/e2e_room_keys.sql b/synapse/storage/schema/delta/46/e2e_room_keys.sql index 6b344c5ad7..463f828c66 100644 --- a/synapse/storage/schema/delta/46/e2e_room_keys.sql +++ b/synapse/storage/schema/delta/46/e2e_room_keys.sql @@ -34,7 +34,7 @@ CREATE TABLE e2e_room_key_versions ( user_id TEXT NOT NULL, version INT NOT NULL, algorithm TEXT NOT NULL, - dummy_session_data TEXT NOT NULL + auth_data TEXT NOT NULL ); CREATE UNIQUE INDEX e2e_room_key_user_idx ON e2e_room_keys(user_id); -- cgit 1.4.1 From 69e51c7ba48a84b48ab64c8c290a232d14193a18 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 6 Dec 2017 10:02:49 +0100 Subject: make /room_keys/version work --- synapse/handlers/e2e_room_keys.py | 18 +++++++++++------- synapse/rest/client/v2_alpha/room_keys.py | 9 ++++++++- synapse/storage/e2e_room_keys.py | 22 +++++++++++++++++----- synapse/storage/schema/delta/46/e2e_room_keys.sql | 4 ++-- 4 files changed, 38 insertions(+), 15 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index 4333ca610c..bd58be6558 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -48,13 +48,16 @@ class E2eRoomKeysHandler(object): # TODO: Validate the JSON to make sure it has the right keys. # Check that the version we're trying to upload is the current version + try: version_info = yield self.get_version_info(user_id, version) except StoreError as e: if e.code == 404: - raise SynapseError(404, "Version '%d' not found" % (version,)) + raise SynapseError(404, "Version '%s' not found" % (version,)) + else: + raise e - if version_info.version != version: + if version_info['version'] != version: raise RoomKeysVersionError(current_version=version_info.version) # XXX: perhaps we should use a finer grained lock here? @@ -81,7 +84,7 @@ class E2eRoomKeysHandler(object): if e.code == 404: pass else: - raise + raise e # check whether we merge or not. spelling it out with if/elifs rather # than lots of booleans for legibility. @@ -106,20 +109,21 @@ class E2eRoomKeysHandler(object): ) @defer.inlineCallbacks - def create_version(self, user_id, version, version_info): + def create_version(self, user_id, version_info): # TODO: Validate the JSON to make sure it has the right keys. # lock everyone out until we've switched version with (yield self._upload_linearizer.queue(user_id)): - yield self.store.create_version( - user_id, version, version_info + new_version = yield self.store.create_e2e_room_key_version( + user_id, version_info ) + defer.returnValue(new_version) @defer.inlineCallbacks def get_version_info(self, user_id, version): with (yield self._upload_linearizer.queue(user_id)): - results = yield self.store.get_e2e_room_key_version( + results = yield self.store.get_e2e_room_key_version_info( user_id, version ) defer.returnValue(results) diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 4d76e1d824..128b732fb1 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -17,6 +17,7 @@ import logging from twisted.internet import defer +from synapse.api.errors import SynapseError from synapse.http.servlet import ( RestServlet, parse_json_object_from_request ) @@ -237,15 +238,21 @@ class RoomKeysVersionServlet(RestServlet): @defer.inlineCallbacks def on_POST(self, request, version): + if version: + raise SynapseError(405, "Cannot POST to a specific version") + requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() info = parse_json_object_from_request(request) new_version = yield self.e2e_room_keys_handler.create_version( - user_id, version, info + user_id, info ) defer.returnValue((200, {"version": new_version})) + # we deliberately don't have a PUT /version, as these things really should + # be immutable to avoid people footgunning + @defer.inlineCallbacks def on_GET(self, request, version): requester = yield self.auth.get_user_by_req(request, allow_guest=False) diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index 994878acf6..8efca11a8c 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -172,7 +172,7 @@ class EndToEndRoomKeyStore(SQLBaseStore): ) @defer.inlineCallbacks - def get_e2e_room_key_version(self, user_id, version): + def get_e2e_room_key_version_info(self, user_id, version): row = yield self._simple_select_one( table="e2e_room_key_versions", @@ -191,23 +191,35 @@ class EndToEndRoomKeyStore(SQLBaseStore): defer.returnValue(row) - def create_e2e_room_key_version(self, user_id, version, info): + def create_e2e_room_key_version(self, user_id, info): + """Atomically creates a new version of this user's e2e_room_keys store + with the given version info. + """ def _create_e2e_room_key_version_txn(txn): + txn.execute( + "SELECT MAX(version) FROM e2e_room_key_versions WHERE user_id=?", + (user_id,) + ) + current_version = txn.fetchone()[0] + if current_version is None: + current_version = 0 + + new_version = current_version + 1 + self._simple_insert_txn( txn, table="e2e_room_key_versions", values={ "user_id": user_id, - "version": version, + "version": new_version, "algorithm": info["algorithm"], "auth_data": info["auth_data"], }, - lock=False, ) - return True + return new_version return self.runInteraction( "create_e2e_room_key_version_txn", _create_e2e_room_key_version_txn diff --git a/synapse/storage/schema/delta/46/e2e_room_keys.sql b/synapse/storage/schema/delta/46/e2e_room_keys.sql index 463f828c66..0d2a85fbe6 100644 --- a/synapse/storage/schema/delta/46/e2e_room_keys.sql +++ b/synapse/storage/schema/delta/46/e2e_room_keys.sql @@ -18,7 +18,7 @@ CREATE TABLE e2e_room_keys ( user_id TEXT NOT NULL, room_id TEXT NOT NULL, session_id TEXT NOT NULL, - version INT NOT NULL, + version TEXT NOT NULL, first_message_index INT, forwarded_count INT, is_verified BOOLEAN, @@ -32,7 +32,7 @@ CREATE UNIQUE INDEX e2e_room_keys_session_idx ON e2e_room_keys(session_id); -- the metadata for each generation of encrypted e2e session backups CREATE TABLE e2e_room_key_versions ( user_id TEXT NOT NULL, - version INT NOT NULL, + version TEXT NOT NULL, algorithm TEXT NOT NULL, auth_data TEXT NOT NULL ); -- cgit 1.4.1 From 0abb205b47158a4160ddceb317c0245d640b6e3f Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Mon, 18 Dec 2017 01:52:46 +0000 Subject: blindly incorporate PR review - needs testing & fixing --- synapse/api/errors.py | 11 ++- synapse/handlers/e2e_room_keys.py | 88 +++++++++++++++-------- synapse/rest/client/v2_alpha/room_keys.py | 2 + synapse/storage/e2e_room_keys.py | 69 ++++++++---------- synapse/storage/schema/delta/46/e2e_room_keys.sql | 8 +-- 5 files changed, 99 insertions(+), 79 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 8c97e91ba1..d37bcb4082 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -289,9 +289,14 @@ class LimitExceededError(SynapseError): class RoomKeysVersionError(SynapseError): """A client has tried to upload to a non-current version of the room_keys store """ - def __init__(self, code=403, msg="Wrong room_keys version", current_version=None, - errcode=Codes.WRONG_ROOM_KEYS_VERSION): - super(RoomKeysVersionError, self).__init__(code, msg, errcode) + def __init__(self, current_version): + """ + Args: + current_version (str): the current version of the store they should have used + """ + super(RoomKeysVersionError, self).__init__( + 403, "Wrong room_keys version", Codes.WRONG_ROOM_KEYS_VERSION + ) self.current_version = current_version def error_dict(self): diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index bd58be6558..dda31fdd24 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -24,8 +24,21 @@ logger = logging.getLogger(__name__) class E2eRoomKeysHandler(object): + """ + Implements an optional realtime backup mechanism for encrypted E2E megolm room keys. + This gives a way for users to store and recover their megolm keys if they lose all + their clients. It should also extend easily to future room key mechanisms. + The actual payload of the encrypted keys is completely opaque to the handler. + """ + def __init__(self, hs): self.store = hs.get_datastore() + + # Used to lock whenever a client is uploading key data. This prevents collisions + # between clients trying to upload the details of a new session, given all + # clients belonging to a user will receive and try to upload a new session at + # roughly the same time. Also used to lock out uploads when the key is being + # changed. self._upload_linearizer = Linearizer("upload_room_keys_lock") @defer.inlineCallbacks @@ -40,33 +53,34 @@ class E2eRoomKeysHandler(object): @defer.inlineCallbacks def delete_room_keys(self, user_id, version, room_id, session_id): - yield self.store.delete_e2e_room_keys(user_id, version, room_id, session_id) + # lock for consistency with uploading + with (yield self._upload_linearizer.queue(user_id)): + yield self.store.delete_e2e_room_keys(user_id, version, room_id, session_id) @defer.inlineCallbacks def upload_room_keys(self, user_id, version, room_keys): # TODO: Validate the JSON to make sure it has the right keys. - # Check that the version we're trying to upload is the current version - - try: - version_info = yield self.get_version_info(user_id, version) - except StoreError as e: - if e.code == 404: - raise SynapseError(404, "Version '%s' not found" % (version,)) - else: - raise e - - if version_info['version'] != version: - raise RoomKeysVersionError(current_version=version_info.version) - # XXX: perhaps we should use a finer grained lock here? with (yield self._upload_linearizer.queue(user_id)): - - # go through the room_keys - for room_id in room_keys['rooms']: - for session_id in room_keys['rooms'][room_id]['sessions']: - room_key = room_keys['rooms'][room_id]['sessions'][session_id] + # Check that the version we're trying to upload is the current version + try: + version_info = yield self.get_version_info(user_id, version) + except StoreError as e: + if e.code == 404: + raise SynapseError(404, "Version '%s' not found" % (version,)) + else: + raise e + + if version_info['version'] != version: + raise RoomKeysVersionError(current_version=version_info.version) + + # go through the room_keys. + # XXX: this should/could be done concurrently, given we're in a lock. + for room_id, room in room_keys['rooms'].iteritems(): + for session_id, session in room['sessions'].iteritems(): + room_key = session[session_id] yield self._upload_room_key( user_id, version, room_id, session_id, room_key @@ -86,10 +100,29 @@ class E2eRoomKeysHandler(object): else: raise e - # check whether we merge or not. spelling it out with if/elifs rather - # than lots of booleans for legibility. - upsert = True + if _should_replace_room_key(current_room_key, room_key): + yield self.store.set_e2e_room_key( + user_id, version, room_id, session_id, room_key + ) + + def _should_replace_room_key(current_room_key, room_key): + """ + Determine whether to replace the current_room_key in our backup for this + session (if any) with a new room_key that has been uploaded. + + Args: + current_room_key (dict): Optional, the current room_key dict if any + room_key (dict): The new room_key dict which may or may not be fit to + replace the current_room_key + + Returns: + True if current_room_key should be replaced by room_key in the backup + """ + if current_room_key: + # spelt out with if/elifs rather than nested boolean expressions + # purely for legibility. + if room_key['is_verified'] and not current_room_key['is_verified']: pass elif ( @@ -97,16 +130,11 @@ class E2eRoomKeysHandler(object): current_room_key['first_message_index'] ): pass - elif room_key['forwarded_count'] < room_key['forwarded_count']: + elif room_key['forwarded_count'] < current_room_key['forwarded_count']: pass else: - upsert = False - - # if so, we set the new room_key - if upsert: - yield self.store.set_e2e_room_key( - user_id, version, room_id, session_id, room_key - ) + return False + return True @defer.inlineCallbacks def create_version(self, user_id, version_info): diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 128b732fb1..70b7b4573f 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -68,6 +68,8 @@ class RoomKeysServlet(RestServlet): * lower forwarded_count always wins over higher forwarded_count We trust the clients not to lie and corrupt their own backups. + It also means that if your access_token is stolen, the attacker could + delete your backup. POST /room_keys/keys/!abc:matrix.org/c0ff33?version=1 HTTP/1.1 Content-Type: application/json diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index 8efca11a8c..c11417c415 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -44,30 +44,21 @@ class EndToEndRoomKeyStore(SQLBaseStore): def set_e2e_room_key(self, user_id, version, room_id, session_id, room_key): - def _set_e2e_room_key_txn(txn): - - self._simple_upsert_txn( - txn, - table="e2e_room_keys", - keyvalues={ - "user_id": user_id, - "room_id": room_id, - "session_id": session_id, - }, - values={ - "version": version, - "first_message_index": room_key['first_message_index'], - "forwarded_count": room_key['forwarded_count'], - "is_verified": room_key['is_verified'], - "session_data": room_key['session_data'], - }, - lock=False, - ) - - return True - - return self.runInteraction( - "set_e2e_room_key", _set_e2e_room_key_txn + yield self._simple_upsert( + table="e2e_room_keys", + keyvalues={ + "user_id": user_id, + "room_id": room_id, + "session_id": session_id, + }, + values={ + "version": version, + "first_message_index": room_key['first_message_index'], + "forwarded_count": room_key['forwarded_count'], + "is_verified": room_key['is_verified'], + "session_data": room_key['session_data'], + }, + lock=False, ) # XXX: this isn't currently used and isn't tested anywhere @@ -107,7 +98,9 @@ class EndToEndRoomKeyStore(SQLBaseStore): ) @defer.inlineCallbacks - def get_e2e_room_keys(self, user_id, version, room_id, session_id): + def get_e2e_room_keys( + self, user_id, version, room_id=room_id, session_id=session_id + ): keyvalues = { "user_id": user_id, @@ -115,8 +108,8 @@ class EndToEndRoomKeyStore(SQLBaseStore): } if room_id: keyvalues['room_id'] = room_id - if session_id: - keyvalues['session_id'] = session_id + if session_id: + keyvalues['session_id'] = session_id rows = yield self._simple_select_list( table="e2e_room_keys", @@ -133,18 +126,10 @@ class EndToEndRoomKeyStore(SQLBaseStore): desc="get_e2e_room_keys", ) - # perlesque autovivification from https://stackoverflow.com/a/19829714/6764493 - class AutoVivification(dict): - def __getitem__(self, item): - try: - return dict.__getitem__(self, item) - except KeyError: - value = self[item] = type(self)() - return value - - sessions = AutoVivification() + sessions = {} for row in rows: - sessions['rooms'][row['room_id']]['sessions'][row['session_id']] = { + room_entry = sessions['rooms'].setdefault(row['room_id'], {"sessions": {}}) + room_entry['sessions'][row['session_id']] = { "first_message_index": row["first_message_index"], "forwarded_count": row["forwarded_count"], "is_verified": row["is_verified"], @@ -154,7 +139,9 @@ class EndToEndRoomKeyStore(SQLBaseStore): defer.returnValue(sessions) @defer.inlineCallbacks - def delete_e2e_room_keys(self, user_id, version, room_id, session_id): + def delete_e2e_room_keys( + self, user_id, version, room_id=room_id, session_id=session_id + ): keyvalues = { "user_id": user_id, @@ -162,8 +149,8 @@ class EndToEndRoomKeyStore(SQLBaseStore): } if room_id: keyvalues['room_id'] = room_id - if session_id: - keyvalues['session_id'] = session_id + if session_id: + keyvalues['session_id'] = session_id yield self._simple_delete( table="e2e_room_keys", diff --git a/synapse/storage/schema/delta/46/e2e_room_keys.sql b/synapse/storage/schema/delta/46/e2e_room_keys.sql index 0d2a85fbe6..16499ac34c 100644 --- a/synapse/storage/schema/delta/46/e2e_room_keys.sql +++ b/synapse/storage/schema/delta/46/e2e_room_keys.sql @@ -25,16 +25,14 @@ CREATE TABLE e2e_room_keys ( session_data TEXT NOT NULL ); -CREATE UNIQUE INDEX e2e_room_keys_user_idx ON e2e_room_keys(user_id); -CREATE UNIQUE INDEX e2e_room_keys_room_idx ON e2e_room_keys(room_id); -CREATE UNIQUE INDEX e2e_room_keys_session_idx ON e2e_room_keys(session_id); +CREATE UNIQUE INDEX e2e_room_keys_idx ON e2e_room_keys(user_id, room_id, session_id); -- the metadata for each generation of encrypted e2e session backups -CREATE TABLE e2e_room_key_versions ( +CREATE TABLE e2e_room_keys_versions ( user_id TEXT NOT NULL, version TEXT NOT NULL, algorithm TEXT NOT NULL, auth_data TEXT NOT NULL ); -CREATE UNIQUE INDEX e2e_room_key_user_idx ON e2e_room_keys(user_id); +CREATE UNIQUE INDEX e2e_room_keys_versions_user_idx ON e2e_room_keys_versions(user_id); -- cgit 1.4.1 From 9f500cb39efa608c02f212711a5ad2177757bf4b Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 24 Dec 2017 17:42:17 +0000 Subject: more docstring for the e2e_room_keys rest --- synapse/handlers/e2e_room_keys.py | 2 -- synapse/rest/client/v2_alpha/room_keys.py | 51 +++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 2 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index b67d6a2a7e..7a940d1c21 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -59,7 +59,6 @@ class E2eRoomKeysHandler(object): @defer.inlineCallbacks def upload_room_keys(self, user_id, version, room_keys): - # TODO: Validate the JSON to make sure it has the right keys. # XXX: perhaps we should use a finer grained lock here? @@ -139,7 +138,6 @@ class E2eRoomKeysHandler(object): @defer.inlineCallbacks def create_version(self, user_id, version_info): - # TODO: Validate the JSON to make sure it has the right keys. # lock everyone out until we've switched version diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 70b7b4573f..04547c7d43 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -208,6 +208,10 @@ class RoomKeysServlet(RestServlet): """ Deletes one or more encrypted E2E room keys for a user for backup purposes. + DELETE /room_keys/keys/!abc:matrix.org/c0ff33?version=1 + HTTP/1.1 200 OK + {} + room_id: the ID of the room whose keys to delete (optional) session_id: the ID for the E2E session to delete (optional) version: the version of the user's backup which this data is for. @@ -240,6 +244,33 @@ class RoomKeysVersionServlet(RestServlet): @defer.inlineCallbacks def on_POST(self, request, version): + """ + Create a new backup version for this user's room_keys with the given + info. The version is allocated by the server and returned to the user + in the response. This API is intended to be used whenever the user + changes the encryption key for their backups, ensuring that backups + encrypted with different keys don't collide. + + The algorithm passed in the version info is a reverse-DNS namespaced + identifier to describe the format of the encrypted backupped keys. + + The auth_data is { user_id: "user_id", nonce: } + encrypted using the algorithm and current encryption key described above. + + POST /room_keys/version + Content-Type: application/json + { + "algorithm": "m.megolm_backup.v1", + "auth_data": "dGhpcyBzaG91bGQgYWN0dWFsbHkgYmUgZW5jcnlwdGVkIGpzb24K" + } + + HTTP/1.1 200 OK + Content-Type: application/json + { + "version": 12345 + } + """ + if version: raise SynapseError(405, "Cannot POST to a specific version") @@ -257,6 +288,17 @@ class RoomKeysVersionServlet(RestServlet): @defer.inlineCallbacks def on_GET(self, request, version): + """ + Retrieve the version information about a given version of the user's + room_keys backup. + + GET /room_keys/version/12345 HTTP/1.1 + { + "algorithm": "m.megolm_backup.v1", + "auth_data": "dGhpcyBzaG91bGQgYWN0dWFsbHkgYmUgZW5jcnlwdGVkIGpzb24K" + } + """ + requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() @@ -267,6 +309,15 @@ class RoomKeysVersionServlet(RestServlet): @defer.inlineCallbacks def on_DELETE(self, request, version): + """ + Delete the information about a given version of the user's + room_keys backup. Doesn't delete the actual room data. + + DELETE /room_keys/version/12345 HTTP/1.1 + HTTP/1.1 200 OK + {} + """ + requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() -- cgit 1.4.1 From 14b3da63a339333292a83410c0ba3148bcb644ba Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Wed, 27 Dec 2017 23:37:44 +0000 Subject: add a tonne of docstring; make upload_room_keys properly assert version --- synapse/handlers/e2e_room_keys.py | 111 +++++++++++++++++++++++++++--- synapse/rest/client/v2_alpha/room_keys.py | 11 ++- 2 files changed, 113 insertions(+), 9 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index 7a940d1c21..2fa025bfc7 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -42,7 +42,16 @@ class E2eRoomKeysHandler(object): self._upload_linearizer = Linearizer("upload_room_keys_lock") @defer.inlineCallbacks - def get_room_keys(self, user_id, version, room_id, session_id): + def get_room_keys(self, user_id, version, room_id=None, session_id=None): + """Bulk get the E2E room keys for a given backup, optionally filtered to a given + room, or a given session. + See EndToEndRoomKeyStore.get_e2e_room_keys for full details. + + Returns: + A deferred list of dicts giving the session_data and message metadata for + these room keys. + """ + # we deliberately take the lock to get keys so that changing the version # works atomically with (yield self._upload_linearizer.queue(user_id)): @@ -52,20 +61,56 @@ class E2eRoomKeysHandler(object): defer.returnValue(results) @defer.inlineCallbacks - def delete_room_keys(self, user_id, version, room_id, session_id): + def delete_room_keys(self, user_id, version, room_id=None, session_id=None): + """Bulk delete the E2E room keys for a given backup, optionally filtered to a given + room or a given session. + See EndToEndRoomKeyStore.delete_e2e_room_keys for full details. + + Returns: + A deferred of the deletion transaction + """ + # lock for consistency with uploading with (yield self._upload_linearizer.queue(user_id)): yield self.store.delete_e2e_room_keys(user_id, version, room_id, session_id) @defer.inlineCallbacks def upload_room_keys(self, user_id, version, room_keys): + """Bulk upload a list of room keys into a given backup version, asserting + that the given version is the current backup version. room_keys are merged + into the current backup as described in RoomKeysServlet.on_PUT(). + + Args: + user_id(str): the user whose backup we're setting + version(str): the version ID of the backup we're updating + room_keys(dict): a nested dict describing the room_keys we're setting: + + { + "rooms": { + "!abc:matrix.org": { + "sessions": { + "c0ff33": { + "first_message_index": 1, + "forwarded_count": 1, + "is_verified": false, + "session_data": "SSBBTSBBIEZJU0gK" + } + } + } + } + } + + Raises: + SynapseError: with code 404 if there are no versions defined + RoomKeysVersionError: if the uploaded version is not the current version + """ + # TODO: Validate the JSON to make sure it has the right keys. # XXX: perhaps we should use a finer grained lock here? with (yield self._upload_linearizer.queue(user_id)): # Check that the version we're trying to upload is the current version - try: - version_info = yield self.get_version_info(user_id, version) + version_info = yield self.get_current_version_info(user_id) except StoreError as e: if e.code == 404: raise SynapseError(404, "Version '%s' not found" % (version,)) @@ -87,6 +132,17 @@ class E2eRoomKeysHandler(object): @defer.inlineCallbacks def _upload_room_key(self, user_id, version, room_id, session_id, room_key): + """Upload a given room_key for a given room and session into a given + version of the backup. Merges the key with any which might already exist. + + Args: + user_id(str): the user whose backup we're setting + version(str): the version ID of the backup we're updating + room_id(str): the ID of the room whose keys we're setting + session_id(str): the session whose room_key we're setting + room_key(dict): the room_key being set + """ + # get the room_key for this particular row current_room_key = None try: @@ -138,6 +194,23 @@ class E2eRoomKeysHandler(object): @defer.inlineCallbacks def create_version(self, user_id, version_info): + """Create a new backup version. This automatically becomes the new + backup version for the user's keys; previous backups will no longer be + writeable to. + + Args: + user_id(str): the user whose backup version we're creating + version_info(dict): metadata about the new version being created + + { + "algorithm": "m.megolm_backup.v1", + "auth_data": "dGhpcyBzaG91bGQgYWN0dWFsbHkgYmUgZW5jcnlwdGVkIGpzb24K" + } + + Returns: + A deferred of a string that gives the new version number. + """ + # TODO: Validate the JSON to make sure it has the right keys. # lock everyone out until we've switched version @@ -148,14 +221,36 @@ class E2eRoomKeysHandler(object): defer.returnValue(new_version) @defer.inlineCallbacks - def get_version_info(self, user_id, version): + def get_current_version_info(self, user_id): + """Get the user's current backup version. + + Args: + user_id(str): the user whose current backup version we're querying + Raises: + StoreError: code 404 if there is no current backup version + Returns: + A deferred of a info dict that gives the info about the new version. + + { + "algorithm": "m.megolm_backup.v1", + "auth_data": "dGhpcyBzaG91bGQgYWN0dWFsbHkgYmUgZW5jcnlwdGVkIGpzb24K" + } + """ + with (yield self._upload_linearizer.queue(user_id)): - results = yield self.store.get_e2e_room_keys_version_info( - user_id, version - ) + results = yield self.store.get_e2e_room_keys_version_info(user_id) defer.returnValue(results) @defer.inlineCallbacks def delete_version(self, user_id, version): + """Deletes a given version of the user's e2e_room_keys backup + + Args: + user_id(str): the user whose current backup version we're deleting + version(str): the version id of the backup being deleted + Raises: + StoreError: code 404 if this backup version doesn't exist + """ + with (yield self._upload_linearizer.queue(user_id)): yield self.store.delete_e2e_room_keys_version(user_id, version) diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 04547c7d43..d3f857aba2 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -47,7 +47,7 @@ class RoomKeysServlet(RestServlet): room_id: the ID of the room the keys are for (optional) session_id: the ID for the E2E room keys for the room (optional) version: the version of the user's backup which this data is for. - the version must already have been created via the /change_secret API. + the version must already have been created via the /room_keys/version API. Each session has: * first_message_index: a numeric index indicating the oldest message @@ -59,6 +59,9 @@ class RoomKeysServlet(RestServlet): * session_data: base64-encrypted data describing the session. Returns 200 OK on success with body {} + Returns 403 Forbidden if the version in question is not the most recently + created version (i.e. if this is an old client trying to write to a stale backup) + Returns 404 Not Found if the version in question doesn't exist The API is designed to be otherwise agnostic to the room_key encryption algorithm being used. Sessions are merged with existing ones in the @@ -251,6 +254,9 @@ class RoomKeysVersionServlet(RestServlet): changes the encryption key for their backups, ensuring that backups encrypted with different keys don't collide. + It takes out an exclusive lock on this user's room_key backups, to ensure + clients only upload to the current backup. + The algorithm passed in the version info is a reverse-DNS namespaced identifier to describe the format of the encrypted backupped keys. @@ -292,6 +298,9 @@ class RoomKeysVersionServlet(RestServlet): Retrieve the version information about a given version of the user's room_keys backup. + It takes out an exclusive lock on this user's room_key backups, to ensure + clients only upload to the current backup. + GET /room_keys/version/12345 HTTP/1.1 { "algorithm": "m.megolm_backup.v1", -- cgit 1.4.1 From 93d174bcc4cf773bea2534d3683a1e91e5489c89 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 31 Dec 2017 14:10:49 +0000 Subject: improve docstring --- synapse/rest/client/v2_alpha/room_keys.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) (limited to 'synapse/rest') diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index d3f857aba2..ca69ced1e3 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -296,13 +296,17 @@ class RoomKeysVersionServlet(RestServlet): def on_GET(self, request, version): """ Retrieve the version information about a given version of the user's - room_keys backup. + room_keys backup. If the version part is missing, returns info about the + most current backup version (if any) It takes out an exclusive lock on this user's room_key backups, to ensure clients only upload to the current backup. + Returns 404 is the given version does not exist. + GET /room_keys/version/12345 HTTP/1.1 { + "version": "12345", "algorithm": "m.megolm_backup.v1", "auth_data": "dGhpcyBzaG91bGQgYWN0dWFsbHkgYmUgZW5jcnlwdGVkIGpzb24K" } -- cgit 1.4.1 From fe87890b18f57f0268bd65aeca881e7817bbe9e4 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 31 Dec 2017 17:47:11 +0000 Subject: implement remaining tests and make them work --- synapse/handlers/e2e_room_keys.py | 35 +++- synapse/rest/client/v2_alpha/room_keys.py | 6 + synapse/storage/e2e_room_keys.py | 3 +- tests/handlers/test_e2e_room_keys.py | 276 +++++++++++++++++++++++++++--- 4 files changed, 287 insertions(+), 33 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index f08d80da3e..09c2888db6 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -58,6 +58,10 @@ class E2eRoomKeysHandler(object): results = yield self.store.get_e2e_room_keys( user_id, version, room_id, session_id ) + + if results['rooms'] == {}: + raise SynapseError(404, "No room_keys found") + defer.returnValue(results) @defer.inlineCallbacks @@ -109,9 +113,10 @@ class E2eRoomKeysHandler(object): # XXX: perhaps we should use a finer grained lock here? with (yield self._upload_linearizer.queue(user_id)): + # Check that the version we're trying to upload is the current version try: - version_info = yield self.get_version_info(user_id) + version_info = yield self._get_version_info_unlocked(user_id) except StoreError as e: if e.code == 404: raise SynapseError(404, "Version '%s' not found" % (version,)) @@ -119,16 +124,23 @@ class E2eRoomKeysHandler(object): raise e if version_info['version'] != version: - raise RoomKeysVersionError(current_version=version_info.version) + # Check that the version we're trying to upload actually exists + try: + version_info = yield self._get_version_info_unlocked(user_id, version) + # if we get this far, the version must exist + raise RoomKeysVersionError(current_version=version_info['version']) + except StoreError as e: + if e.code == 404: + raise SynapseError(404, "Version '%s' not found" % (version,)) + else: + raise e # go through the room_keys. # XXX: this should/could be done concurrently, given we're in a lock. for room_id, room in room_keys['rooms'].iteritems(): for session_id, session in room['sessions'].iteritems(): - room_key = session[session_id] - yield self._upload_room_key( - user_id, version, room_id, session_id, room_key + user_id, version, room_id, session_id, session ) @defer.inlineCallbacks @@ -242,8 +254,17 @@ class E2eRoomKeysHandler(object): """ with (yield self._upload_linearizer.queue(user_id)): - results = yield self.store.get_e2e_room_keys_version_info(user_id) - defer.returnValue(results) + res = yield self._get_version_info_unlocked(user_id, version) + defer.returnValue(res) + + @defer.inlineCallbacks + def _get_version_info_unlocked(self, user_id, version=None): + """Get the info about a given version of the user's backup + without obtaining the upload_linearizer lock. For params see get_version_info + """ + + results = yield self.store.get_e2e_room_keys_version_info(user_id, version) + defer.returnValue(results) @defer.inlineCallbacks def delete_version(self, user_id, version): diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index ca69ced1e3..8f10e4e1cd 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -204,6 +204,12 @@ class RoomKeysServlet(RestServlet): room_keys = yield self.e2e_room_keys_handler.get_room_keys( user_id, version, room_id, session_id ) + + if session_id: + room_keys = room_keys['rooms'][room_id]['sessions'][session_id] + elif room_id: + room_keys = room_keys['rooms'][room_id] + defer.returnValue((200, room_keys)) @defer.inlineCallbacks diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index e4d56b7c37..8e8e4e457c 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -58,6 +58,7 @@ class EndToEndRoomKeyStore(SQLBaseStore): defer.returnValue(row) + @defer.inlineCallbacks def set_e2e_room_key(self, user_id, version, room_id, session_id, room_key): """Replaces or inserts the encrypted E2E room key for a given session in a given backup @@ -135,7 +136,7 @@ class EndToEndRoomKeyStore(SQLBaseStore): desc="get_e2e_room_keys", ) - sessions = {} + sessions = { 'rooms': {} } for row in rows: room_entry = sessions['rooms'].setdefault(row['room_id'], {"sessions": {}}) room_entry['sessions'][row['session_id']] = { diff --git a/tests/handlers/test_e2e_room_keys.py b/tests/handlers/test_e2e_room_keys.py index afe6ecf27b..3cbfd6f9d0 100644 --- a/tests/handlers/test_e2e_room_keys.py +++ b/tests/handlers/test_e2e_room_keys.py @@ -17,6 +17,7 @@ import mock from synapse.api import errors from twisted.internet import defer +import copy import synapse.api.errors import synapse.handlers.e2e_room_keys @@ -25,6 +26,22 @@ import synapse.storage from tests import unittest, utils +# sample room_key data for use in the tests +room_keys = { + "rooms": { + "!abc:matrix.org": { + "sessions": { + "c0ff33": { + "first_message_index": 1, + "forwarded_count": 1, + "is_verified": False, + "session_data": "SSBBTSBBIEZJU0gK" + } + } + } + } +} + class E2eRoomKeysHandlerTestCase(unittest.TestCase): def __init__(self, *args, **kwargs): super(E2eRoomKeysHandlerTestCase, self).__init__(*args, **kwargs) @@ -38,46 +55,44 @@ class E2eRoomKeysHandlerTestCase(unittest.TestCase): replication_layer=mock.Mock(), ) self.handler = synapse.handlers.e2e_room_keys.E2eRoomKeysHandler(self.hs) + self.local_user = "@boris:" + self.hs.hostname; @defer.inlineCallbacks def test_get_missing_current_version_info(self): """Check that we get a 404 if we ask for info about the current version if there is no version. """ - local_user = "@boris:" + self.hs.hostname res = None try: - res = yield self.handler.get_version_info(local_user) + yield self.handler.get_version_info(self.local_user) except errors.SynapseError as e: - self.assertEqual(e.code, 404) - self.assertEqual(res, None) + res = e.code + self.assertEqual(res, 404) @defer.inlineCallbacks def test_get_missing_version_info(self): """Check that we get a 404 if we ask for info about a specific version if it doesn't exist. """ - local_user = "@boris:" + self.hs.hostname res = None try: - res = yield self.handler.get_version_info(local_user, "mrflibble") + yield self.handler.get_version_info(self.local_user, "bogus_version") except errors.SynapseError as e: - self.assertEqual(e.code, 404) - self.assertEqual(res, None) + res = e.code + self.assertEqual(res, 404) @defer.inlineCallbacks def test_create_version(self): """Check that we can create and then retrieve versions. """ - local_user = "@boris:" + self.hs.hostname - res = yield self.handler.create_version(local_user, { + res = yield self.handler.create_version(self.local_user, { "algorithm": "m.megolm_backup.v1", "auth_data": "first_version_auth_data", }) self.assertEqual(res, "1") # check we can retrieve it as the current version - res = yield self.handler.get_version_info(local_user) + res = yield self.handler.get_version_info(self.local_user) self.assertDictEqual(res, { "version": "1", "algorithm": "m.megolm_backup.v1", @@ -85,7 +100,7 @@ class E2eRoomKeysHandlerTestCase(unittest.TestCase): }) # check we can retrieve it as a specific version - res = yield self.handler.get_version_info(local_user, "1") + res = yield self.handler.get_version_info(self.local_user, "1") self.assertDictEqual(res, { "version": "1", "algorithm": "m.megolm_backup.v1", @@ -93,14 +108,14 @@ class E2eRoomKeysHandlerTestCase(unittest.TestCase): }) # upload a new one... - res = yield self.handler.create_version(local_user, { + res = yield self.handler.create_version(self.local_user, { "algorithm": "m.megolm_backup.v1", "auth_data": "second_version_auth_data", }) self.assertEqual(res, "2") # check we can retrieve it as the current version - res = yield self.handler.get_version_info(local_user) + res = yield self.handler.get_version_info(self.local_user) self.assertDictEqual(res, { "version": "2", "algorithm": "m.megolm_backup.v1", @@ -111,32 +126,243 @@ class E2eRoomKeysHandlerTestCase(unittest.TestCase): def test_delete_version(self): """Check that we can create and then delete versions. """ - local_user = "@boris:" + self.hs.hostname - res = yield self.handler.create_version(local_user, { + res = yield self.handler.create_version(self.local_user, { "algorithm": "m.megolm_backup.v1", "auth_data": "first_version_auth_data", }) self.assertEqual(res, "1") # check we can delete it - yield self.handler.delete_version(local_user, "1") + yield self.handler.delete_version(self.local_user, "1") # check that it's gone res = None try: - res = yield self.handler.get_version_info(local_user, "1") + yield self.handler.get_version_info(self.local_user, "1") except errors.SynapseError as e: - self.assertEqual(e.code, 404) - self.assertEqual(res, None) + res = e.code + self.assertEqual(res, 404) @defer.inlineCallbacks - def test_get_room_keys(self): - yield None + def test_get_missing_room_keys(self): + """Check that we get a 404 on querying missing room_keys + """ + res = None + try: + yield self.handler.get_room_keys(self.local_user, "bogus_version") + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 404) + + # check we also get a 404 even if the version is valid + version = yield self.handler.create_version(self.local_user, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "first_version_auth_data", + }) + self.assertEqual(version, "1") + + res = None + try: + yield self.handler.get_room_keys(self.local_user, version) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 404) + + # TODO: test the locking semantics when uploading room_keys, + # although this is probably best done in sytest @defer.inlineCallbacks - def test_upload_room_keys(self): - yield None + def test_upload_room_keys_no_versions(self): + """Check that we get a 404 on uploading keys when no versions are defined + """ + res = None + try: + yield self.handler.upload_room_keys(self.local_user, "no_version", room_keys) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 404) + + @defer.inlineCallbacks + def test_upload_room_keys_bogus_version(self): + """Check that we get a 404 on uploading keys when an nonexistent version is specified + """ + version = yield self.handler.create_version(self.local_user, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "first_version_auth_data", + }) + self.assertEqual(version, "1") + + res = None + try: + yield self.handler.upload_room_keys(self.local_user, "bogus_version", room_keys) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 404) + + @defer.inlineCallbacks + def test_upload_room_keys_wrong_version(self): + """Check that we get a 403 on uploading keys for an old version + """ + version = yield self.handler.create_version(self.local_user, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "first_version_auth_data", + }) + self.assertEqual(version, "1") + + version = yield self.handler.create_version(self.local_user, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "second_version_auth_data", + }) + self.assertEqual(version, "2") + + res = None + try: + yield self.handler.upload_room_keys(self.local_user, "1", room_keys) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 403) + + @defer.inlineCallbacks + def test_upload_room_keys_insert(self): + """Check that we can insert and retrieve keys for a session + """ + version = yield self.handler.create_version(self.local_user, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "first_version_auth_data", + }) + self.assertEqual(version, "1") + + yield self.handler.upload_room_keys(self.local_user, version, room_keys) + + res = yield self.handler.get_room_keys(self.local_user, version) + self.assertDictEqual(res, room_keys) + + # check getting room_keys for a given room + res = yield self.handler.get_room_keys( + self.local_user, + version, + room_id="!abc:matrix.org" + ) + self.assertDictEqual(res, room_keys) + + # check getting room_keys for a given session_id + res = yield self.handler.get_room_keys( + self.local_user, + version, + room_id="!abc:matrix.org", + session_id="c0ff33", + ) + self.assertDictEqual(res, room_keys) + + @defer.inlineCallbacks + def test_upload_room_keys_merge(self): + """Check that we can upload a new room_key for an existing session and + have it correctly merged""" + version = yield self.handler.create_version(self.local_user, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "first_version_auth_data", + }) + self.assertEqual(version, "1") + + yield self.handler.upload_room_keys(self.local_user, version, room_keys) + + new_room_keys = copy.deepcopy(room_keys) + + # test that increasing the message_index doesn't replace the existing session + new_room_keys['rooms']['!abc:matrix.org']['sessions']['c0ff33']['first_message_index'] = 2 + new_room_keys['rooms']['!abc:matrix.org']['sessions']['c0ff33']['session_data'] = 'new' + yield self.handler.upload_room_keys(self.local_user, version, new_room_keys) + + res = yield self.handler.get_room_keys(self.local_user, version) + self.assertEqual( + res['rooms']['!abc:matrix.org']['sessions']['c0ff33']['session_data'], + "SSBBTSBBIEZJU0gK" + ) + + # test that marking the session as verified however /does/ replace it + new_room_keys['rooms']['!abc:matrix.org']['sessions']['c0ff33']['is_verified'] = True + yield self.handler.upload_room_keys(self.local_user, version, new_room_keys) + + res = yield self.handler.get_room_keys(self.local_user, version) + self.assertEqual( + res['rooms']['!abc:matrix.org']['sessions']['c0ff33']['session_data'], + "new" + ) + + # test that a session with a higher forwarded_count doesn't replace one + # with a lower forwarding count + new_room_keys['rooms']['!abc:matrix.org']['sessions']['c0ff33']['forwarded_count'] = 2 + new_room_keys['rooms']['!abc:matrix.org']['sessions']['c0ff33']['session_data'] = 'other' + yield self.handler.upload_room_keys(self.local_user, version, new_room_keys) + + res = yield self.handler.get_room_keys(self.local_user, version) + self.assertEqual( + res['rooms']['!abc:matrix.org']['sessions']['c0ff33']['session_data'], + "new" + ) + + # TODO: check edge cases as well as the common variations here @defer.inlineCallbacks def test_delete_room_keys(self): - yield None + """Check that we can insert and delete keys for a session + """ + version = yield self.handler.create_version(self.local_user, { + "algorithm": "m.megolm_backup.v1", + "auth_data": "first_version_auth_data", + }) + self.assertEqual(version, "1") + + # check for bulk-delete + yield self.handler.upload_room_keys(self.local_user, version, room_keys) + yield self.handler.delete_room_keys(self.local_user, version) + res = None + try: + yield self.handler.get_room_keys( + self.local_user, + version, + room_id="!abc:matrix.org", + session_id="c0ff33", + ) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 404) + + # check for bulk-delete per room + yield self.handler.upload_room_keys(self.local_user, version, room_keys) + yield self.handler.delete_room_keys( + self.local_user, + version, + room_id="!abc:matrix.org", + ) + res = None + try: + yield self.handler.get_room_keys( + self.local_user, + version, + room_id="!abc:matrix.org", + session_id="c0ff33", + ) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 404) + + # check for bulk-delete per session + yield self.handler.upload_room_keys(self.local_user, version, room_keys) + yield self.handler.delete_room_keys( + self.local_user, + version, + room_id="!abc:matrix.org", + session_id="c0ff33", + ) + res = None + try: + yield self.handler.get_room_keys( + self.local_user, + version, + room_id="!abc:matrix.org", + session_id="c0ff33", + ) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 404) -- cgit 1.4.1 From 72788cf9c1f018a5346a8f9204c6bfe058289fd2 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 7 Jan 2018 23:45:26 +0000 Subject: support DELETE /version with no args --- synapse/handlers/e2e_room_keys.py | 2 +- synapse/rest/client/v2_alpha/room_keys.py | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index 09c2888db6..a43fc7fc7e 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -267,7 +267,7 @@ class E2eRoomKeysHandler(object): defer.returnValue(results) @defer.inlineCallbacks - def delete_version(self, user_id, version): + def delete_version(self, user_id, version=None): """Deletes a given version of the user's e2e_room_keys backup Args: diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 8f10e4e1cd..63b1f62f90 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -308,7 +308,7 @@ class RoomKeysVersionServlet(RestServlet): It takes out an exclusive lock on this user's room_key backups, to ensure clients only upload to the current backup. - Returns 404 is the given version does not exist. + Returns 404 if the given version does not exist. GET /room_keys/version/12345 HTTP/1.1 { @@ -330,7 +330,8 @@ class RoomKeysVersionServlet(RestServlet): def on_DELETE(self, request, version): """ Delete the information about a given version of the user's - room_keys backup. Doesn't delete the actual room data. + room_keys backup. If the version part is missing, deletes the most + current backup version (if any). Doesn't delete the actual room data. DELETE /room_keys/version/12345 HTTP/1.1 HTTP/1.1 200 OK -- cgit 1.4.1 From 54ac18e8327e689b9f91b93ad8caac2f6b3dd29c Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 7 Jan 2018 23:48:22 +0000 Subject: use parse_string --- synapse/rest/client/v2_alpha/room_keys.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 63b1f62f90..afb8a36dd0 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -123,7 +123,7 @@ class RoomKeysServlet(RestServlet): requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() body = parse_json_object_from_request(request) - version = request.args.get("version")[0] + version = parse_string(request, "version") if session_id: body = { @@ -199,7 +199,7 @@ class RoomKeysServlet(RestServlet): """ requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() - version = request.args.get("version")[0] + version = parse_string(request, "version") room_keys = yield self.e2e_room_keys_handler.get_room_keys( user_id, version, room_id, session_id @@ -229,7 +229,7 @@ class RoomKeysServlet(RestServlet): requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() - version = request.args.get("version")[0] + version = parse_string(request, "version") yield self.e2e_room_keys_handler.delete_room_keys( user_id, version, room_id, session_id -- cgit 1.4.1 From 4f7064f6b5a1822067bfbf358317bc1dbb51b9c6 Mon Sep 17 00:00:00 2001 From: Matthew Hodgson Date: Sun, 7 Jan 2018 23:59:07 +0000 Subject: missing import --- synapse/rest/client/v2_alpha/room_keys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/rest') diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index afb8a36dd0..9f0172e6f5 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -19,7 +19,7 @@ from twisted.internet import defer from synapse.api.errors import SynapseError from synapse.http.servlet import ( - RestServlet, parse_json_object_from_request + RestServlet, parse_json_object_from_request, parse_string ) from ._base import client_v2_patterns -- cgit 1.4.1 From 3801b8aa035594972c400c8bd036894a388c4ab3 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Thu, 6 Sep 2018 11:23:16 -0400 Subject: try to make flake8 and isort happy --- synapse/api/errors.py | 1 + synapse/handlers/e2e_room_keys.py | 2 +- synapse/rest/client/v2_alpha/room_keys.py | 5 ++++- synapse/storage/e2e_room_keys.py | 6 ++++-- tests/handlers/test_e2e_room_keys.py | 9 +++++---- 5 files changed, 15 insertions(+), 8 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 3002c95dd1..140dbfe8b8 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -326,6 +326,7 @@ class RoomKeysVersionError(SynapseError): ) self.current_version = current_version + class IncompatibleRoomVersionError(SynapseError): """A server is trying to join a room whose version it does not support.""" diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index c09816b372..2c330382cf 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -17,7 +17,7 @@ import logging from twisted.internet import defer -from synapse.api.errors import StoreError, SynapseError, RoomKeysVersionError +from synapse.api.errors import RoomKeysVersionError, StoreError, SynapseError from synapse.util.async_helpers import Linearizer logger = logging.getLogger(__name__) diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 9f0172e6f5..1ed18e986f 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -19,8 +19,11 @@ from twisted.internet import defer from synapse.api.errors import SynapseError from synapse.http.servlet import ( - RestServlet, parse_json_object_from_request, parse_string + RestServlet, + parse_json_object_from_request, + parse_string, ) + from ._base import client_v2_patterns logger = logging.getLogger(__name__) diff --git a/synapse/storage/e2e_room_keys.py b/synapse/storage/e2e_room_keys.py index b695570a7b..969f4aef9c 100644 --- a/synapse/storage/e2e_room_keys.py +++ b/synapse/storage/e2e_room_keys.py @@ -13,9 +13,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import simplejson as json + from twisted.internet import defer + from synapse.api.errors import StoreError -import simplejson as json from ._base import SQLBaseStore @@ -58,7 +60,7 @@ class EndToEndRoomKeyStore(SQLBaseStore): desc="get_e2e_room_key", ) - row["session_data"] = json.loads(row["session_data"]); + row["session_data"] = json.loads(row["session_data"]) defer.returnValue(row) diff --git a/tests/handlers/test_e2e_room_keys.py b/tests/handlers/test_e2e_room_keys.py index 7fa4264441..9e08eac0a5 100644 --- a/tests/handlers/test_e2e_room_keys.py +++ b/tests/handlers/test_e2e_room_keys.py @@ -14,17 +14,18 @@ # See the License for the specific language governing permissions and # limitations under the License. +import copy + import mock -from synapse.api import errors + from twisted.internet import defer -import copy import synapse.api.errors import synapse.handlers.e2e_room_keys - import synapse.storage -from tests import unittest, utils +from synapse.api import errors +from tests import unittest, utils # sample room_key data for use in the tests room_keys = { -- cgit 1.4.1 From bc74925c5b94f0c02603297d4ccb7e05008d5124 Mon Sep 17 00:00:00 2001 From: David Baker Date: Thu, 13 Sep 2018 17:02:59 +0100 Subject: WIP e2e key backups Continues from uhoreg's branch This just fixed the errcode on /room_keys/version if no backup and updates the schema delta to be on the latest so it gets run --- synapse/rest/client/v2_alpha/room_keys.py | 14 ++++++--- synapse/storage/schema/delta/46/e2e_room_keys.sql | 38 ----------------------- synapse/storage/schema/delta/51/e2e_room_keys.sql | 38 +++++++++++++++++++++++ 3 files changed, 48 insertions(+), 42 deletions(-) delete mode 100644 synapse/storage/schema/delta/46/e2e_room_keys.sql create mode 100644 synapse/storage/schema/delta/51/e2e_room_keys.sql (limited to 'synapse/rest') diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 1ed18e986f..ea114bc8b4 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -17,7 +17,7 @@ import logging from twisted.internet import defer -from synapse.api.errors import SynapseError +from synapse.api.errors import SynapseError, Codes from synapse.http.servlet import ( RestServlet, parse_json_object_from_request, @@ -324,9 +324,15 @@ class RoomKeysVersionServlet(RestServlet): requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() - info = yield self.e2e_room_keys_handler.get_version_info( - user_id, version - ) + try: + info = yield self.e2e_room_keys_handler.get_version_info( + user_id, version + ) + except SynapseError as e: + if e.code == 404: + e.errcode = Codes.NOT_FOUND + e.msg = "No backup found" + raise e defer.returnValue((200, info)) @defer.inlineCallbacks diff --git a/synapse/storage/schema/delta/46/e2e_room_keys.sql b/synapse/storage/schema/delta/46/e2e_room_keys.sql deleted file mode 100644 index 4531fd56ee..0000000000 --- a/synapse/storage/schema/delta/46/e2e_room_keys.sql +++ /dev/null @@ -1,38 +0,0 @@ -/* Copyright 2017 New Vector Ltd - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - --- users' optionally backed up encrypted e2e sessions -CREATE TABLE e2e_room_keys ( - user_id TEXT NOT NULL, - room_id TEXT NOT NULL, - session_id TEXT NOT NULL, - version TEXT NOT NULL, - first_message_index INT, - forwarded_count INT, - is_verified BOOLEAN, - session_data TEXT NOT NULL -); - -CREATE UNIQUE INDEX e2e_room_keys_idx ON e2e_room_keys(user_id, room_id, session_id); - --- the metadata for each generation of encrypted e2e session backups -CREATE TABLE e2e_room_keys_versions ( - user_id TEXT NOT NULL, - version TEXT NOT NULL, - algorithm TEXT NOT NULL, - auth_data TEXT NOT NULL -); - -CREATE UNIQUE INDEX e2e_room_keys_versions_idx ON e2e_room_keys_versions(user_id, version); diff --git a/synapse/storage/schema/delta/51/e2e_room_keys.sql b/synapse/storage/schema/delta/51/e2e_room_keys.sql new file mode 100644 index 0000000000..4531fd56ee --- /dev/null +++ b/synapse/storage/schema/delta/51/e2e_room_keys.sql @@ -0,0 +1,38 @@ +/* Copyright 2017 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +-- users' optionally backed up encrypted e2e sessions +CREATE TABLE e2e_room_keys ( + user_id TEXT NOT NULL, + room_id TEXT NOT NULL, + session_id TEXT NOT NULL, + version TEXT NOT NULL, + first_message_index INT, + forwarded_count INT, + is_verified BOOLEAN, + session_data TEXT NOT NULL +); + +CREATE UNIQUE INDEX e2e_room_keys_idx ON e2e_room_keys(user_id, room_id, session_id); + +-- the metadata for each generation of encrypted e2e session backups +CREATE TABLE e2e_room_keys_versions ( + user_id TEXT NOT NULL, + version TEXT NOT NULL, + algorithm TEXT NOT NULL, + auth_data TEXT NOT NULL +); + +CREATE UNIQUE INDEX e2e_room_keys_versions_idx ON e2e_room_keys_versions(user_id, version); -- cgit 1.4.1 From d3464ce708cd857216d88d3e67867f1cf0c8bf60 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 9 Oct 2018 10:33:59 +0100 Subject: isort --- synapse/rest/client/v2_alpha/room_keys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse/rest') diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index ea114bc8b4..539893a5d6 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -17,7 +17,7 @@ import logging from twisted.internet import defer -from synapse.api.errors import SynapseError, Codes +from synapse.api.errors import Codes, SynapseError from synapse.http.servlet import ( RestServlet, parse_json_object_from_request, -- cgit 1.4.1 From 83e72bb2f0c6ef282190a378941c856afbb33c16 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 12 Oct 2018 11:26:18 +0100 Subject: PR feedback pt. 1 --- synapse/api/errors.py | 8 ------ synapse/handlers/e2e_room_keys.py | 41 ++++++++++++++++--------------- synapse/rest/client/v2_alpha/room_keys.py | 2 +- 3 files changed, 22 insertions(+), 29 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 0a6e78711f..48b903374d 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -348,14 +348,6 @@ class IncompatibleRoomVersionError(SynapseError): ) -def cs_exception(exception): - if isinstance(exception, CodeMessageException): - return exception.error_dict() - else: - logger.error("Unknown exception type: %s", type(exception)) - return {} - - def cs_error(msg, code=Codes.UNKNOWN, **kwargs): """ Utility method for constructing an error response for client-server interactions. diff --git a/synapse/handlers/e2e_room_keys.py b/synapse/handlers/e2e_room_keys.py index bf2a83cc31..4e3141dac8 100644 --- a/synapse/handlers/e2e_room_keys.py +++ b/synapse/handlers/e2e_room_keys.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# Copyright 2017 New Vector Ltd +# Copyright 2017, 2018 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -49,6 +49,11 @@ class E2eRoomKeysHandler(object): room, or a given session. See EndToEndRoomKeyStore.get_e2e_room_keys for full details. + Args: + user_id(str): the user whose keys we're getting + version(str): the version ID of the backup we're getting keys from + room_id(string): room ID to get keys for, for None to get keys for all rooms + session_id(string): session ID to get keys for, for None to get keys for all sessions Returns: A deferred list of dicts giving the session_data and message metadata for these room keys. @@ -72,6 +77,11 @@ class E2eRoomKeysHandler(object): room or a given session. See EndToEndRoomKeyStore.delete_e2e_room_keys for full details. + Args: + user_id(str): the user whose backup we're deleting + version(str): the version ID of the backup we're deleting + room_id(string): room ID to delete keys for, for None to delete keys for all rooms + session_id(string): session ID to delete keys for, for None to delete keys for all sessions Returns: A deferred of the deletion transaction """ @@ -118,24 +128,24 @@ class E2eRoomKeysHandler(object): # Check that the version we're trying to upload is the current version try: - version_info = yield self._get_version_info_unlocked(user_id) + version_info = yield self.store.get_e2e_room_keys_version_info(user_id) except StoreError as e: if e.code == 404: raise SynapseError(404, "Version '%s' not found" % (version,)) else: - raise e + raise if version_info['version'] != version: # Check that the version we're trying to upload actually exists try: - version_info = yield self._get_version_info_unlocked(user_id, version) + version_info = yield self.store.get_e2e_room_keys_version_info(user_id, version) # if we get this far, the version must exist raise RoomKeysVersionError(current_version=version_info['version']) except StoreError as e: if e.code == 404: raise SynapseError(404, "Version '%s' not found" % (version,)) else: - raise e + raise # go through the room_keys. # XXX: this should/could be done concurrently, given we're in a lock. @@ -168,9 +178,9 @@ class E2eRoomKeysHandler(object): if e.code == 404: pass else: - raise e + raise - if E2eRoomKeysHandler._should_replace_room_key(current_room_key, room_key): + if self._should_replace_room_key(current_room_key, room_key): yield self.store.set_e2e_room_key( user_id, version, room_id, session_id, room_key ) @@ -195,14 +205,14 @@ class E2eRoomKeysHandler(object): # purely for legibility. if room_key['is_verified'] and not current_room_key['is_verified']: - pass + return True elif ( room_key['first_message_index'] < current_room_key['first_message_index'] ): - pass + return True elif room_key['forwarded_count'] < current_room_key['forwarded_count']: - pass + return True else: return False return True @@ -256,18 +266,9 @@ class E2eRoomKeysHandler(object): """ with (yield self._upload_linearizer.queue(user_id)): - res = yield self._get_version_info_unlocked(user_id, version) + res = yield self.store.get_e2e_room_keys_version_info(user_id, version) defer.returnValue(res) - @defer.inlineCallbacks - def _get_version_info_unlocked(self, user_id, version=None): - """Get the info about a given version of the user's backup - without obtaining the upload_linearizer lock. For params see get_version_info - """ - - results = yield self.store.get_e2e_room_keys_version_info(user_id, version) - defer.returnValue(results) - @defer.inlineCallbacks def delete_version(self, user_id, version=None): """Deletes a given version of the user's e2e_room_keys backup diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 539893a5d6..4807170ea6 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -1,5 +1,5 @@ # -*- coding: utf-8 -*- -# Copyright 2017 New Vector Ltd +# Copyright 2017, 2018 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. -- cgit 1.4.1 From 86ef9760a786e7de128120c11d623e3303178505 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 12 Oct 2018 11:35:08 +0100 Subject: Split /room_keys/version into 2 servlets --- synapse/rest/client/v2_alpha/room_keys.py | 30 +++++++++++++++++++++--------- 1 file changed, 21 insertions(+), 9 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 4807170ea6..aee6419179 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -240,9 +240,9 @@ class RoomKeysServlet(RestServlet): defer.returnValue((200, {})) -class RoomKeysVersionServlet(RestServlet): +class RoomKeysNewVersionServlet(RestServlet): PATTERNS = client_v2_patterns( - "/room_keys/version(/(?P[^/]+))?$" + "/room_keys/version$" ) def __init__(self, hs): @@ -250,12 +250,12 @@ class RoomKeysVersionServlet(RestServlet): Args: hs (synapse.server.HomeServer): server """ - super(RoomKeysVersionServlet, self).__init__() + super(RoomKeysNewVersionServlet, self).__init__() self.auth = hs.get_auth() self.e2e_room_keys_handler = hs.get_e2e_room_keys_handler() @defer.inlineCallbacks - def on_POST(self, request, version): + def on_POST(self, request): """ Create a new backup version for this user's room_keys with the given info. The version is allocated by the server and returned to the user @@ -285,10 +285,6 @@ class RoomKeysVersionServlet(RestServlet): "version": 12345 } """ - - if version: - raise SynapseError(405, "Cannot POST to a specific version") - requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() info = parse_json_object_from_request(request) @@ -301,6 +297,20 @@ class RoomKeysVersionServlet(RestServlet): # we deliberately don't have a PUT /version, as these things really should # be immutable to avoid people footgunning +class RoomKeysVersionServlet(RestServlet): + PATTERNS = client_v2_patterns( + "/room_keys/version(/(?P[^/]+))?$" + ) + + def __init__(self, hs): + """ + Args: + hs (synapse.server.HomeServer): server + """ + super(RoomKeysVersionServlet, self).__init__() + self.auth = hs.get_auth() + self.e2e_room_keys_handler = hs.get_e2e_room_keys_handler() + @defer.inlineCallbacks def on_GET(self, request, version): """ @@ -320,7 +330,6 @@ class RoomKeysVersionServlet(RestServlet): "auth_data": "dGhpcyBzaG91bGQgYWN0dWFsbHkgYmUgZW5jcnlwdGVkIGpzb24K" } """ - requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() @@ -346,6 +355,8 @@ class RoomKeysVersionServlet(RestServlet): HTTP/1.1 200 OK {} """ + if version is None: + raise SynapseError(400, "No version specified to delete") requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() @@ -359,3 +370,4 @@ class RoomKeysVersionServlet(RestServlet): def register_servlets(hs, http_server): RoomKeysServlet(hs).register(http_server) RoomKeysVersionServlet(hs).register(http_server) + RoomKeysNewVersionServlet(hs).register(http_server) -- cgit 1.4.1 From bddfad253a2ca8ea0645d10e89055c4ce2ff1c38 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 12 Oct 2018 11:48:02 +0100 Subject: Don't mangle exceptions --- synapse/rest/client/v2_alpha/room_keys.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index aee6419179..8d006d819d 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -339,9 +339,7 @@ class RoomKeysVersionServlet(RestServlet): ) except SynapseError as e: if e.code == 404: - e.errcode = Codes.NOT_FOUND - e.msg = "No backup found" - raise e + raise SynapseError(404, "No backup found", Codes.NOT_FOUND) defer.returnValue((200, info)) @defer.inlineCallbacks @@ -356,7 +354,7 @@ class RoomKeysVersionServlet(RestServlet): {} """ if version is None: - raise SynapseError(400, "No version specified to delete") + raise SynapseError(400, "No version specified to delete", Codes.NOT_FOUND) requester = yield self.auth.get_user_by_req(request, allow_guest=False) user_id = requester.user.to_string() -- cgit 1.4.1 From a45f2c3a002dd50ba4dc6dcbe45a51f2da4057b2 Mon Sep 17 00:00:00 2001 From: David Baker Date: Fri, 12 Oct 2018 14:33:55 +0100 Subject: missed one --- synapse/rest/client/v2_alpha/room_keys.py | 1 + 1 file changed, 1 insertion(+) (limited to 'synapse/rest') diff --git a/synapse/rest/client/v2_alpha/room_keys.py b/synapse/rest/client/v2_alpha/room_keys.py index 8d006d819d..45b5817d8b 100644 --- a/synapse/rest/client/v2_alpha/room_keys.py +++ b/synapse/rest/client/v2_alpha/room_keys.py @@ -297,6 +297,7 @@ class RoomKeysNewVersionServlet(RestServlet): # we deliberately don't have a PUT /version, as these things really should # be immutable to avoid people footgunning + class RoomKeysVersionServlet(RestServlet): PATTERNS = client_v2_patterns( "/room_keys/version(/(?P[^/]+))?$" -- cgit 1.4.1 From f6a0a02a62db1a3030e5563d6454d01c4a76c38d Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 17 Oct 2018 16:09:34 +0100 Subject: Fix bug where we raised StopIteration in a generator This made python 3.7 unhappy --- synapse/rest/media/v1/preview_url_resource.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py index af01040a38..8c892ff187 100644 --- a/synapse/rest/media/v1/preview_url_resource.py +++ b/synapse/rest/media/v1/preview_url_resource.py @@ -596,10 +596,13 @@ def _iterate_over_text(tree, *tags_to_ignore): # to be returned. elements = iter([tree]) while True: - el = next(elements) + el = next(elements, None) + if el is None: + return + if isinstance(el, string_types): yield el - elif el is not None and el.tag not in tags_to_ignore: + elif el.tag not in tags_to_ignore: # el.text is the text before the first child, so we can immediately # return it if the text exists. if el.text: -- cgit 1.4.1 From 74e761708398d5170783912f02d120f20113205e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 18 Oct 2018 16:14:24 +0100 Subject: Clean up room alias creation --- synapse/handlers/directory.py | 77 ++++++++++++++++++++++--------------- synapse/handlers/room.py | 5 ++- synapse/rest/client/v1/directory.py | 37 +++--------------- 3 files changed, 55 insertions(+), 64 deletions(-) (limited to 'synapse/rest') diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py index 18741c5fac..02f12f6645 100644 --- a/synapse/handlers/directory.py +++ b/synapse/handlers/directory.py @@ -80,42 +80,60 @@ class DirectoryHandler(BaseHandler): ) @defer.inlineCallbacks - def create_association(self, user_id, room_alias, room_id, servers=None): - # association creation for human users - # TODO(erikj): Do user auth. + def create_association(self, requester, room_alias, room_id, servers=None, + send_event=True): + """Attempt to create a new alias - if not self.spam_checker.user_may_create_room_alias(user_id, room_alias): - raise SynapseError( - 403, "This user is not permitted to create this alias", - ) + Args: + requester (Requester) + room_alias (RoomAlias) + room_id (str) + servers (list[str]|None): List of servers that others servers + should try and join via + send_event (bool): Whether to send an updated m.room.aliases event - can_create = yield self.can_modify_alias( - room_alias, - user_id=user_id - ) - if not can_create: - raise SynapseError( - 400, "This alias is reserved by an application service.", - errcode=Codes.EXCLUSIVE - ) - yield self._create_association(room_alias, room_id, servers, creator=user_id) + Returns: + Deferred + """ - @defer.inlineCallbacks - def create_appservice_association(self, service, room_alias, room_id, - servers=None): - if not service.is_interested_in_alias(room_alias.to_string()): - raise SynapseError( - 400, "This application service has not reserved" - " this kind of alias.", errcode=Codes.EXCLUSIVE + user_id = requester.user.to_string() + + service = requester.app_service + if service: + if not service.is_interested_in_alias(room_alias.to_string()): + raise SynapseError( + 400, "This application service has not reserved" + " this kind of alias.", errcode=Codes.EXCLUSIVE + ) + else: + if not self.spam_checker.user_may_create_room_alias(user_id, room_alias): + raise AuthError( + 403, "This user is not permitted to create this alias", + ) + + can_create = yield self.can_modify_alias( + room_alias, + user_id=user_id ) + if not can_create: + raise AuthError( + 400, "This alias is reserved by an application service.", + errcode=Codes.EXCLUSIVE + ) - # association creation for app services - yield self._create_association(room_alias, room_id, servers) + yield self._create_association(room_alias, room_id, servers, creator=user_id) + if send_event: + yield self.send_room_alias_update_event( + requester, + room_id + ) @defer.inlineCallbacks - def delete_association(self, requester, user_id, room_alias): + def delete_association(self, requester, room_alias): # association deletion for human users + user_id = requester.user.to_string() + try: can_delete = yield self._user_can_delete_alias(room_alias, user_id) except StoreError as e: @@ -143,7 +161,6 @@ class DirectoryHandler(BaseHandler): try: yield self.send_room_alias_update_event( requester, - requester.user.to_string(), room_id ) @@ -261,7 +278,7 @@ class DirectoryHandler(BaseHandler): ) @defer.inlineCallbacks - def send_room_alias_update_event(self, requester, user_id, room_id): + def send_room_alias_update_event(self, requester, room_id): aliases = yield self.store.get_aliases_for_room(room_id) yield self.event_creation_handler.create_and_send_nonmember_event( @@ -270,7 +287,7 @@ class DirectoryHandler(BaseHandler): "type": EventTypes.Aliases, "state_key": self.hs.hostname, "room_id": room_id, - "sender": user_id, + "sender": requester.user.to_string(), "content": {"aliases": aliases}, }, ratelimit=False diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index c3f820b975..ab1571b27b 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -190,10 +190,11 @@ class RoomCreationHandler(BaseHandler): if room_alias: directory_handler = self.hs.get_handlers().directory_handler yield directory_handler.create_association( - user_id=user_id, + requester=requester, room_id=room_id, room_alias=room_alias, servers=[self.hs.hostname], + send_event=False, ) preset_config = config.get( @@ -289,7 +290,7 @@ class RoomCreationHandler(BaseHandler): if room_alias: result["room_alias"] = room_alias.to_string() yield directory_handler.send_room_alias_update_event( - requester, user_id, room_id + requester, room_id ) defer.returnValue(result) diff --git a/synapse/rest/client/v1/directory.py b/synapse/rest/client/v1/directory.py index 97733f3026..0220acf644 100644 --- a/synapse/rest/client/v1/directory.py +++ b/synapse/rest/client/v1/directory.py @@ -74,38 +74,11 @@ class ClientDirectoryServer(ClientV1RestServlet): if room is None: raise SynapseError(400, "Room does not exist") - dir_handler = self.handlers.directory_handler + requester = yield self.auth.get_user_by_req(request) - try: - # try to auth as a user - requester = yield self.auth.get_user_by_req(request) - try: - user_id = requester.user.to_string() - yield dir_handler.create_association( - user_id, room_alias, room_id, servers - ) - yield dir_handler.send_room_alias_update_event( - requester, - user_id, - room_id - ) - except SynapseError as e: - raise e - except Exception: - logger.exception("Failed to create association") - raise - except AuthError: - # try to auth as an application service - service = yield self.auth.get_appservice_by_req(request) - yield dir_handler.create_appservice_association( - service, room_alias, room_id, servers - ) - logger.info( - "Application service at %s created alias %s pointing to %s", - service.url, - room_alias.to_string(), - room_id - ) + yield self.handlers.directory_handler.create_association( + requester, room_alias, room_id, servers + ) defer.returnValue((200, {})) @@ -135,7 +108,7 @@ class ClientDirectoryServer(ClientV1RestServlet): room_alias = RoomAlias.from_string(room_alias) yield dir_handler.delete_association( - requester, user.to_string(), room_alias + requester, room_alias ) logger.info( -- cgit 1.4.1 From 5c445114d356c9c23b99f0a2c4246be983a38b69 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 23 Oct 2018 13:12:32 +0100 Subject: Correctly account for cpu usage by background threads (#4074) Wrap calls to deferToThread() in a thing which uses a child logcontext to attribute CPU usage to the right request. While we're in the area, remove the logcontext_tracer stuff, which is never used, and afaik doesn't work. Fixes #4064 --- changelog.d/4074.bugfix | 1 + synapse/handlers/auth.py | 18 +---- synapse/rest/media/v1/media_repository.py | 24 +++--- synapse/rest/media/v1/media_storage.py | 8 +- synapse/rest/media/v1/storage_provider.py | 6 +- synapse/util/logcontext.py | 120 +++++++++++++++++------------- 6 files changed, 97 insertions(+), 80 deletions(-) create mode 100644 changelog.d/4074.bugfix (limited to 'synapse/rest') diff --git a/changelog.d/4074.bugfix b/changelog.d/4074.bugfix new file mode 100644 index 0000000000..b3b6b00243 --- /dev/null +++ b/changelog.d/4074.bugfix @@ -0,0 +1 @@ +Correctly account for cpu usage by background threads diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 2a5eab124f..329e3c7d71 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -22,7 +22,7 @@ import bcrypt import pymacaroons from canonicaljson import json -from twisted.internet import defer, threads +from twisted.internet import defer from twisted.web.client import PartialDownloadError import synapse.util.stringutils as stringutils @@ -37,8 +37,8 @@ from synapse.api.errors import ( ) from synapse.module_api import ModuleApi from synapse.types import UserID +from synapse.util import logcontext from synapse.util.caches.expiringcache import ExpiringCache -from synapse.util.logcontext import make_deferred_yieldable from ._base import BaseHandler @@ -884,11 +884,7 @@ class AuthHandler(BaseHandler): bcrypt.gensalt(self.bcrypt_rounds), ).decode('ascii') - return make_deferred_yieldable( - threads.deferToThreadPool( - self.hs.get_reactor(), self.hs.get_reactor().getThreadPool(), _do_hash - ), - ) + return logcontext.defer_to_thread(self.hs.get_reactor(), _do_hash) def validate_hash(self, password, stored_hash): """Validates that self.hash(password) == stored_hash. @@ -913,13 +909,7 @@ class AuthHandler(BaseHandler): if not isinstance(stored_hash, bytes): stored_hash = stored_hash.encode('ascii') - return make_deferred_yieldable( - threads.deferToThreadPool( - self.hs.get_reactor(), - self.hs.get_reactor().getThreadPool(), - _do_validate_hash, - ), - ) + return logcontext.defer_to_thread(self.hs.get_reactor(), _do_validate_hash) else: return defer.succeed(False) diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index a828ff4438..08b1867fab 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -25,7 +25,7 @@ from six.moves.urllib import parse as urlparse import twisted.internet.error import twisted.web.http -from twisted.internet import defer, threads +from twisted.internet import defer from twisted.web.resource import Resource from synapse.api.errors import ( @@ -36,8 +36,8 @@ from synapse.api.errors import ( ) from synapse.http.matrixfederationclient import MatrixFederationHttpClient from synapse.metrics.background_process_metrics import run_as_background_process +from synapse.util import logcontext from synapse.util.async_helpers import Linearizer -from synapse.util.logcontext import make_deferred_yieldable from synapse.util.retryutils import NotRetryingDestination from synapse.util.stringutils import is_ascii, random_string @@ -492,10 +492,11 @@ class MediaRepository(object): )) thumbnailer = Thumbnailer(input_path) - t_byte_source = yield make_deferred_yieldable(threads.deferToThread( + t_byte_source = yield logcontext.defer_to_thread( + self.hs.get_reactor(), self._generate_thumbnail, thumbnailer, t_width, t_height, t_method, t_type - )) + ) if t_byte_source: try: @@ -534,10 +535,11 @@ class MediaRepository(object): )) thumbnailer = Thumbnailer(input_path) - t_byte_source = yield make_deferred_yieldable(threads.deferToThread( + t_byte_source = yield logcontext.defer_to_thread( + self.hs.get_reactor(), self._generate_thumbnail, thumbnailer, t_width, t_height, t_method, t_type - )) + ) if t_byte_source: try: @@ -620,15 +622,17 @@ class MediaRepository(object): for (t_width, t_height, t_type), t_method in iteritems(thumbnails): # Generate the thumbnail if t_method == "crop": - t_byte_source = yield make_deferred_yieldable(threads.deferToThread( + t_byte_source = yield logcontext.defer_to_thread( + self.hs.get_reactor(), thumbnailer.crop, t_width, t_height, t_type, - )) + ) elif t_method == "scale": - t_byte_source = yield make_deferred_yieldable(threads.deferToThread( + t_byte_source = yield logcontext.defer_to_thread( + self.hs.get_reactor(), thumbnailer.scale, t_width, t_height, t_type, - )) + ) else: logger.error("Unrecognized method: %r", t_method) continue diff --git a/synapse/rest/media/v1/media_storage.py b/synapse/rest/media/v1/media_storage.py index a6189224ee..896078fe76 100644 --- a/synapse/rest/media/v1/media_storage.py +++ b/synapse/rest/media/v1/media_storage.py @@ -21,9 +21,10 @@ import sys import six -from twisted.internet import defer, threads +from twisted.internet import defer from twisted.protocols.basic import FileSender +from synapse.util import logcontext from synapse.util.file_consumer import BackgroundFileConsumer from synapse.util.logcontext import make_deferred_yieldable @@ -64,9 +65,10 @@ class MediaStorage(object): with self.store_into_file(file_info) as (f, fname, finish_cb): # Write to the main repository - yield make_deferred_yieldable(threads.deferToThread( + yield logcontext.defer_to_thread( + self.hs.get_reactor(), _write_file_synchronously, source, f, - )) + ) yield finish_cb() defer.returnValue(fname) diff --git a/synapse/rest/media/v1/storage_provider.py b/synapse/rest/media/v1/storage_provider.py index 7b9f8b4d79..5aa03031f6 100644 --- a/synapse/rest/media/v1/storage_provider.py +++ b/synapse/rest/media/v1/storage_provider.py @@ -17,9 +17,10 @@ import logging import os import shutil -from twisted.internet import defer, threads +from twisted.internet import defer from synapse.config._base import Config +from synapse.util import logcontext from synapse.util.logcontext import run_in_background from .media_storage import FileResponder @@ -120,7 +121,8 @@ class FileStorageProviderBackend(StorageProvider): if not os.path.exists(dirname): os.makedirs(dirname) - return threads.deferToThread( + return logcontext.defer_to_thread( + self.hs.get_reactor(), shutil.copyfile, primary_fname, backup_fname, ) diff --git a/synapse/util/logcontext.py b/synapse/util/logcontext.py index 89224b26cc..4c6e92beb8 100644 --- a/synapse/util/logcontext.py +++ b/synapse/util/logcontext.py @@ -25,7 +25,7 @@ See doc/log_contexts.rst for details on how this works. import logging import threading -from twisted.internet import defer +from twisted.internet import defer, threads logger = logging.getLogger(__name__) @@ -562,58 +562,76 @@ def _set_context_cb(result, context): return result -# modules to ignore in `logcontext_tracer` -_to_ignore = [ - "synapse.util.logcontext", - "synapse.http.server", - "synapse.storage._base", - "synapse.util.async_helpers", -] +def defer_to_thread(reactor, f, *args, **kwargs): + """ + Calls the function `f` using a thread from the reactor's default threadpool and + returns the result as a Deferred. + + Creates a new logcontext for `f`, which is created as a child of the current + logcontext (so its CPU usage metrics will get attributed to the current + logcontext). `f` should preserve the logcontext it is given. + + The result deferred follows the Synapse logcontext rules: you should `yield` + on it. + + Args: + reactor (twisted.internet.base.ReactorBase): The reactor in whose main thread + the Deferred will be invoked, and whose threadpool we should use for the + function. + + Normally this will be hs.get_reactor(). + + f (callable): The function to call. + args: positional arguments to pass to f. -def logcontext_tracer(frame, event, arg): - """A tracer that logs whenever a logcontext "unexpectedly" changes within - a function. Probably inaccurate. + kwargs: keyword arguments to pass to f. - Use by calling `sys.settrace(logcontext_tracer)` in the main thread. + Returns: + Deferred: A Deferred which fires a callback with the result of `f`, or an + errback if `f` throws an exception. """ - if event == 'call': - name = frame.f_globals["__name__"] - if name.startswith("synapse"): - if name == "synapse.util.logcontext": - if frame.f_code.co_name in ["__enter__", "__exit__"]: - tracer = frame.f_back.f_trace - if tracer: - tracer.just_changed = True - - tracer = frame.f_trace - if tracer: - return tracer - - if not any(name.startswith(ig) for ig in _to_ignore): - return LineTracer() - - -class LineTracer(object): - __slots__ = ["context", "just_changed"] - - def __init__(self): - self.context = LoggingContext.current_context() - self.just_changed = False - - def __call__(self, frame, event, arg): - if event in 'line': - if self.just_changed: - self.context = LoggingContext.current_context() - self.just_changed = False - else: - c = LoggingContext.current_context() - if c != self.context: - logger.info( - "Context changed! %s -> %s, %s, %s", - self.context, c, - frame.f_code.co_filename, frame.f_lineno - ) - self.context = c + return defer_to_threadpool(reactor, reactor.getThreadPool(), f, *args, **kwargs) - return self + +def defer_to_threadpool(reactor, threadpool, f, *args, **kwargs): + """ + A wrapper for twisted.internet.threads.deferToThreadpool, which handles + logcontexts correctly. + + Calls the function `f` using a thread from the given threadpool and returns + the result as a Deferred. + + Creates a new logcontext for `f`, which is created as a child of the current + logcontext (so its CPU usage metrics will get attributed to the current + logcontext). `f` should preserve the logcontext it is given. + + The result deferred follows the Synapse logcontext rules: you should `yield` + on it. + + Args: + reactor (twisted.internet.base.ReactorBase): The reactor in whose main thread + the Deferred will be invoked. Normally this will be hs.get_reactor(). + + threadpool (twisted.python.threadpool.ThreadPool): The threadpool to use for + running `f`. Normally this will be hs.get_reactor().getThreadPool(). + + f (callable): The function to call. + + args: positional arguments to pass to f. + + kwargs: keyword arguments to pass to f. + + Returns: + Deferred: A Deferred which fires a callback with the result of `f`, or an + errback if `f` throws an exception. + """ + logcontext = LoggingContext.current_context() + + def g(): + with LoggingContext(parent_context=logcontext): + return f(*args, **kwargs) + + return make_deferred_yieldable( + threads.deferToThreadPool(reactor, threadpool, g) + ) -- cgit 1.4.1 From ef771cc4c2d988e5188ba7e75df9adebb0ebafe1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 24 Oct 2018 10:35:01 +0100 Subject: Fix a number of flake8 errors Broadly three things here: * disable W504 which seems a bit whacko * remove a bunch of `as e` expressions from exception handlers that don't use them * use `r""` for strings which include backslashes Also, we don't use pep8 any more, so we can get rid of the duplicate config there. --- changelog.d/4082.misc | 1 + scripts-dev/tail-synapse.py | 2 +- scripts/synapse_port_db | 3 ++- setup.cfg | 17 ++++++++--------- synapse/config/repository.py | 2 +- synapse/crypto/keyclient.py | 2 +- synapse/federation/federation_server.py | 2 +- synapse/rest/client/v2_alpha/auth.py | 2 +- synapse/rest/media/v1/preview_url_resource.py | 2 +- synapse/storage/registration.py | 2 +- tests/config/test_generate.py | 2 +- tests/events/test_utils.py | 4 ++-- 12 files changed, 21 insertions(+), 20 deletions(-) create mode 100644 changelog.d/4082.misc (limited to 'synapse/rest') diff --git a/changelog.d/4082.misc b/changelog.d/4082.misc new file mode 100644 index 0000000000..a81faf5e9b --- /dev/null +++ b/changelog.d/4082.misc @@ -0,0 +1 @@ +Clean up some bits of code which were flagged by the linter diff --git a/scripts-dev/tail-synapse.py b/scripts-dev/tail-synapse.py index 1a36b94038..7c9985d9f0 100644 --- a/scripts-dev/tail-synapse.py +++ b/scripts-dev/tail-synapse.py @@ -48,7 +48,7 @@ def main(): row.name: row.position for row in replicate(server, {"streams": "-1"})["streams"].rows } - except requests.exceptions.ConnectionError as e: + except requests.exceptions.ConnectionError: time.sleep(0.1) while True: diff --git a/scripts/synapse_port_db b/scripts/synapse_port_db index 2f6e69e552..3c7b606323 100755 --- a/scripts/synapse_port_db +++ b/scripts/synapse_port_db @@ -501,7 +501,8 @@ class Porter(object): try: yield self.postgres_store.runInteraction("alter_table", alter_table) - except Exception as e: + except Exception: + # On Error Resume Next pass yield self.postgres_store.runInteraction( diff --git a/setup.cfg b/setup.cfg index 52feaa9cc7..b6b4aa740d 100644 --- a/setup.cfg +++ b/setup.cfg @@ -14,17 +14,16 @@ ignore = pylint.cfg tox.ini -[pep8] -max-line-length = 90 -# W503 requires that binary operators be at the end, not start, of lines. Erik -# doesn't like it. E203 is contrary to PEP8. E731 is silly. -ignore = W503,E203,E731 - [flake8] -# note that flake8 inherits the "ignore" settings from "pep8" (because it uses -# pep8 to do those checks), but not the "max-line-length" setting max-line-length = 90 -ignore=W503,E203,E731 + +# see https://pycodestyle.readthedocs.io/en/latest/intro.html#error-codes +# for error codes. The ones we ignore are: +# W503: line break before binary operator +# W504: line break after binary operator +# E203: whitespace before ':' (which is contrary to pep8?) +# E731: do not assign a lambda expression, use a def +ignore=W503,W504,E203,E731 [isort] line_length = 89 diff --git a/synapse/config/repository.py b/synapse/config/repository.py index fc909c1fac..06c62ab62c 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -178,7 +178,7 @@ class ContentRepositoryConfig(Config): def default_config(self, **kwargs): media_store = self.default_path("media_store") uploads_path = self.default_path("uploads") - return """ + return r""" # Directory where uploaded images and attachments are stored. media_store_path: "%(media_store)s" diff --git a/synapse/crypto/keyclient.py b/synapse/crypto/keyclient.py index 57d4665e84..080c81f14b 100644 --- a/synapse/crypto/keyclient.py +++ b/synapse/crypto/keyclient.py @@ -55,7 +55,7 @@ def fetch_server_key(server_name, tls_client_options_factory, path=KEY_API_V1): raise IOError("Cannot get key for %r" % server_name) except (ConnectError, DomainError) as e: logger.warn("Error getting key for %r: %s", server_name, e) - except Exception as e: + except Exception: logger.exception("Error getting key for %r", server_name) raise IOError("Cannot get key for %r" % server_name) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 4efe95faa4..af0107a46e 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -800,7 +800,7 @@ class FederationHandlerRegistry(object): yield handler(origin, content) except SynapseError as e: logger.info("Failed to handle edu %r: %r", edu_type, e) - except Exception as e: + except Exception: logger.exception("Failed to handle edu %r", edu_type) def on_query(self, query_type, args): diff --git a/synapse/rest/client/v2_alpha/auth.py b/synapse/rest/client/v2_alpha/auth.py index bd8b5f4afa..693b303881 100644 --- a/synapse/rest/client/v2_alpha/auth.py +++ b/synapse/rest/client/v2_alpha/auth.py @@ -99,7 +99,7 @@ class AuthRestServlet(RestServlet): cannot be handled in the normal flow (with requests to the same endpoint). Current use is for web fallback auth. """ - PATTERNS = client_v2_patterns("/auth/(?P[\w\.]*)/fallback/web") + PATTERNS = client_v2_patterns(r"/auth/(?P[\w\.]*)/fallback/web") def __init__(self, hs): super(AuthRestServlet, self).__init__() diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py index 8c892ff187..1a7bfd6b56 100644 --- a/synapse/rest/media/v1/preview_url_resource.py +++ b/synapse/rest/media/v1/preview_url_resource.py @@ -674,7 +674,7 @@ def summarize_paragraphs(text_nodes, min_size=200, max_size=500): # This splits the paragraph into words, but keeping the # (preceeding) whitespace intact so we can easily concat # words back together. - for match in re.finditer("\s*\S+", description): + for match in re.finditer(r"\s*\S+", description): word = match.group() # Keep adding words while the total length is less than diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 26b429e307..2dd14aba1c 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -567,7 +567,7 @@ class RegistrationStore(RegistrationWorkerStore, def _find_next_generated_user_id(txn): txn.execute("SELECT name FROM users") - regex = re.compile("^@(\d+):") + regex = re.compile(r"^@(\d+):") found = set() diff --git a/tests/config/test_generate.py b/tests/config/test_generate.py index f88d28a19d..0c23068bcf 100644 --- a/tests/config/test_generate.py +++ b/tests/config/test_generate.py @@ -67,6 +67,6 @@ class ConfigGenerationTestCase(unittest.TestCase): with open(log_config_file) as f: config = f.read() # find the 'filename' line - matches = re.findall("^\s*filename:\s*(.*)$", config, re.M) + matches = re.findall(r"^\s*filename:\s*(.*)$", config, re.M) self.assertEqual(1, len(matches)) self.assertEqual(matches[0], expected) diff --git a/tests/events/test_utils.py b/tests/events/test_utils.py index ff217ca8b9..d0cc492deb 100644 --- a/tests/events/test_utils.py +++ b/tests/events/test_utils.py @@ -156,7 +156,7 @@ class SerializeEventTestCase(unittest.TestCase): room_id="!foo:bar", content={"key.with.dots": {}}, ), - ["content.key\.with\.dots"], + [r"content.key\.with\.dots"], ), {"content": {"key.with.dots": {}}}, ) @@ -172,7 +172,7 @@ class SerializeEventTestCase(unittest.TestCase): "nested.dot.key": {"leaf.key": 42, "not_me_either": 1}, }, ), - ["content.nested\.dot\.key.leaf\.key"], + [r"content.nested\.dot\.key.leaf\.key"], ), {"content": {"nested.dot.key": {"leaf.key": 42}}}, ) -- cgit 1.4.1