Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure TXT RRs always end up in the Additional section except for ANY or TXT queries #4362

Merged
merged 1 commit into from
Jul 10, 2018
Merged

Ensure TXT RRs always end up in the Additional section except for ANY or TXT queries #4362

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 38 additions & 22 deletions agent/dns.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,8 +376,11 @@ func (d *DNSServer) nameservers(edns bool) (ns []dns.RR, extra []dns.RR) {
}
ns = append(ns, nsrr)

glue := d.formatNodeRecord(nil, addr, fqdn, dns.TypeANY, d.config.NodeTTL, edns, false)
glue, meta := d.formatNodeRecord(nil, addr, fqdn, dns.TypeANY, d.config.NodeTTL, edns)
extra = append(extra, glue...)
if meta != nil && d.config.NodeMetaTXT {
extra = append(extra, meta...)
}

// don't provide more than 3 servers
if len(ns) >= 3 {
Expand Down Expand Up @@ -592,10 +595,15 @@ RPC:
n := out.NodeServices.Node
edns := req.IsEdns0() != nil
addr := d.agent.TranslateAddress(datacenter, n.Address, n.TaggedAddresses)
records := d.formatNodeRecord(out.NodeServices.Node, addr, req.Question[0].Name, qType, d.config.NodeTTL, edns, true)
records, meta := d.formatNodeRecord(out.NodeServices.Node, addr, req.Question[0].Name, qType, d.config.NodeTTL, edns)
if records != nil {
resp.Answer = append(resp.Answer, records...)
}
if meta != nil && (qType == dns.TypeANY || qType == dns.TypeTXT) {
resp.Answer = append(resp.Answer, meta...)
} else if meta != nil && d.config.NodeMetaTXT {
resp.Extra = append(resp.Extra, meta...)
}
}

// encodeKVasRFC1464 encodes a key-value pair according to RFC1464
Expand All @@ -619,8 +627,12 @@ func encodeKVasRFC1464(key, value string) (txt string) {
return key + "=" + value
}

// formatNodeRecord takes a Node and returns an A, AAAA, TXT or CNAME record
func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qType uint16, ttl time.Duration, edns, answer bool) (records []dns.RR) {
// formatNodeRecord takes a Node and returns the RRs associated with that node
//
// The return value is two slices. The first slice is the main answer slice (containing the A, AAAA, CNAME) RRs for the node
// and the second slice contains any TXT RRs created from the node metadata. It is up to the caller to determine where the
// generated RRs should go and if they should be used at all.
func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qType uint16, ttl time.Duration, edns bool) (records, meta []dns.RR) {
// Parse the IP
ip := net.ParseIP(addr)
var ipv4 net.IP
Expand Down Expand Up @@ -681,26 +693,14 @@ func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qTy
}
}

node_meta_txt := false

if node == nil {
node_meta_txt = false
} else if answer {
node_meta_txt = true
} else {
// Use configuration when the TXT RR would
// end up in the Additional section of the
// DNS response
node_meta_txt = d.config.NodeMetaTXT
}

if node_meta_txt {
if node != nil {
for key, value := range node.Meta {
txt := value
if !strings.HasPrefix(strings.ToLower(key), "rfc1035-") {
txt = encodeKVasRFC1464(key, value)
}
records = append(records, &dns.TXT{

meta = append(meta, &dns.TXT{
Hdr: dns.RR_Header{
Name: qName,
Rrtype: dns.TypeTXT,
Expand All @@ -712,7 +712,7 @@ func (d *DNSServer) formatNodeRecord(node *structs.Node, addr, qName string, qTy
}
}

return records
return records, meta
}

// indexRRs populates a map which indexes a given list of RRs by name. NOTE that
Expand Down Expand Up @@ -1167,9 +1167,21 @@ func (d *DNSServer) serviceNodeRecords(dc string, nodes structs.CheckServiceNode
handled[addr] = struct{}{}

// Add the node record
records := d.formatNodeRecord(node.Node, addr, qName, qType, ttl, edns, true)
had_answer := false
records, meta := d.formatNodeRecord(node.Node, addr, qName, qType, ttl, edns)
if records != nil {
resp.Answer = append(resp.Answer, records...)
had_answer = true
}

if meta != nil && (qType == dns.TypeANY || qType == dns.TypeTXT) {
resp.Answer = append(resp.Answer, meta...)
had_answer = true
} else if meta != nil && d.config.NodeMetaTXT {
resp.Extra = append(resp.Extra, meta...)
}

if had_answer {
count++
if count == d.config.ARecordLimit {
// We stop only if greater than 0 or we reached the limit
Expand Down Expand Up @@ -1216,7 +1228,7 @@ func (d *DNSServer) serviceSRVRecords(dc string, nodes structs.CheckServiceNodes
}

// Add the extra record
records := d.formatNodeRecord(node.Node, addr, srvRec.Target, dns.TypeANY, ttl, edns, false)
records, meta := d.formatNodeRecord(node.Node, addr, srvRec.Target, dns.TypeANY, ttl, edns)
if len(records) > 0 {
// Use the node address if it doesn't differ from the service address
if addr == node.Node.Address {
Expand Down Expand Up @@ -1246,6 +1258,10 @@ func (d *DNSServer) serviceSRVRecords(dc string, nodes structs.CheckServiceNodes
resp.Extra = append(resp.Extra, records...)
}
}

if meta != nil && d.config.NodeMetaTXT {
resp.Extra = append(resp.Extra, meta...)
}
}
}
}
Expand Down
130 changes: 83 additions & 47 deletions agent/dns_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,9 @@ func TestDNS_NodeLookup(t *testing.T) {
TaggedAddresses: map[string]string{
"wan": "127.0.0.2",
},
NodeMeta: map[string]string{
"key": "value",
},
}

var out struct{}
Expand All @@ -190,72 +193,70 @@ func TestDNS_NodeLookup(t *testing.T) {

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)
}
require.NoError(t, err)
require.Len(t, in.Answer, 2)
require.Len(t, in.Extra, 0)

aRec, ok := in.Answer[0].(*dns.A)
if !ok {
t.Fatalf("Bad: %#v", in.Answer[0])
}
if aRec.A.String() != "127.0.0.1" {
t.Fatalf("Bad: %#v", in.Answer[0])
}
if aRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Answer[0])
}
require.True(t, ok, "First answer is not an A record")
require.Equal(t, "127.0.0.1", aRec.A.String())
require.Equal(t, uint32(0), aRec.Hdr.Ttl)

txt, ok := in.Answer[1].(*dns.TXT)
require.True(t, ok, "Second answer is not a TXT record")
require.Len(t, txt.Txt, 1)
require.Equal(t, "key=value", txt.Txt[0])

// Re-do the query, but only for an A RR

m = new(dns.Msg)
m.SetQuestion("foo.node.consul.", dns.TypeA)

c = new(dns.Client)
in, _, err = c.Exchange(m, a.DNSAddr())
require.NoError(t, err)
require.Len(t, in.Answer, 1)
require.Len(t, in.Extra, 1)

aRec, ok = in.Answer[0].(*dns.A)
require.True(t, ok, "Answer is not an A record")
require.Equal(t, "127.0.0.1", aRec.A.String())
require.Equal(t, uint32(0), aRec.Hdr.Ttl)

txt, ok = in.Extra[0].(*dns.TXT)
require.True(t, ok, "Extra record is not a TXT record")
require.Len(t, txt.Txt, 1)
require.Equal(t, "key=value", txt.Txt[0])

// Re-do the query, but specify the DC
m = new(dns.Msg)
m.SetQuestion("foo.node.dc1.consul.", 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)
}
require.NoError(t, err)
require.Len(t, in.Answer, 2)
require.Len(t, in.Extra, 0)

aRec, ok = in.Answer[0].(*dns.A)
if !ok {
t.Fatalf("Bad: %#v", in.Answer[0])
}
if aRec.A.String() != "127.0.0.1" {
t.Fatalf("Bad: %#v", in.Answer[0])
}
if aRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Answer[0])
}
require.True(t, ok, "First answer is not an A record")
require.Equal(t, "127.0.0.1", aRec.A.String())
require.Equal(t, uint32(0), aRec.Hdr.Ttl)

txt, ok = in.Answer[1].(*dns.TXT)
require.True(t, ok, "Second answer is not a TXT record")

// lookup a non-existing node, we should receive a SOA
m = new(dns.Msg)
m.SetQuestion("nofoo.node.dc1.consul.", dns.TypeANY)

c = new(dns.Client)
in, _, err = c.Exchange(m, a.DNSAddr())
if err != nil {
t.Fatalf("err: %v", err)
}

if len(in.Ns) != 1 {
t.Fatalf("Bad: %#v %#v", in, len(in.Answer))
}

require.NoError(t, err)
require.Len(t, in.Ns, 1)
soaRec, ok := in.Ns[0].(*dns.SOA)
if !ok {
t.Fatalf("Bad: %#v", in.Ns[0])
}
if soaRec.Hdr.Ttl != 0 {
t.Fatalf("Bad: %#v", in.Ns[0])
}

require.True(t, ok, "NS RR is not a SOA record")
require.Equal(t, uint32(0), soaRec.Hdr.Ttl)
}

func TestDNS_CaseInsensitiveNodeLookup(t *testing.T) {
Expand Down Expand Up @@ -598,6 +599,41 @@ func TestDNS_NodeLookup_ANY_DontSuppressTXT(t *testing.T) {
verify.Values(t, "answer", in.Answer, wantAnswer)
}

func TestDNS_NodeLookup_A_SuppressTXT(t *testing.T) {
a := NewTestAgent(t.Name(), `dns_config = { enable_additional_node_meta_txt = false }`)
defer a.Shutdown()

args := &structs.RegisterRequest{
Datacenter: "dc1",
Node: "bar",
Address: "127.0.0.1",
NodeMeta: map[string]string{
"key": "value",
},
}

var out struct{}
require.NoError(t, a.RPC("Catalog.Register", args, &out))

m := new(dns.Msg)
m.SetQuestion("bar.node.consul.", dns.TypeA)

c := new(dns.Client)
in, _, err := c.Exchange(m, a.DNSAddr())
require.NoError(t, err)

wantAnswer := []dns.RR{
&dns.A{
Hdr: dns.RR_Header{Name: "bar.node.consul.", Rrtype: dns.TypeA, Class: dns.ClassINET, Rdlength: 0x4},
A: []byte{0x7f, 0x0, 0x0, 0x1}, // 127.0.0.1
},
}
verify.Values(t, "answer", in.Answer, wantAnswer)

// ensure TXT RR suppression
require.Len(t, in.Extra, 0)
}

func TestDNS_EDNS0(t *testing.T) {
t.Parallel()
a := NewTestAgent(t.Name(), "")
Expand Down