From e1c1521ad564dbae4106fba7c8cf29d9bb62778c Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Mon, 7 Aug 2017 13:24:09 -0700 Subject: [PATCH] Core: convert IP addresses to reverse zone (#838) * Core: convert IP addresses to reverse zone If we see IP/mask syntax and the mask mod 8 == 0 we assume a reverse zone and convert to in-addr or .arpa. * typos * integration test * Addr is not used * core: clean up normalize Create a SplitHostPort function that can be used both from normalize.go and address.go. This removes some (not all!) duplication between the both and makes it work with reverse address notations. * More tests --- README.md | 13 +++++++ core/dnsserver/address.go | 21 +++-------- core/dnsserver/address_test.go | 35 +++++++++++++++++ middleware/normalize.go | 68 ++++++++++++++++++++++++++-------- middleware/normalize_test.go | 18 ++------- test/reverse_test.go | 37 ++++++++++++++++++ 6 files changed, 145 insertions(+), 47 deletions(-) diff --git a/README.md b/README.md index 4d9af837f..2533d0721 100644 --- a/README.md +++ b/README.md @@ -168,6 +168,19 @@ example.org { } ~~~ +IP addresses are also allowed. They are automatically converted to reverse zones: + +~~~ txt +10.0.0.0/24 { + # ... +} +~~~ +Means you are authoritative for `0.0.10.in-addr.arpa.`. + +The netmask must be dividable by 8, if it is not the reverse conversion is not done. This also works +for IPv6 addresses. If for some reason you want to serve a zone named `10.0.0.0/24` add the closing +dot: `10.0.0.0/24.` as this also stops the conversion. + Listening on TLS and for gRPC? Use: ~~~ txt diff --git a/core/dnsserver/address.go b/core/dnsserver/address.go index ded8e0420..a6087df9f 100644 --- a/core/dnsserver/address.go +++ b/core/dnsserver/address.go @@ -1,10 +1,10 @@ package dnsserver import ( - "fmt" - "net" "strings" + "github.com/coredns/coredns/middleware" + "github.com/miekg/dns" ) @@ -32,8 +32,6 @@ func Transport(s string) string { // normalizeZone parses an zone string into a structured format with separate // host, and port portions, as well as the original input string. -// -// TODO(miek): possibly move this to middleware/normalize.go func normalizeZone(str string) (zoneAddr, error) { var err error @@ -52,18 +50,9 @@ func normalizeZone(str string) (zoneAddr, error) { str = str[len(TransportGRPC+"://"):] } - host, port, err := net.SplitHostPort(str) + host, port, err := middleware.SplitHostPort(str) if err != nil { - host, port, err = net.SplitHostPort(str + ":") - // no error check here; return err at end of function - } - - if len(host) > 255 { // TODO(miek): this should take escaping into account. - return zoneAddr{}, fmt.Errorf("specified zone is too long: %d > 255", len(host)) - } - _, d := dns.IsDomainName(host) - if !d { - return zoneAddr{}, fmt.Errorf("zone is not a valid domain name: %s", host) + return zoneAddr{}, err } if port == "" { @@ -78,7 +67,7 @@ func normalizeZone(str string) (zoneAddr, error) { } } - return zoneAddr{Zone: strings.ToLower(dns.Fqdn(host)), Port: port, Transport: trans}, err + return zoneAddr{Zone: dns.Fqdn(host), Port: port, Transport: trans}, nil } // Supported transports. diff --git a/core/dnsserver/address_test.go b/core/dnsserver/address_test.go index 84747d872..5bf0d4f3d 100644 --- a/core/dnsserver/address_test.go +++ b/core/dnsserver/address_test.go @@ -12,6 +12,41 @@ func TestNormalizeZone(t *testing.T) { {".:54", "dns://.:54", false}, {"..", "://:", true}, {"..", "://:", true}, + {".:", "://:", 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.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.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://2003::53/67.:53", false}, + {"10.0.0.0/25.", "dns://10.0.0.0/25.:53", false}, } { addr, err := normalizeZone(test.input) actual := addr.String() diff --git a/middleware/normalize.go b/middleware/normalize.go index 77ef97993..18fe58f61 100644 --- a/middleware/normalize.go +++ b/middleware/normalize.go @@ -1,7 +1,9 @@ package middleware import ( + "fmt" "net" + "strconv" "strings" "github.com/miekg/dns" @@ -53,8 +55,6 @@ func (n Name) Normalize() string { return strings.ToLower(dns.Fqdn(string(n))) } type ( // Host represents a host from the Corefile, may contain port. Host string // Host represents a host from the Corefile, may contain port. - // Addr represents an address in the Corefile. - Addr string // Addr resprents an address in the Corefile. ) // Normalize will return the host portion of host, stripping @@ -72,24 +72,60 @@ func (h Host) Normalize() string { s = s[len(TransportGRPC+"://"):] } - // separate host and port - host, _, err := net.SplitHostPort(s) - if err != nil { - host, _, _ = net.SplitHostPort(s + ":") - } + // The error can be ignore here, because this function is called after the corefile + // has already been vetted. + host, _, _ := SplitHostPort(s) return Name(host).Normalize() } -// Normalize will return a normalized address, if not port is specified -// port 53 is added, otherwise the port will be left as is. -func (a Addr) Normalize() string { - // separate host and port - addr, port, err := net.SplitHostPort(string(a)) - if err != nil { - addr, port, _ = net.SplitHostPort(string(a) + ":53") +// 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:// +func SplitHostPort(s string) (host, 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 its a number. + host = s + + colon := strings.LastIndex(s, ":") + if colon == len(s)-1 { + return "", "", fmt.Errorf("expecting data after last colon: %q", s) } - // TODO(miek): lowercase it? - return net.JoinHostPort(addr, port) + if colon != -1 { + if p, err := strconv.Atoi(s[colon+1:]); err == nil { + port = strconv.Itoa(p) + host = s[:colon] + } + } + + // TODO(miek): this should take escaping into account. + if len(host) > 255 { + return "", "", fmt.Errorf("specified zone is too long: %d > 255", len(host)) + } + + _, d := dns.IsDomainName(host) + if !d { + return "", "", fmt.Errorf("zone is not a valid domain name: %s", host) + } + + // Check if it parses as a reverse zone, if so we use that. Must be fully + // specified IP and mask and mask % 8 = 0. + ip, net, err := net.ParseCIDR(host) + if err == nil { + if rev, e := dns.ReverseAddr(ip.String()); e == nil { + ones, bits := net.Mask.Size() + if (bits-ones)%8 == 0 { + offset, end := 0, false + for i := 0; i < (bits-ones)/8; i++ { + offset, end = dns.NextLabel(rev, offset) + if end { + break + } + } + host = rev[offset:] + } + } + } + return host, port, nil } // Duplicated from core/dnsserver/address.go ! diff --git a/middleware/normalize_test.go b/middleware/normalize_test.go index 285e185fd..e3d2268d3 100644 --- a/middleware/normalize_test.go +++ b/middleware/normalize_test.go @@ -69,7 +69,9 @@ func TestNameNormalize(t *testing.T) { } func TestHostNormalize(t *testing.T) { - hosts := []string{".:53", ".", "example.org:53", "example.org.", "example.org.:53", "example.org."} + 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.0.0.0/9.", + "dns://example.org", "example.org."} for i := 0; i < len(hosts); i += 2 { ts := hosts[i] @@ -80,17 +82,3 @@ func TestHostNormalize(t *testing.T) { } } } - -func TestAddrNormalize(t *testing.T) { - addrs := []string{".:53", ".:53", "example.org", "example.org:53", "example.org.:1053", "example.org.:1053"} - - for i := 0; i < len(addrs); i += 2 { - ts := addrs[i] - expected := addrs[i+1] - actual := Addr(ts).Normalize() - if expected != actual { - t.Errorf("Expected %v, got %v\n", expected, actual) - } - } - -} diff --git a/test/reverse_test.go b/test/reverse_test.go index 6342f66b8..2a32ed968 100644 --- a/test/reverse_test.go +++ b/test/reverse_test.go @@ -93,3 +93,40 @@ func TestReverseFallthrough(t *testing.T) { t.Errorf("Expected 127.0.0.1, got: %s", resp.Answer[0].(*dns.A).A.String()) } } + +func TestReverseCorefile(t *testing.T) { + corefile := `10.0.0.0/24:0 { + whoami + }` + + i, err := CoreDNSServer(corefile) + if err != nil { + t.Fatalf("Could not get CoreDNS serving instance: %s", err) + } + defer i.Stop() + + udp, _ := CoreDNSServerPorts(i, 0) + if udp == "" { + t.Fatalf("Could not get UDP listening port") + } + + log.SetOutput(ioutil.Discard) + + p := proxy.NewLookup([]string{udp}) + state := request.Request{W: &test.ResponseWriter{}, Req: new(dns.Msg)} + resp, err := p.Lookup(state, "17.0.0.10.in-addr.arpa.", dns.TypePTR) + if err != nil { + t.Fatal("Expected to receive reply, but didn't") + } + + if len(resp.Extra) != 2 { + t.Fatal("Expected to at least two RRs in the extra section, got none") + } + // Second one is SRV, first one can be A or AAAA depending on system. + if resp.Extra[1].Header().Rrtype != dns.TypeSRV { + t.Errorf("Expected RR to SRV, got: %d", resp.Extra[1].Header().Rrtype) + } + if resp.Extra[1].Header().Name != "_udp.17.0.0.10.in-addr.arpa." { + t.Errorf("Expected _udp.17.0.0.10.in-addr.arpa. got: %s", resp.Extra[1].Header().Name) + } +}