From 145f57897db3df082a06a791e547332278d32bfe Mon Sep 17 00:00:00 2001 From: Ike Johnson Date: Sun, 2 Jun 2019 23:10:27 +0800 Subject: Update HAProxy example rules These new rules allow a user to instead route only matrix traffic, allowing them to run matrix on the domain without affecting their existing websites --- docs/reverse_proxy.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'docs') diff --git a/docs/reverse_proxy.rst b/docs/reverse_proxy.rst index 7619b1097b..e4b870411c 100644 --- a/docs/reverse_proxy.rst +++ b/docs/reverse_proxy.rst @@ -89,8 +89,10 @@ Let's assume that we expect clients to connect to our server at bind :::443 v4v6 ssl crt /etc/ssl/haproxy/ strict-sni alpn h2,http/1.1 # Matrix client traffic - acl matrix hdr(host) -i matrix.example.com - use_backend matrix if matrix + acl matrix-host hdr(host) -i matrix.example.com + acl matrix-path path_beg /_matrix + + use_backend matrix if matrix-host matrix-path frontend matrix-federation bind :::8448 v4v6 ssl crt /etc/ssl/haproxy/synapse.pem alpn h2,http/1.1 -- cgit 1.5.1 From 3f8a252dd86fecff6cdda58043aaba7b79436e01 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 21 Jun 2019 13:46:39 +0100 Subject: Add "--open-private-ports" cmdline option This is helpful when generating a config file for running synapse under docker. --- docs/sample_config.yaml | 2 +- synapse/config/_base.py | 14 ++++++++++++++ synapse/config/server.py | 17 ++++++++++++----- 3 files changed, 27 insertions(+), 6 deletions(-) (limited to 'docs') diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index bb07b02f4e..522ec7e8ff 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -209,7 +209,7 @@ listeners: - names: [client, federation] compress: false - # example additonal_resources: + # example additional_resources: # #additional_resources: # "/_matrix/my/custom/endpoint": diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 8654b0f4a1..965478d8d5 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -150,6 +150,7 @@ class Config(object): server_name, generate_secrets=False, report_stats=None, + open_private_ports=False, ): """Build a default configuration file @@ -173,6 +174,9 @@ class Config(object): report_stats (bool|None): Initial setting for the report_stats setting. If None, report_stats will be left unset. + open_private_ports (bool): True to leave private ports (such as the non-TLS + HTTP listener) open to the internet. + Returns: str: the yaml config file """ @@ -185,6 +189,7 @@ class Config(object): server_name=server_name, generate_secrets=generate_secrets, report_stats=report_stats, + open_private_ports=open_private_ports, ) ) @@ -298,6 +303,14 @@ class Config(object): " stored. Defaults to the current working directory." ), ) + generate_group.add_argument( + "--open-private-ports", + action="store_true", + help=( + "Leave private ports (such as the non-TLS HTTP listener) open to the" + " internet. Do not use this unless you know what you are doing." + ), + ) config_args, remaining_args = config_parser.parse_known_args(argv) @@ -351,6 +364,7 @@ class Config(object): server_name=server_name, report_stats=(config_args.report_stats == "yes"), generate_secrets=True, + open_private_ports=config_args.open_private_ports, ) if not cls.path_exists(config_dir_path): diff --git a/synapse/config/server.py b/synapse/config/server.py index 2f5d1a6ae3..c1b2ccfe45 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -307,7 +307,9 @@ class ServerConfig(Config): def has_tls_listener(self): return any(l["tls"] for l in self.listeners) - def generate_config_section(self, server_name, data_dir_path, **kwargs): + def generate_config_section( + self, server_name, data_dir_path, open_private_ports, **kwargs + ): _, bind_port = parse_and_validate_server_name(server_name) if bind_port is not None: unsecure_port = bind_port - 400 @@ -320,6 +322,13 @@ class ServerConfig(Config): # Bring DEFAULT_ROOM_VERSION into the local-scope for use in the # default config string default_room_version = DEFAULT_ROOM_VERSION + + unsecure_http_binding = "port: %i\n tls: false" % (unsecure_port,) + if not open_private_ports: + unsecure_http_binding += ( + "\n bind_addresses: ['::1', '127.0.0.1']" + ) + return ( """\ ## Server ## @@ -511,9 +520,7 @@ class ServerConfig(Config): # If you plan to use a reverse proxy, please see # https://github.com/matrix-org/synapse/blob/master/docs/reverse_proxy.rst. # - - port: %(unsecure_port)s - tls: false - bind_addresses: ['::1', '127.0.0.1'] + - %(unsecure_http_binding)s type: http x_forwarded: true @@ -521,7 +528,7 @@ class ServerConfig(Config): - names: [client, federation] compress: false - # example additonal_resources: + # example additional_resources: # #additional_resources: # "/_matrix/my/custom/endpoint": -- cgit 1.5.1 From bfe84e051e8472a1c3534e1287f1fb727df63259 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 24 Jun 2019 11:45:11 +0100 Subject: Split public rooms directory auth config in two --- changelog.d/5534.feature | 1 + docs/sample_config.yaml | 12 ++++++---- synapse/config/server.py | 44 ++++++++++++++++++++++++++-------- synapse/federation/transport/server.py | 8 +++---- synapse/rest/client/v1/room.py | 2 +- tests/rest/client/v1/test_rooms.py | 2 +- 6 files changed, 49 insertions(+), 20 deletions(-) create mode 100644 changelog.d/5534.feature (limited to 'docs') diff --git a/changelog.d/5534.feature b/changelog.d/5534.feature new file mode 100644 index 0000000000..2e279c9b77 --- /dev/null +++ b/changelog.d/5534.feature @@ -0,0 +1 @@ +Split public rooms directory auth config in two settings, in order to manage client auth independently from the federation part of it. Obsoletes the "restrict_public_rooms_to_local_users" configuration setting. If "restrict_public_rooms_to_local_users" is set in the config, Synapse will act as if both new options are enabled, i.e. require authentication through the client API and deny federation requests. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index d5cc3e7abc..f4fd113211 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -54,11 +54,15 @@ pid_file: DATADIR/homeserver.pid # #require_auth_for_profile_requests: true -# If set to 'true', requires authentication to access the server's -# public rooms directory through the client API, and forbids any other -# homeserver to fetch it via federation. Defaults to 'false'. +# If set to 'false', requires authentication to access the server's public rooms +# directory through the client API. Defaults to 'true'. # -#restrict_public_rooms_to_local_users: true +#allow_public_rooms_without_auth: false + +# If set to 'false', forbids any other homeserver to fetch the server's public +# rooms directory via federation. Defaults to 'true'. +# +#allow_public_rooms_over_federation: false # The default room version for newly created rooms. # diff --git a/synapse/config/server.py b/synapse/config/server.py index 1e58b2e91b..7cbb699a66 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -82,12 +82,32 @@ class ServerConfig(Config): "require_auth_for_profile_requests", False ) - # If set to 'True', requires authentication to access the server's - # public rooms directory through the client API, and forbids any other - # homeserver to fetch it via federation. - self.restrict_public_rooms_to_local_users = config.get( - "restrict_public_rooms_to_local_users", False - ) + if "restrict_public_rooms_to_local_users" in config and ( + "allow_public_rooms_without_auth" in config + or "allow_public_rooms_over_federation" in config + ): + raise ConfigError( + "Can't use 'restrict_public_rooms_to_local_users' if" + " 'allow_public_rooms_without_auth' and/or" + " 'allow_public_rooms_over_federation' is set." + ) + + # Check if the legacy "restrict_public_rooms_to_local_users" flag is set. This + # flag is now obsolete but we need to check it for backward-compatibility. + if config.get("restrict_public_rooms_to_local_users", False): + self.allow_public_rooms_without_auth = False + self.allow_public_rooms_over_federation = False + else: + # If set to 'False', requires authentication to access the server's public + # rooms directory through the client API. Defaults to 'True'. + self.allow_public_rooms_without_auth = config.get( + "allow_public_rooms_without_auth", True + ) + # If set to 'False', forbids any other homeserver to fetch the server's public + # rooms directory via federation. Defaults to 'True'. + self.allow_public_rooms_over_federation = config.get( + "allow_public_rooms_over_federation", True + ) default_room_version = config.get("default_room_version", DEFAULT_ROOM_VERSION) @@ -366,11 +386,15 @@ class ServerConfig(Config): # #require_auth_for_profile_requests: true - # If set to 'true', requires authentication to access the server's - # public rooms directory through the client API, and forbids any other - # homeserver to fetch it via federation. Defaults to 'false'. + # If set to 'false', requires authentication to access the server's public rooms + # directory through the client API. Defaults to 'true'. + # + #allow_public_rooms_without_auth: false + + # If set to 'false', forbids any other homeserver to fetch the server's public + # rooms directory via federation. Defaults to 'true'. # - #restrict_public_rooms_to_local_users: true + #allow_public_rooms_over_federation: false # The default room version for newly created rooms. # diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index b4854e82f6..955f0f4308 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -721,15 +721,15 @@ class PublicRoomList(BaseFederationServlet): PATH = "/publicRooms" - def __init__(self, handler, authenticator, ratelimiter, server_name, deny_access): + def __init__(self, handler, authenticator, ratelimiter, server_name, allow_access): super(PublicRoomList, self).__init__( handler, authenticator, ratelimiter, server_name ) - self.deny_access = deny_access + self.allow_access = allow_access @defer.inlineCallbacks def on_GET(self, origin, content, query): - if self.deny_access: + if not self.allow_access: raise FederationDeniedError(origin) limit = parse_integer_from_args(query, "limit", 0) @@ -1436,7 +1436,7 @@ def register_servlets(hs, resource, authenticator, ratelimiter, servlet_groups=N authenticator=authenticator, ratelimiter=ratelimiter, server_name=hs.hostname, - deny_access=hs.config.restrict_public_rooms_to_local_users, + allow_access=hs.config.allow_public_rooms_over_federation, ).register(resource) if "group_server" in servlet_groups: diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index a028337125..cca7e45ddb 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -311,7 +311,7 @@ class PublicRoomListRestServlet(TransactionRestServlet): # Option to allow servers to require auth when accessing # /publicRooms via CS API. This is especially helpful in private # federations. - if self.hs.config.restrict_public_rooms_to_local_users: + if not self.hs.config.allow_public_rooms_without_auth: raise # We allow people to not be authed if they're just looking at our diff --git a/tests/rest/client/v1/test_rooms.py b/tests/rest/client/v1/test_rooms.py index 2e3a765bf3..fe741637f5 100644 --- a/tests/rest/client/v1/test_rooms.py +++ b/tests/rest/client/v1/test_rooms.py @@ -920,7 +920,7 @@ class PublicRoomsRestrictedTestCase(unittest.HomeserverTestCase): self.url = b"/_matrix/client/r0/publicRooms" config = self.default_config() - config["restrict_public_rooms_to_local_users"] = True + config["allow_public_rooms_without_auth"] = False self.hs = self.setup_test_homeserver(config=config) return self.hs -- cgit 1.5.1 From 28604ab03df92fa9636933a11c1cbf30f3111eed Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Mon, 24 Jun 2019 17:48:05 +0100 Subject: Add info about black to code_style.rst (#5537) Fixes #5533 Adds information about how to install and run black on the codebase. --- changelog.d/5537.misc | 1 + docs/code_style.rst | 87 +++++++++++++++++++++++++-------------------------- 2 files changed, 43 insertions(+), 45 deletions(-) create mode 100644 changelog.d/5537.misc (limited to 'docs') diff --git a/changelog.d/5537.misc b/changelog.d/5537.misc new file mode 100644 index 0000000000..870a5ff18b --- /dev/null +++ b/changelog.d/5537.misc @@ -0,0 +1 @@ +Add information about how to install and run `black` on the codebase to code_style.rst. diff --git a/docs/code_style.rst b/docs/code_style.rst index 62800b5b3e..e3ca626bfd 100644 --- a/docs/code_style.rst +++ b/docs/code_style.rst @@ -1,70 +1,67 @@ -- Everything should comply with PEP8. Code should pass - ``pep8 --max-line-length=100`` without any warnings. +# Code Style -- **Indenting**: +The Synapse codebase uses a number of code formatting tools in order to +quickly and automatically check for formatting (and sometimes logical) errors +in code. - - NEVER tabs. 4 spaces to indent. +The necessary tools are detailed below. - - follow PEP8; either hanging indent or multiline-visual indent depending - on the size and shape of the arguments and what makes more sense to the - author. In other words, both this:: +## Formatting tools - print("I am a fish %s" % "moo") +The Synapse codebase uses [black](https://pypi.org/project/black/) as an +opinionated code formatter, ensuring all comitted code is properly +formatted. - and this:: +First install ``black`` with:: - print("I am a fish %s" % - "moo") + pip install --upgrade black - and this:: +Have ``black`` auto-format your code (it shouldn't change any +functionality) with:: - print( - "I am a fish %s" % - "moo", - ) + black . --exclude="\.tox|build|env" - ...are valid, although given each one takes up 2x more vertical space than - the previous, it's up to the author's discretion as to which layout makes - most sense for their function invocation. (e.g. if they want to add - comments per-argument, or put expressions in the arguments, or group - related arguments together, or want to deliberately extend or preserve - vertical/horizontal space) +- **flake8** -- **Line length**: + ``flake8`` is a code checking tool. We require code to pass ``flake8`` before being merged into the codebase. - Max line length is 79 chars (with flexibility to overflow by a "few chars" if - the overflowing content is not semantically significant and avoids an - explosion of vertical whitespace). + Install ``flake8`` with:: - Use parentheses instead of ``\`` for line continuation where ever possible - (which is pretty much everywhere). + pip install --upgrade flake8 -- **Naming**: + Check all application and test code with:: - - Use camel case for class and type names - - Use underscores for functions and variables. + flake8 synapse tests -- Use double quotes ``"foo"`` rather than single quotes ``'foo'``. +- **isort** + + ``isort`` ensures imports are nicely formatted, and can suggest and + auto-fix issues such as double-importing. -- **Blank lines**: + Install ``isort`` with:: - - There should be max a single new line between: + pip install --upgrade isort - - statements - - functions in a class + Auto-fix imports with:: - - There should be two new lines between: + isort -rc synapse tests - - definitions in a module (e.g., between different classes) + ``-rc`` means to recursively search the given directories. -- **Whitespace**: +It's worth noting that modern IDEs and text editors can run these tools +automatically on save. It may be worth looking into whether this +functionality is supported in your editor for a more convenient development +workflow. It is not, however, recommended to run ``flake8`` on save as it +takes a while and is very resource intensive. - There should be spaces where spaces should be and not where there shouldn't - be: +## General rules - - a single space after a comma - - a single space before and after for '=' when used as assignment - - no spaces before and after for '=' for default values and keyword arguments. +- **Naming**: + + - Use camel case for class and type names + - Use underscores for functions and variables. + +- Use double quotes ``"foo"`` rather than single quotes ``'foo'``. - **Comments**: should follow the `google code style `_. @@ -76,7 +73,7 @@ - **Imports**: - - Prefer to import classes and functions than packages or modules. + - Prefer to import classes and functions rather than packages or modules. Example:: -- cgit 1.5.1 From c8cb1862604d24ea1b161a50b03e65a6251d1a7f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 25 Jun 2019 12:27:56 +0100 Subject: Fix broken link in MSC1711 FAQ --- docs/MSC1711_certificates_FAQ.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'docs') diff --git a/docs/MSC1711_certificates_FAQ.md b/docs/MSC1711_certificates_FAQ.md index 7f9a23ff31..83497380df 100644 --- a/docs/MSC1711_certificates_FAQ.md +++ b/docs/MSC1711_certificates_FAQ.md @@ -14,7 +14,7 @@ upgraded, however it may be of use to those with old installs returning to the project. If you are setting up a server from scratch you almost certainly should look at -the [installation guide](INSTALL.md) instead. +the [installation guide](../INSTALL.md) instead. ## Introduction The goal of Synapse 0.99.0 is to act as a stepping stone to Synapse 1.0.0. It -- cgit 1.5.1 From c548dbc4b16a8bff6226fa4a6d7c86180c4f5704 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Thu, 27 Jun 2019 18:20:17 +0100 Subject: Make it clearer that the template dir is relative to synapse's root dir (#5543) Helps address #5444 --- changelog.d/5543.misc | 1 + docs/sample_config.yaml | 10 +++++++++- synapse/config/emailconfig.py | 10 +++++++++- 3 files changed, 19 insertions(+), 2 deletions(-) create mode 100644 changelog.d/5543.misc (limited to 'docs') diff --git a/changelog.d/5543.misc b/changelog.d/5543.misc new file mode 100644 index 0000000000..793620a731 --- /dev/null +++ b/changelog.d/5543.misc @@ -0,0 +1 @@ +Make the config clearer in that email.template_dir is relative to the Synapse's root directory, not the `synapse/` folder within it. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index da10788e96..18be376e1e 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1070,11 +1070,13 @@ password_config: # app_name: Matrix # # # Enable email notifications by default +# # # notif_for_new_users: True # # # Defining a custom URL for Riot is only needed if email notifications # # should contain links to a self-hosted installation of Riot; when set # # the "app_name" setting is ignored +# # # riot_base_url: "http://localhost/riot" # # # Enable sending password reset emails via the configured, trusted @@ -1087,16 +1089,22 @@ password_config: # # # # If this option is set to false and SMTP options have not been # # configured, resetting user passwords via email will be disabled +# # # #trust_identity_server_for_password_resets: false # # # Configure the time that a validation email or text message code # # will expire after sending # # # # This is currently used for password resets +# # # #validation_token_lifetime: 1h # # # Template directory. All template files should be stored within this -# # directory +# # directory. If not set, default templates from within the Synapse +# # package will be used +# # +# # For the list of default templates, please see +# # https://github.com/matrix-org/synapse/tree/master/synapse/res/templates # # # #template_dir: res/templates # diff --git a/synapse/config/emailconfig.py b/synapse/config/emailconfig.py index cf39936da7..fcd55d3e3d 100644 --- a/synapse/config/emailconfig.py +++ b/synapse/config/emailconfig.py @@ -233,11 +233,13 @@ class EmailConfig(Config): # app_name: Matrix # # # Enable email notifications by default + # # # notif_for_new_users: True # # # Defining a custom URL for Riot is only needed if email notifications # # should contain links to a self-hosted installation of Riot; when set # # the "app_name" setting is ignored + # # # riot_base_url: "http://localhost/riot" # # # Enable sending password reset emails via the configured, trusted @@ -250,16 +252,22 @@ class EmailConfig(Config): # # # # If this option is set to false and SMTP options have not been # # configured, resetting user passwords via email will be disabled + # # # #trust_identity_server_for_password_resets: false # # # Configure the time that a validation email or text message code # # will expire after sending # # # # This is currently used for password resets + # # # #validation_token_lifetime: 1h # # # Template directory. All template files should be stored within this - # # directory + # # directory. If not set, default templates from within the Synapse + # # package will be used + # # + # # For the list of default templates, please see + # # https://github.com/matrix-org/synapse/tree/master/synapse/res/templates # # # #template_dir: res/templates # -- cgit 1.5.1 From 9646a593ac555e7b68c6133c29a9f5bac83d1c2f Mon Sep 17 00:00:00 2001 From: Daniel Hoffend Date: Thu, 27 Jun 2019 19:37:29 +0200 Subject: Added possibilty to disable local password authentication (#5092) Signed-off-by: Daniel Hoffend --- changelog.d/5092.feature | 1 + docs/sample_config.yaml | 6 ++++++ synapse/config/password.py | 7 +++++++ synapse/handlers/auth.py | 2 +- synapse/handlers/set_password.py | 3 +++ 5 files changed, 18 insertions(+), 1 deletion(-) create mode 100644 changelog.d/5092.feature (limited to 'docs') diff --git a/changelog.d/5092.feature b/changelog.d/5092.feature new file mode 100644 index 0000000000..c22f586d08 --- /dev/null +++ b/changelog.d/5092.feature @@ -0,0 +1 @@ +Added possibilty to disable local password authentication. Contributed by Daniel Hoffend. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 18be376e1e..a01e1152f7 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1046,6 +1046,12 @@ password_config: # #enabled: false + # Uncomment to disable authentication against the local password + # database. This is ignored if `enabled` is false, and is only useful + # if you have other password_providers. + # + #localdb_enabled: false + # Uncomment and change to a secret random string for extra security. # DO NOT CHANGE THIS AFTER INITIAL SETUP! # diff --git a/synapse/config/password.py b/synapse/config/password.py index 598f84fc0c..d5b5953f2f 100644 --- a/synapse/config/password.py +++ b/synapse/config/password.py @@ -26,6 +26,7 @@ class PasswordConfig(Config): password_config = {} self.password_enabled = password_config.get("enabled", True) + self.password_localdb_enabled = password_config.get("localdb_enabled", True) self.password_pepper = password_config.get("pepper", "") def generate_config_section(self, config_dir_path, server_name, **kwargs): @@ -35,6 +36,12 @@ class PasswordConfig(Config): # #enabled: false + # Uncomment to disable authentication against the local password + # database. This is ignored if `enabled` is false, and is only useful + # if you have other password_providers. + # + #localdb_enabled: false + # Uncomment and change to a secret random string for extra security. # DO NOT CHANGE THIS AFTER INITIAL SETUP! # diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 97b21c4093..c8c1ed3246 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -743,7 +743,7 @@ class AuthHandler(BaseHandler): result = (result, None) defer.returnValue(result) - if login_type == LoginType.PASSWORD: + if login_type == LoginType.PASSWORD and self.hs.config.password_localdb_enabled: known_login_type = True canonical_user_id = yield self._check_local_password( diff --git a/synapse/handlers/set_password.py b/synapse/handlers/set_password.py index 5a0995d4fe..d90c9e0108 100644 --- a/synapse/handlers/set_password.py +++ b/synapse/handlers/set_password.py @@ -33,6 +33,9 @@ class SetPasswordHandler(BaseHandler): @defer.inlineCallbacks def set_password(self, user_id, newpassword, requester=None): + if not self.hs.config.password_localdb_enabled: + raise SynapseError(403, "Password change disabled", errcode=Codes.FORBIDDEN) + password_hash = yield self._auth_handler.hash(newpassword) except_device_id = requester.device_id if requester else None -- cgit 1.5.1 From be3b901ccdf28d0f81d312d7cd8b7bedb22b4049 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Fri, 28 Jun 2019 18:19:09 +1000 Subject: Update the TLS cipher string and provide configurability for TLS on outgoing federation (#5550) --- changelog.d/5550.feature | 1 + changelog.d/5550.misc | 1 + docs/sample_config.yaml | 9 +++ scripts/generate_config | 2 +- synapse/config/tls.py | 32 ++++++++++- synapse/crypto/context_factory.py | 39 +++++++++++-- tests/config/test_tls.py | 115 +++++++++++++++++++++++++++++++++++++- 7 files changed, 190 insertions(+), 9 deletions(-) create mode 100644 changelog.d/5550.feature create mode 100644 changelog.d/5550.misc (limited to 'docs') diff --git a/changelog.d/5550.feature b/changelog.d/5550.feature new file mode 100644 index 0000000000..79ecedf3b8 --- /dev/null +++ b/changelog.d/5550.feature @@ -0,0 +1 @@ +The minimum TLS version used for outgoing federation requests can now be set with `federation_client_minimum_tls_version`. diff --git a/changelog.d/5550.misc b/changelog.d/5550.misc new file mode 100644 index 0000000000..ad5693338e --- /dev/null +++ b/changelog.d/5550.misc @@ -0,0 +1 @@ +Synapse will now only allow TLS v1.2 connections when serving federation, if it terminates TLS. As Synapse's allowed ciphers were only able to be used in TLSv1.2 before, this does not change behaviour. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index a01e1152f7..bf9cd88b15 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -317,6 +317,15 @@ listeners: # #federation_verify_certificates: false +# The minimum TLS version that will be used for outbound federation requests. +# +# Defaults to `1`. Configurable to `1`, `1.1`, `1.2`, or `1.3`. Note +# that setting this value higher than `1.2` will prevent federation to most +# of the public Matrix network: only configure it to `1.3` if you have an +# entirely private federation setup and you can ensure TLS 1.3 support. +# +#federation_client_minimum_tls_version: 1.2 + # Skip federation certificate verification on the following whitelist # of domains. # diff --git a/scripts/generate_config b/scripts/generate_config index 93b6406992..771cbf8d95 100755 --- a/scripts/generate_config +++ b/scripts/generate_config @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 import argparse import shutil diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 8fcf801418..ca508a224f 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -23,7 +23,7 @@ import six from unpaddedbase64 import encode_base64 -from OpenSSL import crypto +from OpenSSL import SSL, crypto from twisted.internet._sslverify import Certificate, trustRootFromCertificates from synapse.config._base import Config, ConfigError @@ -81,6 +81,27 @@ class TlsConfig(Config): "federation_verify_certificates", True ) + # Minimum TLS version to use for outbound federation traffic + self.federation_client_minimum_tls_version = str( + config.get("federation_client_minimum_tls_version", 1) + ) + + if self.federation_client_minimum_tls_version not in ["1", "1.1", "1.2", "1.3"]: + raise ConfigError( + "federation_client_minimum_tls_version must be one of: 1, 1.1, 1.2, 1.3" + ) + + # Prevent people shooting themselves in the foot here by setting it to + # the biggest number blindly + if self.federation_client_minimum_tls_version == "1.3": + if getattr(SSL, "OP_NO_TLSv1_3", None) is None: + raise ConfigError( + ( + "federation_client_minimum_tls_version cannot be 1.3, " + "your OpenSSL does not support it" + ) + ) + # Whitelist of domains to not verify certificates for fed_whitelist_entries = config.get( "federation_certificate_verification_whitelist", [] @@ -261,6 +282,15 @@ class TlsConfig(Config): # #federation_verify_certificates: false + # The minimum TLS version that will be used for outbound federation requests. + # + # Defaults to `1`. Configurable to `1`, `1.1`, `1.2`, or `1.3`. Note + # that setting this value higher than `1.2` will prevent federation to most + # of the public Matrix network: only configure it to `1.3` if you have an + # entirely private federation setup and you can ensure TLS 1.3 support. + # + #federation_client_minimum_tls_version: 1.2 + # Skip federation certificate verification on the following whitelist # of domains. # diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 2bc5cc3807..4f48e8e88d 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -24,12 +24,25 @@ from OpenSSL import SSL, crypto from twisted.internet._sslverify import _defaultCurveName from twisted.internet.abstract import isIPAddress, isIPv6Address from twisted.internet.interfaces import IOpenSSLClientConnectionCreator -from twisted.internet.ssl import CertificateOptions, ContextFactory, platformTrust +from twisted.internet.ssl import ( + CertificateOptions, + ContextFactory, + TLSVersion, + platformTrust, +) from twisted.python.failure import Failure logger = logging.getLogger(__name__) +_TLS_VERSION_MAP = { + "1": TLSVersion.TLSv1_0, + "1.1": TLSVersion.TLSv1_1, + "1.2": TLSVersion.TLSv1_2, + "1.3": TLSVersion.TLSv1_3, +} + + class ServerContextFactory(ContextFactory): """Factory for PyOpenSSL SSL contexts that are used to handle incoming connections.""" @@ -43,16 +56,18 @@ class ServerContextFactory(ContextFactory): try: _ecCurve = crypto.get_elliptic_curve(_defaultCurveName) context.set_tmp_ecdh(_ecCurve) - except Exception: logger.exception("Failed to enable elliptic curve for TLS") - context.set_options(SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3) + + context.set_options( + SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3 | SSL.OP_NO_TLSv1 | SSL.OP_NO_TLSv1_1 + ) context.use_certificate_chain_file(config.tls_certificate_file) context.use_privatekey(config.tls_private_key) # https://hynek.me/articles/hardening-your-web-servers-ssl-ciphers/ context.set_cipher_list( - "ECDH+AESGCM:ECDH+CHACHA20:ECDH+AES256:ECDH+AES128:!aNULL:!SHA1" + "ECDH+AESGCM:ECDH+CHACHA20:ECDH+AES256:ECDH+AES128:!aNULL:!SHA1:!AESCCM" ) def getContext(self): @@ -79,10 +94,22 @@ class ClientTLSOptionsFactory(object): # Use CA root certs provided by OpenSSL trust_root = platformTrust() - self._verify_ssl_context = CertificateOptions(trustRoot=trust_root).getContext() + # "insecurelyLowerMinimumTo" is the argument that will go lower than + # Twisted's default, which is why it is marked as "insecure" (since + # Twisted's defaults are reasonably secure). But, since Twisted is + # moving to TLS 1.2 by default, we want to respect the config option if + # it is set to 1.0 (which the alternate option, raiseMinimumTo, will not + # let us do). + minTLS = _TLS_VERSION_MAP[config.federation_client_minimum_tls_version] + + self._verify_ssl = CertificateOptions( + trustRoot=trust_root, insecurelyLowerMinimumTo=minTLS + ) + self._verify_ssl_context = self._verify_ssl.getContext() self._verify_ssl_context.set_info_callback(self._context_info_cb) - self._no_verify_ssl_context = CertificateOptions().getContext() + self._no_verify_ssl = CertificateOptions(insecurelyLowerMinimumTo=minTLS) + self._no_verify_ssl_context = self._no_verify_ssl.getContext() self._no_verify_ssl_context.set_info_callback(self._context_info_cb) def get_options(self, host): diff --git a/tests/config/test_tls.py b/tests/config/test_tls.py index a5d88d644a..4f8a87a3df 100644 --- a/tests/config/test_tls.py +++ b/tests/config/test_tls.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2019 New Vector Ltd +# Copyright 2019 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. @@ -15,7 +16,10 @@ import os -from synapse.config.tls import TlsConfig +from OpenSSL import SSL + +from synapse.config.tls import ConfigError, TlsConfig +from synapse.crypto.context_factory import ClientTLSOptionsFactory from tests.unittest import TestCase @@ -78,3 +82,112 @@ s4niecZKPBizL6aucT59CsunNmmb5Glq8rlAcU+1ZTZZzGYqVYhF6axB9Qg= "or use Synapse's ACME support to provision one." ), ) + + def test_tls_client_minimum_default(self): + """ + The default client TLS version is 1.0. + """ + config = {} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + + self.assertEqual(t.federation_client_minimum_tls_version, "1") + + def test_tls_client_minimum_set(self): + """ + The default client TLS version can be set to 1.0, 1.1, and 1.2. + """ + config = {"federation_client_minimum_tls_version": 1} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_client_minimum_tls_version, "1") + + config = {"federation_client_minimum_tls_version": 1.1} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_client_minimum_tls_version, "1.1") + + config = {"federation_client_minimum_tls_version": 1.2} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_client_minimum_tls_version, "1.2") + + # Also test a string version + config = {"federation_client_minimum_tls_version": "1"} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_client_minimum_tls_version, "1") + + config = {"federation_client_minimum_tls_version": "1.2"} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_client_minimum_tls_version, "1.2") + + def test_tls_client_minimum_1_point_3_missing(self): + """ + If TLS 1.3 support is missing and it's configured, it will raise a + ConfigError. + """ + # thanks i hate it + if hasattr(SSL, "OP_NO_TLSv1_3"): + OP_NO_TLSv1_3 = SSL.OP_NO_TLSv1_3 + delattr(SSL, "OP_NO_TLSv1_3") + self.addCleanup(setattr, SSL, "SSL.OP_NO_TLSv1_3", OP_NO_TLSv1_3) + assert not hasattr(SSL, "OP_NO_TLSv1_3") + + config = {"federation_client_minimum_tls_version": 1.3} + t = TestConfig() + with self.assertRaises(ConfigError) as e: + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual( + e.exception.args[0], + ( + "federation_client_minimum_tls_version cannot be 1.3, " + "your OpenSSL does not support it" + ), + ) + + def test_tls_client_minimum_1_point_3_exists(self): + """ + If TLS 1.3 support exists and it's configured, it will be settable. + """ + # thanks i hate it, still + if not hasattr(SSL, "OP_NO_TLSv1_3"): + SSL.OP_NO_TLSv1_3 = 0x00 + self.addCleanup(lambda: delattr(SSL, "OP_NO_TLSv1_3")) + assert hasattr(SSL, "OP_NO_TLSv1_3") + + config = {"federation_client_minimum_tls_version": 1.3} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_client_minimum_tls_version, "1.3") + + def test_tls_client_minimum_set_passed_through_1_2(self): + """ + The configured TLS version is correctly configured by the ContextFactory. + """ + config = {"federation_client_minimum_tls_version": 1.2} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + + cf = ClientTLSOptionsFactory(t) + + # The context has had NO_TLSv1_1 and NO_TLSv1_0 set, but not NO_TLSv1_2 + self.assertNotEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1, 0) + self.assertNotEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1_1, 0) + self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1_2, 0) + + def test_tls_client_minimum_set_passed_through_1_0(self): + """ + The configured TLS version is correctly configured by the ContextFactory. + """ + config = {"federation_client_minimum_tls_version": 1} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + + cf = ClientTLSOptionsFactory(t) + + # The context has not had any of the NO_TLS set. + self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1, 0) + self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1_1, 0) + self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1_2, 0) -- cgit 1.5.1