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

Remove unused federation endpoint (query_auth) #7026

Merged
merged 4 commits into from
Mar 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog.d/7026.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove the unused query_auth federation endpoint per MSC2451.
82 changes: 0 additions & 82 deletions synapse/federation/federation_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand All @@ -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]
Expand Down
80 changes: 79 additions & 1 deletion synapse/federation/federation_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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
Expand Down Expand Up @@ -345,6 +346,83 @@ async def get_room_state_ids(

return state_event_ids, auth_event_ids

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
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, # type: ignore
Copy link
Member Author

Choose a reason for hiding this comment

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

Without ignoring this for mypy it complains:

synapse/federation/federation_client.py:399: error: Argument "room_version" to "get_pdu" of "FederationClient" has incompatible type "str"; expected "RoomVersion"  [arg-type]

I attempted to fix this in e5e5b4c, but that seems to break SyTest. I'm not 100% confident whether mypy is in error or the code is, although I did trace through it and it seems like the code is wrong. 🤷‍♂

Copy link
Member Author

Choose a reason for hiding this comment

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

(Or maybe the typing is wrong on one of these functions.)

Copy link
Member

Choose a reason for hiding this comment

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

uh yeah, mypy has a point: this doesn't look right.

It's a bit of an edge-case so I can believe it's been broken for a few weeks without us noticing. Looks like it got broken in f84700f, for the same reason that 0cb0c7b: I didn't find this use of get_pdu when searching.

To fix it, it looks like it should be easy enough to pass a RoomVersion object into _check_sigs_and_hash_and_fetch instead of a str? Don't know if you want to do that in this PR or another.

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'd like to do it in a follow-up if that's OK. I think this is what my commit above does and it ends up being a bit more invasive than expected.

Copy link
Member Author

@clokep clokep Mar 17, 2020

Choose a reason for hiding this comment

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

See #7089. This is a rebase of e5e5b4c (with a newsfile a couple of other things).

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 = await 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)

Expand Down
51 changes: 0 additions & 51 deletions synapse/federation/federation_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 0 additions & 12 deletions synapse/federation/transport/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -643,17 +643,6 @@ async def on_POST(self, origin, content, query):
return 200, response


class FederationQueryAuthServlet(BaseFederationServlet):
PATH = "/query_auth/(?P<context>[^/]*)/(?P<event_id>[^/]*)"

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<room_id>[^/]*)/?"
Expand Down Expand Up @@ -1412,7 +1401,6 @@ async def on_GET(self, origin, content, query, room_id):
FederationV2SendLeaveServlet,
FederationV1InviteServlet,
FederationV2InviteServlet,
FederationQueryAuthServlet,
FederationGetMissingEventsServlet,
FederationEventAuthServlet,
FederationClientKeysQueryServlet,
Expand Down