diff --git a/core/dnsserver/config.go b/core/dnsserver/config.go index 4ff2ecda1..d0a432a85 100644 --- a/core/dnsserver/config.go +++ b/core/dnsserver/config.go @@ -37,11 +37,6 @@ type Config struct { // may depend on it. HTTPRequestValidateFunc func(*http.Request) bool - // If this function is not nil it will be used to further filter access - // to this handler. The primary use is to limit access to a reverse zone - // on a non-octet boundary, i.e. /17 - FilterFunc func(string) bool - // TLSConfig when listening for encrypted connections (gRPC, DNS-over-TLS). TLSConfig *tls.Config diff --git a/core/dnsserver/onstartup.go b/core/dnsserver/onstartup.go index 7aa519e98..113bf15e5 100644 --- a/core/dnsserver/onstartup.go +++ b/core/dnsserver/onstartup.go @@ -1,6 +1,9 @@ package dnsserver -import "fmt" +import ( + "fmt" + "sort" +) // startUpZones creates the text that we show when starting up: // grpc://example.com.:1055 @@ -8,7 +11,15 @@ import "fmt" func startUpZones(protocol, addr string, zones map[string]*Config) string { s := "" - for zone := range zones { + keys := make([]string, len(zones)) + i := 0 + for k := range zones { + keys[i] = k + i++ + } + sort.Strings(keys) + + for _, zone := range keys { // split addr into protocol, IP and Port _, ip, port, err := SplitProtocolHostPort(addr) diff --git a/core/dnsserver/register.go b/core/dnsserver/register.go index b4e9f4fde..505aa8580 100644 --- a/core/dnsserver/register.go +++ b/core/dnsserver/register.go @@ -4,13 +4,11 @@ import ( "flag" "fmt" "net" - "strings" "time" "github.com/coredns/caddy" "github.com/coredns/caddy/caddyfile" "github.com/coredns/coredns/plugin" - "github.com/coredns/coredns/plugin/pkg/dnsutil" "github.com/coredns/coredns/plugin/pkg/parse" "github.com/coredns/coredns/plugin/pkg/transport" ) @@ -60,6 +58,28 @@ var _ caddy.Context = &dnsContext{} func (h *dnsContext) InspectServerBlocks(sourceFile string, serverBlocks []caddyfile.ServerBlock) ([]caddyfile.ServerBlock, error) { // Normalize and check all the zone names and check for duplicates for ib, s := range serverBlocks { + // Walk the s.Keys and expand any reverse address in their proper DNS in-addr zones. If the expansions leads for + // more than one reverse zone, replace the current value and add the rest to s.Keys. + for ik, k := range s.Keys { + _, k1 := parse.Transport(k) // get rid of any dns:// or other scheme. + _, port, ipnet, err := plugin.SplitHostPort(k1) + if ipnet == nil || err != nil { // err will be caught below + continue + } + if port != "" { + port = ":" + port + } + nets := classFromCIDR(ipnet) + if len(nets) > 1 { + s.Keys[ik] = nets[0] + port // replace for the first + for _, n := range nets[1:] { // add the rest + s.Keys = append(s.Keys, n+port) + } + } + } + + serverBlocks[ib].Keys = s.Keys // important to save back the new keys that are potentially created here. + for ik, k := range s.Keys { za, err := normalizeZone(k) if err != nil { @@ -74,22 +94,6 @@ func (h *dnsContext) InspectServerBlocks(sourceFile string, serverBlocks []caddy Transport: za.Transport, } keyConfig := keyForConfig(ib, ik) - if za.IPNet == nil { - h.saveConfig(keyConfig, cfg) - continue - } - - ones, bits := za.IPNet.Mask.Size() - if (bits-ones)%8 != 0 { // only do this for non-octet boundaries - cfg.FilterFunc = func(s string) bool { - // TODO(miek): strings.ToLower! Slow and allocates new string. - addr := dnsutil.ExtractAddressFromReverse(strings.ToLower(s)) - if addr == "" { - return true - } - return za.IPNet.Contains(net.ParseIP(addr)) - } - } h.saveConfig(keyConfig, cfg) } } @@ -222,7 +226,6 @@ func (h *dnsContext) validateZonesAndListeningAddresses() error { // address (what you pass into net.Listen) to the list of site configs. // This function does NOT vet the configs to ensure they are compatible. func groupConfigsByListenAddr(configs []*Config) (map[string][]*Config, error) { - groups := make(map[string][]*Config) for _, conf := range configs { for _, h := range conf.ListenHosts { diff --git a/core/dnsserver/reverse.go b/core/dnsserver/reverse.go new file mode 100644 index 000000000..2d0a7c740 --- /dev/null +++ b/core/dnsserver/reverse.go @@ -0,0 +1,38 @@ +package dnsserver + +import ( + "math" + "net" + + "github.com/apparentlymart/go-cidr/cidr" +) + +// classFromCIDR return slice of "classful" (/8, /16, /24 or /32 only) CIDR's from the CIDR in net. +func classFromCIDR(n *net.IPNet) []string { + ones, _ := n.Mask.Size() + if ones%8 == 0 { + return []string{n.String()} + } + + mask := int(math.Ceil(float64(ones)/8)) * 8 + networks := subnets(n, mask) + cidrs := make([]string, len(networks)) + for i := range networks { + cidrs[i] = networks[i].String() + } + return cidrs +} + +// subnets return a slice of prefixes with the desired mask subnetted from original network. +func subnets(network *net.IPNet, newPrefixLen int) []*net.IPNet { + prefixLen, _ := network.Mask.Size() + maxSubnets := int(math.Exp2(float64(newPrefixLen)) / math.Exp2(float64(prefixLen))) + nets := []*net.IPNet{{network.IP, net.CIDRMask(newPrefixLen, 8*len(network.IP))}} + + for i := 1; i < maxSubnets; i++ { + next, _ := cidr.NextSubnet(nets[len(nets)-1], newPrefixLen) + nets = append(nets, next) + } + + return nets +} diff --git a/core/dnsserver/reverse_test.go b/core/dnsserver/reverse_test.go new file mode 100644 index 000000000..bc6909b8a --- /dev/null +++ b/core/dnsserver/reverse_test.go @@ -0,0 +1,31 @@ +package dnsserver + +import ( + "net" + "testing" +) + +func TestClassFromCIDR(t *testing.T) { + tests := []struct { + in string + expected []string + }{ + {"10.0.0.0/15", []string{"10.0.0.0/16", "10.1.0.0/16"}}, + {"10.0.0.0/16", []string{"10.0.0.0/16"}}, + {"192.168.1.1/23", []string{"192.168.0.0/24", "192.168.1.0/24"}}, + {"10.129.60.0/22", []string{"10.129.60.0/24", "10.129.61.0/24", "10.129.62.0/24", "10.129.63.0/24"}}, + } + for i, tc := range tests { + _, n, _ := net.ParseCIDR(tc.in) + nets := classFromCIDR(n) + if len(nets) != len(tc.expected) { + t.Errorf("Test %d, expected %d subnets, got %d", i, len(tc.expected), len(nets)) + continue + } + for j := range nets { + if nets[j] != tc.expected[j] { + t.Errorf("Test %d, expected %s, got %s", i, tc.expected[j], nets[j]) + } + } + } +} diff --git a/core/dnsserver/server.go b/core/dnsserver/server.go index c7304d763..638cbb8fe 100644 --- a/core/dnsserver/server.go +++ b/core/dnsserver/server.go @@ -247,22 +247,11 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) return } if r.Question[0].Qtype != dns.TypeDS { - if h.FilterFunc == nil { - rcode, _ := h.pluginChain.ServeDNS(ctx, w, r) - if !plugin.ClientWrite(rcode) { - errorFunc(s.Addr, w, r, rcode) - } - return - } - // FilterFunc is set, call it to see if we should use this handler. - // This is given to full query name. - if h.FilterFunc(q) { - rcode, _ := h.pluginChain.ServeDNS(ctx, w, r) - if !plugin.ClientWrite(rcode) { - errorFunc(s.Addr, w, r, rcode) - } - return + rcode, _ := h.pluginChain.ServeDNS(ctx, w, r) + if !plugin.ClientWrite(rcode) { + errorFunc(s.Addr, w, r, rcode) } + return } // The type is DS, keep the handler, but keep on searching as maybe we are serving // the parent as well and the DS should be routed to it - this will probably *misroute* DS diff --git a/go.mod b/go.mod index fe06668e3..6f146dd84 100644 --- a/go.mod +++ b/go.mod @@ -8,6 +8,7 @@ require ( github.com/Azure/go-autorest/autorest/azure/auth v0.5.7 github.com/Azure/go-autorest/autorest/to v0.2.0 // indirect github.com/DataDog/datadog-go v3.5.0+incompatible // indirect + github.com/apparentlymart/go-cidr v1.1.0 // indirect github.com/aws/aws-sdk-go v1.38.30 github.com/coredns/caddy v1.1.0 github.com/dnstap/golang-dnstap v0.4.0 diff --git a/go.sum b/go.sum index eba9b9157..1f476c8f0 100644 --- a/go.sum +++ b/go.sum @@ -70,6 +70,8 @@ github.com/alecthomas/units v0.0.0-20190717042225-c3de453c63f4/go.mod h1:ybxpYRF github.com/alecthomas/units v0.0.0-20190924025748-f65c72e2690d/go.mod h1:rBZYJk541a8SKzHPHnH3zbiI+7dagKZ0cgpgrD7Fyho= github.com/apache/thrift v0.12.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ= github.com/apache/thrift v0.13.0/go.mod h1:cp2SuWMxlEZw2r+iP2GNCdIi4C1qmUzdZFSVb+bacwQ= +github.com/apparentlymart/go-cidr v1.1.0 h1:2mAhrMoF+nhXqxTzSZMUzDHkLjmIHC+Zzn4tdgBZjnU= +github.com/apparentlymart/go-cidr v1.1.0/go.mod h1:EBcsNrHc3zQeuaeCeCtQruQm+n9/YjEn/vI25Lg7Gwc= github.com/armon/circbuf v0.0.0-20150827004946-bbbad097214e/go.mod h1:3U/XgcO3hCbHZ8TKRvWD2dDTCfh9M9ya+I9JpbB7O8o= github.com/armon/go-metrics v0.0.0-20180917152333-f0300d1749da/go.mod h1:Q73ZrmVTwzkszR9V5SSuryQ31EELlFMUz1kKyl939pY= github.com/armon/go-radix v0.0.0-20180808171621-7fddfc383310/go.mod h1:ufUuZ+zHj4x4TnLV4JWEpy2hxWSpsRywHrMgIH9cCH8= diff --git a/test/server_test.go b/test/server_test.go index f03a27749..c58915ee7 100644 --- a/test/server_test.go +++ b/test/server_test.go @@ -53,3 +53,57 @@ func chaosTest(t *testing.T, server string) { t.Fatalf("Expected version.bind. reply, got %s", r.Answer[0].String()) } } + +func TestReverseExpansion(t *testing.T) { + // this test needs a fixed port, because with :0 the expanded reverse zone will listen on different + // addresses and we can't check which ones... + corefile := `10.0.0.0/15:5053 { + whoami + }` + + server, udp, _, err := CoreDNSServerAndPorts(corefile) + if err != nil { + t.Fatalf("Could not get CoreDNS serving instance: %s", err) + } + + defer server.Stop() + + m := new(dns.Msg) + m.SetQuestion("whoami.0.10.in-addr.arpa.", dns.TypeA) + + r, err := dns.Exchange(m, udp) + if err != nil { + t.Fatalf("Could not send message: %s", err) + } + if r.Rcode != dns.RcodeSuccess { + t.Errorf("Expected NOERROR, got %d", r.Rcode) + } + if len(r.Extra) != 2 { + t.Errorf("Expected 2 RRs in additional section, got %d", len(r.Extra)) + } + + m.SetQuestion("whoami.1.10.in-addr.arpa.", dns.TypeA) + r, err = dns.Exchange(m, udp) + if err != nil { + t.Fatalf("Could not send message: %s", err) + } + if r.Rcode != dns.RcodeSuccess { + t.Errorf("Expected NOERROR, got %d", r.Rcode) + } + if len(r.Extra) != 2 { + t.Errorf("Expected 2 RRs in additional section, got %d", len(r.Extra)) + } + + // should be refused + m.SetQuestion("whoami.2.10.in-addr.arpa.", dns.TypeA) + r, err = dns.Exchange(m, udp) + if err != nil { + t.Fatalf("Could not send message: %s", err) + } + if r.Rcode != dns.RcodeRefused { + t.Errorf("Expected REFUSED, got %d", r.Rcode) + } + if len(r.Extra) != 0 { + t.Errorf("Expected 0 RRs in additional section, got %d", len(r.Extra)) + } +}