diff --git a/middleware/cache/cache.go b/middleware/cache/cache.go index 1bd3d3352..e2a669723 100644 --- a/middleware/cache/cache.go +++ b/middleware/cache/cache.go @@ -63,7 +63,7 @@ type ResponseWriter struct { // WriteMsg implements the dns.ResponseWriter interface. func (c *ResponseWriter) WriteMsg(res *dns.Msg) error { do := false - mt, opt := response.Typify(res) + mt, opt := response.Typify(res, time.Now().UTC()) if opt != nil { do = opt.Do() } diff --git a/middleware/cache/cache_test.go b/middleware/cache/cache_test.go index d703e2a58..18aa05fe5 100644 --- a/middleware/cache/cache_test.go +++ b/middleware/cache/cache_test.go @@ -21,6 +21,7 @@ type cacheTestCase struct { Authoritative bool RecursionAvailable bool Truncated bool + shouldCache bool } var cacheTestCases = []cacheTestCase{ @@ -40,6 +41,7 @@ var cacheTestCases = []cacheTestCase{ test.MX("miek.nl. 3601 IN MX 10 aspmx2.googlemail.com."), }, }, + shouldCache: true, }, { RecursionAvailable: true, AuthenticatedData: true, Authoritative: true, @@ -57,6 +59,7 @@ var cacheTestCases = []cacheTestCase{ test.MX("mIEK.nL. 3601 IN MX 10 aspmx2.googlemail.com."), }, }, + shouldCache: true, }, { Truncated: true, @@ -64,7 +67,8 @@ var cacheTestCases = []cacheTestCase{ Qname: "miek.nl.", Qtype: dns.TypeMX, Answer: []dns.RR{test.MX("miek.nl. 1800 IN MX 1 aspmx.l.google.com.")}, }, - in: test.Case{}, + in: test.Case{}, + shouldCache: false, }, { RecursionAvailable: true, Authoritative: true, @@ -82,6 +86,51 @@ var cacheTestCases = []cacheTestCase{ test.SOA("example.org. 3600 IN SOA sns.dns.icann.org. noc.dns.icann.org. 2016082540 7200 3600 1209600 3600"), }, }, + shouldCache: true, + }, + { + RecursionAvailable: true, Authoritative: true, + Case: test.Case{ + Qname: "miek.nl.", Qtype: dns.TypeMX, + Do: true, + 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.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="), + }, + }, + in: test.Case{ + Qname: "miek.nl.", Qtype: dns.TypeMX, + Do: true, + 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.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, + }, + { + RecursionAvailable: true, Authoritative: true, + Case: test.Case{ + Qname: "example.org.", Qtype: dns.TypeMX, + Do: true, + Answer: []dns.RR{ + test.MX("example.org. 3600 IN MX 1 aspmx.l.google.com."), + test.MX("example.org. 3600 IN MX 10 aspmx2.googlemail.com."), + test.RRSIG("example.org. 3600 IN RRSIG MX 8 2 1800 20170521031301 20170421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="), + }, + }, + in: test.Case{ + Qname: "example.org.", Qtype: dns.TypeMX, + Do: true, + Answer: []dns.RR{ + test.MX("example.org. 3600 IN MX 1 aspmx.l.google.com."), + test.MX("example.org. 3600 IN MX 10 aspmx2.googlemail.com."), + test.RRSIG("example.org. 1800 IN RRSIG MX 8 2 1800 20170521031301 20170421031301 12051 miek.nl. lAaEzB5teQLLKyDenatmyhca7blLRg9DoGNrhe3NReBZN5C5/pMQk8Jc u25hv2fW23/SLm5IC2zaDpp2Fzgm6Jf7e90/yLcwQPuE7JjS55WMF+HE LEh7Z6AEb+Iq4BWmNhUz6gPxD4d9eRMs7EAzk13o1NYi5/JhfL6IlaYy qkc="), + }, + }, + shouldCache: true, }, } @@ -93,7 +142,7 @@ func cacheMsg(m *dns.Msg, tc cacheTestCase) *dns.Msg { m.Truncated = tc.Truncated m.Answer = tc.in.Answer m.Ns = tc.in.Ns - // m.Extra = tc.in.Extra , not the OPT record! + // m.Extra = tc.in.Extra don't copy Extra, because we don't care and fake EDNS0 DO with tc.Do. return m } @@ -107,6 +156,9 @@ func newTestCache(ttl time.Duration) (*Cache, *ResponseWriter) { } func TestCache(t *testing.T) { + now, _ := time.Parse(time.UnixDate, "Fri Apr 21 10:51:21 BST 2017") + utc := now.UTC() + c, crr := newTestCache(maxTTL) log.SetOutput(ioutil.Discard) @@ -116,15 +168,18 @@ func TestCache(t *testing.T) { m = cacheMsg(m, tc) do := tc.in.Do - mt, _ := response.Typify(m) + mt, _ := response.Typify(m, utc) k := key(m, mt, do) + crr.set(m, k, mt, c.pttl) name := middleware.Name(m.Question[0].Name).Normalize() qtype := m.Question[0].Qtype + i, ok, _ := c.get(name, qtype, do) - if ok && m.Truncated { - t.Errorf("Truncated message should not have been cached") + + if ok != tc.shouldCache { + t.Errorf("cached message that should not have been cached: %s", name) continue } diff --git a/middleware/dnssec/dnssec.go b/middleware/dnssec/dnssec.go index 539ef0b98..a0a072349 100644 --- a/middleware/dnssec/dnssec.go +++ b/middleware/dnssec/dnssec.go @@ -41,7 +41,7 @@ func New(zones []string, keys []*DNSKEY, next middleware.Handler, cache *lru.Cac func (d Dnssec) Sign(state request.Request, zone string, now time.Time) *dns.Msg { req := state.Req - mt, _ := response.Typify(req) // TODO(miek): need opt record here? + mt, _ := response.Typify(req, time.Now().UTC()) // TODO(miek): need opt record here? if mt == response.Delegation { return req } diff --git a/middleware/log/log.go b/middleware/log/log.go index 137411f27..456ed9aba 100644 --- a/middleware/log/log.go +++ b/middleware/log/log.go @@ -52,7 +52,8 @@ func (l Logger) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) rc = 0 } - class, _ := response.Classify(rrw.Msg) + tpe, _ := response.Typify(rrw.Msg, time.Now().UTC()) + class := response.Classify(tpe) if rule.Class == response.All || rule.Class == class { rep := replacer.New(r, rrw, CommonLogEmptyValue) rule.Log.Println(rep.Replace(rule.Format)) diff --git a/middleware/pkg/response/classify.go b/middleware/pkg/response/classify.go index e9f17fe00..2e705cb0b 100644 --- a/middleware/pkg/response/classify.go +++ b/middleware/pkg/response/classify.go @@ -1,10 +1,6 @@ package response -import ( - "fmt" - - "github.com/miekg/dns" -) +import "fmt" // Class holds sets of Types type Class int @@ -50,14 +46,8 @@ func ClassFromString(s string) (Class, error) { return All, fmt.Errorf("invalid Class: %s", s) } -// Classify classifies a dns message: it returns its Class. -func Classify(m *dns.Msg) (Class, *dns.OPT) { - t, o := Typify(m) - return classify(t), o -} - -// Does need to be exported? -func classify(t Type) Class { +// Classify classifies the Type t, it returns its Class. +func Classify(t Type) Class { switch t { case NoError, Delegation: return Success diff --git a/middleware/pkg/response/typify.go b/middleware/pkg/response/typify.go index d2bbeb47e..bbde98444 100644 --- a/middleware/pkg/response/typify.go +++ b/middleware/pkg/response/typify.go @@ -2,11 +2,12 @@ package response import ( "fmt" + "time" "github.com/miekg/dns" ) -// Type is the type of the message +// Type is the type of the message. type Type int const ( @@ -26,54 +27,39 @@ const ( OtherError ) -func (t Type) String() string { - switch t { - case NoError: - return "NOERROR" - case NameError: - return "NXDOMAIN" - case NoData: - return "NODATA" - case Delegation: - return "DELEGATION" - case Meta: - return "META" - case Update: - return "UPDATE" - case OtherError: - return "OTHERERROR" - } - return "" +var toString = map[Type]string{ + NoError: "NOERROR", + NameError: "NXDOMAIN", + NoData: "NODATA", + Delegation: "DELEGATION", + Meta: "META", + Update: "UPDATE", + OtherError: "OTHERERROR", } +func (t Type) String() string { return toString[t] } + // TypeFromString returns the type from the string s. If not type matches // the OtherError type and an error are returned. func TypeFromString(s string) (Type, error) { - switch s { - case "NOERROR": - return NoError, nil - case "NXDOMAIN": - return NameError, nil - case "NODATA": - return NoData, nil - case "DELEGATION": - return Delegation, nil - case "META": - return Meta, nil - case "UPDATE": - return Update, nil - case "OTHERERROR": - return OtherError, nil + for t, str := range toString { + if s == str { + return t, nil + } } return NoError, fmt.Errorf("invalid Type: %s", s) } // Typify classifies a message, it returns the Type. -func Typify(m *dns.Msg) (Type, *dns.OPT) { +func Typify(m *dns.Msg, t time.Time) (Type, *dns.OPT) { if m == nil { return OtherError, nil } opt := m.IsEdns0() + do := false + if opt != nil { + do = opt.Do() + } if m.Opcode == dns.OpcodeUpdate { return Update, opt @@ -90,6 +76,13 @@ func Typify(m *dns.Msg) (Type, *dns.OPT) { } } + // If our message contains any expired sigs and we care about that, we should return expired + if do { + if expired := typifyExpired(m, t); expired { + return OtherError, opt + } + } + if len(m.Answer) > 0 && m.Rcode == dns.RcodeSuccess { return NoError, opt } @@ -107,6 +100,7 @@ func Typify(m *dns.Msg) (Type, *dns.OPT) { } // Check length of different sections, and drop stuff that is just to large? TODO(miek). + if soa && m.Rcode == dns.RcodeSuccess { return NoData, opt } @@ -114,7 +108,7 @@ func Typify(m *dns.Msg) (Type, *dns.OPT) { return NameError, opt } - if ns > 0 && ns == len(m.Ns) && m.Rcode == dns.RcodeSuccess { + if ns > 0 && ns > 0 && m.Rcode == dns.RcodeSuccess { return Delegation, opt } @@ -124,3 +118,29 @@ func Typify(m *dns.Msg) (Type, *dns.OPT) { return OtherError, opt } + +func typifyExpired(m *dns.Msg, t time.Time) bool { + if expired := typifyExpiredRRSIG(m.Answer, t); expired { + return true + } + if expired := typifyExpiredRRSIG(m.Ns, t); expired { + return true + } + if expired := typifyExpiredRRSIG(m.Extra, t); expired { + return true + } + return false +} + +func typifyExpiredRRSIG(rrs []dns.RR, t time.Time) bool { + for _, r := range rrs { + if r.Header().Rrtype != dns.TypeRRSIG { + continue + } + ok := r.(*dns.RRSIG).ValidityPeriod(t) + if !ok { + return true + } + } + return false +} diff --git a/middleware/pkg/response/typify_test.go b/middleware/pkg/response/typify_test.go index 3ae795e23..738c6066d 100644 --- a/middleware/pkg/response/typify_test.go +++ b/middleware/pkg/response/typify_test.go @@ -2,6 +2,7 @@ package response import ( "testing" + "time" "github.com/coredns/coredns/middleware/test" @@ -11,17 +12,39 @@ import ( func TestTypifyNilMsg(t *testing.T) { var m *dns.Msg - ty, _ := Typify(m) + ty, _ := Typify(m, time.Now().UTC()) if ty != OtherError { - t.Errorf("message wrongly typified, expected OtherError, got %d", ty) + t.Errorf("message wrongly typified, expected OtherError, got %s", ty) } } -func TestClassifyDelegation(t *testing.T) { +func TestTypifyDelegation(t *testing.T) { m := delegationMsg() - mt, _ := Typify(m) + mt, _ := Typify(m, time.Now().UTC()) if mt != Delegation { - t.Errorf("message is wrongly classified, expected delegation, got %d", mt) + t.Errorf("message is wrongly typified, expected Delegation, got %s", mt) + } +} + +func TestTypifyRRSIG(t *testing.T) { + now, _ := time.Parse(time.UnixDate, "Fri Apr 21 10:51:21 BST 2017") + utc := now.UTC() + + m := delegationMsgRRSIGOK() + if mt, _ := Typify(m, utc); mt != Delegation { + t.Errorf("message is wrongly typified, expected Delegation, got %s", mt) + } + + // Still a Delegation because EDNS0 OPT DO bool is not set, so we won't check the sigs. + m = delegationMsgRRSIGFail() + if mt, _ := Typify(m, utc); mt != Delegation { + t.Errorf("message is wrongly typified, expected Delegation, got %s", mt) + } + + m = delegationMsgRRSIGFail() + m = addOpt(m) + if mt, _ := Typify(m, utc); mt != OtherError { + t.Errorf("message is wrongly typified, expected OtherError, got %s", mt) } } @@ -38,3 +61,24 @@ func delegationMsg() *dns.Msg { }, } } + +func delegationMsgRRSIGOK() *dns.Msg { + del := delegationMsg() + del.Ns = append(del.Ns, + test.RRSIG("miek.nl. 1800 IN RRSIG NS 8 2 1800 20170521031301 20170421031301 12051 miek.nl. PIUu3TKX/sB/N1n1E1yWxHHIcPnc2q6Wq9InShk+5ptRqChqKdZNMLDm gCq+1bQAZ7jGvn2PbwTwE65JzES7T+hEiqR5PU23DsidvZyClbZ9l0xG JtKwgzGXLtUHxp4xv/Plq+rq/7pOG61bNCxRyS7WS7i7QcCCWT1BCcv+ wZ0="), + ) + return del +} + +func delegationMsgRRSIGFail() *dns.Msg { + del := delegationMsg() + del.Ns = append(del.Ns, + test.RRSIG("miek.nl. 1800 IN RRSIG NS 8 2 1800 20160521031301 20160421031301 12051 miek.nl. PIUu3TKX/sB/N1n1E1yWxHHIcPnc2q6Wq9InShk+5ptRqChqKdZNMLDm gCq+1bQAZ7jGvn2PbwTwE65JzES7T+hEiqR5PU23DsidvZyClbZ9l0xG JtKwgzGXLtUHxp4xv/Plq+rq/7pOG61bNCxRyS7WS7i7QcCCWT1BCcv+ wZ0="), + ) + return del +} + +func addOpt(m *dns.Msg) *dns.Msg { + m.Extra = append(m.Extra, test.OPT(4096, true)) + return m +}