Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Initial APIs for storing E2E room keys #2731

Closed
wants to merge 30 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
824b339
total WIP skeleton for /room_keys API
ara4n Dec 5, 2017
e558f21
interim WIP checkin; doesn't build yet
ara4n Dec 5, 2017
7983840
make it work and fix pep8
ara4n Dec 5, 2017
602b647
document the API
ara4n Dec 5, 2017
552934e
implement /room_keys/version too (untested)
ara4n Dec 6, 2017
124efee
make /room_keys/version work
ara4n Dec 6, 2017
3e98a00
blindly incorporate PR review - needs testing & fixing
ara4n Dec 18, 2017
356829c
rename room_key_version table correctly, and fix opt args
ara4n Dec 18, 2017
59aa35b
Merge branch 'develop' into matthew/e2e_backups
ara4n Dec 24, 2017
ace298f
actually spell example right
ara4n Dec 24, 2017
a658f4e
fix factoring out of _should_replace_room_key
ara4n Dec 24, 2017
dee6849
add storage docstring; remove unused set_e2e_room_keys
ara4n Dec 24, 2017
a22968d
more docstring for the e2e_room_keys rest
ara4n Dec 24, 2017
98fcffd
add a tonne of docstring; make upload_room_keys properly assert version
ara4n Dec 27, 2017
fecfa63
add a tonne of docstring; make upload_room_keys properly assert version
ara4n Dec 27, 2017
1aa87fc
fix typos
ara4n Dec 27, 2017
68e9cfe
fix flakes
ara4n Dec 27, 2017
eb1a429
switch get_current_version_info back to being get_version_info
ara4n Dec 31, 2017
ef75daf
improve docstring
ara4n Dec 31, 2017
3a8d9ab
don't needlessly return user_id
ara4n Dec 31, 2017
7ab77f6
first cut at a UT
ara4n Dec 31, 2017
843ce19
fix idiocies and so make tests pass
ara4n Dec 31, 2017
4fdd02f
linting
ara4n Dec 31, 2017
10cec6f
implement remaining tests and make them work
ara4n Dec 31, 2017
ebfaeac
flake8
ara4n Dec 31, 2017
1976540
support DELETE /version with no args
ara4n Jan 7, 2018
4cd381d
404 nicely if you try to interact with a missing current version
ara4n Jan 7, 2018
e3bc9f2
use parse_string
ara4n Jan 7, 2018
c199970
missing import
ara4n Jan 7, 2018
f27afe1
missing import
ara4n Jan 7, 2018
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions synapse/api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class Codes(object):
THREEPID_NOT_FOUND = "M_THREEPID_NOT_FOUND"
INVALID_USERNAME = "M_INVALID_USERNAME"
SERVER_NOT_TRUSTED = "M_SERVER_NOT_TRUSTED"
WRONG_ROOM_KEYS_VERSION = "M_WRONG_ROOM_KEYS_VERSION"


class CodeMessageException(RuntimeError):
Expand Down Expand Up @@ -233,6 +234,22 @@ def error_dict(self):
)


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):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want the code, msg and errcode to be configurable?

Do we not want to force the caller to include a current_version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

have made code, msg & errcode unconfigurable, and de-optionalised current_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()
Expand Down
134 changes: 134 additions & 0 deletions synapse/handlers/e2e_room_keys.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
# -*- 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 StoreError, SynapseError, RoomKeysVersionError
from synapse.util.async import Linearizer

logger = logging.getLogger(__name__)


class E2eRoomKeysHandler(object):
def __init__(self, hs):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good to have a quick comment on the handler to say what its responsible for. A two line description would probably be good enough,

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

self.store = hs.get_datastore()
self._upload_linearizer = Linearizer("upload_room_keys_lock")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth commenting on what this protects

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


@defer.inlineCallbacks
def get_room_keys(self, user_id, version, room_id, session_id):
# 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):
yield self.store.delete_e2e_room_keys(user_id, version, room_id, session_id)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No lock?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops; added for consistency with uploads now


@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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we need to lock to ensure that we don't change version while doing the checks?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm. fixed.


# 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]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd do the loop as something like:

for room_id, entry in room_keys['rooms'].iteritems():
    for session_id in entry['sessions']:
        etc...

So that we don't have to constantly look the key up

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


yield self._upload_room_key(
user_id, version, room_id, session_id, room_key
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be tempted to do some of the different rooms concurrently, given its in a lock.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this is preemie optimisation; we don't even have a flow yet where the bulk upload is used. so have added a comment instead.


@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 e

# 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be tempted to split this logic out into a freestanding function, as its business logic that could be easily tested. It also gives a chance to document whats going on a bit.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.


# 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
)

@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
with (yield self._upload_linearizer.queue(user_id)):
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_info(
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)
2 changes: 2 additions & 0 deletions synapse/rest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
auth,
receipts,
read_marker,
room_keys,
keys,
tokenrefresh,
tags,
Expand Down Expand Up @@ -92,6 +93,7 @@ def register_servlets(client_resource, hs):
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)
Expand Down
Loading