From 3a23f5139b63e8bfa4636183b31ab80e2fc17263 Mon Sep 17 00:00:00 2001 From: Michael Zalimeni Date: Thu, 9 May 2024 12:20:29 -0400 Subject: [PATCH] security: remove `coredns` dependency We've already replaced this dependency in `main` and `release/1.18`. Given bumping it is non-trivial on older versions of Consul, instead we can remove the single use of its `dnsutil` package by backporting a portion of the replacing changes. Resolves CVE-2024-0874. --- agent/dns.go | 8 +++-- go.mod | 1 - go.sum | 2 -- internal/dnsutil/dns.go | 55 ++++++++++++++++++++++++++++++++++ internal/dnsutil/dns_test.go | 57 ++++++++++++++++++++++++++++++++++++ 5 files changed, 118 insertions(+), 5 deletions(-) create mode 100644 internal/dnsutil/dns.go create mode 100644 internal/dnsutil/dns_test.go diff --git a/agent/dns.go b/agent/dns.go index 5804dc97dd..90718d1daf 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" @@ -28,6 +27,7 @@ import ( agentdns "github.com/hashicorp/consul/agent/dns" "github.com/hashicorp/consul/agent/structs" "github.com/hashicorp/consul/api" + libdns "github.com/hashicorp/consul/internal/dnsutil" "github.com/hashicorp/consul/ipaddr" "github.com/hashicorp/consul/lib" "github.com/hashicorp/consul/logging" @@ -448,7 +448,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/go.mod b/go.mod index dc23eac792..e1b7aabbc4 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/docker/go-connections v0.4.0 github.com/envoyproxy/go-control-plane v0.11.1 diff --git a/go.sum b/go.sum index ce86d3adf6..389dedb843 100644 --- a/go.sum +++ b/go.sum @@ -164,8 +164,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 new file mode 100644 index 0000000000..52658e9f21 --- /dev/null +++ b/internal/dnsutil/dns.go @@ -0,0 +1,55 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package dnsutil + +import ( + "net" + "slices" + "strings" + + "github.com/miekg/dns" +) + +type TranslateAddressAccept int + +const ( + arpaLabel = "arpa" + arpaIPV4Label = "in-addr" + arpaIPV6Label = "ip6" +) + +// 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 new file mode 100644 index 0000000000..d48c1dcd61 --- /dev/null +++ b/internal/dnsutil/dns_test.go @@ -0,0 +1,57 @@ +// Copyright (c) HashiCorp, Inc. +// SPDX-License-Identifier: BUSL-1.1 + +package dnsutil + +import ( + "net" + "testing" + + "github.com/stretchr/testify/require" +) + +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) + }) + } +}