From 5f41d8eb1f74621ada05968dd6b0d24f9ae742df Mon Sep 17 00:00:00 2001 From: Miek Gieben <miek@miek.nl> Date: Mon, 17 May 2021 22:19:54 +0200 Subject: [PATCH] reverse zone: fix Normalize (#4621) Make normalize return multiple "hosts" (= reverse zones) when a non-octet boundary cidr is given. Added pkg/cidr package that holds the cidr calculation routines; felt they didn't really fit dnsutil. This change means the IPNet return parameter isn't needed, the hosts are all correct. The tests that tests this is also removed: TestSplitHostPortReverse The fallout was that zoneAddr _also_ doesn't need the IPNet member, that in turn make it visible that zoneAddr in address.go duplicated a bunch of stuff from register.go; removed/refactored that too. Created a plugin.OriginsFromArgsOrServerBlock to help plugins do the right things, by consuming ZONE arguments; this now expands reverse zones correctly. This is mostly mechanical. Remove the reverse test in plugin/kubernetes which is a copy-paste from a core test (which has since been fixed). Remove MustNormalize as it has no plugin users. This change is not backwards compatible to plugins that have a ZONE argument that they parse in the setup util. All in-tree plugins have been updated. Signed-off-by: Miek Gieben <miek@miek.nl> --- core/dnsserver/address.go | 37 +--------- core/dnsserver/address_test.go | 77 ------------------- core/dnsserver/register.go | 47 +++++++----- core/dnsserver/reverse.go | 38 ---------- core/dnsserver/reverse_test.go | 31 -------- plugin/acl/acl_test.go | 2 +- plugin/acl/setup.go | 11 +-- plugin/auto/setup.go | 10 +-- plugin/autopath/setup.go | 10 +-- plugin/cache/setup.go | 11 +-- plugin/dnssec/setup.go | 13 +--- plugin/etcd/setup.go | 10 +-- plugin/file/setup.go | 14 +--- plugin/forward/setup.go | 5 +- plugin/grpc/setup.go | 5 +- plugin/hosts/setup.go | 11 +-- plugin/k8s_external/setup.go | 10 +-- plugin/kubernetes/setup.go | 14 +--- plugin/kubernetes/setup_reverse_test.go | 36 --------- plugin/loop/README.md | 2 + plugin/loop/setup.go | 6 +- plugin/metadata/setup.go | 13 +--- plugin/metrics/setup.go | 5 +- plugin/normalize.go | 98 ++++++++++++------------- plugin/normalize_test.go | 72 ++++++------------ plugin/pkg/cidr/cidr.go | 74 +++++++++++++++++++ plugin/pkg/cidr/cidr_test.go | 46 ++++++++++++ plugin/pkg/fall/fall.go | 7 +- plugin/secondary/setup.go | 8 +- plugin/sign/setup.go | 13 +--- plugin/template/setup.go | 10 +-- plugin/transfer/setup.go | 23 +----- 32 files changed, 259 insertions(+), 510 deletions(-) delete mode 100644 core/dnsserver/reverse.go delete mode 100644 core/dnsserver/reverse_test.go delete mode 100644 plugin/kubernetes/setup_reverse_test.go create mode 100644 plugin/pkg/cidr/cidr.go create mode 100644 plugin/pkg/cidr/cidr_test.go diff --git a/core/dnsserver/address.go b/core/dnsserver/address.go index 1a69c33b8..b8563c87e 100644 --- a/core/dnsserver/address.go +++ b/core/dnsserver/address.go @@ -4,20 +4,13 @@ import ( "fmt" "net" "strings" - - "github.com/coredns/coredns/plugin" - "github.com/coredns/coredns/plugin/pkg/parse" - "github.com/coredns/coredns/plugin/pkg/transport" - - "github.com/miekg/dns" ) type zoneAddr struct { Zone string Port string - Transport string // dns, tls or grpc - IPNet *net.IPNet // if reverse zone this hold the IPNet - Address string // used for bound zoneAddr - validation of overlapping + Transport string // dns, tls or grpc + Address string // used for bound zoneAddr - validation of overlapping } // String returns the string representation of z. @@ -29,32 +22,6 @@ func (z zoneAddr) String() string { return s } -// normalizeZone parses a zone string into a structured format with separate -// host, and port portions, as well as the original input string. -func normalizeZone(str string) (zoneAddr, error) { - trans, str := parse.Transport(str) - - host, port, ipnet, err := plugin.SplitHostPort(str) - if err != nil { - return zoneAddr{}, err - } - - if port == "" { - switch trans { - case transport.DNS: - port = Port - case transport.TLS: - port = transport.TLSPort - case transport.GRPC: - port = transport.GRPCPort - case transport.HTTPS: - port = transport.HTTPSPort - } - } - - return zoneAddr{Zone: dns.Fqdn(host), Port: port, Transport: trans, IPNet: ipnet}, nil -} - // SplitProtocolHostPort splits a full formed address like "dns://[::1]:53" into parts. func SplitProtocolHostPort(address string) (protocol string, ip string, port string, err error) { parts := strings.Split(address, "://") diff --git a/core/dnsserver/address_test.go b/core/dnsserver/address_test.go index 6d4d0beab..9f92a3fe6 100644 --- a/core/dnsserver/address_test.go +++ b/core/dnsserver/address_test.go @@ -2,83 +2,6 @@ package dnsserver import "testing" -func TestNormalizeZone(t *testing.T) { - for i, test := range []struct { - input string - expected string - shouldErr bool - }{ - {".", "dns://.:53", false}, - {".:54", "dns://.:54", false}, - {"..", "://:", true}, - {".:", "://:", true}, - {"dns://.", "dns://.:53", false}, - {"dns://.:5353", "dns://.:5353", false}, - {"dns://..", "://:", true}, - {"dns://.:", "://:", true}, - {"tls://.", "tls://.:853", false}, - {"tls://.:8953", "tls://.:8953", false}, - {"tls://..", "://:", true}, - {"tls://.:", "://:", true}, - {"grpc://.", "grpc://.:443", false}, - {"grpc://.:8443", "grpc://.:8443", false}, - {"grpc://..", "://:", true}, - {"grpc://.:", "://:", true}, - {"https://.", "https://.:443", false}, - {"https://.:8443", "https://.:8443", false}, - {"https://..", "://:", true}, - {"https://.:", "://:", true}, - } { - addr, err := normalizeZone(test.input) - actual := addr.String() - if test.shouldErr && err == nil { - t.Errorf("Test %d: Expected error, but there wasn't any", i) - } - if !test.shouldErr && err != nil { - t.Errorf("Test %d: Expected no error, but there was one: %v", i, err) - } - if actual != test.expected { - t.Errorf("Test %d: Expected %s but got %s", i, test.expected, actual) - } - } -} - -func TestNormalizeZoneReverse(t *testing.T) { - for i, test := range []struct { - input string - expected string - shouldErr bool - }{ - {"2003::1/64", "dns://0.0.0.0.0.0.0.0.0.0.0.0.3.0.0.2.ip6.arpa.:53", false}, - {"2003::1/64.", "dns://2003::1/64.:53", false}, // OK, with closing dot the parse will fail. - {"2003::1/64:53", "dns://0.0.0.0.0.0.0.0.0.0.0.0.3.0.0.2.ip6.arpa.:53", false}, - {"2003::1/64.:53", "dns://2003::1/64.:53", false}, - - {"10.0.0.0/24", "dns://0.0.10.in-addr.arpa.:53", false}, - {"10.0.0.0/24.", "dns://10.0.0.0/24.:53", false}, - {"10.0.0.0/24:53", "dns://0.0.10.in-addr.arpa.:53", false}, - {"10.0.0.0/24.:53", "dns://10.0.0.0/24.:53", false}, - - // non %8==0 netmasks - {"2003::53/67", "dns://0.0.0.0.0.0.0.0.0.0.0.0.0.3.0.0.2.ip6.arpa.:53", false}, - {"10.0.0.0/25.", "dns://10.0.0.0/25.:53", false}, // has dot - {"10.0.0.0/25", "dns://0.0.10.in-addr.arpa.:53", false}, - {"fd00:77:30::0/110", "dns://0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.3.0.0.7.7.0.0.0.0.d.f.ip6.arpa.:53", false}, - } { - addr, err := normalizeZone(test.input) - actual := addr.String() - if test.shouldErr && err == nil { - t.Errorf("Test %d: Expected error, but there wasn't any", i) - } - if !test.shouldErr && err != nil { - t.Errorf("Test %d: Expected no error, but there was one: %v", i, err) - } - if actual != test.expected { - t.Errorf("Test %d: Expected %s but got %s", i, test.expected, actual) - } - } -} - func TestSplitProtocolHostPort(t *testing.T) { for i, test := range []struct { input string diff --git a/core/dnsserver/register.go b/core/dnsserver/register.go index 505aa8580..1eb457b8e 100644 --- a/core/dnsserver/register.go +++ b/core/dnsserver/register.go @@ -11,6 +11,8 @@ import ( "github.com/coredns/coredns/plugin" "github.com/coredns/coredns/plugin/pkg/parse" "github.com/coredns/coredns/plugin/pkg/transport" + + "github.com/miekg/dns" ) const serverType = "dns" @@ -60,31 +62,42 @@ func (h *dnsContext) InspectServerBlocks(sourceFile string, serverBlocks []caddy 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. + zoneAddrs := []zoneAddr{} 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 + trans, k1 := parse.Transport(k) // get rid of any dns:// or other scheme. + hosts, port, err := plugin.SplitHostPort(k1) + if err != nil { + return nil, err } - 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) + + if port == "" { + switch trans { + case transport.DNS: + port = Port + case transport.TLS: + port = transport.TLSPort + case transport.GRPC: + port = transport.GRPCPort + case transport.HTTPS: + port = transport.HTTPSPort } } + + if len(hosts) > 1 { + s.Keys[ik] = hosts[0] + ":" + port // replace for the first + for _, h := range hosts[1:] { // add the rest + s.Keys = append(s.Keys, h+":"+port) + } + } + for i := range hosts { + zoneAddrs = append(zoneAddrs, zoneAddr{Zone: dns.Fqdn(hosts[i]), Port: port, Transport: trans}) + } } 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 { - return nil, err - } + for ik := range s.Keys { + za := zoneAddrs[ik] s.Keys[ik] = za.String() // Save the config to our master list, and key it for lookups. cfg := &Config{ diff --git a/core/dnsserver/reverse.go b/core/dnsserver/reverse.go deleted file mode 100644 index 2d0a7c740..000000000 --- a/core/dnsserver/reverse.go +++ /dev/null @@ -1,38 +0,0 @@ -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 deleted file mode 100644 index bc6909b8a..000000000 --- a/core/dnsserver/reverse_test.go +++ /dev/null @@ -1,31 +0,0 @@ -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/plugin/acl/acl_test.go b/plugin/acl/acl_test.go index 4c6df95e5..0ab6c1d77 100644 --- a/plugin/acl/acl_test.go +++ b/plugin/acl/acl_test.go @@ -233,7 +233,7 @@ func TestACLServeDNS(t *testing.T) { }, { "Fine-Grained 2 REFUSED", - `acl { + `acl example.org { block net 192.168.1.0/24 }`, []string{"example.org"}, diff --git a/plugin/acl/setup.go b/plugin/acl/setup.go index 1a688a485..3adde0aec 100644 --- a/plugin/acl/setup.go +++ b/plugin/acl/setup.go @@ -43,15 +43,8 @@ func parse(c *caddy.Controller) (ACL, error) { a := ACL{} for c.Next() { r := rule{} - r.zones = c.RemainingArgs() - if len(r.zones) == 0 { - // if empty, the zones from the configuration block are used. - r.zones = make([]string, len(c.ServerBlockKeys)) - copy(r.zones, c.ServerBlockKeys) - } - for i := range r.zones { - r.zones[i] = plugin.Host(r.zones[i]).Normalize() - } + args := c.RemainingArgs() + r.zones = plugin.OriginsFromArgsOrServerBlock(args, c.ServerBlockKeys) for c.NextBlock() { p := policy{} diff --git a/plugin/auto/setup.go b/plugin/auto/setup.go index bebf3a65b..1162274a0 100644 --- a/plugin/auto/setup.go +++ b/plugin/auto/setup.go @@ -87,16 +87,8 @@ func autoParse(c *caddy.Controller) (Auto, error) { for c.Next() { // auto [ZONES...] - a.Zones.origins = make([]string, len(c.ServerBlockKeys)) - copy(a.Zones.origins, c.ServerBlockKeys) - args := c.RemainingArgs() - if len(args) > 0 { - a.Zones.origins = args - } - for i := range a.Zones.origins { - a.Zones.origins[i] = plugin.Host(a.Zones.origins[i]).Normalize() - } + a.Zones.origins = plugin.OriginsFromArgsOrServerBlock(args, c.ServerBlockKeys) a.loader.upstream = upstream.New() for c.NextBlock() { diff --git a/plugin/autopath/setup.go b/plugin/autopath/setup.go index 39beef770..ed536933a 100644 --- a/plugin/autopath/setup.go +++ b/plugin/autopath/setup.go @@ -62,14 +62,8 @@ func autoPathParse(c *caddy.Controller) (*AutoPath, string, error) { plugin.Zones(ap.search).Normalize() ap.search = append(ap.search, "") // sentinel value as demanded. } - ap.Zones = zoneAndresolv[:len(zoneAndresolv)-1] - if len(ap.Zones) == 0 { - ap.Zones = make([]string, len(c.ServerBlockKeys)) - copy(ap.Zones, c.ServerBlockKeys) - } - for i, str := range ap.Zones { - ap.Zones[i] = plugin.Host(str).Normalize() - } + zones := zoneAndresolv[:len(zoneAndresolv)-1] + ap.Zones = plugin.OriginsFromArgsOrServerBlock(zones, c.ServerBlockKeys) } return ap, mw, nil } diff --git a/plugin/cache/setup.go b/plugin/cache/setup.go index d936db895..bae4c3d95 100644 --- a/plugin/cache/setup.go +++ b/plugin/cache/setup.go @@ -41,10 +41,7 @@ func cacheParse(c *caddy.Controller) (*Cache, error) { j++ // cache [ttl] [zones..] - origins := make([]string, len(c.ServerBlockKeys)) - copy(origins, c.ServerBlockKeys) args := c.RemainingArgs() - if len(args) > 0 { // first args may be just a number, then it is the ttl, if not it is a zone ttl, err := strconv.Atoi(args[0]) @@ -57,10 +54,8 @@ func cacheParse(c *caddy.Controller) (*Cache, error) { ca.nttl = time.Duration(ttl) * time.Second args = args[1:] } - if len(args) > 0 { - copy(origins, args) - } } + origins := plugin.OriginsFromArgsOrServerBlock(args, c.ServerBlockKeys) // Refinements? In an extra block. for c.NextBlock() { @@ -189,11 +184,7 @@ func cacheParse(c *caddy.Controller) (*Cache, error) { } } - for i := range origins { - origins[i] = plugin.Host(origins[i]).Normalize() - } ca.Zones = origins - ca.pcache = cache.New(ca.pcap) ca.ncache = cache.New(ca.ncap) } diff --git a/plugin/dnssec/setup.go b/plugin/dnssec/setup.go index 2bf321857..b82e67648 100644 --- a/plugin/dnssec/setup.go +++ b/plugin/dnssec/setup.go @@ -44,9 +44,7 @@ func setup(c *caddy.Controller) error { func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, bool, error) { zones := []string{} - keys := []*DNSKEY{} - capacity := defaultCap i := 0 @@ -57,12 +55,7 @@ func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, bool, error) { i++ // dnssec [zones...] - zones = make([]string, len(c.ServerBlockKeys)) - copy(zones, c.ServerBlockKeys) - args := c.RemainingArgs() - if len(args) > 0 { - zones = args - } + zones = plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) for c.NextBlock() { @@ -89,10 +82,6 @@ func dnssecParse(c *caddy.Controller) ([]string, []*DNSKEY, int, bool, error) { } } - for i := range zones { - zones[i] = plugin.Host(zones[i]).Normalize() - } - // Check if we have both KSKs and ZSKs. zsk, ksk := 0, 0 for _, k := range keys { diff --git a/plugin/etcd/setup.go b/plugin/etcd/setup.go index 6a214d3fa..751d741e1 100644 --- a/plugin/etcd/setup.go +++ b/plugin/etcd/setup.go @@ -41,15 +41,7 @@ func etcdParse(c *caddy.Controller) (*Etcd, error) { etc.Upstream = upstream.New() for c.Next() { - etc.Zones = c.RemainingArgs() - if len(etc.Zones) == 0 { - etc.Zones = make([]string, len(c.ServerBlockKeys)) - copy(etc.Zones, c.ServerBlockKeys) - } - for i, str := range etc.Zones { - etc.Zones[i] = plugin.Host(str).Normalize() - } - + etc.Zones = plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) for c.NextBlock() { switch c.Val() { case "stubzones": diff --git a/plugin/file/setup.go b/plugin/file/setup.go index c5ff4c203..0444836b5 100644 --- a/plugin/file/setup.go +++ b/plugin/file/setup.go @@ -82,13 +82,7 @@ func fileParse(c *caddy.Controller) (Zones, error) { } fileName := c.Val() - origins := make([]string, len(c.ServerBlockKeys)) - copy(origins, c.ServerBlockKeys) - args := c.RemainingArgs() - if len(args) > 0 { - origins = args - } - + origins := plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) if !filepath.IsAbs(fileName) && config.Root != "" { fileName = filepath.Join(config.Root, fileName) } @@ -99,16 +93,14 @@ func fileParse(c *caddy.Controller) (Zones, error) { } for i := range origins { - origins[i] = plugin.Host(origins[i]).Normalize() z[origins[i]] = NewZone(origins[i], fileName) if openErr == nil { reader.Seek(0, 0) zone, err := Parse(reader, origins[i], fileName, 0) - if err == nil { - z[origins[i]] = zone - } else { + if err != nil { return Zones{}, err } + z[origins[i]] = zone } names = append(names, origins[i]) } diff --git a/plugin/forward/setup.go b/plugin/forward/setup.go index 7504e9409..b183044a8 100644 --- a/plugin/forward/setup.go +++ b/plugin/forward/setup.go @@ -92,7 +92,7 @@ func parseStanza(c *caddy.Controller) (*Forward, error) { if !c.Args(&f.from) { return f, c.ArgErr() } - f.from = plugin.Host(f.from).Normalize() + f.from = plugin.Host(f.from).Normalize()[0] // there can only be one here, won't work with non-octet reverse to := c.RemainingArgs() if len(to) == 0 { @@ -151,9 +151,8 @@ func parseBlock(c *caddy.Controller, f *Forward) error { return c.ArgErr() } for i := 0; i < len(ignore); i++ { - ignore[i] = plugin.Host(ignore[i]).Normalize() + f.ignored = append(f.ignored, plugin.Host(ignore[i]).Normalize()...) } - f.ignored = ignore case "max_fails": if !c.NextArg() { return c.ArgErr() diff --git a/plugin/grpc/setup.go b/plugin/grpc/setup.go index 6c29292d3..e96527802 100644 --- a/plugin/grpc/setup.go +++ b/plugin/grpc/setup.go @@ -56,7 +56,7 @@ func parseStanza(c *caddy.Controller) (*GRPC, error) { if !c.Args(&g.from) { return g, c.ArgErr() } - g.from = plugin.Host(g.from).Normalize() + g.from = plugin.Host(g.from).Normalize()[0] // only the first is used. to := c.RemainingArgs() if len(to) == 0 { @@ -100,9 +100,8 @@ func parseBlock(c *caddy.Controller, g *GRPC) error { return c.ArgErr() } for i := 0; i < len(ignore); i++ { - ignore[i] = plugin.Host(ignore[i]).Normalize() + g.ignored = append(g.ignored, plugin.Host(ignore[i]).Normalize()...) } - g.ignored = ignore case "tls": args := c.RemainingArgs() if len(args) > 3 { diff --git a/plugin/hosts/setup.go b/plugin/hosts/setup.go index f79b2446c..c256dc19a 100644 --- a/plugin/hosts/setup.go +++ b/plugin/hosts/setup.go @@ -106,16 +106,7 @@ func hostsParse(c *caddy.Controller) (Hosts, error) { } } - origins := make([]string, len(c.ServerBlockKeys)) - copy(origins, c.ServerBlockKeys) - if len(args) > 0 { - origins = args - } - - for i := range origins { - origins[i] = plugin.Host(origins[i]).Normalize() - } - h.Origins = origins + h.Origins = plugin.OriginsFromArgsOrServerBlock(args, c.ServerBlockKeys) for c.NextBlock() { switch c.Val() { diff --git a/plugin/k8s_external/setup.go b/plugin/k8s_external/setup.go index 8783507bd..1f408dc80 100644 --- a/plugin/k8s_external/setup.go +++ b/plugin/k8s_external/setup.go @@ -44,15 +44,7 @@ func parse(c *caddy.Controller) (*External, error) { e := New() for c.Next() { // external - zones := c.RemainingArgs() - e.Zones = zones - if len(zones) == 0 { - e.Zones = make([]string, len(c.ServerBlockKeys)) - copy(e.Zones, c.ServerBlockKeys) - } - for i, str := range e.Zones { - e.Zones[i] = plugin.Host(str).Normalize() - } + e.Zones = plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) for c.NextBlock() { switch c.Val() { case "ttl": diff --git a/plugin/kubernetes/setup.go b/plugin/kubernetes/setup.go index 8b9bd2c42..c85170d9b 100644 --- a/plugin/kubernetes/setup.go +++ b/plugin/kubernetes/setup.go @@ -96,19 +96,7 @@ func ParseStanza(c *caddy.Controller) (*Kubernetes, error) { } k8s.opts = opts - zones := c.RemainingArgs() - - if len(zones) != 0 { - k8s.Zones = zones - for i := 0; i < len(k8s.Zones); i++ { - k8s.Zones[i] = plugin.Host(k8s.Zones[i]).Normalize() - } - } else { - k8s.Zones = make([]string, len(c.ServerBlockKeys)) - for i := 0; i < len(c.ServerBlockKeys); i++ { - k8s.Zones[i] = plugin.Host(c.ServerBlockKeys[i]).Normalize() - } - } + k8s.Zones = plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) k8s.primaryZoneIndex = -1 for i, z := range k8s.Zones { diff --git a/plugin/kubernetes/setup_reverse_test.go b/plugin/kubernetes/setup_reverse_test.go deleted file mode 100644 index e9b878319..000000000 --- a/plugin/kubernetes/setup_reverse_test.go +++ /dev/null @@ -1,36 +0,0 @@ -package kubernetes - -import ( - "testing" - - "github.com/coredns/caddy" -) - -func TestKubernetesParseReverseZone(t *testing.T) { - tests := []struct { - input string // Corefile data as string - expectedZones []string // expected count of defined zones. - }{ - {`kubernetes coredns.local 10.0.0.0/16`, []string{"coredns.local.", "0.10.in-addr.arpa."}}, - {`kubernetes coredns.local 10.0.0.0/17`, []string{"coredns.local.", "0.10.in-addr.arpa."}}, - {`kubernetes coredns.local fd00:77:30::0/110`, []string{"coredns.local.", "0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.3.0.0.7.7.0.0.0.0.d.f.ip6.arpa."}}, - } - - for i, tc := range tests { - c := caddy.NewTestController("dns", tc.input) - k, err := kubernetesParse(c) - if err != nil { - t.Fatalf("Test %d: Expected no error, got %q", i, err) - } - - zl := len(k.Zones) - if zl != len(tc.expectedZones) { - t.Errorf("Test %d: Expected kubernetes to be initialized with %d zones, found %d zones", i, len(tc.expectedZones), zl) - } - for i, z := range tc.expectedZones { - if k.Zones[i] != z { - t.Errorf("Test %d: Expected zones to be %q, got %q", i, z, k.Zones[i]) - } - } - } -} diff --git a/plugin/loop/README.md b/plugin/loop/README.md index b0fb4835d..826f5c577 100644 --- a/plugin/loop/README.md +++ b/plugin/loop/README.md @@ -13,6 +13,8 @@ The plugin will try to send the query for up to 30 seconds. This is done to give to start up. Once a query has been successfully sent, *loop* disables itself to prevent a query of death. +Note that *loop* will _only_ send "looping queries" for the first zone given in the Server Block. + The query sent is `<random number>.<random number>.zone` with type set to HINFO. ## Syntax diff --git a/plugin/loop/setup.go b/plugin/loop/setup.go index 68af97e5e..13551c2b6 100644 --- a/plugin/loop/setup.go +++ b/plugin/loop/setup.go @@ -59,7 +59,7 @@ func setup(c *caddy.Controller) error { func parse(c *caddy.Controller) (*Loop, error) { i := 0 - zone := "." + zones := []string{"."} for c.Next() { if i > 0 { return nil, plugin.ErrOnce @@ -70,10 +70,10 @@ func parse(c *caddy.Controller) (*Loop, error) { } if len(c.ServerBlockKeys) > 0 { - zone = plugin.Host(c.ServerBlockKeys[0]).Normalize() + zones = plugin.Host(c.ServerBlockKeys[0]).Normalize() } } - return New(zone), nil + return New(zones[0]), nil } // qname returns a random name. <rand.Int()>.<rand.Int().<zone>. diff --git a/plugin/metadata/setup.go b/plugin/metadata/setup.go index 7a0985a2d..90b1cf997 100644 --- a/plugin/metadata/setup.go +++ b/plugin/metadata/setup.go @@ -34,19 +34,8 @@ func setup(c *caddy.Controller) error { func metadataParse(c *caddy.Controller) (*Metadata, error) { m := &Metadata{} c.Next() - zones := c.RemainingArgs() - if len(zones) != 0 { - m.Zones = zones - for i := 0; i < len(m.Zones); i++ { - m.Zones[i] = plugin.Host(m.Zones[i]).Normalize() - } - } else { - m.Zones = make([]string, len(c.ServerBlockKeys)) - for i := 0; i < len(c.ServerBlockKeys); i++ { - m.Zones[i] = plugin.Host(c.ServerBlockKeys[i]).Normalize() - } - } + m.Zones = plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) if c.NextBlock() || c.Next() { return nil, plugin.Error("metadata", c.ArgErr()) diff --git a/plugin/metrics/setup.go b/plugin/metrics/setup.go index 3858fb519..5ac7d8f7f 100644 --- a/plugin/metrics/setup.go +++ b/plugin/metrics/setup.go @@ -80,8 +80,9 @@ func parse(c *caddy.Controller) (*Metrics, error) { } i++ - for _, z := range c.ServerBlockKeys { - met.AddZone(plugin.Host(z).Normalize()) + zones := plugin.OriginsFromArgsOrServerBlock(nil /* args */, c.ServerBlockKeys) + for _, z := range zones { + met.AddZone(z) } args := c.RemainingArgs() diff --git a/plugin/normalize.go b/plugin/normalize.go index 56b086fcd..96ec59c76 100644 --- a/plugin/normalize.go +++ b/plugin/normalize.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" + "github.com/coredns/coredns/plugin/pkg/cidr" "github.com/coredns/coredns/plugin/pkg/parse" "github.com/miekg/dns" @@ -62,82 +63,77 @@ type ( // Normalize will return the host portion of host, stripping // of any port or transport. The host will also be fully qualified and lowercased. -// An empty string is returned on failure -func (h Host) Normalize() string { +// An empty slice is returned on failure +func (h Host) Normalize() []string { // The error can be ignored here, because this function should only be called after the corefile has already been vetted. - host, _ := h.MustNormalize() - return host -} - -// MustNormalize will return the host portion of host, stripping -// of any port or transport. The host will also be fully qualified and lowercased. -// An error is returned on error -func (h Host) MustNormalize() (string, error) { s := string(h) _, s = parse.Transport(s) - // The error can be ignored here, because this function is called after the corefile has already been vetted. - host, _, _, err := SplitHostPort(s) + hosts, _, err := SplitHostPort(s) if err != nil { - return "", err + return nil } - return Name(host).Normalize(), nil + for i := range hosts { + hosts[i] = Name(hosts[i]).Normalize() + + } + return hosts } -// SplitHostPort splits s up in a host and port portion, taking reverse address notation into account. -// String the string s should *not* be prefixed with any protocols, i.e. dns://. The returned ipnet is the -// *net.IPNet that is used when the zone is a reverse and a netmask is given. -func SplitHostPort(s string) (host, port string, ipnet *net.IPNet, err error) { +// SplitHostPort splits s up in a host(s) and port portion, taking reverse address notation into account. +// String the string s should *not* be prefixed with any protocols, i.e. dns://. SplitHostPort can return +// multiple hosts when a reverse notation on a non-octet boundary is given. +func SplitHostPort(s string) (hosts []string, port string, err error) { // If there is: :[0-9]+ on the end we assume this is the port. This works for (ascii) domain // names and our reverse syntax, which always needs a /mask *before* the port. // So from the back, find first colon, and then check if it's a number. - host = s - colon := strings.LastIndex(s, ":") if colon == len(s)-1 { - return "", "", nil, fmt.Errorf("expecting data after last colon: %q", s) + return nil, "", fmt.Errorf("expecting data after last colon: %q", s) } if colon != -1 { if p, err := strconv.Atoi(s[colon+1:]); err == nil { port = strconv.Itoa(p) - host = s[:colon] + s = s[:colon] } } // TODO(miek): this should take escaping into account. - if len(host) > 255 { - return "", "", nil, fmt.Errorf("specified zone is too long: %d > 255", len(host)) + if len(s) > 255 { + return nil, "", fmt.Errorf("specified zone is too long: %d > 255", len(s)) } - _, d := dns.IsDomainName(host) - if !d { - return "", "", nil, fmt.Errorf("zone is not a valid domain name: %s", host) + if _, ok := dns.IsDomainName(s); !ok { + return nil, "", fmt.Errorf("zone is not a valid domain name: %s", s) } // Check if it parses as a reverse zone, if so we use that. Must be fully specified IP and mask. - ip, n, err := net.ParseCIDR(host) - ones, bits := 0, 0 - if err == nil { - if rev, e := dns.ReverseAddr(ip.String()); e == nil { - ones, bits = n.Mask.Size() - // get the size, in bits, of each portion of hostname defined in the reverse address. (8 for IPv4, 4 for IPv6) - sizeDigit := 8 - if len(n.IP) == net.IPv6len { - sizeDigit = 4 - } - // Get the first lower octet boundary to see what encompassing zone we should be authoritative for. - mod := (bits - ones) % sizeDigit - nearest := (bits - ones) + mod - offset := 0 - var end bool - for i := 0; i < nearest/sizeDigit; i++ { - offset, end = dns.NextLabel(rev, offset) - if end { - break - } - } - host = rev[offset:] - } + _, n, err := net.ParseCIDR(s) + if err != nil { + return []string{s}, port, nil } - return host, port, n, nil + + // now check if multiple hosts must be returned. + nets := cidr.Class(n) + hosts = cidr.Reverse(nets) + return hosts, port, nil +} + +// OriginsFromArgsOrServerBlock returns the normalized args if that slice +// is not empty, otherwise the serverblock slice is returned (in a newly copied slice). +func OriginsFromArgsOrServerBlock(args, serverblock []string) []string { + if len(args) == 0 { + s := make([]string, len(serverblock)) + copy(s, serverblock) + for i := range s { + s[i] = Host(s[i]).Normalize()[0] // expansion of these already happened in dnsserver/registrer.go + } + return s + } + s := []string{} + for i := range args { + s = append(s, Host(args[i]).Normalize()...) + } + + return s } diff --git a/plugin/normalize_test.go b/plugin/normalize_test.go index 2a82271ba..ac761f7ea 100644 --- a/plugin/normalize_test.go +++ b/plugin/normalize_test.go @@ -1,6 +1,9 @@ package plugin -import "testing" +import ( + "sort" + "testing" +) func TestZoneMatches(t *testing.T) { child := "example.org." @@ -69,55 +72,26 @@ func TestNameNormalize(t *testing.T) { } func TestHostNormalize(t *testing.T) { - hosts := []string{".:53", ".", "example.org:53", "example.org.", "example.org.:53", "example.org.", - "10.0.0.0/8:53", "10.in-addr.arpa.", "10.0.0.0/9", "10.in-addr.arpa.", - "dns://example.org", "example.org."} + tests := []struct { + in string + out []string + }{ + {".:53", []string{"."}}, + {"example.org:53", []string{"example.org."}}, + {"example.org.:53", []string{"example.org."}}, + {"10.0.0.0/8:53", []string{"10.in-addr.arpa."}}, + {"10.0.0.0/15", []string{"0.10.in-addr.arpa.", "1.10.in-addr.arpa."}}, + {"dns://example.org", []string{"example.org."}}, + } - for i := 0; i < len(hosts); i += 2 { - ts := hosts[i] - expected := hosts[i+1] - actual := Host(ts).Normalize() - if expected != actual { - t.Errorf("Expected %v, got %v", expected, actual) - } - } -} - -func TestHostMustNormalizeFail(t *testing.T) { - hosts := []string{"..:53", "::", ""} - for i := 0; i < len(hosts); i++ { - ts := hosts[i] - h, err := Host(ts).MustNormalize() - if err == nil { - t.Errorf("Expected error, got %v", h) - } - } -} - -func TestSplitHostPortReverse(t *testing.T) { - tests := map[string]int{ - "example.org.": 0, - "10.0.0.0/9": 32 - 9, - "10.0.0.0/8": 32 - 8, - "10.0.0.0/17": 32 - 17, - "10.0.0.0/0": 32 - 0, - "10.0.0.0/64": 0, - "10.0.0.0": 0, - "10.0.0": 0, - "2003::1/65": 128 - 65, - } - for in, expect := range tests { - _, _, n, err := SplitHostPort(in) - if err != nil { - t.Errorf("Expected no error, got %q for %s", in, err) - } - if n == nil { - continue - } - ones, bits := n.Mask.Size() - got := bits - ones - if got != expect { - t.Errorf("Expected %d, got %d for %s", expect, got, in) + for i := range tests { + actual := Host(tests[i].in).Normalize() + expected := tests[i].out + sort.Strings(expected) + for j := range expected { + if expected[j] != actual[j] { + t.Errorf("Test %d, expected %v, got %v", i, expected, actual) + } } } } diff --git a/plugin/pkg/cidr/cidr.go b/plugin/pkg/cidr/cidr.go new file mode 100644 index 000000000..c1b02d25e --- /dev/null +++ b/plugin/pkg/cidr/cidr.go @@ -0,0 +1,74 @@ +// Package cidr contains functions that deal with classless reverse zones in the DNS. +package cidr + +import ( + "math" + "net" + + "github.com/apparentlymart/go-cidr/cidr" + "github.com/miekg/dns" +) + +// Class return slice of "classful" (/8, /16, /24 or /32 only) CIDR's from the CIDR in net. +func Class(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 := nets(n, mask) + cidrs := make([]string, len(networks)) + for i := range networks { + cidrs[i] = networks[i].String() + } + return cidrs +} + +// nets return a slice of prefixes with the desired mask subnetted from original network. +func nets(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, exceeds := cidr.NextSubnet(nets[len(nets)-1], newPrefixLen) + nets = append(nets, next) + if exceeds { + break + } + } + + return nets +} + +// Reverse return the reverse zones that are authoritative for each net in ns. +func Reverse(nets []string) []string { + rev := make([]string, len(nets)) + for i := range nets { + ip, n, _ := net.ParseCIDR(nets[i]) + r, err1 := dns.ReverseAddr(ip.String()) + if err1 != nil { + continue + } + ones, bits := n.Mask.Size() + // get the size, in bits, of each portion of hostname defined in the reverse address. (8 for IPv4, 4 for IPv6) + sizeDigit := 8 + if len(n.IP) == net.IPv6len { + sizeDigit = 4 + } + // Get the first lower octet boundary to see what encompassing zone we should be authoritative for. + mod := (bits - ones) % sizeDigit + nearest := (bits - ones) + mod + offset := 0 + var end bool + for i := 0; i < nearest/sizeDigit; i++ { + offset, end = dns.NextLabel(r, offset) + if end { + break + } + } + rev[i] = r[offset:] + } + return rev +} diff --git a/plugin/pkg/cidr/cidr_test.go b/plugin/pkg/cidr/cidr_test.go new file mode 100644 index 000000000..86ea28bee --- /dev/null +++ b/plugin/pkg/cidr/cidr_test.go @@ -0,0 +1,46 @@ +package cidr + +import ( + "net" + "testing" +) + +var tests = []struct { + in string + expected []string + zones []string +}{ + {"10.0.0.0/15", []string{"10.0.0.0/16", "10.1.0.0/16"}, []string{"0.10.in-addr.arpa.", "1.10.in-addr.arpa."}}, + {"10.0.0.0/16", []string{"10.0.0.0/16"}, []string{"0.10.in-addr.arpa."}}, + {"192.168.1.1/23", []string{"192.168.0.0/24", "192.168.1.0/24"}, []string{"0.168.192.in-addr.arpa.", "1.168.192.in-addr.arpa."}}, + {"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"}, []string{"60.129.10.in-addr.arpa.", "61.129.10.in-addr.arpa.", "62.129.10.in-addr.arpa.", "63.129.10.in-addr.arpa."}}, +} + +func TestClass(t *testing.T) { + for i, tc := range tests { + _, n, _ := net.ParseCIDR(tc.in) + nets := Class(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]) + } + } + } +} + +func TestReverse(t *testing.T) { + for i, tc := range tests { + _, n, _ := net.ParseCIDR(tc.in) + nets := Class(n) + reverse := Reverse(nets) + for j := range reverse { + if reverse[j] != tc.zones[j] { + t.Errorf("Test %d, expected %s, got %s", i, tc.zones[j], reverse[j]) + } + } + } +} diff --git a/plugin/pkg/fall/fall.go b/plugin/pkg/fall/fall.go index 6d0463fd2..dd42f8e04 100644 --- a/plugin/pkg/fall/fall.go +++ b/plugin/pkg/fall/fall.go @@ -31,10 +31,11 @@ func (f F) Through(qname string) bool { // setZones will set zones in f. func (f *F) setZones(zones []string) { + z := []string{} for i := range zones { - zones[i] = plugin.Host(zones[i]).Normalize() + z = append(z, plugin.Host(zones[i]).Normalize()...) } - f.Zones = zones + f.Zones = z } // SetZonesFromArgs sets zones in f to the passed value or to "." if the slice is empty. @@ -47,7 +48,7 @@ func (f *F) SetZonesFromArgs(zones []string) { } // Equal returns true if f and g are equal. -func (f F) Equal(g F) bool { +func (f *F) Equal(g F) bool { if len(f.Zones) != len(g.Zones) { return false } diff --git a/plugin/secondary/setup.go b/plugin/secondary/setup.go index 83d8e56ef..c09147276 100644 --- a/plugin/secondary/setup.go +++ b/plugin/secondary/setup.go @@ -47,14 +47,8 @@ func secondaryParse(c *caddy.Controller) (file.Zones, error) { if c.Val() == "secondary" { // secondary [origin] - origins := make([]string, len(c.ServerBlockKeys)) - copy(origins, c.ServerBlockKeys) - args := c.RemainingArgs() - if len(args) > 0 { - origins = args - } + origins := plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) for i := range origins { - origins[i] = plugin.Host(origins[i]).Normalize() z[origins[i]] = file.NewZone(origins[i], "stdin") names = append(names, origins[i]) } diff --git a/plugin/sign/setup.go b/plugin/sign/setup.go index 50c297051..e5f5295a0 100644 --- a/plugin/sign/setup.go +++ b/plugin/sign/setup.go @@ -50,21 +50,12 @@ func parse(c *caddy.Controller) (*Sign, error) { dbfile = filepath.Join(config.Root, dbfile) } - origins := make([]string, len(c.ServerBlockKeys)) - copy(origins, c.ServerBlockKeys) - args := c.RemainingArgs() - if len(args) > 0 { - origins = args - } - for i := range origins { - origins[i] = plugin.Host(origins[i]).Normalize() - } - + origins := plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) signers := make([]*Signer, len(origins)) for i := range origins { signers[i] = &Signer{ dbfile: dbfile, - origin: plugin.Host(origins[i]).Normalize(), + origin: origins[i], jitterIncep: time.Duration(float32(durationInceptionJitter) * rand.Float32()), jitterExpir: time.Duration(float32(durationExpirationDayJitter) * rand.Float32()), directory: "/var/lib/coredns", diff --git a/plugin/template/setup.go b/plugin/template/setup.go index 978f1ffc2..03a5322f8 100644 --- a/plugin/template/setup.go +++ b/plugin/template/setup.go @@ -49,16 +49,8 @@ func templateParse(c *caddy.Controller) (handler Handler, err error) { return handler, c.Errf("invalid RR class %s", c.Val()) } - zones := c.RemainingArgs() - if len(zones) == 0 { - zones = make([]string, len(c.ServerBlockKeys)) - copy(zones, c.ServerBlockKeys) - } - for i, str := range zones { - zones[i] = plugin.Host(str).Normalize() - } + zones := plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) handler.Zones = append(handler.Zones, zones...) - t := template{qclass: class, qtype: qtype, zones: zones} t.regex = make([]*regexp.Regexp, 0) diff --git a/plugin/transfer/setup.go b/plugin/transfer/setup.go index e92d442bc..604a26959 100644 --- a/plugin/transfer/setup.go +++ b/plugin/transfer/setup.go @@ -47,28 +47,7 @@ func parseTransfer(c *caddy.Controller) (*Transfer, error) { t := &Transfer{} for c.Next() { x := &xfr{} - zones := c.RemainingArgs() - - if len(zones) != 0 { - x.Zones = zones - for i := 0; i < len(x.Zones); i++ { - nzone, err := plugin.Host(x.Zones[i]).MustNormalize() - if err != nil { - return nil, err - } - x.Zones[i] = nzone - } - } else { - x.Zones = make([]string, len(c.ServerBlockKeys)) - for i := 0; i < len(c.ServerBlockKeys); i++ { - nzone, err := plugin.Host(c.ServerBlockKeys[i]).MustNormalize() - if err != nil { - return nil, err - } - x.Zones[i] = nzone - } - } - + x.Zones = plugin.OriginsFromArgsOrServerBlock(c.RemainingArgs(), c.ServerBlockKeys) for c.NextBlock() { switch c.Val() { case "to":