diff options
| -rw-r--r-- | changelog.d/12568.misc | 1 | ||||
| -rw-r--r-- | synapse/events/presence_router.py | 12 | ||||
| -rw-r--r-- | synapse/events/spamcheck.py | 36 | ||||
| -rw-r--r-- | synapse/events/third_party_rules.py | 36 | ||||
| -rw-r--r-- | synapse/handlers/account_validity.py | 3 | ||||
| -rw-r--r-- | synapse/handlers/auth.py | 25 |
6 files changed, 86 insertions, 27 deletions
diff --git a/changelog.d/12568.misc b/changelog.d/12568.misc new file mode 100644 index 0000000000..f64dc67c4f --- /dev/null +++ b/changelog.d/12568.misc @@ -0,0 +1 @@ +Protect module callbacks with read semantics against cancellation. diff --git a/synapse/events/presence_router.py b/synapse/events/presence_router.py index 8437ce52dc..bb4a6bd957 100644 --- a/synapse/events/presence_router.py +++ b/synapse/events/presence_router.py @@ -28,8 +28,10 @@ from typing import ( from typing_extensions import ParamSpec +from twisted.internet.defer import CancelledError + from synapse.api.presence import UserPresenceState -from synapse.util.async_helpers import maybe_awaitable +from synapse.util.async_helpers import delay_cancellation, maybe_awaitable if TYPE_CHECKING: from synapse.server import HomeServer @@ -158,7 +160,9 @@ class PresenceRouter: try: # Note: result is an object here, because we don't trust modules to # return the types they're supposed to. - result: object = await callback(state_updates) + result: object = await delay_cancellation(callback(state_updates)) + except CancelledError: + raise except Exception as e: logger.warning("Failed to run module API callback %s: %s", callback, e) continue @@ -210,7 +214,9 @@ class PresenceRouter: # run all the callbacks for get_interested_users and combine the results for callback in self._get_interested_users_callbacks: try: - result = await callback(user_id) + result = await delay_cancellation(callback(user_id)) + except CancelledError: + raise except Exception as e: logger.warning("Failed to run module API callback %s: %s", callback, e) continue diff --git a/synapse/events/spamcheck.py b/synapse/events/spamcheck.py index cd80fcf9d1..3b6795d40f 100644 --- a/synapse/events/spamcheck.py +++ b/synapse/events/spamcheck.py @@ -31,7 +31,7 @@ from synapse.rest.media.v1._base import FileInfo from synapse.rest.media.v1.media_storage import ReadableFileWrapper from synapse.spam_checker_api import RegistrationBehaviour from synapse.types import RoomAlias, UserProfile -from synapse.util.async_helpers import maybe_awaitable +from synapse.util.async_helpers import delay_cancellation, maybe_awaitable if TYPE_CHECKING: import synapse.events @@ -255,7 +255,7 @@ class SpamChecker: will be used as the error message returned to the user. """ for callback in self._check_event_for_spam_callbacks: - res: Union[bool, str] = await callback(event) + res: Union[bool, str] = await delay_cancellation(callback(event)) if res: return res @@ -276,7 +276,10 @@ class SpamChecker: Whether the user may join the room """ for callback in self._user_may_join_room_callbacks: - if await callback(user_id, room_id, is_invited) is False: + may_join_room = await delay_cancellation( + callback(user_id, room_id, is_invited) + ) + if may_join_room is False: return False return True @@ -297,7 +300,10 @@ class SpamChecker: True if the user may send an invite, otherwise False """ for callback in self._user_may_invite_callbacks: - if await callback(inviter_userid, invitee_userid, room_id) is False: + may_invite = await delay_cancellation( + callback(inviter_userid, invitee_userid, room_id) + ) + if may_invite is False: return False return True @@ -322,7 +328,10 @@ class SpamChecker: True if the user may send the invite, otherwise False """ for callback in self._user_may_send_3pid_invite_callbacks: - if await callback(inviter_userid, medium, address, room_id) is False: + may_send_3pid_invite = await delay_cancellation( + callback(inviter_userid, medium, address, room_id) + ) + if may_send_3pid_invite is False: return False return True @@ -339,7 +348,8 @@ class SpamChecker: True if the user may create a room, otherwise False """ for callback in self._user_may_create_room_callbacks: - if await callback(userid) is False: + may_create_room = await delay_cancellation(callback(userid)) + if may_create_room is False: return False return True @@ -359,7 +369,10 @@ class SpamChecker: True if the user may create a room alias, otherwise False """ for callback in self._user_may_create_room_alias_callbacks: - if await callback(userid, room_alias) is False: + may_create_room_alias = await delay_cancellation( + callback(userid, room_alias) + ) + if may_create_room_alias is False: return False return True @@ -377,7 +390,8 @@ class SpamChecker: True if the user may publish the room, otherwise False """ for callback in self._user_may_publish_room_callbacks: - if await callback(userid, room_id) is False: + may_publish_room = await delay_cancellation(callback(userid, room_id)) + if may_publish_room is False: return False return True @@ -400,7 +414,7 @@ class SpamChecker: for callback in self._check_username_for_spam_callbacks: # Make a copy of the user profile object to ensure the spam checker cannot # modify it. - if await callback(user_profile.copy()): + if await delay_cancellation(callback(user_profile.copy())): return True return False @@ -428,7 +442,7 @@ class SpamChecker: """ for callback in self._check_registration_for_spam_callbacks: - behaviour = await ( + behaviour = await delay_cancellation( callback(email_threepid, username, request_info, auth_provider_id) ) assert isinstance(behaviour, RegistrationBehaviour) @@ -472,7 +486,7 @@ class SpamChecker: """ for callback in self._check_media_file_for_spam_callbacks: - spam = await callback(file_wrapper, file_info) + spam = await delay_cancellation(callback(file_wrapper, file_info)) if spam: return True diff --git a/synapse/events/third_party_rules.py b/synapse/events/third_party_rules.py index ef68e20282..9f4ff9799c 100644 --- a/synapse/events/third_party_rules.py +++ b/synapse/events/third_party_rules.py @@ -14,12 +14,14 @@ import logging from typing import TYPE_CHECKING, Any, Awaitable, Callable, List, Optional, Tuple +from twisted.internet.defer import CancelledError + from synapse.api.errors import ModuleFailedException, SynapseError from synapse.events import EventBase from synapse.events.snapshot import EventContext from synapse.storage.roommember import ProfileInfo from synapse.types import Requester, StateMap -from synapse.util.async_helpers import maybe_awaitable +from synapse.util.async_helpers import delay_cancellation, maybe_awaitable if TYPE_CHECKING: from synapse.server import HomeServer @@ -263,7 +265,11 @@ class ThirdPartyEventRules: for callback in self._check_event_allowed_callbacks: try: - res, replacement_data = await callback(event, state_events) + res, replacement_data = await delay_cancellation( + callback(event, state_events) + ) + except CancelledError: + raise except SynapseError as e: # FIXME: Being able to throw SynapseErrors is relied upon by # some modules. PR #10386 accidentally broke this ability. @@ -333,8 +339,13 @@ class ThirdPartyEventRules: for callback in self._check_threepid_can_be_invited_callbacks: try: - if await callback(medium, address, state_events) is False: + threepid_can_be_invited = await delay_cancellation( + callback(medium, address, state_events) + ) + if threepid_can_be_invited is False: return False + except CancelledError: + raise except Exception as e: logger.warning("Failed to run module API callback %s: %s", callback, e) @@ -361,8 +372,13 @@ class ThirdPartyEventRules: for callback in self._check_visibility_can_be_modified_callbacks: try: - if await callback(room_id, state_events, new_visibility) is False: + visibility_can_be_modified = await delay_cancellation( + callback(room_id, state_events, new_visibility) + ) + if visibility_can_be_modified is False: return False + except CancelledError: + raise except Exception as e: logger.warning("Failed to run module API callback %s: %s", callback, e) @@ -400,8 +416,11 @@ class ThirdPartyEventRules: """ for callback in self._check_can_shutdown_room_callbacks: try: - if await callback(user_id, room_id) is False: + can_shutdown_room = await delay_cancellation(callback(user_id, room_id)) + if can_shutdown_room is False: return False + except CancelledError: + raise except Exception as e: logger.exception( "Failed to run module API callback %s: %s", callback, e @@ -422,8 +441,13 @@ class ThirdPartyEventRules: """ for callback in self._check_can_deactivate_user_callbacks: try: - if await callback(user_id, by_admin) is False: + can_deactivate_user = await delay_cancellation( + callback(user_id, by_admin) + ) + if can_deactivate_user is False: return False + except CancelledError: + raise except Exception as e: logger.exception( "Failed to run module API callback %s: %s", callback, e diff --git a/synapse/handlers/account_validity.py b/synapse/handlers/account_validity.py index 05a138410e..33e45e3a11 100644 --- a/synapse/handlers/account_validity.py +++ b/synapse/handlers/account_validity.py @@ -23,6 +23,7 @@ from synapse.api.errors import AuthError, StoreError, SynapseError from synapse.metrics.background_process_metrics import wrap_as_background_process from synapse.types import UserID from synapse.util import stringutils +from synapse.util.async_helpers import delay_cancellation if TYPE_CHECKING: from synapse.server import HomeServer @@ -150,7 +151,7 @@ class AccountValidityHandler: Whether the user has expired. """ for callback in self._is_user_expired_callbacks: - expired = await callback(user_id) + expired = await delay_cancellation(callback(user_id)) if expired is not None: return expired diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index ad41337b28..1b9050ea96 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -41,6 +41,7 @@ import pymacaroons import unpaddedbase64 from pymacaroons.exceptions import MacaroonVerificationFailedException +from twisted.internet.defer import CancelledError from twisted.web.server import Request from synapse.api.constants import LoginType @@ -67,7 +68,7 @@ from synapse.metrics.background_process_metrics import run_as_background_process from synapse.storage.roommember import ProfileInfo from synapse.types import JsonDict, Requester, UserID from synapse.util import stringutils as stringutils -from synapse.util.async_helpers import maybe_awaitable +from synapse.util.async_helpers import delay_cancellation, maybe_awaitable from synapse.util.macaroons import get_value_from_macaroon, satisfy_expiry from synapse.util.msisdn import phone_number_to_msisdn from synapse.util.stringutils import base62_encode @@ -2202,7 +2203,11 @@ class PasswordAuthProvider: # other than None (i.e. until a callback returns a success) for callback in self.auth_checker_callbacks[login_type]: try: - result = await callback(username, login_type, login_dict) + result = await delay_cancellation( + callback(username, login_type, login_dict) + ) + except CancelledError: + raise except Exception as e: logger.warning("Failed to run module API callback %s: %s", callback, e) continue @@ -2263,7 +2268,9 @@ class PasswordAuthProvider: for callback in self.check_3pid_auth_callbacks: try: - result = await callback(medium, address, password) + result = await delay_cancellation(callback(medium, address, password)) + except CancelledError: + raise except Exception as e: logger.warning("Failed to run module API callback %s: %s", callback, e) continue @@ -2345,7 +2352,7 @@ class PasswordAuthProvider: """ for callback in self.get_username_for_registration_callbacks: try: - res = await callback(uia_results, params) + res = await delay_cancellation(callback(uia_results, params)) if isinstance(res, str): return res @@ -2359,6 +2366,8 @@ class PasswordAuthProvider: callback, res, ) + except CancelledError: + raise except Exception as e: logger.error( "Module raised an exception in get_username_for_registration: %s", @@ -2388,7 +2397,7 @@ class PasswordAuthProvider: """ for callback in self.get_displayname_for_registration_callbacks: try: - res = await callback(uia_results, params) + res = await delay_cancellation(callback(uia_results, params)) if isinstance(res, str): return res @@ -2402,6 +2411,8 @@ class PasswordAuthProvider: callback, res, ) + except CancelledError: + raise except Exception as e: logger.error( "Module raised an exception in get_displayname_for_registration: %s", @@ -2429,7 +2440,7 @@ class PasswordAuthProvider: """ for callback in self.is_3pid_allowed_callbacks: try: - res = await callback(medium, address, registration) + res = await delay_cancellation(callback(medium, address, registration)) if res is False: return res @@ -2443,6 +2454,8 @@ class PasswordAuthProvider: callback, res, ) + except CancelledError: + raise except Exception as e: logger.error("Module raised an exception in is_3pid_allowed: %s", e) raise SynapseError(code=500, msg="Internal Server Error") |