From 88d2566b0c8231f75a1d8dacc646fa625aa401c0 Mon Sep 17 00:00:00 2001 From: Kento Okamoto Date: Fri, 4 Jun 2021 01:07:59 -0700 Subject: [PATCH 1/6] Add warnings to ip_range_blacklist usage with proxies --- docs/sample_config.yaml | 4 ++++ synapse/config/repository.py | 24 +++++++++++++++++++----- synapse/config/server.py | 2 ++ 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 2952f2ba3201..a7a52561d71c 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -193,6 +193,8 @@ presence: # # This option replaces federation_ip_range_blacklist in Synapse v1.25.0. # +# Note: The value is ignored when an HTTP proxy is in use +# #ip_range_blacklist: # - '127.0.0.0/8' # - '10.0.0.0/8' @@ -1035,6 +1037,8 @@ media_store_path: "DATADIR/media_store" # This must be specified if url_preview_enabled is set. It is recommended that # you uncomment the following list as a starting point. # +# Note: The value is ignored when an HTTP proxy is in use +# #url_preview_ip_range_blacklist: # - '127.0.0.0/8' # - '10.0.0.0/8' diff --git a/synapse/config/repository.py b/synapse/config/repository.py index c78a83abe16e..fa1f9ec4e454 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import logging import os from collections import namedtuple from typing import Dict, List @@ -22,6 +23,8 @@ from ._base import Config, ConfigError +logger = logging.Logger(__name__) + DEFAULT_THUMBNAIL_SIZES = [ {"width": 32, "height": 32, "method": "crop"}, {"width": 96, "height": 96, "method": "crop"}, @@ -36,6 +39,9 @@ # method: %(method)s """ +HTTP_PROXY_SET_WARNING = """\ +The Synapse config url_preview_ip_range_blacklist will be ignored as %s set.""" + ThumbnailRequirement = namedtuple( "ThumbnailRequirement", ["width", "height", "method", "media_type"] ) @@ -181,11 +187,17 @@ def read_config(self, config, **kwargs): ) if "url_preview_ip_range_blacklist" not in config: - raise ConfigError( - "For security, you must specify an explicit target IP address " - "blacklist in url_preview_ip_range_blacklist for url previewing " - "to work" - ) + if "HTTP_PROXY" not in os.environ and "HTTPS_PROXY" not in os.environ: + raise ConfigError( + "For security, you must specify an explicit target IP address " + "blacklist in url_preview_ip_range_blacklist for url previewing " + "to work" + ) + else: + proxy_var = ( + "HTTP_PROXY" if "HTTP_PROXY" in os.environ else "HTTPS_PROXY" + ) + logger.warning("".join(HTTP_PROXY_SET_WARNING % proxy_var)) # we always blacklist '0.0.0.0' and '::', which are supposed to be # unroutable addresses. @@ -288,6 +300,8 @@ def generate_config_section(self, data_dir_path, **kwargs): # This must be specified if url_preview_enabled is set. It is recommended that # you uncomment the following list as a starting point. # + # Note: The value is ignored when an HTTP proxy is in use + # #url_preview_ip_range_blacklist: %(ip_range_blacklist)s diff --git a/synapse/config/server.py b/synapse/config/server.py index c290a35a9285..ff5785027d82 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -959,6 +959,8 @@ def generate_config_section( # # This option replaces federation_ip_range_blacklist in Synapse v1.25.0. # + # Note: The value is ignored when an HTTP proxy is in use + # #ip_range_blacklist: %(ip_range_blacklist)s From 9117e731b81d13b13781fd4d504fec3963a0a1cd Mon Sep 17 00:00:00 2001 From: Kento Okamoto Date: Fri, 4 Jun 2021 17:26:29 -0700 Subject: [PATCH 2/6] Adding changelog 10129.bugfix file --- changelog.d/10129.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10129.bugfix diff --git a/changelog.d/10129.bugfix b/changelog.d/10129.bugfix new file mode 100644 index 000000000000..6c106bbde877 --- /dev/null +++ b/changelog.d/10129.bugfix @@ -0,0 +1 @@ +Fixed the interaction between `*ip_range_blacklist` setting and outbound HTTP proxies to reduce confusion. From 87f862f536bbe197390517f284cd78eedc73ba2b Mon Sep 17 00:00:00 2001 From: Kento Okamoto Date: Mon, 21 Jun 2021 21:13:02 -0700 Subject: [PATCH 3/6] Corrected blacklist warning logic and warning message --- changelog.d/10129.bugfix | 2 +- synapse/config/repository.py | 13 ++++++------- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/changelog.d/10129.bugfix b/changelog.d/10129.bugfix index 6c106bbde877..9a410b2fdad7 100644 --- a/changelog.d/10129.bugfix +++ b/changelog.d/10129.bugfix @@ -1 +1 @@ -Fixed the interaction between `*ip_range_blacklist` setting and outbound HTTP proxies to reduce confusion. +Add some clarification to the sample config file. Contributed by @Kentokamoto diff --git a/synapse/config/repository.py b/synapse/config/repository.py index fa1f9ec4e454..8662717733b6 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -40,7 +40,7 @@ """ HTTP_PROXY_SET_WARNING = """\ -The Synapse config url_preview_ip_range_blacklist will be ignored as %s set.""" +The Synapse config url_preview_ip_range_blacklist will be ignored as an HTTP(s) proxy is configured.""" ThumbnailRequirement = namedtuple( "ThumbnailRequirement", ["width", "height", "method", "media_type"] @@ -186,18 +186,17 @@ def read_config(self, config, **kwargs): e.message # noqa: B306, DependencyException.message is a property ) + proxies = ["HTTP_PROXY", "HTTPS_PROXY"] if "url_preview_ip_range_blacklist" not in config: - if "HTTP_PROXY" not in os.environ and "HTTPS_PROXY" not in os.environ: + if not any(proxy in os.environ for proxy in proxies): raise ConfigError( "For security, you must specify an explicit target IP address " "blacklist in url_preview_ip_range_blacklist for url previewing " "to work" ) - else: - proxy_var = ( - "HTTP_PROXY" if "HTTP_PROXY" in os.environ else "HTTPS_PROXY" - ) - logger.warning("".join(HTTP_PROXY_SET_WARNING % proxy_var)) + else: + if any(proxy in os.environ for proxy in proxies): + logger.warning("".join(HTTP_PROXY_SET_WARNING)) # we always blacklist '0.0.0.0' and '::', which are supposed to be # unroutable addresses. From b273c2498728586002c9c7fc80477efceaf70e51 Mon Sep 17 00:00:00 2001 From: Kento Okamoto Date: Tue, 22 Jun 2021 09:05:29 -0700 Subject: [PATCH 4/6] Update changelog.d/10129.bugfix Co-authored-by: Erik Johnston --- changelog.d/10129.bugfix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/10129.bugfix b/changelog.d/10129.bugfix index 9a410b2fdad7..292676ec8d6f 100644 --- a/changelog.d/10129.bugfix +++ b/changelog.d/10129.bugfix @@ -1 +1 @@ -Add some clarification to the sample config file. Contributed by @Kentokamoto +Add some clarification to the sample config file. Contributed by @Kentokamoto. From 3de1c3050a9637c787a4e1868374c95e7024fdce Mon Sep 17 00:00:00 2001 From: Kento Okamoto Date: Wed, 14 Jul 2021 09:43:28 -0700 Subject: [PATCH 5/6] Utilize urllib's proxy function instead of checking env variables --- synapse/config/repository.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/synapse/config/repository.py b/synapse/config/repository.py index 8662717733b6..05c040e45d88 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -16,6 +16,7 @@ import os from collections import namedtuple from typing import Dict, List +from urllib.request import getproxies_environment from synapse.config.server import DEFAULT_IP_RANGE_BLACKLIST, generate_ip_set from synapse.python_dependencies import DependencyException, check_requirements @@ -23,7 +24,7 @@ from ._base import Config, ConfigError -logger = logging.Logger(__name__) +logger = logging.getLogger(__name__) DEFAULT_THUMBNAIL_SIZES = [ {"width": 32, "height": 32, "method": "crop"}, @@ -186,16 +187,16 @@ def read_config(self, config, **kwargs): e.message # noqa: B306, DependencyException.message is a property ) - proxies = ["HTTP_PROXY", "HTTPS_PROXY"] + proxy_env = getproxies_environment() if "url_preview_ip_range_blacklist" not in config: - if not any(proxy in os.environ for proxy in proxies): + if "http" not in proxy_env or "https" not in proxy_env: raise ConfigError( "For security, you must specify an explicit target IP address " "blacklist in url_preview_ip_range_blacklist for url previewing " "to work" ) else: - if any(proxy in os.environ for proxy in proxies): + if "http" in proxy_env or "https" in proxy_env: logger.warning("".join(HTTP_PROXY_SET_WARNING)) # we always blacklist '0.0.0.0' and '::', which are supposed to be From d05946adf3cb89b987853cb9d6d3547859c9589f Mon Sep 17 00:00:00 2001 From: Kento Okamoto Date: Sun, 18 Jul 2021 16:59:51 -0700 Subject: [PATCH 6/6] ignoring getproxies_environment mypy error --- synapse/config/repository.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/repository.py b/synapse/config/repository.py index 05c040e45d88..8947ba6833a1 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -16,7 +16,7 @@ import os from collections import namedtuple from typing import Dict, List -from urllib.request import getproxies_environment +from urllib.request import getproxies_environment # type: ignore from synapse.config.server import DEFAULT_IP_RANGE_BLACKLIST, generate_ip_set from synapse.python_dependencies import DependencyException, check_requirements