From 172f264ed38e8bef857552f93114b4ee113a880b Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Mon, 28 Oct 2019 12:43:23 +0000 Subject: [PATCH 1/2] Improve signature checking on some federation APIs (#6262) Make sure that we check that events sent over /send_join, /send_leave, and /invite, are correctly signed and come from the expected servers. --- changelog.d/6262.bugfix | 1 + synapse/federation/federation_base.py | 7 ++----- synapse/federation/federation_server.py | 7 +++++++ synapse/handlers/federation.py | 20 ++++++++++++++++++-- 4 files changed, 28 insertions(+), 7 deletions(-) create mode 100644 changelog.d/6262.bugfix diff --git a/changelog.d/6262.bugfix b/changelog.d/6262.bugfix new file mode 100644 index 000000000..32687f0d2 --- /dev/null +++ b/changelog.d/6262.bugfix @@ -0,0 +1 @@ +Improve signature checking on some federation APIs. diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py index 5a1e23a14..223aace0d 100644 --- a/synapse/federation/federation_base.py +++ b/synapse/federation/federation_base.py @@ -278,9 +278,7 @@ def _check_sigs_on_pdus(keyring, room_version, pdus): pdu_to_check.sender_domain, e.getErrorMessage(), ) - # XX not really sure if these are the right codes, but they are what - # we've done for ages - raise SynapseError(400, errmsg, Codes.UNAUTHORIZED) + raise SynapseError(403, errmsg, Codes.FORBIDDEN) for p, d in zip(pdus_to_check_sender, more_deferreds): d.addErrback(sender_err, p) @@ -314,8 +312,7 @@ def _check_sigs_on_pdus(keyring, room_version, pdus): "event id %s: unable to verify signature for event id domain: %s" % (pdu_to_check.pdu.event_id, e.getErrorMessage()) ) - # XX as above: not really sure if these are the right codes - raise SynapseError(400, errmsg, Codes.UNAUTHORIZED) + raise SynapseError(403, errmsg, Codes.FORBIDDEN) for p, d in zip(pdus_to_check_event_id, more_deferreds): d.addErrback(event_err, p) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 21e52c969..5fc7c1d67 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -370,6 +370,7 @@ class FederationServer(FederationBase): pdu = event_from_pdu_json(content, format_ver) origin_host, _ = parse_server_name(origin) yield self.check_server_matches_acl(origin_host, pdu.room_id) + pdu = yield self._check_sigs_and_hash(room_version, pdu) ret_pdu = yield self.handler.on_invite_request(origin, pdu) time_now = self._clock.time_msec() return {"event": ret_pdu.get_pdu_json(time_now)} @@ -386,6 +387,9 @@ class FederationServer(FederationBase): yield self.check_server_matches_acl(origin_host, pdu.room_id) logger.debug("on_send_join_request: pdu sigs: %s", pdu.signatures) + + pdu = yield self._check_sigs_and_hash(room_version, pdu) + res_pdus = yield self.handler.on_send_join_request(origin, pdu) time_now = self._clock.time_msec() return ( @@ -421,6 +425,9 @@ class FederationServer(FederationBase): yield self.check_server_matches_acl(origin_host, pdu.room_id) logger.debug("on_send_leave_request: pdu sigs: %s", pdu.signatures) + + pdu = yield self._check_sigs_and_hash(room_version, pdu) + yield self.handler.on_send_leave_request(origin, pdu) return 200, {} diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 4b4c6c15f..488058fe6 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1222,7 +1222,6 @@ class FederationHandler(BaseHandler): Returns: Deferred[FrozenEvent] """ - if get_domain_from_id(user_id) != origin: logger.info( "Got /make_join request for user %r from different origin %s, ignoring", @@ -1280,11 +1279,20 @@ class FederationHandler(BaseHandler): event = pdu logger.debug( - "on_send_join_request: Got event: %s, signatures: %s", + "on_send_join_request from %s: Got event: %s, signatures: %s", + origin, event.event_id, event.signatures, ) + if get_domain_from_id(event.sender) != origin: + logger.info( + "Got /send_join request for user %r from different origin %s", + event.sender, + origin, + ) + raise SynapseError(403, "User not from origin", Codes.FORBIDDEN) + event.internal_metadata.outlier = False # Send this event on behalf of the origin server. # @@ -1503,6 +1511,14 @@ class FederationHandler(BaseHandler): event.signatures, ) + if get_domain_from_id(event.sender) != origin: + logger.info( + "Got /send_leave request for user %r from different origin %s", + event.sender, + origin, + ) + raise SynapseError(403, "User not from origin", Codes.FORBIDDEN) + event.internal_metadata.outlier = False context = yield self._handle_new_event(origin, event) From c482d458221945d56dec1762c27205d229255eb3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 28 Oct 2019 12:48:18 +0000 Subject: [PATCH 2/2] 1.5.0rc2 --- CHANGES.md | 18 ++++++++++++++++++ changelog.d/6247.bugfix | 1 - changelog.d/6248.misc | 1 - changelog.d/6255.misc | 1 - changelog.d/6256.bugfix | 1 - changelog.d/6262.bugfix | 1 - synapse/__init__.py | 2 +- 7 files changed, 19 insertions(+), 6 deletions(-) delete mode 100644 changelog.d/6247.bugfix delete mode 100644 changelog.d/6248.misc delete mode 100644 changelog.d/6255.misc delete mode 100644 changelog.d/6256.bugfix delete mode 100644 changelog.d/6262.bugfix diff --git a/CHANGES.md b/CHANGES.md index d438c5272..c59b139ea 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,3 +1,21 @@ +Synapse 1.5.0rc2 (2019-10-28) +============================= + +Bugfixes +-------- + +- Update list of boolean columns in `synapse_port_db`. ([\#6247](https://github.com/matrix-org/synapse/issues/6247)) +- Fix /keys/query API on workers. ([\#6256](https://github.com/matrix-org/synapse/issues/6256)) +- Improve signature checking on some federation APIs. ([\#6262](https://github.com/matrix-org/synapse/issues/6262)) + + +Internal Changes +---------------- + +- Move schema delta files to the correct data store. ([\#6248](https://github.com/matrix-org/synapse/issues/6248)) +- Small performance improvement by removing repeated config lookups in room stats calculation. ([\#6255](https://github.com/matrix-org/synapse/issues/6255)) + + Synapse 1.5.0rc1 (2019-10-24) ========================== diff --git a/changelog.d/6247.bugfix b/changelog.d/6247.bugfix deleted file mode 100644 index 3122ba0bd..000000000 --- a/changelog.d/6247.bugfix +++ /dev/null @@ -1 +0,0 @@ -Update list of boolean columns in `synapse_port_db`. diff --git a/changelog.d/6248.misc b/changelog.d/6248.misc deleted file mode 100644 index 97176bcfc..000000000 --- a/changelog.d/6248.misc +++ /dev/null @@ -1 +0,0 @@ -Move schema delta files to the correct data store. diff --git a/changelog.d/6255.misc b/changelog.d/6255.misc deleted file mode 100644 index 45bc49364..000000000 --- a/changelog.d/6255.misc +++ /dev/null @@ -1 +0,0 @@ -Small performance improvement by removing repeated config lookups in room stats calculation. diff --git a/changelog.d/6256.bugfix b/changelog.d/6256.bugfix deleted file mode 100644 index 4b619f8cf..000000000 --- a/changelog.d/6256.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix /keys/query API on workers. diff --git a/changelog.d/6262.bugfix b/changelog.d/6262.bugfix deleted file mode 100644 index 32687f0d2..000000000 --- a/changelog.d/6262.bugfix +++ /dev/null @@ -1 +0,0 @@ -Improve signature checking on some federation APIs. diff --git a/synapse/__init__.py b/synapse/__init__.py index bcc2f8c04..d0f92ffbf 100644 --- a/synapse/__init__.py +++ b/synapse/__init__.py @@ -36,7 +36,7 @@ try: except ImportError: pass -__version__ = "1.5.0rc1" +__version__ = "1.5.0rc2" if bool(os.environ.get("SYNAPSE_TEST_PATCH_LOG_CONTEXTS", False)): # We import here so that we don't have to install a bunch of deps when