From cac5495b5d3732f7b726bce1b780ffe970aab3ae Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 22 Sep 2023 19:20:15 +0200 Subject: [PATCH] Fix allowlist deserialization in file sources --- lib/galaxy/celery/__init__.py | 1 + lib/galaxy/config/__init__.py | 10 ++-------- lib/galaxy/config/parsers.py | 21 +++++++++++++++++++++ lib/galaxy/files/__init__.py | 15 ++++++++------- lib/galaxy/files/uris.py | 9 ++------- lib/galaxy_test/api/test_drs.py | 3 ++- lib/galaxy_test/driver/driver_util.py | 2 +- packages/files/setup.cfg | 1 + 8 files changed, 38 insertions(+), 24 deletions(-) create mode 100644 lib/galaxy/config/parsers.py diff --git a/lib/galaxy/celery/__init__.py b/lib/galaxy/celery/__init__.py index 842418458a30..49120a4fdcec 100644 --- a/lib/galaxy/celery/__init__.py +++ b/lib/galaxy/celery/__init__.py @@ -115,6 +115,7 @@ def get_app_properties(): def get_config(): kwargs = get_app_properties() or {} kwargs["override_tempdir"] = False + kwargs["fetch_url_allowlist"] = ["127.0.0.0/24"] return Configuration(**kwargs) diff --git a/lib/galaxy/config/__init__.py b/lib/galaxy/config/__init__.py index 9bc573842d66..574676f264bc 100644 --- a/lib/galaxy/config/__init__.py +++ b/lib/galaxy/config/__init__.py @@ -4,7 +4,6 @@ # absolute_import needed for tool_shed package. import configparser -import ipaddress import json import locale import logging @@ -34,6 +33,7 @@ import yaml +from galaxy.config.parsers import parse_allowlist_ips from galaxy.config.schema import AppSchema from galaxy.exceptions import ConfigurationError from galaxy.util import ( @@ -906,13 +906,7 @@ def _process_config(self, kwargs: Dict[str, Any]) -> None: self.tool_secret = kwargs.get("tool_secret", "") self.metadata_strategy = kwargs.get("metadata_strategy", "directory") self.use_remote_user = self.use_remote_user or self.single_user - self.fetch_url_allowlist_ips = [ - ipaddress.ip_network(unicodify(ip.strip())) # If it has a slash, assume 127.0.0.1/24 notation - if "/" in ip - else ipaddress.ip_address(unicodify(ip.strip())) # Otherwise interpret it as an ip address. - for ip in kwargs.get("fetch_url_allowlist", "").split(",") - if len(ip.strip()) > 0 - ] + self.fetch_url_allowlist_ips = parse_allowlist_ips(listify(kwargs.get("fetch_url_allowlist"))) self.job_queue_cleanup_interval = int(kwargs.get("job_queue_cleanup_interval", "5")) # Fall back to legacy job_working_directory config variable if set. diff --git a/lib/galaxy/config/parsers.py b/lib/galaxy/config/parsers.py new file mode 100644 index 000000000000..00ef641f28c2 --- /dev/null +++ b/lib/galaxy/config/parsers.py @@ -0,0 +1,21 @@ +import ipaddress +from typing import ( + List, + Union, +) + +from galaxy.util import unicodify + +IpAddressT = Union[ipaddress.IPv4Address, ipaddress.IPv6Address] +IpNetworkT = Union[ipaddress.IPv4Network, ipaddress.IPv6Network] +IpAllowedListEntryT = Union[IpAddressT, IpNetworkT] + + +def parse_allowlist_ips(fetch_url_allowlist: List[str]) -> List[IpAllowedListEntryT]: + return [ + ipaddress.ip_network(unicodify(ip.strip())) # If it has a slash, assume 127.0.0.1/24 notation + if "/" in ip + else ipaddress.ip_address(unicodify(ip.strip())) # Otherwise interpret it as an ip address. + for ip in fetch_url_allowlist + if len(ip.strip()) > 0 + ] diff --git a/lib/galaxy/files/__init__.py b/lib/galaxy/files/__init__.py index a16843d4a3fe..736358866e98 100644 --- a/lib/galaxy/files/__init__.py +++ b/lib/galaxy/files/__init__.py @@ -13,6 +13,7 @@ ) from galaxy import exceptions +from galaxy.config.parsers import parse_allowlist_ips from galaxy.util import plugin_config from galaxy.util.dictifiable import Dictifiable @@ -235,12 +236,12 @@ def from_app_config(config): # Formalize what we read in from config to create a more clear interface # for this component. kwds = {} - kwds["symlink_allowlist"] = getattr(config, "user_library_import_symlink_allowlist", []) - kwds["fetch_url_allowlist"] = getattr(config, "fetch_url_allowlist", []) - kwds["library_import_dir"] = getattr(config, "library_import_dir", None) - kwds["user_library_import_dir"] = getattr(config, "user_library_import_dir", None) - kwds["ftp_upload_dir"] = getattr(config, "ftp_upload_dir", None) - kwds["ftp_upload_purge"] = getattr(config, "ftp_upload_purge", True) + kwds["symlink_allowlist"] = config.user_library_import_symlink_allowlist + kwds["fetch_url_allowlist"] = [str(ip) for ip in config.fetch_url_allowlist_ips] + kwds["library_import_dir"] = config.library_import_dir + kwds["user_library_import_dir"] = config.user_library_import_dir + kwds["ftp_upload_dir"] = config.ftp_upload_dir + kwds["ftp_upload_purge"] = config.ftp_upload_purge return ConfiguredFileSourcesConfig(**kwds) def to_dict(self): @@ -257,7 +258,7 @@ def to_dict(self): def from_dict(as_dict): return ConfiguredFileSourcesConfig( symlink_allowlist=as_dict["symlink_allowlist"], - fetch_url_allowlist=as_dict["fetch_url_allowlist"], + fetch_url_allowlist=parse_allowlist_ips(as_dict["fetch_url_allowlist"]), library_import_dir=as_dict["library_import_dir"], user_library_import_dir=as_dict["user_library_import_dir"], ftp_upload_dir=as_dict["ftp_upload_dir"], diff --git a/lib/galaxy/files/uris.py b/lib/galaxy/files/uris.py index 4fbfdb41de09..d6240fd4ce1b 100644 --- a/lib/galaxy/files/uris.py +++ b/lib/galaxy/files/uris.py @@ -6,10 +6,10 @@ from typing import ( List, Optional, - Union, ) from urllib.parse import urlparse +from galaxy.config.parsers import IpAllowedListEntryT from galaxy.exceptions import ( AdminRequiredException, ConfigDoesNotAllowException, @@ -66,11 +66,6 @@ def stream_to_file(stream, suffix="", prefix="", dir=None, text=False, **kwd): return stream_to_open_named_file(stream, fd, temp_name, **kwd) -IpAddressT = Union[ipaddress.IPv4Address, ipaddress.IPv6Address] -IpNetworkT = Union[ipaddress.IPv4Network, ipaddress.IPv6Network] -IpAllowedListEntryT = Union[IpAddressT, IpNetworkT] - - def validate_uri_access(uri: str, is_admin: bool, ip_allowlist: List[IpAllowedListEntryT]) -> None: """Perform uniform checks on supplied URIs. @@ -128,7 +123,7 @@ def validate_non_local(uri: str, ip_allowlist: List[IpAllowedListEntryT]) -> str parsed_url = parsed_url[:idx] # safe to log out, no credentials/request path, just an IP + port - log.debug("parsed url, port: %s : %s", parsed_url, port) + log.debug("parsed url %s, port: %s", parsed_url, port) # Call getaddrinfo to resolve hostname into tuples containing IPs. addrinfo = socket.getaddrinfo(parsed_url, port) # Get the IP addresses that this entry resolves to (uniquely) diff --git a/lib/galaxy_test/api/test_drs.py b/lib/galaxy_test/api/test_drs.py index c221e486a03c..0fa0e1015729 100644 --- a/lib/galaxy_test/api/test_drs.py +++ b/lib/galaxy_test/api/test_drs.py @@ -11,6 +11,7 @@ import requests +from galaxy.config.parsers import parse_allowlist_ips from galaxy.files import ( ConfiguredFileSources, ConfiguredFileSourcesConfig, @@ -32,7 +33,7 @@ def user_context_fixture(): - file_sources_config = ConfiguredFileSourcesConfig() + file_sources_config = ConfiguredFileSourcesConfig(fetch_url_allowlist=parse_allowlist_ips(["127.0.0.0/24"])) file_sources = ConfiguredFileSources(file_sources_config, load_stock_plugins=True) user_context = DictFileSourcesUserContext( preferences={ diff --git a/lib/galaxy_test/driver/driver_util.py b/lib/galaxy_test/driver/driver_util.py index c745f93f13ee..7a2c47368ec1 100644 --- a/lib/galaxy_test/driver/driver_util.py +++ b/lib/galaxy_test/driver/driver_util.py @@ -237,7 +237,7 @@ def setup_galaxy_config( logging=LOGGING_CONFIG_DEFAULT, monitor_thread_join_timeout=5, object_store_store_by="uuid", - fetch_url_allowlist="127.0.0.1", + fetch_url_allowlist=["127.0.0.0/24"], job_handler_monitor_sleep=0.2, job_runner_monitor_sleep=0.2, workflow_monitor_sleep=0.2, diff --git a/packages/files/setup.cfg b/packages/files/setup.cfg index e379c2d99611..895207d5fbdd 100644 --- a/packages/files/setup.cfg +++ b/packages/files/setup.cfg @@ -33,6 +33,7 @@ version = 23.1.dev0 include_package_data = True install_requires = galaxy-util + galaxy-config fs typing-extensions packages = find: