Fetch verify key locally rather than trying to do so over federation if origin and host are the same. (#11129)

* add tests for fetching key locally

* add logic to check if origin server is same as host and fetch verify key locally rather than over federation

* add changelog

* slight refactor, add docstring, change changelog entry

* Make changelog entry one line

* remove verify_json_locally and push locality check to process_request, add function process_request_locally

* remove leftover code reference

* refactor to add common call to 'verify_json and associated handling code

* add type hint to process_json

* add some docstrings + very slight refactor

changelog.d/11129.bugfix

@@ -0,0 +1 @@
+Fix long-standing bug where verification requests could fail in certain cases if whitelist was in place but did not include your own homeserver.

synapse/crypto/keyring.py

@@ -22,6 +22,7 @@ import attr
 from signedjson.key import (
     decode_verify_key_bytes,
     encode_verify_key_base64,
+    get_verify_key,
     is_signing_algorithm_supported,
 )
 from signedjson.sign import (
@@ -30,6 +31,7 @@ from signedjson.sign import (
     signature_ids,
     verify_signed_json,
 )
+from signedjson.types import VerifyKey
 from unpaddedbase64 import decode_base64
 
 from twisted.internet import defer
@@ -177,6 +179,8 @@ class Keyring:
             clock=hs.get_clock(),
             process_batch_callback=self._inner_fetch_key_requests,
         )
+        self.verify_key = get_verify_key(hs.signing_key)
+        self.hostname = hs.hostname
 
     async def verify_json_for_server(
         self,
@@ -196,6 +200,7 @@ class Keyring:
             validity_time: timestamp at which we require the signing key to
                 be valid. (0 implies we don't care)
         """
+
         request = VerifyJsonRequest.from_json_object(
             server_name,
             json_object,
@@ -262,6 +267,11 @@ class Keyring:
                 Codes.UNAUTHORIZED,
             )
 
+        # If we are the originating server don't fetch verify key for self over federation
+        if verify_request.server_name == self.hostname:
+            await self._process_json(self.verify_key, verify_request)
+            return
+
         # Add the keys we need to verify to the queue for retrieval. We queue
         # up requests for the same server so we don't end up with many in flight
         # requests for the same keys.
@@ -285,15 +295,28 @@ class Keyring:
             if key_result.valid_until_ts < verify_request.minimum_valid_until_ts:
                 continue
 
-            verify_key = key_result.verify_key
+            await self._process_json(key_result.verify_key, verify_request)
-            json_object = verify_request.get_json_object()
+            verified = True
+
+        if not verified:
+            raise SynapseError(
+                401,
+                f"Failed to find any key to satisfy: {key_request}",
+                Codes.UNAUTHORIZED,
+            )
+
+    async def _process_json(
+        self, verify_key: VerifyKey, verify_request: VerifyJsonRequest
+    ) -> None:
+        """Processes the `VerifyJsonRequest`. Raises if the signature can't be
+        verified.
+        """
         try:
             verify_signed_json(
-                    json_object,
+                verify_request.get_json_object(),
                 verify_request.server_name,
                 verify_key,
             )
-                verified = True
         except SignatureVerifyException as e:
             logger.debug(
                 "Error verifying signature for %s:%s:%s with key %s: %s",
@@ -315,13 +338,6 @@ class Keyring:
                 Codes.UNAUTHORIZED,
             )
 
-        if not verified:
-            raise SynapseError(
-                401,
-                f"Failed to find any key to satisfy: {key_request}",
-                Codes.UNAUTHORIZED,
-            )
-
     async def _inner_fetch_key_requests(
         self, requests: List[_FetchKeyRequest]
     ) -> Dict[str, Dict[str, FetchKeyResult]]:

tests/crypto/test_keyring.py

@@ -197,6 +197,18 @@ class KeyringTestCase(unittest.HomeserverTestCase):
         # self.assertFalse(d.called)
         self.get_success(d)
 
+    def test_verify_for_server_locally(self):
+        """Ensure that locally signed JSON can be verified without fetching keys
+        over federation
+        """
+        kr = keyring.Keyring(self.hs)
+        json1 = {}
+        signedjson.sign.sign_json(json1, self.hs.hostname, self.hs.signing_key)
+
+        # Test that verify_json_for_server succeeds on a object signed by ourselves
+        d = kr.verify_json_for_server(self.hs.hostname, json1, 0)
+        self.get_success(d)
+
     def test_verify_json_for_server_with_null_valid_until_ms(self):
         """Tests that we correctly handle key requests for keys we've stored
         with a null `ts_valid_until_ms`