Merge remote-tracking branch 'origin/release-v1.8.0' into develop

8 files changed, 184 insertions, 28 deletions

diff --git a/changelog.d/6563.bugfix b/changelog.d/6563.bugfix
new file mode 100644
index 0000000000..3325fb1dcf
--- /dev/null
+++ b/changelog.d/6563.bugfix
@@ -0,0 +1 @@
+Fix GET request on /_synapse/admin/v2/users endpoint. Contributed by Awesome Technologies Innovationslabor GmbH.
\ No newline at end of file
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/synapse/storage/data_stores/main/__init__.py b/synapse/storage/data_stores/main/__init__.py
index c577c0df5f..2700cca822 100644
--- a/synapse/storage/data_stores/main/__init__.py
+++ b/synapse/storage/data_stores/main/__init__.py
@@ -526,9 +526,9 @@ class DataStore(
 
         attr_filter = {}
         if not guests:
-            attr_filter["is_guest"] = False
+            attr_filter["is_guest"] = 0
         if not deactivated:
-            attr_filter["deactivated"] = False
+            attr_filter["deactivated"] = 0
 
         return self.db.simple_select_list_paginate(
             desc="get_users_paginate",
diff --git a/tests/rest/admin/test_admin.py b/tests/rest/admin/test_admin.py
index 0ed2594381..325bd6a608 100644
--- a/tests/rest/admin/test_admin.py
+++ b/tests/rest/admin/test_admin.py
@@ -341,6 +341,47 @@ class UserRegisterTestCase(unittest.HomeserverTestCase):
         self.assertEqual("Invalid user type", channel.json_body["error"])
 
 
+class UsersListTestCase(unittest.HomeserverTestCase):
+
+    servlets = [
+        synapse.rest.admin.register_servlets,
+        login.register_servlets,
+    ]
+    url = "/_synapse/admin/v2/users"
+
+    def prepare(self, reactor, clock, hs):
+        self.admin_user = self.register_user("admin", "pass", admin=True)
+        self.admin_user_tok = self.login("admin", "pass")
+
+        self.register_user("user1", "pass1", admin=False)
+        self.register_user("user2", "pass2", admin=False)
+
+    def test_no_auth(self):
+        """
+        Try to list users without authentication.
+        """
+        request, channel = self.make_request("GET", self.url, b"{}")
+        self.render(request)
+
+        self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual("M_MISSING_TOKEN", channel.json_body["errcode"])
+
+    def test_all_users(self):
+        """
+        List all users, including deactivated users.
+        """
+        request, channel = self.make_request(
+            "GET",
+            self.url + "?deactivated=true",
+            b"{}",
+            access_token=self.admin_user_tok,
+        )
+        self.render(request)
+
+        self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"])
+        self.assertEqual(3, len(channel.json_body["users"]))
+
+
 class ShutdownRoomTestCase(unittest.HomeserverTestCase):
     servlets = [
         synapse.rest.admin.register_servlets_for_client_rest_resource,
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),
+        )