diff --git a/.gitignore b/.gitignore index 67780a471..5a6dd1245 100644 --- a/.gitignore +++ b/.gitignore @@ -1,6 +1,7 @@ # only add build artifacts concerning coredns - no editor related files coredns coredns.exe +Corefile build/ release/ vendor/ diff --git a/core/dnsserver/config.go b/core/dnsserver/config.go index 3da86271e..9e1116650 100644 --- a/core/dnsserver/config.go +++ b/core/dnsserver/config.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "fmt" "net/http" + "time" "github.com/coredns/caddy" "github.com/coredns/coredns/plugin" @@ -53,6 +54,11 @@ type Config struct { // TLSConfig when listening for encrypted connections (gRPC, DNS-over-TLS). TLSConfig *tls.Config + // Timeouts for TCP, TLS and HTTPS servers. + ReadTimeout time.Duration + WriteTimeout time.Duration + IdleTimeout time.Duration + // TSIG secrets, [name]key. TsigSecret map[string]string diff --git a/core/dnsserver/register.go b/core/dnsserver/register.go index 176be49b8..2b673cab3 100644 --- a/core/dnsserver/register.go +++ b/core/dnsserver/register.go @@ -150,6 +150,9 @@ func (h *dnsContext) MakeServers() ([]caddy.Server, error) { // Fork TLSConfig for each encrypted connection c.TLSConfig = c.firstConfigInBlock.TLSConfig.Clone() + c.ReadTimeout = c.firstConfigInBlock.ReadTimeout + c.WriteTimeout = c.firstConfigInBlock.WriteTimeout + c.IdleTimeout = c.firstConfigInBlock.IdleTimeout c.TsigSecret = c.firstConfigInBlock.TsigSecret } @@ -223,7 +226,8 @@ func (c *Config) AddPlugin(m plugin.Plugin) { } // registerHandler adds a handler to a site's handler registration. Handlers -// use this to announce that they exist to other plugin. +// +// use this to announce that they exist to other plugin. func (c *Config) registerHandler(h plugin.Handler) { if c.registry == nil { c.registry = make(map[string]plugin.Handler) diff --git a/core/dnsserver/server.go b/core/dnsserver/server.go index 478287bf8..2107e8d01 100644 --- a/core/dnsserver/server.go +++ b/core/dnsserver/server.go @@ -44,6 +44,9 @@ type Server struct { debug bool // disable recover() stacktrace bool // enable stacktrace in recover error log classChaos bool // allow non-INET class queries + idleTimeout time.Duration // Idle timeout for TCP + readTimeout time.Duration // Read timeout for TCP + writeTimeout time.Duration // Write timeout for TCP tsigSecret map[string]string } @@ -60,6 +63,9 @@ func NewServer(addr string, group []*Config) (*Server, error) { Addr: addr, zones: make(map[string][]*Config), graceTimeout: 5 * time.Second, + idleTimeout: 10 * time.Second, + readTimeout: 3 * time.Second, + writeTimeout: 5 * time.Second, tsigSecret: make(map[string]string), } @@ -81,6 +87,17 @@ func NewServer(addr string, group []*Config) (*Server, error) { // append the config to the zone's configs s.zones[site.Zone] = append(s.zones[site.Zone], site) + // set timeouts + if site.ReadTimeout != 0 { + s.readTimeout = site.ReadTimeout + } + if site.WriteTimeout != 0 { + s.writeTimeout = site.WriteTimeout + } + if site.IdleTimeout != 0 { + s.idleTimeout = site.IdleTimeout + } + // copy tsig secrets for key, secret := range site.TsigSecret { s.tsigSecret[key] = secret @@ -130,11 +147,22 @@ var _ caddy.GracefulServer = &Server{} // This implements caddy.TCPServer interface. func (s *Server) Serve(l net.Listener) error { s.m.Lock() - s.server[tcp] = &dns.Server{Listener: l, Net: "tcp", Handler: dns.HandlerFunc(func(w dns.ResponseWriter, r *dns.Msg) { - ctx := context.WithValue(context.Background(), Key{}, s) - ctx = context.WithValue(ctx, LoopKey{}, 0) - s.ServeDNS(ctx, w, r) - }), TsigSecret: s.tsigSecret} + + s.server[tcp] = &dns.Server{Listener: l, + Net: "tcp", + TsigSecret: s.tsigSecret, + MaxTCPQueries: tcpMaxQueries, + ReadTimeout: s.readTimeout, + WriteTimeout: s.writeTimeout, + IdleTimeout: func() time.Duration { + return s.idleTimeout + }, + Handler: dns.HandlerFunc(func(w dns.ResponseWriter, r *dns.Msg) { + ctx := context.WithValue(context.Background(), Key{}, s) + ctx = context.WithValue(ctx, LoopKey{}, 0) + s.ServeDNS(ctx, w, r) + })} + s.m.Unlock() return s.server[tcp].ActivateAndServe() @@ -404,6 +432,8 @@ func errorAndMetricsFunc(server string, w dns.ResponseWriter, r *dns.Msg, rc int const ( tcp = 0 udp = 1 + + tcpMaxQueries = -1 ) type ( diff --git a/core/dnsserver/server_https.go b/core/dnsserver/server_https.go index eda39c140..cddf59890 100644 --- a/core/dnsserver/server_https.go +++ b/core/dnsserver/server_https.go @@ -75,9 +75,9 @@ func NewServerHTTPS(addr string, group []*Config) (*ServerHTTPS, error) { } srv := &http.Server{ - ReadTimeout: 5 * time.Second, - WriteTimeout: 10 * time.Second, - IdleTimeout: 120 * time.Second, + ReadTimeout: s.readTimeout, + WriteTimeout: s.writeTimeout, + IdleTimeout: s.idleTimeout, ErrorLog: stdlog.New(&loggerAdapter{}, "", 0), } sh := &ServerHTTPS{ diff --git a/core/dnsserver/server_tls.go b/core/dnsserver/server_tls.go index 6fff61d5c..f2251efb4 100644 --- a/core/dnsserver/server_tls.go +++ b/core/dnsserver/server_tls.go @@ -5,6 +5,7 @@ import ( "crypto/tls" "fmt" "net" + "time" "github.com/coredns/caddy" "github.com/coredns/coredns/plugin/pkg/reuseport" @@ -50,11 +51,20 @@ func (s *ServerTLS) Serve(l net.Listener) error { } // Only fill out the TCP server for this one. - s.server[tcp] = &dns.Server{Listener: l, Net: "tcp-tls", Handler: dns.HandlerFunc(func(w dns.ResponseWriter, r *dns.Msg) { - ctx := context.WithValue(context.Background(), Key{}, s.Server) - ctx = context.WithValue(ctx, LoopKey{}, 0) - s.ServeDNS(ctx, w, r) - })} + s.server[tcp] = &dns.Server{Listener: l, + Net: "tcp-tls", + MaxTCPQueries: tlsMaxQueries, + ReadTimeout: s.readTimeout, + WriteTimeout: s.writeTimeout, + IdleTimeout: func() time.Duration { + return s.idleTimeout + }, + Handler: dns.HandlerFunc(func(w dns.ResponseWriter, r *dns.Msg) { + ctx := context.WithValue(context.Background(), Key{}, s.Server) + ctx = context.WithValue(ctx, LoopKey{}, 0) + s.ServeDNS(ctx, w, r) + })} + s.m.Unlock() return s.server[tcp].ActivateAndServe() @@ -87,3 +97,7 @@ func (s *ServerTLS) OnStartupComplete() { fmt.Print(out) } } + +const ( + tlsMaxQueries = -1 +) diff --git a/core/dnsserver/zdirectives.go b/core/dnsserver/zdirectives.go index 38425fb06..6d7137580 100644 --- a/core/dnsserver/zdirectives.go +++ b/core/dnsserver/zdirectives.go @@ -14,6 +14,7 @@ var Directives = []string{ "geoip", "cancel", "tls", + "timeouts", "reload", "nsid", "bufsize", diff --git a/core/plugin/zplugin.go b/core/plugin/zplugin.go index 08003f460..b97cd85c5 100644 --- a/core/plugin/zplugin.go +++ b/core/plugin/zplugin.go @@ -49,6 +49,7 @@ import ( _ "github.com/coredns/coredns/plugin/secondary" _ "github.com/coredns/coredns/plugin/sign" _ "github.com/coredns/coredns/plugin/template" + _ "github.com/coredns/coredns/plugin/timeouts" _ "github.com/coredns/coredns/plugin/tls" _ "github.com/coredns/coredns/plugin/trace" _ "github.com/coredns/coredns/plugin/transfer" diff --git a/plugin.cfg b/plugin.cfg index 5632a3b97..407a668eb 100644 --- a/plugin.cfg +++ b/plugin.cfg @@ -23,6 +23,7 @@ metadata:metadata geoip:geoip cancel:cancel tls:tls +timeouts:timeouts reload:reload nsid:nsid bufsize:bufsize diff --git a/plugin/pkg/durations/durations.go b/plugin/pkg/durations/durations.go new file mode 100644 index 000000000..37771e79d --- /dev/null +++ b/plugin/pkg/durations/durations.go @@ -0,0 +1,26 @@ +package durations + +import ( + "fmt" + "strconv" + "time" +) + +// NewDurationFromArg returns a time.Duration from a configuration argument +// (string) which has come from the Corefile. The argument has some basic +// validation applied before returning a time.Duration. If the argument has no +// time unit specified and is numeric the argument will be treated as seconds +// rather than GO's default of nanoseconds. +func NewDurationFromArg(arg string) (time.Duration, error) { + _, err := strconv.Atoi(arg) + if err == nil { + arg = arg + "s" + } + + d, err := time.ParseDuration(arg) + if err != nil { + return 0, fmt.Errorf("failed to parse duration '%s'", arg) + } + + return d, nil +} diff --git a/plugin/pkg/durations/durations_test.go b/plugin/pkg/durations/durations_test.go new file mode 100644 index 000000000..12008a713 --- /dev/null +++ b/plugin/pkg/durations/durations_test.go @@ -0,0 +1,51 @@ +package durations + +import ( + "testing" + "time" +) + +func TestNewDurationFromArg(t *testing.T) { + tests := []struct { + name string + arg string + wantErr bool + want time.Duration + }{ + { + name: "valid GO duration - seconds", + arg: "30s", + want: 30 * time.Second, + }, + { + name: "valid GO duration - minutes", + arg: "2m", + want: 2 * time.Minute, + }, + { + name: "number - fallback to seconds", + arg: "30", + want: 30 * time.Second, + }, + { + name: "invalid duration", + arg: "twenty seconds", + wantErr: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + actual, err := NewDurationFromArg(test.arg) + if test.wantErr && err == nil { + t.Error("error was expected") + } + if !test.wantErr && err != nil { + t.Error("error was not expected") + } + + if test.want != actual { + t.Errorf("expected '%v' got '%v'", test.want, actual) + } + }) + } +} diff --git a/plugin/timeouts/README.md b/plugin/timeouts/README.md new file mode 100644 index 000000000..098c9ccac --- /dev/null +++ b/plugin/timeouts/README.md @@ -0,0 +1,76 @@ +# timeouts + +## Name + +*timeouts* - allows you to configure the server read, write and idle timeouts for the TCP, TLS and DoH servers. + +## Description + +CoreDNS is configured with sensible timeouts for server connections by default. +However in some cases for example where CoreDNS is serving over a slow mobile +data connection the default timeouts are not optimal. + +Additionally some routers hold open connections when using DNS over TLS or DNS +over HTTPS. Allowing a longer idle timeout helps performance and reduces issues +with such routers. + +The *timeouts* "plugin" allows you to configure CoreDNS server read, write and +idle timeouts. + +## Syntax + +~~~ txt +timeouts { + read DURATION + write DURATION + idle DURATION +} +~~~ + +For any timeouts that are not provided, default values are used which may vary +depending on the server type. At least one timeout must be specified otherwise +the entire timeouts block should be omitted. + +## Examples + +Start a DNS-over-TLS server that picks up incoming DNS-over-TLS queries on port +5553 and uses the nameservers defined in `/etc/resolv.conf` to resolve the +query. This proxy path uses plain old DNS. A 10 second read timeout, 20 +second write timeout and a 60 second idle timeout have been configured. + +~~~ +tls://.:5553 { + tls cert.pem key.pem ca.pem + timeouts { + read 10s + write 20s + idle 60s + } + forward . /etc/resolv.conf +} +~~~ + +Start a DNS-over-HTTPS server that is similar to the previous example. Only the +read timeout has been configured for 1 minute. + +~~~ +https://. { + tls cert.pem key.pem ca.pem + timeouts { + read 1m + } + forward . /etc/resolv.conf +} +~~~ + +Start a standard TCP/UDP server on port 1053. A read and write timeout has been +configured. The timeouts are only applied to the TCP side of the server. +~~~ +.:1053 { + timeouts { + read 15s + write 30s + } + forward . /etc/resolv.conf +} +~~~ diff --git a/plugin/timeouts/timeouts.go b/plugin/timeouts/timeouts.go new file mode 100644 index 000000000..eea6a6488 --- /dev/null +++ b/plugin/timeouts/timeouts.go @@ -0,0 +1,69 @@ +package timeouts + +import ( + "time" + + "github.com/coredns/caddy" + "github.com/coredns/coredns/core/dnsserver" + "github.com/coredns/coredns/plugin" + "github.com/coredns/coredns/plugin/pkg/durations" +) + +func init() { plugin.Register("timeouts", setup) } + +func setup(c *caddy.Controller) error { + err := parseTimeouts(c) + if err != nil { + return plugin.Error("timeouts", err) + } + return nil +} + +func parseTimeouts(c *caddy.Controller) error { + config := dnsserver.GetConfig(c) + + for c.Next() { + args := c.RemainingArgs() + if len(args) > 0 { + return plugin.Error("timeouts", c.ArgErr()) + } + + b := 0 + for c.NextBlock() { + block := c.Val() + timeoutArgs := c.RemainingArgs() + if len(timeoutArgs) != 1 { + return c.ArgErr() + } + + timeout, err := durations.NewDurationFromArg(timeoutArgs[0]) + if err != nil { + return c.Err(err.Error()) + } + + if timeout < (1*time.Second) || timeout > (24*time.Hour) { + return c.Errf("timeout provided '%s' needs to be between 1 second and 24 hours", timeout) + } + + switch block { + case "read": + config.ReadTimeout = timeout + + case "write": + config.WriteTimeout = timeout + + case "idle": + config.IdleTimeout = timeout + + default: + return c.Errf("unknown option: '%s'", block) + } + b++ + } + + if b == 0 { + return plugin.Error("timeouts", c.Err("timeouts block with no timeouts specified")) + } + } + return nil +} diff --git a/plugin/timeouts/timeouts_test.go b/plugin/timeouts/timeouts_test.go new file mode 100644 index 000000000..c01d3a072 --- /dev/null +++ b/plugin/timeouts/timeouts_test.go @@ -0,0 +1,75 @@ +package timeouts + +import ( + "strings" + "testing" + + "github.com/coredns/caddy" +) + +func TestTimeouts(t *testing.T) { + tests := []struct { + input string + shouldErr bool + expectedRoot string // expected root, set to the controller. Empty for negative cases. + expectedErrContent string // substring from the expected error. Empty for positive cases. + }{ + // positive + {`timeouts { + read 30s + }`, false, "", ""}, + {`timeouts { + read 1m + write 2m + }`, false, "", ""}, + {` timeouts { + idle 1h + }`, false, "", ""}, + {`timeouts { + read 10 + write 20 + idle 60 + }`, false, "", ""}, + // negative + {`timeouts`, true, "", "block with no timeouts specified"}, + {`timeouts { + }`, true, "", "block with no timeouts specified"}, + {`timeouts { + read 10s + giraffe 30s + }`, true, "", "unknown option"}, + {`timeouts { + read 10s 20s + write 30s + }`, true, "", "Wrong argument"}, + {`timeouts { + write snake + }`, true, "", "failed to parse duration"}, + {`timeouts { + idle 0s + }`, true, "", "needs to be between"}, + {`timeouts { + read 48h + }`, true, "", "needs to be between"}, + } + + for i, test := range tests { + c := caddy.NewTestController("dns", test.input) + err := setup(c) + //cfg := dnsserver.GetConfig(c) + + if test.shouldErr && err == nil { + t.Errorf("Test %d: Expected error but found %s for input %s", i, err, test.input) + } + + if err != nil { + if !test.shouldErr { + t.Errorf("Test %d: Expected no error but found one for input %s. Error was: %v", i, test.input, err) + } + + if !strings.Contains(err.Error(), test.expectedErrContent) { + t.Errorf("Test %d: Expected error to contain: %v, found error: %v, input: %s", i, test.expectedErrContent, err, test.input) + } + } + } +}