Merge commit from fork

EndpointSlice FQDN addresses were copied into endpoint records as if
they were IP addresses. CoreDNS then treated each value as a CNAME
target. A tenant could build a branching chain of headless Services that
made one query perform exponential work and exhaust CoreDNS memory.

Publish endpoints only from IPv4 and IPv6 slices and validate every
address against its declared family. Deduplicate CNAME targets for the
whole query so repeated or converging paths are chased once.

Protect internal self-lookups with an independent depth limit. This
stops direct and indirect k8s_external CNAME loops without consuming the
loop counter already maintained by the file and rewrite plugins.

Add regression coverage for address types, recursive convergence,
repeated k8s_external targets, and direct and indirect self-reference.

Signed-off-by: Ville Vesilehto <ville@vesilehto.fi>

plugin/pkg/upstream/upstream.go

@@ -3,6 +3,7 @@ package upstream
 
 import (
 	"context"
+	"errors"
 	"fmt"
 
 	"github.com/coredns/coredns/core/dnsserver"
@@ -18,14 +19,41 @@ type Upstream struct{}
 // New creates a new Upstream to resolve names using the coredns process.
 func New() *Upstream { return &Upstream{} }
 
+// maxDepth is the number of nested self-lookups one client query may cause.
+// Each Lookup re-enters the server's plugin chain, and a plugin that answers
+// with another CNAME re-enters it again. Backends serve data that other
+// parties write (a Kubernetes tenant's Service and EndpointSlice, an etcd
+// record), so a target that leads back to itself must stop here instead of
+// growing the Go stack until the process dies. This matches the generic
+// backend's CNAME-chain limit.
+const maxDepth = 10
+
+// errMaxDepth is returned when a lookup would nest deeper than maxDepth.
+var errMaxDepth = errors.New("upstream lookup depth exceeded")
+
+// depthKey is deliberately separate from dnsserver.LoopKey. The file and
+// rewrite plugins already maintain LoopKey around their own CNAME processing;
+// sharing it here would count each re-entry twice and shorten valid chains.
+type depthKey struct{}
+
 // Lookup routes lookups to our selves to make it follow the plugin chain *again*, but with a (possibly) new query. As
 // we are doing the query against ourselves again, there is no actual new hop, as such RFC 6891 does not apply and we
 // need the EDNS0 option present in the *original* query to be present here too.
+//
+// The nesting depth travels with the context so chains that alternate between
+// plugins are bounded as one. Lookup returns errMaxDepth once maxDepth nested
+// lookups are in progress for the client query.
 func (u *Upstream) Lookup(ctx context.Context, state request.Request, name string, typ uint16) (*dns.Msg, error) {
 	server, ok := ctx.Value(dnsserver.Key{}).(*dnsserver.Server)
 	if !ok {
 		return nil, fmt.Errorf("no full server is running")
 	}
+	depth, _ := ctx.Value(depthKey{}).(int)
+	if depth >= maxDepth {
+		return nil, errMaxDepth
+	}
+	ctx = context.WithValue(ctx, depthKey{}, depth+1)
+
 	req := state.NewWithQuestion(name, typ)
 
 	nw := nonwriter.New(state.W)

plugin/pkg/upstream/upstream_test.go

@@ -0,0 +1,101 @@
+package upstream
+
+import (
+	"context"
+	"errors"
+	"testing"
+
+	"github.com/coredns/coredns/core/dnsserver"
+	"github.com/coredns/coredns/plugin"
+	"github.com/coredns/coredns/plugin/test"
+	"github.com/coredns/coredns/request"
+
+	"github.com/miekg/dns"
+)
+
+// selfReferencing answers every query with a CNAME to the query name itself
+// and then asks the upstream for that target, the way a backend does for a
+// CNAME-typed service whose target leads back to the backend.
+type selfReferencing struct {
+	up    *Upstream
+	calls int
+}
+
+func (s *selfReferencing) Name() string { return "selfref" }
+
+func (s *selfReferencing) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) (int, error) {
+	s.calls++
+	state := request.Request{W: w, Req: r}
+	m := new(dns.Msg)
+	m.SetReply(r)
+	m.Answer = []dns.RR{test.CNAME(state.QName() + " 5 IN CNAME " + state.QName())}
+	if resp, err := s.up.Lookup(ctx, state, state.QName(), state.QType()); err == nil && resp != nil {
+		m.Answer = append(m.Answer, resp.Answer...)
+	}
+	w.WriteMsg(m)
+	return dns.RcodeSuccess, nil
+}
+
+func TestLookupDepthIsBounded(t *testing.T) {
+	h := &selfReferencing{up: New()}
+	cfg := &dnsserver.Config{Zone: ".", Plugin: []plugin.Plugin{func(plugin.Handler) plugin.Handler { return h }}}
+	server, err := dnsserver.NewServer("dns://:53", []*dnsserver.Config{cfg})
+	if err != nil {
+		t.Fatalf("NewServer: %v", err)
+	}
+
+	ctx := context.WithValue(context.Background(), dnsserver.Key{}, server)
+	m := new(dns.Msg)
+	m.SetQuestion("x.example.org.", dns.TypeA)
+	rec := &test.ResponseWriter{}
+	server.ServeDNS(ctx, rec, m)
+
+	// The client query plus maxDepth nested lookups.
+	if want := maxDepth + 1; h.calls != want {
+		t.Fatalf("expected %d invocations of the plugin chain, got %d", want, h.calls)
+	}
+}
+
+func TestLookupReturnsErrMaxDepth(t *testing.T) {
+	server, err := dnsserver.NewServer("dns://:53", []*dnsserver.Config{{Zone: "."}})
+	if err != nil {
+		t.Fatalf("NewServer: %v", err)
+	}
+	ctx := context.WithValue(context.Background(), dnsserver.Key{}, server)
+	ctx = context.WithValue(ctx, depthKey{}, maxDepth)
+
+	m := new(dns.Msg)
+	m.SetQuestion("x.example.org.", dns.TypeA)
+	state := request.Request{W: &test.ResponseWriter{}, Req: m}
+	if _, err := New().Lookup(ctx, state, "y.example.org.", dns.TypeA); !errors.Is(err, errMaxDepth) {
+		t.Fatalf("expected errMaxDepth, got %v", err)
+	}
+}
+
+func TestLookupDoesNotConsumeServerLoopDepth(t *testing.T) {
+	h := &selfReferencing{up: New()}
+	cfg := &dnsserver.Config{Zone: ".", Plugin: []plugin.Plugin{func(plugin.Handler) plugin.Handler { return h }}}
+	server, err := dnsserver.NewServer("dns://:53", []*dnsserver.Config{cfg})
+	if err != nil {
+		t.Fatalf("NewServer: %v", err)
+	}
+
+	ctx := context.WithValue(context.Background(), dnsserver.Key{}, server)
+	ctx = context.WithValue(ctx, dnsserver.LoopKey{}, maxDepth)
+
+	m := new(dns.Msg)
+	m.SetQuestion("x.example.org.", dns.TypeA)
+	state := request.Request{W: &test.ResponseWriter{}, Req: m}
+	if _, err := New().Lookup(ctx, state, "y.example.org.", dns.TypeA); err != nil {
+		t.Fatalf("Lookup unexpectedly used dnsserver.LoopKey: %v", err)
+	}
+}
+
+func TestLookupWithoutServer(t *testing.T) {
+	m := new(dns.Msg)
+	m.SetQuestion("x.example.org.", dns.TypeA)
+	state := request.Request{W: &test.ResponseWriter{}, Req: m}
+	if _, err := New().Lookup(context.Background(), state, "y.example.org.", dns.TypeA); err == nil {
+		t.Fatal("expected an error without a server in the context")
+	}
+}