diff --git a/README.md b/README.md index 0aca2d9..28b873e 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,7 @@ Certain values can be set via environment variables, using the `-e` parameter on * __YTDL_OPTIONS_PRESETS__: Named bundles of yt-dlp options, selectable per download in the UI. See [Configuring yt-dlp options](#%EF%B8%8F-configuring-yt-dlp-options) for format and examples. * __YTDL_OPTIONS_PRESETS_FILE__: Path to a JSON file containing presets. Monitored and reloaded automatically on changes. See [Configuring yt-dlp options](#%EF%B8%8F-configuring-yt-dlp-options). * __ALLOW_YTDL_OPTIONS_OVERRIDES__: Whether to show a free-text field in the UI for per-download yt-dlp option overrides. Defaults to `false`. See [Configuring yt-dlp options](#%EF%B8%8F-configuring-yt-dlp-options) for details and security considerations. -* __ALLOW_PRIVATE_ADDRESSES__: Whether to allow downloads from private, loopback, link-local and other non-global addresses. Defaults to `false`, which protects against SSRF by refusing URLs that resolve to internal hosts. Set to `true` only in trusted environments — for example when routing traffic through a proxy/VPN client in Fake-IP mode (sing-box, Clash, Mihomo), which resolves hosts to the `198.18.0.0/15` range. Enabling this disables the SSRF protection entirely, so only use it when you control the network. You do **not** need this to use a proxy on an internal address: a proxy configured through the `proxy` option in `YTDL_OPTIONS` (or the `*_proxy` environment variables) is always reachable at its own host and port, wherever it lives. +* __ALLOW_PRIVATE_ADDRESSES__: Whether to allow downloads from private, loopback, link-local and other non-global addresses. Defaults to `false`, which protects against SSRF by refusing URLs that resolve to internal hosts. Set to `true` only in trusted environments — for example when routing traffic through a proxy/VPN client in Fake-IP mode (sing-box, Clash, Mihomo), which resolves hosts to the `198.18.0.0/15` range. Enabling this disables the SSRF protection entirely, so only use it when you control the network. You do **not** need this to use a proxy on an internal address: a proxy configured through the `proxy` option in `YTDL_OPTIONS` (or the `*_proxy` environment variables) is always reachable at its own host and port, wherever it lives. Nor do you need it for a proxy that resolves hostnames itself (an HTTP proxy, `socks5`, `socks5h` or `socks4a`): MeTube leaves those lookups to the proxy rather than resolving submitted URLs locally, so none leak and proxy-only hosts still work. * __YTDL_NIGHTLY_UPDATE_TIME__: If set, will cause MeTube to use [nightly yt-dlp builds](https://github.com/yt-dlp/yt-dlp-nightly-builds) instead of the stable releases. Set to the time (`HH:MM`, 24-hour) when you want the daily upgrades and MeTube restart to happen. Defaults to empty (disabled). A filename that would exceed the limit the filesystem accepts is shortened to fit, keeping its extension, with room left for the suffixes yt-dlp adds while downloading. Sites that put a long description in the title would otherwise fail the download outright with `File name too long`. Use `trim_file_name` in `YTDL_OPTIONS` if you want names shorter than the filesystem's own limit, or `restrictfilenames` to strip non-ASCII characters. diff --git a/app/subscriptions.py b/app/subscriptions.py index 5afe878..f4d71b2 100644 --- a/app/subscriptions.py +++ b/app/subscriptions.py @@ -19,7 +19,7 @@ import yt_dlp.networking.impersonate import bg_tasks from dl_formats import merge_ytdl_option_layers from state_store import AtomicJsonStore, read_legacy_shelf -from url_guard import validate_url +from url_guard import validate_url, download_proxies log = logging.getLogger("subscriptions") @@ -116,12 +116,18 @@ def extract_flat_playlist( if media_entries: return info, media_entries if _depth < 1: + proxies = download_proxies({**config.YTDL_OPTIONS, **(extra_opts or {})}) for ent in entries[:5]: nested_url = _entry_video_url(ent) if not nested_url: continue - # nested_url comes from remote playlist content; guard it too. - if validate_url(nested_url, allow_private=getattr(config, "ALLOW_PRIVATE_ADDRESSES", False)) is not None: + # nested_url comes from remote playlist content; guard it too, + # against the same proxy map this scan is using. + if validate_url( + nested_url, + allow_private=getattr(config, "ALLOW_PRIVATE_ADDRESSES", False), + proxies=proxies, + ) is not None: continue nested_info, nested_entries = extract_flat_playlist( config, @@ -621,8 +627,16 @@ class SubscriptionManager: return {"status": "error", "msg": "Missing URL"} # SSRF guard: block non-http(s) schemes and internal/metadata hosts # before yt-dlp fetches the feed. May do a DNS lookup, so run off-loop. + # The scan's own options pick the proxy, so a feed fetched through one + # is not resolved here — see validate_url. + proxies = download_proxies({ + **self.config.YTDL_OPTIONS, + **self._scan_extra_opts(ytdl_options_presets, ytdl_options_overrides), + }) url_error = await asyncio.get_running_loop().run_in_executor( - None, partial(validate_url, url, allow_private=getattr(self.config, "ALLOW_PRIVATE_ADDRESSES", False))) + None, partial(validate_url, url, + allow_private=getattr(self.config, "ALLOW_PRIVATE_ADDRESSES", False), + proxies=proxies)) if url_error is not None: log.warning('Rejected subscription URL "%s": %s', url, url_error) return {"status": "error", "msg": url_error} diff --git a/app/tests/test_download_queue.py b/app/tests/test_download_queue.py index 0e6f0a7..f11c3a6 100644 --- a/app/tests/test_download_queue.py +++ b/app/tests/test_download_queue.py @@ -5,6 +5,7 @@ from __future__ import annotations import copy import os import re +import socket import tempfile from unittest.mock import AsyncMock, MagicMock, patch @@ -27,6 +28,9 @@ def dq_env(): cfg.AUDIO_DOWNLOAD_DIR = dl cfg.TEMP_DIR = dl cfg.MAX_CONCURRENT_DOWNLOADS = "3" + # Explicit: an unset attribute on a MagicMock is truthy, which would + # make validate_url bypass every SSRF check it is asked to run. + cfg.ALLOW_PRIVATE_ADDRESSES = False cfg.YTDL_OPTIONS = {} cfg.YTDL_OPTIONS_PRESETS = {} cfg.CUSTOM_DIRS = True @@ -164,6 +168,43 @@ async def test_add_ssrf_rejected_url_recorded_as_failed_entry(dq_env): notifier.completed.assert_awaited() +@pytest.mark.asyncio +async def test_add_does_not_resolve_hostname_when_proxied(dq_env): + """With a remote-DNS proxy configured, adding a URL must not look its host + up here: that both leaks the hostname to the local resolver and fails closed + when only the proxy can resolve it (issue #1079).""" + dq_env.YTDL_OPTIONS = {"proxy": "socks5h://tor:9050"} + notifier = AsyncMock() + + def fake_extract(self, url, *_args, **_kwargs): + return {"_type": "video", "id": "vid1", "title": "t", "webpage_url": url} + + dq = DownloadQueue(dq_env, notifier) + with patch("url_guard.socket.getaddrinfo", side_effect=AssertionError("resolved")), \ + patch.object(DownloadQueue, "_DownloadQueue__extract_info", fake_extract): + result = await dq.add( + "https://only-the-proxy-can-resolve.invalid/x", + "video", "auto", "any", "best", "", "", 0, auto_start=False, + ) + assert result["status"] == "ok" + dq.close() + + +@pytest.mark.asyncio +async def test_add_resolves_hostname_when_not_proxied(dq_env): + """Without a proxy the address check still runs and still rejects.""" + notifier = AsyncMock() + url = "https://internal.invalid/x" + + dq = DownloadQueue(dq_env, notifier) + with patch("url_guard.socket.getaddrinfo", + return_value=[(socket.AF_INET, socket.SOCK_STREAM, socket.IPPROTO_TCP, "", + ("169.254.169.254", 0))]): + result = await dq.add(url, "video", "auto", "any", "best", "", "", 0, auto_start=False) + assert result["status"] == "error" + dq.close() + + @pytest.mark.asyncio async def test_cancel_removes_from_pending(dq_env): notifier = AsyncMock() diff --git a/app/tests/test_url_guard.py b/app/tests/test_url_guard.py index 54de12a..b66e082 100644 --- a/app/tests/test_url_guard.py +++ b/app/tests/test_url_guard.py @@ -13,6 +13,7 @@ from url_guard import ( _address_is_global, _guarded_getaddrinfo, _url_endpoint, + download_proxies, install_socket_guard, ) @@ -300,6 +301,110 @@ class GuardedGetaddrinfoTests(unittest.TestCase): _guarded_getaddrinfo("evil.example", 1080) +class DownloadProxiesTests(unittest.TestCase): + """The proxy map is assembled the way YoutubeDL.proxies assembles it.""" + + def test_explicit_proxy_option_replaces_environment(self): + with mock.patch("url_guard.urllib.request.getproxies", + return_value={"http": "http://env:3128", "no": "example.com"}): + self.assertEqual(download_proxies({"proxy": "socks5h://tor:9050"}), + {"all": "socks5h://tor:9050"}) + + def test_empty_proxy_option_means_no_proxy(self): + self.assertEqual(download_proxies({"proxy": ""}), {"all": "__noproxy__"}) + + def test_environment_used_when_no_proxy_option(self): + with mock.patch("url_guard.urllib.request.getproxies", + return_value={"http": "http://env:3128"}): + # http_proxy alone also covers https, as in yt-dlp. + self.assertEqual(download_proxies({}), + {"http": "http://env:3128", "https": "http://env:3128"}) + + def test_no_options_at_all(self): + with mock.patch("url_guard.urllib.request.getproxies", return_value={}): + self.assertEqual(download_proxies(), {}) + + +class ProxiedHostnameTests(unittest.TestCase): + """A proxy that resolves hostnames itself makes a local lookup both wrong + and harmful, so the address check is skipped for hostnames behind one.""" + + def _validate(self, url, proxies): + with mock.patch("url_guard.socket.getaddrinfo") as gai: + gai.side_effect = AssertionError("resolved a hostname that the proxy resolves") + return validate_url(url, proxies=proxies) + + def test_socks5h_hostname_not_resolved(self): + self.assertIsNone(self._validate("https://youtube.com/x", {"all": "socks5h://tor:9050"})) + + def test_plain_socks5_treated_as_remote_dns(self): + # yt-dlp rewrites socks5 to socks5h on every request for compatibility. + self.assertIsNone(self._validate("https://youtube.com/x", {"all": "socks5://tor:9050"})) + + def test_socks4a_hostname_not_resolved(self): + self.assertIsNone(self._validate("https://youtube.com/x", {"all": "socks4a://tor:9050"})) + + def test_http_proxy_hostname_not_resolved(self): + self.assertIsNone(self._validate("https://youtube.com/x", {"https": "http://squid:3128"})) + + def test_scheme_less_proxy_treated_as_http(self): + self.assertIsNone(self._validate("https://youtube.com/x", {"all": "squid:3128"})) + + def test_per_scheme_entry_selected(self): + # Only http is proxied here, so an https URL keeps the check. + proxies = {"http": "http://squid:3128"} + self.assertIsNone(self._validate("http://youtube.com/x", proxies)) + with mock.patch("url_guard.socket.getaddrinfo", return_value=_addrinfo("10.0.0.5")): + self.assertIsNotNone(validate_url("https://youtube.com/x", proxies=proxies)) + + def test_socks4_still_resolved_locally(self): + # SOCKS4 resolves in this process, so the address check still applies. + with mock.patch("url_guard.socket.getaddrinfo", return_value=_addrinfo("10.0.0.5")): + self.assertIsNotNone(validate_url("https://youtube.com/x", + proxies={"all": "socks4://tor:9050"})) + + def test_noproxy_host_still_resolved(self): + # A host excluded from the proxy is fetched directly, so it is checked. + with mock.patch("url_guard.socket.getaddrinfo", return_value=_addrinfo("169.254.169.254")): + self.assertIsNotNone(validate_url( + "http://metadata.internal/x", + proxies={"all": "socks5h://tor:9050", "no": "metadata.internal"})) + + def test_noproxy_marker_keeps_check(self): + with mock.patch("url_guard.socket.getaddrinfo", return_value=_addrinfo("10.0.0.5")): + self.assertIsNotNone(validate_url("https://youtube.com/x", + proxies={"all": "__noproxy__"})) + + def test_ip_literal_still_checked_behind_a_proxy(self): + # An IP literal needs no name resolution, so nothing leaks by judging it + # and the proxy changes nothing: the request names this address either + # way. Unmocked on purpose — getaddrinfo on a literal never hits DNS. + self.assertIsNotNone(validate_url("http://169.254.169.254/latest/meta-data/", + proxies={"all": "socks5h://tor:9050"})) + + def test_ipv6_literal_still_checked_behind_a_proxy(self): + self.assertIsNotNone(validate_url("http://[::1]:8080/x", + proxies={"all": "socks5h://tor:9050"})) + + def test_blocked_hostname_still_blocked_behind_a_proxy(self): + self.assertIsNotNone(validate_url("http://localhost:8080/x", + proxies={"all": "socks5h://tor:9050"})) + + def test_scheme_still_enforced_behind_a_proxy(self): + self.assertIsNotNone(validate_url("file:///etc/passwd", + proxies={"all": "socks5h://tor:9050"})) + + def test_unparseable_proxy_keeps_check(self): + with mock.patch("url_guard.socket.getaddrinfo", return_value=_addrinfo("10.0.0.5")): + self.assertIsNotNone(validate_url("https://youtube.com/x", + proxies={"all": "gopher://weird:70"})) + + def test_no_proxies_argument_keeps_check(self): + # The default: callers that pass nothing get today's behaviour. + with mock.patch("url_guard.socket.getaddrinfo", return_value=_addrinfo("10.0.0.5")): + self.assertIsNotNone(validate_url("https://youtube.com/x")) + + class AllowPrivateBypassTests(unittest.TestCase): """ALLOW_PRIVATE_ADDRESSES: trusted proxy/VPN environments opt out of the SSRF address checks (e.g. Fake-IP clients that resolve to 198.18.0.0/15).""" diff --git a/app/url_guard.py b/app/url_guard.py index cb2c524..2b73988 100644 --- a/app/url_guard.py +++ b/app/url_guard.py @@ -25,6 +25,14 @@ all of these: impact, blind SSRF, since the extraction response is not written to disk). * Native resolvers (curl_cffi/libcurl via ``--impersonate``) resolve outside Python's socket module and bypass the connect-time guard entirely. +* When a proxy carries the fetch and resolves hostnames itself (an HTTP proxy, + ``socks5h``, ``socks4a``, or the plain ``socks5`` yt-dlp rewrites to + ``socks5h``), ``validate_url`` cannot check where a *hostname* leads: the + proxy resolves it on its own network, and looking it up here would both + describe the wrong network and leak the hostname to the local resolver. The + address check is skipped for those, and what the proxy's network exposes is + the proxy's to police. Hosts written as IP literals are still checked, and + the connect-time guard still covers everything dialled directly. """ import ipaddress @@ -47,6 +55,13 @@ _SCHEME_DEFAULT_PORTS = { 'socks5h': 1080, } +# Proxy schemes that hand the destination hostname to the proxy instead of +# resolving it here. yt-dlp rewrites a scheme-less proxy to ``http`` and plain +# ``socks5`` to ``socks5h`` on every request (``clean_proxies``), so only SOCKS4 +# — and the non-standard ``socks`` alias yt-dlp maps onto it — still resolves +# locally and is therefore absent from this list. +_REMOTE_DNS_PROXY_SCHEMES = ('http', 'https', 'socks5', 'socks5h', 'socks4a') + # Hostnames that must be blocked without needing a lookup. ``localhost`` and any # subdomain of it are conventionally loopback, and the GCP metadata name is a # well-known SSRF target that may resolve via a resolver we don't control. @@ -183,6 +198,68 @@ def _collect_proxy_endpoints(proxy_urls) -> set: return _endpoints(candidates) +def _proxy_scheme(proxy: str) -> str: + """The scheme of a configured proxy URL. + + Defaults to ``http`` for the bare ``host:port`` form the ``*_proxy`` + variables accept, which is the same default yt-dlp applies to them. + """ + if not isinstance(proxy, str): + return '' + candidate = proxy.strip() + if '://' not in candidate: + return 'http' if candidate else '' + return urlsplit(candidate).scheme.lower() + + +def download_proxies(ytdl_opts=None) -> dict: + """The proxy map a fetch will use, assembled as ``YoutubeDL.proxies`` does. + + An explicit yt-dlp ``proxy`` option replaces the environment wholesale — + including any ``no_proxy`` exceptions — while without one the ``*_proxy`` + variables apply as they stand. Mirrored rather than imported because + ``YoutubeDL.proxies`` is only reachable from a constructed instance, and + because this decides whether a security check runs: a quiet upstream change + should leave the check in place rather than silently skip it. + """ + opts_proxy = (ytdl_opts or {}).get('proxy') + if opts_proxy is not None: + # '' means "no proxy, ignore the environment", which yt-dlp spells + # '__noproxy__' internally. + return {'all': opts_proxy or '__noproxy__'} + proxies = urllib.request.getproxies() + # compat, as in yt-dlp: http_proxy alone also covers https. + if 'http' in proxies and 'https' not in proxies: + proxies['https'] = proxies['http'] + return proxies + + +def _proxy_resolves_remotely(parts, proxies) -> bool: + """True when the proxy carrying this URL looks its hostname up itself. + + Follows yt-dlp's ``select_proxy``: ``no_proxy`` exclusions first, then the + per-scheme entry, then the catch-all. Anything unrecognised answers False, + so an unparseable or unusual configuration keeps the address check. + """ + if not proxies: + return False + hostname = parts.hostname + if not hostname: + return False + no_proxy = proxies.get('no') + if no_proxy: + hostport = hostname if parts.port is None else f'{hostname}:{parts.port}' + try: + if urllib.request.proxy_bypass_environment(hostport, {'no': no_proxy}): + return False + except (ValueError, UnicodeError): + return False + proxy = proxies.get(parts.scheme.lower()) or proxies.get('all') + if not proxy or proxy == '__noproxy__': + return False + return _proxy_scheme(proxy) in _REMOTE_DNS_PROXY_SCHEMES + + # Captured at import so re-installing the guard never wraps the wrapper. _real_getaddrinfo = socket.getaddrinfo @@ -264,7 +341,7 @@ def install_socket_guard(allow_private: bool = False, proxy_urls=(), service_url socket.getaddrinfo = _guarded_getaddrinfo -def validate_url(url: str, allow_private: bool = False) -> str | None: +def validate_url(url: str, allow_private: bool = False, proxies: dict | None = None) -> str | None: """Return an error message if the URL is disallowed, else ``None``. Inputs without a ``://`` scheme separator (bare video IDs, ``ytsearch:`` @@ -275,6 +352,10 @@ def validate_url(url: str, allow_private: bool = False) -> str | None: and internal-address checks are skipped so that trusted proxy/VPN setups — e.g. Fake-IP clients that resolve YouTube to ``198.18.0.0/15`` — can be used. Scheme validation (http/https only) still applies. + + *proxies* is the proxy map the fetch will use (see ``download_proxies``). + When it routes this URL through a proxy that resolves hostnames itself, the + address check is skipped — see the comment at that branch for why. """ if not isinstance(url, str): return 'Invalid URL' @@ -300,6 +381,21 @@ def validate_url(url: str, allow_private: bool = False) -> str | None: if _hostname_is_blocked(hostname): return f'Refusing to fetch internal host "{hostname}"' + # A host written as an IP literal needs no lookup, so it is judged directly + # whatever the proxy setup: nothing leaves this process, and the address is + # exactly the one the request will name. + if _normalise_ip(hostname) is None and _proxy_resolves_remotely(parts, proxies): + # The proxy resolves this hostname on its own network, so a lookup here + # answers a different question than the one that matters, and asking it + # is itself the harm: it leaks the hostname of every queued URL to the + # local resolver, which is the single thing a SOCKS/Tor setup exists to + # prevent. It also fails closed against resolvers this process cannot + # reach — a container pointed at the proxy's own DNS port resolves + # nothing here and every add is refused. What the proxy's network + # exposes is the proxy's to police; the connect-time guard still holds + # everything that is dialled directly. + return None + try: addrinfo = socket.getaddrinfo(hostname, parts.port, proto=socket.IPPROTO_TCP) except socket.gaierror: diff --git a/app/ytdl.py b/app/ytdl.py index 569250a..2774465 100644 --- a/app/ytdl.py +++ b/app/ytdl.py @@ -27,7 +27,7 @@ from music_metadata import MusicMetadataPreProcessor from datetime import datetime from state_store import AtomicJsonStore, from_json_compatible, read_legacy_shelf, to_json_compatible from subscriptions import _entry_id -from url_guard import validate_url, install_socket_guard +from url_guard import validate_url, install_socket_guard, download_proxies from urllib.parse import urlsplit log = logging.getLogger('ytdl') @@ -2007,8 +2007,15 @@ class DownloadQueue: # SSRF guard: reject non-http(s) schemes and hosts resolving to # internal/loopback/link-local/metadata addresses before yt-dlp fetches # anything. run_in_executor because validate_url may perform a DNS lookup. + # The merged options decide the proxy, same as the connect-time guard + # reads `proxy` from them — a proxied fetch resolves at the proxy, so + # the address check is skipped rather than leaking the hostname here. + proxies = download_proxies( + self._build_ytdl_options(ytdl_options_presets, ytdl_options_overrides)) url_error = await asyncio.get_running_loop().run_in_executor( - None, partial(validate_url, url, allow_private=self.config.ALLOW_PRIVATE_ADDRESSES)) + None, partial(validate_url, url, + allow_private=self.config.ALLOW_PRIVATE_ADDRESSES, + proxies=proxies)) if url_error is not None: log.warning('Rejected URL "%s": %s', url, url_error) await self.__record_add_failure(