diff options
| -rw-r--r-- | changelog.d/6657.bugfix | 1 | ||||
| -rw-r--r-- | synapse/rest/key/v2/remote_key_resource.py | 30 | ||||
| -rw-r--r-- | tests/rest/key/__init__.py | 0 | ||||
| -rw-r--r-- | tests/rest/key/v2/__init__.py | 0 | ||||
| -rw-r--r-- | tests/rest/key/v2/test_remote_key_resource.py | 135 |
5 files changed, 140 insertions, 26 deletions
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), + ) |