From f78539ba15b19014ddc5fe670c48a146dc50cbb0 Mon Sep 17 00:00:00 2001 From: Joeseph Grey <212606152+StressTestor@users.noreply.github.com> Date: Sat, 6 Jun 2026 22:05:24 -0600 Subject: [PATCH] fix(caldav): disable redirects on the sync/write-back DAVClient (SSRF) (#2663) validate_caldav_url resolves and vets the initial host, but caldav's niquests session follows 3xx redirects by default, so a validated public URL can be redirected at request time to loopback/link-local/private space, re-opening the SSRF the host check closes. The existing redirect guard only covered the settings test-connection path. Add a shared _build_dav_client helper that pins the session to zero redirects (any 3xx then raises instead of silently following an attacker-chosen Location), and route both the pull (_sync_blocking) and write-back (_writeback_blocking) paths through it. Mirrors the follow_redirects=False already used on the test-connection path. Tests exercise the real DAVClient request path (a 302 toward an internal host is refused, the sink is never contacted; the PROPFIND is asserted to reach the public server first so the check can't pass vacuously), confirm the helper disables redirects on the installed client, guard against a raw DAVClient creeping back in, cover mixed public/internal DNS results in both orderings, and add the resolves-to-no-usable-records fail-closed branch. --- src/caldav_sync.py | 29 +++++- src/caldav_writeback.py | 6 +- tests/test_caldav_google_principal_url.py | 3 + tests/test_caldav_redirect_hardening.py | 105 ++++++++++++++++++++++ tests/test_caldav_url_hardening.py | 33 +++++++ 5 files changed, 172 insertions(+), 4 deletions(-) create mode 100644 tests/test_caldav_redirect_hardening.py diff --git a/src/caldav_sync.py b/src/caldav_sync.py index a2ce22acf..0fe0e96c4 100644 --- a/src/caldav_sync.py +++ b/src/caldav_sync.py @@ -216,18 +216,43 @@ def _open_url_as_calendar(client, url: str): return client.calendar(url=target) +def _build_dav_client(url: str, username: str, password: str): + """Construct a CalDAV client with automatic redirects disabled. + + ``validate_caldav_url`` resolves and vets the *initial* host, but caldav's + underlying HTTP session follows 3xx redirects by default. So a URL that + passes validation can still be redirected — at request time — to + loopback / link-local / private space, re-opening the SSRF the host check + closes. Pin the session to zero redirects: any 3xx then raises instead of + silently following an attacker-chosen ``Location``. This mirrors the + test-connection path in ``routes/calendar_routes.py``, which already sets + ``follow_redirects=False``. + + DAVClient exposes no per-request redirect flag, so we set it on the session + after construction (the session is created in ``__init__``). + """ + import caldav + + client = caldav.DAVClient(url=url, username=username, password=password) + # Unconditional: a redirect-disable that only sometimes applies is not a + # control. The session exists right after __init__ on every real client; + # test_build_dav_client_disables_redirects asserts it against installed + # caldav in CI. + client.session.max_redirects = 0 + return client + + def _sync_blocking(owner: str, url: str, username: str, password: str, account_id: str = "") -> dict: """The actual sync — synchronous, intended to run in a threadpool. Returns counts: {calendars, events, deleted, errors}.""" # Lazy imports so a missing `caldav` dep doesn't break app startup — # the integrations form still works, sync just no-ops with an error. - import caldav from caldav.lib.error import AuthorizationError, NotFoundError from core.database import CalendarCal, CalendarEvent, SessionLocal result = {"calendars": 0, "events": 0, "deleted": 0, "errors": []} - client = caldav.DAVClient(url=url, username=username, password=password) + client = _build_dav_client(url, username, password) # Discovery: try principal → calendars first; if the server doesn't # support discovery (or the URL points directly at a calendar), fall diff --git a/src/caldav_writeback.py b/src/caldav_writeback.py index b1b92c05f..0866e1467 100644 --- a/src/caldav_writeback.py +++ b/src/caldav_writeback.py @@ -143,8 +143,10 @@ def _discover_calendars(client): def _writeback_blocking(local_cal_id, ev, delete, url, username, password, owner="", account_id="") -> dict: - import caldav - client = caldav.DAVClient(url=url, username=username, password=password) + from src.caldav_sync import _build_dav_client + # Redirects disabled here too: the write-back path opens its own DAVClient, + # so it needs the same SSRF-via-redirect protection as the pull path. + client = _build_dav_client(url, username, password) calendars = _discover_calendars(client) if not calendars: return {"ok": False, "error": "no remote calendars discovered"} diff --git a/tests/test_caldav_google_principal_url.py b/tests/test_caldav_google_principal_url.py index ce9cefed8..f4eb06b0f 100644 --- a/tests/test_caldav_google_principal_url.py +++ b/tests/test_caldav_google_principal_url.py @@ -83,6 +83,9 @@ class _FakePrincipal: class _FakeClient: def __init__(self, url=None, username=None, password=None): self.url = url + # Mirror the real DAVClient: _build_dav_client sets + # session.max_redirects = 0 right after construction. + self.session = types.SimpleNamespace(max_redirects=30) def principal(self): return _FakePrincipal() diff --git a/tests/test_caldav_redirect_hardening.py b/tests/test_caldav_redirect_hardening.py new file mode 100644 index 000000000..0d3ce91b7 --- /dev/null +++ b/tests/test_caldav_redirect_hardening.py @@ -0,0 +1,105 @@ +"""CalDAV SSRF-via-redirect hardening. + +``validate_caldav_url`` resolves and vets the initial host, but the CalDAV +client's HTTP session follows 3xx redirects by default — so a validated public +URL can be redirected, at request time, into loopback/private space (an SSRF +that bypasses the host check). ``_build_dav_client`` pins the session to zero +redirects. These tests exercise the real DAVClient request path (the sync / +write-back surface), not just the settings/test-connection endpoint. +""" + +import http.server +import socketserver +import threading + +import pytest + +from src import caldav_sync, caldav_writeback + + +def test_build_dav_client_disables_redirects(): + """The hardened client must carry a redirect-disabled session.""" + pytest.importorskip("caldav") + client = caldav_sync._build_dav_client("https://calendar.example.com/dav", "u", "p") + assert client.session.max_redirects == 0 + + +def test_dav_client_does_not_follow_redirect_to_internal_host(): + """End-to-end through the real DAVClient: a 302 toward an internal host + must NOT be followed. Without the fix the sink is contacted (SSRF); with it + the redirect is refused and the sink is never reached.""" + pytest.importorskip("caldav") + + sink_hits: list[str] = [] + public_methods: list[str] = [] + + class _Internal(http.server.BaseHTTPRequestHandler): + # Stand-in for an internal service the attacker redirects toward. + def do_GET(self): # noqa: N802 + sink_hits.append(self.path) + self.send_response(207) + self.end_headers() + + do_PROPFIND = do_GET + + def log_message(self, *a): # silence test server + pass + + class _Public(http.server.BaseHTTPRequestHandler): + # The "validated" public CalDAV server that redirects everything inward. + def do_GET(self): # noqa: N802 + public_methods.append(self.command) + self.send_response(302) + self.send_header("Location", f"http://127.0.0.1:{internal_port}/leak") + self.end_headers() + + do_PROPFIND = do_GET + + def log_message(self, *a): + pass + + internal = socketserver.TCPServer(("127.0.0.1", 0), _Internal) + internal_port = internal.server_address[1] + public = socketserver.TCPServer(("127.0.0.1", 0), _Public) + public_port = public.server_address[1] + threading.Thread(target=internal.serve_forever, daemon=True).start() + threading.Thread(target=public.serve_forever, daemon=True).start() + try: + public_url = f"http://127.0.0.1:{public_port}/dav" + client = caldav_sync._build_dav_client(public_url, "u", "p") + client.timeout = 5 + try: + client.request(public_url, "PROPFIND", "") + except Exception: + # Refusing the redirect surfaces as an exception (TooManyRedirects); + # that is the intended fail-closed behavior. The security assertion + # is that the internal sink was never contacted. + pass + # The request must actually have left the building — otherwise an early + # error would make "sink not hit" pass vacuously. + assert public_methods == ["PROPFIND"], "the PROPFIND must reach the public server first" + assert sink_hits == [], "redirect toward an internal host must not be followed" + finally: + internal.shutdown() + public.shutdown() + + +def test_sync_and_writeback_construct_clients_through_the_helper(): + """Guard against a raw DAVClient (redirects enabled) creeping back in. + Every DAVClient on the sync/write-back paths must go through + ``_build_dav_client`` so the redirect protection can't be bypassed.""" + sync_src = (caldav_sync.__file__) + wb_src = (caldav_writeback.__file__) + with open(sync_src, encoding="utf-8") as f: + sync_text = f.read() + with open(wb_src, encoding="utf-8") as f: + wb_text = f.read() + + # In caldav_sync the only raw construction lives inside the helper itself. + assert sync_text.count("caldav.DAVClient(") == 1 + assert "max_redirects = 0" in sync_text + assert "_build_dav_client(" in sync_text + + # Write-back must not construct its own raw client; it reuses the helper. + assert "caldav.DAVClient(" not in wb_text + assert "_build_dav_client(" in wb_text diff --git a/tests/test_caldav_url_hardening.py b/tests/test_caldav_url_hardening.py index 0ea8b2bf9..c00fbcd9d 100644 --- a/tests/test_caldav_url_hardening.py +++ b/tests/test_caldav_url_hardening.py @@ -82,6 +82,39 @@ def test_validate_caldav_url_fails_closed_when_hostname_does_not_resolve(monkeyp caldav_sync.validate_caldav_url("https://calendar.example.com/dav") +def test_validate_caldav_url_fails_closed_when_host_resolves_to_no_usable_records(monkeypatch): + # Distinct from the OSError path above: here resolution *succeeds* but yields + # no usable A/AAAA records (the `if not addrs` branch). Fail closed there too + # rather than letting an un-vetted host through. + monkeypatch.setattr(caldav_sync, "_resolve_caldav_host_ips", lambda host: []) + + with pytest.raises(ValueError, match="host does not resolve"): + caldav_sync.validate_caldav_url("https://calendar.example.com/dav") + + +@pytest.mark.parametrize( + "addrs", + [ + ["93.184.216.34", "127.0.0.1"], # public first, internal second + ["127.0.0.1", "93.184.216.34"], # internal first, public second + ], +) +def test_validate_caldav_url_blocks_mixed_dns_in_any_order(monkeypatch, addrs): + # A host that resolves to BOTH a public and an internal address must be + # rejected regardless of record order — every resolved address is checked, + # so one internal answer is enough to block. Defends DNS round-robin and a + # rebind that slips an internal A-record alongside a public one. + monkeypatch.delenv("ODYSSEUS_ALLOW_PRIVATE_CALDAV", raising=False) + monkeypatch.setattr( + caldav_sync, + "_resolve_caldav_host_ips", + lambda host: [ipaddress.ip_address(a) for a in addrs], + ) + + with pytest.raises(ValueError, match="host is not allowed"): + caldav_sync.validate_caldav_url("https://calendar.example.com/dav") + + def test_sync_caldav_decrypts_stored_password_and_validates_url(monkeypatch): monkeypatch.setattr( caldav_sync,