Files
Gondulf/docs/designs/dns-verification-bug-fix.md
Phil Skentelbery 1ef5cd9229 fix(dns): query _gondulf subdomain for domain verification
The DNS TXT verification was querying the base domain instead of
_gondulf.{domain}, causing verification to always fail even when
users had correctly configured their DNS records.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
2025-11-22 17:46:38 -07:00

7.5 KiB

DNS Verification Bug Fix Design

Purpose

Fix critical bug in DNS TXT record verification where the code queries the wrong domain location, preventing successful domain verification even when users have correctly configured their DNS records.

Problem Statement

Current Incorrect Behavior

The DNS verification service currently queries the wrong domain for TXT records:

  1. User instructions (correctly shown in template): Set TXT record at _gondulf.{domain}
  2. User action: Creates TXT record at _gondulf.thesatelliteoflove.com with value gondulf-verify-domain
  3. Code behavior (INCORRECT): Queries thesatelliteoflove.com instead of _gondulf.thesatelliteoflove.com
  4. Result: Verification always fails

Root Cause

In src/gondulf/dns.py, the verify_txt_record method passes the domain directly to get_txt_records, which then queries that exact domain. The calling code in src/gondulf/routers/authorization.py also passes just the base domain without the _gondulf. prefix.

Design Overview

The fix requires modifying the DNS verification logic to correctly prefix the domain with _gondulf. when querying TXT records for Gondulf domain verification purposes.

Component Details

1. DNSService Updates (src/gondulf/dns.py)

Update the verify_txt_record method to handle Gondulf-specific verification by prefixing the domain:

def verify_txt_record(self, domain: str, expected_value: str) -> bool:
    """
    Verify that domain has a TXT record with the expected value.

    For Gondulf domain verification (expected_value="gondulf-verify-domain"),
    queries the _gondulf.{domain} subdomain as per specification.

    Args:
        domain: Domain name to verify (e.g., "example.com")
        expected_value: Expected TXT record value

    Returns:
        True if expected value found in TXT records, False otherwise
    """
    try:
        # For Gondulf domain verification, query _gondulf subdomain
        if expected_value == "gondulf-verify-domain":
            query_domain = f"_gondulf.{domain}"
        else:
            query_domain = domain

        txt_records = self.get_txt_records(query_domain)

        # Check if expected value is in any TXT record
        for record in txt_records:
            if expected_value in record:
                logger.info(
                    f"TXT record verification successful for domain={domain} "
                    f"(queried {query_domain})"
                )
                return True

        logger.debug(
            f"TXT record verification failed: expected value not found "
            f"for domain={domain} (queried {query_domain})"
        )
        return False

    except DNSError as e:
        logger.warning(f"TXT record verification failed for domain={domain}: {e}")
        return False

Add a new method specifically for Gondulf verification:

def verify_gondulf_domain(self, domain: str) -> bool:
    """
    Verify Gondulf domain ownership via TXT record at _gondulf.{domain}.

    Args:
        domain: Domain name to verify (e.g., "example.com")

    Returns:
        True if gondulf-verify-domain found in _gondulf.{domain} TXT records
    """
    gondulf_subdomain = f"_gondulf.{domain}"
    return self.verify_txt_record(gondulf_subdomain, "gondulf-verify-domain")

Recommendation: Use Option A. It keeps the fix localized to the DNS service and maintains backward compatibility while fixing the bug with minimal changes.

2. No Changes Required in Authorization Router

With Option A, no changes are needed in src/gondulf/routers/authorization.py since the fix is entirely contained within the DNS service. The existing call remains correct:

dns_verified = dns_service.verify_txt_record(domain, "gondulf-verify-domain")

3. Template Remains Correct

The template (src/gondulf/templates/verification_error.html) already shows the correct instructions and needs no changes.

Data Models

No data model changes required.

API Contracts

No API changes required. This is an internal bug fix.

Error Handling

DNS Query Errors

The existing error handling in get_txt_records is sufficient:

  • NXDOMAIN: Domain doesn't exist (including subdomain)
  • NoAnswer: No TXT records found
  • Timeout: DNS server timeout
  • Other DNS exceptions: General failure

All these cases correctly return False for verification failure.

Logging Updates

Update log messages to include which domain was actually queried:

  • Success: Include both the requested domain and the queried domain
  • Failure: Include both domains to aid debugging

Security Considerations

  1. No New Attack Vectors: The fix doesn't introduce new security concerns
  2. DNS Rebinding: Not applicable (we're only reading TXT records)
  3. Cache Poisoning: Existing DNS resolver safeguards apply
  4. Subdomain Takeover: The _gondulf prefix is specifically chosen to avoid conflicts

Testing Strategy

Unit Tests Required

  1. Test Gondulf domain verification with correct TXT record

    • Mock DNS response for _gondulf.example.com with value gondulf-verify-domain
    • Verify verify_txt_record("example.com", "gondulf-verify-domain") returns True
  2. Test Gondulf domain verification with missing TXT record

    • Mock DNS response for _gondulf.example.com with no TXT records
    • Verify verify_txt_record("example.com", "gondulf-verify-domain") returns False
  3. Test Gondulf domain verification with wrong TXT value

    • Mock DNS response for _gondulf.example.com with value wrong-value
    • Verify verify_txt_record("example.com", "gondulf-verify-domain") returns False
  4. Test non-Gondulf TXT verification still works

    • Mock DNS response for example.com (not prefixed) with value other-value
    • Verify verify_txt_record("example.com", "other-value") returns True
    • Ensures backward compatibility for any other TXT verification uses
  5. Test NXDOMAIN handling

    • Mock NXDOMAIN for _gondulf.example.com
    • Verify verify_txt_record("example.com", "gondulf-verify-domain") returns False

Integration Test

  1. End-to-end authorization flow test
    • Set up test domain with _gondulf.{domain} TXT record
    • Attempt authorization flow
    • Verify DNS verification passes

Manual Testing

  1. Configure real DNS record: _gondulf.yourdomain.com with value gondulf-verify-domain
  2. Test authorization flow
  3. Verify successful DNS verification
  4. Check logs show correct domain being queried

Acceptance Criteria

  1. DNS verification queries _gondulf.{domain} when verifying Gondulf domain ownership
  2. Users with correctly configured TXT records can successfully verify their domain
  3. Log messages clearly show which domain was queried for debugging
  4. Non-Gondulf TXT verification (if used elsewhere) continues to work
  5. All existing tests pass
  6. New unit tests cover the fix
  7. Manual testing confirms real DNS records work

Implementation Notes

  1. Critical Bug: This is a P0 bug that completely breaks domain verification
  2. Simple Fix: The fix is straightforward - just add the prefix when appropriate
  3. Test Thoroughly: While the fix is simple, ensure comprehensive testing
  4. Verify Logs: Update logging to be clear about what domain is being queried

Migration Considerations

None required. This is a bug fix that makes the code work as originally intended. No database migrations or data changes needed.