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>
This commit is contained in:
Miek Gieben
2021-05-17 22:19:54 +02:00
committed by GitHub
parent 5409379648
commit 5f41d8eb1f
32 changed files with 259 additions and 510 deletions

View File

@@ -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, "://")

View File

@@ -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

View File

@@ -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{

View File

@@ -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
}

View File

@@ -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])
}
}
}
}