From a2d6f3a5c72d4c14f822cd9a485a46d1d84c8ac6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 26 Feb 2020 13:06:52 -0500 Subject: [PATCH 1/4] Remove the unused query_auth federation endpoint. --- changelog.d/7026.removal | 1 + synapse/federation/federation_server.py | 51 ------------------------- synapse/federation/transport/server.py | 12 ------ 3 files changed, 1 insertion(+), 63 deletions(-) create mode 100644 changelog.d/7026.removal diff --git a/changelog.d/7026.removal b/changelog.d/7026.removal new file mode 100644 index 000000000000..4c8c563bb059 --- /dev/null +++ b/changelog.d/7026.removal @@ -0,0 +1 @@ +Remove the unused query_auth federation endpoint per MSC2451. diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 7f9da493267e..275b9c99d778 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -470,57 +470,6 @@ async def on_event_auth(self, origin, room_id, event_id): res = {"auth_chain": [a.get_pdu_json(time_now) for a in auth_pdus]} return 200, res - async def on_query_auth_request(self, origin, content, room_id, event_id): - """ - Content is a dict with keys:: - auth_chain (list): A list of events that give the auth chain. - missing (list): A list of event_ids indicating what the other - side (`origin`) think we're missing. - rejects (dict): A mapping from event_id to a 2-tuple of reason - string and a proof (or None) of why the event was rejected. - The keys of this dict give the list of events the `origin` has - rejected. - - Args: - origin (str) - content (dict) - event_id (str) - - Returns: - Deferred: Results in `dict` with the same format as `content` - """ - with (await self._server_linearizer.queue((origin, room_id))): - origin_host, _ = parse_server_name(origin) - await self.check_server_matches_acl(origin_host, room_id) - - room_version = await self.store.get_room_version(room_id) - - auth_chain = [ - event_from_pdu_json(e, room_version) for e in content["auth_chain"] - ] - - signed_auth = await self._check_sigs_and_hash_and_fetch( - origin, auth_chain, outlier=True, room_version=room_version.identifier - ) - - ret = await self.handler.on_query_auth( - origin, - event_id, - room_id, - signed_auth, - content.get("rejects", []), - content.get("missing", []), - ) - - time_now = self._clock.time_msec() - send_content = { - "auth_chain": [e.get_pdu_json(time_now) for e in ret["auth_chain"]], - "rejects": ret.get("rejects", []), - "missing": ret.get("missing", []), - } - - return 200, send_content - @log_function def on_query_client_keys(self, origin, content): return self.on_query_request("client_keys", content) diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index 92a9ae232041..af4595498c0b 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -643,17 +643,6 @@ async def on_POST(self, origin, content, query): return 200, response -class FederationQueryAuthServlet(BaseFederationServlet): - PATH = "/query_auth/(?P[^/]*)/(?P[^/]*)" - - async def on_POST(self, origin, content, query, context, event_id): - new_content = await self.handler.on_query_auth_request( - origin, content, context, event_id - ) - - return 200, new_content - - class FederationGetMissingEventsServlet(BaseFederationServlet): # TODO(paul): Why does this path alone end with "/?" optional? PATH = "/get_missing_events/(?P[^/]*)/?" @@ -1412,7 +1401,6 @@ async def on_GET(self, origin, content, query, room_id): FederationV2SendLeaveServlet, FederationV1InviteServlet, FederationV2InviteServlet, - FederationQueryAuthServlet, FederationGetMissingEventsServlet, FederationEventAuthServlet, FederationClientKeysQueryServlet, From 4a8a2915c712515e6048469d125bdf4727eb74aa Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 26 Feb 2020 13:07:13 -0500 Subject: [PATCH 2/4] Move the _check_sigs_and_hash_and_fetch from FederationBase to FederationClient. --- synapse/federation/federation_base.py | 82 ------------------------- synapse/federation/federation_client.py | 81 +++++++++++++++++++++++- 2 files changed, 80 insertions(+), 83 deletions(-) diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py index 190ea1fba1f2..5c991e541237 100644 --- a/synapse/federation/federation_base.py +++ b/synapse/federation/federation_base.py @@ -39,10 +39,8 @@ LoggingContext, PreserveLoggingContext, make_deferred_yieldable, - preserve_fn, ) from synapse.types import JsonDict, get_domain_from_id -from synapse.util import unwrapFirstError logger = logging.getLogger(__name__) @@ -57,86 +55,6 @@ def __init__(self, hs): self.store = hs.get_datastore() self._clock = hs.get_clock() - @defer.inlineCallbacks - def _check_sigs_and_hash_and_fetch( - self, - origin: str, - pdus: List[EventBase], - room_version: str, - outlier: bool = False, - include_none: bool = False, - ): - """Takes a list of PDUs and checks the signatures and hashs of each - one. If a PDU fails its signature check then we check if we have it in - the database and if not then request if from the originating server of - that PDU. - - If a PDU fails its content hash check then it is redacted. - - The given list of PDUs are not modified, instead the function returns - a new list. - - Args: - origin - pdu - room_version - outlier: Whether the events are outliers or not - include_none: Whether to include None in the returned list - for events that have failed their checks - - Returns: - Deferred : A list of PDUs that have valid signatures and hashes. - """ - deferreds = self._check_sigs_and_hashes(room_version, pdus) - - @defer.inlineCallbacks - def handle_check_result(pdu: EventBase, deferred: Deferred): - try: - res = yield make_deferred_yieldable(deferred) - except SynapseError: - res = None - - if not res: - # Check local db. - res = yield self.store.get_event( - pdu.event_id, allow_rejected=True, allow_none=True - ) - - if not res and pdu.origin != origin: - try: - # This should not exist in the base implementation, until - # this is fixed, ignore it for typing. See issue #6997. - res = yield defer.ensureDeferred( - self.get_pdu( # type: ignore - destinations=[pdu.origin], - event_id=pdu.event_id, - room_version=room_version, - outlier=outlier, - timeout=10000, - ) - ) - except SynapseError: - pass - - if not res: - logger.warning( - "Failed to find copy of %s with valid signature", pdu.event_id - ) - - return res - - handle = preserve_fn(handle_check_result) - deferreds2 = [handle(pdu, deferred) for pdu, deferred in zip(pdus, deferreds)] - - valid_pdus = yield make_deferred_yieldable( - defer.gatherResults(deferreds2, consumeErrors=True) - ).addErrback(unwrapFirstError) - - if include_none: - return valid_pdus - else: - return [p for p in valid_pdus if p] - def _check_sigs_and_hash(self, room_version: str, pdu: EventBase) -> Deferred: return make_deferred_yieldable( self._check_sigs_and_hashes(room_version, [pdu])[0] diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index b5538bc07a56..e7b79a0738e3 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -33,6 +33,7 @@ from prometheus_client import Counter from twisted.internet import defer +from twisted.internet.defer import Deferred from synapse.api.constants import EventTypes, Membership from synapse.api.errors import ( @@ -51,7 +52,7 @@ ) from synapse.events import EventBase, builder from synapse.federation.federation_base import FederationBase, event_from_pdu_json -from synapse.logging.context import make_deferred_yieldable +from synapse.logging.context import make_deferred_yieldable, preserve_fn from synapse.logging.utils import log_function from synapse.types import JsonDict from synapse.util import unwrapFirstError @@ -345,6 +346,84 @@ async def get_room_state_ids( return state_event_ids, auth_event_ids + @defer.inlineCallbacks + def _check_sigs_and_hash_and_fetch( + self, + origin: str, + pdus: List[EventBase], + room_version: str, + outlier: bool = False, + include_none: bool = False, + ): + """Takes a list of PDUs and checks the signatures and hashs of each + one. If a PDU fails its signature check then we check if we have it in + the database and if not then request if from the originating server of + that PDU. + + If a PDU fails its content hash check then it is redacted. + + The given list of PDUs are not modified, instead the function returns + a new list. + + Args: + origin + pdu + room_version + outlier: Whether the events are outliers or not + include_none: Whether to include None in the returned list + for events that have failed their checks + + Returns: + Deferred : A list of PDUs that have valid signatures and hashes. + """ + deferreds = self._check_sigs_and_hashes(room_version, pdus) + + @defer.inlineCallbacks + def handle_check_result(pdu: EventBase, deferred: Deferred): + try: + res = yield make_deferred_yieldable(deferred) + except SynapseError: + res = None + + if not res: + # Check local db. + res = yield self.store.get_event( + pdu.event_id, allow_rejected=True, allow_none=True + ) + + if not res and pdu.origin != origin: + try: + res = yield defer.ensureDeferred( + self.get_pdu( + destinations=[pdu.origin], + event_id=pdu.event_id, + room_version=room_version, + outlier=outlier, + timeout=10000, + ) + ) + except SynapseError: + pass + + if not res: + logger.warning( + "Failed to find copy of %s with valid signature", pdu.event_id + ) + + return res + + handle = preserve_fn(handle_check_result) + deferreds2 = [handle(pdu, deferred) for pdu, deferred in zip(pdus, deferreds)] + + valid_pdus = yield make_deferred_yieldable( + defer.gatherResults(deferreds2, consumeErrors=True) + ).addErrback(unwrapFirstError) + + if include_none: + return valid_pdus + else: + return [p for p in valid_pdus if p] + async def get_event_auth(self, destination, room_id, event_id): res = await self.transport_layer.get_event_auth(destination, room_id, event_id) From d9b4d919de7105eea9b515f8932c5efc9bdd8981 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Mon, 2 Mar 2020 15:48:25 -0500 Subject: [PATCH 3/4] Convert to async/await. --- synapse/federation/federation_client.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index e7b79a0738e3..7dd608c1a5c0 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -346,15 +346,14 @@ async def get_room_state_ids( return state_event_ids, auth_event_ids - @defer.inlineCallbacks - def _check_sigs_and_hash_and_fetch( + async def _check_sigs_and_hash_and_fetch( self, origin: str, pdus: List[EventBase], room_version: str, outlier: bool = False, include_none: bool = False, - ): + ) -> List[EventBase]: """Takes a list of PDUs and checks the signatures and hashs of each one. If a PDU fails its signature check then we check if we have it in the database and if not then request if from the originating server of @@ -415,7 +414,7 @@ def handle_check_result(pdu: EventBase, deferred: Deferred): handle = preserve_fn(handle_check_result) deferreds2 = [handle(pdu, deferred) for pdu, deferred in zip(pdus, deferreds)] - valid_pdus = yield make_deferred_yieldable( + valid_pdus = await make_deferred_yieldable( defer.gatherResults(deferreds2, consumeErrors=True) ).addErrback(unwrapFirstError) From 4d1511d2a9699006a5fd5d97b3c5055c2916919c Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Fri, 6 Mar 2020 10:32:15 -0500 Subject: [PATCH 4/4] Ignore mypy error. --- synapse/federation/federation_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index 7dd608c1a5c0..8c6b8394785f 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -396,7 +396,7 @@ def handle_check_result(pdu: EventBase, deferred: Deferred): self.get_pdu( destinations=[pdu.origin], event_id=pdu.event_id, - room_version=room_version, + room_version=room_version, # type: ignore outlier=outlier, timeout=10000, )