diff --git a/changelog.d/6657.bugfix b/changelog.d/6657.bugfix
new file mode 100644
index 0000000000..94e51a9896
--- /dev/null
+++ b/changelog.d/6657.bugfix
@@ -0,0 +1 @@
+Fix incorrect signing of responses from the key server implementation.
\ No newline at end of file
diff --git a/synapse/rest/key/v2/remote_key_resource.py b/synapse/rest/key/v2/remote_key_resource.py
index bf5e0eb844..e7fc3f0431 100644
--- a/synapse/rest/key/v2/remote_key_resource.py
+++ b/synapse/rest/key/v2/remote_key_resource.py
@@ -15,7 +15,6 @@
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
@@ -217,28 +216,15 @@ 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)
- 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"]
+ else:
+ signed_keys = []
+ for key_json in json_results:
+ key_json = json.loads(key_json)
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)
+ key_json = sign_json(key_json, self.config.server_name, signing_key)
- signed_keys.append(key_json)
+ signed_keys.append(key_json)
- results = {"server_keys": signed_keys}
+ results = {"server_keys": signed_keys}
- respond_with_json_bytes(request, 200, encode_canonical_json(results))
+ respond_with_json_bytes(request, 200, encode_canonical_json(results))
diff --git a/tests/rest/key/__init__.py b/tests/rest/key/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/tests/rest/key/__init__.py
diff --git a/tests/rest/key/v2/__init__.py b/tests/rest/key/v2/__init__.py
new file mode 100644
index 0000000000..e69de29bb2
--- /dev/null
+++ b/tests/rest/key/v2/__init__.py
diff --git a/tests/rest/key/v2/test_remote_key_resource.py b/tests/rest/key/v2/test_remote_key_resource.py
index d8246b4e78..6776a56cad 100644
--- a/tests/rest/key/v2/test_remote_key_resource.py
+++ b/tests/rest/key/v2/test_remote_key_resource.py
@@ -13,25 +13,30 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import urllib.parse
-from io import BytesIO
+from io import BytesIO, StringIO
from mock import Mock
import signedjson.key
+from canonicaljson import encode_canonical_json
from nacl.signing import SigningKey
from signedjson.sign import sign_json
from twisted.web.resource import NoResource
+from synapse.crypto.keyring import PerspectivesKeyFetcher
from synapse.http.site import SynapseRequest
from synapse.rest.key.v2 import KeyApiV2Resource
+from synapse.storage.keys import FetchKeyResult
from synapse.util.httpresourcetree import create_resource_tree
+from synapse.util.stringutils import random_string
from tests import unittest
from tests.server import FakeChannel, wait_until_result
+from tests.utils import default_config
-class RemoteKeyResourceTestCase(unittest.HomeserverTestCase):
+class BaseRemoteKeyResourceTestCase(unittest.HomeserverTestCase):
def make_homeserver(self, reactor, clock):
self.http_client = Mock()
return self.setup_test_homeserver(http_client=self.http_client)
@@ -73,6 +78,8 @@ class RemoteKeyResourceTestCase(unittest.HomeserverTestCase):
self.http_client.get_json.side_effect = get_json
+
+class RemoteKeyResourceTestCase(BaseRemoteKeyResourceTestCase):
def make_notary_request(self, server_name: str, key_id: str) -> dict:
"""Send a GET request to the test server requesting the given key.
@@ -125,6 +132,126 @@ class RemoteKeyResourceTestCase(unittest.HomeserverTestCase):
oursigs = sigs[self.hs.hostname]
self.assertEqual(len(oursigs), 2)
- # and both keys should be present in the verify_keys section
+ # the requested key 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"])
+
+
+class EndToEndPerspectivesTests(BaseRemoteKeyResourceTestCase):
+ """End-to-end tests of the perspectives fetch case
+
+ The idea here is to actually wire up a PerspectivesKeyFetcher to the notary
+ endpoint, to check that the two implementations are compatible.
+ """
+
+ def default_config(self, *args, **kwargs):
+ config = super().default_config(*args, **kwargs)
+
+ # replace the signing key with our own
+ self.hs_signing_key = signedjson.key.generate_signing_key("kssk")
+ strm = StringIO()
+ signedjson.key.write_signing_keys(strm, [self.hs_signing_key])
+ config["signing_key"] = strm.getvalue()
+
+ return config
+
+ def prepare(self, reactor, clock, homeserver):
+ # make a second homeserver, configured to use the first one as a key notary
+ self.http_client2 = Mock()
+ config = default_config(name="keyclient")
+ config["trusted_key_servers"] = [
+ {
+ "server_name": self.hs.hostname,
+ "verify_keys": {
+ "ed25519:%s"
+ % (
+ self.hs_signing_key.version,
+ ): signedjson.key.encode_verify_key_base64(
+ self.hs_signing_key.verify_key
+ )
+ },
+ }
+ ]
+ self.hs2 = self.setup_test_homeserver(
+ http_client=self.http_client2, config=config
+ )
+
+ # wire up outbound POST /key/v2/query requests from hs2 so that they
+ # will be forwarded to hs1
+ def post_json(destination, path, data):
+ self.assertEqual(destination, self.hs.hostname)
+ self.assertEqual(
+ path, "/_matrix/key/v2/query",
+ )
+
+ channel = FakeChannel(self.site, self.reactor)
+ req = SynapseRequest(channel)
+ req.content = BytesIO(encode_canonical_json(data))
+
+ req.requestReceived(
+ b"POST", path.encode("utf-8"), b"1.1",
+ )
+ wait_until_result(self.reactor, req)
+ self.assertEqual(channel.code, 200)
+ resp = channel.json_body
+ return resp
+
+ self.http_client2.post_json.side_effect = post_json
+
+ def test_get_key(self):
+ """Fetch a key belonging to a random server"""
+ # make up a key to be fetched.
+ testkey = signedjson.key.generate_signing_key("abc")
+
+ # we expect hs1 to make a regular key request to the target server
+ self.expect_outgoing_key_request("targetserver", testkey)
+ keyid = "ed25519:%s" % (testkey.version,)
+
+ fetcher = PerspectivesKeyFetcher(self.hs2)
+ d = fetcher.get_keys({"targetserver": {keyid: 1000}})
+ res = self.get_success(d)
+ self.assertIn("targetserver", res)
+ keyres = res["targetserver"][keyid]
+ assert isinstance(keyres, FetchKeyResult)
+ self.assertEqual(
+ signedjson.key.encode_verify_key_base64(keyres.verify_key),
+ signedjson.key.encode_verify_key_base64(testkey.verify_key),
+ )
+
+ def test_get_notary_key(self):
+ """Fetch a key belonging to the notary server"""
+ # make up a key to be fetched. We randomise the keyid to try to get it to
+ # appear before the key server signing key sometimes (otherwise we bail out
+ # before fetching its signature)
+ testkey = signedjson.key.generate_signing_key(random_string(5))
+
+ # we expect hs1 to make a regular key request to itself
+ self.expect_outgoing_key_request(self.hs.hostname, testkey)
+ keyid = "ed25519:%s" % (testkey.version,)
+
+ fetcher = PerspectivesKeyFetcher(self.hs2)
+ d = fetcher.get_keys({self.hs.hostname: {keyid: 1000}})
+ res = self.get_success(d)
+ self.assertIn(self.hs.hostname, res)
+ keyres = res[self.hs.hostname][keyid]
+ assert isinstance(keyres, FetchKeyResult)
+ self.assertEqual(
+ signedjson.key.encode_verify_key_base64(keyres.verify_key),
+ signedjson.key.encode_verify_key_base64(testkey.verify_key),
+ )
+
+ def test_get_notary_keyserver_key(self):
+ """Fetch the notary's keyserver key"""
+ # we expect hs1 to make a regular key request to itself
+ self.expect_outgoing_key_request(self.hs.hostname, self.hs_signing_key)
+ keyid = "ed25519:%s" % (self.hs_signing_key.version,)
+
+ fetcher = PerspectivesKeyFetcher(self.hs2)
+ d = fetcher.get_keys({self.hs.hostname: {keyid: 1000}})
+ res = self.get_success(d)
+ self.assertIn(self.hs.hostname, res)
+ keyres = res[self.hs.hostname][keyid]
+ assert isinstance(keyres, FetchKeyResult)
+ self.assertEqual(
+ signedjson.key.encode_verify_key_base64(keyres.verify_key),
+ signedjson.key.encode_verify_key_base64(self.hs_signing_key.verify_key),
+ )
|
