This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Cross-signing again [1/3] #4970
Closed
Closed
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
8567ff6
implement uploading/downloading of cross-signing keys
uhoreg b233061
return the user's own user-signing key
uhoreg 5b632c8
fix typo
uhoreg 422e1e0
Merge branch 'develop' into cross-signing
uhoreg 158104c
add/fix some doc strings
uhoreg 1b94f00
use updated function name for patterns
uhoreg 723619f
add unit tests for storing/retrieving cross-signing keys
uhoreg fb88842
implement some review feedback
uhoreg f5bad36
make hidden field nullable
uhoreg 0a8d70e
Merge branch 'develop' into cross-signing
uhoreg 8f63558
make black happy
uhoreg c44c684
make flake8 happy
uhoreg File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Add support for cross-signing. | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -1,6 +1,7 @@ | ||||||
# -*- coding: utf-8 -*- | ||||||
# Copyright 2016 OpenMarket Ltd | ||||||
# Copyright 2018 New Vector Ltd | ||||||
# Copyright 2018-2019 New Vector Ltd | ||||||
# Copyright 2019 The Matrix.org Foundation C.I.C. | ||||||
# | ||||||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||||||
# you may not use this file except in compliance with the License. | ||||||
|
@@ -19,12 +20,17 @@ | |||||
from six import iteritems | ||||||
|
||||||
from canonicaljson import encode_canonical_json, json | ||||||
from signedjson.sign import SignatureVerifyException, verify_signed_json | ||||||
|
||||||
from twisted.internet import defer | ||||||
|
||||||
from synapse.api.errors import CodeMessageException, SynapseError | ||||||
from synapse.api.errors import CodeMessageException, Codes, SynapseError | ||||||
from synapse.logging.context import make_deferred_yieldable, run_in_background | ||||||
from synapse.types import UserID, get_domain_from_id | ||||||
from synapse.types import ( | ||||||
UserID, | ||||||
get_domain_from_id, | ||||||
get_verify_key_from_cross_signing_key, | ||||||
) | ||||||
from synapse.util.retryutils import NotRetryingDestination | ||||||
|
||||||
logger = logging.getLogger(__name__) | ||||||
|
@@ -46,7 +52,7 @@ def __init__(self, hs): | |||||
) | ||||||
|
||||||
@defer.inlineCallbacks | ||||||
def query_devices(self, query_body, timeout): | ||||||
def query_devices(self, query_body, timeout, from_user_id=None): | ||||||
""" Handle a device key query from a client | ||||||
|
||||||
{ | ||||||
|
@@ -64,6 +70,11 @@ def query_devices(self, query_body, timeout): | |||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
Args: | ||||||
from_user_id (str): the user making the query. This is used when | ||||||
adding cross-signing signatures to limit what signatures users | ||||||
can see. | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
""" | ||||||
device_keys_query = query_body.get("device_keys", {}) | ||||||
|
||||||
|
@@ -118,6 +129,11 @@ def query_devices(self, query_body, timeout): | |||||
r = remote_queries_not_in_cache.setdefault(domain, {}) | ||||||
r[user_id] = remote_queries[user_id] | ||||||
|
||||||
# Get cached cross-signing keys | ||||||
cross_signing_keys = yield self.query_cross_signing_keys( | ||||||
device_keys_query, from_user_id | ||||||
) | ||||||
|
||||||
# Now fetch any devices that we don't have in our cache | ||||||
@defer.inlineCallbacks | ||||||
def do_remote_query(destination): | ||||||
|
@@ -131,6 +147,14 @@ def do_remote_query(destination): | |||||
if user_id in destination_query: | ||||||
results[user_id] = keys | ||||||
|
||||||
for user_id, key in remote_result["master_keys"].items(): | ||||||
if user_id in destination_query: | ||||||
cross_signing_keys["master"][user_id] = key | ||||||
|
||||||
for user_id, key in remote_result["self_signing_keys"].items(): | ||||||
if user_id in destination_query: | ||||||
cross_signing_keys["self_signing"][user_id] = key | ||||||
|
||||||
except Exception as e: | ||||||
failures[destination] = _exception_to_failure(e) | ||||||
|
||||||
|
@@ -144,7 +168,81 @@ def do_remote_query(destination): | |||||
) | ||||||
) | ||||||
|
||||||
defer.returnValue({"device_keys": results, "failures": failures}) | ||||||
ret = {"device_keys": results, "failures": failures} | ||||||
|
||||||
for key, value in iteritems(cross_signing_keys): | ||||||
ret[key + "_keys"] = value | ||||||
|
||||||
defer.returnValue(ret) | ||||||
|
||||||
@defer.inlineCallbacks | ||||||
def query_cross_signing_keys(self, query, from_user_id=None): | ||||||
"""Get cross-signing keys for users | ||||||
|
||||||
Args: | ||||||
query (dict[string, *]): map from user_id. This function only looks | ||||||
at the dict's keys, and the values are ignored, so the query | ||||||
format used for query_devices can be used. | ||||||
from_user_id (str): the user making the query. This is used when | ||||||
adding cross-signing signatures to limit what signatures users | ||||||
can see. | ||||||
|
||||||
Returns: | ||||||
defer.Deferred: (resolves to dict[string, dict[string, dict]]): map from | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
(master|self_signing) -> map from user_id -> master key | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
""" | ||||||
master_keys = {} | ||||||
self_signing_keys = {} | ||||||
user_signing_keys = {} | ||||||
|
||||||
@defer.inlineCallbacks | ||||||
def get_cross_signing_key(user_id): | ||||||
try: | ||||||
key = yield self.store.get_e2e_cross_signing_key( | ||||||
user_id, "master", from_user_id | ||||||
) | ||||||
if key: | ||||||
master_keys[user_id] = key | ||||||
except Exception: | ||||||
pass | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. please don't swallow all exceptions with no logging or anything: this will lead to very hard-to-debug problems. |
||||||
|
||||||
try: | ||||||
key = yield self.store.get_e2e_cross_signing_key( | ||||||
user_id, "self_signing", from_user_id | ||||||
) | ||||||
if key: | ||||||
self_signing_keys[user_id] = key | ||||||
except Exception: | ||||||
pass | ||||||
|
||||||
# users can see other users' master and self-signing keys, but can | ||||||
# only see their own user-signing keys | ||||||
if from_user_id == user_id: | ||||||
try: | ||||||
key = yield self.store.get_e2e_cross_signing_key( | ||||||
user_id, "user_signing", from_user_id | ||||||
) | ||||||
if key: | ||||||
user_signing_keys[user_id] = key | ||||||
except Exception: | ||||||
pass | ||||||
|
||||||
yield make_deferred_yieldable( | ||||||
defer.gatherResults( | ||||||
[ | ||||||
run_in_background(get_cross_signing_key, user_id) | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
for user_id in query.keys() | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
] | ||||||
) | ||||||
) | ||||||
|
||||||
defer.returnValue( | ||||||
{ | ||||||
"master": master_keys, | ||||||
"self_signing": self_signing_keys, | ||||||
"user_signing": user_signing_keys, | ||||||
} | ||||||
) | ||||||
|
||||||
@defer.inlineCallbacks | ||||||
def query_local_devices(self, query): | ||||||
|
@@ -342,6 +440,105 @@ def _upload_one_time_keys_for_user( | |||||
|
||||||
yield self.store.add_e2e_one_time_keys(user_id, device_id, time_now, new_keys) | ||||||
|
||||||
@defer.inlineCallbacks | ||||||
def upload_signing_keys_for_user(self, user_id, keys): | ||||||
"""Upload signing keys for cross-signing | ||||||
|
||||||
Args: | ||||||
user_id (string): the user uploading the keys | ||||||
keys (dict[string, dict]): the signing keys | ||||||
""" | ||||||
|
||||||
# if a master key is uploaded, then check it. Otherwise, load the | ||||||
# stored master key, to check signatures on other keys | ||||||
if "master_key" in keys: | ||||||
master_key = keys["master_key"] | ||||||
|
||||||
_check_cross_signing_key(master_key, user_id, "master") | ||||||
else: | ||||||
master_key = yield self.store.get_e2e_cross_signing_key(user_id, "master") | ||||||
|
||||||
# if there is no master key, then we can't do anything, because all the | ||||||
# other cross-signing keys need to be signed by the master key | ||||||
if not master_key: | ||||||
raise SynapseError(400, "No master key available", Codes.MISSING_PARAM) | ||||||
|
||||||
master_key_id, master_verify_key = get_verify_key_from_cross_signing_key( | ||||||
master_key | ||||||
) | ||||||
|
||||||
# for the other cross-signing keys, make sure that they have valid | ||||||
# signatures from the master key | ||||||
if "self_signing_key" in keys: | ||||||
self_signing_key = keys["self_signing_key"] | ||||||
|
||||||
_check_cross_signing_key( | ||||||
self_signing_key, user_id, "self_signing", master_verify_key | ||||||
) | ||||||
|
||||||
if "user_signing_key" in keys: | ||||||
user_signing_key = keys["user_signing_key"] | ||||||
|
||||||
_check_cross_signing_key( | ||||||
user_signing_key, user_id, "user_signing", master_verify_key | ||||||
) | ||||||
|
||||||
# if everything checks out, then store the keys and send notifications | ||||||
deviceids = [] | ||||||
if "master_key" in keys: | ||||||
yield self.store.set_e2e_cross_signing_key(user_id, "master", master_key) | ||||||
deviceids.append(master_verify_key.version) | ||||||
if "self_signing_key" in keys: | ||||||
yield self.store.set_e2e_cross_signing_key( | ||||||
user_id, "self_signing", self_signing_key | ||||||
) | ||||||
deviceids.append( | ||||||
get_verify_key_from_cross_signing_key(self_signing_key)[1].version | ||||||
) | ||||||
if "user_signing_key" in keys: | ||||||
yield self.store.set_e2e_cross_signing_key( | ||||||
user_id, "user_signing", user_signing_key | ||||||
) | ||||||
# the signature stream matches the semantics that we want for | ||||||
# user-signing key updates: only the user themselves is notified of | ||||||
# their own user-signing key updates | ||||||
yield self.device_handler.notify_user_signature_update(user_id, [user_id]) | ||||||
|
||||||
# master key and self-signing key updates match the semantics of device | ||||||
# list updates: all users who share an encrypted room are notified | ||||||
if len(deviceids): | ||||||
yield self.device_handler.notify_device_update(user_id, deviceids) | ||||||
|
||||||
defer.returnValue({}) | ||||||
|
||||||
|
||||||
def _check_cross_signing_key(key, user_id, key_type, signing_key=None): | ||||||
"""Check a cross-signing key uploaded by a user. Performs some basic sanity | ||||||
checking, and ensures that it is signed, if a signature is required. | ||||||
|
||||||
Args: | ||||||
key (dict): the key data to verify | ||||||
user_id (str): the user whose key is being checked | ||||||
key_type (str): the type of key that the key should be | ||||||
signing_key (VerifyKey): (optional) the signing key that the key should | ||||||
be signed with. If omitted, signatures will not be checked. | ||||||
""" | ||||||
if ( | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
"user_id" not in key | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
or key["user_id"] != user_id | ||||||
or "usage" not in key | ||||||
or key_type not in key["usage"] | ||||||
): | ||||||
raise SynapseError(400, ("Invalid %s key" % key_type), Codes.INVALID_PARAM) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. generally we prefer to use an explicit tuple for %-interpolation:
Suggested change
|
||||||
|
||||||
if signing_key: | ||||||
try: | ||||||
verify_signed_json(key, user_id, signing_key) | ||||||
except SignatureVerifyException: | ||||||
raise SynapseError( | ||||||
400, ("Invalid signature or %s key" % key_type), Codes.INVALID_SIGNATURE | ||||||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
) | ||||||
|
||||||
|
||||||
def _exception_to_failure(e): | ||||||
if isinstance(e, CodeMessageException): | ||||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or something