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.
This commit is contained in:
Joeseph Grey
2026-06-06 22:05:24 -06:00
committed by GitHub
parent 95c2dca4b5
commit f78539ba15
5 changed files with 172 additions and 4 deletions
+27 -2
View File
@@ -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
+4 -2
View File
@@ -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"}
@@ -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()
+105
View File
@@ -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
+33
View File
@@ -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,