diff --git a/core/dnsserver/config.go b/core/dnsserver/config.go index 85e07e1ee..9d5e136e6 100644 --- a/core/dnsserver/config.go +++ b/core/dnsserver/config.go @@ -135,6 +135,10 @@ type Config struct { // TSIG secrets, [name]key. TsigSecret map[string]string + // allowedOpcodes contains non-default DNS opcodes that plugins have explicitly + // requested for this server block. QUERY and NOTIFY are accepted by default. + allowedOpcodes map[int]struct{} + // Plugin stack. Plugin []plugin.Plugin diff --git a/core/dnsserver/config_test.go b/core/dnsserver/config_test.go index 8569b7899..9acd8e0df 100644 --- a/core/dnsserver/config_test.go +++ b/core/dnsserver/config_test.go @@ -5,6 +5,8 @@ import ( "github.com/coredns/caddy" "github.com/coredns/coredns/plugin" + + "github.com/miekg/dns" ) func TestKeyForConfig(t *testing.T) { @@ -102,3 +104,16 @@ func TestPropagateConfigParamsMaxTCPQueries(t *testing.T) { t.Fatalf("expected MaxTCPQueries to propagate to second config as %d, got %v", n, second.MaxTCPQueries) } } + +func TestPropagateConfigParamsAllowedOpcodes(t *testing.T) { + first := &Config{} + first.firstConfigInBlock = first + first.AllowOpcode(dns.OpcodeUpdate) + second := &Config{firstConfigInBlock: first} + + propagateConfigParams([]*Config{first, second}) + + if !second.acceptsOpcode(dns.OpcodeUpdate) { + t.Fatal("expected UPDATE admission to propagate to every zone in the server block") + } +} diff --git a/core/dnsserver/register.go b/core/dnsserver/register.go index 0295d3515..f85948d52 100644 --- a/core/dnsserver/register.go +++ b/core/dnsserver/register.go @@ -184,6 +184,17 @@ func (c *Config) AddPlugin(m plugin.Plugin) { c.Plugin = append(c.Plugin, m) } +// AllowOpcode permits a non-default DNS opcode to reach this config's plugin chain +// on UDP, TCP, and DNS-over-TLS listeners. Plugins should call it during setup. +// The listener still requires exactly one question, and configs that do not opt in +// continue to reject the opcode. +func (c *Config) AllowOpcode(opcode int) { + if c.allowedOpcodes == nil { + c.allowedOpcodes = make(map[int]struct{}) + } + c.allowedOpcodes[opcode] = struct{}{} +} + // registerHandler adds a handler to a site's handler registration. Handlers // // use this to announce that they exist to other plugin. @@ -276,6 +287,7 @@ func propagateConfigParams(configs []*Config) { c.IdleTimeout = c.firstConfigInBlock.IdleTimeout c.MaxTCPQueries = c.firstConfigInBlock.MaxTCPQueries c.TsigSecret = c.firstConfigInBlock.TsigSecret + c.allowedOpcodes = c.firstConfigInBlock.allowedOpcodes // Propagate HTTPRequestValidateFunc so that custom path validators work in // multi-transport blocks. Otherwise HTTPS 404s on non-"/dns-query" paths. diff --git a/core/dnsserver/server.go b/core/dnsserver/server.go index 453285f8b..ad6af9e06 100644 --- a/core/dnsserver/server.go +++ b/core/dnsserver/server.go @@ -65,7 +65,8 @@ type Server struct { stacktrace bool // enable stacktrace in recover error log classChaos bool // allow non-INET class queries - tsigSecret map[string]string + tsigSecret map[string]string + allowedOpcodes map[int]struct{} // udpDecorateWriterFunc is selected in NewServer from the group configs in // stable order (last one set wins), so the choice is deterministic when @@ -86,14 +87,15 @@ type MetadataCollector interface { // queries are blocked unless queries from enableChaos are loaded. func NewServer(addr string, group []*Config) (*Server, error) { s := &Server{ - Addr: addr, - zones: make(map[string][]*Config), - graceTimeout: 5 * time.Second, - IdleTimeout: 10 * time.Second, - ReadTimeout: 3 * time.Second, - WriteTimeout: 5 * time.Second, - MaxTCPQueries: tcpMaxQueries, - tsigSecret: make(map[string]string), + Addr: addr, + zones: make(map[string][]*Config), + graceTimeout: 5 * time.Second, + IdleTimeout: 10 * time.Second, + ReadTimeout: 3 * time.Second, + WriteTimeout: 5 * time.Second, + MaxTCPQueries: tcpMaxQueries, + tsigSecret: make(map[string]string), + allowedOpcodes: make(map[int]struct{}), } for _, site := range group { @@ -122,6 +124,7 @@ func NewServer(addr string, group []*Config) (*Server, error) { // copy tsig secrets maps.Copy(s.tsigSecret, site.TsigSecret) + maps.Copy(s.allowedOpcodes, site.allowedOpcodes) // compile custom plugin for everything var stack plugin.Handler @@ -183,6 +186,7 @@ func (s *Server) Serve(l net.Listener) error { s.server[tcp] = &dns.Server{Listener: l, Net: "tcp", TsigSecret: s.tsigSecret, + MsgAcceptFunc: s.msgAcceptFunc(), MaxTCPQueries: s.MaxTCPQueries, ReadTimeout: s.ReadTimeout, WriteTimeout: s.WriteTimeout, @@ -213,7 +217,7 @@ func (s *Server) ServePacket(p net.PacketConn) error { ctx := context.WithValue(context.Background(), Key{}, s) ctx = context.WithValue(ctx, LoopKey{}, 0) s.ServeDNS(ctx, w, r) - }), TsigSecret: s.tsigSecret, DecorateWriter: dw} + }), TsigSecret: s.tsigSecret, MsgAcceptFunc: s.msgAcceptFunc(), DecorateWriter: dw} s.m.Unlock() return s.server[udp].ActivateAndServe() @@ -344,11 +348,15 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) // If all filter funcs pass, use this config. if passAllFilterFuncs(ctx, h.FilterFuncs, &request.Request{Req: r, W: w}) { + if !h.acceptsOpcode(r.Opcode) { + errorAndMetricsFunc(s.Addr, w, r, dns.RcodeNotImplemented) + return + } if h.ViewName != "" { // if there was a view defined for this Config, set the view name in the context ctx = context.WithValue(ctx, ViewKey{}, h.ViewName) } - if r.Question[0].Qtype != dns.TypeDS { + if r.Opcode != dns.OpcodeQuery || r.Question[0].Qtype != dns.TypeDS { rcode, _ := h.pluginChain.ServeDNS(ctx, w, r) if !plugin.ClientWrite(rcode) { errorFunc(s.Addr, w, r, rcode) @@ -393,6 +401,10 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) // If all filter funcs pass, use this config. if passAllFilterFuncs(ctx, h.FilterFuncs, &request.Request{Req: r, W: w}) { + if !h.acceptsOpcode(r.Opcode) { + errorAndMetricsFunc(s.Addr, w, r, dns.RcodeNotImplemented) + return + } if h.ViewName != "" { // if there was a view defined for this Config, set the view name in the context ctx = context.WithValue(ctx, ViewKey{}, h.ViewName) @@ -410,6 +422,39 @@ func (s *Server) ServeDNS(ctx context.Context, w dns.ResponseWriter, r *dns.Msg) errorAndMetricsFunc(s.Addr, w, r, dns.RcodeRefused) } +// msgAcceptFunc returns nil when no plugin has opted into an additional opcode, +// preserving miekg/dns's default request policy exactly. +func (s *Server) msgAcceptFunc() dns.MsgAcceptFunc { + if len(s.allowedOpcodes) == 0 { + return nil + } + return s.acceptMessage +} + +func (s *Server) acceptMessage(header dns.Header) dns.MsgAcceptAction { + action := dns.DefaultMsgAcceptFunc(header) + if action != dns.MsgRejectNotImplemented { + return action + } + + opcode := int(header.Bits>>11) & 0xF + if _, ok := s.allowedOpcodes[opcode]; !ok { + return action + } + if header.Qdcount != 1 { + return dns.MsgReject + } + return dns.MsgAccept +} + +func (c *Config) acceptsOpcode(opcode int) bool { + if opcode == dns.OpcodeQuery || opcode == dns.OpcodeNotify { + return true + } + _, ok := c.allowedOpcodes[opcode] + return ok +} + // passAllFilterFuncs returns true if all filter funcs evaluate to true for the given request func passAllFilterFuncs(ctx context.Context, filterFuncs []FilterFunc, req *request.Request) bool { for _, ff := range filterFuncs { diff --git a/core/dnsserver/server_test.go b/core/dnsserver/server_test.go index 4475f7228..3576d62ae 100644 --- a/core/dnsserver/server_test.go +++ b/core/dnsserver/server_test.go @@ -41,15 +41,22 @@ func (p *updateResponsePlugin) ServeDNS(_ context.Context, w dns.ResponseWriter, return dns.RcodeSuccess, nil } -func mustPackRFC2136Update(t *testing.T) []byte { +func newRFC2136Update(t *testing.T, zone string) *dns.Msg { t.Helper() - m := new(dns.Msg).SetUpdate("example.com.") - rr, err := dns.NewRR("foo.example.com. 300 IN A 192.0.2.123") + m := new(dns.Msg).SetUpdate(zone) + rr, err := dns.NewRR("host." + zone + " 300 IN A 192.0.2.123") if err != nil { t.Fatalf("dns.NewRR() failed: %v", err) } m.Insert([]dns.RR{rr}) + return m +} + +func mustPackRFC2136Update(t *testing.T) []byte { + t.Helper() + + m := newRFC2136Update(t, "example.com.") // DNS-over-QUIC requires the DNS message ID to be zero. m.Id = 0 @@ -114,6 +121,225 @@ func TestNewServer(t *testing.T) { } } +func TestUpdateAdmission(t *testing.T) { + for _, network := range []string{"udp", "tcp"} { + for _, allow := range []bool{false, true} { + name := network + "/default-reject" + if allow { + name = network + "/explicit-opt-in" + } + t.Run(name, func(t *testing.T) { + handler := new(updateResponsePlugin) + cfg := testConfig("dns", handler) + if allow { + cfg.AllowOpcode(dns.OpcodeUpdate) + } + + response := exchangeWithTestServer(t, network, []*Config{cfg}, newRFC2136Update(t, "example.com.")) + wantRcode := dns.RcodeNotImplemented + if allow { + wantRcode = dns.RcodeSuccess + } + if response.Rcode != wantRcode { + t.Fatalf("rcode = %s, want %s", dns.RcodeToString[response.Rcode], dns.RcodeToString[wantRcode]) + } + if handler.called.Load() != allow { + t.Fatalf("plugin called = %v, want %v", handler.called.Load(), allow) + } + }) + } + } +} + +func TestUpdateAdmissionIsScopedToConfig(t *testing.T) { + dynamicHandler := new(updateResponsePlugin) + dynamicConfig := testConfig("dns", dynamicHandler) + dynamicConfig.Zone = "dynamic.example." + dynamicConfig.AllowOpcode(dns.OpcodeUpdate) + + staticHandler := new(updateResponsePlugin) + staticConfig := testConfig("dns", staticHandler) + staticConfig.Zone = "static.example." + + response := exchangeWithTestServer(t, "udp", []*Config{dynamicConfig, staticConfig}, newRFC2136Update(t, "static.example.")) + if response.Rcode != dns.RcodeNotImplemented { + t.Fatalf("rcode = %s, want NOTIMP", dns.RcodeToString[response.Rcode]) + } + if dynamicHandler.called.Load() || staticHandler.called.Load() { + t.Fatalf("UPDATE reached a plugin: dynamic=%v static=%v", dynamicHandler.called.Load(), staticHandler.called.Load()) + } +} + +func TestUpdateAdmissionHeaderChecks(t *testing.T) { + s := &Server{allowedOpcodes: map[int]struct{}{dns.OpcodeUpdate: {}}} + header := dns.Header{ + Bits: uint16(dns.OpcodeUpdate << 11), + Qdcount: 1, + Ancount: 3, + Nscount: 3, + Arcount: 3, + } + if got := s.acceptMessage(header); got != dns.MsgAccept { + t.Fatalf("valid UPDATE action = %v, want MsgAccept", got) + } + + header.Qdcount = 0 + if got := s.acceptMessage(header); got != dns.MsgReject { + t.Fatalf("zero-question UPDATE action = %v, want MsgReject", got) + } + header.Qdcount = 2 + if got := s.acceptMessage(header); got != dns.MsgReject { + t.Fatalf("two-question UPDATE action = %v, want MsgReject", got) + } + + header = dns.Header{Bits: uint16(dns.OpcodeUpdate<<11) | 1<<15, Qdcount: 1} + if got := s.acceptMessage(header); got != dns.MsgIgnore { + t.Fatalf("UPDATE response action = %v, want MsgIgnore", got) + } + + header = dns.Header{Bits: uint16(dns.OpcodeStatus << 11), Qdcount: 1} + if got := s.acceptMessage(header); got != dns.MsgRejectNotImplemented { + t.Fatalf("unregistered opcode action = %v, want MsgRejectNotImplemented", got) + } + + queryHeader := dns.Header{Qdcount: 1, Nscount: 2} + if got := s.acceptMessage(queryHeader); got != dns.MsgReject { + t.Fatalf("invalid QUERY action = %v, want MsgReject", got) + } +} + +func TestUpdateAdmissionPreservesTSIGStatus(t *testing.T) { + const ( + keyName = "update-key.example." + secret = "MTIzNDU2Nzg5MDEyMzQ1Ng==" + ) + + called := make(chan struct{}, 1) + handler := tsigStatusCheckPlugin{ + t: t, + called: called, + check: func(t *testing.T, status error) { + t.Helper() + if status != nil { + t.Fatalf("TsigStatus() = %v, want nil", status) + } + }, + } + cfg := testConfig("dns", handler) + cfg.AllowOpcode(dns.OpcodeUpdate) + cfg.TsigSecret = map[string]string{keyName: secret} + + request := newRFC2136Update(t, "example.com.") + request.SetTsig(keyName, dns.HmacSHA256, 300, time.Now().Unix()) + client := &dns.Client{TsigSecret: map[string]string{keyName: secret}} + response := exchangeWithTestServerUsingClient(t, "udp", []*Config{cfg}, request, client) + if response.Rcode != dns.RcodeSuccess { + t.Fatalf("rcode = %s, want NOERROR", dns.RcodeToString[response.Rcode]) + } + select { + case <-called: + default: + t.Fatal("TSIG status plugin was not called") + } +} + +func TestUpdateAdmissionPreservesTSIGFailure(t *testing.T) { + const ( + keyName = "update-key.example." + serverSecret = "MTIzNDU2Nzg5MDEyMzQ1Ng==" + clientSecret = "YWJjZGVmZ2hpamtsbW5vcA==" + ) + + status := make(chan error, 1) + handler := tsigStatusCheckPlugin{ + t: t, + called: make(chan struct{}, 1), + check: func(_ *testing.T, got error) { + status <- got + }, + } + cfg := testConfig("dns", handler) + cfg.AllowOpcode(dns.OpcodeUpdate) + cfg.TsigSecret = map[string]string{keyName: serverSecret} + + server, err := NewServer("127.0.0.1:0", []*Config{cfg}) + if err != nil { + t.Fatalf("NewServer() failed: %v", err) + } + packetConn, err := net.ListenPacket("udp", "127.0.0.1:0") + if err != nil { + t.Fatalf("net.ListenPacket() failed: %v", err) + } + go func() { _ = server.ServePacket(packetConn) }() + defer func() { + _ = server.Stop() + _ = packetConn.Close() + }() + + request := newRFC2136Update(t, "example.com.") + request.SetTsig(keyName, dns.HmacSHA256, 300, time.Now().Unix()) + client := &dns.Client{ + Net: "udp", + Timeout: 2 * time.Second, + TsigSecret: map[string]string{keyName: clientSecret}, + } + _, _, _ = client.Exchange(request, packetConn.LocalAddr().String()) + + select { + case got := <-status: + if got == nil { + t.Fatal("TsigStatus() = nil, want verification error") + } + case <-time.After(2 * time.Second): + t.Fatal("UPDATE with invalid TSIG did not reach the status-check plugin") + } +} + +func exchangeWithTestServer(t *testing.T, network string, configs []*Config, request *dns.Msg) *dns.Msg { + t.Helper() + return exchangeWithTestServerUsingClient(t, network, configs, request, new(dns.Client)) +} + +func exchangeWithTestServerUsingClient(t *testing.T, network string, configs []*Config, request *dns.Msg, client *dns.Client) *dns.Msg { + t.Helper() + + s, err := NewServer("127.0.0.1:0", configs) + if err != nil { + t.Fatalf("NewServer() failed: %v", err) + } + + var addr string + switch network { + case "udp": + pc, err := net.ListenPacket("udp", "127.0.0.1:0") + if err != nil { + t.Fatalf("net.ListenPacket() failed: %v", err) + } + addr = pc.LocalAddr().String() + go func() { _ = s.ServePacket(pc) }() + t.Cleanup(func() { _ = pc.Close() }) + case "tcp": + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("net.Listen() failed: %v", err) + } + addr = listener.Addr().String() + go func() { _ = s.Serve(listener) }() + t.Cleanup(func() { _ = listener.Close() }) + default: + t.Fatalf("unsupported network %q", network) + } + t.Cleanup(func() { _ = s.Stop() }) + + client.Net = network + client.Timeout = 2 * time.Second + response, _, err := client.Exchange(request, addr) + if err != nil { + t.Fatalf("dns exchange failed: %v", err) + } + return response +} + func TestDebug(t *testing.T) { configNoDebug, configDebug := testConfig("dns", testPlugin{}), testConfig("dns", testPlugin{}) configDebug.Debug = true diff --git a/core/dnsserver/server_tls.go b/core/dnsserver/server_tls.go index 1f74806a8..d3696870f 100644 --- a/core/dnsserver/server_tls.go +++ b/core/dnsserver/server_tls.go @@ -55,6 +55,7 @@ func (s *ServerTLS) Serve(l net.Listener) error { s.server[tcp] = &dns.Server{Listener: l, Net: "tcp-tls", TsigSecret: s.tsigSecret, + MsgAcceptFunc: s.msgAcceptFunc(), MaxTCPQueries: s.MaxTCPQueries, ReadTimeout: s.ReadTimeout, WriteTimeout: s.WriteTimeout, diff --git a/core/dnsserver/server_tls_test.go b/core/dnsserver/server_tls_test.go index ce7d55733..28e770edb 100644 --- a/core/dnsserver/server_tls_test.go +++ b/core/dnsserver/server_tls_test.go @@ -1,9 +1,13 @@ package dnsserver import ( + "crypto/tls" "errors" "net" "testing" + "time" + + "github.com/miekg/dns" ) type stubListener struct { @@ -55,6 +59,63 @@ func TestServerTLSSetsTsigSecret(t *testing.T) { } } +func TestServerTLSUpdateAdmission(t *testing.T) { + for _, allow := range []bool{false, true} { + name := "default-reject" + if allow { + name = "explicit-opt-in" + } + t.Run(name, func(t *testing.T) { + handler := new(updateResponsePlugin) + config := testConfig("tls", handler) + cert, err := tls.LoadX509KeyPair("../../plugin/tls/test_cert.pem", "../../plugin/tls/test_key.pem") + if err != nil { + t.Fatalf("tls.LoadX509KeyPair() failed: %v", err) + } + config.TLSConfig = &tls.Config{Certificates: []tls.Certificate{cert}} + if allow { + config.AllowOpcode(dns.OpcodeUpdate) + } + server, err := NewServerTLS("tls://127.0.0.1:0", []*Config{config}) + if err != nil { + t.Fatalf("NewServerTLS() failed: %v", err) + } + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("net.Listen() failed: %v", err) + } + go func() { _ = server.Serve(listener) }() + t.Cleanup(func() { + _ = server.Stop() + _ = listener.Close() + }) + + client := &dns.Client{ + Net: "tcp-tls", + Timeout: 2 * time.Second, + TLSConfig: &tls.Config{ + InsecureSkipVerify: true, // #nosec G402 -- the checked-in test certificate has no SAN. + }, + } + response, _, err := client.Exchange(newRFC2136Update(t, "example.com."), listener.Addr().String()) + if err != nil { + t.Fatalf("dns exchange failed: %v", err) + } + + wantRcode := dns.RcodeNotImplemented + if allow { + wantRcode = dns.RcodeSuccess + } + if response.Rcode != wantRcode { + t.Fatalf("rcode = %s, want %s", dns.RcodeToString[response.Rcode], dns.RcodeToString[wantRcode]) + } + if handler.called.Load() != allow { + t.Fatalf("plugin called = %v, want %v", handler.called.Load(), allow) + } + }) + } +} + func TestServerSetsMaxTCPQueries(t *testing.T) { n := 128