1.139.2

Co-authored-by: Andrew Morgan <andrew@amorgan.xyz>
Co-authored-by: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com>
Co-authored-by: Eric Eastwood <erice@element.io>

CHANGES.md

@@ -1,3 +1,50 @@
+# Synapse 1.139.2 (2025-10-07)
+
+## Bugfixes
+
+- Fix a bug introduced in 1.139.1 where a client could receive an Internal Server Error if they set `device_keys: null` in the request to [`POST /_matrix/client/v3/keys/upload`](https://spec.matrix.org/v1.16/client-server-api/#post_matrixclientv3keysupload). ([\#19023](https://github.com/element-hq/synapse/issues/19023))
+
+
+
+
+# Synapse 1.139.1 (2025-10-07)
+
+## Security Fixes
+
+- Fix [CVE-2025-61672](https://www.cve.org/CVERecord?id=CVE-2025-61672) / [GHSA-fh66-fcv5-jjfr](https://github.com/element-hq/synapse/security/advisories/GHSA-fh66-fcv5-jjfr). Lack of validation for device keys in Synapse before 1.139.1 allows an attacker registered on the victim homeserver to degrade federation functionality, unpredictably breaking outbound federation to other homeservers. ([\#17097](https://github.com/element-hq/synapse/issues/17097))
+
+## Deprecations and Removals
+
+- Drop support for unstable field names from the long-accepted [MSC2732](https://github.com/matrix-org/matrix-spec-proposals/pull/2732) (Olm fallback keys) proposal. This change allows unit tests to pass following the security patch above. ([\#18996](https://github.com/element-hq/synapse/issues/18996))
+
+
+
+# Synapse 1.139.0 (2025-09-30)
+
+### `/register` requests from old application service implementations may break when using MAS
+
+If you are using Matrix Authentication Service (MAS), as of this release any
+Application Services that do not set `inhibit_login=true` when calling `POST
+/_matrix/client/v3/register` will receive the error
+`IO.ELEMENT.MSC4190.M_APPSERVICE_LOGIN_UNSUPPORTED` in response. Please see [the
+upgrade
+notes](https://element-hq.github.io/synapse/develop/upgrade.html#register-requests-from-old-application-service-implementations-may-break-when-using-mas)
+for more information.
+
+No significant changes since 1.139.0rc3.
+
+
+
+
+# Synapse 1.139.0rc3 (2025-09-25)
+
+## Bugfixes
+
+- Fix a bug introduced in 1.139.0rc1 where `run_coroutine_in_background(...)` incorrectly handled logcontexts, resulting in partially broken logging. ([\#18964](https://github.com/element-hq/synapse/issues/18964))
+
+
+
+
 # Synapse 1.139.0rc2 (2025-09-23)
 
 ## Internal Changes

debian/changelog

@@ -1,3 +1,27 @@
+matrix-synapse-py3 (1.139.2) stable; urgency=medium
+
+  * New Synapse release 1.139.2.
+
+ -- Synapse Packaging team <packages@matrix.org>  Tue, 07 Oct 2025 16:29:47 +0100
+
+matrix-synapse-py3 (1.139.1) stable; urgency=medium
+
+  * New Synapse release 1.139.1.
+
+ -- Synapse Packaging team <packages@matrix.org>  Tue, 07 Oct 2025 11:46:51 +0100
+
+matrix-synapse-py3 (1.139.0) stable; urgency=medium
+
+  * New Synapse release 1.139.0.
+
+ -- Synapse Packaging team <packages@matrix.org>  Tue, 30 Sep 2025 11:58:55 +0100
+
+matrix-synapse-py3 (1.139.0~rc3) stable; urgency=medium
+
+  * New Synapse release 1.139.0rc3.
+
+ -- Synapse Packaging team <packages@matrix.org>  Thu, 25 Sep 2025 12:13:23 +0100
+
 matrix-synapse-py3 (1.139.0~rc2) stable; urgency=medium
 
   * New Synapse release 1.139.0rc2.

docs/upgrade.md

@@ -125,6 +125,21 @@ Ubuntu 24.10 Oracular Oriole [has been end-of-life since 10 Jul
 2025](https://endoflife.date/ubuntu). This release drops support for Ubuntu
 24.10, and in its place adds support for Ubuntu 25.04 Plucky Puffin.
 
+## `/register` requests from old application service implementations may break when using MAS
+
+Application Services that do not set `inhibit_login=true` when calling `POST
+/_matrix/client/v3/register` will receive the error
+`IO.ELEMENT.MSC4190.M_APPSERVICE_LOGIN_UNSUPPORTED` in response. This is a
+result of [MSC4190: Device management for application
+services](https://github.com/matrix-org/matrix-spec-proposals/pull/4190) which
+adds new endpoints for application services to create encryption-ready devices
+with other than `/login` or `/register` without `inhibit_login=true`.
+
+If an application service you use starts to fail with the mentioned error,
+ensure it is up to date. If it is, then kindly let the author know that they
+need to update their implementation to call `/register` with
+`inhibit_login=true`.
+
 # Upgrading to v1.136.0
 
 ## Deprecate `run_as_background_process` exported as part of the module API interface in favor of `ModuleApi.run_as_background_process`

pyproject.toml

@@ -101,7 +101,7 @@ module-name = "synapse.synapse_rust"
 
 [tool.poetry]
 name = "matrix-synapse"
-version = "1.139.0rc2"
+version = "1.139.2"
 description = "Homeserver for the Matrix decentralised comms protocol"
 authors = ["Matrix.org Team and Contributors <packages@matrix.org>"]
 license = "AGPL-3.0-or-later"

synapse/handlers/e2e_keys.py

@@ -57,7 +57,6 @@ if TYPE_CHECKING:
 
 logger = logging.getLogger(__name__)
 
-
 ONE_TIME_KEY_UPLOAD = "one_time_key_upload_lock"
 
 
@@ -848,14 +847,22 @@ class E2eKeysHandler:
         """
         time_now = self.clock.time_msec()
 
-        # TODO: Validate the JSON to make sure it has the right keys.
         device_keys = keys.get("device_keys", None)
         if device_keys:
+            log_kv(
+                {
+                    "message": "Updating device_keys for user.",
+                    "user_id": user_id,
+                    "device_id": device_id,
+                }
+            )
             await self.upload_device_keys_for_user(
                 user_id=user_id,
                 device_id=device_id,
                 keys={"device_keys": device_keys},
             )
+        else:
+            log_kv({"message": "Did not update device_keys", "reason": "not a dict"})
 
         one_time_keys = keys.get("one_time_keys", None)
         if one_time_keys:
@@ -873,10 +880,9 @@ class E2eKeysHandler:
             log_kv(
                 {"message": "Did not update one_time_keys", "reason": "no keys given"}
             )
-        fallback_keys = keys.get("fallback_keys") or keys.get(
-            "org.matrix.msc2732.fallback_keys"
-        )
-        if fallback_keys and isinstance(fallback_keys, dict):
+
+        fallback_keys = keys.get("fallback_keys")
+        if fallback_keys:
             log_kv(
                 {
                     "message": "Updating fallback_keys for device.",
@@ -885,8 +891,6 @@ class E2eKeysHandler:
                 }
             )
             await self.store.set_e2e_fallback_keys(user_id, device_id, fallback_keys)
-        elif fallback_keys:
-            log_kv({"message": "Did not update fallback_keys", "reason": "not a dict"})
         else:
             log_kv(
                 {"message": "Did not update fallback_keys", "reason": "no keys given"}

synapse/logging/context.py

@@ -802,13 +802,15 @@ def run_in_background(
     deferred returned by the function completes.
 
     To explain how the log contexts work here:
-     - When `run_in_background` is called, the current context is stored ("original"),
-       we kick off the background task in the current context, and we restore that
-       original context before returning
-     - When the background task finishes, we don't want to leak our context into the
-       reactor which would erroneously get attached to the next operation picked up by
-       the event loop. We add a callback to the deferred which will clear the logging
-       context after it finishes and yields control back to the reactor.
+     - When `run_in_background` is called, the calling logcontext is stored
+       ("original"), we kick off the background task in the current context, and we
+       restore that original context before returning.
+     - For a completed deferred, that's the end of the story.
+     - For an incomplete deferred, when the background task finishes, we don't want to
+       leak our context into the reactor which would erroneously get attached to the
+       next operation picked up by the event loop. We add a callback to the deferred
+       which will clear the logging context after it finishes and yields control back to
+       the reactor.
 
     Useful for wrapping functions that return a deferred or coroutine, which you don't
     yield or await on (for instance because you want to pass it to
@@ -857,22 +859,36 @@ def run_in_background(
 
     # The deferred has already completed
     if d.called and not d.paused:
-        # The function should have maintained the logcontext, so we can
-        # optimise out the messing about
+        # If the function messes with logcontexts, we can assume it follows the Synapse
+        # logcontext rules (Rules for functions returning awaitables: "If the awaitable
+        # is already complete, the function returns with the same logcontext it started
+        # with."). If it function doesn't touch logcontexts at all, we can also assume
+        # the logcontext is unchanged.
+        #
+        # Either way, the function should have maintained the calling logcontext, so we
+        # can avoid messing with it further. Additionally, if the deferred has already
+        # completed, then it would be a mistake to then add a deferred callback (below)
+        # to reset the logcontext to the sentinel logcontext as that would run
+        # immediately (remember our goal is to maintain the calling logcontext when we
+        # return).
         return d
 
-    # The function may have reset the context before returning, so we need to restore it
-    # now.
+    # Since the function we called may follow the Synapse logcontext rules (Rules for
+    # functions returning awaitables: "If the awaitable is incomplete, the function
+    # clears the logcontext before returning"), the function may have reset the
+    # logcontext before returning, so we need to restore the calling logcontext now
+    # before we return ourselves.
     #
     # Our goal is to have the caller logcontext unchanged after firing off the
     # background task and returning.
     set_current_context(calling_context)
 
-    # The original logcontext will be restored when the deferred completes, but
-    # there is nothing waiting for it, so it will get leaked into the reactor (which
-    # would then get picked up by the next thing the reactor does). We therefore
-    # need to reset the logcontext here (set the `sentinel` logcontext) before
-    # yielding control back to the reactor.
+    # If the function we called is playing nice and following the Synapse logcontext
+    # rules, it will restore original calling logcontext when the deferred completes;
+    # but there is nothing waiting for it, so it will get leaked into the reactor (which
+    # would then get picked up by the next thing the reactor does). We therefore need to
+    # reset the logcontext here (set the `sentinel` logcontext) before yielding control
+    # back to the reactor.
     #
     # (If this feels asymmetric, consider it this way: we are
     # effectively forking a new thread of execution. We are
@@ -894,10 +910,9 @@ def run_coroutine_in_background(
     Useful for wrapping coroutines that you don't yield or await on (for
     instance because you want to pass it to deferred.gatherResults()).
 
-    This is a special case of `run_in_background` where we can accept a
-    coroutine directly rather than a function. We can do this because coroutines
-    do not run until called, and so calling an async function without awaiting
-    cannot change the log contexts.
+    This is a special case of `run_in_background` where we can accept a coroutine
+    directly rather than a function. We can do this because coroutines do not continue
+    running once they have yielded.
 
     This is an ergonomic helper so we can do this:
     ```python
@@ -908,33 +923,7 @@ def run_coroutine_in_background(
     run_in_background(lambda: func1(arg1))
     ```
     """
-    calling_context = current_context()
-
-    # Wrap the coroutine in a deferred, which will have the side effect of executing the
-    # coroutine in the background.
-    d = defer.ensureDeferred(coroutine)
-
-    # The function may have reset the context before returning, so we need to restore it
-    # now.
-    #
-    # Our goal is to have the caller logcontext unchanged after firing off the
-    # background task and returning.
-    set_current_context(calling_context)
-
-    # The original logcontext will be restored when the deferred completes, but
-    # there is nothing waiting for it, so it will get leaked into the reactor (which
-    # would then get picked up by the next thing the reactor does). We therefore
-    # need to reset the logcontext here (set the `sentinel` logcontext) before
-    # yielding control back to the reactor.
-    #
-    # (If this feels asymmetric, consider it this way: we are
-    # effectively forking a new thread of execution. We are
-    # probably currently within a ``with LoggingContext()`` block,
-    # which is supposed to have a single entry and exit point. But
-    # by spawning off another deferred, we are effectively
-    # adding a new exit point.)
-    d.addBoth(_set_context_cb, SENTINEL_CONTEXT)
-    return d
+    return run_in_background(lambda: coroutine)
 
 
 T = TypeVar("T")

synapse/rest/client/keys.py

@@ -23,10 +23,19 @@
 import logging
 import re
 from collections import Counter
-from typing import TYPE_CHECKING, Any, Dict, Optional, Tuple
+from http import HTTPStatus
+from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Tuple, Union
 
+from typing_extensions import Self
+
+from synapse._pydantic_compat import (
+    StrictBool,
+    StrictStr,
+    validator,
+)
 from synapse.api.auth.mas import MasDelegatedAuth
 from synapse.api.errors import (
+    Codes,
     InteractiveAuthIncompleteError,
     InvalidAPICallError,
     SynapseError,
@@ -37,11 +46,13 @@ from synapse.http.servlet import (
     parse_integer,
     parse_json_object_from_request,
     parse_string,
+    validate_json_object,
 )
 from synapse.http.site import SynapseRequest
 from synapse.logging.opentracing import log_kv, set_tag
 from synapse.rest.client._base import client_patterns, interactive_auth_handler
 from synapse.types import JsonDict, StreamToken
+from synapse.types.rest import RequestBodyModel
 from synapse.util.cancellation import cancellable
 
 if TYPE_CHECKING:
@@ -59,7 +70,6 @@ class KeyUploadServlet(RestServlet):
         "device_keys": {
             "user_id": "<user_id>",
             "device_id": "<device_id>",
-            "valid_until_ts": <millisecond_timestamp>,
             "algorithms": [
                 "m.olm.curve25519-aes-sha2",
             ]
@@ -111,12 +121,123 @@ class KeyUploadServlet(RestServlet):
         self._clock = hs.get_clock()
         self._store = hs.get_datastores().main
 
+    class KeyUploadRequestBody(RequestBodyModel):
+        """
+        The body of a `POST /_matrix/client/v3/keys/upload` request.
+
+        Based on https://spec.matrix.org/v1.16/client-server-api/#post_matrixclientv3keysupload.
+        """
+
+        class DeviceKeys(RequestBodyModel):
+            algorithms: List[StrictStr]
+            """The encryption algorithms supported by this device."""
+
+            device_id: StrictStr
+            """The ID of the device these keys belong to. Must match the device ID used when logging in."""
+
+            keys: Mapping[StrictStr, StrictStr]
+            """
+            Public identity keys. The names of the properties should be in the
+            format `<algorithm>:<device_id>`. The keys themselves should be encoded as
+            specified by the key algorithm.
+            """
+
+            signatures: Mapping[StrictStr, Mapping[StrictStr, StrictStr]]
+            """Signatures for the device key object. A map from user ID, to a map from "<algorithm>:<device_id>" to the signature."""
+
+            user_id: StrictStr
+            """The ID of the user the device belongs to. Must match the user ID used when logging in."""
+
+        class KeyObject(RequestBodyModel):
+            key: StrictStr
+            """The key, encoded using unpadded base64."""
+
+            fallback: Optional[StrictBool] = False
+            """Whether this is a fallback key. Only used when handling fallback keys."""
+
+            signatures: Mapping[StrictStr, Mapping[StrictStr, StrictStr]]
+            """Signature for the device. Mapped from user ID to another map of key signing identifier to the signature itself.
+
+            See the following for more detail: https://spec.matrix.org/v1.16/appendices/#signing-details
+            """
+
+        device_keys: Optional[DeviceKeys] = None
+        """Identity keys for the device. May be absent if no new identity keys are required."""
+
+        fallback_keys: Optional[Mapping[StrictStr, Union[StrictStr, KeyObject]]]
+        """
+        The public key which should be used if the device's one-time keys are
+        exhausted. The fallback key is not deleted once used, but should be
+        replaced when additional one-time keys are being uploaded. The server
+        will notify the client of the fallback key being used through `/sync`.
+
+        There can only be at most one key per algorithm uploaded, and the server
+        will only persist one key per algorithm.
+
+        When uploading a signed key, an additional fallback: true key should be
+        included to denote that the key is a fallback key.
+
+        May be absent if a new fallback key is not required.
+        """
+
+        @validator("fallback_keys", pre=True)
+        def validate_fallback_keys(cls: Self, v: Any) -> Any:
+            if v is None:
+                return v
+            if not isinstance(v, dict):
+                raise TypeError("fallback_keys must be a mapping")
+
+            for k in v.keys():
+                if not len(k.split(":")) == 2:
+                    raise SynapseError(
+                        code=HTTPStatus.BAD_REQUEST,
+                        errcode=Codes.BAD_JSON,
+                        msg=f"Invalid fallback_keys key {k!r}. "
+                        'Expected "<algorithm>:<device_id>".',
+                    )
+            return v
+
+        one_time_keys: Optional[Mapping[StrictStr, Union[StrictStr, KeyObject]]] = None
+        """
+        One-time public keys for "pre-key" messages. The names of the properties
+        should be in the format `<algorithm>:<key_id>`.
+
+        The format of the key is determined by the key algorithm, see:
+        https://spec.matrix.org/v1.16/client-server-api/#key-algorithms.
+        """
+
+        @validator("one_time_keys", pre=True)
+        def validate_one_time_keys(cls: Self, v: Any) -> Any:
+            if v is None:
+                return v
+            if not isinstance(v, dict):
+                raise TypeError("one_time_keys must be a mapping")
+
+            for k, _ in v.items():
+                if not len(k.split(":")) == 2:
+                    raise SynapseError(
+                        code=HTTPStatus.BAD_REQUEST,
+                        errcode=Codes.BAD_JSON,
+                        msg=f"Invalid one_time_keys key {k!r}. "
+                        'Expected "<algorithm>:<device_id>".',
+                    )
+            return v
+
     async def on_POST(
         self, request: SynapseRequest, device_id: Optional[str]
     ) -> Tuple[int, JsonDict]:
         requester = await self.auth.get_user_by_req(request, allow_guest=True)
         user_id = requester.user.to_string()
+
+        # Parse the request body. Validate separately, as the handler expects a
+        # plain dict, rather than any parsed object.
+        #
+        # Note: It would be nice to work with a parsed object, but the handler
+        # needs to encode portions of the request body as canonical JSON before
+        # storing the result in the DB. There's little point in converted to a
+        # parsed object and then back to a dict.
         body = parse_json_object_from_request(request)
+        validate_json_object(body, self.KeyUploadRequestBody)
 
         if device_id is not None:
             # Providing the device_id should only be done for setting keys
@@ -149,8 +270,31 @@ class KeyUploadServlet(RestServlet):
                 400, "To upload keys, you must pass device_id when authenticating"
             )
 
+        if "device_keys" in body and isinstance(body["device_keys"], dict):
+            # Validate the provided `user_id` and `device_id` fields in
+            # `device_keys` match that of the requesting user. We can't do
+            # this directly in the pydantic model as we don't have access
+            # to the requester yet.
+            #
+            # TODO: We could use ValidationInfo when we switch to Pydantic v2.
+            # https://docs.pydantic.dev/latest/concepts/validators/#validation-info
+            if body["device_keys"].get("user_id") != user_id:
+                raise SynapseError(
+                    code=HTTPStatus.BAD_REQUEST,
+                    errcode=Codes.BAD_JSON,
+                    msg="Provided `user_id` in `device_keys` does not match that of the authenticated user",
+                )
+            if body["device_keys"].get("device_id") != device_id:
+                raise SynapseError(
+                    code=HTTPStatus.BAD_REQUEST,
+                    errcode=Codes.BAD_JSON,
+                    msg="Provided `device_id` in `device_keys` does not match that of the authenticated user device",
+                )
+
         result = await self.e2e_keys_handler.upload_keys_for_user(
-            user_id=user_id, device_id=device_id, keys=body
+            user_id=user_id,
+            device_id=device_id,
+            keys=body,
         )
 
         return 200, result

synapse/rest/client/sync.py

@@ -363,9 +363,6 @@ class SyncRestServlet(RestServlet):
 
         # https://github.com/matrix-org/matrix-doc/blob/54255851f642f84a4f1aaf7bc063eebe3d76752b/proposals/2732-olm-fallback-keys.md
         # states that this field should always be included, as long as the server supports the feature.
-        response["org.matrix.msc2732.device_unused_fallback_key_types"] = (
-            sync_result.device_unused_fallback_key_types
-        )
         response["device_unused_fallback_key_types"] = (
             sync_result.device_unused_fallback_key_types
         )

tests/handlers/test_e2e_keys.py

@@ -410,7 +410,6 @@ class E2eKeysHandlerTestCase(unittest.HomeserverTestCase):
         device_id = "xyz"
         fallback_key = {"alg1:k1": "fallback_key1"}
         fallback_key2 = {"alg1:k2": "fallback_key2"}
-        fallback_key3 = {"alg1:k2": "fallback_key3"}
         otk = {"alg1:k2": "key2"}
 
         # we shouldn't have any unused fallback keys yet
@@ -531,28 +530,6 @@ class E2eKeysHandlerTestCase(unittest.HomeserverTestCase):
             {"failures": {}, "one_time_keys": {local_user: {device_id: fallback_key2}}},
         )
 
-        # using the unstable prefix should also set the fallback key
-        self.get_success(
-            self.handler.upload_keys_for_user(
-                local_user,
-                device_id,
-                {"org.matrix.msc2732.fallback_keys": fallback_key3},
-            )
-        )
-
-        claim_res = self.get_success(
-            self.handler.claim_one_time_keys(
-                {local_user: {device_id: {"alg1": 1}}},
-                self.requester,
-                timeout=None,
-                always_include_fallback_keys=False,
-            )
-        )
-        self.assertEqual(
-            claim_res,
-            {"failures": {}, "one_time_keys": {local_user: {device_id: fallback_key3}}},
-        )
-
     def test_fallback_key_bulk(self) -> None:
         """Like test_fallback_key, but claims multiple keys in one handler call."""
         alice = f"@alice:{self.hs.hostname}"

tests/rest/client/test_keys.py

@@ -40,6 +40,147 @@ from tests.unittest import override_config
 from tests.utils import HAS_AUTHLIB
 
 
+class KeyUploadTestCase(unittest.HomeserverTestCase):
+    servlets = [
+        keys.register_servlets,
+        admin.register_servlets_for_client_rest_resource,
+        login.register_servlets,
+    ]
+
+    def test_upload_keys_fails_on_invalid_structure(self) -> None:
+        """Check that we validate the structure of keys upon upload.
+
+        Regression test for https://github.com/element-hq/synapse/pull/17097
+        """
+        self.register_user("alice", "wonderland")
+        alice_token = self.login("alice", "wonderland")
+
+        channel = self.make_request(
+            "POST",
+            "/_matrix/client/v3/keys/upload",
+            {
+                # Error: device_keys must be a dict
+                "device_keys": ["some", "stuff", "weewoo"]
+            },
+            alice_token,
+        )
+        self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result)
+        self.assertEqual(
+            channel.json_body["errcode"],
+            Codes.BAD_JSON,
+            channel.result,
+        )
+
+        channel = self.make_request(
+            "POST",
+            "/_matrix/client/v3/keys/upload",
+            {
+                # Error: properties of fallback_keys must be in the form `<algorithm>:<device_id>`
+                "fallback_keys": {"invalid_key": "signature_base64"}
+            },
+            alice_token,
+        )
+        self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result)
+        self.assertEqual(
+            channel.json_body["errcode"],
+            Codes.BAD_JSON,
+            channel.result,
+        )
+
+        channel = self.make_request(
+            "POST",
+            "/_matrix/client/v3/keys/upload",
+            {
+                # Same as above, but for one_time_keys
+                "one_time_keys": {"invalid_key": "signature_base64"}
+            },
+            alice_token,
+        )
+        self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result)
+        self.assertEqual(
+            channel.json_body["errcode"],
+            Codes.BAD_JSON,
+            channel.result,
+        )
+
+    def test_upload_keys_fails_on_invalid_user_id_or_device_id(self) -> None:
+        """
+        Validate that the requesting user is uploading their own keys and nobody
+        else's.
+        """
+        device_id = "DEVICE_ID"
+        alice_user_id = self.register_user("alice", "wonderland")
+        alice_token = self.login("alice", "wonderland", device_id=device_id)
+
+        channel = self.make_request(
+            "POST",
+            "/_matrix/client/v3/keys/upload",
+            {
+                "device_keys": {
+                    # Included `user_id` does not match requesting user.
+                    "user_id": "@unknown_user:test",
+                    "device_id": device_id,
+                    "algorithms": ["m.olm.curve25519-aes-sha2"],
+                    "keys": {
+                        f"ed25519:{device_id}": "publickey",
+                    },
+                    "signatures": {},
+                }
+            },
+            alice_token,
+        )
+        self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result)
+        self.assertEqual(
+            channel.json_body["errcode"],
+            Codes.BAD_JSON,
+            channel.result,
+        )
+
+        channel = self.make_request(
+            "POST",
+            "/_matrix/client/v3/keys/upload",
+            {
+                "device_keys": {
+                    "user_id": alice_user_id,
+                    # Included `device_id` does not match requesting user's.
+                    "device_id": "UNKNOWN_DEVICE_ID",
+                    "algorithms": ["m.olm.curve25519-aes-sha2"],
+                    "keys": {
+                        f"ed25519:{device_id}": "publickey",
+                    },
+                    "signatures": {},
+                }
+            },
+            alice_token,
+        )
+        self.assertEqual(channel.code, HTTPStatus.BAD_REQUEST, channel.result)
+        self.assertEqual(
+            channel.json_body["errcode"],
+            Codes.BAD_JSON,
+            channel.result,
+        )
+
+    def test_upload_keys_succeeds_when_fields_are_explicitly_set_to_null(self) -> None:
+        """
+        This is a regression test for https://github.com/element-hq/synapse/pull/19023.
+        """
+        device_id = "DEVICE_ID"
+        self.register_user("alice", "wonderland")
+        alice_token = self.login("alice", "wonderland", device_id=device_id)
+
+        channel = self.make_request(
+            "POST",
+            "/_matrix/client/v3/keys/upload",
+            {
+                "device_keys": None,
+                "one_time_keys": None,
+                "fallback_keys": None,
+            },
+            alice_token,
+        )
+        self.assertEqual(channel.code, HTTPStatus.OK, channel.result)
+
+
 class KeyQueryTestCase(unittest.HomeserverTestCase):
     servlets = [
         keys.register_servlets,

tests/util/test_logcontext.py

@@ -22,7 +22,6 @@
 import logging
 from typing import Callable, Generator, cast
 
-import twisted.python.failure
 from twisted.internet import defer, reactor as _reactor
 
 from synapse.logging.context import (
@@ -33,6 +32,7 @@ from synapse.logging.context import (
     current_context,
     make_deferred_yieldable,
     nested_logging_context,
+    run_coroutine_in_background,
     run_in_background,
 )
 from synapse.types import ISynapseReactor
@@ -249,73 +249,80 @@ class LoggingContextTestCase(unittest.TestCase):
         # Back to the sentinel context
         self._check_test_key("sentinel")
 
-    def _test_run_in_background(self, function: Callable[[], object]) -> defer.Deferred:
-        sentinel_context = current_context()
+    async def _test_run_in_background(self, function: Callable[[], object]) -> None:
+        clock = Clock(reactor)
 
-        callback_completed = False
+        # Sanity check that we start in the sentinel context
+        self._check_test_key("sentinel")
+
+        callback_finished = False
 
         with LoggingContext("foo"):
-            # fire off function, but don't wait on it.
-            d2 = run_in_background(function)
+            # Fire off the function, but don't wait on it.
+            deferred = run_in_background(function)
+            self._check_test_key("foo")
 
-            def cb(res: object) -> object:
-                nonlocal callback_completed
-                callback_completed = True
-                return res
+            def callback(result: object) -> object:
+                nonlocal callback_finished
+                callback_finished = True
+                # Pass through the result
+                return result
 
-            d2.addCallback(cb)
+            # We `addBoth` because when exceptions happen, we still want to mark the
+            # callback as finished so that the test can complete and we see the
+            # underlying error.
+            deferred.addBoth(callback)
 
             self._check_test_key("foo")
 
-        # now wait for the function under test to have run, and check that
-        # the logcontext is left in a sane state.
-        d2 = defer.Deferred()
+            # Now wait for the function under test to have run, and check that
+            # the logcontext is left in a sane state.
+            while not callback_finished:
+                await clock.sleep(0)
+                self._check_test_key("foo")
 
-        def check_logcontext() -> None:
-            if not callback_completed:
-                reactor.callLater(0.01, check_logcontext)
-                return
+        self.assertTrue(
+            callback_finished,
+            "Callback never finished which means the test probably didn't wait long enough",
+        )
 
-            # make sure that the context was reset before it got thrown back
-            # into the reactor
-            try:
-                self.assertIs(current_context(), sentinel_context)
-                d2.callback(None)
-            except BaseException:
-                d2.errback(twisted.python.failure.Failure())
-
-        reactor.callLater(0.01, check_logcontext)
-
-        # test is done once d2 finishes
-        return d2
+        # Back to the sentinel context
+        self._check_test_key("sentinel")
 
     @logcontext_clean
-    def test_run_in_background_with_blocking_fn(self) -> defer.Deferred:
+    async def test_run_in_background_with_blocking_fn(self) -> None:
         async def blocking_function() -> None:
             await Clock(reactor).sleep(0)
 
-        return self._test_run_in_background(blocking_function)
+        await self._test_run_in_background(blocking_function)
 
     @logcontext_clean
-    def test_run_in_background_with_non_blocking_fn(self) -> defer.Deferred:
+    async def test_run_in_background_with_non_blocking_fn(self) -> None:
         @defer.inlineCallbacks
         def nonblocking_function() -> Generator["defer.Deferred[object]", object, None]:
             with PreserveLoggingContext():
                 yield defer.succeed(None)
 
-        return self._test_run_in_background(nonblocking_function)
+        await self._test_run_in_background(nonblocking_function)
 
     @logcontext_clean
-    def test_run_in_background_with_chained_deferred(self) -> defer.Deferred:
+    async def test_run_in_background_with_chained_deferred(self) -> None:
         # a function which returns a deferred which looks like it has been
         # called, but is actually paused
         def testfunc() -> defer.Deferred:
             return make_deferred_yieldable(_chained_deferred_function())
 
-        return self._test_run_in_background(testfunc)
+        await self._test_run_in_background(testfunc)
 
     @logcontext_clean
-    def test_run_in_background_with_coroutine(self) -> defer.Deferred:
+    async def test_run_in_background_with_coroutine(self) -> None:
+        """
+        Test `run_in_background` with a coroutine that yields control back to the
+        reactor.
+
+        This will stress the logic around incomplete deferreds in `run_in_background`.
+        """
+
         async def testfunc() -> None:
             self._check_test_key("foo")
             d = defer.ensureDeferred(Clock(reactor).sleep(0))
@@ -323,14 +330,111 @@ class LoggingContextTestCase(unittest.TestCase):
             await d
             self._check_test_key("foo")
 
-        return self._test_run_in_background(testfunc)
+        await self._test_run_in_background(testfunc)
 
     @logcontext_clean
-    def test_run_in_background_with_nonblocking_coroutine(self) -> defer.Deferred:
+    async def test_run_in_background_with_nonblocking_coroutine(self) -> None:
+        """
+        Test `run_in_background` with a "nonblocking" coroutine (never yields control
+        back to the reactor).
+
+        This will stress the logic around completed deferreds in `run_in_background`.
+        """
+
         async def testfunc() -> None:
             self._check_test_key("foo")
 
-        return self._test_run_in_background(testfunc)
+        await self._test_run_in_background(testfunc)
+
+    @logcontext_clean
+    async def test_run_coroutine_in_background(self) -> None:
+        """
+        Test `run_coroutine_in_background` with a coroutine that yields control back to the
+        reactor.
+
+        This will stress the logic around incomplete deferreds in `run_coroutine_in_background`.
+        """
+        clock = Clock(reactor)
+
+        # Sanity check that we start in the sentinel context
+        self._check_test_key("sentinel")
+
+        callback_finished = False
+
+        async def competing_callback() -> None:
+            nonlocal callback_finished
+            try:
+                # The callback should have the same logcontext as the caller
+                self._check_test_key("foo")
+
+                with LoggingContext("competing"):
+                    await clock.sleep(0)
+                    self._check_test_key("competing")
+
+                self._check_test_key("foo")
+            finally:
+                # When exceptions happen, we still want to mark the callback as finished
+                # so that the test can complete and we see the underlying error.
+                callback_finished = True
+
+        with LoggingContext("foo"):
+            run_coroutine_in_background(competing_callback())
+            self._check_test_key("foo")
+            await clock.sleep(0)
+            self._check_test_key("foo")
+
+        self.assertTrue(
+            callback_finished,
+            "Callback never finished which means the test probably didn't wait long enough",
+        )
+
+        # Back to the sentinel context
+        self._check_test_key("sentinel")
+
+    @logcontext_clean
+    async def test_run_coroutine_in_background_with_nonblocking_coroutine(self) -> None:
+        """
+        Test `run_coroutine_in_background` with a "nonblocking" coroutine (never yields control
+        back to the reactor).
+
+        This will stress the logic around completed deferreds in `run_coroutine_in_background`.
+        """
+        # Sanity check that we start in the sentinel context
+        self._check_test_key("sentinel")
+
+        callback_finished = False
+
+        async def competing_callback() -> None:
+            nonlocal callback_finished
+            try:
+                # The callback should have the same logcontext as the caller
+                self._check_test_key("foo")
+
+                with LoggingContext("competing"):
+                    # We `await` here but there is nothing to wait for here since the
+                    # deferred is already complete so we should immediately continue
+                    # executing in the same context.
+                    await defer.succeed(None)
+
+                    self._check_test_key("competing")
+
+                self._check_test_key("foo")
+            finally:
+                # When exceptions happen, we still want to mark the callback as finished
+                # so that the test can complete and we see the underlying error.
+                callback_finished = True
+
+        with LoggingContext("foo"):
+            run_coroutine_in_background(competing_callback())
+            self._check_test_key("foo")
+
+        self.assertTrue(
+            callback_finished,
+            "Callback never finished which means the test probably didn't wait long enough",
+        )
+
+        # Back to the sentinel context
+        self._check_test_key("sentinel")
 
     @logcontext_clean
     @defer.inlineCallbacks