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

未关闭 适合新手
  • 哪里不对？
#687 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看

维护者通常 1 天内回复

还没有人认领这个 Issue。

评估

难度
2/5
预计耗时
1-3 小时
新手友好度
62/100
Issue 类型
文档
描述清晰度
基本清楚
活跃度
冷清
技术栈
python
领域
documentation

调研方向

阅读require_tls文档以及DAVClient和AsyncDAVClient构造函数的docstring；issue未指定具体文件。明确该选项目前适用于RFC 6764发现，而不适用于显式提供的URL，并且全局强制执行推迟到4.0。完成标准是文档说明的范围与当前行为一致。

由索引模型根据 Issue 内容生成。

描述

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 ⚠️

主要语言
Python
星标
412
派生
113
平均合并
8 小时 43 分钟
30 天内合并 PR
74

环境准备

  • 没有 Dockerfile 或 Docker Compose 文件
  • 没有 Pull Request 模板
  • 阅读贡献指南

从这里开始

  1. 先读完整个 Issue，再读项目的贡献指南。
  2. 在 Issue 下留言说明你要接手 —— 这能避免两个人做同样的事。
  3. Fork 仓库，在一个分支上完成修改。
  4. 提交 Pull Request，并在描述里引用这个 Issue 编号。

python-caldav/caldav 的其他 Issue

查看 python-caldav/caldav 的全部 Issue

相似的 Issue

更多 Python Issue

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。