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 <dnephin@hashicorp.com>

* 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 <dnephin@hashicorp.com>

* use require for test validation

* use variable to init struct

* fix failing test

* Update agent/dns.go

Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>

* Update .changelog/10401.txt

Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>

* Update agent/dns.go

Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>

* Update agent/dns.go

Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>

* Update agent/dns.go

Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>

* fix compilation error

Co-authored-by: Daniel Nephin <dnephin@hashicorp.com>

.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.
+```

agent/dns.go

@@ -782,6 +782,7 @@ func (d *DNSServer) doDispatch(network string, remoteAddr net.Addr, req, resp *d
 
 	case "addr":
 		// <address>.addr.<suffixes>.<domain> - 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()
 			}
-
+			//check if the query type is  A for IPv4 or ANY
-			resp.Answer = append(resp.Answer, &dns.A{
+			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()
 			}
-
+			//check if the query type is  AAAA for IPv6 or ANY
-			resp.Answer = append(resp.Answer, &dns.AAAA{
+			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

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 {
+		require.Len(t, in.Answer, 1)
-			t.Fatalf("Bad: %#v", in)
-		}
 
+		require.Equal(t, dns.TypeA, in.Answer[0].Header().Rrtype)
 		aRec, ok := in.Answer[0].(*dns.A)
-		if !ok {
+		require.True(t, ok)
-			t.Fatalf("Bad: %#v", in.Answer[0])
+		require.Equal(t, aRec.A.To4().String(), answer)
-		}
+		require.Zero(t, aRec.Hdr.Ttl)
-		if aRec.A.To4().String() != answer {
+	}
-			t.Fatalf("Bad: %#v", aRec)
+}
-		}
+
-		if aRec.Hdr.Ttl != 0 {
+func TestDNS_AddressLookupANY(t *testing.T) {
-			t.Fatalf("Bad: %#v", in.Answer[0])
+	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.