Skip to content

caldav: a strict SSRF tier for instances whose users are not the operator, off by default - #46

Open
distronode-com wants to merge 1 commit into
Calnode:mainfrom
distronode-com:up/caldav-ssrf-tiers
Open

caldav: a strict SSRF tier for instances whose users are not the operator, off by default#46
distronode-com wants to merge 1 commit into
Calnode:mainfrom
distronode-com:up/caldav-ssrf-tiers

Conversation

@distronode-com

Copy link
Copy Markdown
Contributor

server_url on the CalDAV connect form is a bring-your-own-server field, and the client deliberately dials it through the narrow guard (MetadataSafeTransport, cloud-metadata range only), because a self-hoster pointing it at a Nextcloud, Radicale or Baïkal on their own LAN or on localhost is the intended configuration of a self-hostable product. That default is right and this PR does not change it.

It adds the other tier for the other kind of instance. Where the people connecting calendars are not the operator, every term inverts: the string is supplied by someone else, the private network it can reach is the operator's (the pod network, the node's exporters, the database), and the connect form's own error is the oracle, because connect-success versus "could not reach", plus timing, is a port scan any account holder can run against a hostname they control.

Three pieces, all off by default:

  • netutil.GuardedTransport(resolve, logger, msg) is now the exported implementation under SafeTransport and MetadataSafeTransport (both unchanged in signature and produced text), taking a Resolver so a caller can pick its tier per instance. The dialer fails with a sentinel, netutil.ErrBlockedAddress, wrapped into the same "netutil: target resolved to a blocked address" string as before; the resolved address stays in the log line and never enters the error.
  • caldav.New(db, key, opts...) gains WithStrictSSRFGuard(bool). Strict routes every dial, the initial one and each manual redirect hop in propfind (which already re-enters c.hc.Do, so each hop is re-checked), through ResolveSafe. Under strict, and only under strict, a refused dial collapses in do() to the sentence findPrincipal has always produced when discovery runs out of candidates, caldav: could not reach the CalDAV server, so a refused address and a dead host are indistinguishable to the caller. The narrow tier keeps netutil's own message, since refusing one fixed well-known address is an answer about the typed URL, not a probe result.
  • CALDAV_STRICT_SSRF (bool, default false), read next to the other booleans in config.go, wired in server.go, documented in ARCHITECTURE alongside TRUSTED_PROXY_CIDRS, which is the same operator-versus-users theme.

Tests: GuardedTransport refuses a blocked resolution with the sentinel and no address in the string; strict refuses a private name with the generic sentence; a 302 from a permitted host to a blocked one is refused on the hop (asserting the resolver was asked for the redirect target); the default transport is unchanged; the env var parses true/false/unset/garbage. A mutation run with the collapse disabled fails three of the four strict tests.

Noted, left alone: the save-time validateBYOServerURL in ConnectCalDAV stays narrow, so under strict a private URL passes save-time and is refused at dial with the generic sentence, which is the correct outcome and closes no oracle, but the two checks now disagree on policy. The BYO-LLM endpoint and the LiveKit URL also use the narrow tier; they are admin-configured, so the threat model differs, but GuardedTransport makes the same choice available to them.

🤖 Generated with Claude Code

…ator, off by default

`server_url` is a bring-your-own-server field, so the CalDAV client deliberately uses
netutil's narrow, metadata-only dial guard: a self-hoster pointing Calnode at a Nextcloud,
Radicale or Baikal on their own LAN, or on localhost, is the intended configuration of a
self-hostable product, and blocking private ranges there would break the feature for the
people it was written for. That reasoning is correct and the default is unchanged.

Every term of it inverts once the people using an instance are not the operator. The URL is
supplied by somebody else; the private network it can reach is the OPERATOR's, the service
network, the node's exporters, the database; and the connect endpoint is available to any
account holder. Connect-success versus connect-failure, plus timing, times a hostname the
caller chooses, is a port scan of that network, and the connect error is the oracle that
reads it out: handler.ConnectCalDAV writes err.Error() straight into a 400.

So the tier becomes a property of the instance rather than of the package.
CALDAV_STRICT_SSRF (config.CalDAVStrictSSRF, default false) is passed to
caldav.WithStrictSSRFGuard from server.BuildHandler, and in that mode every dial goes
through netutil.ResolveSafe, the guard webhook delivery already uses: private, loopback,
link-local, CGNAT and ULA all refused. Each redirect hop is covered too, because CalDAV
discovery follows redirects by hand to preserve the PROPFIND method and every hop re-enters
c.do and so the same guarded client (internal/caldav/webdav.go:79).

The user-facing error is the second half, not a detail. A refusal saying "resolved to a
blocked address" would hand the caller exactly the oracle the guard closes, and with a
hostname that resolves several ways, which address was picked. Under the strict tier a
blocked dial now produces the sentence discovery has always produced for an unreachable
server, verbatim and with no address in it. The resolved address goes to the log line
netutil already writes and nowhere else. netutil gains ErrBlockedAddress as a sentinel so
the mapping is one errors.Is at the call site rather than string matching through
*url.Error, and GuardedTransport plus the Resolver type so a caller can pick its tier per
instance; SafeTransport and MetadataSafeTransport are now thin wrappers over it and their
behaviour, including the text of the error they produce, is unchanged.

The default is byte-identical to before: same transport, and the error collapse is scoped
to the strict tier, so the narrow tier still surfaces netutil's own text for the one thing
it refuses. TestDefaultTransportIsUnchanged pins both halves.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — two doc gaps worth closing before or with merge.

Reviewed changes One-commit security addition: an opt-in strict CalDAV dial guard for instances whose users are not the operator, default off so self-host LAN/localhost CalDAV still works.

  • netutil.GuardedTransport / ErrBlockedAddress — shared implementation under the existing named tiers; blocked resolve becomes a sentinel and keeps the same public error text, with the address only in logs.
  • caldav.WithStrictSSRFGuard + CALDAV_STRICT_SSRF — strict path uses ResolveSafe on every dial; wired from server.BuildHandler only.
  • Connect oracle collapse — under strict, refused dials map in do() to errCouldNotReach, matching discovery exhaustion; narrow tier behavior stays unchanged.
  • Redirect coverage — manual propfind hops re-enter the guarded client; tests pin hop re-check and default-off private allow.
  • Docs/tests — CHANGELOG + ARCHITECTURE §16; config and caldav/netutil tests. Package tests for netutil/caldav/config pass in this review.

ℹ️ claims.yaml caveat still describes only the narrow guard

audit/claims.yaml entry caldav-connect-self-service-no-admin-gate still says non-admin CalDAV connect is bounded only by the metadata-only SSRF guard. After this PR that is still the default, but the residual risk can be closed with CALDAV_STRICT_SSRF. Repo convention is to keep claim text in sync when trust rules change.

Technical details
# Update caldav self-service SSRF caveat

## Affected sites
- `audit/claims.yaml` (`caldav-connect-self-service-no-admin-gate` caveat) — still says exposure is bounded only by `ResolveNotMetadata` / narrow guard

## Required outcome
- Caveat should state default remains narrow, and that multi-tenant / non-operator-user deployments can set `CALDAV_STRICT_SSRF=true` for the strict tier + generic connect error
- Prefer a verify bullet that points at `WithStrictSSRFGuard` / `cfg.CalDAVStrictSSRF` wiring if that matches how neighboring claims are written

ℹ️ DEPLOY.md env table omits CALDAV_STRICT_SSRF

ARCHITECTURE and CHANGELOG document the flag; the operator-facing env table in DEPLOY.md (where TRUSTED_PROXY_CIDRS already lives) does not. Multi-tenant operators who only read the deploy guide will not discover the control that closes the threat this PR describes.

Technical details
# Document CALDAV_STRICT_SSRF in DEPLOY.md

## Affected sites
- `DEPLOY.md` §1 configuration table — no row for `CALDAV_STRICT_SSRF`

## Required outcome
- Add a row: optional, default `false`, short note that `true` widens CalDAV dial refusal to private/loopback/CGNAT/ULA and collapses refused connect errors (point at ARCHITECTURE §16)

Pullfrog  | Fix it ➔View workflow run | Using Grok𝕏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant