Merge branch 'master' into develop

CHANGES.md

@@ -1,3 +1,18 @@
+Synapse 1.11.1 (2020-03-03)
+===========================
+
+This release includes a security fix impacting installations using Single Sign-On (i.e. SAML2 or CAS) for authentication. Administrators of such installations are encouraged to upgrade as soon as possible.
+
+The release also includes fixes for a couple of other bugs.
+
+Bugfixes
+--------
+
+- Add a confirmation step to the SSO login flow before redirecting users to the redirect URL. ([b2bd54a2](https://github.com/matrix-org/synapse/commit/b2bd54a2e31d9a248f73fadb184ae9b4cbdb49f9), [65c73cdf](https://github.com/matrix-org/synapse/commit/65c73cdfec1876a9fec2fd2c3a74923cd146fe0b), [a0178df1](https://github.com/matrix-org/synapse/commit/a0178df10422a76fd403b82d2b2a4ed28a9a9d1e))
+- Fixed set a user as an admin with the admin API `PUT /_synapse/admin/v2/users/<user_id>`. Contributed by @dklimpel. ([\#6910](https://github.com/matrix-org/synapse/issues/6910))
+- Fix bug introduced in Synapse 1.11.0 which sometimes caused errors when joining rooms over federation, with `'coroutine' object has no attribute 'event_id'`. ([\#6996](https://github.com/matrix-org/synapse/issues/6996))
+
+
 Synapse 1.11.0 (2020-02-21)
 ===========================
 

changelog.d/6996.bugfix

@@ -1 +0,0 @@
-Fix bug which caused an error when joining a room, with `'coroutine' object has no attribute 'event_id'`.

debian/changelog

@@ -1,3 +1,9 @@
+matrix-synapse-py3 (1.11.1) stable; urgency=medium
+
+  * New synapse release 1.11.1.
+
+ -- Synapse Packaging team <packages@matrix.org>  Tue, 03 Mar 2020 15:01:22 +0000
+
 matrix-synapse-py3 (1.11.0) stable; urgency=medium
 
   * New synapse release 1.11.0.

docs/sample_config.yaml

@@ -1360,6 +1360,56 @@ saml2_config:
 #   #    name: value
 
 
+# Additional settings to use with single-sign on systems such as SAML2 and CAS.
+#
+sso:
+    # A list of client URLs which are whitelisted so that the user does not
+    # have to confirm giving access to their account to the URL. Any client
+    # whose URL starts with an entry in the following list will not be subject
+    # to an additional confirmation step after the SSO login is completed.
+    #
+    # WARNING: An entry such as "https://my.client" is insecure, because it
+    # will also match "https://my.client.evil.site", exposing your users to
+    # phishing attacks from evil.site. To avoid this, include a slash after the
+    # hostname: "https://my.client/".
+    #
+    # By default, this list is empty.
+    #
+    #client_whitelist:
+    #  - https://riot.im/develop
+    #  - https://my.custom.client/
+
+    # Directory in which Synapse will try to find the template files below.
+    # If not set, default templates from within the Synapse package will be used.
+    #
+    # DO NOT UNCOMMENT THIS SETTING unless you want to customise the templates.
+    # If you *do* uncomment it, you will need to make sure that all the templates
+    # below are in the directory.
+    #
+    # Synapse will look for the following templates in this directory:
+    #
+    # * HTML page for a confirmation step before redirecting back to the client
+    #   with the login token: 'sso_redirect_confirm.html'.
+    #
+    #   When rendering, this template is given three variables:
+    #     * redirect_url: the URL the user is about to be redirected to. Needs
+    #                     manual escaping (see
+    #                     https://jinja.palletsprojects.com/en/2.11.x/templates/#html-escaping).
+    #
+    #     * display_url: the same as `redirect_url`, but with the query
+    #                    parameters stripped. The intention is to have a
+    #                    human-readable URL to show to users, not to use it as
+    #                    the final address to redirect to. Needs manual escaping
+    #                    (see https://jinja.palletsprojects.com/en/2.11.x/templates/#html-escaping).
+    #
+    #     * server_name: the homeserver's name.
+    #
+    # You can see the default templates at:
+    # https://github.com/matrix-org/synapse/tree/master/synapse/res/templates
+    #
+    #template_dir: "res/templates"
+
+
 # The JWT needs to contain a globally unique "sub" (subject) claim.
 #
 #jwt_config:

synapse/__init__.py

@@ -36,7 +36,7 @@ try:
 except ImportError:
     pass
 
-__version__ = "1.11.0"
+__version__ = "1.11.1"
 
 if bool(os.environ.get("SYNAPSE_TEST_PATCH_LOG_CONTEXTS", False)):
     # We import here so that we don't have to install a bunch of deps when

synapse/config/_base.pyi

@@ -24,6 +24,7 @@ from synapse.config import (
     server,
     server_notices_config,
     spam_checker,
+    sso,
     stats,
     third_party_event_rules,
     tls,
@@ -57,6 +58,7 @@ class RootConfig:
     key: key.KeyConfig
     saml2: saml2_config.SAML2Config
     cas: cas.CasConfig
+    sso: sso.SSOConfig
     jwt: jwt_config.JWTConfig
     password: password.PasswordConfig
     email: emailconfig.EmailConfig

synapse/config/homeserver.py

@@ -38,6 +38,7 @@ from .saml2_config import SAML2Config
 from .server import ServerConfig
 from .server_notices_config import ServerNoticesConfig
 from .spam_checker import SpamCheckerConfig
+from .sso import SSOConfig
 from .stats import StatsConfig
 from .third_party_event_rules import ThirdPartyRulesConfig
 from .tls import TlsConfig
@@ -65,6 +66,7 @@ class HomeServerConfig(RootConfig):
         KeyConfig,
         SAML2Config,
         CasConfig,
+        SSOConfig,
         JWTConfig,
         PasswordConfig,
         EmailConfig,

synapse/config/sso.py

@@ -0,0 +1,92 @@
+# -*- coding: utf-8 -*-
+# Copyright 2020 The Matrix.org Foundation C.I.C.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+from typing import Any, Dict
+
+import pkg_resources
+
+from ._base import Config
+
+
+class SSOConfig(Config):
+    """SSO Configuration
+    """
+
+    section = "sso"
+
+    def read_config(self, config, **kwargs):
+        sso_config = config.get("sso") or {}  # type: Dict[str, Any]
+
+        # Pick a template directory in order of:
+        # * The sso-specific template_dir
+        # * /path/to/synapse/install/res/templates
+        template_dir = sso_config.get("template_dir")
+        if not template_dir:
+            template_dir = pkg_resources.resource_filename("synapse", "res/templates",)
+
+        self.sso_redirect_confirm_template_dir = template_dir
+
+        self.sso_client_whitelist = sso_config.get("client_whitelist") or []
+
+    def generate_config_section(self, **kwargs):
+        return """\
+        # Additional settings to use with single-sign on systems such as SAML2 and CAS.
+        #
+        sso:
+            # A list of client URLs which are whitelisted so that the user does not
+            # have to confirm giving access to their account to the URL. Any client
+            # whose URL starts with an entry in the following list will not be subject
+            # to an additional confirmation step after the SSO login is completed.
+            #
+            # WARNING: An entry such as "https://my.client" is insecure, because it
+            # will also match "https://my.client.evil.site", exposing your users to
+            # phishing attacks from evil.site. To avoid this, include a slash after the
+            # hostname: "https://my.client/".
+            #
+            # By default, this list is empty.
+            #
+            #client_whitelist:
+            #  - https://riot.im/develop
+            #  - https://my.custom.client/
+
+            # Directory in which Synapse will try to find the template files below.
+            # If not set, default templates from within the Synapse package will be used.
+            #
+            # DO NOT UNCOMMENT THIS SETTING unless you want to customise the templates.
+            # If you *do* uncomment it, you will need to make sure that all the templates
+            # below are in the directory.
+            #
+            # Synapse will look for the following templates in this directory:
+            #
+            # * HTML page for a confirmation step before redirecting back to the client
+            #   with the login token: 'sso_redirect_confirm.html'.
+            #
+            #   When rendering, this template is given three variables:
+            #     * redirect_url: the URL the user is about to be redirected to. Needs
+            #                     manual escaping (see
+            #                     https://jinja.palletsprojects.com/en/2.11.x/templates/#html-escaping).
+            #
+            #     * display_url: the same as `redirect_url`, but with the query
+            #                    parameters stripped. The intention is to have a
+            #                    human-readable URL to show to users, not to use it as
+            #                    the final address to redirect to. Needs manual escaping
+            #                    (see https://jinja.palletsprojects.com/en/2.11.x/templates/#html-escaping).
+            #
+            #     * server_name: the homeserver's name.
+            #
+            # You can see the default templates at:
+            # https://github.com/matrix-org/synapse/tree/master/synapse/res/templates
+            #
+            #template_dir: "res/templates"
+        """

synapse/handlers/auth.py

@@ -17,6 +17,8 @@
 import logging
 import time
 import unicodedata
+import urllib.parse
+from typing import Any
 
 import attr
 import bcrypt
@@ -38,8 +40,11 @@ from synapse.api.errors import (
 from synapse.api.ratelimiting import Ratelimiter
 from synapse.handlers.ui_auth import INTERACTIVE_AUTH_CHECKERS
 from synapse.handlers.ui_auth.checkers import UserInteractiveAuthChecker
+from synapse.http.server import finish_request
+from synapse.http.site import SynapseRequest
 from synapse.logging.context import defer_to_thread
 from synapse.module_api import ModuleApi
+from synapse.push.mailer import load_jinja2_templates
 from synapse.types import UserID
 from synapse.util.caches.expiringcache import ExpiringCache
 
@@ -108,6 +113,16 @@ class AuthHandler(BaseHandler):
 
         self._clock = self.hs.get_clock()
 
+        # Load the SSO redirect confirmation page HTML template
+        self._sso_redirect_confirm_template = load_jinja2_templates(
+            hs.config.sso_redirect_confirm_template_dir, ["sso_redirect_confirm.html"],
+        )[0]
+
+        self._server_name = hs.config.server_name
+
+        # cast to tuple for use with str.startswith
+        self._whitelisted_sso_clients = tuple(hs.config.sso_client_whitelist)
+
     @defer.inlineCallbacks
     def validate_user_via_ui_auth(self, requester, request_body, clientip):
         """
@@ -927,6 +942,65 @@ class AuthHandler(BaseHandler):
         else:
             return defer.succeed(False)
 
+    def complete_sso_login(
+        self,
+        registered_user_id: str,
+        request: SynapseRequest,
+        client_redirect_url: str,
+    ):
+        """Having figured out a mxid for this user, complete the HTTP request
+
+        Args:
+            registered_user_id: The registered user ID to complete SSO login for.
+            request: The request to complete.
+            client_redirect_url: The URL to which to redirect the user at the end of the
+                process.
+        """
+        # Create a login token
+        login_token = self.macaroon_gen.generate_short_term_login_token(
+            registered_user_id
+        )
+
+        # Append the login token to the original redirect URL (i.e. with its query
+        # parameters kept intact) to build the URL to which the template needs to
+        # redirect the users once they have clicked on the confirmation link.
+        redirect_url = self.add_query_param_to_url(
+            client_redirect_url, "loginToken", login_token
+        )
+
+        # if the client is whitelisted, we can redirect straight to it
+        if client_redirect_url.startswith(self._whitelisted_sso_clients):
+            request.redirect(redirect_url)
+            finish_request(request)
+            return
+
+        # Otherwise, serve the redirect confirmation page.
+
+        # Remove the query parameters from the redirect URL to get a shorter version of
+        # it. This is only to display a human-readable URL in the template, but not the
+        # URL we redirect users to.
+        redirect_url_no_params = client_redirect_url.split("?")[0]
+
+        html = self._sso_redirect_confirm_template.render(
+            display_url=redirect_url_no_params,
+            redirect_url=redirect_url,
+            server_name=self._server_name,
+        ).encode("utf-8")
+
+        request.setResponseCode(200)
+        request.setHeader(b"Content-Type", b"text/html; charset=utf-8")
+        request.setHeader(b"Content-Length", b"%d" % (len(html),))
+        request.write(html)
+        finish_request(request)
+
+    @staticmethod
+    def add_query_param_to_url(url: str, param_name: str, param: Any):
+        url_parts = list(urllib.parse.urlparse(url))
+        query = dict(urllib.parse.parse_qsl(url_parts[4]))
+        query.update({param_name: param})
+        url_parts[4] = urllib.parse.urlencode(query)
+        return urllib.parse.urlunparse(url_parts)
+
 
 @attr.s
 class MacaroonGenerator(object):

synapse/handlers/saml_handler.py

@@ -25,7 +25,6 @@ from synapse.api.errors import SynapseError
 from synapse.config import ConfigError
 from synapse.http.servlet import parse_string
 from synapse.module_api import ModuleApi
-from synapse.rest.client.v1.login import SSOAuthHandler
 from synapse.types import (
     UserID,
     map_username_to_mxid_localpart,
@@ -48,7 +47,7 @@ class Saml2SessionData:
 class SamlHandler:
     def __init__(self, hs):
         self._saml_client = Saml2Client(hs.config.saml2_sp_config)
-        self._sso_auth_handler = SSOAuthHandler(hs)
+        self._auth_handler = hs.get_auth_handler()
         self._registration_handler = hs.get_registration_handler()
 
         self._clock = hs.get_clock()
@@ -116,7 +115,7 @@ class SamlHandler:
         self.expire_sessions()
 
         user_id = await self._map_saml_response_to_user(resp_bytes, relay_state)
-        self._sso_auth_handler.complete_sso_login(user_id, request, relay_state)
+        self._auth_handler.complete_sso_login(user_id, request, relay_state)
 
     async def _map_saml_response_to_user(self, resp_bytes, client_redirect_url):
         try:

synapse/module_api/__init__.py

@@ -17,6 +17,7 @@ import logging
 
 from twisted.internet import defer
 
+from synapse.http.site import SynapseRequest
 from synapse.logging.context import make_deferred_yieldable, run_in_background
 from synapse.types import UserID
 
@@ -211,3 +212,21 @@ class ModuleApi(object):
             Deferred[object]: result of func
         """
         return self._store.db.runInteraction(desc, func, *args, **kwargs)
+
+    def complete_sso_login(
+        self, registered_user_id: str, request: SynapseRequest, client_redirect_url: str
+    ):
+        """Complete a SSO login by redirecting the user to a page to confirm whether they
+        want their access token sent to `client_redirect_url`, or redirect them to that
+        URL with a token directly if the URL matches with one of the whitelisted clients.
+
+        Args:
+            registered_user_id: The MXID that has been registered as a previous step of
+                of this SSO login.
+            request: The request to respond to.
+            client_redirect_url: The URL to which to offer to redirect the user (or to
+                redirect them directly if whitelisted).
+        """
+        self._auth_handler.complete_sso_login(
+            registered_user_id, request, client_redirect_url,
+        )

synapse/res/templates/sso_redirect_confirm.html

@@ -0,0 +1,14 @@
+<!DOCTYPE html>
+<html lang="en">
+<head>
+    <meta charset="UTF-8">
+    <title>SSO redirect confirmation</title>
+</head>
+    <body>
+        <p>The application at <span style="font-weight:bold">{{ display_url | e }}</span> is requesting full access to your <span style="font-weight:bold">{{ server_name }}</span> Matrix account.</p>
+        <p>If you don't recognise this address, you should ignore this and close this tab.</p>
+        <p>
+            <a href="{{ redirect_url | e }}">I trust this address</a>
+        </p>
+    </body>
+</html>

synapse/rest/client/v1/login.py

@@ -28,7 +28,7 @@ from synapse.http.servlet import (
     parse_json_object_from_request,
     parse_string,
 )
-from synapse.http.site import SynapseRequest
+from synapse.push.mailer import load_jinja2_templates
 from synapse.rest.client.v2_alpha._base import client_patterns
 from synapse.rest.well_known import WellKnownBuilder
 from synapse.types import UserID, map_username_to_mxid_localpart
@@ -548,6 +548,16 @@ class SSOAuthHandler(object):
         self._registration_handler = hs.get_registration_handler()
         self._macaroon_gen = hs.get_macaroon_generator()
 
+        # Load the redirect page HTML template
+        self._template = load_jinja2_templates(
+            hs.config.sso_redirect_confirm_template_dir, ["sso_redirect_confirm.html"],
+        )[0]
+
+        self._server_name = hs.config.server_name
+
+        # cast to tuple for use with str.startswith
+        self._whitelisted_sso_clients = tuple(hs.config.sso_client_whitelist)
+
     async def on_successful_auth(
         self, username, request, client_redirect_url, user_display_name=None
     ):
@@ -580,36 +590,9 @@ class SSOAuthHandler(object):
                 localpart=localpart, default_display_name=user_display_name
             )
 
-        self.complete_sso_login(registered_user_id, request, client_redirect_url)
+        self._auth_handler.complete_sso_login(
-
+            registered_user_id, request, client_redirect_url
-    def complete_sso_login(
-        self, registered_user_id: str, request: SynapseRequest, client_redirect_url: str
-    ):
-        """Having figured out a mxid for this user, complete the HTTP request
-
-        Args:
-            registered_user_id:
-            request:
-            client_redirect_url:
-        """
-
-        login_token = self._macaroon_gen.generate_short_term_login_token(
-            registered_user_id
         )
-        redirect_url = self._add_login_token_to_redirect_url(
-            client_redirect_url, login_token
-        )
-        # Load page
-        request.redirect(redirect_url)
-        finish_request(request)
-
-    @staticmethod
-    def _add_login_token_to_redirect_url(url, token):
-        url_parts = list(urllib.parse.urlparse(url))
-        query = dict(urllib.parse.parse_qsl(url_parts[4]))
-        query.update({"loginToken": token})
-        url_parts[4] = urllib.parse.urlencode(query)
-        return urllib.parse.urlunparse(url_parts)
 
 
 def register_servlets(hs, http_server):

tests/rest/client/v1/test_login.py

@@ -1,4 +1,7 @@
 import json
+import urllib.parse
+
+from mock import Mock
 
 import synapse.rest.admin
 from synapse.rest.client.v1 import login
@@ -252,3 +255,111 @@ class LoginRestServletTestCase(unittest.HomeserverTestCase):
         )
         self.render(request)
         self.assertEquals(channel.code, 200, channel.result)
+
+
+class CASRedirectConfirmTestCase(unittest.HomeserverTestCase):
+
+    servlets = [
+        login.register_servlets,
+    ]
+
+    def make_homeserver(self, reactor, clock):
+        self.base_url = "https://matrix.goodserver.com/"
+        self.redirect_path = "_synapse/client/login/sso/redirect/confirm"
+
+        config = self.default_config()
+        config["cas_config"] = {
+            "enabled": True,
+            "server_url": "https://fake.test",
+            "service_url": "https://matrix.goodserver.com:8448",
+        }
+
+        async def get_raw(uri, args):
+            """Return an example response payload from a call to the `/proxyValidate`
+            endpoint of a CAS server, copied from
+            https://apereo.github.io/cas/5.0.x/protocol/CAS-Protocol-V2-Specification.html#26-proxyvalidate-cas-20
+
+            This needs to be returned by an async function (as opposed to set as the
+            mock's return value) because the corresponding Synapse code awaits on it.
+            """
+            return """
+                <cas:serviceResponse xmlns:cas='http://www.yale.edu/tp/cas'>
+                  <cas:authenticationSuccess>
+                      <cas:user>username</cas:user>
+                      <cas:proxyGrantingTicket>PGTIOU-84678-8a9d...</cas:proxyGrantingTicket>
+                      <cas:proxies>
+                          <cas:proxy>https://proxy2/pgtUrl</cas:proxy>
+                          <cas:proxy>https://proxy1/pgtUrl</cas:proxy>
+                      </cas:proxies>
+                  </cas:authenticationSuccess>
+                </cas:serviceResponse>
+            """
+
+        mocked_http_client = Mock(spec=["get_raw"])
+        mocked_http_client.get_raw.side_effect = get_raw
+
+        self.hs = self.setup_test_homeserver(
+            config=config, proxied_http_client=mocked_http_client,
+        )
+
+        return self.hs
+
+    def test_cas_redirect_confirm(self):
+        """Tests that the SSO login flow serves a confirmation page before redirecting a
+        user to the redirect URL.
+        """
+        base_url = "/_matrix/client/r0/login/cas/ticket?redirectUrl"
+        redirect_url = "https://dodgy-site.com/"
+
+        url_parts = list(urllib.parse.urlparse(base_url))
+        query = dict(urllib.parse.parse_qsl(url_parts[4]))
+        query.update({"redirectUrl": redirect_url})
+        query.update({"ticket": "ticket"})
+        url_parts[4] = urllib.parse.urlencode(query)
+        cas_ticket_url = urllib.parse.urlunparse(url_parts)
+
+        # Get Synapse to call the fake CAS and serve the template.
+        request, channel = self.make_request("GET", cas_ticket_url)
+        self.render(request)
+
+        # Test that the response is HTML.
+        self.assertEqual(channel.code, 200)
+        content_type_header_value = ""
+        for header in channel.result.get("headers", []):
+            if header[0] == b"Content-Type":
+                content_type_header_value = header[1].decode("utf8")
+
+        self.assertTrue(content_type_header_value.startswith("text/html"))
+
+        # Test that the body isn't empty.
+        self.assertTrue(len(channel.result["body"]) > 0)
+
+        # And that it contains our redirect link
+        self.assertIn(redirect_url, channel.result["body"].decode("UTF-8"))
+
+    @override_config(
+        {
+            "sso": {
+                "client_whitelist": [
+                    "https://legit-site.com/",
+                    "https://other-site.com/",
+                ]
+            }
+        }
+    )
+    def test_cas_redirect_whitelisted(self):
+        """Tests that the SSO login flow serves a redirect to a whitelisted url
+        """
+        redirect_url = "https://legit-site.com/"
+        cas_ticket_url = (
+            "/_matrix/client/r0/login/cas/ticket?redirectUrl=%s&ticket=ticket"
+            % (urllib.parse.quote(redirect_url))
+        )
+
+        # Get Synapse to call the fake CAS and serve the template.
+        request, channel = self.make_request("GET", cas_ticket_url)
+        self.render(request)
+
+        self.assertEqual(channel.code, 302)
+        location_headers = channel.headers.getRawHeaders("Location")
+        self.assertEqual(location_headers[0][: len(redirect_url)], redirect_url)