plugin/hosts: make unsupported type fallthrough opt-in (#8282)

notes/coredns-1.14.5.md

@@ -70,3 +70,11 @@ plugin/trace: Correct Zipkin v2 endpoint docs (https://github.com/coredns/coredn
 plugin/transfer: Configure notify source address (https://github.com/coredns/coredns/pull/8192)
 plugin/transfer: Fix panic in CoreDNS transfer plugin caused by empty DNS record (https://github.com/coredns/coredns/pull/8207)
 plugin/tsig: Don't echo client's TSIG.Error if verification is successful (https://github.com/coredns/coredns/pull/8215)
+
+### Hosts Fallthrough Behavior Change
+
+In CoreDNS 1.14.5, queries for record types unsupported by the *hosts* plugin continue to the next
+plugin when `fallthrough` is configured, even if the queried name has an A or AAAA entry in the
+hosts data. Previously, those queries received an authoritative NODATA response. Deployments that
+use hosts entries as split-horizon overrides should review whether forwarding TXT, HTTPS, SVCB, or
+other unsupported query types upstream is appropriate.

plugin/hosts/README.md

@@ -73,6 +73,7 @@ hosts [FILE [ZONES...]] {
     no_reverse
     reload DURATION
     fallthrough [ZONES...]
+    fallthrough_unsupported
 }
 ~~~
 
@@ -93,6 +94,12 @@ hosts [FILE [ZONES...]] {
   If **[ZONES...]** is omitted, then fallthrough happens for all zones for which the plugin
   is authoritative. If specific zones are listed (for example `in-addr.arpa` and `ip6.arpa`), then only
   queries for those zones will be subject to fallthrough.
+  By default, queries for unsupported record types return an authoritative NODATA response when
+  the queried name has an A or AAAA entry in the hosts data.
+* `fallthrough_unsupported` extends `fallthrough` to unsupported query types when the queried name
+  exists in the hosts data. For example, TXT, HTTPS, and SVCB queries for a name with an A or AAAA
+  entry are passed to the next plugin. If that plugin is *forward*, these queries are sent upstream.
+  This option requires `fallthrough` and uses the same zone scope.
 
 ## Metrics
 

plugin/hosts/hosts.go

@@ -18,6 +18,8 @@ type Hosts struct {
 	*Hostsfile
 
 	Fall fall.F
+
+	fallthroughUnsupported bool
 }
 
 // ServeDNS implements the plugin.Handle interface.
@@ -51,7 +53,7 @@ func (h Hosts) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (
 		ips := h.LookupStaticHostV6(qname)
 		answers = aaaa(qname, h.options.ttl, ips)
 	default:
-		if h.Fall.Through(qname) {
+		if h.fallthroughUnsupported && h.Fall.Through(qname) {
 			return plugin.NextOrFailure(h.Name(), h.Next, ctx, w, r)
 		}
 	}

plugin/hosts/hosts_test.go

@@ -58,31 +58,81 @@ func TestLookupA(t *testing.T) {
 }
 
 func TestFallthroughUnsupportedType(t *testing.T) {
-	h := Hosts{
-		Next: test.NextHandler(dns.RcodeRefused, nil),
-		Hostsfile: &Hostsfile{
-			Origins: []string{"."},
-			hmap:    newMap(),
-			inline:  newMap(),
-			options: newOptions(),
+	tests := []struct {
+		name              string
+		qname             string
+		fall              fall.F
+		unsupported       bool
+		expectFallthrough bool
+	}{
+		{
+			name:  "existing name returns nodata by default",
+			qname: "example.org.",
+			fall:  fall.Root,
+		},
+		{
+			name:              "existing name falls through with opt-in",
+			qname:             "example.org.",
+			fall:              fall.Root,
+			unsupported:       true,
+			expectFallthrough: true,
+		},
+		{
+			name:        "opt-in respects fallthrough zones",
+			qname:       "example.org.",
+			fall:        fall.F{Zones: []string{"example.net."}},
+			unsupported: true,
+		},
+		{
+			name:              "missing name still falls through",
+			qname:             "missing.example.org.",
+			fall:              fall.Root,
+			expectFallthrough: true,
 		},
-		Fall: fall.Root,
 	}
-	h.hmap = h.parse(strings.NewReader(hostsExample))
 
-	m := new(dns.Msg)
-	m.SetQuestion("example.org.", dns.TypeTXT)
-	rec := dnstest.NewRecorder(&test.ResponseWriter{})
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			h := Hosts{
+				Next: test.NextHandler(dns.RcodeRefused, nil),
+				Hostsfile: &Hostsfile{
+					Origins: []string{"."},
+					hmap:    newMap(),
+					inline:  newMap(),
+					options: newOptions(),
+				},
+				Fall:                   tc.fall,
+				fallthroughUnsupported: tc.unsupported,
+			}
+			h.hmap = h.parse(strings.NewReader(hostsExample))
 
-	rcode, err := h.ServeDNS(context.Background(), rec, m)
-	if err != nil {
-		t.Fatalf("Expected no error, got %v", err)
-	}
-	if rcode != dns.RcodeRefused {
-		t.Fatalf("Expected fallthrough rcode %d, got %d", dns.RcodeRefused, rcode)
-	}
-	if rec.Msg != nil {
-		t.Fatalf("Expected no response from hosts after fallthrough, got %#v", rec.Msg)
+			m := new(dns.Msg)
+			m.SetQuestion(tc.qname, dns.TypeTXT)
+			rec := dnstest.NewRecorder(&test.ResponseWriter{})
+
+			rcode, err := h.ServeDNS(context.Background(), rec, m)
+			if err != nil {
+				t.Fatalf("Expected no error, got %v", err)
+			}
+			if tc.expectFallthrough {
+				if rcode != dns.RcodeRefused {
+					t.Fatalf("Expected fallthrough rcode %d, got %d", dns.RcodeRefused, rcode)
+				}
+				if rec.Msg != nil {
+					t.Fatalf("Expected no response from hosts after fallthrough, got %#v", rec.Msg)
+				}
+				return
+			}
+			if rcode != dns.RcodeSuccess {
+				t.Fatalf("Expected authoritative NODATA rcode %d, got %d", dns.RcodeSuccess, rcode)
+			}
+			if rec.Msg == nil {
+				t.Fatal("Expected authoritative NODATA response from hosts, got no response")
+			}
+			if !rec.Msg.Authoritative || len(rec.Msg.Answer) != 0 {
+				t.Fatalf("Expected authoritative NODATA response, got %#v", rec.Msg)
+			}
+		})
 	}
 }
 

plugin/hosts/setup.go

@@ -112,6 +112,11 @@ func hostsParse(c *caddy.Controller) (Hosts, error) {
 			switch c.Val() {
 			case "fallthrough":
 				h.Fall.SetZonesFromArgs(c.RemainingArgs())
+			case "fallthrough_unsupported":
+				if len(c.RemainingArgs()) != 0 {
+					return h, c.ArgErr()
+				}
+				h.fallthroughUnsupported = true
 			case "no_reverse":
 				h.options.autoReverse = false
 			case "ttl":
@@ -150,6 +155,9 @@ func hostsParse(c *caddy.Controller) (Hosts, error) {
 			}
 		}
 	}
+	if h.fallthroughUnsupported && len(h.Fall.Zones) == 0 {
+		return h, c.Err("fallthrough_unsupported requires fallthrough")
+	}
 
 	h.initInline(inline)
 

plugin/hosts/setup_test.go

@@ -167,3 +167,60 @@ func TestHostsInlineParse(t *testing.T) {
 		}
 	}
 }
+
+func TestHostsParseFallthroughUnsupported(t *testing.T) {
+	tests := []struct {
+		name      string
+		input     string
+		shouldErr bool
+	}{
+		{
+			name: "with fallthrough",
+			input: `hosts {
+				fallthrough
+				fallthrough_unsupported
+			}`,
+		},
+		{
+			name: "before fallthrough",
+			input: `hosts {
+				fallthrough_unsupported
+				fallthrough example.org
+			}`,
+		},
+		{
+			name: "without fallthrough",
+			input: `hosts {
+				fallthrough_unsupported
+			}`,
+			shouldErr: true,
+		},
+		{
+			name: "with arguments",
+			input: `hosts {
+				fallthrough
+				fallthrough_unsupported example.org
+			}`,
+			shouldErr: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			c := caddy.NewTestController("dns", test.input)
+			h, err := hostsParse(c)
+			if test.shouldErr {
+				if err == nil {
+					t.Fatal("Expected an error, got none")
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("Expected no error, got %v", err)
+			}
+			if !h.fallthroughUnsupported {
+				t.Fatal("Expected fallthrough_unsupported to be enabled")
+			}
+		})
+	}
+}