diff options
| -rw-r--r-- | CHANGES.md | 17 | ||||
| -rw-r--r-- | debian/changelog | 8 | ||||
| -rw-r--r-- | docs/modules/spam_checker_callbacks.md | 31 | ||||
| -rw-r--r-- | docs/upgrade.md | 8 | ||||
| -rw-r--r-- | pyproject.toml | 2 | ||||
| -rw-r--r-- | synapse/events/spamcheck.py | 47 | ||||
| -rw-r--r-- | synapse/federation/federation_base.py | 3 | ||||
| -rw-r--r-- | synapse/handlers/message.py | 28 | ||||
| -rw-r--r-- | synapse/module_api/__init__.py | 8 | ||||
| -rw-r--r-- | synapse/spam_checker_api/__init__.py | 25 |
10 files changed, 91 insertions, 86 deletions
diff --git a/CHANGES.md b/CHANGES.md index 40e362e920..2bf8cdea75 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -1,5 +1,5 @@ -Synapse 1.60.0rc2 (2022-05-27) -============================== +Synapse 1.60.0 (2022-05-31) +=========================== This release of Synapse adds a unique index to the `state_group_edges` table, in order to prevent accidentally introducing duplicate information (for example, @@ -14,6 +14,15 @@ should update their modules to use the new signature where possible. See [the upgrade notes](https://github.com/matrix-org/synapse/blob/develop/docs/upgrade.md#upgrading-to-v1600) for more details. +Bugfixes +-------- + +- Fix a bug introduced in Synapse 1.60.0rc1 that would break some imports from `synapse.module_api`. ([\#12918](https://github.com/matrix-org/synapse/issues/12918)) + + +Synapse 1.60.0rc2 (2022-05-27) +============================== + Features -------- @@ -60,6 +69,7 @@ Bugfixes - Fix a bug introduced in Synapse 1.30.0 where empty rooms could be automatically created if a monthly active users limit is set. ([\#12713](https://github.com/matrix-org/synapse/issues/12713)) - Fix push to dismiss notifications when read on another client. Contributed by @SpiritCroc @ Beeper. ([\#12721](https://github.com/matrix-org/synapse/issues/12721)) - Fix poor database performance when reading the cache invalidation stream for large servers with lots of workers. ([\#12747](https://github.com/matrix-org/synapse/issues/12747)) +- Fix a long-standing bug where the user directory background process would fail to make forward progress if a user included a null codepoint in their display name or avatar. ([\#12762](https://github.com/matrix-org/synapse/issues/12762)) - Delete events from the `federation_inbound_events_staging` table when a room is purged through the admin API. ([\#12770](https://github.com/matrix-org/synapse/issues/12770)) - Give a meaningful error message when a client tries to create a room with an invalid alias localpart. ([\#12779](https://github.com/matrix-org/synapse/issues/12779)) - Fix a bug introduced in 1.43.0 where a file (`providers.json`) was never closed. Contributed by @arkamar. ([\#12794](https://github.com/matrix-org/synapse/issues/12794)) @@ -115,7 +125,6 @@ Internal Changes - Drop the logging level of status messages for the URL preview cache expiry job from INFO to DEBUG. ([\#12720](https://github.com/matrix-org/synapse/issues/12720)) - Downgrade some OIDC errors to warnings in the logs, to reduce the noise of Sentry reports. ([\#12723](https://github.com/matrix-org/synapse/issues/12723)) - Update configs used by Complement to allow more invites/3PID validations during tests. ([\#12731](https://github.com/matrix-org/synapse/issues/12731)) -- Fix a long-standing bug where the user directory background process would fail to make forward progress if a user included a null codepoint in their display name or avatar. ([\#12762](https://github.com/matrix-org/synapse/issues/12762)) - Tweak the mypy plugin so that `@cached` can accept `on_invalidate=None`. ([\#12769](https://github.com/matrix-org/synapse/issues/12769)) - Move methods that call `add_push_rule` to the `PushRuleStore` class. ([\#12772](https://github.com/matrix-org/synapse/issues/12772)) - Make handling of federation Authorization header (more) compliant with RFC7230. ([\#12774](https://github.com/matrix-org/synapse/issues/12774)) @@ -222,7 +231,7 @@ Deprecations and Removals ------------------------- - Remove unstable identifiers from [MSC3069](https://github.com/matrix-org/matrix-doc/pull/3069). ([\#12596](https://github.com/matrix-org/synapse/issues/12596)) -- Remove the unspecified `m.login.jwt` login type and the unstable `uk.half-shot.msc2778.login.application_service` from +- Remove the unspecified `m.login.jwt` login type and the unstable `uk.half-shot.msc2778.login.application_service` from [MSC2778](https://github.com/matrix-org/matrix-doc/pull/2778). ([\#12597](https://github.com/matrix-org/synapse/issues/12597)) - Synapse now requires at least Python 3.7.1 (up from 3.7.0), for compatibility with the latest Twisted trunk. ([\#12613](https://github.com/matrix-org/synapse/issues/12613)) diff --git a/debian/changelog b/debian/changelog index b1e61e7c8a..8f08972743 100644 --- a/debian/changelog +++ b/debian/changelog @@ -1,10 +1,16 @@ -matrix-synapse-py3 (1.60.0~rc2+nmu1) UNRELEASED; urgency=medium +matrix-synapse-py3 (1.61.0~rc1+nmu1) UNRELEASED; urgency=medium * Non-maintainer upload. * Remove unused `jitsimeetbridge` experiment from `contrib` directory. -- Synapse Packaging team <packages@matrix.org> Sun, 29 May 2022 14:44:45 +0100 +matrix-synapse-py3 (1.60.0) stable; urgency=medium + + * New Synapse release 1.60.0. + + -- Synapse Packaging team <packages@matrix.org> Tue, 31 May 2022 13:41:22 +0100 + matrix-synapse-py3 (1.60.0~rc2) stable; urgency=medium * New Synapse release 1.60.0rc2. diff --git a/docs/modules/spam_checker_callbacks.md b/docs/modules/spam_checker_callbacks.md index 71f6f9f0ab..ad35e667ed 100644 --- a/docs/modules/spam_checker_callbacks.md +++ b/docs/modules/spam_checker_callbacks.md @@ -11,29 +11,28 @@ The available spam checker callbacks are: ### `check_event_for_spam` _First introduced in Synapse v1.37.0_ -_Signature extended to support Allow and Code in Synapse v1.60.0_ -_Boolean and string return value types deprecated in Synapse v1.60.0_ + +_Changed in Synapse v1.60.0: `synapse.module_api.NOT_SPAM` and `synapse.module_api.errors.Codes` can be returned by this callback. Returning a boolean or a string is now deprecated._ ```python -async def check_event_for_spam(event: "synapse.module_api.EventBase") -> Union["synapse.module_api.ALLOW", "synapse.module_api.error.Codes", str, bool] +async def check_event_for_spam(event: "synapse.module_api.EventBase") -> Union["synapse.module_api.NOT_SPAM", "synapse.module_api.errors.Codes", str, bool] ``` -Called when receiving an event from a client or via federation. The callback must return either: - - `synapse.module_api.ALLOW`, to allow the operation. Other callbacks - may still decide to reject it. - - `synapse.api.Codes` to reject the operation with an error code. In case - of doubt, `synapse.api.error.Codes.FORBIDDEN` is a good error code. - - (deprecated) a `str` to reject the operation and specify an error message. Note that clients +Called when receiving an event from a client or via federation. The callback must return one of: + - `synapse.module_api.NOT_SPAM`, to allow the operation. Other callbacks may still + decide to reject it. + - `synapse.module_api.errors.Codes` to reject the operation with an error code. In case + of doubt, `synapse.module_api.errors.Codes.FORBIDDEN` is a good error code. + - (deprecated) a non-`Codes` `str` to reject the operation and specify an error message. Note that clients typically will not localize the error message to the user's preferred locale. - - (deprecated) on `False`, behave as `ALLOW`. Deprecated as confusing, as some - callbacks in expect `True` to allow and others `True` to reject. - - (deprecated) on `True`, behave as `synapse.api.error.Codes.FORBIDDEN`. Deprecated as confusing, as - some callbacks in expect `True` to allow and others `True` to reject. + - (deprecated) `False`, which is the same as returning `synapse.module_api.NOT_SPAM`. + - (deprecated) `True`, which is the same as returning `synapse.module_api.errors.Codes.FORBIDDEN`. If multiple modules implement this callback, they will be considered in order. If a -callback returns `synapse.module_api.ALLOW`, Synapse falls through to the next one. The value of the -first callback that does not return `synapse.module_api.ALLOW` will be used. If this happens, Synapse -will not call any of the subsequent implementations of this callback. +callback returns `synapse.module_api.NOT_SPAM`, Synapse falls through to the next one. +The value of the first callback that does not return `synapse.module_api.NOT_SPAM` will +be used. If this happens, Synapse will not call any of the subsequent implementations of +this callback. ### `user_may_join_room` diff --git a/docs/upgrade.md b/docs/upgrade.md index e7eadadb64..e3c64da17f 100644 --- a/docs/upgrade.md +++ b/docs/upgrade.md @@ -177,11 +177,11 @@ has queries that can be used to check a database for this problem in advance. </details> -## SpamChecker API's `check_event_for_spam` has a new signature. +## New signature for the spam checker callback `check_event_for_spam` The previous signature has been deprecated. -Whereas `check_event_for_spam` callbacks used to return `Union[str, bool]`, they should now return `Union["synapse.module_api.Allow", "synapse.module_api.errors.Codes"]`. +Whereas `check_event_for_spam` callbacks used to return `Union[str, bool]`, they should now return `Union["synapse.module_api.NOT_SPAM", "synapse.module_api.errors.Codes"]`. This is part of an ongoing refactoring of the SpamChecker API to make it less ambiguous and more powerful. @@ -204,8 +204,8 @@ async def check_event_for_spam(event): # Event is spam, mark it as forbidden (you may use some more precise error # code if it is useful). return synapse.module_api.errors.Codes.FORBIDDEN - # Event is not spam, mark it as `ALLOW`. - return synapse.module_api.ALLOW + # Event is not spam, mark it as such. + return synapse.module_api.NOT_SPAM ``` # Upgrading to v1.59.0 diff --git a/pyproject.toml b/pyproject.toml index 59cff590b5..75251c863d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,7 +54,7 @@ skip_gitignore = true [tool.poetry] name = "matrix-synapse" -version = "1.60.0rc2" +version = "1.60.0" description = "Homeserver for the Matrix decentralised comms protocol" authors = ["Matrix.org Team and Contributors <packages@matrix.org>"] license = "Apache-2.0" diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index 82998ca490..d2e06c754e 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -31,7 +31,7 @@ from typing import ( from synapse.api.errors import Codes from synapse.rest.media.v1._base import FileInfo from synapse.rest.media.v1.media_storage import ReadableFileWrapper -from synapse.spam_checker_api import Allow, Decision, RegistrationBehaviour +from synapse.spam_checker_api import RegistrationBehaviour from synapse.types import RoomAlias, UserProfile from synapse.util.async_helpers import delay_cancellation, maybe_awaitable from synapse.util.metrics import Measure @@ -46,7 +46,7 @@ CHECK_EVENT_FOR_SPAM_CALLBACK = Callable[ ["synapse.events.EventBase"], Awaitable[ Union[ - Allow, + str, Codes, # Highly experimental, not officially part of the spamchecker API, may # disappear without warning depending on the results of ongoing @@ -55,8 +55,6 @@ CHECK_EVENT_FOR_SPAM_CALLBACK = Callable[ Tuple[Codes, Dict], # Deprecated bool, - # Deprecated - str, ] ], ] @@ -183,6 +181,8 @@ def load_legacy_spam_checkers(hs: "synapse.server.HomeServer") -> None: class SpamChecker: + NOT_SPAM = "NOT_SPAM" + def __init__(self, hs: "synapse.server.HomeServer") -> None: self.hs = hs self.clock = hs.get_clock() @@ -275,7 +275,7 @@ class SpamChecker: async def check_event_for_spam( self, event: "synapse.events.EventBase" - ) -> Union[Decision, Tuple[Codes, Dict], str]: + ) -> Union[Tuple[Codes, Dict], str]: """Checks if a given event is considered "spammy" by this server. If the server considers an event spammy, then it will be rejected if @@ -286,22 +286,20 @@ class SpamChecker: event: the event to be checked Returns: - - on `ALLOW`, the event is considered good (non-spammy) and should - be let through. Other spamcheck filters may still reject it. - - on `Code`, the event is considered spammy and is rejected with a specific + - `NOT_SPAM` if the event is considered good (non-spammy) and should be let + through. Other spamcheck filters may still reject it. + - A `Code` if the event is considered spammy and is rejected with a specific error message/code. - - on `str`, the event is considered spammy and the string is used as error - message. This usage is generally discouraged as it doesn't support - internationalization. + - A string that isn't `NOT_SPAM` if the event is considered spammy and the + string should be used as the client-facing error message. This usage is + generally discouraged as it doesn't support internationalization. """ for callback in self._check_event_for_spam_callbacks: with Measure( self.clock, "{}.{}".format(callback.__module__, callback.__qualname__) ): - res: Union[ - Decision, Tuple[Codes, Dict], str, bool - ] = await delay_cancellation(callback(event)) - if res is False or res is Allow.ALLOW: + res = await delay_cancellation(callback(event)) + if res is False or res == self.NOT_SPAM: # This spam-checker accepts the event. # Other spam-checkers may reject it, though. continue @@ -309,14 +307,23 @@ class SpamChecker: # This spam-checker rejects the event with deprecated # return value `True` return Codes.FORBIDDEN + elif not isinstance(res, str): + # mypy complains that we can't reach this code because of the + # return type in CHECK_EVENT_FOR_SPAM_CALLBACK, but we don't know + # for sure that the module actually returns it. + logger.warning( + "Module returned invalid value, rejecting message as spam" + ) + res = "This message has been rejected as probable spam" else: - # This spam-checker rejects the event either with a `str`, - # with a `Codes` or with a `Tuple[Codes, Dict]`. In either - # case, we stop here. - return res + # The module rejected the event either with a `Codes` + # or some other `str`. In either case, we stop here. + pass + + return res # No spam-checker has rejected the event, let it pass. - return Allow.ALLOW + return self.NOT_SPAM async def should_drop_federated_event( self, event: "synapse.events.EventBase" diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py index 1e866b19d8..7bc54b9988 100644 --- a/synapse/federation/federation_base.py +++ b/synapse/federation/federation_base.py @@ -15,7 +15,6 @@ import logging from typing import TYPE_CHECKING -import synapse from synapse.api.constants import MAX_DEPTH, EventContentFields, EventTypes, Membership from synapse.api.errors import Codes, SynapseError from synapse.api.room_versions import EventFormatVersions, RoomVersion @@ -101,7 +100,7 @@ class FederationBase: spam_check = await self.spam_checker.check_event_for_spam(pdu) - if spam_check is not synapse.spam_checker_api.Allow.ALLOW: + if spam_check != self.spam_checker.NOT_SPAM: logger.warning("Event contains spam, soft-failing %s", pdu.event_id) # we redact (to save disk space) as well as soft-failing (to stop # using the event in prev_events). diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index f377769071..cf7c2d1979 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -23,7 +23,6 @@ from canonicaljson import encode_canonical_json from twisted.internet.interfaces import IDelayedCall -import synapse from synapse import event_auth from synapse.api.constants import ( EventContentFields, @@ -897,11 +896,11 @@ class EventCreationHandler: event.sender, ) - spam_check = await self.spam_checker.check_event_for_spam(event) - if spam_check is not synapse.spam_checker_api.Allow.ALLOW: - if isinstance(spam_check, tuple): + spam_check_result = await self.spam_checker.check_event_for_spam(event) + if spam_check_result != self.spam_checker.NOT_SPAM: + if isinstance(spam_check_result, tuple): try: - [code, dict] = spam_check + [code, dict] = spam_check_result raise SynapseError( 403, "This message had been rejected as probable spam", @@ -911,11 +910,24 @@ class EventCreationHandler: except ValueError: logger.error( "Spam-check module returned invalid error value. Expecting [code, dict], got %s", - spam_check, + spam_check_result, ) - spam_check = Codes.FORBIDDEN + spam_check_result = Codes.FORBIDDEN + + if isinstance(spam_check_result, Codes): + raise SynapseError( + 403, + "This message has been rejected as probable spam", + spam_check_result, + ) + + # Backwards compatibility: if the return value is not an error code, it + # means the module returned an error message to be included in the + # SynapseError (which is now deprecated). raise SynapseError( - 403, "This message had been rejected as probable spam", spam_check + 403, + spam_check_result, + Codes.FORBIDDEN, ) ev = await self.handle_new_client_event( diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index 6668f64c90..b7451fc870 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -35,7 +35,6 @@ from typing_extensions import ParamSpec from twisted.internet import defer from twisted.web.resource import Resource -from synapse import spam_checker_api from synapse.api.errors import SynapseError from synapse.events import EventBase from synapse.events.presence_router import ( @@ -55,6 +54,7 @@ from synapse.events.spamcheck import ( USER_MAY_JOIN_ROOM_CALLBACK, USER_MAY_PUBLISH_ROOM_CALLBACK, USER_MAY_SEND_3PID_INVITE_CALLBACK, + SpamChecker, ) from synapse.events.third_party_rules import ( CHECK_CAN_DEACTIVATE_USER_CALLBACK, @@ -140,9 +140,7 @@ are loaded into Synapse. """ PRESENCE_ALL_USERS = PresenceRouter.ALL_USERS - -ALLOW = spam_checker_api.Allow.ALLOW -# Singleton value used to mark a message as permitted. +NOT_SPAM = SpamChecker.NOT_SPAM __all__ = [ "errors", @@ -151,7 +149,7 @@ __all__ = [ "respond_with_html", "run_in_background", "cached", - "Allow", + "NOT_SPAM", "UserID", "DatabasePool", "LoggingTransaction", diff --git a/synapse/spam_checker_api/__init__.py b/synapse/spam_checker_api/__init__.py index 95132c80b7..75578270ac 100644 --- a/synapse/spam_checker_api/__init__.py +++ b/synapse/spam_checker_api/__init__.py @@ -12,9 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. from enum import Enum -from typing import Union - -from synapse.api.errors import Codes class RegistrationBehaviour(Enum): @@ -25,25 +22,3 @@ class RegistrationBehaviour(Enum): ALLOW = "allow" SHADOW_BAN = "shadow_ban" DENY = "deny" - - -# We define the following singleton enum rather than a string to be able to -# write `Union[Allow, ..., str]` in some of the callbacks for the spam-checker -# API, where the `str` is required to maintain backwards compatibility with -# previous versions of the API. -class Allow(Enum): - """ - Singleton to allow events to pass through in SpamChecker APIs. - """ - - ALLOW = "allow" - - -Decision = Union[Allow, Codes] -""" -Union to define whether a request should be allowed or rejected. - -To accept a request, return `ALLOW`. - -To reject a request without any specific information, use `Codes.FORBIDDEN`. -""" |