diff --git a/README.md b/README.md index a5710eb..36cd2d8 100644 --- a/README.md +++ b/README.md @@ -98,7 +98,7 @@ Enabling `writeinfojson` or `writethumbnail` in `YTDL_OPTIONS` also writes a fee * __HTTPS__: Use `https` instead of `http` (__CERTFILE__ and __KEYFILE__ required). Defaults to `false`. * __CERTFILE__: HTTPS certificate file path. * __KEYFILE__: HTTPS key file path. -* __CORS_ALLOWED_ORIGINS__: Comma-separated list of origins permitted to make cross-origin requests to the MeTube API; `*` allows all. When unset or empty, all cross-origin requests are denied. Required for browser extensions and bookmarklets — see [Sending links to MeTube](#-sending-links-to-metube). +* __CORS_ALLOWED_ORIGINS__: Comma-separated list of origins permitted to make cross-origin requests to the MeTube API; `*` allows all. When unset or empty, all cross-origin requests are denied. Required for browser extensions and bookmarklets — see [Sending links to MeTube](#-sending-links-to-metube). Naming origins explicitly also lets them send credentials (a login cookie, or the `Authorization` header a reverse proxy checks), which `*` deliberately does not: it would let any site you visit drive your instance with your own session. * __ROBOTS_TXT__: A path to a `robots.txt` file mounted in the container. ## 🎛️ Configuring yt-dlp options @@ -243,7 +243,7 @@ __Browser extensions__ allow right-clicking videos and sending them directly to * __Chrome:__ contributed by [Rpsl](https://github.com/rpsl) — install from the [Chrome Webstore](https://chrome.google.com/webstore/detail/metube-downloader/fbmkmdnlhacefjljljlbhkodfmfkijdh) or [from sources](https://github.com/Rpsl/metube-browser-extension). * __Firefox:__ contributed by [nanocortex](https://github.com/nanocortex) — install from [Firefox Addons](https://addons.mozilla.org/en-US/firefox/addon/metube-downloader) or get sources [here](https://github.com/nanocortex/metube-firefox-addon). -__Bookmarklets__ send the currently open page to MeTube with one click. Add the origins of the sites where you use them to `CORS_ALLOWED_ORIGINS`, e.g. `https://www.youtube.com,https://www.vimeo.com`. The code (Chrome and Firefox variants, contributed by [kushfest](https://github.com/kushfest) and [shoonya75](https://github.com/shoonya75)) is in the [Bookmarklets wiki page](https://github.com/alexta69/metube/wiki/Bookmarklets). +__Bookmarklets__ send the currently open page to MeTube with one click. Add the origins of the sites where you use them to `CORS_ALLOWED_ORIGINS`, e.g. `https://www.youtube.com,https://www.vimeo.com`. If your instance sits behind authentication, list the origins individually rather than using `*` — only named origins are allowed to send credentials. The code (Chrome and Firefox variants, contributed by [kushfest](https://github.com/kushfest) and [shoonya75](https://github.com/shoonya75)) is in the [Bookmarklets wiki page](https://github.com/alexta69/metube/wiki/Bookmarklets). __iOS Shortcut:__ [rithask](https://github.com/rithask) created an [iOS shortcut](https://www.icloud.com/shortcuts/66627a9f334c467baabdb2769763a1a6) for sending URLs to MeTube from Safari's share menu; it prompts for your instance address on first use. diff --git a/app/main.py b/app/main.py index e48ab43..592a5a5 100644 --- a/app/main.py +++ b/app/main.py @@ -335,6 +335,12 @@ async def state_dir_guard(request, handler): app = web.Application(middlewares=[state_dir_guard]) _cors_origins = [o.strip() for o in config.CORS_ALLOWED_ORIGINS.split(',') if o.strip()] if config.CORS_ALLOWED_ORIGINS else [] +if '*' in _cors_origins and len(_cors_origins) > 1: + log.warning( + "CORS_ALLOWED_ORIGINS mixes '*' with named origins %s. '*' wins, and credentialed " + "cross-origin requests stay disabled for every origin in the list. Remove '*' if you " + "need a bookmarklet to reach an authenticated instance.", + [o for o in _cors_origins if o != '*']) sio = socketio.AsyncServer(cors_allowed_origins=_cors_origins if _cors_origins else []) sio.attach(app, socketio_path=config.URL_PREFIX + 'socket.io') routes = web.RouteTableDef() @@ -1286,9 +1292,44 @@ app.router.add_route('OPTIONS', config.URL_PREFIX + 'delete-cookies', add_cors) async def on_prepare(request, response): origin = request.headers.get('Origin') - if origin and _cors_origins and ('*' in _cors_origins or origin in _cors_origins): - response.headers['Access-Control-Allow-Origin'] = origin - response.headers['Access-Control-Allow-Headers'] = 'Content-Type' + if not origin or not _cors_origins: + return + + # Naming an origin in CORS_ALLOWED_ORIGINS is a deliberate trust grant, so + # such an origin may send credentials: the cookie or Authorization header + # that a reverse proxy in front of MeTube checks. Without this a bookmarklet + # cannot reach an authenticated instance at all (issue #155). + # + # The '*' wildcard is emphatically not such a grant — it matches origins the + # operator never enumerated, including every site the user happens to visit. + # Echoing the origin back (which we must do, since '*' is illegal alongside + # credentials) and allowing credentials would let any page drive the user's + # instance with the user's own session. So the wildcard keeps exactly the + # uncredentialed behaviour it has always had, and a wildcard anywhere in the + # list disables credentials for every origin in it. + # + # Derived here rather than held in a second module global so the wildcard + # test and the membership test can never disagree about the same list. + wildcard = '*' in _cors_origins + trusted = not wildcard and origin in _cors_origins + if not (wildcard or trusted): + return + + response.headers['Access-Control-Allow-Origin'] = origin + # Authorization rides on the same grant: allowing it under the wildcard + # would let an arbitrary page attempt credentials against an instance it + # can already reach, from inside the victim's network. + response.headers['Access-Control-Allow-Headers'] = 'Content-Type, Authorization' if trusted else 'Content-Type' + if trusted: + response.headers['Access-Control-Allow-Credentials'] = 'true' + + # The response now differs per Origin, so a shared cache must not hand one + # origin's Allow-Origin to another. + vary = response.headers.get('Vary') + if not vary: + response.headers['Vary'] = 'Origin' + elif 'origin' not in (v.strip().lower() for v in vary.split(',')): + response.headers['Vary'] = f'{vary}, Origin' app.on_response_prepare.append(on_prepare) diff --git a/app/tests/test_api.py b/app/tests/test_api.py index 0ed7ee0..1b9199a 100644 --- a/app/tests/test_api.py +++ b/app/tests/test_api.py @@ -525,3 +525,130 @@ async def test_download_blocks_state_dir_files(monkeypatch): (download_dir / "video.mp4").unlink(missing_ok=True) (download_dir / percent_filename).unlink(missing_ok=True) state_dir.rmdir() + +# --- CORS (issue #155) ------------------------------------------------------- +# +# The security property under test: credentials are granted only to an origin +# the operator named explicitly, and never under the '*' wildcard. Each test +# builds a fresh Application because main.app binds to the first event loop +# that runs it; the logic under test lives entirely in main.on_prepare, and the +# real main.add_cors preflight handler is mounted so the preflight path is the +# production one. + +async def _cors_version(request): + return web.Response(text="v") + + +def _cors_app(): + app = web.Application() + app.router.add_route("OPTIONS", "/add", main.add_cors) + app.router.add_get("/version", _cors_version) + app.on_response_prepare.append(main.on_prepare) + return app + + +async def _cors_headers(monkeypatch, origins, origin, path="/add", method="OPTIONS"): + monkeypatch.setattr(main, "_cors_origins", origins) + async with TestClient(TestServer(_cors_app())) as client: + resp = await client.request( + method, path, + headers={ + "Origin": origin, + "Access-Control-Request-Method": "POST", + "Access-Control-Request-Headers": "content-type,authorization", + }, + ) + return resp.headers + + +@pytest.mark.asyncio +async def test_cors_listed_origin_gets_credentials(monkeypatch): + h = await _cors_headers(monkeypatch, ["https://www.youtube.com"], "https://www.youtube.com") + assert h["Access-Control-Allow-Origin"] == "https://www.youtube.com" + assert h["Access-Control-Allow-Credentials"] == "true" + assert "Authorization" in h["Access-Control-Allow-Headers"] + assert "Origin" in h["Vary"] + + +@pytest.mark.asyncio +async def test_cors_wildcard_never_grants_credentials(monkeypatch): + h = await _cors_headers(monkeypatch, ["*"], "https://evil.example") + # The wildcard still reflects the origin, exactly as before... + assert h["Access-Control-Allow-Origin"] == "https://evil.example" + # ...but must not hand out the user's session, nor let a page attempt + # credentials of its own. + assert "Access-Control-Allow-Credentials" not in h + assert h["Access-Control-Allow-Headers"] == "Content-Type" + + +@pytest.mark.asyncio +async def test_cors_wildcard_mixed_with_named_origin_still_denies_credentials(monkeypatch): + # '*' anywhere in the list disables credentials for everyone in it, so a + # stray wildcard cannot silently widen a named grant. + h = await _cors_headers(monkeypatch, ["*", "https://www.youtube.com"], "https://www.youtube.com") + assert h["Access-Control-Allow-Origin"] == "https://www.youtube.com" + assert "Access-Control-Allow-Credentials" not in h + assert h["Access-Control-Allow-Headers"] == "Content-Type" + + +@pytest.mark.asyncio +async def test_cors_unlisted_origin_gets_nothing(monkeypatch): + h = await _cors_headers(monkeypatch, ["https://www.youtube.com"], "https://evil.example") + assert "Access-Control-Allow-Origin" not in h + assert "Access-Control-Allow-Credentials" not in h + + +@pytest.mark.asyncio +async def test_cors_disabled_by_default(monkeypatch): + h = await _cors_headers(monkeypatch, [], "https://www.youtube.com") + assert "Access-Control-Allow-Origin" not in h + assert "Access-Control-Allow-Credentials" not in h + + +@pytest.mark.asyncio +async def test_cors_credentials_apply_to_actual_response_not_just_preflight(monkeypatch): + # The browser checks Allow-Credentials on the real response too, so a + # preflight-only grant would still fail. + h = await _cors_headers( + monkeypatch, ["https://www.youtube.com"], "https://www.youtube.com", + path="/version", method="GET") + assert h["Access-Control-Allow-Credentials"] == "true" + + +@pytest.mark.asyncio +async def test_cors_origin_match_is_exact(monkeypatch): + # Substring or suffix matching here would be a bypass. + for impostor in ( + "https://www.youtube.com.evil.example", + "https://evilwww.youtube.com", + "http://www.youtube.com", + "https://www.youtube.com:8443", + ): + h = await _cors_headers(monkeypatch, ["https://www.youtube.com"], impostor) + assert "Access-Control-Allow-Origin" not in h, impostor + assert "Access-Control-Allow-Credentials" not in h, impostor + + +@pytest.mark.asyncio +async def test_cors_null_origin_is_not_trusted(monkeypatch): + # Sandboxed iframes and some file:// contexts send Origin: null. + h = await _cors_headers(monkeypatch, ["https://www.youtube.com"], "null") + assert "Access-Control-Allow-Origin" not in h + assert "Access-Control-Allow-Credentials" not in h + + +@pytest.mark.asyncio +async def test_cors_vary_appends_to_existing_value(monkeypatch): + # Static responses can already carry a Vary; clobbering it would break + # content negotiation. + monkeypatch.setattr(main, "_cors_origins", ["https://www.youtube.com"]) + + async def handler(request): + return web.Response(text="x", headers={"Vary": "Accept-Encoding"}) + + app = web.Application() + app.router.add_get("/v", handler) + app.on_response_prepare.append(main.on_prepare) + async with TestClient(TestServer(app)) as client: + resp = await client.get("/v", headers={"Origin": "https://www.youtube.com"}) + assert resp.headers["Vary"] == "Accept-Encoding, Origin"