From 8bc8b8bb0a0b00ac9bdf34f0faa34a15c13b222a Mon Sep 17 00:00:00 2001 From: hc-github-team-consul-core Date: Wed, 21 Feb 2024 13:47:45 -0500 Subject: [PATCH] Backport of NET-7813 - DNS : SERVFAIL when resolving PTR records into release/1.17.x (#20689) NET-7813 - DNS : SERVFAIL when resolving PTR records Co-authored-by: John Murret --- .changelog/20679.txt | 3 + agent/dns.go | 12 +- agent/dns_reverse_lookup_test.go | 390 +++++++++++++++++++++++++++++++ agent/dns_test.go | 348 --------------------------- 4 files changed, 402 insertions(+), 351 deletions(-) create mode 100644 .changelog/20679.txt create mode 100644 agent/dns_reverse_lookup_test.go diff --git a/.changelog/20679.txt b/.changelog/20679.txt new file mode 100644 index 0000000000..0efb6b2763 --- /dev/null +++ b/.changelog/20679.txt @@ -0,0 +1,3 @@ +```release-note:bug +dns: SERVFAIL when resolving not found PTR records. +``` \ No newline at end of file diff --git a/agent/dns.go b/agent/dns.go index b55338ea3e..f0aa7af4a2 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -401,7 +401,8 @@ func (d *DNSServer) handlePtr(resp dns.ResponseWriter, req *dns.Msg) { m.SetReply(req) m.Compress = !cfg.DisableCompression m.Authoritative = true - m.RecursionAvailable = (len(cfg.Recursors) > 0) + recursionAvailable := atomic.LoadUint32(&(d.recursorEnabled)) == 1 + m.RecursionAvailable = recursionAvailable // Only add the SOA if requested if req.Question[0].Qtype == dns.TypeSOA { @@ -476,8 +477,13 @@ func (d *DNSServer) handlePtr(resp dns.ResponseWriter, req *dns.Msg) { // nothing found locally, recurse if len(m.Answer) == 0 { - d.handleRecurse(resp, req) - return + if recursionAvailable { + d.handleRecurse(resp, req) + return + } else { + m.SetRcode(req, dns.RcodeNameError) + d.addSOA(cfg, m, q.Name) + } } // ptr record responses are globally valid diff --git a/agent/dns_reverse_lookup_test.go b/agent/dns_reverse_lookup_test.go new file mode 100644 index 0000000000..c4e3c9f3f7 --- /dev/null +++ b/agent/dns_reverse_lookup_test.go @@ -0,0 +1,390 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package agent + +import ( + "context" + "testing" + + "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/testrpc" + "github.com/miekg/dns" + "github.com/stretchr/testify/require" +) + +func TestDNS_ReverseLookup(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + a := NewTestAgent(t, "") + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Register node + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo2", + Address: "127.0.0.2", + } + + var out struct{} + if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + + m := new(dns.Msg) + m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + if err != nil { + t.Fatalf("err: %v", err) + } + + if len(in.Answer) != 1 { + t.Fatalf("Bad: %#v", in) + } + + ptrRec, ok := in.Answer[0].(*dns.PTR) + if !ok { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + if ptrRec.Ptr != "foo2.node.dc1.consul." { + t.Fatalf("Bad: %#v", ptrRec) + } +} + +func TestDNS_ReverseLookup_CustomDomain(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + a := NewTestAgent(t, ` + domain = "custom" + `) + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Register node + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo2", + Address: "127.0.0.2", + } + + var out struct{} + if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + + m := new(dns.Msg) + m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + if err != nil { + t.Fatalf("err: %v", err) + } + + if len(in.Answer) != 1 { + t.Fatalf("Bad: %#v", in) + } + + ptrRec, ok := in.Answer[0].(*dns.PTR) + if !ok { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + if ptrRec.Ptr != "foo2.node.dc1.custom." { + t.Fatalf("Bad: %#v", ptrRec) + } + +} + +func TestDNS_ReverseLookup_IPV6(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + a := NewTestAgent(t, "") + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Register node + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "bar", + Address: "::4242:4242", + } + + var out struct{} + if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + + m := new(dns.Msg) + m.SetQuestion("2.4.2.4.2.4.2.4.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa.", dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + if err != nil { + t.Fatalf("err: %v", err) + } + + if len(in.Answer) != 1 { + t.Fatalf("Bad: %#v", in) + } + + ptrRec, ok := in.Answer[0].(*dns.PTR) + if !ok { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + if ptrRec.Ptr != "bar.node.dc1.consul." { + t.Fatalf("Bad: %#v", ptrRec) + } +} + +func TestDNS_Compression_ReverseLookup(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + a := NewTestAgent(t, "") + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Register node. + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo2", + Address: "127.0.0.2", + } + var out struct{} + if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + + m := new(dns.Msg) + m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) + + conn, err := dns.Dial("udp", a.DNSAddr()) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Do a manual exchange with compression on (the default). + if err := conn.WriteMsg(m); err != nil { + t.Fatalf("err: %v", err) + } + p := make([]byte, dns.MaxMsgSize) + compressed, err := conn.Read(p) + if err != nil { + t.Fatalf("err: %v", err) + } + + // Disable compression and try again. + a.DNSDisableCompression(true) + if err := conn.WriteMsg(m); err != nil { + t.Fatalf("err: %v", err) + } + unc, err := conn.Read(p) + if err != nil { + t.Fatalf("err: %v", err) + } + + // We can't see the compressed status given the DNS API, so we just make + // sure the message is smaller to see if it's respecting the flag. + if compressed == 0 || unc == 0 || compressed >= unc { + t.Fatalf("doesn't look compressed: %d vs. %d", compressed, unc) + } +} + +func TestDNS_ServiceReverseLookup(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + a := NewTestAgent(t, "") + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Register a node with a service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "127.0.0.1", + Service: &structs.NodeService{ + Service: "db", + Tags: []string{"primary"}, + Port: 12345, + Address: "127.0.0.2", + }, + } + + var out struct{} + if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + } + + m := new(dns.Msg) + m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + if err != nil { + t.Fatalf("err: %v", err) + } + + if len(in.Answer) != 1 { + t.Fatalf("Bad: %#v", in) + } + + ptrRec, ok := in.Answer[0].(*dns.PTR) + if !ok { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + if ptrRec.Ptr != serviceCanonicalDNSName("db", "service", "dc1", "consul", nil)+"." { + t.Fatalf("Bad: %#v", ptrRec) + } +} + +func TestDNS_ServiceReverseLookup_IPV6(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + a := NewTestAgent(t, "") + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Register a node with a service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "2001:db8::1", + Service: &structs.NodeService{ + Service: "db", + Tags: []string{"primary"}, + Port: 12345, + Address: "2001:db8::ff00:42:8329", + }, + } + + var out struct{} + if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + } + + m := new(dns.Msg) + m.SetQuestion("9.2.3.8.2.4.0.0.0.0.f.f.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.", dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + if err != nil { + t.Fatalf("err: %v", err) + } + + if len(in.Answer) != 1 { + t.Fatalf("Bad: %#v", in) + } + + ptrRec, ok := in.Answer[0].(*dns.PTR) + if !ok { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + if ptrRec.Ptr != serviceCanonicalDNSName("db", "service", "dc1", "consul", nil)+"." { + t.Fatalf("Bad: %#v", ptrRec) + } +} + +func TestDNS_ServiceReverseLookup_CustomDomain(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + a := NewTestAgent(t, ` + domain = "custom" + `) + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Register a node with a service. + { + args := &structs.RegisterRequest{ + Datacenter: "dc1", + Node: "foo", + Address: "127.0.0.1", + Service: &structs.NodeService{ + Service: "db", + Tags: []string{"primary"}, + Port: 12345, + Address: "127.0.0.2", + }, + } + + var out struct{} + if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { + t.Fatalf("err: %v", err) + } + } + + m := new(dns.Msg) + m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + if err != nil { + t.Fatalf("err: %v", err) + } + + if len(in.Answer) != 1 { + t.Fatalf("Bad: %#v", in) + } + + ptrRec, ok := in.Answer[0].(*dns.PTR) + if !ok { + t.Fatalf("Bad: %#v", in.Answer[0]) + } + if ptrRec.Ptr != serviceCanonicalDNSName("db", "service", "dc1", "custom", nil)+"." { + t.Fatalf("Bad: %#v", ptrRec) + } +} + +func TestDNS_ReverseLookup_NotFound(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + // do not configure recursors + a := NewTestAgent(t, "") + defer a.Shutdown() + testrpc.WaitForLeader(t, a.RPC, "dc1") + + // Do not register any nodes + m := new(dns.Msg) + qName := "2.0.0.127.in-addr.arpa." + m.SetQuestion(qName, dns.TypeANY) + + c := new(dns.Client) + in, _, err := c.Exchange(m, a.DNSAddr()) + require.NoError(t, err) + require.Nil(t, in.Answer) + require.Nil(t, in.Extra) + + require.Equal(t, dns.RcodeNameError, in.Rcode) + + question := in.Question[0] + require.Equal(t, qName, question.Name) + require.Equal(t, dns.TypeANY, question.Qtype) + require.Equal(t, uint16(dns.ClassINET), question.Qclass) + + soa, ok := in.Ns[0].(*dns.SOA) + require.True(t, ok) + require.Equal(t, "ns.consul.", soa.Ns) + require.Equal(t, "hostmaster.consul.", soa.Mbox) +} diff --git a/agent/dns_test.go b/agent/dns_test.go index f2b4f9311b..4ad438ddb7 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -951,298 +951,6 @@ func TestDNS_EDNS0_ECS(t *testing.T) { } } -func TestDNS_ReverseLookup(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") - - // Register node - args := &structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo2", - Address: "127.0.0.2", - } - - var out struct{} - if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { - t.Fatalf("err: %v", err) - } - - m := new(dns.Msg) - m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) - - c := new(dns.Client) - in, _, err := c.Exchange(m, a.DNSAddr()) - if err != nil { - t.Fatalf("err: %v", err) - } - - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) - } - - ptrRec, ok := in.Answer[0].(*dns.PTR) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if ptrRec.Ptr != "foo2.node.dc1.consul." { - t.Fatalf("Bad: %#v", ptrRec) - } -} - -func TestDNS_ReverseLookup_CustomDomain(t *testing.T) { - if testing.Short() { - t.Skip("too slow for testing.Short") - } - - t.Parallel() - a := NewTestAgent(t, ` - domain = "custom" - `) - defer a.Shutdown() - testrpc.WaitForLeader(t, a.RPC, "dc1") - - // Register node - args := &structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo2", - Address: "127.0.0.2", - } - - var out struct{} - if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { - t.Fatalf("err: %v", err) - } - - m := new(dns.Msg) - m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) - - c := new(dns.Client) - in, _, err := c.Exchange(m, a.DNSAddr()) - if err != nil { - t.Fatalf("err: %v", err) - } - - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) - } - - ptrRec, ok := in.Answer[0].(*dns.PTR) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if ptrRec.Ptr != "foo2.node.dc1.custom." { - t.Fatalf("Bad: %#v", ptrRec) - } -} - -func TestDNS_ReverseLookup_IPV6(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") - - // Register node - args := &structs.RegisterRequest{ - Datacenter: "dc1", - Node: "bar", - Address: "::4242:4242", - } - - var out struct{} - if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { - t.Fatalf("err: %v", err) - } - - m := new(dns.Msg) - m.SetQuestion("2.4.2.4.2.4.2.4.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.ip6.arpa.", dns.TypeANY) - - c := new(dns.Client) - in, _, err := c.Exchange(m, a.DNSAddr()) - if err != nil { - t.Fatalf("err: %v", err) - } - - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) - } - - ptrRec, ok := in.Answer[0].(*dns.PTR) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if ptrRec.Ptr != "bar.node.dc1.consul." { - t.Fatalf("Bad: %#v", ptrRec) - } -} - -func TestDNS_ServiceReverseLookup(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") - - // Register a node with a service. - { - args := &structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo", - Address: "127.0.0.1", - Service: &structs.NodeService{ - Service: "db", - Tags: []string{"primary"}, - Port: 12345, - Address: "127.0.0.2", - }, - } - - var out struct{} - if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { - t.Fatalf("err: %v", err) - } - } - - m := new(dns.Msg) - m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) - - c := new(dns.Client) - in, _, err := c.Exchange(m, a.DNSAddr()) - if err != nil { - t.Fatalf("err: %v", err) - } - - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) - } - - ptrRec, ok := in.Answer[0].(*dns.PTR) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if ptrRec.Ptr != serviceCanonicalDNSName("db", "service", "dc1", "consul", nil)+"." { - t.Fatalf("Bad: %#v", ptrRec) - } -} - -func TestDNS_ServiceReverseLookup_IPV6(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") - - // Register a node with a service. - { - args := &structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo", - Address: "2001:db8::1", - Service: &structs.NodeService{ - Service: "db", - Tags: []string{"primary"}, - Port: 12345, - Address: "2001:db8::ff00:42:8329", - }, - } - - var out struct{} - if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { - t.Fatalf("err: %v", err) - } - } - - m := new(dns.Msg) - m.SetQuestion("9.2.3.8.2.4.0.0.0.0.f.f.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.", dns.TypeANY) - - c := new(dns.Client) - in, _, err := c.Exchange(m, a.DNSAddr()) - if err != nil { - t.Fatalf("err: %v", err) - } - - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) - } - - ptrRec, ok := in.Answer[0].(*dns.PTR) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if ptrRec.Ptr != serviceCanonicalDNSName("db", "service", "dc1", "consul", nil)+"." { - t.Fatalf("Bad: %#v", ptrRec) - } -} - -func TestDNS_ServiceReverseLookup_CustomDomain(t *testing.T) { - if testing.Short() { - t.Skip("too slow for testing.Short") - } - - t.Parallel() - a := NewTestAgent(t, ` - domain = "custom" - `) - defer a.Shutdown() - testrpc.WaitForLeader(t, a.RPC, "dc1") - - // Register a node with a service. - { - args := &structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo", - Address: "127.0.0.1", - Service: &structs.NodeService{ - Service: "db", - Tags: []string{"primary"}, - Port: 12345, - Address: "127.0.0.2", - }, - } - - var out struct{} - if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { - t.Fatalf("err: %v", err) - } - } - - m := new(dns.Msg) - m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) - - c := new(dns.Client) - in, _, err := c.Exchange(m, a.DNSAddr()) - if err != nil { - t.Fatalf("err: %v", err) - } - - if len(in.Answer) != 1 { - t.Fatalf("Bad: %#v", in) - } - - ptrRec, ok := in.Answer[0].(*dns.PTR) - if !ok { - t.Fatalf("Bad: %#v", in.Answer[0]) - } - if ptrRec.Ptr != serviceCanonicalDNSName("db", "service", "dc1", "custom", nil)+"." { - t.Fatalf("Bad: %#v", ptrRec) - } -} - func TestDNS_SOA_Settings(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short") @@ -8050,62 +7758,6 @@ func TestDNS_Compression_Query(t *testing.T) { } } -func TestDNS_Compression_ReverseLookup(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") - - // Register node. - args := &structs.RegisterRequest{ - Datacenter: "dc1", - Node: "foo2", - Address: "127.0.0.2", - } - var out struct{} - if err := a.RPC(context.Background(), "Catalog.Register", args, &out); err != nil { - t.Fatalf("err: %v", err) - } - - m := new(dns.Msg) - m.SetQuestion("2.0.0.127.in-addr.arpa.", dns.TypeANY) - - conn, err := dns.Dial("udp", a.DNSAddr()) - if err != nil { - t.Fatalf("err: %v", err) - } - - // Do a manual exchange with compression on (the default). - if err := conn.WriteMsg(m); err != nil { - t.Fatalf("err: %v", err) - } - p := make([]byte, dns.MaxMsgSize) - compressed, err := conn.Read(p) - if err != nil { - t.Fatalf("err: %v", err) - } - - // Disable compression and try again. - a.DNSDisableCompression(true) - if err := conn.WriteMsg(m); err != nil { - t.Fatalf("err: %v", err) - } - unc, err := conn.Read(p) - if err != nil { - t.Fatalf("err: %v", err) - } - - // We can't see the compressed status given the DNS API, so we just make - // sure the message is smaller to see if it's respecting the flag. - if compressed == 0 || unc == 0 || compressed >= unc { - t.Fatalf("doesn't look compressed: %d vs. %d", compressed, unc) - } -} - func TestDNS_Compression_Recurse(t *testing.T) { if testing.Short() { t.Skip("too slow for testing.Short")