From dbb3319e5c55c4b9eb2ab53f8460034891c62403 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 28 Mar 2019 18:06:31 +0000 Subject: [PATCH 01/56] Config option for verifying federation certificates --- synapse/config/server.py | 28 +++++++++++++++++++ synapse/crypto/context_factory.py | 3 +- .../federation/matrix_federation_agent.py | 1 + 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 08e4e45482e1..affba6d92012 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -110,6 +110,22 @@ def read_config(self, config): # due to resource constraints self.admin_contact = config.get("admin_contact", None) + self.federation_verify_certificates = config.get( + "federation_verify_certificates", False, + ) + + # Whitelist of domains to not verify certificates for + self.federation_certificate_verification_whitelist = None + federation_certificate_verification_whitelist = config.get( + "federation_certificate_verification_whitelist", None + ) + + # Store whitelisted domains in a hash for fast lookup + if federation_certificate_verification_whitelist is not None: + self.federation_certificate_verification_whitelist = {} + for domain in federation_certificate_verification_whitelist: + self.federation_certificate_verification_whitelist[domain] = True + # FIXME: federation_domain_whitelist needs sytests self.federation_domain_whitelist = None federation_domain_whitelist = config.get( @@ -339,6 +355,18 @@ def default_config(self, server_name, data_dir_path, **kwargs): # #enable_search: false + # Whether to verify TLS certificates when sending federation traffic. + # + #federation_verify_certificates: true + + # Prevent federation certificate validation on the following whitelist + # of domains. Only effective if federation_verify_certicates is true. + # + #federation_certificate_validation_whitelist: + # - lon.example.com + # - nyc.example.com + # - syd.example.com + # Restrict federation to the following whitelist of domains. # N.B. we recommend also firewalling your federation listener to limit # inbound federation traffic as early as possible, rather than relying diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 49cbc7098f54..96eeb862d17d 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -127,8 +127,7 @@ class ClientTLSOptionsFactory(object): to remote servers for federation.""" def __init__(self, config): - # We don't use config options yet - self._options = CertificateOptions(verify=False) + self._options = CertificateOptions(verify=config.federation_verify_certificates) def get_options(self, host): # Use _makeContext so that we get a fresh OpenSSL CTX each time. diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py index 1334c630ccb5..b254faa4e13c 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/synapse/http/federation/matrix_federation_agent.py @@ -148,6 +148,7 @@ def request(self, method, uri, headers=None, bodyProducer=None): if self._tls_client_options_factory is None: tls_options = None else: + # TODO: Check the server we're sending to here and change verify value if necessary tls_options = self._tls_client_options_factory.get_options( res.tls_server_name.decode("ascii") ) From 5fd4cd0ddd69f1ed9b6683e1e51edf71fa5208e8 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Fri, 29 Mar 2019 14:09:07 +0000 Subject: [PATCH 02/56] Whitelist per domain --- synapse/crypto/context_factory.py | 18 +++++++++++++++--- .../http/federation/matrix_federation_agent.py | 3 +-- 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 96eeb862d17d..b99159dbbdcf 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -127,8 +127,20 @@ class ClientTLSOptionsFactory(object): to remote servers for federation.""" def __init__(self, config): - self._options = CertificateOptions(verify=config.federation_verify_certificates) + # We don't use config options yet + self._options_validate = CertificateOptions(verify=True) + self._options_novalidate = CertificateOptions(verify=False) - def get_options(self, host): + def get_options(self, host, config): # Use _makeContext so that we get a fresh OpenSSL CTX each time. - return ClientTLSOptions(host, self._options._makeContext()) + + # Check if certificate validation has been enabled + if config.federation_verify_certificates: + # Check if this host is whitelisted + if host in config.federation_certificate_validation_whitelist: + return ClientTLSOptions(host, self._options_novalidate._makeContext()) + + # Otherwise require validation + return ClientTLSOptions(host, self._options_validate._makeContext()) + + return ClientTLSOptions(host, self._options_novalidate._makeContext()) diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py index b254faa4e13c..89856492272c 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/synapse/http/federation/matrix_federation_agent.py @@ -148,9 +148,8 @@ def request(self, method, uri, headers=None, bodyProducer=None): if self._tls_client_options_factory is None: tls_options = None else: - # TODO: Check the server we're sending to here and change verify value if necessary tls_options = self._tls_client_options_factory.get_options( - res.tls_server_name.decode("ascii") + res.tls_server_name.decode("ascii"), self.hs.config, ) # make sure that the Host header is set correctly From 91460594195350a23b0c6ea5cffdcbfc55f0b00d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 14:09:33 +0100 Subject: [PATCH 03/56] Ability to specify list of custom CA certificates --- synapse/config/tls.py | 43 +++++++++++++++++++++++++++++++ synapse/crypto/context_factory.py | 5 +++- 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index f0014902da72..5e4ed8289dd5 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -19,6 +19,8 @@ from datetime import datetime from hashlib import sha256 +from twisted.internet._sslverify import trustRootFromCertificates, Certificate + import six from unpaddedbase64 import encode_base64 @@ -70,6 +72,37 @@ def read_config(self, config): self.tls_fingerprints = list(self._original_tls_fingerprints) + # List of custom certificate authorities for TLS verification + self.federation_custom_ca_list = config.get( + "federation_custom_ca_list", [], + ) + + # Read in the CA certificates + cert_contents = [] + try: + for ca_file in self.federation_custom_ca_list: + logger.debug("Reading custom CA certificate file: %s", ca_file) + with open(ca_file, 'rb') as f: + cert_contents.append(ca_file.read()) + except: + logger.exception("Failed to read custom CA certificate off disk!") + raise + + # Parse the CA certificates + certs = [] + try: + for cert in certs: + logger.debug("Parsing custom CA certificate file: %s", ca_file) + cert_base = Certificate.loadPEM(content) + certs.append(cert_base) + + trust_root = trustRootFromCertificates(certs) + except: + logger.exception("Failed to parse custom CA certificate off disk!") + raise + + self.federation_custom_ca_list = trust_root + # This config option applies to non-federation HTTP clients # (e.g. for talking to recaptcha, identity servers, and such) # It should never be used in production, and is intended for @@ -192,6 +225,16 @@ def default_config(self, config_dir_path, server_name, **kwargs): # #tls_private_key_path: "%(tls_private_key_path)s" + # List of custom certificate authorities for federation traffic. + # + # Note that this list will replace those that are provided by your + # operating environment. Certificates must be in PEM format. + # + #federation_custom_ca_list: + # - myca1.pem + # - myca2.pem + # - myca3.pem + # ACME support: This will configure Synapse to request a valid TLS certificate # for your configured `server_name` via Let's Encrypt. # diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index b99159dbbdcf..7f747cd55a6b 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -128,7 +128,10 @@ class ClientTLSOptionsFactory(object): def __init__(self, config): # We don't use config options yet - self._options_validate = CertificateOptions(verify=True) + self._options_validate = CertificateOptions( + # This option implies verify=True + trustRoot=config.federation_custom_ca_list, + ) self._options_novalidate = CertificateOptions(verify=False) def get_options(self, host, config): From 4d1002fd5290d157ee1a4cd1dc5836621937fac5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 14:39:05 +0100 Subject: [PATCH 04/56] Documentation of new options --- docs/MSC1711_certificates_FAQ.md | 35 +++++++++++++++++++++++++++++++ synapse/config/server.py | 28 ------------------------- synapse/config/tls.py | 36 +++++++++++++++++++++++++++++--- 3 files changed, 68 insertions(+), 31 deletions(-) diff --git a/docs/MSC1711_certificates_FAQ.md b/docs/MSC1711_certificates_FAQ.md index 8eb22656db1a..c7959a27ca65 100644 --- a/docs/MSC1711_certificates_FAQ.md +++ b/docs/MSC1711_certificates_FAQ.md @@ -177,6 +177,41 @@ You can do this with a `.well-known` file as follows: on `customer.example.net:8000` it correctly handles HTTP requests with Host header set to `customer.example.net:8000`. +## Turning off certificate validation + +It is possible to turn off certificate validation for remote servers, but +note that this must be explicitly enabled and is thus only suitable for +private federations. This will only disable TLS certificate validation on +federation endpoints; other requests made to recaptcha, identity services +etc. will be unaffected. + +``` +tls.federation_verify_certificates = false +``` + +You can also only disable certificate validation for a specific set of +homeservers: + +``` +tls.federation_certificate_verification_whitelist: + - subdomain.my-server.org + - example.org + - 1.2.3.4 +``` + +## Specifying your own Certificate Authorities + +If you would like to specify your own list of trusted Certificate +Authorities, you can do so with the following option. **Note that this list +will replace any certificates provided by your operating environment:** + +``` +tls.federation_custom_ca_list: + - myCA1.pem + - myCA2.pem +``` + +Certificate files must be provided in PEM format. ## FAQ diff --git a/synapse/config/server.py b/synapse/config/server.py index affba6d92012..08e4e45482e1 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -110,22 +110,6 @@ def read_config(self, config): # due to resource constraints self.admin_contact = config.get("admin_contact", None) - self.federation_verify_certificates = config.get( - "federation_verify_certificates", False, - ) - - # Whitelist of domains to not verify certificates for - self.federation_certificate_verification_whitelist = None - federation_certificate_verification_whitelist = config.get( - "federation_certificate_verification_whitelist", None - ) - - # Store whitelisted domains in a hash for fast lookup - if federation_certificate_verification_whitelist is not None: - self.federation_certificate_verification_whitelist = {} - for domain in federation_certificate_verification_whitelist: - self.federation_certificate_verification_whitelist[domain] = True - # FIXME: federation_domain_whitelist needs sytests self.federation_domain_whitelist = None federation_domain_whitelist = config.get( @@ -355,18 +339,6 @@ def default_config(self, server_name, data_dir_path, **kwargs): # #enable_search: false - # Whether to verify TLS certificates when sending federation traffic. - # - #federation_verify_certificates: true - - # Prevent federation certificate validation on the following whitelist - # of domains. Only effective if federation_verify_certicates is true. - # - #federation_certificate_validation_whitelist: - # - lon.example.com - # - nyc.example.com - # - syd.example.com - # Restrict federation to the following whitelist of domains. # N.B. we recommend also firewalling your federation listener to limit # inbound federation traffic as early as possible, rather than relying diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 5e4ed8289dd5..e2e4e15c3f85 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -72,6 +72,23 @@ def read_config(self, config): self.tls_fingerprints = list(self._original_tls_fingerprints) + # Whether to verify certificates on outbound federation traffic + self.federation_verify_certificates = config.get( + "federation_verify_certificates", False, + ) + + # Whitelist of domains to not verify certificates for + self.federation_certificate_verification_whitelist = None + federation_certificate_verification_whitelist = config.get( + "federation_certificate_verification_whitelist", None + ) + + # Store whitelisted domains in a hash for fast lookup + if federation_certificate_verification_whitelist is not None: + self.federation_certificate_verification_whitelist = {} + for domain in federation_certificate_verification_whitelist: + self.federation_certificate_verification_whitelist[domain] = True + # List of custom certificate authorities for TLS verification self.federation_custom_ca_list = config.get( "federation_custom_ca_list", [], @@ -225,15 +242,28 @@ def default_config(self, config_dir_path, server_name, **kwargs): # #tls_private_key_path: "%(tls_private_key_path)s" + # Whether to verify TLS certificates when sending federation traffic. + # + #federation_verify_certificates: true + + # Prevent federation certificate validation on the following whitelist + # of domains. Only effective if federation_verify_certicates is true. + # + #federation_certificate_validation_whitelist: + # - lon.example.com + # - nyc.example.com + # - syd.example.com + + # List of custom certificate authorities for federation traffic. # # Note that this list will replace those that are provided by your # operating environment. Certificates must be in PEM format. # #federation_custom_ca_list: - # - myca1.pem - # - myca2.pem - # - myca3.pem + # - myCA1.pem + # - myCA2.pem + # - myCA3.pem # ACME support: This will configure Synapse to request a valid TLS certificate # for your configured `server_name` via Let's Encrypt. From 904ea6c7e056ffa4b756d2e4ef6341b4bb4d1bf6 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 14:41:09 +0100 Subject: [PATCH 05/56] Add changelog --- changelog.d/4967.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4967.feature diff --git a/changelog.d/4967.feature b/changelog.d/4967.feature new file mode 100644 index 000000000000..7f9f81f849ff --- /dev/null +++ b/changelog.d/4967.feature @@ -0,0 +1 @@ +Implementation of [MSC1711](https://github.com/matrix-org/matrix-doc/pull/1711) including config options for requiring valid TLS certificates for federation traffic, the ability to disable TLS validation for specific domains, and the ability to specify your own list of CA certificates. From 8e89fc5fe373c6dc3c5d5f9c59f2b52f1070a09c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 14:45:32 +0100 Subject: [PATCH 06/56] fixes and lint --- synapse/config/tls.py | 13 ++++++------- synapse/federation/sender/per_destination_queue.py | 2 -- tests/replication/tcp/streams/test_receipts.py | 2 -- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index e2e4e15c3f85..92b4903dd072 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -19,13 +19,12 @@ from datetime import datetime from hashlib import sha256 -from twisted.internet._sslverify import trustRootFromCertificates, Certificate - import six from unpaddedbase64 import encode_base64 from OpenSSL import crypto +from twisted.internet._sslverify import trustRootFromCertificates, Certificate from synapse.config._base import Config, ConfigError @@ -100,21 +99,21 @@ def read_config(self, config): for ca_file in self.federation_custom_ca_list: logger.debug("Reading custom CA certificate file: %s", ca_file) with open(ca_file, 'rb') as f: - cert_contents.append(ca_file.read()) - except: + cert_contents.append(f.read()) + except Exception: logger.exception("Failed to read custom CA certificate off disk!") raise # Parse the CA certificates certs = [] try: - for cert in certs: + for content in cert_contents: logger.debug("Parsing custom CA certificate file: %s", ca_file) - cert_base = Certificate.loadPEM(content) + cert_base = Certificate.loadPEM(cert_contents) certs.append(cert_base) trust_root = trustRootFromCertificates(certs) - except: + except Exception: logger.exception("Failed to parse custom CA certificate off disk!") raise diff --git a/synapse/federation/sender/per_destination_queue.py b/synapse/federation/sender/per_destination_queue.py index be992110032b..46e327ea0d13 100644 --- a/synapse/federation/sender/per_destination_queue.py +++ b/synapse/federation/sender/per_destination_queue.py @@ -25,12 +25,10 @@ HttpResponseException, RequestSendFailed, ) -from synapse.events import EventBase from synapse.federation.units import Edu from synapse.handlers.presence import format_user_presence_state from synapse.metrics import sent_transactions_counter from synapse.metrics.background_process_metrics import run_as_background_process -from synapse.storage import UserPresenceState from synapse.util.retryutils import NotRetryingDestination, get_retry_limiter logger = logging.getLogger(__name__) diff --git a/tests/replication/tcp/streams/test_receipts.py b/tests/replication/tcp/streams/test_receipts.py index d5a99f6caaf3..f6dda20eba40 100644 --- a/tests/replication/tcp/streams/test_receipts.py +++ b/tests/replication/tcp/streams/test_receipts.py @@ -12,8 +12,6 @@ # 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.replication.tcp.streams._base import ReceiptsStreamRow - from tests.replication.tcp.streams._base import BaseStreamTestCase USER_ID = "@feeling:blue" From 6de7ab82cab5e6ee4bf3efb17e2a27c8e90196cd Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 14:47:19 +0100 Subject: [PATCH 07/56] flake8 lied to me :c --- synapse/federation/sender/per_destination_queue.py | 2 ++ tests/replication/tcp/streams/test_receipts.py | 2 ++ 2 files changed, 4 insertions(+) diff --git a/synapse/federation/sender/per_destination_queue.py b/synapse/federation/sender/per_destination_queue.py index 46e327ea0d13..be992110032b 100644 --- a/synapse/federation/sender/per_destination_queue.py +++ b/synapse/federation/sender/per_destination_queue.py @@ -25,10 +25,12 @@ HttpResponseException, RequestSendFailed, ) +from synapse.events import EventBase from synapse.federation.units import Edu from synapse.handlers.presence import format_user_presence_state from synapse.metrics import sent_transactions_counter from synapse.metrics.background_process_metrics import run_as_background_process +from synapse.storage import UserPresenceState from synapse.util.retryutils import NotRetryingDestination, get_retry_limiter logger = logging.getLogger(__name__) diff --git a/tests/replication/tcp/streams/test_receipts.py b/tests/replication/tcp/streams/test_receipts.py index f6dda20eba40..d5a99f6caaf3 100644 --- a/tests/replication/tcp/streams/test_receipts.py +++ b/tests/replication/tcp/streams/test_receipts.py @@ -12,6 +12,8 @@ # 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.replication.tcp.streams._base import ReceiptsStreamRow + from tests.replication.tcp.streams._base import BaseStreamTestCase USER_ID = "@feeling:blue" From 8b1c4592db49c7723dac06afeeb68ee507eb7d01 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 14:48:31 +0100 Subject: [PATCH 08/56] lint --- synapse/config/tls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 92b4903dd072..704e01b3753d 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -24,7 +24,7 @@ from unpaddedbase64 import encode_base64 from OpenSSL import crypto -from twisted.internet._sslverify import trustRootFromCertificates, Certificate +from twisted.internet._sslverify import Certificate, trustRootFromCertificates from synapse.config._base import Config, ConfigError From ffc9c10b6e521140b4f12136fd5257c25dd12ada Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 14:50:48 +0100 Subject: [PATCH 09/56] punctuation --- docs/MSC1711_certificates_FAQ.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/MSC1711_certificates_FAQ.md b/docs/MSC1711_certificates_FAQ.md index c7959a27ca65..2f949469513a 100644 --- a/docs/MSC1711_certificates_FAQ.md +++ b/docs/MSC1711_certificates_FAQ.md @@ -203,7 +203,7 @@ tls.federation_certificate_verification_whitelist: If you would like to specify your own list of trusted Certificate Authorities, you can do so with the following option. **Note that this list -will replace any certificates provided by your operating environment:** +will replace any certificates provided by your operating environment.** ``` tls.federation_custom_ca_list: From da23aa26c5958fcb243a6ab1a21d18ba2247c0cb Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 14:56:36 +0100 Subject: [PATCH 10/56] Cleaner code logic --- synapse/crypto/context_factory.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 7f747cd55a6b..2c2bfa3a89d3 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -138,12 +138,10 @@ def get_options(self, host, config): # Use _makeContext so that we get a fresh OpenSSL CTX each time. # Check if certificate validation has been enabled - if config.federation_verify_certificates: - # Check if this host is whitelisted - if host in config.federation_certificate_validation_whitelist: - return ClientTLSOptions(host, self._options_novalidate._makeContext()) - - # Otherwise require validation + if (config.federation_verify_certificates and + host not in config.federation_certificate_validation_whitelist): + # Require validation return ClientTLSOptions(host, self._options_validate._makeContext()) + # Otherwise don't require validation return ClientTLSOptions(host, self._options_novalidate._makeContext()) From 2325928149719e8bde5284c1d282edb7813e1b55 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 14:59:45 +0100 Subject: [PATCH 11/56] consolidate logic --- synapse/config/tls.py | 41 ++++++++++++++++++----------------------- 1 file changed, 18 insertions(+), 23 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 704e01b3753d..ed113ee8335f 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -88,36 +88,31 @@ def read_config(self, config): for domain in federation_certificate_verification_whitelist: self.federation_certificate_verification_whitelist[domain] = True - # List of custom certificate authorities for TLS verification + # List of custom certificate authorities for federation traffic validation self.federation_custom_ca_list = config.get( "federation_custom_ca_list", [], ) - # Read in the CA certificates - cert_contents = [] - try: - for ca_file in self.federation_custom_ca_list: - logger.debug("Reading custom CA certificate file: %s", ca_file) + # Read in and parse custom CA certificates + certs = [] + for ca_file in self.federation_custom_ca_list: + logger.debug("Reading custom CA certificate file: %s", ca_file) + try: with open(ca_file, 'rb') as f: - cert_contents.append(f.read()) - except Exception: - logger.exception("Failed to read custom CA certificate off disk!") - raise + content = f.read() + except Exception: + logger.exception("Failed to read custom CA certificate off disk!") + raise - # Parse the CA certificates - certs = [] - try: - for content in cert_contents: - logger.debug("Parsing custom CA certificate file: %s", ca_file) - cert_base = Certificate.loadPEM(cert_contents) + # Parse the CA certificates + try: + cert_base = Certificate.loadPEM(content) certs.append(cert_base) - - trust_root = trustRootFromCertificates(certs) - except Exception: - logger.exception("Failed to parse custom CA certificate off disk!") - raise - - self.federation_custom_ca_list = trust_root + except Exception: + logger.exception("Failed to parse custom CA certificate off disk!") + raise + + self.federation_custom_ca_list = trustRootFromCertificates(certs) # This config option applies to non-federation HTTP clients # (e.g. for talking to recaptcha, identity servers, and such) From 0ce5b5bcfe0481ac6865cc7aaec182c49e92b519 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 15:01:10 +0100 Subject: [PATCH 12/56] words --- synapse/config/tls.py | 2 +- synapse/crypto/context_factory.py | 7 +++---- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index ed113ee8335f..63ee3386edca 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -111,7 +111,7 @@ def read_config(self, config): except Exception: logger.exception("Failed to parse custom CA certificate off disk!") raise - + self.federation_custom_ca_list = trustRootFromCertificates(certs) # This config option applies to non-federation HTTP clients diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 2c2bfa3a89d3..bfdcd239597d 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -127,7 +127,6 @@ class ClientTLSOptionsFactory(object): to remote servers for federation.""" def __init__(self, config): - # We don't use config options yet self._options_validate = CertificateOptions( # This option implies verify=True trustRoot=config.federation_custom_ca_list, @@ -137,11 +136,11 @@ def __init__(self, config): def get_options(self, host, config): # Use _makeContext so that we get a fresh OpenSSL CTX each time. - # Check if certificate validation has been enabled + # Check if certificate verification has been enabled if (config.federation_verify_certificates and host not in config.federation_certificate_validation_whitelist): - # Require validation + # Require verification return ClientTLSOptions(host, self._options_validate._makeContext()) - # Otherwise don't require validation + # Otherwise don't require verification return ClientTLSOptions(host, self._options_novalidate._makeContext()) From 2851e647e9e01f67b86d409f0bba331d464102b6 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 15:06:58 +0100 Subject: [PATCH 13/56] Generate config and remove extra newline --- docs/sample_config.yaml | 23 +++++++++++++++++++++++ synapse/config/tls.py | 1 - 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 4ada0fba0e68..4ffe01620229 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -257,6 +257,29 @@ listeners: # #tls_private_key_path: "CONFDIR/SERVERNAME.tls.key" +# Whether to verify TLS certificates when sending federation traffic. +# +#federation_verify_certificates: true + +# Prevent federation certificate validation on the following whitelist +# of domains. Only effective if federation_verify_certicates is true. +# +#federation_certificate_validation_whitelist: +# - lon.example.com +# - nyc.example.com +# - syd.example.com + + +# List of custom certificate authorities for federation traffic. +# +# Note that this list will replace those that are provided by your +# operating environment. Certificates must be in PEM format. +# +#federation_custom_ca_list: +# - myCA1.pem +# - myCA2.pem +# - myCA3.pem + # ACME support: This will configure Synapse to request a valid TLS certificate # for your configured `server_name` via Let's Encrypt. # diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 63ee3386edca..f799ff780fe6 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -248,7 +248,6 @@ def default_config(self, config_dir_path, server_name, **kwargs): # - nyc.example.com # - syd.example.com - # List of custom certificate authorities for federation traffic. # # Note that this list will replace those that are provided by your From d95b4efb24ada82b2ab3f46a4b12d87ba1b201f4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 15:18:32 +0100 Subject: [PATCH 14/56] No tls. --- docs/MSC1711_certificates_FAQ.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/MSC1711_certificates_FAQ.md b/docs/MSC1711_certificates_FAQ.md index 2f949469513a..84bc580931a5 100644 --- a/docs/MSC1711_certificates_FAQ.md +++ b/docs/MSC1711_certificates_FAQ.md @@ -186,14 +186,14 @@ federation endpoints; other requests made to recaptcha, identity services etc. will be unaffected. ``` -tls.federation_verify_certificates = false +federation_verify_certificates = false ``` You can also only disable certificate validation for a specific set of homeservers: ``` -tls.federation_certificate_verification_whitelist: +federation_certificate_verification_whitelist: - subdomain.my-server.org - example.org - 1.2.3.4 @@ -206,7 +206,7 @@ Authorities, you can do so with the following option. **Note that this list will replace any certificates provided by your operating environment.** ``` -tls.federation_custom_ca_list: +federation_custom_ca_list: - myCA1.pem - myCA2.pem ``` From 0f0775494f04aae69a6604b7de6bce13a67dcd78 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 15:19:02 +0100 Subject: [PATCH 15/56] Fix documentation --- docs/sample_config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 4ffe01620229..937c992748e1 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -269,7 +269,6 @@ listeners: # - nyc.example.com # - syd.example.com - # List of custom certificate authorities for federation traffic. # # Note that this list will replace those that are provided by your From 9d27f8e9e8a23067afbb3c48d174db8387433cda Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 18:14:30 +0100 Subject: [PATCH 16/56] Cache config --- synapse/crypto/context_factory.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index bfdcd239597d..41f23a3e1a0b 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -127,18 +127,19 @@ class ClientTLSOptionsFactory(object): to remote servers for federation.""" def __init__(self, config): + self._config = config self._options_validate = CertificateOptions( # This option implies verify=True trustRoot=config.federation_custom_ca_list, ) self._options_novalidate = CertificateOptions(verify=False) - def get_options(self, host, config): + def get_options(self, host): # Use _makeContext so that we get a fresh OpenSSL CTX each time. # Check if certificate verification has been enabled if (config.federation_verify_certificates and - host not in config.federation_certificate_validation_whitelist): + host not in self._config.federation_certificate_validation_whitelist): # Require verification return ClientTLSOptions(host, self._options_validate._makeContext()) From 4f177c5002007be0072f4f70439cc2f9b5cf1b6e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 18:20:32 +0100 Subject: [PATCH 17/56] again --- synapse/crypto/context_factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 41f23a3e1a0b..fbe2bd454d79 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -138,7 +138,7 @@ def get_options(self, host): # Use _makeContext so that we get a fresh OpenSSL CTX each time. # Check if certificate verification has been enabled - if (config.federation_verify_certificates and + if (self._config.federation_verify_certificates and host not in self._config.federation_certificate_validation_whitelist): # Require verification return ClientTLSOptions(host, self._options_validate._makeContext()) From 5575f7a9a617e7d39dfebfe4273563581ef12f38 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 18:35:06 +0100 Subject: [PATCH 18/56] again --- synapse/crypto/context_factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index fbe2bd454d79..9667b0c80686 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -130,7 +130,7 @@ def __init__(self, config): self._config = config self._options_validate = CertificateOptions( # This option implies verify=True - trustRoot=config.federation_custom_ca_list, + trustRoot=self._config.federation_custom_ca_list, ) self._options_novalidate = CertificateOptions(verify=False) From ee0c7e1ab487b7fec7d143d06eeb5ad7d7ec4fbd Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 1 Apr 2019 18:52:13 +0100 Subject: [PATCH 19/56] again --- synapse/crypto/context_factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 9667b0c80686..fbe2bd454d79 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -130,7 +130,7 @@ def __init__(self, config): self._config = config self._options_validate = CertificateOptions( # This option implies verify=True - trustRoot=self._config.federation_custom_ca_list, + trustRoot=config.federation_custom_ca_list, ) self._options_novalidate = CertificateOptions(verify=False) From a7d7c5a060f56306006248b5583117d42cb4e0f9 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 10:53:03 +0100 Subject: [PATCH 20/56] Don't run validation code if validation is turned off --- synapse/config/tls.py | 42 ++++++++++--------- synapse/crypto/context_factory.py | 15 +++++-- .../test_matrix_federation_agent.py | 2 +- 3 files changed, 34 insertions(+), 25 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index f799ff780fe6..4e0f2d9d75cb 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -90,29 +90,31 @@ def read_config(self, config): # List of custom certificate authorities for federation traffic validation self.federation_custom_ca_list = config.get( - "federation_custom_ca_list", [], + "federation_custom_ca_list", None, ) # Read in and parse custom CA certificates - certs = [] - for ca_file in self.federation_custom_ca_list: - logger.debug("Reading custom CA certificate file: %s", ca_file) - try: - with open(ca_file, 'rb') as f: - content = f.read() - except Exception: - logger.exception("Failed to read custom CA certificate off disk!") - raise - - # Parse the CA certificates - try: - cert_base = Certificate.loadPEM(content) - certs.append(cert_base) - except Exception: - logger.exception("Failed to parse custom CA certificate off disk!") - raise - - self.federation_custom_ca_list = trustRootFromCertificates(certs) + if self.federation_custom_ca_list is not None: + certs = [] + for ca_file in self.federation_custom_ca_list: + logger.debug("Reading custom CA certificate file: %s", ca_file) + try: + with open(ca_file, 'rb') as f: + content = f.read() + except Exception: + logger.exception("Failed to read custom CA certificate off disk!") + raise + + # Parse the CA certificates + try: + cert_base = Certificate.loadPEM(content) + certs.append(cert_base) + except Exception: + logger.exception("Failed to parse custom CA certificate off disk!") + raise + + if len(certs) > 0: + self.federation_custom_ca_list = trustRootFromCertificates(certs) # This config option applies to non-federation HTTP clients # (e.g. for talking to recaptcha, identity servers, and such) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index fbe2bd454d79..97c796a04715 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -128,10 +128,17 @@ class ClientTLSOptionsFactory(object): def __init__(self, config): self._config = config - self._options_validate = CertificateOptions( - # This option implies verify=True - trustRoot=config.federation_custom_ca_list, - ) + + # Check if we're using a custom list of a CA certificates + if config.federation_custom_ca_list is not None: + self._options_validate = CertificateOptions( + # This option implies verify=True + trustRoot=config.federation_custom_ca_list, + ) + else: + # If not, verify using those provided by the operating environment + self._options_validate = CertificateOptions(verify=True) + self._options_novalidate = CertificateOptions(verify=False) def get_options(self, host): diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index dcf184d3cf64..2ca91635a94d 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -53,7 +53,7 @@ def setUp(self): self.agent = MatrixFederationAgent( reactor=self.reactor, - tls_client_options_factory=ClientTLSOptionsFactory(None), + tls_client_options_factory=ClientTLSOptionsFactory(#TODO How to deal with None config in tests???), _well_known_tls_policy=TrustingTLSPolicyForHTTPS(), _srv_resolver=self.mock_resolver, _well_known_cache=self.well_known_cache, From fec0c9a0745f5c3ebaad98f8b1788c5b74a93bef Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 10:57:18 +0100 Subject: [PATCH 21/56] Remove TODO --- synapse/crypto/context_factory.py | 1 + tests/http/federation/test_matrix_federation_agent.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 97c796a04715..0035846de00f 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -146,6 +146,7 @@ def get_options(self, host): # Check if certificate verification has been enabled if (self._config.federation_verify_certificates and + self._config.federation_certificate_validation_whitelist and host not in self._config.federation_certificate_validation_whitelist): # Require verification return ClientTLSOptions(host, self._options_validate._makeContext()) diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index 2ca91635a94d..dcf184d3cf64 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -53,7 +53,7 @@ def setUp(self): self.agent = MatrixFederationAgent( reactor=self.reactor, - tls_client_options_factory=ClientTLSOptionsFactory(#TODO How to deal with None config in tests???), + tls_client_options_factory=ClientTLSOptionsFactory(None), _well_known_tls_policy=TrustingTLSPolicyForHTTPS(), _srv_resolver=self.mock_resolver, _well_known_cache=self.well_known_cache, From aeffa4d84aa61622c12388a706efd1fc854c5ceb Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 11:09:43 +0100 Subject: [PATCH 22/56] Use platformTrust instead of verify=True --- synapse/crypto/context_factory.py | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 0035846de00f..a9561ac0b720 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -21,7 +21,7 @@ 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 +from twisted.internet.ssl import CertificateOptions, ContextFactory, platformTrust from twisted.python.failure import Failure logger = logging.getLogger(__name__) @@ -132,14 +132,17 @@ def __init__(self, config): # Check if we're using a custom list of a CA certificates if config.federation_custom_ca_list is not None: self._options_validate = CertificateOptions( - # This option implies verify=True + # Use custom CA trusted root certs trustRoot=config.federation_custom_ca_list, ) else: # If not, verify using those provided by the operating environment - self._options_validate = CertificateOptions(verify=True) + self._options_validate = CertificateOptions( + # Use CA root certs provided by OpenSSL + trustRoot=platformTrust(), + ) - self._options_novalidate = CertificateOptions(verify=False) + self._options_novalidate = CertificateOptions() def get_options(self, host): # Use _makeContext so that we get a fresh OpenSSL CTX each time. From 821baa4e7b8660243561299c8400a51abe8fabf4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 11:30:51 +0100 Subject: [PATCH 23/56] Give tests config with default config values --- synapse/crypto/context_factory.py | 1 - tests/http/federation/test_matrix_federation_agent.py | 7 ++++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index a9561ac0b720..9d890ae8900a 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -149,7 +149,6 @@ def get_options(self, host): # Check if certificate verification has been enabled if (self._config.federation_verify_certificates and - self._config.federation_certificate_validation_whitelist and host not in self._config.federation_certificate_validation_whitelist): # Require verification return ClientTLSOptions(host, self._options_validate._makeContext()) diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index dcf184d3cf64..0e732580cce9 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -49,11 +49,16 @@ def setUp(self): self.mock_resolver = Mock() + config = Mock() + config.federation_custom_ca_list = None + config.federation_verify_certificates = False + config.federation_certificate_validation_whitelist = [] + self.well_known_cache = TTLCache("test_cache", timer=self.reactor.seconds) self.agent = MatrixFederationAgent( reactor=self.reactor, - tls_client_options_factory=ClientTLSOptionsFactory(None), + tls_client_options_factory=ClientTLSOptionsFactory(config), _well_known_tls_policy=TrustingTLSPolicyForHTTPS(), _srv_resolver=self.mock_resolver, _well_known_cache=self.well_known_cache, From 40702b638ee46ea64b78fe35e5472e36e8f0ba28 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 11:47:00 +0100 Subject: [PATCH 24/56] Check for type instead of not None --- synapse/crypto/context_factory.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 9d890ae8900a..941ae7debc5c 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -21,7 +21,7 @@ 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, platformTrust, OpenSSLCertificateAuthorities from twisted.python.failure import Failure logger = logging.getLogger(__name__) @@ -129,20 +129,21 @@ class ClientTLSOptionsFactory(object): def __init__(self, config): self._config = config + self._options_novalidate = CertificateOptions() + # Check if we're using a custom list of a CA certificates - if config.federation_custom_ca_list is not None: + if isinstance(config.federation_custom_ca_list, OpenSSLCertificateAuthorities): self._options_validate = CertificateOptions( # Use custom CA trusted root certs trustRoot=config.federation_custom_ca_list, ) - else: - # If not, verify using those provided by the operating environment - self._options_validate = CertificateOptions( - # Use CA root certs provided by OpenSSL - trustRoot=platformTrust(), - ) + return - self._options_novalidate = CertificateOptions() + # If not, verify using those provided by the operating environment + self._options_validate = CertificateOptions( + # Use CA root certs provided by OpenSSL + trustRoot=platformTrust(), + ) def get_options(self, host): # Use _makeContext so that we get a fresh OpenSSL CTX each time. From cc149b11ea89d4d8962b5a5117fa2c747fcd96e5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 11:49:10 +0100 Subject: [PATCH 25/56] Give MFA config access --- synapse/http/federation/matrix_federation_agent.py | 5 +++-- synapse/http/matrixfederationclient.py | 1 + 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py index 89856492272c..7c5df38f5f62 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/synapse/http/federation/matrix_federation_agent.py @@ -79,10 +79,11 @@ class MatrixFederationAgent(object): def __init__( self, reactor, tls_client_options_factory, - _well_known_tls_policy=None, + config, _well_known_tls_policy=None, _srv_resolver=None, _well_known_cache=well_known_cache, ): + self.config = config self._reactor = reactor self._clock = Clock(reactor) @@ -149,7 +150,7 @@ def request(self, method, uri, headers=None, bodyProducer=None): tls_options = None else: tls_options = self._tls_client_options_factory.get_options( - res.tls_server_name.decode("ascii"), self.hs.config, + res.tls_server_name.decode("ascii"), self.config, ) # make sure that the Host header is set correctly diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index ff63d0b2a838..cd17b48b7151 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -177,6 +177,7 @@ def __init__(self, hs, tls_client_options_factory): self.agent = MatrixFederationAgent( hs.get_reactor(), tls_client_options_factory, + hs.config, ) self.clock = hs.get_clock() self._store = hs.get_datastore() From 86470139e06879c161526debb3f73c32f61f7272 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 12:05:40 +0100 Subject: [PATCH 26/56] lint --- synapse/crypto/context_factory.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 941ae7debc5c..687b6be961c9 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -21,7 +21,12 @@ 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, OpenSSLCertificateAuthorities +from twisted.internet.ssl import ( + CertificateOptions, + ContextFactory, + platformTrust, + OpenSSLCertificateAuthorities, +) from twisted.python.failure import Failure logger = logging.getLogger(__name__) From d194e5daf5b63765ebe1a6148683caed502c6a96 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 12:07:38 +0100 Subject: [PATCH 27/56] liiint --- synapse/crypto/context_factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 687b6be961c9..bfd7910f3065 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -24,8 +24,8 @@ from twisted.internet.ssl import ( CertificateOptions, ContextFactory, - platformTrust, OpenSSLCertificateAuthorities, + platformTrust, ) from twisted.python.failure import Failure From 3adf15d3579bc4361844bab046f7c203fceb0ef0 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 13:12:21 +0100 Subject: [PATCH 28/56] Correct imports --- synapse/crypto/context_factory.py | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index bfd7910f3065..3faca79200b2 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -21,12 +21,8 @@ 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, - OpenSSLCertificateAuthorities, - platformTrust, -) +from twisted.internet.ssl import CertificateOptions, ContextFactory, platformTrust +from twisted.internet._sslverify import OpenSSLCertificateAuthorities from twisted.python.failure import Failure logger = logging.getLogger(__name__) From 61a39a44d3577307f638f76f85a8f722a1ba15f6 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 13:15:23 +0100 Subject: [PATCH 29/56] isort --- synapse/crypto/context_factory.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 3faca79200b2..a236d4896b0b 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -18,11 +18,10 @@ from zope.interface import implementer 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._sslverify import OpenSSLCertificateAuthorities +from twisted.internet._sslverify import OpenSSLCertificateAuthorities, _defaultCurveName from twisted.python.failure import Failure logger = logging.getLogger(__name__) From 3e29d458f277d3414bc4b6b695da4695f351c08c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 13:18:49 +0100 Subject: [PATCH 30/56] isort --- synapse/crypto/context_factory.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index a236d4896b0b..db2fbb40976b 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -18,10 +18,10 @@ from zope.interface import implementer from OpenSSL import SSL, crypto +from twisted.internet._sslverify import OpenSSLCertificateAuthorities, _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._sslverify import OpenSSLCertificateAuthorities, _defaultCurveName from twisted.python.failure import Failure logger = logging.getLogger(__name__) From f38da61ae7d7a803eb5af5f14b1a35f6e76c1680 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 13:35:53 +0100 Subject: [PATCH 31/56] Pass config --- tests/http/federation/test_matrix_federation_agent.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index 0e732580cce9..a9424104926d 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -59,6 +59,7 @@ def setUp(self): self.agent = MatrixFederationAgent( reactor=self.reactor, tls_client_options_factory=ClientTLSOptionsFactory(config), + config=config, _well_known_tls_policy=TrustingTLSPolicyForHTTPS(), _srv_resolver=self.mock_resolver, _well_known_cache=self.well_known_cache, From 3f1fe92a3bf5768b064b6da165220e721ff8b88f Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 13:45:58 +0100 Subject: [PATCH 32/56] Shuffle config --- synapse/http/federation/matrix_federation_agent.py | 5 ++--- tests/http/federation/test_matrix_federation_agent.py | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py index 7c5df38f5f62..b4cbe97b41fa 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/synapse/http/federation/matrix_federation_agent.py @@ -79,11 +79,10 @@ class MatrixFederationAgent(object): def __init__( self, reactor, tls_client_options_factory, - config, _well_known_tls_policy=None, + _well_known_tls_policy=None, _srv_resolver=None, _well_known_cache=well_known_cache, ): - self.config = config self._reactor = reactor self._clock = Clock(reactor) @@ -150,7 +149,7 @@ def request(self, method, uri, headers=None, bodyProducer=None): tls_options = None else: tls_options = self._tls_client_options_factory.get_options( - res.tls_server_name.decode("ascii"), self.config, + res.tls_server_name.decode("ascii"), ) # make sure that the Host header is set correctly diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index a9424104926d..0e732580cce9 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -59,7 +59,6 @@ def setUp(self): self.agent = MatrixFederationAgent( reactor=self.reactor, tls_client_options_factory=ClientTLSOptionsFactory(config), - config=config, _well_known_tls_policy=TrustingTLSPolicyForHTTPS(), _srv_resolver=self.mock_resolver, _well_known_cache=self.well_known_cache, From a8adde06244a3ecb891f9c5146275db68f0c11dd Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Tue, 2 Apr 2019 14:39:58 +0100 Subject: [PATCH 33/56] Restart build From e083ae39cd451b42f75c1ca5f24505b663227718 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 18:10:13 +0100 Subject: [PATCH 34/56] Address changes. Make federation_domain_whitelist not None --- synapse/config/server.py | 10 +++--- synapse/config/tls.py | 35 ++++++++++--------- synapse/crypto/context_factory.py | 9 +++-- synapse/federation/transport/server.py | 5 +-- synapse/http/matrixfederationclient.py | 6 +--- synapse/rest/key/v2/remote_key_resource.py | 5 +-- synapse/rest/media/v1/media_repository.py | 10 ++---- .../test_matrix_federation_agent.py | 8 ++--- tests/utils.py | 5 ++- 9 files changed, 40 insertions(+), 53 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 08e4e45482e1..aa2bb0d040ec 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -111,15 +111,13 @@ def read_config(self, config): self.admin_contact = config.get("admin_contact", None) # FIXME: federation_domain_whitelist needs sytests - self.federation_domain_whitelist = None + self.federation_domain_whitelist = {} federation_domain_whitelist = config.get( - "federation_domain_whitelist", None + "federation_domain_whitelist", [], ) # turn the whitelist into a hash for speed of lookup - if federation_domain_whitelist is not None: - self.federation_domain_whitelist = {} - for domain in federation_domain_whitelist: - self.federation_domain_whitelist[domain] = True + for domain in federation_domain_whitelist: + self.federation_domain_whitelist[domain] = True if self.public_baseurl is not None: if self.public_baseurl[-1] != '/': diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 4e0f2d9d75cb..05b6a6165f1b 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -77,16 +77,14 @@ def read_config(self, config): ) # Whitelist of domains to not verify certificates for - self.federation_certificate_verification_whitelist = None + self.federation_certificate_verification_whitelist = {} federation_certificate_verification_whitelist = config.get( - "federation_certificate_verification_whitelist", None + "federation_certificate_verification_whitelist", [], ) # Store whitelisted domains in a hash for fast lookup - if federation_certificate_verification_whitelist is not None: - self.federation_certificate_verification_whitelist = {} - for domain in federation_certificate_verification_whitelist: - self.federation_certificate_verification_whitelist[domain] = True + for domain in federation_certificate_verification_whitelist: + self.federation_certificate_verification_whitelist[domain] = True # List of custom certificate authorities for federation traffic validation self.federation_custom_ca_list = config.get( @@ -102,7 +100,7 @@ def read_config(self, config): with open(ca_file, 'rb') as f: content = f.read() except Exception: - logger.exception("Failed to read custom CA certificate off disk!") + logger.fatal("Failed to read custom CA certificate off disk!") raise # Parse the CA certificates @@ -110,11 +108,10 @@ def read_config(self, config): cert_base = Certificate.loadPEM(content) certs.append(cert_base) except Exception: - logger.exception("Failed to parse custom CA certificate off disk!") + logger.fatal("Failed to parse custom CA certificate off disk!") raise - if len(certs) > 0: - self.federation_custom_ca_list = trustRootFromCertificates(certs) + self.federation_custom_ca_list = trustRootFromCertificates(certs) # This config option applies to non-federation HTTP clients # (e.g. for talking to recaptcha, identity servers, and such) @@ -146,14 +143,12 @@ def is_disk_cert_valid(self, allow_self_signed=True): with open(self.tls_certificate_file, 'rb') as f: cert_pem = f.read() except Exception: - logger.exception("Failed to read existing certificate off disk!") - raise + logger.fatal("Failed to read existing certificate off disk!") try: tls_certificate = crypto.load_certificate(crypto.FILETYPE_PEM, cert_pem) except Exception: - logger.exception("Failed to parse existing certificate off disk!") - raise + logger.fatal("Failed to parse existing certificate off disk!") if not allow_self_signed: if tls_certificate.get_subject() == tls_certificate.get_issuer(): @@ -240,10 +235,18 @@ def default_config(self, config_dir_path, server_name, **kwargs): # Whether to verify TLS certificates when sending federation traffic. # + # This currently defaults to `false`, however this will change in + # Synapse 1.0 when valid federation certificates will be required. + # #federation_verify_certificates: true - # Prevent federation certificate validation on the following whitelist - # of domains. Only effective if federation_verify_certicates is true. + # Skip federation certificate validation on the following whitelist of + # domains. + # + # Note that this should only be used within the context of private + # federation as it will otherwise break things. + # + # Only effective if federation_verify_certicates is `true`. # #federation_certificate_validation_whitelist: # - lon.example.com diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index db2fbb40976b..d566e1bf23f9 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -18,7 +18,10 @@ from zope.interface import implementer from OpenSSL import SSL, crypto -from twisted.internet._sslverify import OpenSSLCertificateAuthorities, _defaultCurveName +from twisted.internet._sslverify import ( + ClientTLSOptions as ClientTLSOptionsVerify, + _defaultCurveName, +) from twisted.internet.abstract import isIPAddress, isIPv6Address from twisted.internet.interfaces import IOpenSSLClientConnectionCreator from twisted.internet.ssl import CertificateOptions, ContextFactory, platformTrust @@ -132,7 +135,7 @@ def __init__(self, config): self._options_novalidate = CertificateOptions() # Check if we're using a custom list of a CA certificates - if isinstance(config.federation_custom_ca_list, OpenSSLCertificateAuthorities): + if config.federation_custom_ca_list is not None: self._options_validate = CertificateOptions( # Use custom CA trusted root certs trustRoot=config.federation_custom_ca_list, @@ -152,7 +155,7 @@ def get_options(self, host): if (self._config.federation_verify_certificates and host not in self._config.federation_certificate_validation_whitelist): # Require verification - return ClientTLSOptions(host, self._options_validate._makeContext()) + return ClientTLSOptionsVerify(host, self._options_validate._makeContext()) # Otherwise don't require verification return ClientTLSOptions(host, self._options_novalidate._makeContext()) diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index 452599e1a15e..f28672f7e228 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -127,10 +127,7 @@ def authenticate_request(self, request, content): json_request["origin"] = origin json_request["signatures"].setdefault(origin, {})[key] = sig - if ( - self.federation_domain_whitelist is not None and - origin not in self.federation_domain_whitelist - ): + if (origin not in self.federation_domain_whitelist): raise FederationDeniedError(origin) if not json_request["signatures"]: diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index cd17b48b7151..c4fdcd0524a0 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -177,7 +177,6 @@ def __init__(self, hs, tls_client_options_factory): self.agent = MatrixFederationAgent( hs.get_reactor(), tls_client_options_factory, - hs.config, ) self.clock = hs.get_clock() self._store = hs.get_datastore() @@ -284,10 +283,7 @@ def _send_request( else: _sec_timeout = self.default_timeout - if ( - self.hs.config.federation_domain_whitelist is not None and - request.destination not in self.hs.config.federation_domain_whitelist - ): + if (request.destination not in self.hs.config.federation_domain_whitelist): raise FederationDeniedError(request.destination) limiter = yield synapse.util.retryutils.get_retry_limiter( diff --git a/synapse/rest/key/v2/remote_key_resource.py b/synapse/rest/key/v2/remote_key_resource.py index eb8782aa6e1a..dbd4512b74db 100644 --- a/synapse/rest/key/v2/remote_key_resource.py +++ b/synapse/rest/key/v2/remote_key_resource.py @@ -139,10 +139,7 @@ def query_keys(self, request, query, query_remote_on_cache_miss=False): store_queries = [] for server_name, key_ids in query.items(): - if ( - self.federation_domain_whitelist is not None and - server_name not in self.federation_domain_whitelist - ): + if (server_name not in self.federation_domain_whitelist): logger.debug("Federation denied with %s", server_name) continue diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index bdffa9780558..dec1206e39c8 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -231,10 +231,7 @@ def get_remote_media(self, request, server_name, media_id, name): Deferred: Resolves once a response has successfully been written to request """ - if ( - self.federation_domain_whitelist is not None and - server_name not in self.federation_domain_whitelist - ): + if (server_name not in self.federation_domain_whitelist): raise FederationDeniedError(server_name) self.mark_recently_accessed(server_name, media_id) @@ -271,10 +268,7 @@ def get_remote_media_info(self, server_name, media_id): Returns: Deferred[dict]: The media_info of the file """ - if ( - self.federation_domain_whitelist is not None and - server_name not in self.federation_domain_whitelist - ): + if (server_name not in self.federation_domain_whitelist): raise FederationDeniedError(server_name) # We linearize here to ensure that we don't try and download remote diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index 0e732580cce9..dec91e86a3bf 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -39,6 +39,7 @@ from tests.http import ServerTLSContext from tests.server import FakeTransport, ThreadedMemoryReactorClock from tests.unittest import TestCase +from tests.utils import default_config logger = logging.getLogger(__name__) @@ -49,16 +50,11 @@ def setUp(self): self.mock_resolver = Mock() - config = Mock() - config.federation_custom_ca_list = None - config.federation_verify_certificates = False - config.federation_certificate_validation_whitelist = [] - self.well_known_cache = TTLCache("test_cache", timer=self.reactor.seconds) self.agent = MatrixFederationAgent( reactor=self.reactor, - tls_client_options_factory=ClientTLSOptionsFactory(config), + tls_client_options_factory=ClientTLSOptionsFactory(default_config()), _well_known_tls_policy=TrustingTLSPolicyForHTTPS(), _srv_resolver=self.mock_resolver, _well_known_cache=self.well_known_cache, diff --git a/tests/utils.py b/tests/utils.py index cb75514851ea..c6952938d032 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -136,7 +136,10 @@ def default_config(name): config.worker_app = None config.email_enable_notifs = False config.block_non_admin_invites = False - config.federation_domain_whitelist = None + config.federation_domain_whitelist = [] + config.federation_certificate_verification_whitelist = [] + config.federation_custom_ca_list = [] + config.federation_verify_certificates = False config.federation_rc_reject_limit = 10 config.federation_rc_sleep_limit = 10 config.federation_rc_sleep_delay = 100 From a87b556ab89bfa838f4c8c82e02f3195dc4ebbc4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Tue, 2 Apr 2019 18:13:48 +0100 Subject: [PATCH 35/56] regen sample config --- docs/sample_config.yaml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 937c992748e1..88ed8f6f0c33 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -259,10 +259,18 @@ listeners: # Whether to verify TLS certificates when sending federation traffic. # +# This currently defaults to `false`, however this will change in +# Synapse 1.0 when valid federation certificates will be required. +# #federation_verify_certificates: true -# Prevent federation certificate validation on the following whitelist -# of domains. Only effective if federation_verify_certicates is true. +# Skip federation certificate validation on the following whitelist of +# domains. +# +# Note that this should only be used within the context of private +# federation as it will otherwise break things. +# +# Only effective if federation_verify_certicates is `true`. # #federation_certificate_validation_whitelist: # - lon.example.com From 983474d2507f4b5221e16fa8105ea3dcbe456687 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 10:50:37 +0100 Subject: [PATCH 36/56] Remove turning cert validation off from faq --- docs/MSC1711_certificates_FAQ.md | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/docs/MSC1711_certificates_FAQ.md b/docs/MSC1711_certificates_FAQ.md index 84bc580931a5..8549f126a449 100644 --- a/docs/MSC1711_certificates_FAQ.md +++ b/docs/MSC1711_certificates_FAQ.md @@ -177,28 +177,6 @@ You can do this with a `.well-known` file as follows: on `customer.example.net:8000` it correctly handles HTTP requests with Host header set to `customer.example.net:8000`. -## Turning off certificate validation - -It is possible to turn off certificate validation for remote servers, but -note that this must be explicitly enabled and is thus only suitable for -private federations. This will only disable TLS certificate validation on -federation endpoints; other requests made to recaptcha, identity services -etc. will be unaffected. - -``` -federation_verify_certificates = false -``` - -You can also only disable certificate validation for a specific set of -homeservers: - -``` -federation_certificate_verification_whitelist: - - subdomain.my-server.org - - example.org - - 1.2.3.4 -``` - ## Specifying your own Certificate Authorities If you would like to specify your own list of trusted Certificate From 1fd5680496586f8d4ab3d902aa629f13ce0bde25 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 10:51:43 +0100 Subject: [PATCH 37/56] Don't laugh at my own jokes --- synapse/config/tls.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 05b6a6165f1b..dbfeb236e44b 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -100,7 +100,7 @@ def read_config(self, config): with open(ca_file, 'rb') as f: content = f.read() except Exception: - logger.fatal("Failed to read custom CA certificate off disk!") + logger.fatal("Failed to read custom CA certificate off disk") raise # Parse the CA certificates @@ -108,7 +108,7 @@ def read_config(self, config): cert_base = Certificate.loadPEM(content) certs.append(cert_base) except Exception: - logger.fatal("Failed to parse custom CA certificate off disk!") + logger.fatal("Failed to parse custom CA certificate off disk") raise self.federation_custom_ca_list = trustRootFromCertificates(certs) @@ -143,12 +143,12 @@ def is_disk_cert_valid(self, allow_self_signed=True): with open(self.tls_certificate_file, 'rb') as f: cert_pem = f.read() except Exception: - logger.fatal("Failed to read existing certificate off disk!") + logger.fatal("Failed to read existing certificate off disk") try: tls_certificate = crypto.load_certificate(crypto.FILETYPE_PEM, cert_pem) except Exception: - logger.fatal("Failed to parse existing certificate off disk!") + logger.fatal("Failed to parse existing certificate off disk") if not allow_self_signed: if tls_certificate.get_subject() == tls_certificate.get_issuer(): From 7c432de63e0cdf575ceff352ece45fe7606356d7 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 10:58:50 +0100 Subject: [PATCH 38/56] Simplify with better exception handling --- synapse/config/tls.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index dbfeb236e44b..2022da51474d 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -96,20 +96,15 @@ def read_config(self, config): certs = [] for ca_file in self.federation_custom_ca_list: logger.debug("Reading custom CA certificate file: %s", ca_file) - try: - with open(ca_file, 'rb') as f: - content = f.read() - except Exception: - logger.fatal("Failed to read custom CA certificate off disk") - raise + content = self.read_file(ca_file) # Parse the CA certificates try: cert_base = Certificate.loadPEM(content) certs.append(cert_base) - except Exception: - logger.fatal("Failed to parse custom CA certificate off disk") - raise + except Exception as e: + raise ConfigError("Error parsing custom CA certificate file %s: %s" + % (ca_file, e)) self.federation_custom_ca_list = trustRootFromCertificates(certs) From 4f03528d345e95ec5ed63615c37df55a6e7ca93d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 11:11:13 +0100 Subject: [PATCH 39/56] Raise config error if empty ca list --- synapse/config/tls.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 2022da51474d..e8d417d02495 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -93,6 +93,10 @@ def read_config(self, config): # Read in and parse custom CA certificates if self.federation_custom_ca_list is not None: + if self.federation_custom_ca_list: + raise ConfigError("federation_custom_ca_list specified without " + "any certificate files") + certs = [] for ca_file in self.federation_custom_ca_list: logger.debug("Reading custom CA certificate file: %s", ca_file) From 892c71dd535ee35f29a2ecef5a0af827b935624e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 11:12:01 +0100 Subject: [PATCH 40/56] Change test defaults --- tests/utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/utils.py b/tests/utils.py index c6952938d032..054a48bb246a 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -136,9 +136,9 @@ def default_config(name): config.worker_app = None config.email_enable_notifs = False config.block_non_admin_invites = False - config.federation_domain_whitelist = [] - config.federation_certificate_verification_whitelist = [] - config.federation_custom_ca_list = [] + config.federation_domain_whitelist = None + config.federation_certificate_verification_whitelist = None + config.federation_custom_ca_list = None config.federation_verify_certificates = False config.federation_rc_reject_limit = 10 config.federation_rc_sleep_limit = 10 From a5ab4afced2074daf3eafc33c6d42d85d7c8ccb5 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 11:34:37 +0100 Subject: [PATCH 41/56] None is very different from [] --- synapse/config/tls.py | 5 ++++- synapse/crypto/context_factory.py | 19 +++++++++++-------- synapse/federation/transport/server.py | 3 ++- synapse/http/matrixfederationclient.py | 3 ++- synapse/rest/key/v2/remote_key_resource.py | 3 ++- synapse/rest/media/v1/media_repository.py | 6 ++++-- 6 files changed, 25 insertions(+), 14 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index e8d417d02495..7dbf41887b24 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -77,11 +77,14 @@ def read_config(self, config): ) # Whitelist of domains to not verify certificates for - self.federation_certificate_verification_whitelist = {} federation_certificate_verification_whitelist = config.get( "federation_certificate_verification_whitelist", [], ) + self.federation_certificate_verification_whitelist = None + if len(federation_certificate_verification_whitelist) > 0: + self.federation_certificate_verification_whitelist = {} + # Store whitelisted domains in a hash for fast lookup for domain in federation_certificate_verification_whitelist: self.federation_certificate_verification_whitelist[domain] = True diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index d566e1bf23f9..e2b5ce173bf6 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -132,18 +132,18 @@ class ClientTLSOptionsFactory(object): def __init__(self, config): self._config = config - self._options_novalidate = CertificateOptions() + self._options_noverify = CertificateOptions() # Check if we're using a custom list of a CA certificates if config.federation_custom_ca_list is not None: - self._options_validate = CertificateOptions( + self._options_verify = CertificateOptions( # Use custom CA trusted root certs trustRoot=config.federation_custom_ca_list, ) return # If not, verify using those provided by the operating environment - self._options_validate = CertificateOptions( + self._options_verify = CertificateOptions( # Use CA root certs provided by OpenSSL trustRoot=platformTrust(), ) @@ -152,10 +152,13 @@ def get_options(self, host): # Use _makeContext so that we get a fresh OpenSSL CTX each time. # Check if certificate verification has been enabled - if (self._config.federation_verify_certificates and - host not in self._config.federation_certificate_validation_whitelist): - # Require verification - return ClientTLSOptionsVerify(host, self._options_validate._makeContext()) + if (self._config.federation_verify_certificates): + # and if the host is whitelisted against it + if (self._config.federation_certificate_verification_whitelist and + host in self._config.federation_certificate_verification_whitelist): + return ClientTLSOptions(host, self._options_noverify._makeContext()) + + return ClientTLSOptionsVerify(host, self._options_verify._makeContext()) # Otherwise don't require verification - return ClientTLSOptions(host, self._options_novalidate._makeContext()) + return ClientTLSOptions(host, self._options_noverify._makeContext()) diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index f28672f7e228..1ddfbbd7f4f7 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -127,7 +127,8 @@ def authenticate_request(self, request, content): json_request["origin"] = origin json_request["signatures"].setdefault(origin, {})[key] = sig - if (origin not in self.federation_domain_whitelist): + if (self.federation_domain_whitelist is not None and + origin not in self.federation_domain_whitelist): raise FederationDeniedError(origin) if not json_request["signatures"]: diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index c4fdcd0524a0..36d1015514ad 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -283,7 +283,8 @@ def _send_request( else: _sec_timeout = self.default_timeout - if (request.destination not in self.hs.config.federation_domain_whitelist): + if (self.hs.config.federation_domain_whitelist and + request.destination not in self.hs.config.federation_domain_whitelist): raise FederationDeniedError(request.destination) limiter = yield synapse.util.retryutils.get_retry_limiter( diff --git a/synapse/rest/key/v2/remote_key_resource.py b/synapse/rest/key/v2/remote_key_resource.py index dbd4512b74db..426c05e79c11 100644 --- a/synapse/rest/key/v2/remote_key_resource.py +++ b/synapse/rest/key/v2/remote_key_resource.py @@ -139,7 +139,8 @@ def query_keys(self, request, query, query_remote_on_cache_miss=False): store_queries = [] for server_name, key_ids in query.items(): - if (server_name not in self.federation_domain_whitelist): + if (self.federation_domain_whitelist and + server_name not in self.federation_domain_whitelist): logger.debug("Federation denied with %s", server_name) continue diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index dec1206e39c8..8b16ceb3e9b1 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -231,7 +231,8 @@ def get_remote_media(self, request, server_name, media_id, name): Deferred: Resolves once a response has successfully been written to request """ - if (server_name not in self.federation_domain_whitelist): + if (self.federation_domain_whitelist is not None and + server_name not in self.federation_domain_whitelist): raise FederationDeniedError(server_name) self.mark_recently_accessed(server_name, media_id) @@ -268,7 +269,8 @@ def get_remote_media_info(self, server_name, media_id): Returns: Deferred[dict]: The media_info of the file """ - if (server_name not in self.federation_domain_whitelist): + if (self.federation_domain_whitelist and + server_name not in self.federation_domain_whitelist): raise FederationDeniedError(server_name) # We linearize here to ensure that we don't try and download remote From 999f7db6b0ad735ee7c952858cc19d617aac61b4 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 11:41:28 +0100 Subject: [PATCH 42/56] Don't break logic when refactoring --- synapse/http/matrixfederationclient.py | 2 +- synapse/rest/key/v2/remote_key_resource.py | 2 +- synapse/rest/media/v1/media_repository.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 36d1015514ad..b834d062196a 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -283,7 +283,7 @@ def _send_request( else: _sec_timeout = self.default_timeout - if (self.hs.config.federation_domain_whitelist and + if (self.hs.config.federation_domain_whitelist is not None and request.destination not in self.hs.config.federation_domain_whitelist): raise FederationDeniedError(request.destination) diff --git a/synapse/rest/key/v2/remote_key_resource.py b/synapse/rest/key/v2/remote_key_resource.py index 426c05e79c11..5a14cfd426b1 100644 --- a/synapse/rest/key/v2/remote_key_resource.py +++ b/synapse/rest/key/v2/remote_key_resource.py @@ -139,7 +139,7 @@ def query_keys(self, request, query, query_remote_on_cache_miss=False): store_queries = [] for server_name, key_ids in query.items(): - if (self.federation_domain_whitelist and + if (self.federation_domain_whitelist is not None and server_name not in self.federation_domain_whitelist): logger.debug("Federation denied with %s", server_name) continue diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index 8b16ceb3e9b1..40b10e68c069 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -269,7 +269,7 @@ def get_remote_media_info(self, server_name, media_id): Returns: Deferred[dict]: The media_info of the file """ - if (self.federation_domain_whitelist and + if (self.federation_domain_whitelist is not None and server_name not in self.federation_domain_whitelist): raise FederationDeniedError(server_name) From 507cdf2b6f00317e88a76809511239766b5656b9 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 11:57:09 +0100 Subject: [PATCH 43/56] fix domain whitelist --- docs/sample_config.yaml | 13 +++---------- synapse/config/server.py | 6 +++++- synapse/config/tls.py | 6 +++--- synapse/crypto/context_factory.py | 1 - 4 files changed, 11 insertions(+), 15 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 88ed8f6f0c33..4ffe01620229 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -259,24 +259,17 @@ listeners: # Whether to verify TLS certificates when sending federation traffic. # -# This currently defaults to `false`, however this will change in -# Synapse 1.0 when valid federation certificates will be required. -# #federation_verify_certificates: true -# Skip federation certificate validation on the following whitelist of -# domains. -# -# Note that this should only be used within the context of private -# federation as it will otherwise break things. -# -# Only effective if federation_verify_certicates is `true`. +# Prevent federation certificate validation on the following whitelist +# of domains. Only effective if federation_verify_certicates is true. # #federation_certificate_validation_whitelist: # - lon.example.com # - nyc.example.com # - syd.example.com + # List of custom certificate authorities for federation traffic. # # Note that this list will replace those that are provided by your diff --git a/synapse/config/server.py b/synapse/config/server.py index aa2bb0d040ec..f55a71d508ef 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -111,10 +111,14 @@ def read_config(self, config): self.admin_contact = config.get("admin_contact", None) # FIXME: federation_domain_whitelist needs sytests - self.federation_domain_whitelist = {} federation_domain_whitelist = config.get( "federation_domain_whitelist", [], ) + + self.federation_domain_whitelist = None + if len(federation_domain_whitelist) > 0: + self.federation_domain_whitelist = {} + # turn the whitelist into a hash for speed of lookup for domain in federation_domain_whitelist: self.federation_domain_whitelist[domain] = True diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 7dbf41887b24..d157e310e4b2 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -242,15 +242,15 @@ def default_config(self, config_dir_path, server_name, **kwargs): # #federation_verify_certificates: true - # Skip federation certificate validation on the following whitelist of - # domains. + # Skip federation certificate verification on the following whitelist + # of domains. # # Note that this should only be used within the context of private # federation as it will otherwise break things. # # Only effective if federation_verify_certicates is `true`. # - #federation_certificate_validation_whitelist: + #federation_certificate_verification_whitelist: # - lon.example.com # - nyc.example.com # - syd.example.com diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index e2b5ce173bf6..1ee87cdd13c5 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -131,7 +131,6 @@ class ClientTLSOptionsFactory(object): def __init__(self, config): self._config = config - self._options_noverify = CertificateOptions() # Check if we're using a custom list of a CA certificates From 9bd1432f91c9984fbe842219fd38a89280720693 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 12:01:27 +0100 Subject: [PATCH 44/56] regen config --- docs/sample_config.yaml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 4ffe01620229..13482c16fe70 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -264,12 +264,11 @@ listeners: # Prevent federation certificate validation on the following whitelist # of domains. Only effective if federation_verify_certicates is true. # -#federation_certificate_validation_whitelist: +#federation_certificate_verification_whitelist: # - lon.example.com # - nyc.example.com # - syd.example.com - # List of custom certificate authorities for federation traffic. # # Note that this list will replace those that are provided by your From d3f926a5c3ab687447b4dd916c2d14b8433f178f Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 13:27:03 +0100 Subject: [PATCH 45/56] regen --- docs/sample_config.yaml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 13482c16fe70..be5ae548ece6 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -259,10 +259,18 @@ listeners: # Whether to verify TLS certificates when sending federation traffic. # +# This currently defaults to `false`, however this will change in +# Synapse 1.0 when valid federation certificates will be required. +# #federation_verify_certificates: true -# Prevent federation certificate validation on the following whitelist -# of domains. Only effective if federation_verify_certicates is true. +# Skip federation certificate verification on the following whitelist +# of domains. +# +# Note that this should only be used within the context of private +# federation as it will otherwise break things. +# +# Only effective if federation_verify_certicates is `true`. # #federation_certificate_verification_whitelist: # - lon.example.com From 09f66222d9d00d39a8250b70614afee18f30add9 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 3 Apr 2019 13:29:16 +0100 Subject: [PATCH 46/56] provide an arg to default_config --- tests/http/federation/test_matrix_federation_agent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index dec91e86a3bf..e9eb662c4c71 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -54,7 +54,7 @@ def setUp(self): self.agent = MatrixFederationAgent( reactor=self.reactor, - tls_client_options_factory=ClientTLSOptionsFactory(default_config()), + tls_client_options_factory=ClientTLSOptionsFactory(default_config("test")), _well_known_tls_policy=TrustingTLSPolicyForHTTPS(), _srv_resolver=self.mock_resolver, _well_known_cache=self.well_known_cache, From e337c2d9dbebaf6ef6e7a257394f01352c312904 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 4 Apr 2019 15:27:50 +0100 Subject: [PATCH 47/56] Addressed changes --- docs/MSC1711_certificates_FAQ.md | 14 -------------- synapse/config/tls.py | 28 +++++++++++++++++----------- synapse/crypto/context_factory.py | 29 ++++++++++------------------- 3 files changed, 27 insertions(+), 44 deletions(-) diff --git a/docs/MSC1711_certificates_FAQ.md b/docs/MSC1711_certificates_FAQ.md index 8549f126a449..ebfb20f5c86c 100644 --- a/docs/MSC1711_certificates_FAQ.md +++ b/docs/MSC1711_certificates_FAQ.md @@ -177,20 +177,6 @@ You can do this with a `.well-known` file as follows: on `customer.example.net:8000` it correctly handles HTTP requests with Host header set to `customer.example.net:8000`. -## Specifying your own Certificate Authorities - -If you would like to specify your own list of trusted Certificate -Authorities, you can do so with the following option. **Note that this list -will replace any certificates provided by your operating environment.** - -``` -federation_custom_ca_list: - - myCA1.pem - - myCA2.pem -``` - -Certificate files must be provided in PEM format. - ## FAQ ### Synapse 0.99.0 has just been released, what do I need to do right now? diff --git a/synapse/config/tls.py b/synapse/config/tls.py index d157e310e4b2..3fc6cf9a3f3a 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -90,18 +90,19 @@ def read_config(self, config): self.federation_certificate_verification_whitelist[domain] = True # List of custom certificate authorities for federation traffic validation - self.federation_custom_ca_list = config.get( + custom_ca_list = config.get( "federation_custom_ca_list", None, ) # Read in and parse custom CA certificates - if self.federation_custom_ca_list is not None: - if self.federation_custom_ca_list: + self.federation_ca_trust_root = None + if custom_ca_list is not None: + if len(custom_ca_list) == 0: raise ConfigError("federation_custom_ca_list specified without " "any certificate files") certs = [] - for ca_file in self.federation_custom_ca_list: + for ca_file in custom_ca_list: logger.debug("Reading custom CA certificate file: %s", ca_file) content = self.read_file(ca_file) @@ -113,7 +114,7 @@ def read_config(self, config): raise ConfigError("Error parsing custom CA certificate file %s: %s" % (ca_file, e)) - self.federation_custom_ca_list = trustRootFromCertificates(certs) + self.federation_ca_trust_root = trustRootFromCertificates(certs) # This config option applies to non-federation HTTP clients # (e.g. for talking to recaptcha, identity servers, and such) @@ -144,13 +145,15 @@ def is_disk_cert_valid(self, allow_self_signed=True): try: with open(self.tls_certificate_file, 'rb') as f: cert_pem = f.read() - except Exception: - logger.fatal("Failed to read existing certificate off disk") + except Exception as e: + raise ConfigError("Failed to read existing certificate file %s: %s" + % (self.tls_certificate_file, e)) try: tls_certificate = crypto.load_certificate(crypto.FILETYPE_PEM, cert_pem) - except Exception: - logger.fatal("Failed to parse existing certificate off disk") + except Exception as e: + raise ConfigError("Failed to parse existing certificate file %s: %s" + % (self.tls_certificate_file, e)) if not allow_self_signed: if tls_certificate.get_subject() == tls_certificate.get_issuer(): @@ -245,8 +248,8 @@ def default_config(self, config_dir_path, server_name, **kwargs): # Skip federation certificate verification on the following whitelist # of domains. # - # Note that this should only be used within the context of private - # federation as it will otherwise break things. + # This setting should only normally be used within a private network of + # homeservers. # # Only effective if federation_verify_certicates is `true`. # @@ -257,6 +260,9 @@ def default_config(self, config_dir_path, server_name, **kwargs): # List of custom certificate authorities for federation traffic. # + # This setting should only normally be used within a private network of + # homeservers. + # # Note that this list will replace those that are provided by your # operating environment. Certificates must be in PEM format. # diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 1ee87cdd13c5..8a3fea043bc6 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -18,10 +18,7 @@ from zope.interface import implementer from OpenSSL import SSL, crypto -from twisted.internet._sslverify import ( - ClientTLSOptions as ClientTLSOptionsVerify, - _defaultCurveName, -) +from twisted.internet._sslverify import ClientTLSOptions, _defaultCurveName from twisted.internet.abstract import isIPAddress, isIPv6Address from twisted.internet.interfaces import IOpenSSLClientConnectionCreator from twisted.internet.ssl import CertificateOptions, ContextFactory, platformTrust @@ -93,7 +90,7 @@ def infoCallback(connection, where, ret): @implementer(IOpenSSLClientConnectionCreator) -class ClientTLSOptions(object): +class ClientTLSOptionsNoVerify(object): """ Client creator for TLS without certificate identity verification. This is a copy of twisted.internet._sslverify.ClientTLSOptions with the identity @@ -134,18 +131,12 @@ def __init__(self, config): self._options_noverify = CertificateOptions() # Check if we're using a custom list of a CA certificates - if config.federation_custom_ca_list is not None: - self._options_verify = CertificateOptions( - # Use custom CA trusted root certs - trustRoot=config.federation_custom_ca_list, - ) - return - - # If not, verify using those provided by the operating environment - self._options_verify = CertificateOptions( + trust_root = config.federation_ca_trust_root + if trust_root is None: # Use CA root certs provided by OpenSSL - trustRoot=platformTrust(), - ) + trust_root = platformTrust() + + self._options_verify = CertificateOptions(trustRoot=trust_root) def get_options(self, host): # Use _makeContext so that we get a fresh OpenSSL CTX each time. @@ -155,9 +146,9 @@ def get_options(self, host): # and if the host is whitelisted against it if (self._config.federation_certificate_verification_whitelist and host in self._config.federation_certificate_verification_whitelist): - return ClientTLSOptions(host, self._options_noverify._makeContext()) + return ClientTLSOptionsNoVerify(host, self._options_noverify._makeContext()) - return ClientTLSOptionsVerify(host, self._options_verify._makeContext()) + return ClientTLSOptions(host, self._options_verify._makeContext()) # Otherwise don't require verification - return ClientTLSOptions(host, self._options_noverify._makeContext()) + return ClientTLSOptionsNoVerify(host, self._options_noverify._makeContext()) From 433db40f6e2d0bfcda20bfc9b59497ee57e948a8 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 4 Apr 2019 15:47:12 +0100 Subject: [PATCH 48/56] Address changes --- synapse/config/tls.py | 5 +---- synapse/crypto/context_factory.py | 13 ++++++------- tests/utils.py | 3 --- 3 files changed, 7 insertions(+), 14 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 3fc6cf9a3f3a..162099dc5e7e 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -81,11 +81,8 @@ def read_config(self, config): "federation_certificate_verification_whitelist", [], ) - self.federation_certificate_verification_whitelist = None - if len(federation_certificate_verification_whitelist) > 0: - self.federation_certificate_verification_whitelist = {} - # Store whitelisted domains in a hash for fast lookup + self.federation_certificate_verification_whitelist = {} for domain in federation_certificate_verification_whitelist: self.federation_certificate_verification_whitelist[domain] = True diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 8a3fea043bc6..6fda5e677d4c 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -142,13 +142,12 @@ def get_options(self, host): # Use _makeContext so that we get a fresh OpenSSL CTX each time. # Check if certificate verification has been enabled - if (self._config.federation_verify_certificates): - # and if the host is whitelisted against it - if (self._config.federation_certificate_verification_whitelist and - host in self._config.federation_certificate_verification_whitelist): - return ClientTLSOptionsNoVerify(host, self._options_noverify._makeContext()) + should_verify = self._config.federation_verify_certificates - return ClientTLSOptions(host, self._options_verify._makeContext()) + # Check if we've disabled certificate verification for this host + if should_verify and host in self._config.federation_certificate_verification_whitelist: + should_verify = False - # Otherwise don't require verification + if should_verify: + return ClientTLSOptions(host, self._options_verify._makeContext()) return ClientTLSOptionsNoVerify(host, self._options_noverify._makeContext()) diff --git a/tests/utils.py b/tests/utils.py index 054a48bb246a..cb75514851ea 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -137,9 +137,6 @@ def default_config(name): config.email_enable_notifs = False config.block_non_admin_invites = False config.federation_domain_whitelist = None - config.federation_certificate_verification_whitelist = None - config.federation_custom_ca_list = None - config.federation_verify_certificates = False config.federation_rc_reject_limit = 10 config.federation_rc_sleep_limit = 10 config.federation_rc_sleep_delay = 100 From 396eb64d9202c0a8bb3f9169a987f639670b9f88 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 4 Apr 2019 15:55:02 +0100 Subject: [PATCH 49/56] Change federation whitelist stuff back --- synapse/config/server.py | 2 +- synapse/federation/transport/server.py | 6 ++++-- synapse/http/matrixfederationclient.py | 6 ++++-- synapse/rest/key/v2/remote_key_resource.py | 6 ++++-- synapse/rest/media/v1/media_repository.py | 12 ++++++++---- 5 files changed, 21 insertions(+), 11 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index f55a71d508ef..13627fc8bf79 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -112,7 +112,7 @@ def read_config(self, config): # FIXME: federation_domain_whitelist needs sytests federation_domain_whitelist = config.get( - "federation_domain_whitelist", [], + "federation_domain_whitelist", None, ) self.federation_domain_whitelist = None diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index 1ddfbbd7f4f7..452599e1a15e 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -127,8 +127,10 @@ def authenticate_request(self, request, content): json_request["origin"] = origin json_request["signatures"].setdefault(origin, {})[key] = sig - if (self.federation_domain_whitelist is not None and - origin not in self.federation_domain_whitelist): + if ( + self.federation_domain_whitelist is not None and + origin not in self.federation_domain_whitelist + ): raise FederationDeniedError(origin) if not json_request["signatures"]: diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index b834d062196a..ff63d0b2a838 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -283,8 +283,10 @@ def _send_request( else: _sec_timeout = self.default_timeout - if (self.hs.config.federation_domain_whitelist is not None and - request.destination not in self.hs.config.federation_domain_whitelist): + if ( + self.hs.config.federation_domain_whitelist is not None and + request.destination not in self.hs.config.federation_domain_whitelist + ): raise FederationDeniedError(request.destination) limiter = yield synapse.util.retryutils.get_retry_limiter( diff --git a/synapse/rest/key/v2/remote_key_resource.py b/synapse/rest/key/v2/remote_key_resource.py index 5a14cfd426b1..eb8782aa6e1a 100644 --- a/synapse/rest/key/v2/remote_key_resource.py +++ b/synapse/rest/key/v2/remote_key_resource.py @@ -139,8 +139,10 @@ def query_keys(self, request, query, query_remote_on_cache_miss=False): store_queries = [] for server_name, key_ids in query.items(): - if (self.federation_domain_whitelist is not None and - server_name not in self.federation_domain_whitelist): + if ( + self.federation_domain_whitelist is not None and + server_name not in self.federation_domain_whitelist + ): logger.debug("Federation denied with %s", server_name) continue diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index 40b10e68c069..bdffa9780558 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -231,8 +231,10 @@ def get_remote_media(self, request, server_name, media_id, name): Deferred: Resolves once a response has successfully been written to request """ - if (self.federation_domain_whitelist is not None and - server_name not in self.federation_domain_whitelist): + if ( + self.federation_domain_whitelist is not None and + server_name not in self.federation_domain_whitelist + ): raise FederationDeniedError(server_name) self.mark_recently_accessed(server_name, media_id) @@ -269,8 +271,10 @@ def get_remote_media_info(self, server_name, media_id): Returns: Deferred[dict]: The media_info of the file """ - if (self.federation_domain_whitelist is not None and - server_name not in self.federation_domain_whitelist): + if ( + self.federation_domain_whitelist is not None and + server_name not in self.federation_domain_whitelist + ): raise FederationDeniedError(server_name) # We linearize here to ensure that we don't try and download remote From 3dbad0603f269e1324a5de10fc7f10c67ff46047 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 4 Apr 2019 15:56:13 +0100 Subject: [PATCH 50/56] lint --- synapse/crypto/context_factory.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 6fda5e677d4c..6ebbd3b73cf0 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -145,7 +145,10 @@ def get_options(self, host): should_verify = self._config.federation_verify_certificates # Check if we've disabled certificate verification for this host - if should_verify and host in self._config.federation_certificate_verification_whitelist: + if ( + should_verify and + host in self._config.federation_certificate_verification_whitelist + ): should_verify = False if should_verify: From 595d6ecd863c7b929000f4497c3ebb36a7c90143 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 4 Apr 2019 16:01:04 +0100 Subject: [PATCH 51/56] fix domain_whitelist --- synapse/config/server.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 13627fc8bf79..09f05173d90c 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -116,12 +116,12 @@ def read_config(self, config): ) self.federation_domain_whitelist = None - if len(federation_domain_whitelist) > 0: + if federation_domain_whitelist is not None: + # turn the whitelist into a hash for speed of lookup self.federation_domain_whitelist = {} - # turn the whitelist into a hash for speed of lookup - for domain in federation_domain_whitelist: - self.federation_domain_whitelist[domain] = True + for domain in federation_domain_whitelist: + self.federation_domain_whitelist[domain] = True if self.public_baseurl is not None: if self.public_baseurl[-1] != '/': From edf2dd4744b3d3980a7e369425f0ad8d822d426c Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 4 Apr 2019 16:05:04 +0100 Subject: [PATCH 52/56] sample confiiiiiiiiiiiiiiiiiiiiiiig --- docs/sample_config.yaml | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index be5ae548ece6..e1d4ca2eff51 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -267,8 +267,8 @@ listeners: # Skip federation certificate verification on the following whitelist # of domains. # -# Note that this should only be used within the context of private -# federation as it will otherwise break things. +# This setting should only normally be used within a private network of +# homeservers. # # Only effective if federation_verify_certicates is `true`. # @@ -279,6 +279,9 @@ listeners: # List of custom certificate authorities for federation traffic. # +# This setting should only normally be used within a private network of +# homeservers. +# # Note that this list will replace those that are provided by your # operating environment. Certificates must be in PEM format. # From 93850f0ac8269a503178952983d52639494187e6 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 4 Apr 2019 16:20:56 +0100 Subject: [PATCH 53/56] domain globbing --- docs/sample_config.yaml | 4 ++-- synapse/config/tls.py | 17 ++++++++++------- synapse/crypto/context_factory.py | 10 +++++----- 3 files changed, 17 insertions(+), 14 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index e1d4ca2eff51..9c74dcd18620 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -274,8 +274,8 @@ listeners: # #federation_certificate_verification_whitelist: # - lon.example.com -# - nyc.example.com -# - syd.example.com +# - *.domain.com +# - *.onion # List of custom certificate authorities for federation traffic. # diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 162099dc5e7e..6f76cf80b081 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -27,6 +27,7 @@ from twisted.internet._sslverify import Certificate, trustRootFromCertificates from synapse.config._base import Config, ConfigError +from synapse.util import glob_to_regex logger = logging.getLogger(__name__) @@ -77,14 +78,16 @@ def read_config(self, config): ) # Whitelist of domains to not verify certificates for - federation_certificate_verification_whitelist = config.get( + fed_whitelist_entries = config.get( "federation_certificate_verification_whitelist", [], ) - # Store whitelisted domains in a hash for fast lookup - self.federation_certificate_verification_whitelist = {} - for domain in federation_certificate_verification_whitelist: - self.federation_certificate_verification_whitelist[domain] = True + # Support globs (*) in whitelist values + self.federation_certificate_verification_whitelist = [] + for entry in fed_whitelist_entries: + # Convert globs to regex + entry_regex = glob_to_regex(entry) + self.federation_certificate_verification_whitelist.append(entry_regex) # List of custom certificate authorities for federation traffic validation custom_ca_list = config.get( @@ -252,8 +255,8 @@ def default_config(self, config_dir_path, server_name, **kwargs): # #federation_certificate_verification_whitelist: # - lon.example.com - # - nyc.example.com - # - syd.example.com + # - *.domain.com + # - *.onion # List of custom certificate authorities for federation traffic. # diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 6ebbd3b73cf0..59ea087e66d2 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -145,11 +145,11 @@ def get_options(self, host): should_verify = self._config.federation_verify_certificates # Check if we've disabled certificate verification for this host - if ( - should_verify and - host in self._config.federation_certificate_verification_whitelist - ): - should_verify = False + if should_verify: + for regex in self._config.federation_certificate_verification_whitelist: + if regex.match(host): + should_verify = False + break if should_verify: return ClientTLSOptions(host, self._options_verify._makeContext()) From 669199876de27c5667a6eecba035b8a76fc2b0e6 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 8 Apr 2019 13:39:07 +0100 Subject: [PATCH 54/56] Add comment and simplify diff --- synapse/config/server.py | 2 +- synapse/config/tls.py | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 09f05173d90c..654d410fbf13 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -111,11 +111,11 @@ def read_config(self, config): self.admin_contact = config.get("admin_contact", None) # FIXME: federation_domain_whitelist needs sytests + self.federation_domain_whitelist = None federation_domain_whitelist = config.get( "federation_domain_whitelist", None, ) - self.federation_domain_whitelist = None if federation_domain_whitelist is not None: # turn the whitelist into a hash for speed of lookup self.federation_domain_whitelist = {} diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 6f76cf80b081..ea54bd079337 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -98,6 +98,9 @@ def read_config(self, config): self.federation_ca_trust_root = None if custom_ca_list is not None: if len(custom_ca_list) == 0: + # A trustroot cannot be generated without any CA certificates. + # Raise an error if this option has been specified without any + # corresponding certificates. raise ConfigError("federation_custom_ca_list specified without " "any certificate files") From 92cc6b0383482200785f09d6e1531563eb50bb90 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 8 Apr 2019 17:58:54 +0100 Subject: [PATCH 55/56] Heavier warning about disabling TLS verification --- synapse/config/tls.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index ea54bd079337..72dd5926f9f4 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -251,8 +251,9 @@ def default_config(self, config_dir_path, server_name, **kwargs): # Skip federation certificate verification on the following whitelist # of domains. # - # This setting should only normally be used within a private network of - # homeservers. + # This setting should only be used in very specific cases, such as + # federation over Tor hidden services and similar. For private networks + # of homeservers, you likely want to use a private CA instead. # # Only effective if federation_verify_certicates is `true`. # From bc4b14849f03dc3392dcf1cba01133212b6630df Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 8 Apr 2019 18:26:15 +0100 Subject: [PATCH 56/56] regen sample config --- docs/sample_config.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 9c74dcd18620..58f2fa08103c 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -267,8 +267,9 @@ listeners: # Skip federation certificate verification on the following whitelist # of domains. # -# This setting should only normally be used within a private network of -# homeservers. +# This setting should only be used in very specific cases, such as +# federation over Tor hidden services and similar. For private networks +# of homeservers, you likely want to use a private CA instead. # # Only effective if federation_verify_certicates is `true`. #