diff --git a/core/dnsserver/address.go b/core/dnsserver/address.go index 75f56ded9..39d656eff 100644 --- a/core/dnsserver/address.go +++ b/core/dnsserver/address.go @@ -15,10 +15,17 @@ type zoneAddr struct { 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 } // String return the string representation of z. -func (z zoneAddr) String() string { return z.Transport + "://" + z.Zone + ":" + z.Port } +func (z zoneAddr) String() string { + s := z.Transport + "://" + z.Zone + ":" + z.Port + if z.Address != "" { + s += " on " + z.Address + } + return s +} // Transport returns the protocol of the string s func Transport(s string) string { @@ -94,3 +101,37 @@ const ( TransportTLS = "tls" TransportGRPC = "grpc" ) + +type zoneOverlap struct { + registeredAddr map[zoneAddr]zoneAddr // each zoneAddr is registered once by its key + unboundOverlap map[zoneAddr]zoneAddr // the "no bind" equiv ZoneAdddr is registered by its original key +} + +func newOverlapZone() *zoneOverlap { + return &zoneOverlap{registeredAddr: make(map[zoneAddr]zoneAddr), unboundOverlap: make(map[zoneAddr]zoneAddr)} +} + +// registerAndCheck adds a new zoneAddr for validation, it returns information about existing or overlapping with already registered +// we consider that an unbound address is overlapping all bound addresses for same zone, same port +func (zo *zoneOverlap) registerAndCheck(z zoneAddr) (existingZone *zoneAddr, overlappingZone *zoneAddr) { + + if exist, ok := zo.registeredAddr[z]; ok { + // exact same zone already registered + return &exist, nil + } + uz := zoneAddr{Zone: z.Zone, Address: "", Port: z.Port, Transport: z.Transport} + if already, ok := zo.unboundOverlap[uz]; ok { + if z.Address == "" { + // current is not bound to an address, but there is already another zone with a bind address registered + return nil, &already + } + if _, ok := zo.registeredAddr[uz]; ok { + // current zone is bound to an address, but there is already an overlapping zone+port with no bind address + return nil, &uz + } + } + // there is no overlap, keep the current zoneAddr for future checks + zo.registeredAddr[z] = z + zo.unboundOverlap[uz] = z + return nil, nil +} diff --git a/core/dnsserver/address_test.go b/core/dnsserver/address_test.go index 33b8fd898..137bc8e44 100644 --- a/core/dnsserver/address_test.go +++ b/core/dnsserver/address_test.go @@ -108,3 +108,72 @@ func TestSplitProtocolHostPort(t *testing.T) { } } + +type checkCall struct { + zone zoneAddr + same bool + overlap bool + overlapKey string +} + +type checkTest struct { + sequence []checkCall +} + +func TestOverlapAddressChecker(t *testing.T) { + for i, test := range []checkTest{ + {sequence: []checkCall{ + {zoneAddr{Transport: "dns", Zone: ".", Address: "", Port: "53"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: ".", Address: "", Port: "53"}, true, false, ""}, + }, + }, + {sequence: []checkCall{ + {zoneAddr{Transport: "dns", Zone: ".", Address: "", Port: "53"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: ".", Address: "", Port: "54"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: ".", Address: "127.0.0.1", Port: "53"}, false, true, "dns://.:53"}, + }, + }, + {sequence: []checkCall{ + {zoneAddr{Transport: "dns", Zone: ".", Address: "127.0.0.1", Port: "53"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: ".", Address: "", Port: "54"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: ".", Address: "127.0.0.1", Port: "53"}, true, false, ""}, + }, + }, + {sequence: []checkCall{ + {zoneAddr{Transport: "dns", Zone: ".", Address: "127.0.0.1", Port: "53"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: ".", Address: "", Port: "54"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: ".", Address: "128.0.0.1", Port: "53"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: ".", Address: "129.0.0.1", Port: "53"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: ".", Address: "", Port: "53"}, false, true, "dns://.:53 on 129.0.0.1"}, + }, + }, + {sequence: []checkCall{ + {zoneAddr{Transport: "dns", Zone: ".", Address: "127.0.0.1", Port: "53"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: "com.", Address: "127.0.0.1", Port: "53"}, false, false, ""}, + {zoneAddr{Transport: "dns", Zone: "com.", Address: "", Port: "53"}, false, true, "dns://com.:53 on 127.0.0.1"}, + }, + }, + } { + + checker := newOverlapZone() + for _, call := range test.sequence { + same, overlap := checker.registerAndCheck(call.zone) + sZone := call.zone.String() + if (same != nil) != call.same { + t.Errorf("Test %d: error, for zone %s, 'same' (%v) has not the expected value (%v)", i, sZone, same != nil, call.same) + } + if same == nil { + if (overlap != nil) != call.overlap { + t.Errorf("Test %d: error, for zone %s, 'overlap' (%v) has not the expected value (%v)", i, sZone, overlap != nil, call.overlap) + } + if overlap != nil { + if overlap.String() != call.overlapKey { + t.Errorf("Test %d: error, for zone %s, 'overlap Key' (%v) has not the expected value (%v)", i, sZone, overlap.String(), call.overlapKey) + } + + } + } + + } + } +} diff --git a/core/dnsserver/config.go b/core/dnsserver/config.go index 1e952b153..d21a52380 100644 --- a/core/dnsserver/config.go +++ b/core/dnsserver/config.go @@ -2,6 +2,7 @@ package dnsserver import ( "crypto/tls" + "fmt" "net" "github.com/coredns/coredns/plugin" @@ -68,16 +69,22 @@ func (c *Config) HostAddresses() string { return all } +// keyForConfig build a key for identifying the configs during setup time +func keyForConfig(blocIndex int, blocKeyIndex int) string { + return fmt.Sprintf("%d:%d", blocIndex, blocKeyIndex) +} + // GetConfig gets the Config that corresponds to c. // If none exist nil is returned. func GetConfig(c *caddy.Controller) *Config { ctx := c.Context().(*dnsContext) - if cfg, ok := ctx.keysToConfigs[c.Key]; ok { + key := keyForConfig(c.ServerBlockIndex, c.ServerBlockKeyIndex) + if cfg, ok := ctx.keysToConfigs[key]; ok { return cfg } // we should only get here during tests because directive // actions typically skip the server blocks where we make // the configs. - ctx.saveConfig(c.Key, &Config{ListenHosts: []string{""}}) + ctx.saveConfig(key, &Config{ListenHosts: []string{""}}) return GetConfig(c) } diff --git a/core/dnsserver/register.go b/core/dnsserver/register.go index 8be36cd22..779dc6b5f 100644 --- a/core/dnsserver/register.go +++ b/core/dnsserver/register.go @@ -55,28 +55,23 @@ func (h *dnsContext) saveConfig(key string, cfg *Config) { // be parsed and executed. func (h *dnsContext) InspectServerBlocks(sourceFile string, serverBlocks []caddyfile.ServerBlock) ([]caddyfile.ServerBlock, error) { // Normalize and check all the zone names and check for duplicates - dups := map[string]string{} - for _, s := range serverBlocks { - for i, k := range s.Keys { + for ib, s := range serverBlocks { + for ik, k := range s.Keys { za, err := normalizeZone(k) if err != nil { return nil, err } - s.Keys[i] = za.String() - if v, ok := dups[za.String()]; ok { - return nil, fmt.Errorf("cannot serve %s - zone already defined for %v", za, v) - } - dups[za.String()] = za.String() - + s.Keys[ik] = za.String() // Save the config to our master list, and key it for lookups. cfg := &Config{ Zone: za.Zone, + ListenHosts: []string{""}, Port: za.Port, Transport: za.Transport, - ListenHosts: []string{""}, } + keyConfig := keyForConfig(ib, ik) if za.IPNet == nil { - h.saveConfig(za.String(), cfg) + h.saveConfig(keyConfig, cfg) continue } @@ -91,7 +86,7 @@ func (h *dnsContext) InspectServerBlocks(sourceFile string, serverBlocks []caddy return za.IPNet.Contains(net.ParseIP(addr)) } } - h.saveConfig(za.String(), cfg) + h.saveConfig(keyConfig, cfg) } } return serverBlocks, nil @@ -100,6 +95,13 @@ func (h *dnsContext) InspectServerBlocks(sourceFile string, serverBlocks []caddy // MakeServers uses the newly-created siteConfigs to create and return a list of server instances. func (h *dnsContext) MakeServers() ([]caddy.Server, error) { + // Now that all Keys and Directives are parsed and initialized + // lets verify that there is no overlap on the zones and addresses to listen for + errValid := h.validateZonesAndListeningAddresses() + if errValid != nil { + return nil, errValid + } + // we must map (group) each config to a bind address groups, err := groupConfigsByListenAddr(h.configs) if err != nil { @@ -183,14 +185,35 @@ func (c *Config) Handlers() []plugin.Handler { return hs } +func (h *dnsContext) validateZonesAndListeningAddresses() error { + //Validate Zone and addresses + checker := newOverlapZone() + for _, conf := range h.configs { + for _, h := range conf.ListenHosts { + // Validate the overlapping of ZoneAddr + akey := zoneAddr{Transport: conf.Transport, Zone: conf.Zone, Address: h, Port: conf.Port} + existZone, overlapZone := checker.registerAndCheck(akey) + if existZone != nil { + return fmt.Errorf("cannot serve %s - it is already defined", akey.String()) + } + if overlapZone != nil { + return fmt.Errorf("cannot serve %s - zone overlap listener capacity with %v", akey.String(), overlapZone.String()) + } + + } + } + return nil + +} + // groupSiteConfigsByListenAddr groups site configs by their listen // (bind) address, so sites that use the same listener can be served // on the same server instance. The return value maps the listen // 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) + groups := make(map[string][]*Config) for _, conf := range configs { for _, h := range conf.ListenHosts { addr, err := net.ResolveTCPAddr("tcp", net.JoinHostPort(h, conf.Port)) @@ -201,6 +224,7 @@ func groupConfigsByListenAddr(configs []*Config) (map[string][]*Config, error) { groups[addrstr] = append(groups[addrstr], conf) } } + return groups, nil } diff --git a/core/dnsserver/register_test.go b/core/dnsserver/register_test.go index 626fdc710..1bf279a79 100644 --- a/core/dnsserver/register_test.go +++ b/core/dnsserver/register_test.go @@ -51,7 +51,7 @@ func TestGroupingServers(t *testing.T) { expectedGroups: []string{"dns://:53", "dns://:54"}, failing: false}, - // 2 configs on same port, same broadcast address, diff zones -> 1 group + // 2 configs on same port, both not using bind, diff zones -> 1 group {configs: []*Config{ {Transport: "dns", Zone: ".", Port: "53", ListenHosts: []string{""}}, {Transport: "dns", Zone: "com.", Port: "53", ListenHosts: []string{""}}, @@ -59,7 +59,7 @@ func TestGroupingServers(t *testing.T) { expectedGroups: []string{"dns://:53"}, failing: false}, - // 2 configs on same port, same address, diff zones -> 1 group + // 2 configs on same port, one addressed - one not using bind, diff zones -> 1 group {configs: []*Config{ {Transport: "dns", Zone: ".", Port: "53", ListenHosts: []string{"127.0.0.1"}}, {Transport: "dns", Zone: ".", Port: "54", ListenHosts: []string{""}}, @@ -74,7 +74,7 @@ func TestGroupingServers(t *testing.T) { expectedGroups: []string{"dns://127.0.0.1:53", "dns://[::1]:53", "dns://:54"}, failing: false}, - // 2 configs on same port, same unicast address, diff zones -> 1 group + // 2 configs on same port, same address, diff zones -> 1 group {configs: []*Config{ {Transport: "dns", Zone: ".", Port: "53", ListenHosts: []string{"127.0.0.1", "::1"}}, {Transport: "dns", Zone: "com.", Port: "53", ListenHosts: []string{"127.0.0.1", "::1"}}, diff --git a/core/dnsserver/server.go b/core/dnsserver/server.go index f10bdb78b..07137494f 100644 --- a/core/dnsserver/server.go +++ b/core/dnsserver/server.go @@ -65,6 +65,10 @@ func NewServer(addr string, group []*Config) (*Server, error) { // set the config per zone s.zones[site.Zone] = site // compile custom plugin for everything + if site.registry != nil { + // this config is already computed with the chain of plugin + continue + } var stack plugin.Handler for i := len(site.Plugin) - 1; i >= 0; i-- { stack = site.Plugin[i](stack)