Use the federation blacklist for requests to untrusted Identity Servers (#6000)

8 files changed, 57 insertions, 6 deletions

diff --git a/changelog.d/6000.feature b/changelog.d/6000.feature
new file mode 100644
index 0000000000..0a159bd10d
--- /dev/null
+++ b/changelog.d/6000.feature
@@ -0,0 +1 @@
+Apply the federation blacklist to requests to identity servers.
\ No newline at end of file
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index 722f111ad8..1b101b2ff5 100644
--- a/docs/sample_config.yaml
+++ b/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.)
 #
diff --git a/synapse/config/server.py b/synapse/config/server.py
index 71c13828cb..a2a5767ebe 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -682,6 +682,9 @@ class ServerConfig(Config):
         # 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.)
         #
diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py
index a8be376472..2f0f61ad72 100644
--- a/synapse/handlers/identity.py
+++ b/synapse/handlers/identity.py
@@ -36,6 +36,7 @@ from synapse.api.errors import (
     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 @@ class IdentityHandler(BaseHandler):
 
         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 @@ class IdentityHandler(BaseHandler):
             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 @@ class IdentityHandler(BaseHandler):
         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
diff --git a/synapse/handlers/room_member.py b/synapse/handlers/room_member.py
index b3ade954a2..c1cfb2d6b8 100644
--- a/synapse/handlers/room_member.py
+++ b/synapse/handlers/room_member.py
@@ -37,6 +37,7 @@ from synapse.api.errors import (
     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 @@ class RoomMemberHandler(object):
         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
diff --git a/tests/handlers/test_identity.py b/tests/handlers/test_identity.py
index 237e2efb9a..34f6bfb422 100644
--- a/tests/handlers/test_identity.py
+++ b/tests/handlers/test_identity.py
@@ -43,7 +43,8 @@ class ThreepidISRewrittenURLTestCase(unittest.HomeserverTestCase):
             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 @@ class ThreepidISRewrittenURLTestCase(unittest.HomeserverTestCase):
             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 @@ class ThreepidISRewrittenURLTestCase(unittest.HomeserverTestCase):
         * 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"}
diff --git a/tests/rest/client/test_identity.py b/tests/rest/client/test_identity.py
index f81f81602e..b7f57781fc 100644
--- a/tests/rest/client/test_identity.py
+++ b/tests/rest/client/test_identity.py
@@ -131,6 +131,14 @@ class IdentityEnabledTestCase(unittest.HomeserverTestCase):
         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 @@ class IdentityEnabledTestCase(unittest.HomeserverTestCase):
         )
         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": "test@example.com", "medium": "email"},
diff --git a/tests/rest/client/test_room_access_rules.py b/tests/rest/client/test_room_access_rules.py
index d44f5c2c8c..b82d6242ef 100644
--- a/tests/rest/client/test_room_access_rules.py
+++ b/tests/rest/client/test_room_access_rules.py
@@ -96,6 +96,11 @@ class RoomAccessTestCase(unittest.HomeserverTestCase):
             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):