Skip to content

Commit

Permalink
feat: NSEC3 validation for DS records
Browse files Browse the repository at this point in the history
  • Loading branch information
developStorm committed Dec 14, 2024
1 parent 5a7ce49 commit ea40f42
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 49 deletions.
16 changes: 9 additions & 7 deletions src/zdns/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ func isCacheableType(ans WithBaseAnswer) bool {
//// TODO: this is overly broad right now and will unnecessarily cache some leaf A/AAAA records. However,

rrType := ans.BaseAns().RrType
return rrType == dns.TypeA || rrType == dns.TypeAAAA || rrType == dns.TypeNS || rrType == dns.TypeDNAME || rrType == dns.TypeCNAME || rrType == dns.TypeDS || rrType == dns.TypeDNSKEY
return rrType == dns.TypeA || rrType == dns.TypeAAAA || rrType == dns.TypeNS || rrType == dns.TypeDNAME || rrType == dns.TypeCNAME || rrType == dns.TypeDS || rrType == dns.TypeDNSKEY || rrType == dns.TypeNSEC || rrType == dns.TypeNSEC3
}

func (s *Cache) buildCachedResult(res *SingleQueryResult, depth int, layer string) *CachedResult {
Expand Down Expand Up @@ -289,9 +289,11 @@ func (s *Cache) SafeAddCachedAuthority(res *SingleQueryResult, ns *NameServer, d
}
authName := ""
for _, auth := range res.Authorities {
castAuth, ok := auth.(WithBaseAnswer)
castAuth, ok := auth.(Answer)
if !ok {
// if we can't cast, it won't be added to the cache. We'll log in buildCachedResult
// if we can't cast, it won't be added to the cache
// unless it's DS, NSEC, or NSEC3 (which may be under different names so checking for poison doesn't make sense)
// We'll log in buildCachedResult
continue
}
baseAns := castAuth.BaseAns()
Expand All @@ -316,10 +318,10 @@ func (s *Cache) SafeAddCachedAuthority(res *SingleQueryResult, ns *NameServer, d
delegateToDSRRs := make(map[string][]interface{})
var otherRRs []interface{}
for _, rr := range res.Authorities {
if dsRR, ok := rr.(DSAnswer); ok {
delegateName := removeTrailingDotIfNotRoot(dsRR.BaseAns().Name)
delegateToDSRRs[delegateName] = append(delegateToDSRRs[delegateName], dsRR)
} else {
switch rr := rr.(type) {
case DSAnswer, NSECAnswer, NSEC3Answer:
delegateToDSRRs[authName] = append(delegateToDSRRs[authName], rr)
default:
otherRRs = append(otherRRs, rr)
}
}
Expand Down
118 changes: 78 additions & 40 deletions src/zdns/dnssec.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ package zdns

import (
"fmt"
"slices"
"strings"
"time"

Expand All @@ -45,6 +46,8 @@ const (
keySigningKeyFlag = 257
)

const NSEC3OptOutFlag = 0x01

// validate performs DNSSEC validation for all sections of a DNS message.
// It validates the Answer, Additional, and Authority sections independently,
// collects all encountered DS and DNSKEY records, and determines the overall
Expand All @@ -71,13 +74,28 @@ func (v *dNSSECValidator) validate(layer string, msg *dns.Msg, nameServer *NameS
return result, trace
}

v.resetDNSSECValidator(msg, nameServer)

if !hasRRSIG(v.msg) {
v.r.verboseLog(depth, "DNSSEC: No RRSIG records found in message")
result.Status = DNSSECInsecure // This can't be secure, but it could be bogus instead
result.Reason = "No RRSIG records found in message"
dsRecords, hasNSECProof, newTrace, err := v.fetchDSRecords(dns.CanonicalName(layer), trace, depth)
trace = newTrace
if err != nil {
v.r.verboseLog(depth, "DNSSEC: Failed to fetch DS records for zone", layer, "err:", err)
result.Status = DNSSECIndeterminate
result.Reason = err.Error()
} else if hasNSECProof {
v.r.verboseLog(depth, "DNSSEC: NSEC proof found for DS non-existence in zone", layer)
result.Status = DNSSECInsecure
result.Reason = ""
} else if len(dsRecords) == 0 {
v.r.verboseLog(depth, "DNSSEC: No DS records found for zone", layer)
result.Status = DNSSECIndeterminate
result.Reason = "No delegation and no NSEC attesting to the non-existence"
} else if !hasRRSIG(msg) {
v.r.verboseLog(depth, "DNSSEC: DS records found for zone", layer, ", but no RRSIG records found in message")
result.Status = DNSSECBogus
result.Reason = "DS exists but no RRSIG records found in message"
} else {
// Has DS and RRSIG, continue with validation
v.resetDNSSECValidator(msg, nameServer)

// Validate the answer section
var sectionRes []DNSSECPerSetResult
sectionRes, trace = v.validateSection(v.msg.Answer, depth, trace)
Expand Down Expand Up @@ -110,7 +128,7 @@ func (v *dNSSECValidator) validate(layer string, msg *dns.Msg, nameServer *NameS

// DNSKEY/DS queries may have failed, so we need to check the status again here
if v.status != DNSSECSecure {
result := makeDNSSECResult()
result = makeDNSSECResult()
result.Status = v.status
result.Reason = v.reason
return result, trace
Expand Down Expand Up @@ -362,9 +380,14 @@ func (v *dNSSECValidator) getDNSKEYs(signerDomain string, trace Trace, depth int
}

// fetchDSRecords retrieves DS records for a given signer domain
func (v *dNSSECValidator) fetchDSRecords(signerDomain string, trace Trace, depth int) (map[uint16]dns.DS, Trace, error) {
func (v *dNSSECValidator) fetchDSRecords(signerDomain string, trace Trace, depth int) (map[uint16]dns.DS, bool, Trace, error) {
nameWithoutTrailingDot := removeTrailingDotIfNotRoot(signerDomain)

if signerDomain == rootZone {
// Root zone, use the root anchors
return rootanchors.GetValidDSRecords(), false, trace, nil
}

dsQuestion := QuestionWithMetadata{
Q: Question{
Name: nameWithoutTrailingDot,
Expand All @@ -374,44 +397,55 @@ func (v *dNSSECValidator) fetchDSRecords(signerDomain string, trace Trace, depth
RetriesRemaining: &v.r.retriesRemaining,
}

dsRecords := make(map[uint16]dns.DS)
if signerDomain == rootZone {
// Root zone, use the root anchors
dsRecords = rootanchors.GetValidDSRecords()
} else {
// DNSSECResult may be nil if the response is from the cache.
res, newTrace, status, err := v.r.lookup(v.ctx, &dsQuestion, v.r.rootNameServers, v.isIterative, trace)
trace = newTrace
if status != StatusNoError {
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: Failed to get DS records for signer domain %s, query status: %s", signerDomain, status))
return nil, trace, fmt.Errorf("DS fetch failed, query status: %s", status)
} else if err != nil {
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: Failed to get DS records for signer domain %s, err: %v", signerDomain, err))
return nil, trace, fmt.Errorf("DS fetch failed, err: %v", err)
} else if res.DNSSECResult != nil && res.DNSSECResult.Status != DNSSECSecure {
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: Failed to get DS records for signer domain %s, DNSSEC status: %s", signerDomain, res.DNSSECResult.Status))

if prevResult := getResultForRRset(RRsetKey(dsQuestion.Q), res.DNSSECResult.Answer); prevResult != nil && prevResult.Error != "" {
return nil, trace, fmt.Errorf("DS fetch failed: %s", prevResult.Error)
} else {
return nil, trace, errors.New(res.DNSSECResult.Reason)
}
res, newTrace, status, err := v.r.lookup(v.ctx, &dsQuestion, v.r.rootNameServers, v.isIterative, trace)
trace = newTrace
if status != StatusNoError {
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: Failed to get DS records for signer domain %s, query status: %s", signerDomain, status))
return nil, false, trace, fmt.Errorf("DS fetch failed, query status: %s", status)
} else if err != nil {
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: Failed to get DS records for signer domain %s, err: %v", signerDomain, err))
return nil, false, trace, fmt.Errorf("DS fetch failed, err: %v", err)
} else if res.DNSSECResult != nil && res.DNSSECResult.Status != DNSSECSecure {
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: Failed to get DS records for signer domain %s, DNSSEC status: %s", signerDomain, res.DNSSECResult.Status))

if prevResult := getResultForRRset(RRsetKey(dsQuestion.Q), res.DNSSECResult.Answer); prevResult != nil && prevResult.Error != "" {
return nil, false, trace, fmt.Errorf("DS fetch failed: %s", prevResult.Error)
} else {
return nil, false, trace, errors.New(res.DNSSECResult.Reason)
}
}

// RRSIGs of res should have been verified before returning to here.
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: DS record response for signer domain %s: %v", signerDomain, res.Answers))

for _, rr := range res.Answers {
zTypedDS, ok := rr.(DSAnswer)
if !ok {
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: Non-DS RR type in DS answer: %v", rr))
continue
// Check for NSEC3 records in authority section that prove DS non-existence
for _, rr := range res.Answers {
if zTypedNSEC3, ok := rr.(NSEC3Answer); ok {
nsec3 := zTypedNSEC3.ToVanillaType()
if nsec3.Flags&NSEC3OptOutFlag == 1 && nsec3.Cover(signerDomain) {
// Opt-out NSEC3 record covering the signer domain
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: Found covering NSEC3 proving DS non-existence for %s", signerDomain))

Check failure on line 426 in src/zdns/dnssec.go

View workflow job for this annotation

GitHub Actions / lint

fmt.Sprintf can be replaced with string concatenation (perfsprint)
return nil, true, trace, nil
} else if nsec3.Match(signerDomain) && !slices.Contains(nsec3.TypeBitMap, dns.TypeDS) {
// NSEC3 record directly matching the signer domain and proving DS non-existence
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: Found matching NSEC3 proving DS non-existence for %s", signerDomain))

Check failure on line 430 in src/zdns/dnssec.go

View workflow job for this annotation

GitHub Actions / lint

fmt.Sprintf can be replaced with string concatenation (perfsprint)
return nil, true, trace, nil
}
ds := zTypedDS.ToVanillaType()
dsRecords[ds.KeyTag] = *ds
}
}

return dsRecords, trace, nil
// Process DS records from answer section
dsRecords := make(map[uint16]dns.DS)
for _, rr := range res.Answers {
zTypedDS, ok := rr.(DSAnswer)
if !ok {
v.r.verboseLog(depth, fmt.Sprintf("DNSSEC: Non-DS RR type in DS answer: %v", rr))
continue
}
ds := zTypedDS.ToVanillaType()
dsRecords[ds.KeyTag] = *ds
}

return dsRecords, false, trace, nil
}

// findSEPs validates DS records against DNSKEY records,
Expand All @@ -428,11 +462,15 @@ func (v *dNSSECValidator) fetchDSRecords(signerDomain string, trace Trace, depth
// - Trace: Updated trace context
// - error: If validation fails for any DS record
func (v *dNSSECValidator) findSEPs(signerDomain string, dnskeyMap map[uint16]*dns.DNSKEY, trace Trace, depth int) (map[uint16]*dns.DNSKEY, Trace, error) {
dsRecords, trace, err := v.fetchDSRecords(signerDomain, trace, depth)
dsRecords, hasNSECProof, trace, err := v.fetchDSRecords(signerDomain, trace, depth)
if err != nil {
return nil, trace, err
}

if hasNSECProof {
return nil, trace, errors.New("NSEC indicates no DS records should exist")
}

sepKeys := make(map[uint16]*dns.DNSKEY)
for _, key := range dnskeyMap {
authenticDS, ok := dsRecords[key.KeyTag()]
Expand Down
4 changes: 2 additions & 2 deletions src/zdns/lookup.go
Original file line number Diff line number Diff line change
Expand Up @@ -915,9 +915,9 @@ func (r *Resolver) iterateOnAuthorities(ctx context.Context, qWithMeta *Question
})

for _, elem := range authorities {
// Skip DS and RRSIG records
// Skip DNSSEC records
switch elem.(type) {
case DSAnswer, RRSIGAnswer:
case DSAnswer, RRSIGAnswer, NSECAnswer, NSEC3Answer:
continue
}

Expand Down

0 comments on commit ea40f42

Please sign in to comment.