diff --git a/plugin/cache/README.md b/plugin/cache/README.md index ade647599..0c006869a 100644 --- a/plugin/cache/README.md +++ b/plugin/cache/README.md @@ -57,6 +57,7 @@ If monitoring is enabled (via the *prometheus* directive) then the following met * `coredns_cache_capacity{type}` - Total capacity of the cache by cache type. * `coredns_cache_hits_total{type}` - Counter of cache hits by cache type. * `coredns_cache_misses_total{}` - Counter of cache misses. +* `coredns_cache_drops_total{}` - Counter of dropped messages. Cache types are either "denial" or "success". diff --git a/plugin/cache/cache.go b/plugin/cache/cache.go index fd3292b47..911ea11b6 100644 --- a/plugin/cache/cache.go +++ b/plugin/cache/cache.go @@ -10,6 +10,7 @@ import ( "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/plugin/pkg/cache" "github.com/coredns/coredns/plugin/pkg/response" + "github.com/coredns/coredns/request" "github.com/miekg/dns" ) @@ -102,6 +103,7 @@ func hash(qname string, qtype uint16, do bool) uint32 { type ResponseWriter struct { dns.ResponseWriter *Cache + state request.Request prefetch bool // When true write nothing back to the client. } @@ -128,10 +130,15 @@ func (w *ResponseWriter) WriteMsg(res *dns.Msg) error { } if key != -1 && duration > 0 { - w.set(res, key, mt, duration) - cacheSize.WithLabelValues(Success).Set(float64(w.pcache.Len())) - cacheSize.WithLabelValues(Denial).Set(float64(w.ncache.Len())) + if w.state.Match(res) { + w.set(res, key, mt, duration) + cacheSize.WithLabelValues(Success).Set(float64(w.pcache.Len())) + cacheSize.WithLabelValues(Denial).Set(float64(w.ncache.Len())) + } else { + // Don't log it, but increment counter + cacheDrops.Inc() + } } if w.prefetch { diff --git a/plugin/cache/handler.go b/plugin/cache/handler.go index e579aaffc..c2efdc9c7 100644 --- a/plugin/cache/handler.go +++ b/plugin/cache/handler.go @@ -46,7 +46,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) // When prefetching we loose the item i, and with it the frequency // that we've gathered sofar. See we copy the frequencies info back // into the new item that was stored in the cache. - prr := &ResponseWriter{ResponseWriter: w, Cache: c, prefetch: true} + prr := &ResponseWriter{ResponseWriter: w, Cache: c, prefetch: true, state: state} plugin.NextOrFailure(c.Name(), c.Next, ctx, prr, r) if i1 := c.exists(qname, qtype, do); i1 != nil { @@ -58,7 +58,7 @@ func (c *Cache) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) return dns.RcodeSuccess, nil } - crr := &ResponseWriter{ResponseWriter: w, Cache: c} + crr := &ResponseWriter{ResponseWriter: w, Cache: c, state: state} return plugin.NextOrFailure(c.Name(), c.Next, ctx, crr, r) } @@ -127,6 +127,13 @@ var ( Name: "prefetch_total", Help: "The number of time the cache has prefetched a cached item.", }) + + cacheDrops = prometheus.NewCounter(prometheus.CounterOpts{ + Namespace: plugin.Namespace, + Subsystem: "cache", + Name: "drops_total", + Help: "The number responses that are not cached, because the reply is malformed.", + }) ) var once sync.Once diff --git a/plugin/cache/setup.go b/plugin/cache/setup.go index 4ea51d510..57edd3246 100644 --- a/plugin/cache/setup.go +++ b/plugin/cache/setup.go @@ -42,6 +42,7 @@ func setup(c *caddy.Controller) error { x.MustRegister(cacheHits) x.MustRegister(cacheMisses) x.MustRegister(cachePrefetches) + x.MustRegister(cacheDrops) } }) return nil diff --git a/plugin/cache/spoof_test.go b/plugin/cache/spoof_test.go new file mode 100644 index 000000000..c90aee817 --- /dev/null +++ b/plugin/cache/spoof_test.go @@ -0,0 +1,66 @@ +package cache + +import ( + "testing" + + "github.com/coredns/coredns/plugin" + "github.com/coredns/coredns/plugin/pkg/dnstest" + + "github.com/coredns/coredns/plugin/test" + "github.com/miekg/dns" + "golang.org/x/net/context" +) + +func TestSpoof(t *testing.T) { + // Send query for example.org, get reply for example.net; should not be cached. + c := New() + c.Next = spoofHandler() + + req := new(dns.Msg) + req.SetQuestion("example.org.", dns.TypeA) + rec := dnstest.NewRecorder(&test.ResponseWriter{}) + + c.ServeDNS(context.TODO(), rec, req) + + qname := rec.Msg.Question[0].Name + if c.pcache.Len() != 0 { + t.Errorf("cached %s, while reply had %s", "example.org.", qname) + } + + // qtype + c.Next = spoofHandlerType() + req.SetQuestion("example.org.", dns.TypeMX) + + c.ServeDNS(context.TODO(), rec, req) + + qtype := rec.Msg.Question[0].Qtype + if c.pcache.Len() != 0 { + t.Errorf("cached %s type %d, while reply had %d", "example.org.", dns.TypeMX, qtype) + } +} + +// spoofHandler is a fake plugin implementation which returns a single A records for example.org. The qname in the +// question section is set to example.NET (i.e. they *don't* match). +func spoofHandler() plugin.Handler { + return plugin.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { + m := new(dns.Msg) + m.SetQuestion("example.net.", dns.TypeA) + m.Response = true + m.Answer = []dns.RR{test.A("example.org. IN A 127.0.0.53")} + w.WriteMsg(m) + return dns.RcodeSuccess, nil + }) +} + +// spoofHandlerType is a fake plugin implementation which returns a single MX records for example.org. The qtype in the +// question section is set to A. +func spoofHandlerType() plugin.Handler { + return plugin.HandlerFunc(func(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) { + m := new(dns.Msg) + m.SetQuestion("example.org.", dns.TypeA) + m.Response = true + m.Answer = []dns.RR{test.MX("example.org. IN MX 10 mail.example.org.")} + w.WriteMsg(m) + return dns.RcodeSuccess, nil + }) +} diff --git a/plugin/forward/forward.go b/plugin/forward/forward.go index 8a3e7188c..dd0c927fb 100644 --- a/plugin/forward/forward.go +++ b/plugin/forward/forward.go @@ -119,6 +119,13 @@ func (f *Forward) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg break } + // Check if the reply is correct; if not return FormErr. + if !state.Match(ret) { + formerr := state.ErrorMessage(dns.RcodeFormatError) + w.WriteMsg(formerr) + return 0, nil + } + ret.Compress = true // When using force_tcp the upstream can send a message that is too big for // the udp buffer, hence we need to truncate the message to at least make it diff --git a/plugin/proxy/proxy.go b/plugin/proxy/proxy.go index a5df0b95c..af61f424f 100644 --- a/plugin/proxy/proxy.go +++ b/plugin/proxy/proxy.go @@ -100,6 +100,14 @@ func (p Proxy) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) ( taperr := toDnstap(ctx, host.Name, upstream.Exchanger(), state, reply, start) if backendErr == nil { + + // Check if the reply is correct; if not return FormErr. + if !state.Match(reply) { + formerr := state.ErrorMessage(dns.RcodeFormatError) + w.WriteMsg(formerr) + return 0, taperr + } + w.WriteMsg(reply) RequestDuration.WithLabelValues(state.Proto(), upstream.Exchanger().Protocol(), familyToString(state.Family()), host.Name).Observe(time.Since(start).Seconds())