From 8b967b3bb68488a1f7710a7e87afd1b2b2a90fd0 Mon Sep 17 00:00:00 2001 From: Dhia Ayachi Date: Thu, 24 Jun 2021 20:44:44 -0400 Subject: [PATCH] return an empty record when asked for an addr dns with type other then A, AAAA and ANY (#10401) * return an invalid record when asked for an addr dns with type other then A and AAAA * add changelog * fix ANY use case and add a test for it * update changelog type Co-authored-by: Daniel Nephin * return empty response if the question record type do not match for addr * set comment in the right place * return A\AAAA record in extra section if record type is not A\AAAA for addr * Fix failing test * remove commented code Co-authored-by: Daniel Nephin * use require for test validation * use variable to init struct * fix failing test * Update agent/dns.go Co-authored-by: Daniel Nephin * Update .changelog/10401.txt Co-authored-by: Daniel Nephin * Update agent/dns.go Co-authored-by: Daniel Nephin * Update agent/dns.go Co-authored-by: Daniel Nephin * Update agent/dns.go Co-authored-by: Daniel Nephin * fix compilation error Co-authored-by: Daniel Nephin --- .changelog/10401.txt | 3 ++ agent/dns.go | 23 ++++++--- agent/dns_test.go | 115 +++++++++++++++++++++++++++++++++++++------ 3 files changed, 121 insertions(+), 20 deletions(-) create mode 100644 .changelog/10401.txt diff --git a/.changelog/10401.txt b/.changelog/10401.txt new file mode 100644 index 000000000..86ef317ee --- /dev/null +++ b/.changelog/10401.txt @@ -0,0 +1,3 @@ +```release-note:bug +dns: return an empty answer when asked for an addr dns with type other then A and AAAA. +``` diff --git a/agent/dns.go b/agent/dns.go index cad5ac0f3..85ae5c4d6 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -782,6 +782,7 @@ func (d *DNSServer) doDispatch(network string, remoteAddr net.Addr, req, resp *d case "addr": //
.addr.. - addr must be the second label, datacenter is optional + if len(queryParts) != 1 { return invalid() } @@ -793,8 +794,8 @@ func (d *DNSServer) doDispatch(network string, remoteAddr net.Addr, req, resp *d if err != nil { return invalid() } - - resp.Answer = append(resp.Answer, &dns.A{ + //check if the query type is A for IPv4 or ANY + aRecord := &dns.A{ Hdr: dns.RR_Header{ Name: qName + d.domain, Rrtype: dns.TypeA, @@ -802,15 +803,20 @@ func (d *DNSServer) doDispatch(network string, remoteAddr net.Addr, req, resp *d Ttl: uint32(cfg.NodeTTL / time.Second), }, A: ip, - }) + } + if req.Question[0].Qtype != dns.TypeA && req.Question[0].Qtype != dns.TypeANY { + resp.Extra = append(resp.Answer, aRecord) + } else { + resp.Answer = append(resp.Answer, aRecord) + } // IPv6 case 16: ip, err := hex.DecodeString(queryParts[0]) if err != nil { return invalid() } - - resp.Answer = append(resp.Answer, &dns.AAAA{ + //check if the query type is AAAA for IPv6 or ANY + aaaaRecord := &dns.AAAA{ Hdr: dns.RR_Header{ Name: qName + d.domain, Rrtype: dns.TypeAAAA, @@ -818,7 +824,12 @@ func (d *DNSServer) doDispatch(network string, remoteAddr net.Addr, req, resp *d Ttl: uint32(cfg.NodeTTL / time.Second), }, AAAA: ip, - }) + } + if req.Question[0].Qtype != dns.TypeAAAA && req.Question[0].Qtype != dns.TypeANY { + resp.Extra = append(resp.Extra, aaaaRecord) + } else { + resp.Answer = append(resp.Answer, aaaaRecord) + } } } return true diff --git a/agent/dns_test.go b/agent/dns_test.go index 3f3f32a71..5cfe58206 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -6044,7 +6044,7 @@ func TestDNS_AddressLookup(t *testing.T) { } for question, answer := range cases { m := new(dns.Msg) - m.SetQuestion(question, dns.TypeSRV) + m.SetQuestion(question, dns.TypeA) c := new(dns.Client) in, _, err := c.Exchange(m, a.DNSAddr()) @@ -6052,20 +6052,73 @@ func TestDNS_AddressLookup(t *testing.T) { t.Fatalf("err: %v", err) } - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) - } + require.Len(t, in.Answer, 1) + require.Equal(t, dns.TypeA, in.Answer[0].Header().Rrtype) aRec, ok := in.Answer[0].(*dns.A) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if aRec.A.To4().String() != answer { - t.Fatalf("Bad: %#v", aRec) - } - if aRec.Hdr.Ttl != 0 { - t.Fatalf("Bad: %#v", in.Answer[0]) - } + require.True(t, ok) + require.Equal(t, aRec.A.To4().String(), answer) + require.Zero(t, aRec.Hdr.Ttl) + } +} + +func TestDNS_AddressLookupANY(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + a := NewTestAgent(t, "") + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Look up the addresses + cases := map[string]string{ + "7f000001.addr.dc1.consul.": "127.0.0.1", + } + for question, answer := range cases { + m := new(dns.Msg) + m.SetQuestion(question, dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + + require.NoError(t, err) + require.Len(t, in.Answer, 1) + require.Equal(t, in.Answer[0].Header().Rrtype, dns.TypeA) + aRec, ok := in.Answer[0].(*dns.A) + require.True(t, ok) + require.Equal(t, aRec.A.To4().String(), answer) + require.Zero(t, aRec.Hdr.Ttl) + + } +} + +func TestDNS_AddressLookupInvalidType(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + a := NewTestAgent(t, "") + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Look up the addresses + cases := map[string]string{ + "7f000001.addr.dc1.consul.": "", + } + for question := range cases { + m := new(dns.Msg) + m.SetQuestion(question, dns.TypeSRV) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + require.NoError(t, err) + require.Zero(t, in.Rcode) + require.Nil(t, in.Answer) + require.NotNil(t, in.Extra) + require.Len(t, in.Extra, 1) } } @@ -6086,7 +6139,7 @@ func TestDNS_AddressLookupIPV6(t *testing.T) { } for question, answer := range cases { m := new(dns.Msg) - m.SetQuestion(question, dns.TypeSRV) + m.SetQuestion(question, dns.TypeAAAA) c := new(dns.Client) in, _, err := c.Exchange(m, a.DNSAddr()) @@ -6098,6 +6151,9 @@ func TestDNS_AddressLookupIPV6(t *testing.T) { t.Fatalf("Bad: %#v", in) } + if in.Answer[0].Header().Rrtype != dns.TypeAAAA { + t.Fatalf("Invalid type: %#v", in.Answer[0]) + } aaaaRec, ok := in.Answer[0].(*dns.AAAA) if !ok { t.Fatalf("Bad: %#v", in.Answer[0]) @@ -6111,6 +6167,37 @@ func TestDNS_AddressLookupIPV6(t *testing.T) { } } +func TestDNS_AddressLookupIPV6InvalidType(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + a := NewTestAgent(t, "") + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Look up the addresses + cases := map[string]string{ + "2607002040050808000000000000200e.addr.consul.": "2607:20:4005:808::200e", + "2607112040051808ffffffffffff200e.addr.consul.": "2607:1120:4005:1808:ffff:ffff:ffff:200e", + } + for question := range cases { + m := new(dns.Msg) + m.SetQuestion(question, dns.TypeSRV) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + if err != nil { + t.Fatalf("err: %v", err) + } + + if in.Answer != nil { + t.Fatalf("Bad: %#v", in) + } + } +} + // TestDNS_NonExistentDC_Server verifies NXDOMAIN is returned when // Consul server agent is queried for a service in a non-existent // domain.