Skip to content

Commit

Permalink
Fix allowlist deserialization in file sources
Browse files Browse the repository at this point in the history
  • Loading branch information
mvdbeek committed Sep 22, 2023
1 parent 54f3769 commit cac5495
Show file tree
Hide file tree
Showing 8 changed files with 38 additions and 24 deletions.
1 change: 1 addition & 0 deletions lib/galaxy/celery/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)


Expand Down
10 changes: 2 additions & 8 deletions lib/galaxy/config/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
# absolute_import needed for tool_shed package.

import configparser
import ipaddress
import json
import locale
import logging
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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.
Expand Down
21 changes: 21 additions & 0 deletions lib/galaxy/config/parsers.py
Original file line number Diff line number Diff line change
@@ -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
]
15 changes: 8 additions & 7 deletions lib/galaxy/files/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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):
Expand All @@ -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"],
Expand Down
9 changes: 2 additions & 7 deletions lib/galaxy/files/uris.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion lib/galaxy_test/api/test_drs.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

import requests

from galaxy.config.parsers import parse_allowlist_ips
from galaxy.files import (
ConfiguredFileSources,
ConfiguredFileSourcesConfig,
Expand All @@ -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={
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy_test/driver/driver_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 1 addition & 0 deletions packages/files/setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ version = 23.1.dev0
include_package_data = True
install_requires =
galaxy-util
galaxy-config
fs
typing-extensions
packages = find:
Expand Down

0 comments on commit cac5495

Please sign in to comment.