Skip to content

Commit

Permalink
Pull request: 4942 cache poisoning
Browse files Browse the repository at this point in the history
Merge in DNS/dnsproxy from 4942-cache-poisoning to master

Updates AdguardTeam/AdGuardHome#4942.

Squashed commit of the following:

commit 32f384d
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 10 19:54:36 2022 +0300

    proxy: fix docs

commit 8d00f83
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 10 18:50:47 2022 +0300

    proxy: imp doc

commit 673eb98
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 10 18:06:22 2022 +0300

    proxy: imp more

commit f827903
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 10 17:18:58 2022 +0300

    proxy: imp cyclo, docs

commit e8ca674
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 10 16:21:24 2022 +0300

    proxy: imp code

commit 8e6f4eb
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Mon Oct 10 15:56:38 2022 +0300

    proxy: ignore cd

commit c8b5d1a
Author: Eugene Burkov <E.Burkov@AdGuard.COM>
Date:   Fri Oct 7 12:51:46 2022 +0300

    proxy: cache servfails
  • Loading branch information
EugeneOne1 committed Oct 10, 2022
1 parent 1250a0b commit cff4563
Show file tree
Hide file tree
Showing 4 changed files with 179 additions and 131 deletions.
126 changes: 91 additions & 35 deletions proxy/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"sync"
"time"

"github.com/AdguardTeam/dnsproxy/upstream"
glcache "github.com/AdguardTeam/golibs/cache"
"github.com/AdguardTeam/golibs/log"
"github.com/miekg/dns"
Expand Down Expand Up @@ -42,10 +43,34 @@ type cacheItem struct {
// m contains the cached response.
m *dns.Msg

// ttl is the time-to-live value for the item. Should be set before calling
// [cacheItem.pack].
ttl uint32

// u contains an address of the upstream which resolved m.
u string
}

// respToItem converts the pair of the response and upstream resolved the one
// into item for storing it in cache.
func respToItem(m *dns.Msg, u upstream.Upstream) (item *cacheItem) {
ttl := cacheTTL(m)
if ttl == 0 {
return nil
}

upsAddr := ""
if u != nil {
upsAddr = u.Address()
}

return &cacheItem{
m: m,
ttl: ttl,
u: upsAddr,
}
}

const (
// packedMsgLenSz is the exact length of byte slice capable to store the
// length of packed DNS message. It's essentially the size of a uint16.
Expand All @@ -65,7 +90,7 @@ func (ci *cacheItem) pack() (packed []byte) {
packed = make([]byte, minPackedLen, minPackedLen+pmLen+len(ci.u))

// Put expiration time.
binary.BigEndian.PutUint32(packed, uint32(time.Now().Unix())+lowestTTL(ci.m))
binary.BigEndian.PutUint32(packed, uint32(time.Now().Unix())+ci.ttl)

// Put the length of the packed message.
binary.BigEndian.PutUint16(packed[expTimeSz:], uint16(pmLen))
Expand Down Expand Up @@ -247,15 +272,16 @@ func (c *cache) createCache() (glc glcache.Cache) {
}

// set tries to add the ci into cache.
func (c *cache) set(ci *cacheItem) {
if !isCacheable(ci.m) {
func (c *cache) set(m *dns.Msg, u upstream.Upstream) {
item := respToItem(m, u)
if item == nil {
return
}

c.initLazy()

key := msgToKey(ci.m)
packed := ci.pack()
key := msgToKey(m)
packed := item.pack()

c.itemsLock.RLock()
defer c.itemsLock.RUnlock()
Expand All @@ -265,63 +291,69 @@ func (c *cache) set(ci *cacheItem) {

// setWithSubnet tries to add the ci into cache with subnet and ip used to
// calculate the key.
func (c *cache) setWithSubnet(ci *cacheItem, subnet *net.IPNet) {
if !isCacheable(ci.m) {
func (c *cache) setWithSubnet(m *dns.Msg, u upstream.Upstream, subnet *net.IPNet) {
item := respToItem(m, u)
if item == nil {
return
}

c.initLazyWithSubnet()

pref, _ := subnet.Mask.Size()
key := msgToKeyWithSubnet(ci.m, subnet.IP, pref)
packed := ci.pack()
key := msgToKeyWithSubnet(m, subnet.IP, pref)
packed := item.pack()

c.itemsWithSubnetLock.RLock()
defer c.itemsWithSubnetLock.RUnlock()

c.itemsWithSubnet.Set(key, packed)
}

// isCacheable checks if m is valid to be cached. For negative answers it
// follows RFC 2308 on how to cache NXDOMAIN and NODATA kinds of responses.
// cacheTTL returns the number of seconds for which m is valid to be cached.
// For negative answers it follows RFC 2308 on how to cache NXDOMAIN and NODATA
// kinds of responses.
//
// See https://datatracker.ietf.org/doc/html/rfc2308#section-2.1,
// https://datatracker.ietf.org/doc/html/rfc2308#section-2.2.
func isCacheable(m *dns.Msg) bool {
func cacheTTL(m *dns.Msg) (ttl uint32) {
switch {
case m == nil:
return false
return 0
case m.Truncated:
log.Tracef("refusing to cache truncated message")

return false
return 0
case len(m.Question) != 1:
log.Tracef("refusing to cache message with wrong number of questions")

return false
case lowestTTL(m) == 0:
return false
return 0
default:
ttl = calculateTTL(m)
if ttl == 0 {
return 0
}
}

qName := m.Question[0].Name
switch rcode := m.Rcode; rcode {
case dns.RcodeSuccess:
if qType := m.Question[0].Qtype; qType != dns.TypeA && qType != dns.TypeAAAA {
return true
if isCacheableSuccceded(m) {
return ttl
}

return hasIPAns(m) || isCacheableNegative(m)
case dns.RcodeNameError:
return isCacheableNegative(m)
if isCacheableNegative(m) {
return ttl
}
case dns.RcodeServerFailure:
return ttl
default:
log.Tracef(
"%s: refusing to cache message with response code %s",
qName,
m.Question[0].Name,
dns.RcodeToString[rcode],
)

return false
}

return 0
}

// hasIPAns check the m for containing at least one A or AAAA RR in answer
Expand All @@ -336,6 +368,14 @@ func hasIPAns(m *dns.Msg) (ok bool) {
return false
}

// isCacheableSuccceded returns true if m contains useful data to be cached
// treating it as a succeesful response.
func isCacheableSuccceded(m *dns.Msg) (ok bool) {
qType := m.Question[0].Qtype

return (qType != dns.TypeA && qType != dns.TypeAAAA) || hasIPAns(m) || isCacheableNegative(m)
}

// isCacheableNegative returns true if m's header has at least a single SOA RR
// and no NS records so that it can be declared authoritative.
//
Expand All @@ -357,22 +397,38 @@ func isCacheableNegative(m *dns.Msg) (ok bool) {
return ok
}

// lowestTTL returns the lowest TTL in m's RRs or 0 if the information is
// absent.
func lowestTTL(m *dns.Msg) (ttl uint32) {
// ServFailMaxCacheTTL is the maximum time-to-live value for caching
// SERVFAIL responses in seconds. It's consistent with the upper constraint
// of 5 minutes given by RFC 2308.
//
// See https://datatracker.ietf.org/doc/html/rfc2308#section-7.1.
const ServFailMaxCacheTTL = 30

// calculateTTL returns the number of seconds for which m could be cached. It's
// usually the lowest TTL among all m's resource records. It returns 0 if m
// isn't cacheable according to it's contents.
func calculateTTL(m *dns.Msg) (ttl uint32) {
// Use the maximum value as a guard value. If the inner loop is entered,
// it's going to be rewritten with an actual TTL value that is lower than
// MaxUint32. If the inner loop isn't entered, catch that and return zero.
ttl = math.MaxUint32

for _, rrset := range [...][]dns.RR{m.Answer, m.Ns, m.Extra} {
for _, r := range rrset {
ttl = minTTL(r.Header(), ttl)
for _, rr := range rrset {
ttl = minTTL(rr.Header(), ttl)
if ttl == 0 {
return 0
}
}
}

if ttl == math.MaxUint32 {
switch {
case m.Rcode == dns.RcodeServerFailure && ttl > ServFailMaxCacheTTL:
return ServFailMaxCacheTTL
case ttl == math.MaxUint32:
return 0
default:
return ttl
}

return ttl
}

// minTTL returns the minimum of h's ttl and the passed ttl.
Expand Down
Loading

0 comments on commit cff4563

Please sign in to comment.