-
Notifications
You must be signed in to change notification settings - Fork 107
DNSSEC support #572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
tobixen
wants to merge
11
commits into
master
Choose a base branch
from
issue571
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
DNSSEC support #572
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Implements automatic service discovery allowing users to provide just a domain name or email address instead of full URLs. The implementation follows RFC 6764 specification for locating CalDAV and CardDAV services. Features: - DNS SRV record lookup (_caldavs._tcp / _caldav._tcp) - DNS TXT record lookup for service path information - Well-known URI fallback (/.well-known/caldav) - Automatic TLS preference with fallback to non-TLS - Graceful fallback to feature hints if discovery fails - New caldav.discovery module with comprehensive API - New enable_rfc6764 parameter for DAVClient (default: True) Usage examples: DAVClient(url='user@example.com', ...) # Email-based discovery DAVClient(url='calendar.example.com', ...) # Domain-based discovery DAVClient(url='example.com', enable_rfc6764=False) # Disable discovery Changes: - Added caldav/discovery.py with complete RFC 6764 implementation - Enhanced caldav/davclient.py _auto_url() with discovery integration - Added enable_rfc6764 parameter to DAVClient.__init__() - Added dnspython as required dependency in pyproject.toml - Updated CHANGELOG.md with feature documentation - Added RFC6764_IMPLEMENTATION.md for detailed documentation - Added example_rfc6764_usage.py demonstrating all usage patterns Backward compatibility: Fully maintained. Discovery only activates for bare domains/emails. Existing code with full URLs or feature hints works unchanged. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
When users provide an email address (user@example.com) for discovery, the username is now preserved and used if no explicit username parameter is provided to DAVClient. This allows for more convenient usage: DAVClient(url='user@example.com', password='pass') instead of: DAVClient(url='user@example.com', username='user', password='pass') Changes: - Updated _extract_domain() to return (domain, username) tuple - Added username field to ServiceInfo dataclass - Modified discover_service() to preserve username through discovery - Updated _auto_url() to return (url, username) tuple - Modified DAVClient.__init__() to use discovered username if no explicit username was provided - Updated example_rfc6764_usage.py to demonstrate username extraction Behavior: - Email address: username extracted automatically - Bare domain: no username extraction - Explicit username parameter: always takes precedence - URL with embedded credentials: existing behavior unchanged 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
When no URL is provided but username contains @, RFC6764 discovery will automatically attempt to discover the CalDAV service using the username as an email address. This enables the most convenient usage pattern: DAVClient(username='user@example.com', password='pass') Changes: - Added username parameter to _auto_url() function - When url is empty and username contains @, use username for discovery - Updated DAVClient.__init__() to pass username to _auto_url() - Enhanced docstring to document username-based discovery - Added Example 2 to example_rfc6764_usage.py demonstrating this pattern - Renumbered subsequent examples Usage patterns now supported: 1. url='user@example.com' (email in URL) 2. username='user@example.com' (email in username, no URL) 3. url='example.com', username='user' (domain + separate username) All three patterns support RFC6764 discovery and username extraction. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
SECURITY: RFC6764 DNS discovery is vulnerable to attacks if DNS is not secured with DNSSEC. This commit implements critical security mitigations. Attack vectors: - DNS Spoofing: Attackers can provide malicious SRV/TXT records - Downgrade Attacks: Malicious DNS can force plaintext HTTP connections - Man-in-the-Middle: Even with HTTPS, attackers can redirect traffic Mitigations implemented: 1. New require_tls parameter (default: True) - ONLY accepts HTTPS, preventing downgrade attacks where malicious DNS specifies HTTP 2. Comprehensive security warnings in all documentation 3. Logging warnings when require_tls=False (insecure mode) 4. Default behavior is now secure by design Changes: - Added require_tls parameter to all discovery functions (default: True) - Updated DAVClient to accept and pass through require_tls - Added require_tls to CONNKEYS for configuration support - Enhanced module docstring with SECURITY CONSIDERATIONS section - Added detailed security warnings to function docstrings - Updated RFC6764_IMPLEMENTATION.md with critical security section - Updated CHANGELOG.md with security notes - Added logging: DEBUG when require_tls=True, WARNING when False Breaking change: None - secure defaults protect users automatically Users requiring HTTP must explicitly set require_tls=False Addresses security concern raised about DNS spoofing and downgrade attacks. DNSSEC validation will be implemented in a future commit/branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Implements DNSSEC validation to cryptographically verify DNS responses during RFC 6764 service discovery, protecting against DNS spoofing attacks. Changes: - Added verify_dnssec parameter (default: False) to discovery functions - Implemented _validate_dnssec() helper to check for RRSIG records - Updated _srv_lookup() and _txt_lookup() with DNSSEC validation - Added verify_dnssec to DAVClient constructor and CONNKEYS - Added auto-connect.verify_dnssec to compatibility_hints - Created comprehensive test suite (tests/test_dnssec_discovery.py) - Updated documentation with DNSSEC usage and security notes - Enhanced examples/rfc6764_test_conf.py to test DNSSEC validation Security: When verify_dnssec=True, DNS queries request EDNS0 with DO flag and AD flag, then validate that responses contain DNSSEC signatures (RRSIG records). Discovery fails with DiscoveryError if signatures are missing or invalid. This provides cryptographic proof against DNS tampering but requires: - DNSSEC-enabled domains with properly configured DS, DNSKEY, and RRSIG records - DNSSEC-capable recursive resolver Addresses issue #571 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Bug: When verify_dnssec=True but no SRV records exist, discovery would fall back to well-known URI lookup without any DNSSEC validation, incorrectly reporting successful DNSSEC validation. Fix: When verify_dnssec=True and SRV lookup fails (no DNS records to validate), raise DiscoveryError instead of falling back to well-known URI. Rationale: Well-known URI discovery uses HTTPS/TLS, not DNS lookups, so DNSSEC cannot validate this discovery method. If DNSSEC validation is explicitly requested, we should fail when DNS records don't exist rather than silently bypassing validation. This ensures that verify_dnssec=True actually enforces DNSSEC validation and doesn't give false positives for domains without DNSSEC-signed DNS records. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Clarifies that DNSSEC validation only applies to DNS-based discovery (SRV/TXT records), not well-known URI discovery which uses HTTPS/TLS. When verify_dnssec=True and no SRV records exist, discovery correctly fails rather than falling back to unvalidated well-known URI discovery. Also documents future enhancement possibility of implementing a custom DNS resolver for niquests/urllib3_future to validate DNSSEC for the A/AAAA records used in HTTPS connections, though this is complex and beyond current scope. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Major enhancement: When verify_dnssec=True, now validates DNSSEC for ALL DNS lookups, not just RFC 6764 SRV/TXT records. Implementation: - DAVClient now uses DoH (DNS-over-HTTPS) resolver when verify_dnssec=True - Leverages niquests' built-in DNSSEC support via custom resolvers - Uses Cloudflare's 1.1.1.1 DoH service for DNSSEC validation - Applies to both discovery AND all subsequent CalDAV requests Benefits: ✅ Complete DNSSEC coverage for all DNS operations ✅ Works with well-known URI discovery (no longer fails) ✅ Validates A/AAAA records for HTTPS connections ✅ No bypasses or gaps in protection ✅ Seamless integration with niquests/urllib3_future Discovery: Niquests documentation states that custom resolvers automatically provide DNSSEC validation, eliminating the need for complex custom resolver implementation. Updated documentation to reflect comprehensive DNSSEC coverage. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Member
Author
|
I'm not much happy with this
Everything considered, I think I will probably not include DNSSEC support in the next release. |
Member
Author
|
DNSSEC may be considered for some future version after 3.0. In the very best case, 2026-Q2, but possibly never. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ref #571