cache: default to DNSSEC (#4085)

* cache: default to DNSSEC

This change does away with the DNS/DNSSEC distinction the cache
currently makes. Cache will always make coredns perform a DNSSEC query
and store that result. If a client just needs plain DNS, the DNSSEC
records are stripped from the response.

It should also be more memory efficient, because we store a reply once
and not one DNS and another for DNSSEC.

Fixes: #3836

Signed-off-by: Miek Gieben <miek@miek.nl>

* Change OPT RR when one is present in the msg.

Signed-off-by: Miek Gieben <miek@miek.nl>

* Fix comment for isDNSSEC

Signed-off-by: Miek Gieben <miek@miek.nl>

* Update plugin/cache/handler.go

Co-authored-by: Chris O'Haver <cohaver@infoblox.com>

* Update plugin/cache/item.go

Co-authored-by: Chris O'Haver <cohaver@infoblox.com>

* Code review; fix comment for isDNSSEC

Signed-off-by: Miek Gieben <miek@miek.nl>

* Update doc and set AD to false

Set Authenticated Data to false when DNSSEC was not wanted. Also update
the readme with the new behavior.

Signed-off-by: Miek Gieben <miek@miek.nl>

* Update plugin/cache/handler.go

Co-authored-by: Chris O'Haver <cohaver@infoblox.com>

Co-authored-by: Chris O'Haver <cohaver@infoblox.com>
This commit is contained in:
Miek Gieben 2020-09-17 16:28:43 +02:00 committed by GitHub
parent 22b6846626
commit acf9a0fa19
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 207 additions and 47 deletions

View file

@ -10,6 +10,9 @@ With *cache* enabled, all records except zone transfers and metadata records wil
3600s. Caching is mostly useful in a scenario when fetching data from the backend (upstream,
database, etc.) is expensive.
*Cache* will change the query to enable DNSSEC (DNSSEC OK; DO) if it passes through the plugin. If
the client didn't request any DNSSEC (records), these are filtered out when replying.
This plugin can only be used once per Server Block.
## Syntax

64
plugin/cache/cache.go vendored
View file

@ -65,31 +65,21 @@ func New() *Cache {
// key returns key under which we store the item, -1 will be returned if we don't store the message.
// Currently we do not cache Truncated, errors zone transfers or dynamic update messages.
// qname holds the already lowercased qname.
func key(qname string, m *dns.Msg, t response.Type, do bool) (bool, uint64) {
func key(qname string, m *dns.Msg, t response.Type) (bool, uint64) {
// We don't store truncated responses.
if m.Truncated {
return false, 0
}
// Nor errors or Meta or Update
// Nor errors or Meta or Update.
if t == response.OtherError || t == response.Meta || t == response.Update {
return false, 0
}
return true, hash(qname, m.Question[0].Qtype, do)
return true, hash(qname, m.Question[0].Qtype)
}
var one = []byte("1")
var zero = []byte("0")
func hash(qname string, qtype uint16, do bool) uint64 {
func hash(qname string, qtype uint16) uint64 {
h := fnv.New64()
if do {
h.Write(one)
} else {
h.Write(zero)
}
h.Write([]byte{byte(qtype >> 8)})
h.Write([]byte{byte(qtype)})
h.Write([]byte(qname))
@ -152,14 +142,10 @@ func (w *ResponseWriter) RemoteAddr() net.Addr {
// WriteMsg implements the dns.ResponseWriter interface.
func (w *ResponseWriter) WriteMsg(res *dns.Msg) error {
do := false
mt, opt := response.Typify(res, w.now().UTC())
if opt != nil {
do = opt.Do()
}
mt, _ := response.Typify(res, w.now().UTC())
// key returns empty string for anything we don't want to cache.
hasKey, key := key(w.state.Name(), res, mt, do)
hasKey, key := key(w.state.Name(), res, mt)
msgTTL := dnsutil.MinimalTTL(res, mt)
var duration time.Duration
@ -187,18 +173,38 @@ func (w *ResponseWriter) WriteMsg(res *dns.Msg) error {
return nil
}
do := w.state.Do()
// Apply capped TTL to this reply to avoid jarring TTL experience 1799 -> 8 (e.g.)
// We also may need to filter out DNSSEC records, see toMsg() for similar code.
ttl := uint32(duration.Seconds())
for i := range res.Answer {
res.Answer[i].Header().Ttl = ttl
}
for i := range res.Ns {
res.Ns[i].Header().Ttl = ttl
}
for i := range res.Extra {
if res.Extra[i].Header().Rrtype != dns.TypeOPT {
res.Extra[i].Header().Ttl = ttl
j := 0
for _, r := range res.Answer {
if !do && isDNSSEC(r) {
continue
}
res.Answer[j].Header().Ttl = ttl
j++
}
res.Answer = res.Answer[:j]
j = 0
for _, r := range res.Ns {
if !do && isDNSSEC(r) {
continue
}
res.Ns[j].Header().Ttl = ttl
j++
}
res.Ns = res.Ns[:j]
j = 0
for _, r := range res.Extra {
if !do && isDNSSEC(r) {
continue
}
if res.Extra[j].Header().Rrtype != dns.TypeOPT {
res.Extra[j].Header().Ttl = ttl
}
j++
}
return w.ResponseWriter.WriteMsg(res)
}

View file

@ -46,17 +46,19 @@ var cacheTestCases = []cacheTestCase{
{
RecursionAvailable: true, AuthenticatedData: true,
Case: test.Case{
Qname: "mIEK.nL.", Qtype: dns.TypeMX,
Qname: "miek.nl.", Qtype: dns.TypeMX,
Answer: []dns.RR{
test.MX("mIEK.nL. 3600 IN MX 1 aspmx.l.google.com."),
test.MX("mIEK.nL. 3600 IN MX 10 aspmx2.googlemail.com."),
test.MX("miek.nl. 3600 IN MX 1 aspmx.l.google.com."),
test.MX("miek.nl. 3600 IN MX 10 aspmx2.googlemail.com."),
},
},
in: test.Case{
Qname: "mIEK.nL.", Qtype: dns.TypeMX,
Answer: []dns.RR{
test.MX("mIEK.nL. 3601 IN MX 1 aspmx.l.google.com."),
test.MX("mIEK.nL. 3601 IN MX 10 aspmx2.googlemail.com."),
test.MX("miek.nl. 3601 IN MX 1 aspmx.l.google.com."),
test.MX("miek.nl. 3601 IN MX 10 aspmx2.googlemail.com."),
// RRSIG must be here, because we are always doing DNSSEC lookups, and miek.nl MX is tested later in this list as well.
test.RRSIG("miek.nl. 3600 IN RRSIG MX 8 2 1800 20160521031301 20160421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="),
},
},
shouldCache: true,
@ -136,7 +138,7 @@ var cacheTestCases = []cacheTestCase{
test.RRSIG("miek.nl. 1800 IN RRSIG MX 8 2 1800 20160521031301 20160421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="),
},
},
shouldCache: false,
shouldCache: true,
},
{
RecursionAvailable: true,
@ -196,7 +198,7 @@ func TestCache(t *testing.T) {
state := request.Request{W: &test.ResponseWriter{}, Req: m}
mt, _ := response.Typify(m, utc)
valid, k := key(state.Name(), m, mt, state.Do())
valid, k := key(state.Name(), m, mt)
if valid {
crr.set(m, k, mt, c.pttl)
@ -211,14 +213,16 @@ func TestCache(t *testing.T) {
}
if ok {
resp := i.toMsg(m, time.Now().UTC())
resp := i.toMsg(m, time.Now().UTC(), state.Do())
if err := test.Header(tc.Case, resp); err != nil {
t.Logf("Bla %v", resp)
t.Error(err)
continue
}
if err := test.Section(tc.Case, test.Answer, resp.Answer); err != nil {
t.Logf("Bla %v -- %v", test.Answer, resp.Answer)
t.Error(err)
}
if err := test.Section(tc.Case, test.Ns, resp.Ns); err != nil {

75
plugin/cache/do_test.go vendored Normal file
View file

@ -0,0 +1,75 @@
package cache
import (
"context"
"testing"
"github.com/coredns/coredns/plugin"
"github.com/coredns/coredns/plugin/pkg/dnstest"
"github.com/coredns/coredns/plugin/test"
"github.com/miekg/dns"
)
func TestDo(t *testing.T) {
// cache sets Do and requests that don't have them.
c := New()
c.Next = echoHandler()
req := new(dns.Msg)
req.SetQuestion("example.org.", dns.TypeA)
rec := dnstest.NewRecorder(&test.ResponseWriter{})
// No DO set.
c.ServeDNS(context.TODO(), rec, req)
reply := rec.Msg
opt := reply.Extra[len(reply.Extra)-1]
if x, ok := opt.(*dns.OPT); !ok {
t.Fatalf("Expected OPT RR, got %T", x)
}
if !opt.(*dns.OPT).Do() {
t.Errorf("Expected DO bit to be set, got false")
}
if x := opt.(*dns.OPT).UDPSize(); x != defaultUDPBufSize {
t.Errorf("Expected %d bufsize, got %d", defaultUDPBufSize, x)
}
// Do set - so left alone.
const mysize = defaultUDPBufSize * 2
setDo(req)
// set bufsize to something else than default to see cache doesn't touch it
req.Extra[len(req.Extra)-1].(*dns.OPT).SetUDPSize(mysize)
c.ServeDNS(context.TODO(), rec, req)
reply = rec.Msg
opt = reply.Extra[len(reply.Extra)-1]
if x, ok := opt.(*dns.OPT); !ok {
t.Fatalf("Expected OPT RR, got %T", x)
}
if !opt.(*dns.OPT).Do() {
t.Errorf("Expected DO bit to be set, got false")
}
if x := opt.(*dns.OPT).UDPSize(); x != mysize {
t.Errorf("Expected %d bufsize, got %d", mysize, x)
}
// edns0 set, but not DO, so _not_ left alone.
req.Extra[len(req.Extra)-1].(*dns.OPT).SetDo(false)
c.ServeDNS(context.TODO(), rec, req)
reply = rec.Msg
opt = reply.Extra[len(reply.Extra)-1]
if x, ok := opt.(*dns.OPT); !ok {
t.Fatalf("Expected OPT RR, got %T", x)
}
if !opt.(*dns.OPT).Do() {
t.Errorf("Expected DO bit to be set, got false")
}
if x := opt.(*dns.OPT).UDPSize(); x != defaultUDPBufSize {
t.Errorf("Expected %d bufsize, got %d", defaultUDPBufSize, x)
}
}
func echoHandler() plugin.Handler {
return plugin.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
w.WriteMsg(r)
return dns.RcodeSuccess, nil
})
}

View file

@ -15,6 +15,7 @@ import (
// ServeDNS implements the plugin.Handler interface.
func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
state := request.Request{W: w, Req: r}
do := state.Do()
zone := plugin.Zones(c.Zones).Matches(state.Name())
if zone == "" {
@ -22,15 +23,24 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
}
now := c.now().UTC()
server := metrics.WithServer(ctx)
// On cache miss, if the request has the OPT record and the DO bit set we leave the message as-is. If there isn't a DO bit
// set we will modify the request to _add_ one. This means we will always do DNSSEC lookups on cache misses.
// When writing to cache, any DNSSEC RRs in the response are written to cache with the response.
// When sending a response to a non-DNSSEC client, we remove DNSSEC RRs from the response. We use a 2048 buffer size, which is
// less than 4096 (and older default) and more than 1024 which may be too small. We might need to tweaks this
// value to be smaller still to prevent UDP fragmentation?
ttl := 0
i := c.getIgnoreTTL(now, state, server)
if i != nil {
ttl = i.ttl(now)
}
if i == nil {
if !do {
setDo(r)
}
crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state, server: server}
return plugin.NextOrFailure(c.Name(), c.Next, ctx, crr, r)
}
@ -40,11 +50,14 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg)
now = now.Add(time.Duration(ttl) * time.Second)
go func() {
r := r.Copy()
if !do {
setDo(r)
}
crr := &ResponseWriter{Cache: c, state: state, server: server, prefetch: true, remoteAddr: w.LocalAddr()}
plugin.NextOrFailure(c.Name(), c.Next, ctx, crr, r)
}()
}
resp := i.toMsg(r, now)
resp := i.toMsg(r, now, do)
w.WriteMsg(resp)
if c.shouldPrefetch(i, now) {
@ -80,7 +93,7 @@ func (c *Cache) shouldPrefetch(i *item, now time.Time) bool {
func (c *Cache) Name() string { return "cache" }
func (c *Cache) get(now time.Time, state request.Request, server string) (*item, bool) {
k := hash(state.Name(), state.QType(), state.Do())
k := hash(state.Name(), state.QType())
if i, ok := c.ncache.Get(k); ok && i.(*item).ttl(now) > 0 {
cacheHits.WithLabelValues(server, Denial).Inc()
@ -97,7 +110,7 @@ func (c *Cache) get(now time.Time, state request.Request, server string) (*item,
// getIgnoreTTL unconditionally returns an item if it exists in the cache.
func (c *Cache) getIgnoreTTL(now time.Time, state request.Request, server string) *item {
k := hash(state.Name(), state.QType(), state.Do())
k := hash(state.Name(), state.QType())
if i, ok := c.ncache.Get(k); ok {
ttl := i.(*item).ttl(now)
@ -118,7 +131,7 @@ func (c *Cache) getIgnoreTTL(now time.Time, state request.Request, server string
}
func (c *Cache) exists(state request.Request) *item {
k := hash(state.Name(), state.QType(), state.Do())
k := hash(state.Name(), state.QType())
if i, ok := c.ncache.Get(k); ok {
return i.(*item)
}
@ -127,3 +140,22 @@ func (c *Cache) exists(state request.Request) *item {
}
return nil
}
// setDo sets the DO bit and UDP buffer size in the message m.
func setDo(m *dns.Msg) {
o := m.IsEdns0()
if o != nil {
o.SetDo()
o.SetUDPSize(defaultUDPBufSize)
return
}
o = &dns.OPT{Hdr: dns.RR_Header{Name: ".", Rrtype: dns.TypeOPT}}
o.SetDo()
o.SetUDPSize(defaultUDPBufSize)
m.Extra = append(m.Extra, o)
}
// defaultUDPBufsize is the bufsize the cache plugin uses on outgoing requests that don't
// have an OPT RR.
const defaultUDPBufSize = 2048

48
plugin/cache/item.go vendored
View file

@ -55,7 +55,7 @@ func newItem(m *dns.Msg, now time.Time, d time.Duration) *item {
// So we're forced to always set this to 1; regardless if the answer came from the cache or not.
// On newer systems(e.g. ubuntu 16.04 with glib version 2.23), this issue is resolved.
// So we may set this bit back to 0 in the future ?
func (i *item) toMsg(m *dns.Msg, now time.Time) *dns.Msg {
func (i *item) toMsg(m *dns.Msg, now time.Time, do bool) *dns.Msg {
m1 := new(dns.Msg)
m1.SetReply(m)
@ -64,6 +64,9 @@ func (i *item) toMsg(m *dns.Msg, now time.Time) *dns.Msg {
// just set it to true.
m1.Authoritative = true
m1.AuthenticatedData = i.AuthenticatedData
if !do {
m1.AuthenticatedData = false // when DNSSEC was not wanted, it can't be authenticated data.
}
m1.RecursionAvailable = i.RecursionAvailable
m1.Rcode = i.Rcode
@ -72,19 +75,37 @@ func (i *item) toMsg(m *dns.Msg, now time.Time) *dns.Msg {
m1.Extra = make([]dns.RR, len(i.Extra))
ttl := uint32(i.ttl(now))
for j, r := range i.Answer {
j := 0
for _, r := range i.Answer {
if !do && isDNSSEC(r) {
continue
}
m1.Answer[j] = dns.Copy(r)
m1.Answer[j].Header().Ttl = ttl
j++
}
for j, r := range i.Ns {
m1.Answer = m1.Answer[:j]
j = 0
for _, r := range i.Ns {
if !do && isDNSSEC(r) {
continue
}
m1.Ns[j] = dns.Copy(r)
m1.Ns[j].Header().Ttl = ttl
j++
}
m1.Ns = m1.Ns[:j]
// newItem skips OPT records, so we can just use i.Extra as is.
for j, r := range i.Extra {
j = 0
for _, r := range i.Extra {
if !do && isDNSSEC(r) {
continue
}
m1.Extra[j] = dns.Copy(r)
m1.Extra[j].Header().Ttl = ttl
j++
}
m1.Extra = m1.Extra[:j]
return m1
}
@ -92,3 +113,22 @@ func (i *item) ttl(now time.Time) int {
ttl := int(i.origTTL) - int(now.UTC().Sub(i.stored).Seconds())
return ttl
}
// isDNSSEC returns true if r is a DNSSEC record. NSEC,NSEC3,DS and RRSIG/SIG
// are DNSSEC records. DNSKEYs is not in this list on the assumption that the
// client explictly asked for it.
func isDNSSEC(r dns.RR) bool {
switch r.Header().Rrtype {
case dns.TypeNSEC:
return true
case dns.TypeNSEC3:
return true
case dns.TypeDS:
return true
case dns.TypeRRSIG:
return true
case dns.TypeSIG:
return true
}
return false
}

View file

@ -144,7 +144,7 @@ func (r *Request) Family() int {
return 2
}
// Do returns if the request has the DO (DNSSEC OK) bit set.
// Do returns true if the request has the DO (DNSSEC OK) bit set.
func (r *Request) Do() bool {
if r.size != 0 {
return r.do