Skip to content

require_tls only enforced on RFC 6764 discovery, not on explicit URLs #687

Description

@tobixen

require_tls was added together with the .well-known forwarding, as one out of two measures to prevent a man-in-the-middle-attack. However, something went wrong because in the current main the configuration option is not checked anywhere. In the current v3.3.0-dev branch and future v3.3.0-release there is a fix which will honor this variable but only for well-known forwarding. I think this is silly, it should be for all connections.

Now True is and should be the default. If doing this for all connections, it will be a breaking change - so it's procrastinated until the 4.0-release.

I was too lazy to create this issue myself, so below is the initial text from Claude.


⚠️ This issue is AI-generated (Claude Opus 4.8 via Claude Code) on behalf of tobixen ⚠️

Summary

require_tls is named and documented as if it guarantees a TLS-only connection, but it currently only gates the RFC 6764 discovery path. When a fully-qualified URL is passed explicitly, the parameter has no effect.

Current behaviour (v3.3.x)

The parameter is threaded only through discovery:

DAVClient.__init__ / AsyncDAVClient.__init___auto_url(require_tls=...)discover_caldav(require_tls=...)discover_service (where d6267c78 closed the well-known-redirect hole).

But _auto_url returns early for any URL containing /, so discovery — and therefore the require_tls check — is skipped entirely. Result:

# require_tls=True is the DEFAULT, yet this connects over plaintext:
DAVClient(url="http://caldav.example.com/dav/", username=..., password=...)

A parameter literally named require_tls does not require TLS for the connection actually made. This is a name/behaviour mismatch and a potential downgrade footgun.

Proposed fix (deferred to 4.0)

Make enforcement global rather than discovery-only:

  1. Add a shared BaseDAVClient._enforce_require_tls(url) helper (there is no shared __init__ today — the two constructors are separate).
  2. Call it from both DAVClient.__init__ and AsyncDAVClient.__init__ right after URL objectification, rejecting non-https URLs when require_tls=True.
  3. Update docstrings + CHANGELOG.

Why this is deferred (breaking change)

require_tls defaults to True. Making it global would reject every explicitly-passed http:// URL by default, which breaks real usage:

  • The project's own docker integration test servers (Cyrus, SOGo, Baikal, …) are all http://localhost:PORT/....
  • Anyone running an internal/LAN CalDAV server over plain HTTP with default args.

So this changes the default security posture and is a breaking change — appropriate for a major release (4.0), with require_tls=False threaded through the test config.

Out of scope for the constructor check

A constructor-time scheme check does not catch an https → http downgrade via redirect during live requests. If "applies to any connection" is taken literally, the per-request path in each client needs enforcement too — worth tracking separately.

Decision

For now: keep require_tls discovery-only and document the scope precisely (done alongside this issue). Revisit global enforcement in 4.0.

⚠️ This issue is AI-generated (Claude Opus 4.8 via Claude Code) on behalf of tobixen ⚠️

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions