Fix SERVFAIL on DNSSEC-OK query landing on an NSEC zone's empty non-terminal - #2110
Open
Hemsby wants to merge 1 commit into
Open
Conversation
InternalQuery's zone-not-found branch already computes hasSubDomains to correctly keep rCode at NoError for a genuine empty non-terminal (RFC 4035 2.3 forbids creating an NSEC record there), but it always ran the full NXDOMAIN-shaped proof, which after proving the qname itself has no RRset also unconditionally tries to disprove a covering wildcard computed from the qname. When the ENT is the parent of a real wildcard record, that computed wildcard name genuinely exists, and asserting its non-existence throws instead of answering. Pass hasSubDomains as the isWildcardAnswer argument so the ENT/NODATA case skips that inapplicable second step, matching how the same flag is already reused elsewhere (GetNSecProofOfWildcardAnswer). NSEC3 is unaffected: an ENT always gets its own NSEC3 record (RFC 5155 7.1), so it never reaches this branch. Confirmed present unmodified on master, develop, and stats-mem-issue. Validated with delv: the previously SERVFAILing query now returns a fully validated NODATA response using the correct covering NSEC record.
Member
|
Thanks for the PR. Will study this soon. |
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
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.
Bug
Any zone signed with classic NSEC (not NSEC3) that has an empty
non-terminal - a name with real subdomains but no RRset of its own -
returns SERVFAIL for a DNSSEC-OK query that lands exactly on that name for
a type it doesn't have (DS, NSEC, or anything else it lacks). Confirmed
present, unmodified, on master, develop, and stats-mem-issue, so it's not
related to any in-progress branch work.
The easiest way to create such an ENT is a wildcard record, since the
wildcard's owner name (
*.foo.example.com) makes its parent(
foo.example.com) an ENT if nothing else lives there. That's how I foundit: a query for the parent name's DS record while validating an unrelated
change crashed the whole answer instead of returning NODATA.
Root cause
AuthZoneManager.InternalQuery's "zone not found" branch already computeshasSubDomainsand correctly keepsrCode = NoErrorwhen the queried nameis a genuine ENT rather than truly nonexistent. But the DNSSEC proof it
attaches ignores that distinction: it always calls the NXDOMAIN-shaped
proof function, which after proving the qname itself has no NSEC entry,
unconditionally also tries to disprove a covering wildcard computed from
the qname. For an ENT that happens to be the parent of a real wildcard
record, that computed wildcard name genuinely exists in the zone, and the
proof-of-cover code correctly refuses to assert its non-existence - by
throwing, which surfaces as SERVFAIL instead of a clean answer.
Fix
One argument change: pass
hasSubDomainsinstead of a hardcodedfalsefor the
isWildcardAnswerparameter, so the ENT case skips thewildcard-disproof step it doesn't need. The codebase already reuses this
same flag for an analogous purpose elsewhere (
GetNSecProofOfWildcardAnswer),so this isn't introducing a new convention. NSEC3 zones are unaffected -
NSEC3 already gives every ENT its own record per RFC 5155 7.1, so a genuine
ENT never reaches this branch for NSEC3.
Testing
Reproduced on a live NSEC-signed zone with a wildcard-owned record creating
an ENT at its parent name. Before the fix,
dig +dnssecat the ENT namereturned SERVFAIL and the log showed
NSecAddProofOfCoverForthrowing "domain exists" for the wildcard name.After the fix,
delvagainst the same query reports "negative response,fully validated," using the correct covering NSEC record from the closest
real predecessor in the chain.