From 0ca7313b07a30b20bd3c7c68fdc40e961e706e7d Mon Sep 17 00:00:00 2001 From: Dan Stough Date: Mon, 29 Jan 2024 11:40:10 -0500 Subject: [PATCH] feat(v2dns): add PTR query support (#20362) --- agent/agent.go | 2 +- agent/discovery/discovery.go | 26 ++- agent/discovery/query_fetcher_v1.go | 91 +++++++- agent/discovery/query_fetcher_v1_test.go | 3 +- agent/discovery/query_fetcher_v2.go | 2 +- agent/dns.go | 7 +- agent/dns/router.go | 52 +++-- agent/dns/router_ce.go | 33 +++ agent/dns/router_ce_test.go | 149 ++++++++++++++ agent/dns/router_test.go | 252 +++++++++++++++++++++-- agent/dns_service_lookup_test.go | 8 +- agent/dns_test.go | 8 +- go.mod | 1 - go.sum | 2 - internal/dnsutil/dns.go | 48 ++++- internal/dnsutil/dns_test.go | 47 +++++ 16 files changed, 665 insertions(+), 66 deletions(-) create mode 100644 agent/dns/router_ce.go create mode 100644 agent/dns/router_ce_test.go diff --git a/agent/agent.go b/agent/agent.go index f4f397d538..cda7ed0290 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -1106,7 +1106,7 @@ func (a *Agent) listenAndServeV2DNS() error { if a.baseDeps.UseV2Resources() { a.catalogDataFetcher = discovery.NewV2DataFetcher(a.config) } else { - a.catalogDataFetcher = discovery.NewV1DataFetcher(a.config, a.RPC, a.logger.Named("catalog-data-fetcher")) + a.catalogDataFetcher = discovery.NewV1DataFetcher(a.config, a.AgentEnterpriseMeta(), a.RPC, a.logger.Named("catalog-data-fetcher")) } // Generate a Query Processor with the appropriate data fetcher diff --git a/agent/discovery/discovery.go b/agent/discovery/discovery.go index 5228626a2f..e04e3c966f 100644 --- a/agent/discovery/discovery.go +++ b/agent/discovery/discovery.go @@ -15,6 +15,7 @@ import ( var ( ErrNoData = fmt.Errorf("no data") ErrECSNotGlobal = fmt.Errorf("ECS response is not global") + ErrNotSupported = fmt.Errorf("not supported") ) // ECSNotGlobalError may be used to wrap an error or nil, to indicate that the @@ -106,16 +107,23 @@ const ( // It is the responsibility of the DNS encoder to know what to do with // each Result, based on the query type. type Result struct { - Address string // A/AAAA/CNAME records - could be used in the Extra section. CNAME is required to handle hostname addresses in workloads & nodes. - Weight uint32 // SRV queries - Port uint32 // SRV queries - Metadata []string // Used to collect metadata into TXT Records - Type ResultType + Address string // A/AAAA/CNAME records - could be used in the Extra section. CNAME is required to handle hostname addresses in workloads & nodes. + Weight uint32 // SRV queries + Port uint32 // SRV queries + Metadata []string // Used to collect metadata into TXT Records + Type ResultType // Used to reconstruct the fqdn name of the resource // Used in SRV & PTR queries to point at an A/AAAA Record. - // In V1, this could be a full-qualified Service or Node name. - // In V2, this is generally a fully-qualified Workload name. Target string + + Tenancy ResultTenancy +} + +// ResultTenancy is used to reconstruct the fqdn name of the resource. +type ResultTenancy struct { + PeerName string + Datacenter string + EnterpriseMeta acl.EnterpriseMeta // TODO (v2-dns): need something that is compatible with the V2 catalog } // LookupType is used by the CatalogDataFetcher to properly filter endpoints. @@ -198,6 +206,6 @@ func (p *QueryProcessor) QueryByName(query *Query, ctx Context) ([]*Result, erro } // QueryByIP is used to look up a service or node from an IP address. -func (p *QueryProcessor) QueryByIP(ip net.IP, ctx Context) ([]*Result, error) { - return p.dataFetcher.FetchRecordsByIp(ctx, ip) +func (p *QueryProcessor) QueryByIP(ip net.IP, reqCtx Context) ([]*Result, error) { + return p.dataFetcher.FetchRecordsByIp(reqCtx, ip) } diff --git a/agent/discovery/query_fetcher_v1.go b/agent/discovery/query_fetcher_v1.go index b1a4553c33..4d89ec8645 100644 --- a/agent/discovery/query_fetcher_v1.go +++ b/agent/discovery/query_fetcher_v1.go @@ -5,12 +5,14 @@ package discovery import ( "context" + "fmt" "net" "sync/atomic" "time" "github.com/hashicorp/go-hclog" + "github.com/hashicorp/consul/acl" "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/structs" ) @@ -23,6 +25,10 @@ const ( // v1DataFetcherDynamicConfig is used to store the dynamic configuration of the V1 data fetcher. type v1DataFetcherDynamicConfig struct { + // Default request tenancy + datacenter string + + // Catalog configuration allowStale bool maxStale time.Duration useCache bool @@ -32,19 +38,22 @@ type v1DataFetcherDynamicConfig struct { // V1DataFetcher is used to fetch data from the V1 catalog. type V1DataFetcher struct { - dynamicConfig atomic.Value - logger hclog.Logger + defaultEnterpriseMeta acl.EnterpriseMeta + dynamicConfig atomic.Value + logger hclog.Logger rpcFunc func(ctx context.Context, method string, args interface{}, reply interface{}) error } // NewV1DataFetcher creates a new V1 data fetcher. func NewV1DataFetcher(config *config.RuntimeConfig, + entMeta *acl.EnterpriseMeta, rpcFunc func(ctx context.Context, method string, args interface{}, reply interface{}) error, logger hclog.Logger) *V1DataFetcher { f := &V1DataFetcher{ - rpcFunc: rpcFunc, - logger: logger, + defaultEnterpriseMeta: *entMeta, + rpcFunc: rpcFunc, + logger: logger, } f.LoadConfig(config) return f @@ -53,6 +62,7 @@ func NewV1DataFetcher(config *config.RuntimeConfig, // LoadConfig loads the configuration for the V1 data fetcher. func (f *V1DataFetcher) LoadConfig(config *config.RuntimeConfig) { dynamicConfig := &v1DataFetcherDynamicConfig{ + datacenter: config.Datacenter, allowStale: config.DNSAllowStale, maxStale: config.DNSMaxStale, useCache: config.DNSUseCache, @@ -101,14 +111,81 @@ func (f *V1DataFetcher) FetchVirtualIP(ctx Context, req *QueryPayload) (*Result, } // FetchRecordsByIp is used for PTR requests to look up a service/node from an IP. -func (f *V1DataFetcher) FetchRecordsByIp(ctx Context, ip net.IP) ([]*Result, error) { - return nil, nil +// The search is performed in the agent's partition and over all namespaces (or those allowed by the ACL token). +func (f *V1DataFetcher) FetchRecordsByIp(reqCtx Context, ip net.IP) ([]*Result, error) { + configCtx := f.dynamicConfig.Load().(*v1DataFetcherDynamicConfig) + targetIP := ip.String() + + var results []*Result + + args := structs.DCSpecificRequest{ + Datacenter: configCtx.datacenter, + QueryOptions: structs.QueryOptions{ + Token: reqCtx.Token, + AllowStale: configCtx.allowStale, + }, + } + var out structs.IndexedNodes + + // TODO: Replace ListNodes with an internal RPC that can do the filter + // server side to avoid transferring the entire node list. + if err := f.rpcFunc(context.Background(), "Catalog.ListNodes", &args, &out); err == nil { + for _, n := range out.Nodes { + if targetIP == n.Address { + results = append(results, &Result{ + Address: n.Address, + Type: ResultTypeNode, + Target: n.Node, + Tenancy: ResultTenancy{ + EnterpriseMeta: f.defaultEnterpriseMeta, + Datacenter: configCtx.datacenter, + }, + }) + return results, nil + } + } + } + + // only look into the services if we didn't find a node + sargs := structs.ServiceSpecificRequest{ + Datacenter: configCtx.datacenter, + QueryOptions: structs.QueryOptions{ + Token: reqCtx.Token, + AllowStale: configCtx.allowStale, + }, + ServiceAddress: targetIP, + EnterpriseMeta: *f.defaultEnterpriseMeta.WithWildcardNamespace(), + } + + var sout structs.IndexedServiceNodes + if err := f.rpcFunc(context.Background(), "Catalog.ServiceNodes", &sargs, &sout); err == nil { + for _, n := range sout.ServiceNodes { + if n.ServiceAddress == targetIP { + results = append(results, &Result{ + Address: n.ServiceAddress, + Type: ResultTypeService, + Target: n.ServiceName, + Tenancy: ResultTenancy{ + EnterpriseMeta: f.defaultEnterpriseMeta, + Datacenter: configCtx.datacenter, + }, + }) + return results, nil + } + } + } + + // nothing found locally, recurse + // TODO: (v2-dns) implement recursion + //d.handleRecurse(resp, req) + + return nil, fmt.Errorf("unhandled error in FetchRecordsByIp") } // FetchWorkload fetches a single Result associated with // V2 Workload. V2-only. func (f *V1DataFetcher) FetchWorkload(ctx Context, req *QueryPayload) (*Result, error) { - return nil, nil + return nil, ErrNotSupported } // FetchPreparedQuery evaluates the results of a prepared query. diff --git a/agent/discovery/query_fetcher_v1_test.go b/agent/discovery/query_fetcher_v1_test.go index 23864b4d7e..25371f1138 100644 --- a/agent/discovery/query_fetcher_v1_test.go +++ b/agent/discovery/query_fetcher_v1_test.go @@ -11,6 +11,7 @@ import ( "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" + "github.com/hashicorp/consul/acl" cachetype "github.com/hashicorp/consul/agent/cache-types" "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/structs" @@ -95,7 +96,7 @@ func Test_FetchVirtualIP(t *testing.T) { *reply = tc.expectedResult.Address } }) - df := NewV1DataFetcher(rc, mockRPC.RPC, logger) + df := NewV1DataFetcher(rc, acl.DefaultEnterpriseMeta(), mockRPC.RPC, logger) result, err := df.FetchVirtualIP(tc.context, tc.queryPayload) require.Equal(t, tc.expectedErr, err) diff --git a/agent/discovery/query_fetcher_v2.go b/agent/discovery/query_fetcher_v2.go index 2cefc656af..0bcd69d62f 100644 --- a/agent/discovery/query_fetcher_v2.go +++ b/agent/discovery/query_fetcher_v2.go @@ -66,5 +66,5 @@ func (f *V2DataFetcher) FetchWorkload(ctx Context, req *QueryPayload) (*Result, // FetchPreparedQuery is used to fetch a prepared query from the V2 catalog. // Deprecated in V2. func (f *V2DataFetcher) FetchPreparedQuery(ctx Context, req *QueryPayload) ([]*Result, error) { - return nil, nil + return nil, ErrNotSupported } diff --git a/agent/dns.go b/agent/dns.go index 6fe3524692..1bbf485fcb 100644 --- a/agent/dns.go +++ b/agent/dns.go @@ -18,7 +18,6 @@ import ( "github.com/armon/go-metrics" "github.com/armon/go-metrics/prometheus" "github.com/armon/go-radix" - "github.com/coredns/coredns/plugin/pkg/dnsutil" "github.com/hashicorp/go-hclog" "github.com/miekg/dns" @@ -470,7 +469,11 @@ func (d *DNSServer) handlePtr(resp dns.ResponseWriter, req *dns.Msg) { // only look into the services if we didn't find a node if len(m.Answer) == 0 { // lookup the service address - serviceAddress := dnsutil.ExtractAddressFromReverse(qName) + ip := libdns.IPFromARPA(qName) + var serviceAddress string + if ip != nil { + serviceAddress = ip.String() + } sargs := structs.ServiceSpecificRequest{ Datacenter: datacenter, QueryOptions: structs.QueryOptions{ diff --git a/agent/dns/router.go b/agent/dns/router.go index cb437ca142..358aa0ce7a 100644 --- a/agent/dns/router.go +++ b/agent/dns/router.go @@ -8,7 +8,6 @@ import ( "errors" "fmt" "net" - "strings" "sync/atomic" "time" @@ -20,13 +19,15 @@ import ( "github.com/hashicorp/consul/agent/config" "github.com/hashicorp/consul/agent/discovery" "github.com/hashicorp/consul/agent/structs" + "github.com/hashicorp/consul/internal/dnsutil" "github.com/hashicorp/consul/logging" ) const ( addrLabel = "addr" - arpaDomain = "in-addr.arpa." + arpaDomain = "arpa." + arpaLabel = "arpa" suffixFailover = "failover." suffixNoFailover = "no-failover." @@ -214,9 +215,12 @@ func (r *Router) getQueryResults(req *dns.Msg, reqCtx discovery.Context, reqType } return r.processor.QueryByName(query, reqCtx) case requestTypeIP: - // TODO (v2-dns): implement requestTypeIP - // This will call discovery.QueryByIP - return nil, errors.New("requestTypeIP not implemented") + ip := dnsutil.IPFromARPA(req.Question[0].Name) + if ip == nil { + r.logger.Error("error building IP from DNS request", "name", req.Question[0].Name) + return nil, errNameNotFound + } + return r.processor.QueryByIP(ip, reqCtx) case requestTypeAddress: return buildAddressResults(req) } @@ -377,11 +381,12 @@ func isPTRSubdomain(domain string) bool { labels := dns.SplitDomainName(domain) labelCount := len(labels) - if labelCount < 3 { + // We keep this check brief so we can have more specific error handling later. + if labelCount < 1 { return false } - return fmt.Sprintf("%s.%s.", labels[labelCount-2], labels[labelCount-1]) == arpaDomain + return labels[labelCount-1] == arpaLabel } // getDynamicRouterConfig takes agent config and creates/resets the config used by DNS Router @@ -567,25 +572,34 @@ func appendResultToDNSResponse(result *discovery.Result, req *dns.Msg, resp *dns // TODO (v2-dns): skip records that refer to a workload/node that don't have a valid DNS name. // Special case responses - switch qType { - case dns.TypeSOA: - // TODO (v2-dns): fqdn in V1 has the datacenter included, this would need to be added to discovery.Result - // to be returned in the result. - fqdn := fmt.Sprintf("%s.%s.%s", result.Target, strings.ToLower(string(result.Type)), domain) - extraRecord, _ := makeRecord(fqdn, ip, ttl) // TODO (v2-dns): this is not sufficient, because recursion and CNAMES are supported - - resp.Ns = append(resp.Ns, makeNSRecord(domain, fqdn, ttl)) - resp.Extra = append(resp.Extra, extraRecord) + switch { + // PTR requests are first since they are a special case of domain overriding question type + case parseRequestType(req) == requestTypeIP: + ptr := &dns.PTR{ + Hdr: dns.RR_Header{Name: qName, Rrtype: dns.TypePTR, Class: dns.ClassINET, Ttl: 0}, + Ptr: canonicalNameForResult(result, domain), + } + resp.Answer = append(resp.Answer, ptr) return - case dns.TypeNS: + case qType == dns.TypeNS: // TODO (v2-dns): fqdn in V1 has the datacenter included, this would need to be added to discovery.Result - fqdn := fmt.Sprintf("%s.%s.%s.", result.Target, strings.ToLower(string(result.Type)), domain) + fqdn := canonicalNameForResult(result, domain) extraRecord, _ := makeRecord(fqdn, ip, ttl) // TODO (v2-dns): this is not sufficient, because recursion and CNAMES are supported resp.Answer = append(resp.Ns, makeNSRecord(domain, fqdn, ttl)) resp.Extra = append(resp.Extra, extraRecord) return - case dns.TypeSRV: + + case qType == dns.TypeSOA: + // TODO (v2-dns): fqdn in V1 has the datacenter included, this would need to be added to discovery.Result + // to be returned in the result. + fqdn := canonicalNameForResult(result, domain) + extraRecord, _ := makeRecord(fqdn, ip, ttl) // TODO (v2-dns): this is not sufficient, because recursion and CNAMES are supported + + resp.Ns = append(resp.Ns, makeNSRecord(domain, fqdn, ttl)) + resp.Extra = append(resp.Extra, extraRecord) + return + case qType == dns.TypeSRV: // We put A/AAAA/CNAME records in the additional section for SRV requests resp.Extra = append(resp.Extra, record) diff --git a/agent/dns/router_ce.go b/agent/dns/router_ce.go new file mode 100644 index 0000000000..d5fff53235 --- /dev/null +++ b/agent/dns/router_ce.go @@ -0,0 +1,33 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +//go:build !consulent + +package dns + +import ( + "fmt" + + "github.com/hashicorp/consul/agent/discovery" +) + +func canonicalNameForResult(result *discovery.Result, domain string) string { + switch result.Type { + case discovery.ResultTypeService: + return fmt.Sprintf("%s.%s.%s.%s", result.Target, "service", result.Tenancy.Datacenter, domain) + case discovery.ResultTypeNode: + if result.Tenancy.PeerName != "" { + // We must return a more-specific DNS name for peering so + // that there is no ambiguity with lookups. + return fmt.Sprintf("%s.node.%s.peer.%s", + result.Target, + result.Tenancy.PeerName, + domain) + } + // Return a simpler format for non-peering nodes. + return fmt.Sprintf("%s.node.%s.%s", result.Target, result.Tenancy.Datacenter, domain) + case discovery.ResultTypeWorkload: + return fmt.Sprintf("%s.workload.%s", result.Target, domain) + } + return "" +} diff --git a/agent/dns/router_ce_test.go b/agent/dns/router_ce_test.go new file mode 100644 index 0000000000..69f73e2dbf --- /dev/null +++ b/agent/dns/router_ce_test.go @@ -0,0 +1,149 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +//go:build !consulent + +package dns + +import ( + "net" + "testing" + + "github.com/miekg/dns" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + + "github.com/hashicorp/consul/agent/discovery" +) + +func getAdditionalTestCases(t *testing.T) []HandleTestCase { + // PTR Lookups + return []HandleTestCase{ + // PTR Lookups + { + name: "PTR Lookup for node w/ peer name, query type is ANY", + request: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + }, + Question: []dns.Question{ + { + Name: "4.3.2.1.in-addr.arpa", + Qtype: dns.TypeANY, + Qclass: dns.ClassINET, + }, + }, + }, + configureDataFetcher: func(fetcher discovery.CatalogDataFetcher) { + results := []*discovery.Result{ + { + Address: "1.2.3.4", + Type: discovery.ResultTypeNode, + Target: "foo", + Tenancy: discovery.ResultTenancy{ + Datacenter: "dc2", + PeerName: "peer1", + }, + }, + } + + fetcher.(*discovery.MockCatalogDataFetcher). + On("FetchRecordsByIp", mock.Anything, mock.Anything). + Return(results, nil). + Run(func(args mock.Arguments) { + req := args.Get(1).(net.IP) + + require.NotNil(t, req) + require.Equal(t, "1.2.3.4", req.String()) + }) + }, + response: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + Response: true, + Authoritative: true, + }, + Compress: true, + Question: []dns.Question{ + { + Name: "4.3.2.1.in-addr.arpa.", + Qtype: dns.TypeANY, + Qclass: dns.ClassINET, + }, + }, + Answer: []dns.RR{ + &dns.PTR{ + Hdr: dns.RR_Header{ + Name: "4.3.2.1.in-addr.arpa.", + Rrtype: dns.TypePTR, + Class: dns.ClassINET, + }, + Ptr: "foo.node.peer1.peer.consul.", + }, + }, + }, + }, + { + name: "PTR Lookup for service, query type is PTR", + request: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + }, + Question: []dns.Question{ + { + Name: "4.3.2.1.in-addr.arpa", + Qtype: dns.TypePTR, + Qclass: dns.ClassINET, + }, + }, + }, + configureDataFetcher: func(fetcher discovery.CatalogDataFetcher) { + results := []*discovery.Result{ + { + Address: "1.2.3.4", + Type: discovery.ResultTypeService, + Target: "foo", + Tenancy: discovery.ResultTenancy{ + Datacenter: "dc2", + }, + }, + } + + fetcher.(*discovery.MockCatalogDataFetcher). + On("FetchRecordsByIp", mock.Anything, mock.Anything). + Return(results, nil). + Run(func(args mock.Arguments) { + req := args.Get(1).(net.IP) + + require.NotNil(t, req) + require.Equal(t, "1.2.3.4", req.String()) + }) + }, + response: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + Response: true, + Authoritative: true, + }, + Compress: true, + Question: []dns.Question{ + { + Name: "4.3.2.1.in-addr.arpa.", + Qtype: dns.TypePTR, + Qclass: dns.ClassINET, + }, + }, + Answer: []dns.RR{ + &dns.PTR{ + Hdr: dns.RR_Header{ + Name: "4.3.2.1.in-addr.arpa.", + Rrtype: dns.TypePTR, + Class: dns.ClassINET, + }, + Ptr: "foo.service.dc2.consul.", + }, + }, + }, + }, + } +} diff --git a/agent/dns/router_test.go b/agent/dns/router_test.go index d799d27f19..3064204b24 100644 --- a/agent/dns/router_test.go +++ b/agent/dns/router_test.go @@ -28,20 +28,21 @@ import ( // 3. Something case-insensitive // 4. Test the edns settings. -func Test_HandleRequest(t *testing.T) { - type testCase struct { - name string - agentConfig *config.RuntimeConfig // This will override the default test Router Config - configureDataFetcher func(fetcher discovery.CatalogDataFetcher) - configureRecursor func(recursor dnsRecursor) - mockProcessorError error - request *dns.Msg - requestContext *discovery.Context - remoteAddress net.Addr - response *dns.Msg - } +type HandleTestCase struct { + name string + agentConfig *config.RuntimeConfig // This will override the default test Router Config + configureDataFetcher func(fetcher discovery.CatalogDataFetcher) + configureRecursor func(recursor dnsRecursor) + mockProcessorError error + request *dns.Msg + requestContext *discovery.Context + remoteAddress net.Addr + response *dns.Msg +} - testCases := []testCase{ +func Test_HandleRequest(t *testing.T) { + + testCases := []HandleTestCase{ // recursor queries { name: "recursors not configured, non-matching domain", @@ -1026,6 +1027,227 @@ func Test_HandleRequest(t *testing.T) { }, }, }, + // PTR Lookups + { + name: "PTR lookup for node, query type is ANY", + request: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + }, + Question: []dns.Question{ + { + Name: "4.3.2.1.in-addr.arpa", + Qtype: dns.TypeANY, + Qclass: dns.ClassINET, + }, + }, + }, + configureDataFetcher: func(fetcher discovery.CatalogDataFetcher) { + results := []*discovery.Result{ + { + Address: "1.2.3.4", + Type: discovery.ResultTypeNode, + Target: "foo", + Tenancy: discovery.ResultTenancy{ + Datacenter: "dc2", + }, + }, + } + + fetcher.(*discovery.MockCatalogDataFetcher). + On("FetchRecordsByIp", mock.Anything, mock.Anything). + Return(results, nil). + Run(func(args mock.Arguments) { + req := args.Get(1).(net.IP) + + require.NotNil(t, req) + require.Equal(t, "1.2.3.4", req.String()) + }) + }, + response: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + Response: true, + Authoritative: true, + }, + Compress: true, + Question: []dns.Question{ + { + Name: "4.3.2.1.in-addr.arpa.", + Qtype: dns.TypeANY, + Qclass: dns.ClassINET, + }, + }, + Answer: []dns.RR{ + &dns.PTR{ + Hdr: dns.RR_Header{ + Name: "4.3.2.1.in-addr.arpa.", + Rrtype: dns.TypePTR, + Class: dns.ClassINET, + }, + Ptr: "foo.node.dc2.consul.", + }, + }, + }, + }, + { + name: "PTR lookup for IPV6 node", + request: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + }, + Question: []dns.Question{ + { + Name: "b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa", + Qtype: dns.TypePTR, + Qclass: dns.ClassINET, + }, + }, + }, + configureDataFetcher: func(fetcher discovery.CatalogDataFetcher) { + results := []*discovery.Result{ + { + Address: "2001:db8::567:89ab", + Type: discovery.ResultTypeNode, + Target: "foo", + Tenancy: discovery.ResultTenancy{ + Datacenter: "dc2", + }, + }, + } + + fetcher.(*discovery.MockCatalogDataFetcher). + On("FetchRecordsByIp", mock.Anything, mock.Anything). + Return(results, nil). + Run(func(args mock.Arguments) { + req := args.Get(1).(net.IP) + + require.NotNil(t, req) + require.Equal(t, "2001:db8::567:89ab", req.String()) + }) + }, + response: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + Response: true, + Authoritative: true, + }, + Compress: true, + Question: []dns.Question{ + { + Name: "b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.", + Qtype: dns.TypePTR, + Qclass: dns.ClassINET, + }, + }, + Answer: []dns.RR{ + &dns.PTR{ + Hdr: dns.RR_Header{ + Name: "b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa.", + Rrtype: dns.TypePTR, + Class: dns.ClassINET, + }, + Ptr: "foo.node.dc2.consul.", + }, + }, + }, + }, + { + name: "PTR lookup for invalid IP address", + request: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + }, + Question: []dns.Question{ + { + Name: "257.3.2.1.in-addr.arpa", + Qtype: dns.TypeANY, + Qclass: dns.ClassINET, + }, + }, + }, + response: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + Response: true, + Authoritative: true, + Rcode: dns.RcodeNameError, + }, + Compress: true, + Question: []dns.Question{ + { + Name: "257.3.2.1.in-addr.arpa.", + Qtype: dns.TypeANY, + Qclass: dns.ClassINET, + }, + }, + Ns: []dns.RR{ + &dns.SOA{ + Hdr: dns.RR_Header{ + Name: "consul.", + Rrtype: dns.TypeSOA, + Class: dns.ClassINET, + Ttl: 4, + }, + Ns: "ns.consul.", + Serial: uint32(time.Now().Unix()), + Mbox: "hostmaster.consul.", + Refresh: 1, + Expire: 3, + Retry: 2, + Minttl: 4, + }, + }, + }, + }, + { + name: "PTR lookup for invalid subdomain", + request: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + }, + Question: []dns.Question{ + { + Name: "4.3.2.1.blah.arpa", + Qtype: dns.TypeANY, + Qclass: dns.ClassINET, + }, + }, + }, + response: &dns.Msg{ + MsgHdr: dns.MsgHdr{ + Opcode: dns.OpcodeQuery, + Response: true, + Authoritative: true, + Rcode: dns.RcodeNameError, + }, + Compress: true, + Question: []dns.Question{ + { + Name: "4.3.2.1.blah.arpa.", + Qtype: dns.TypeANY, + Qclass: dns.ClassINET, + }, + }, + Ns: []dns.RR{ + &dns.SOA{ + Hdr: dns.RR_Header{ + Name: "consul.", + Rrtype: dns.TypeSOA, + Class: dns.ClassINET, + Ttl: 4, + }, + Ns: "ns.consul.", + Serial: uint32(time.Now().Unix()), + Mbox: "hostmaster.consul.", + Refresh: 1, + Expire: 3, + Retry: 2, + Minttl: 4, + }, + }, + }, + }, // Service Lookup { name: "When no data is return from a query, send SOA", @@ -1089,7 +1311,9 @@ func Test_HandleRequest(t *testing.T) { // TODO (v2-dns): add a test to make sure only 3 records are returned } - run := func(t *testing.T, tc testCase) { + testCases = append(testCases, getAdditionalTestCases(t)...) + + run := func(t *testing.T, tc HandleTestCase) { cdf := discovery.NewMockCatalogDataFetcher(t) if tc.configureDataFetcher != nil { tc.configureDataFetcher(cdf) diff --git a/agent/dns_service_lookup_test.go b/agent/dns_service_lookup_test.go index 9d0683401e..3361477076 100644 --- a/agent/dns_service_lookup_test.go +++ b/agent/dns_service_lookup_test.go @@ -26,7 +26,7 @@ func TestDNS_ServiceReverseLookup(t *testing.T) { } t.Parallel() - for name, experimentsHCL := range getVersionHCL(false) { + for name, experimentsHCL := range getVersionHCL(true) { t.Run(name, func(t *testing.T) { a := NewTestAgent(t, experimentsHCL) defer a.Shutdown() @@ -82,7 +82,7 @@ func TestDNS_ServiceReverseLookup_IPV6(t *testing.T) { } t.Parallel() - for name, experimentsHCL := range getVersionHCL(false) { + for name, experimentsHCL := range getVersionHCL(true) { t.Run(name, func(t *testing.T) { a := NewTestAgent(t, experimentsHCL) defer a.Shutdown() @@ -138,7 +138,7 @@ func TestDNS_ServiceReverseLookup_CustomDomain(t *testing.T) { } t.Parallel() - for name, experimentsHCL := range getVersionHCL(false) { + for name, experimentsHCL := range getVersionHCL(true) { t.Run(name, func(t *testing.T) { a := NewTestAgent(t, ` domain = "custom" @@ -196,7 +196,7 @@ func TestDNS_ServiceReverseLookupNodeAddress(t *testing.T) { } t.Parallel() - for name, experimentsHCL := range getVersionHCL(false) { + for name, experimentsHCL := range getVersionHCL(true) { t.Run(name, func(t *testing.T) { a := NewTestAgent(t, experimentsHCL) defer a.Shutdown() diff --git a/agent/dns_test.go b/agent/dns_test.go index 6a1bf635b2..c6a20c2744 100644 --- a/agent/dns_test.go +++ b/agent/dns_test.go @@ -483,7 +483,7 @@ func TestDNS_ReverseLookup(t *testing.T) { } t.Parallel() - for name, experimentsHCL := range getVersionHCL(false) { + for name, experimentsHCL := range getVersionHCL(true) { t.Run(name, func(t *testing.T) { a := NewTestAgent(t, experimentsHCL) defer a.Shutdown() @@ -531,7 +531,7 @@ func TestDNS_ReverseLookup_CustomDomain(t *testing.T) { } t.Parallel() - for name, experimentsHCL := range getVersionHCL(false) { + for name, experimentsHCL := range getVersionHCL(true) { t.Run(name, func(t *testing.T) { a := NewTestAgent(t, ` domain = "custom" @@ -581,7 +581,7 @@ func TestDNS_ReverseLookup_IPV6(t *testing.T) { } t.Parallel() - for name, experimentsHCL := range getVersionHCL(false) { + for name, experimentsHCL := range getVersionHCL(true) { t.Run(name, func(t *testing.T) { a := NewTestAgent(t, experimentsHCL) defer a.Shutdown() @@ -3433,7 +3433,7 @@ func TestDNS_Compression_ReverseLookup(t *testing.T) { } t.Parallel() - for name, experimentsHCL := range getVersionHCL(false) { + for name, experimentsHCL := range getVersionHCL(true) { t.Run(name, func(t *testing.T) { a := NewTestAgent(t, experimentsHCL) diff --git a/go.mod b/go.mod index 0b0556d886..afe31e38c9 100644 --- a/go.mod +++ b/go.mod @@ -22,7 +22,6 @@ require ( github.com/armon/go-metrics v0.4.1 github.com/armon/go-radix v1.0.0 github.com/aws/aws-sdk-go v1.44.289 - github.com/coredns/coredns v1.10.1 github.com/coreos/go-oidc v2.1.0+incompatible github.com/deckarep/golang-set/v2 v2.3.1 github.com/docker/go-connections v0.4.0 diff --git a/go.sum b/go.sum index b673e561de..2b17e3b7da 100644 --- a/go.sum +++ b/go.sum @@ -181,8 +181,6 @@ github.com/cncf/xds/go v0.0.0-20211011173535-cb28da3451f1/go.mod h1:eXthEFrGJvWH github.com/cncf/xds/go v0.0.0-20230607035331-e9ce68804cb4 h1:/inchEIKaYC1Akx+H+gqO04wryn5h75LSazbRlnya1k= github.com/cncf/xds/go v0.0.0-20230607035331-e9ce68804cb4/go.mod h1:eXthEFrGJvWHgFFCl3hGmgk+/aYT6PnTQLykKQRLhEs= github.com/cockroachdb/apd v1.1.0/go.mod h1:8Sl8LxpKi29FqWXR16WEFZRNSz3SoPzUzeMeY4+DwBQ= -github.com/coredns/coredns v1.10.1 h1:6OyL7tcvYxeNHONj5iQlVM2GXBzAOq57L3/LUKP1DbA= -github.com/coredns/coredns v1.10.1/go.mod h1:oGgoY6cRrdJzKgNrsT30Hztu7/MutSHCYwqGDWngXCc= github.com/coreos/bbolt v1.3.2/go.mod h1:iRUV2dpdMOn7Bo10OQBFzIJO9kkE559Wcmn+qkEiiKk= github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE= github.com/coreos/etcd v3.3.27+incompatible h1:QIudLb9KeBsE5zyYxd1mjzRSkzLg9Wf9QlRwFgd6oTA= diff --git a/internal/dnsutil/dns.go b/internal/dnsutil/dns.go index b652ffd152..7d6877091c 100644 --- a/internal/dnsutil/dns.go +++ b/internal/dnsutil/dns.go @@ -5,11 +5,22 @@ package dnsutil import ( "errors" + "net" "regexp" + "slices" + "strings" + + "github.com/miekg/dns" ) // MaxLabelLength is the maximum length for a name that can be used in DNS. -const MaxLabelLength = 63 +const ( + MaxLabelLength = 63 + + arpaLabel = "arpa" + arpaIPV4Label = "in-addr" + arpaIPV6Label = "ip6" +) // InvalidNameRe is a regex that matches characters which can not be included in // a DNS name. @@ -35,3 +46,38 @@ func ValidateLabel(name string) error { } return nil } + +// IPFromARPA returns the net.IP address from a fully-qualified ARPA PTR domain name. +// If the address is an invalid format, it returns nil. +func IPFromARPA(arpa string) net.IP { + labels := dns.SplitDomainName(arpa) + if len(labels) != 6 && len(labels) != 34 { + return nil + } + + // The last two labels should be "in-addr" or "ip6" and "arpa" + if labels[len(labels)-1] != arpaLabel { + return nil + } + + var ip net.IP + switch labels[len(labels)-2] { + case arpaIPV4Label: + parts := labels[:len(labels)-2] + slices.Reverse(parts) + ip = net.ParseIP(strings.Join(parts, ".")) + case arpaIPV6Label: + parts := labels[:len(labels)-2] + slices.Reverse(parts) + + // Condense the different words of the address + address := strings.Join(parts[0:4], "") + for i := 4; i <= len(parts)-4; i = i + 4 { + word := parts[i : i+4] + address = address + ":" + strings.Join(word, "") + } + ip = net.ParseIP(address) + // default: fallthrough + } + return ip +} diff --git a/internal/dnsutil/dns_test.go b/internal/dnsutil/dns_test.go index 608832a69c..92b3051ca4 100644 --- a/internal/dnsutil/dns_test.go +++ b/internal/dnsutil/dns_test.go @@ -4,6 +4,7 @@ package dnsutil import ( + "net" "testing" "github.com/stretchr/testify/require" @@ -73,3 +74,49 @@ func TestDNSInvalidRegex(t *testing.T) { } } + +func Test_IPFromARPA(t *testing.T) { + testCases := []struct { + name string + input string + expected net.IP + }{ + { + name: "valid ipv4", + input: "4.3.2.1.in-addr.arpa.", + expected: net.ParseIP("1.2.3.4"), + }, + { + name: "valid ipv6", + input: "b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa", + expected: net.ParseIP("2001:db8::567:89ab"), + }, + { + name: "invalid subdomain", + input: "4.3.2.1.addressplz.arpa", + }, + { + name: "invalid ipv4 - invalid octet", + input: "277.3.2.1.in-addr.arpa", + }, + { + name: "invalid ipv4 - too short", + input: "3.2.1.in-addr.arpa", + }, + { + name: "invalid ipv6 - invalid hex char", + input: "x.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa", + }, + { + name: "invalid ipv6 - too long", + input: "d.b.a.9.8.7.6.5.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa", + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + actual := IPFromARPA(tc.input) + require.Equal(t, tc.expected, actual) + }) + } +}