Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
Merge branch 'dinsic' of github.com:matrix-org/synapse-dinsic into di…
Browse files Browse the repository at this point in the history
…nsic-release-v1.1.0

* 'dinsic' of github.com:matrix-org/synapse-dinsic:
  Fixes an attribute error when using the default display name during registration. (#32)
  Share SSL contexts for non-federation requests (#30)
  Share SSL options for well-known requests (#29)
  Fix buggy condition in account validity handler (#28)
  • Loading branch information
anoadragon453 committed Mar 24, 2020
2 parents ca1d073 + e24928d commit 81f9f3f
Show file tree
Hide file tree
Showing 14 changed files with 103 additions and 49 deletions.
1 change: 1 addition & 0 deletions changelog.d/28.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug causing account validity renewal emails to be sent even if the feature is turned off in some cases.
1 change: 1 addition & 0 deletions changelog.d/29.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve performance when making `.well-known` requests by sharing the SSL options between requests.
1 change: 1 addition & 0 deletions changelog.d/30.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve performance when making HTTP requests to sygnal, sydent, etc, by sharing the SSL context object between connections.
1 change: 1 addition & 0 deletions changelog.d/32.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes a bug when using the default display name during registration.
2 changes: 2 additions & 0 deletions synapse/app/client_reader.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@
from synapse.replication.slave.storage.registration import SlavedRegistrationStore
from synapse.replication.slave.storage.room import RoomStore
from synapse.replication.slave.storage.transactions import SlavedTransactionStore
from synapse.replication.slave.storage.user_directory import SlavedUserDirectoryStore
from synapse.replication.tcp.client import ReplicationClientHandler
from synapse.rest.client.v1.login import LoginRestServlet
from synapse.rest.client.v1.push_rule import PushRuleRestServlet
Expand Down Expand Up @@ -87,6 +88,7 @@ class ClientReaderSlavedStore(
SlavedTransactionStore,
SlavedProfileStore,
SlavedClientIpStore,
SlavedUserDirectoryStore,
BaseSlavedStore,
):
pass
Expand Down
66 changes: 47 additions & 19 deletions synapse/crypto/context_factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
platformTrust,
)
from twisted.python.failure import Failure
from twisted.web.iweb import IPolicyForHTTPS

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -74,7 +75,8 @@ def getContext(self):
return self._context


class ClientTLSOptionsFactory(object):
@implementer(IPolicyForHTTPS)
class FederationPolicyForHTTPS(object):
"""Factory for Twisted SSLClientConnectionCreators that are used to make connections
to remote servers for federation.
Expand Down Expand Up @@ -106,11 +108,11 @@ def __init__(self, config):
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._verify_ssl_context.set_info_callback(_context_info_cb)

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)
self._no_verify_ssl_context.set_info_callback(_context_info_cb)

def get_options(self, host):
# Check if certificate verification has been enabled
Expand All @@ -129,22 +131,48 @@ def get_options(self, host):

return SSLClientConnectionCreator(host, ssl_context, should_verify)

@staticmethod
def _context_info_cb(ssl_connection, where, ret):
"""The 'information callback' for our openssl context object."""
# we assume that the app_data on the connection object has been set to
# a TLSMemoryBIOProtocol object. (This is done by SSLClientConnectionCreator)
tls_protocol = ssl_connection.get_app_data()
try:
# ... we further assume that SSLClientConnectionCreator has set the
# '_synapse_tls_verifier' attribute to a ConnectionVerifier object.
tls_protocol._synapse_tls_verifier.verify_context_info_cb(
ssl_connection, where
)
except: # noqa: E722, taken from the twisted implementation
logger.exception("Error during info_callback")
f = Failure()
tls_protocol.failVerification(f)
def creatorForNetloc(self, hostname, port):
"""Implements the IPolicyForHTTPS interace so that this can be passed
directly to agents.
"""
return self.get_options(hostname)


@implementer(IPolicyForHTTPS)
class RegularPolicyForHTTPS(object):
"""Factory for Twisted SSLClientConnectionCreators that are used to make connections
to remote servers, for other than federation.
Always uses the same OpenSSL context object, which uses the default OpenSSL CA
trust root.
"""

def __init__(self):
trust_root = platformTrust()
self._ssl_context = CertificateOptions(trustRoot=trust_root).getContext()
self._ssl_context.set_info_callback(_context_info_cb)

def creatorForNetloc(self, hostname, port):
return SSLClientConnectionCreator(hostname, self._ssl_context, True)


def _context_info_cb(ssl_connection, where, ret):
"""The 'information callback' for our openssl context objects.
Note: Once this is set as the info callback on a Context object, the Context should
only be used with the SSLClientConnectionCreator.
"""
# we assume that the app_data on the connection object has been set to
# a TLSMemoryBIOProtocol object. (This is done by SSLClientConnectionCreator)
tls_protocol = ssl_connection.get_app_data()
try:
# ... we further assume that SSLClientConnectionCreator has set the
# '_synapse_tls_verifier' attribute to a ConnectionVerifier object.
tls_protocol._synapse_tls_verifier.verify_context_info_cb(ssl_connection, where)
except: # noqa: E722, taken from the twisted implementation
logger.exception("Error during info_callback")
f = Failure()
tls_protocol.failVerification(f)


@implementer(IOpenSSLClientConnectionCreator)
Expand Down
6 changes: 5 additions & 1 deletion synapse/handlers/account_validity.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,11 @@ def __init__(self, hs):
self._show_users_in_user_directory = self.hs.config.show_users_in_user_directory
self.profile_handler = self.hs.get_profile_handler()

if self._account_validity.renew_by_email_enabled and load_jinja2_templates:
if (
self._account_validity.enabled
and self._account_validity.renew_by_email_enabled
and load_jinja2_templates
):
# Don't do email-specific configuration if renewal by email is disabled.
try:
app_name = self.hs.config.email_app_name
Expand Down
3 changes: 0 additions & 3 deletions synapse/http/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,9 +243,6 @@ def __getattr__(_self, attr):
pool.maxPersistentPerHost = max((100 * CACHE_SIZE_FACTOR, 5))
pool.cachedConnectionTimeout = 2 * 60

# The default context factory in Twisted 14.0.0 (which we require) is
# BrowserLikePolicyForHTTPS which will do regular cert validation
# 'like a browser'
self.agent = ProxyAgent(
self.reactor,
connectTimeout=15,
Expand Down
17 changes: 6 additions & 11 deletions synapse/http/federation/matrix_federation_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,9 @@ class MatrixFederationAgent(object):
Args:
reactor (IReactor): twisted reactor to use for underlying requests
tls_client_options_factory (ClientTLSOptionsFactory|None):
tls_client_options_factory (FederationPolicyForHTTPS|None):
factory to use for fetching client tls options, or none to disable TLS.
_well_known_tls_policy (IPolicyForHTTPS|None):
TLS policy to use for fetching .well-known files. None to use a default
(browser-like) implementation.
_srv_resolver (SrvResolver|None):
SRVResolver impl to use for looking up SRV records. None to use a default
implementation.
Expand Down Expand Up @@ -98,13 +94,12 @@ def __init__(
self._pool.maxPersistentPerHost = 5
self._pool.cachedConnectionTimeout = 2 * 60

agent_args = {}
if _well_known_tls_policy is not None:
# the param is called 'contextFactory', but actually passing a
# contextfactory is deprecated, and it expects an IPolicyForHTTPS.
agent_args["contextFactory"] = _well_known_tls_policy
_well_known_agent = RedirectAgent(
Agent(self._reactor, pool=self._pool, **agent_args)
Agent(
self._reactor,
pool=self._pool,
contextFactory=tls_client_options_factory,
)
)
self._well_known_agent = _well_known_agent

Expand Down
4 changes: 3 additions & 1 deletion synapse/python_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@
"frozendict>=1",
"unpaddedbase64>=1.1.0",
"canonicaljson>=1.1.3",
"signedjson>=1.0.0",
# Pin signedjson to 1.0.0 because this version of Synapse relies on a function that's
# been removed in 1.1.0. Hopefully, this will be fixed by the upcoming mainline merge.
"signedjson==1.0.0",
"pynacl>=1.2.1",
"idna>=2.5",
# validating SSL certs for IP addresses requires service_identity 18.1.
Expand Down
22 changes: 22 additions & 0 deletions synapse/replication/slave/storage/user_directory.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# -*- 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 synapse.storage.user_directory import UserDirectoryStore

from ._base import BaseSlavedStore


class SlavedUserDirectoryStore(UserDirectoryStore, BaseSlavedStore):
pass
6 changes: 3 additions & 3 deletions synapse/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,14 @@

from twisted.enterprise import adbapi
from twisted.mail.smtp import sendmail
from twisted.web.client import BrowserLikePolicyForHTTPS

from synapse.api.auth import Auth
from synapse.api.filtering import Filtering
from synapse.api.ratelimiting import Ratelimiter
from synapse.appservice.api import ApplicationServiceApi
from synapse.appservice.scheduler import ApplicationServiceScheduler
from synapse.crypto import context_factory
from synapse.crypto.context_factory import RegularPolicyForHTTPS
from synapse.crypto.keyring import Keyring
from synapse.events.builder import EventBuilderFactory
from synapse.events.spamcheck import SpamChecker
Expand Down Expand Up @@ -302,7 +302,7 @@ def build_http_client_context_factory(self):
return (
InsecureInterceptableContextFactory()
if self.config.use_insecure_ssl_client_just_for_testing_do_not_use
else BrowserLikePolicyForHTTPS()
else RegularPolicyForHTTPS()
)

def build_simple_http_client(self):
Expand Down Expand Up @@ -412,7 +412,7 @@ def build_pusherpool(self):
return PusherPool(self)

def build_http_client(self):
tls_client_options_factory = context_factory.ClientTLSOptionsFactory(
tls_client_options_factory = context_factory.FederationPolicyForHTTPS(
self.config
)
return MatrixFederationHttpClient(self, tls_client_options_factory)
Expand Down
6 changes: 3 additions & 3 deletions tests/config/test_tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from OpenSSL import SSL

from synapse.config.tls import ConfigError, TlsConfig
from synapse.crypto.context_factory import ClientTLSOptionsFactory
from synapse.crypto.context_factory import FederationPolicyForHTTPS

from tests.unittest import TestCase

Expand Down Expand Up @@ -170,7 +170,7 @@ def test_tls_client_minimum_set_passed_through_1_2(self):
t = TestConfig()
t.read_config(config, config_dir_path="", data_dir_path="")

cf = ClientTLSOptionsFactory(t)
cf = FederationPolicyForHTTPS(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)
Expand All @@ -185,7 +185,7 @@ def test_tls_client_minimum_set_passed_through_1_0(self):
t = TestConfig()
t.read_config(config, config_dir_path="", data_dir_path="")

cf = ClientTLSOptionsFactory(t)
cf = FederationPolicyForHTTPS(t)

# The context has not had any of the NO_TLS set.
self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1, 0)
Expand Down
16 changes: 8 additions & 8 deletions tests/http/federation/test_matrix_federation_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
from twisted.web.iweb import IPolicyForHTTPS

from synapse.config.homeserver import HomeServerConfig
from synapse.crypto.context_factory import ClientTLSOptionsFactory
from synapse.crypto.context_factory import FederationPolicyForHTTPS
from synapse.http.federation.matrix_federation_agent import (
MatrixFederationAgent,
_cache_period_from_headers,
Expand Down Expand Up @@ -75,15 +75,13 @@ def setUp(self):

config_dict = default_config("test", parse=False)
config_dict["federation_custom_ca_list"] = [get_test_ca_cert_file()]
# config_dict["trusted_key_servers"] = []

self._config = config = HomeServerConfig()
config.parse_config_dict(config_dict, "", "")

self.agent = MatrixFederationAgent(
reactor=self.reactor,
tls_client_options_factory=ClientTLSOptionsFactory(config),
_well_known_tls_policy=TrustingTLSPolicyForHTTPS(),
tls_client_options_factory=FederationPolicyForHTTPS(config),
_srv_resolver=self.mock_resolver,
_well_known_cache=self.well_known_cache,
)
Expand Down Expand Up @@ -696,16 +694,18 @@ def test_get_well_known_unsigned_cert(self):
not signed by a CA
"""

# we use the same test server as the other tests, but use an agent
# with _well_known_tls_policy left to the default, which will not
# trust it (since the presented cert is signed by a test CA)
# we use the same test server as the other tests, but use an agent with
# the config left to the default, which will not trust it (since the
# presented cert is signed by a test CA)

self.mock_resolver.resolve_service.side_effect = lambda _: []
self.reactor.lookups["testserv"] = "1.2.3.4"

config = default_config("test", parse=True)

agent = MatrixFederationAgent(
reactor=self.reactor,
tls_client_options_factory=ClientTLSOptionsFactory(self._config),
tls_client_options_factory=FederationPolicyForHTTPS(config),
_srv_resolver=self.mock_resolver,
_well_known_cache=self.well_known_cache,
)
Expand Down

0 comments on commit 81f9f3f

Please sign in to comment.