Use the federation blacklist for requests to untrusted Identity Serve…

…rs (#6000)

changelog.d/6000.feature

@@ -0,0 +1 @@
+Apply the federation blacklist to requests to identity servers.

docs/sample_config.yaml

@@ -117,6 +117,9 @@ pid_file: DATADIR/homeserver.pid
 # blacklist IP address CIDR ranges. If this option is not specified, or
 # specified with an empty list, no ip range blacklist will be enforced.
 #
+# As of Synapse v1.4.0 this option also affects any outbound requests to identity
+# servers provided by user input.
+#
 # (0.0.0.0 and :: are always blacklisted, whether or not they are explicitly
 # listed here, since they correspond to unroutable addresses.)
 #

synapse/config/server.py

@@ -682,6 +682,9 @@ def generate_config_section(
         # blacklist IP address CIDR ranges. If this option is not specified, or
         # specified with an empty list, no ip range blacklist will be enforced.
         #
+        # As of Synapse v1.4.0 this option also affects any outbound requests to identity
+        # servers provided by user input.
+        #
         # (0.0.0.0 and :: are always blacklisted, whether or not they are explicitly
         # listed here, since they correspond to unroutable addresses.)
         #

synapse/handlers/identity.py

@@ -36,6 +36,7 @@
     SynapseError,
 )
 from synapse.config.emailconfig import ThreepidBehaviour
+from synapse.http.client import SimpleHttpClient
 from synapse.util.stringutils import random_string
 
 from ._base import BaseHandler
@@ -49,6 +50,11 @@ def __init__(self, hs):
 
         self.hs = hs
         self.http_client = hs.get_simple_http_client()
+        # We create a blacklisting instance of SimpleHttpClient for contacting identity
+        # servers specified by clients
+        self.blacklisting_http_client = SimpleHttpClient(
+            hs, ip_blacklist=hs.config.federation_ip_range_blacklist
+        )
         self.federation_http_client = hs.get_http_client()
 
         self.trusted_id_servers = set(hs.config.trusted_third_party_id_servers)
@@ -173,7 +179,9 @@ def bind_threepid(
             bind_url = "https://%s/_matrix/identity/api/v1/3pid/bind" % (id_server_host,)
 
         try:
-            data = yield self.http_client.post_json_get_json(
+            # Use the blacklisting http client as this call is only to identity servers
+            # provided by a client
+            data = yield self.blacklisting_http_client.post_json_get_json(
                 bind_url, bind_data, headers=headers
             )
 
@@ -286,7 +294,11 @@ def try_unbind_threepid_with_id_server(self, mxid, threepid, id_server):
         url = "https://%s/_matrix/identity/api/v1/3pid/unbind" % (id_server,)
 
         try:
-            yield self.http_client.post_json_get_json(url, content, headers)
+            # Use the blacklisting http client as this call is only to identity servers
+            # provided by a client
+            yield self.blacklisting_http_client.post_json_get_json(
+                url, content, headers
+            )
             changed = True
         except HttpResponseException as e:
             changed = False

synapse/handlers/room_member.py

@@ -37,6 +37,7 @@
     SynapseError,
 )
 from synapse.handlers.identity import LookupAlgorithm, create_id_access_token_header
+from synapse.http.client import SimpleHttpClient
 from synapse.types import RoomID, UserID
 from synapse.util.async_helpers import Linearizer
 from synapse.util.distributor import user_joined_room, user_left_room
@@ -66,7 +67,11 @@ def __init__(self, hs):
         self.auth = hs.get_auth()
         self.state_handler = hs.get_state_handler()
         self.config = hs.config
-        self.simple_http_client = hs.get_simple_http_client()
+        # We create a blacklisting instance of SimpleHttpClient for contacting identity
+        # servers specified by clients
+        self.simple_http_client = SimpleHttpClient(
+            hs, ip_blacklist=hs.config.federation_ip_range_blacklist
+        )
 
         self.federation_handler = hs.get_handlers().federation_handler
         self.directory_handler = hs.get_handlers().directory_handler

tests/handlers/test_identity.py

@@ -43,7 +43,8 @@ def make_homeserver(self, reactor, clock):
             self.is_server_name: self.rewritten_is_url
         }
 
-        mock_http_client = Mock(spec=["post_json_get_json"])
+        mock_http_client = Mock(spec=["get_json", "post_json_get_json"])
+        mock_http_client.get_json.side_effect = defer.succeed({})
         mock_http_client.post_json_get_json.return_value = defer.succeed(
             {"address": self.address, "medium": "email"}
         )
@@ -52,6 +53,19 @@ def make_homeserver(self, reactor, clock):
             config=config, simple_http_client=mock_http_client
         )
 
+        mock_blacklisting_http_client = Mock(spec=["get_json", "post_json_get_json"])
+        mock_blacklisting_http_client.get_json.side_effect = defer.succeed({})
+        mock_blacklisting_http_client.post_json_get_json.return_value = defer.succeed(
+            {"address": self.address, "medium": "email"}
+        )
+
+        # TODO: This class does not use a singleton to get it's http client
+        # This should be fixed for easier testing
+        # https://github.com/matrix-org/synapse-dinsic/issues/26
+        self.hs.get_handlers().identity_handler.blacklisting_http_client = (
+            mock_blacklisting_http_client
+        )
+
         return self.hs
 
     def prepare(self, reactor, clock, hs):
@@ -65,7 +79,7 @@ def test_rewritten_id_server(self):
         * the original, non-rewritten, server name is stored in the database
         """
         handler = self.hs.get_handlers().identity_handler
-        post_json_get_json = self.hs.get_simple_http_client().post_json_get_json
+        post_json_get_json = handler.blacklisting_http_client.post_json_get_json
         store = self.hs.get_datastore()
 
         creds = {"sid": "123", "client_secret": "some_secret"}

tests/rest/client/test_identity.py

@@ -131,6 +131,14 @@ def test_3pid_invite_enabled(self):
         self.assertEquals(channel.result["code"], b"200", channel.result)
         room_id = channel.json_body["room_id"]
 
+        # Replace the blacklisting SimpleHttpClient with our mock
+        self.hs.get_room_member_handler().simple_http_client = Mock(
+            spec=["get_json", "post_json_get_json"]
+        )
+        self.hs.get_room_member_handler().simple_http_client.get_json.return_value = (
+            defer.succeed((200, "{}"))
+        )
+
         params = {
             "id_server": "testis",
             "medium": "email",
@@ -143,7 +151,7 @@ def test_3pid_invite_enabled(self):
         )
         self.render(request)
 
-        get_json = self.hs.get_simple_http_client().get_json
+        get_json = self.hs.get_room_member_handler().simple_http_client.get_json
         get_json.assert_called_once_with(
             "https://testis/_matrix/identity/api/v1/lookup",
             {"address": "[email protected]", "medium": "email"},

tests/rest/client/test_room_access_rules.py

@@ -96,6 +96,11 @@ def post_json_get_json(uri, post_json, args={}, headers=None):
             simple_http_client=mock_http_client,
         )
 
+        # TODO: This class does not use a singleton to get it's http client
+        # This should be fixed for easier testing
+        # https://github.com/matrix-org/synapse-dinsic/issues/26
+        self.hs.get_room_member_handler().simple_http_client = mock_http_client
+
         return self.hs
 
     def prepare(self, reactor, clock, homeserver):