Workaround for error when fetching notary's own key (#6620)

* Kill off redundant SynapseRequestFactory

We already get the Site via the Channel, so there's no need for a dedicated
RequestFactory: we can just use the right constructor.

* Workaround for error when fetching notary's own key

As a notary server, when we return our own keys, include all of our signing
keys in verify_keys.

This is a workaround for #6596.

changelog.d/6620.misc

@@ -0,0 +1 @@
+Add a workaround for synapse raising exceptions when fetching the notary's own key from the notary.

synapse/rest/key/v2/remote_key_resource.py

@@ -15,6 +15,7 @@
 import logging
 
 from canonicaljson import encode_canonical_json, json
+from signedjson.key import encode_verify_key_base64
 from signedjson.sign import sign_json
 
 from twisted.internet import defer
@@ -216,10 +217,23 @@ class RemoteKey(DirectServeResource):
         if cache_misses and query_remote_on_cache_miss:
             yield self.fetcher.get_keys(cache_misses)
             yield self.query_keys(request, query, query_remote_on_cache_miss=False)
-        else:
+            return
+
         signed_keys = []
         for key_json in json_results:
             key_json = json.loads(key_json)
+
+            # backwards-compatibility hack for #6596: if the requested key belongs
+            # to us, make sure that all of the signing keys appear in the
+            # "verify_keys" section.
+            if key_json["server_name"] == self.config.server_name:
+                verify_keys = key_json["verify_keys"]
+                for signing_key in self.config.key_server_signing_keys:
+                    key_id = "%s:%s" % (signing_key.alg, signing_key.version)
+                    verify_keys[key_id] = {
+                        "key": encode_verify_key_base64(signing_key.verify_key)
+                    }
+
             for signing_key in self.config.key_server_signing_keys:
                 key_json = sign_json(key_json, self.config.server_name, signing_key)
 

tests/rest/key/v2/test_remote_key_resource.py

@@ -0,0 +1,130 @@
+# -*- coding: utf-8 -*-
+# Copyright 2020 The Matrix.org Foundation C.I.C.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+import urllib.parse
+from io import BytesIO
+
+from mock import Mock
+
+import signedjson.key
+from nacl.signing import SigningKey
+from signedjson.sign import sign_json
+
+from twisted.web.resource import NoResource
+
+from synapse.http.site import SynapseRequest
+from synapse.rest.key.v2 import KeyApiV2Resource
+from synapse.util.httpresourcetree import create_resource_tree
+
+from tests import unittest
+from tests.server import FakeChannel, wait_until_result
+
+
+class RemoteKeyResourceTestCase(unittest.HomeserverTestCase):
+    def make_homeserver(self, reactor, clock):
+        self.http_client = Mock()
+        return self.setup_test_homeserver(http_client=self.http_client)
+
+    def create_test_json_resource(self):
+        return create_resource_tree(
+            {"/_matrix/key/v2": KeyApiV2Resource(self.hs)}, root_resource=NoResource()
+        )
+
+    def expect_outgoing_key_request(
+        self, server_name: str, signing_key: SigningKey
+    ) -> None:
+        """
+        Tell the mock http client to expect an outgoing GET request for the given key
+        """
+
+        def get_json(destination, path, ignore_backoff=False, **kwargs):
+            self.assertTrue(ignore_backoff)
+            self.assertEqual(destination, server_name)
+            key_id = "%s:%s" % (signing_key.alg, signing_key.version)
+            self.assertEqual(
+                path, "/_matrix/key/v2/server/%s" % (urllib.parse.quote(key_id),)
+            )
+
+            response = {
+                "server_name": server_name,
+                "old_verify_keys": {},
+                "valid_until_ts": 200 * 1000,
+                "verify_keys": {
+                    key_id: {
+                        "key": signedjson.key.encode_verify_key_base64(
+                            signing_key.verify_key
+                        )
+                    }
+                },
+            }
+            sign_json(response, server_name, signing_key)
+            return response
+
+        self.http_client.get_json.side_effect = get_json
+
+    def make_notary_request(self, server_name: str, key_id: str) -> dict:
+        """Send a GET request to the test server requesting the given key.
+
+        Checks that the response is a 200 and returns the decoded json body.
+        """
+        channel = FakeChannel(self.site, self.reactor)
+        req = SynapseRequest(channel)
+        req.content = BytesIO(b"")
+        req.requestReceived(
+            b"GET",
+            b"/_matrix/key/v2/query/%s/%s"
+            % (server_name.encode("utf-8"), key_id.encode("utf-8")),
+            b"1.1",
+        )
+        wait_until_result(self.reactor, req)
+        self.assertEqual(channel.code, 200)
+        resp = channel.json_body
+        return resp
+
+    def test_get_key(self):
+        """Fetch a remote key"""
+        SERVER_NAME = "remote.server"
+        testkey = signedjson.key.generate_signing_key("ver1")
+        self.expect_outgoing_key_request(SERVER_NAME, testkey)
+
+        resp = self.make_notary_request(SERVER_NAME, "ed25519:ver1")
+        keys = resp["server_keys"]
+        self.assertEqual(len(keys), 1)
+
+        self.assertIn("ed25519:ver1", keys[0]["verify_keys"])
+        self.assertEqual(len(keys[0]["verify_keys"]), 1)
+
+        # it should be signed by both the origin server and the notary
+        self.assertIn(SERVER_NAME, keys[0]["signatures"])
+        self.assertIn(self.hs.hostname, keys[0]["signatures"])
+
+    def test_get_own_key(self):
+        """Fetch our own key"""
+        testkey = signedjson.key.generate_signing_key("ver1")
+        self.expect_outgoing_key_request(self.hs.hostname, testkey)
+
+        resp = self.make_notary_request(self.hs.hostname, "ed25519:ver1")
+        keys = resp["server_keys"]
+        self.assertEqual(len(keys), 1)
+
+        # it should be signed by both itself, and the notary signing key
+        sigs = keys[0]["signatures"]
+        self.assertEqual(len(sigs), 1)
+        self.assertIn(self.hs.hostname, sigs)
+        oursigs = sigs[self.hs.hostname]
+        self.assertEqual(len(oursigs), 2)
+
+        # and both keys should be present in the verify_keys section
+        self.assertIn("ed25519:ver1", keys[0]["verify_keys"])
+        self.assertIn("ed25519:a_lPym", keys[0]["verify_keys"])

tests/unittest.py

@@ -36,7 +36,7 @@ from synapse.config.homeserver import HomeServerConfig
 from synapse.config.ratelimiting import FederationRateLimitConfig
 from synapse.federation.transport import server as federation_server
 from synapse.http.server import JsonResource
-from synapse.http.site import SynapseRequest
+from synapse.http.site import SynapseRequest, SynapseSite
 from synapse.logging.context import LoggingContext
 from synapse.server import HomeServer
 from synapse.types import Requester, UserID, create_requester
@@ -210,6 +210,15 @@ class HomeserverTestCase(TestCase):
         # Register the resources
         self.resource = self.create_test_json_resource()
 
+        # create a site to wrap the resource.
+        self.site = SynapseSite(
+            logger_name="synapse.access.http.fake",
+            site_tag="test",
+            config={},
+            resource=self.resource,
+            server_version_string="1",
+        )
+
         from tests.rest.client.v1.utils import RestHelper
 
         self.helper = RestHelper(self.hs, self.resource, getattr(self, "user_id", None))