Add idle connection timeouts to http clients and servers (#22885) (#2…

…2916) Sets `http.Server.IdleTimeout` and `http.Client.IdleConnTimeout` on clients and servers which didn't have them set. A default of 360s was chosen to be on par with the default of an NLB without being identical. This was added as another safety measure to prevent leaking any idle connections indefinitely as seen in #22757. `apidefaults.DefaultDialTimeout` was also renamed to `apidefaults.DefaultIOTimeout` to better reflect its usage.
gravitational · Mar 10, 2023 · c6d6c67 · c6d6c67
1 parent 9cfe32a
commit c6d6c67
Show file tree

Hide file tree

Showing 23 changed files with 40 additions and 30 deletions.
diff --git a/api/client/client.go b/api/client/client.go
@@ -522,7 +522,7 @@ func (c *Config) CheckAndSetDefaults() error {
 		c.KeepAliveCount = defaults.KeepAliveCountMax
 	}
 	if c.DialTimeout == 0 {
-		c.DialTimeout = defaults.DefaultDialTimeout
+		c.DialTimeout = defaults.DefaultIOTimeout
 	}
 	if c.CircuitBreakerConfig.Trip == nil || c.CircuitBreakerConfig.IsSuccessful == nil {
 		c.CircuitBreakerConfig = breaker.DefaultBreakerConfig(clockwork.NewRealClock())

diff --git a/api/client/webclient/webclient.go b/api/client/webclient/webclient.go
@@ -78,7 +78,7 @@ func (c *Config) CheckAndSetDefaults() error {
 		return trace.BadParameter(message, "missing parameter ProxyAddr")
 	}
 	if c.Timeout == 0 {
-		c.Timeout = defaults.DefaultDialTimeout
+		c.Timeout = defaults.DefaultIOTimeout
 	}
 	if c.TraceProvider == nil {
 		c.TraceProvider = tracing.DefaultProvider()
@@ -100,6 +100,7 @@ func newWebClient(cfg *Config) (*http.Client, error) {
 		Proxy: func(req *http.Request) (*url.URL, error) {
 			return httpproxy.FromEnvironment().ProxyFunc()(req.URL)
 		},
+		IdleConnTimeout: defaults.DefaultIOTimeout,
 	}, nil)
 
 	return &http.Client{

diff --git a/api/defaults/defaults.go b/api/defaults/defaults.go
@@ -28,9 +28,11 @@ const (
 	// Namespace is default namespace
 	Namespace = "default"
 
-	// DefaultDialTimeout is a default TCP dial timeout we set for our
-	// connection attempts
-	DefaultDialTimeout = 30 * time.Second
+	// DefaultIOTimeout is a default network IO timeout.
+	DefaultIOTimeout = 30 * time.Second
+
+	// DefaultIdleTimeout is a default idle connection timeout.
+	DefaultIdleTimeout = 360 * time.Second
 
 	// KeepAliveCountMax is the number of keep-alive messages that can be sent
 	// without receiving a response from the client before the client is

diff --git a/api/utils/sshutils/ssh.go b/api/utils/sshutils/ssh.go
@@ -109,7 +109,7 @@ func ProxyClientSSHConfig(sshCert *ssh.Certificate, priv crypto.Signer, sshCAs .
 
 	cfg := &ssh.ClientConfig{
 		Auth:    []ssh.AuthMethod{authMethod},
-		Timeout: defaults.DefaultDialTimeout,
+		Timeout: defaults.DefaultIOTimeout,
 	}
 
 	// The KeyId is not always a valid principal, so we use the first valid principal instead.

diff --git a/lib/auth/clt.go b/lib/auth/clt.go
@@ -162,7 +162,7 @@ func NewHTTPClient(cfg client.Config, tls *tls.Config, params ...roundtrip.Clien
 		// custom DialContext overrides this DNS name to the real address.
 		// In addition this dialer tries multiple addresses if provided
 		DialContext:           dialer.DialContext,
-		ResponseHeaderTimeout: apidefaults.DefaultDialTimeout,
+		ResponseHeaderTimeout: apidefaults.DefaultIOTimeout,
 		TLSClientConfig:       tls,
 
 		// Increase the size of the connection pool. This substantially improves the

diff --git a/lib/auth/init.go b/lib/auth/init.go
@@ -818,7 +818,7 @@ func (i *Identity) SSHClientConfig(fips bool) (*ssh.ClientConfig, error) {
 		User:            i.ID.HostUUID,
 		Auth:            []ssh.AuthMethod{ssh.PublicKeys(i.KeySigner)},
 		HostKeyCallback: callback,
-		Timeout:         apidefaults.DefaultDialTimeout,
+		Timeout:         apidefaults.DefaultIOTimeout,
 	}, nil
 }
 

diff --git a/lib/auth/middleware.go b/lib/auth/middleware.go
@@ -167,7 +167,8 @@ func NewTLSServer(cfg TLSServerConfig) (*TLSServer, error) {
 		cfg: cfg,
 		httpServer: &http.Server{
 			Handler:           httplib.MakeTracingHandler(limiter, teleport.ComponentAuth),
-			ReadHeaderTimeout: apidefaults.DefaultDialTimeout,
+			ReadHeaderTimeout: apidefaults.DefaultIOTimeout,
+			IdleTimeout:       apidefaults.DefaultIdleTimeout,
 		},
 		log: logrus.WithFields(logrus.Fields{
 			trace.Component: cfg.Component,

diff --git a/lib/backend/etcdbk/etcd.go b/lib/backend/etcdbk/etcd.go
@@ -325,7 +325,7 @@ func (cfg *Config) Validate() error {
 		cfg.BufferSize = backend.DefaultBufferCapacity
 	}
 	if cfg.DialTimeout == 0 {
-		cfg.DialTimeout = apidefaults.DefaultDialTimeout
+		cfg.DialTimeout = apidefaults.DefaultIOTimeout
 	}
 	if cfg.PasswordFile != "" {
 		out, err := os.ReadFile(cfg.PasswordFile)

diff --git a/lib/client/client.go b/lib/client/client.go
@@ -135,7 +135,7 @@ func (proxy *ProxyClient) GetSites(ctx context.Context) ([]types.Site, error) {
 	}()
 	select {
 	case <-done:
-	case <-time.After(apidefaults.DefaultDialTimeout):
+	case <-time.After(apidefaults.DefaultIOTimeout):
 		return nil, trace.ConnectionProblem(nil, "timeout")
 	}
 	log.Debugf("Found clusters: %v", stdout.String())

diff --git a/lib/client/conntest/connection_tester.go b/lib/client/conntest/connection_tester.go
@@ -84,7 +84,7 @@ func (r *TestConnectionRequest) CheckAndSetDefaults() error {
 	}
 
 	if r.DialTimeout <= 0 {
-		r.DialTimeout = defaults.DefaultDialTimeout
+		r.DialTimeout = defaults.DefaultIOTimeout
 	}
 
 	return nil

diff --git a/lib/events/filesessions/fileasync.go b/lib/events/filesessions/fileasync.go
@@ -528,7 +528,7 @@ func (u *Uploader) upload(ctx context.Context, up *upload) error {
 	// before the files are closed to avoid async writes
 	// the timeout is a defensive measure to avoid blocking
 	// indefinitely in case of unforeseen error (e.g. write taking too long)
-	wctx, wcancel := context.WithTimeout(ctx, apidefaults.DefaultDialTimeout)
+	wctx, wcancel := context.WithTimeout(ctx, apidefaults.DefaultIOTimeout)
 	defer wcancel()
 
 	<-wctx.Done()

diff --git a/lib/kube/proxy/server.go b/lib/kube/proxy/server.go
@@ -149,7 +149,8 @@ func NewTLSServer(cfg TLSServerConfig) (*TLSServer, error) {
 		TLSServerConfig: cfg,
 		Server: &http.Server{
 			Handler:           httplib.MakeTracingHandler(limiter, teleport.ComponentKube),
-			ReadHeaderTimeout: apidefaults.DefaultDialTimeout * 2,
+			ReadHeaderTimeout: apidefaults.DefaultIOTimeout * 2,
+			IdleTimeout:       apidefaults.DefaultIdleTimeout,
 			TLSConfig:         cfg.TLS,
 		},
 	}

diff --git a/lib/reversetunnel/agent_dialer.go b/lib/reversetunnel/agent_dialer.go
@@ -49,7 +49,7 @@ func (d *agentDialer) DialContext(ctx context.Context, addr utils.NetAddr) (SSHC
 	for _, authMethod := range d.authMethods {
 		// Create a dialer (that respects HTTP proxies) and connect to remote host.
 		dialer := proxy.DialerFromEnvironment(addr.Addr, d.options...)
-		pconn, err := dialer.DialTimeout(ctx, addr.AddrNetwork, addr.Addr, apidefaults.DefaultDialTimeout)
+		pconn, err := dialer.DialTimeout(ctx, addr.AddrNetwork, addr.Addr, apidefaults.DefaultIOTimeout)
 		if err != nil {
 			d.log.WithError(err).Debugf("Failed to dial %s.", addr.Addr)
 			continue
@@ -75,7 +75,7 @@ func (d *agentDialer) DialContext(ctx context.Context, addr utils.NetAddr) (SSHC
 			User:            d.username,
 			Auth:            []ssh.AuthMethod{authMethod},
 			HostKeyCallback: callback,
-			Timeout:         apidefaults.DefaultDialTimeout,
+			Timeout:         apidefaults.DefaultIOTimeout,
 		})
 		if err != nil {
 			d.log.WithError(err).Debugf("Failed to create client to %v.", addr.Addr)

diff --git a/lib/reversetunnel/agentpool.go b/lib/reversetunnel/agentpool.go
@@ -663,7 +663,7 @@ func (c *agentPoolRuntimeConfig) updateRemote(ctx context.Context, addr *utils.N
 
 	c.mu.RUnlock()
 
-	ctx, cancel := context.WithTimeout(ctx, defaults.DefaultDialTimeout)
+	ctx, cancel := context.WithTimeout(ctx, defaults.DefaultIOTimeout)
 	defer cancel()
 
 	tlsRoutingEnabled := false

diff --git a/lib/reversetunnel/localsite.go b/lib/reversetunnel/localsite.go
@@ -222,7 +222,7 @@ func (s *localSite) DialAuthServer() (net.Conn, error) {
 	}
 
 	addr := utils.ChooseRandomString(s.authServers)
-	conn, err := net.DialTimeout("tcp", addr, apidefaults.DefaultDialTimeout)
+	conn, err := net.DialTimeout("tcp", addr, apidefaults.DefaultIOTimeout)
 	if err != nil {
 		return nil, trace.ConnectionProblem(err, "unable to connect to auth server")
 	}
@@ -498,7 +498,7 @@ func (s *localSite) getConn(params DialParams) (conn net.Conn, useTunnel bool, e
 
 	// If no tunnel connection was found, dial to the target host.
 	dialer := proxyutils.DialerFromEnvironment(params.To.String())
-	conn, directErr = dialer.DialTimeout(s.srv.Context, params.To.Network(), params.To.String(), apidefaults.DefaultDialTimeout)
+	conn, directErr = dialer.DialTimeout(s.srv.Context, params.To.Network(), params.To.String(), apidefaults.DefaultIOTimeout)
 	if directErr != nil {
 		directMsg := getTunnelErrorMessage(params, "direct dial", directErr)
 		s.log.WithError(directErr).WithField("address", params.To.String()).Debug("Error occurred while dialing directly.")

diff --git a/lib/reversetunnel/transport.go b/lib/reversetunnel/transport.go
@@ -185,7 +185,7 @@ func (p *transport) start() {
 		if req == nil {
 			return
 		}
-	case <-time.After(apidefaults.DefaultDialTimeout):
+	case <-time.After(apidefaults.DefaultIOTimeout):
 		p.log.Warnf("Transport request failed: timed out waiting for request.")
 		return
 	}
@@ -440,7 +440,7 @@ func (p *transport) directDial(addr string) (net.Conn, error) {
 	}
 
 	d := net.Dialer{
-		Timeout: apidefaults.DefaultDialTimeout,
+		Timeout: apidefaults.DefaultIOTimeout,
 	}
 	conn, err := d.DialContext(p.closeContext, "tcp", addr)
 	if err != nil {

diff --git a/lib/service/service.go b/lib/service/service.go
@@ -2666,6 +2666,7 @@ func (process *TeleportProcess) initMetricsService() error {
 	server := &http.Server{
 		Handler:           mux,
 		ReadHeaderTimeout: defaults.ReadHeadersTimeout,
+		IdleTimeout:       apidefaults.DefaultIdleTimeout,
 		ErrorLog:          utils.NewStdlogger(log.Error, teleport.ComponentMetrics),
 		TLSConfig:         tlsConfig,
 	}
@@ -2785,7 +2786,8 @@ func (process *TeleportProcess) initDiagnosticService() error {
 
 	server := &http.Server{
 		Handler:           mux,
-		ReadHeaderTimeout: apidefaults.DefaultDialTimeout,
+		ReadHeaderTimeout: apidefaults.DefaultIOTimeout,
+		IdleTimeout:       apidefaults.DefaultIdleTimeout,
 		ErrorLog:          utils.NewStdlogger(log.Error, teleport.ComponentDiagnostic),
 	}
 
@@ -3627,7 +3629,8 @@ func (process *TeleportProcess) initProxyEndpoint(conn *Connector) error {
 
 		webServer = &http.Server{
 			Handler:           httplib.MakeTracingHandler(proxyLimiter, teleport.ComponentProxy),
-			ReadHeaderTimeout: apidefaults.DefaultDialTimeout,
+			ReadHeaderTimeout: apidefaults.DefaultIOTimeout,
+			IdleTimeout:       apidefaults.DefaultIdleTimeout,
 			ErrorLog:          utils.NewStdlogger(log.Error, teleport.ComponentProxy),
 		}
 		process.RegisterCriticalFunc("proxy.web", func() error {
@@ -4124,7 +4127,8 @@ func (process *TeleportProcess) initMinimalReverseTunnel(listeners *proxyListene
 
 	minimalWebServer = &http.Server{
 		Handler:           httplib.MakeTracingHandler(minimalProxyLimiter, teleport.ComponentProxy),
-		ReadHeaderTimeout: apidefaults.DefaultDialTimeout,
+		ReadHeaderTimeout: apidefaults.DefaultIOTimeout,
+		IdleTimeout:       apidefaults.DefaultIdleTimeout,
 		ErrorLog:          utils.NewStdlogger(log.Error, teleport.ComponentReverseTunnelServer),
 	}
 	process.RegisterCriticalFunc("proxy.reversetunnel.web", func() error {

diff --git a/lib/srv/alpnproxy/auth/auth_proxy.go b/lib/srv/alpnproxy/auth/auth_proxy.go
@@ -103,7 +103,7 @@ func (s *AuthProxyDialerService) dialLocalAuthServer(ctx context.Context) (net.C
 
 	addr := utils.ChooseRandomString(s.authServers)
 	d := &net.Dialer{
-		Timeout: defaults.DefaultDialTimeout,
+		Timeout: defaults.DefaultIOTimeout,
 	}
 	conn, err := d.DialContext(ctx, "tcp", addr)
 	if err != nil {

diff --git a/lib/srv/alpnproxy/conn_upgrade.go b/lib/srv/alpnproxy/conn_upgrade.go
@@ -47,7 +47,7 @@ import (
 // preserve the ALPN and SNI information.
 func IsALPNConnUpgradeRequired(addr string, insecure bool) bool {
 	netDialer := &net.Dialer{
-		Timeout: defaults.DefaultDialTimeout,
+		Timeout: defaults.DefaultIOTimeout,
 	}
 	tlsConfig := &tls.Config{
 		NextProtos:         []string{string(common.ProtocolReverseTunnel)},

diff --git a/lib/srv/app/server.go b/lib/srv/app/server.go
@@ -1011,7 +1011,8 @@ func (s *Server) newHTTPServer() *http.Server {
 
 	return &http.Server{
 		Handler:           httplib.MakeTracingHandler(s.authMiddleware, teleport.ComponentApp),
-		ReadHeaderTimeout: apidefaults.DefaultDialTimeout,
+		ReadHeaderTimeout: apidefaults.DefaultIOTimeout,
+		IdleTimeout:       apidefaults.DefaultIdleTimeout,
 		ErrorLog:          utils.NewStdlogger(s.log.Error, teleport.ComponentApp),
 		ConnContext: func(ctx context.Context, c net.Conn) context.Context {
 			return context.WithValue(ctx, connContextKey, c)

diff --git a/lib/srv/app/tcpserver.go b/lib/srv/app/tcpserver.go
@@ -48,7 +48,7 @@ func (s *tcpServer) handleConnection(ctx context.Context, clientConn net.Conn, i
 		return trace.BadParameter(`unexpected app %q address network, expected "tcp": %+v`, app.GetName(), addr)
 	}
 	dialer := net.Dialer{
-		Timeout: apidefaults.DefaultDialTimeout,
+		Timeout: apidefaults.DefaultIOTimeout,
 	}
 	serverConn, err := dialer.DialContext(ctx, addr.AddrNetwork, addr.String())
 	if err != nil {

diff --git a/lib/srv/forward/sshserver.go b/lib/srv/forward/sshserver.go
@@ -633,7 +633,7 @@ func (s *Server) newRemoteClient(ctx context.Context, systemLogin string) (*trac
 			authMethod,
 		},
 		HostKeyCallback: s.authHandlers.HostKeyAuth,
-		Timeout:         apidefaults.DefaultDialTimeout,
+		Timeout:         apidefaults.DefaultIOTimeout,
 	}
 
 	// Ciphers, KEX, and MACs preferences are honored by both the in-memory

diff --git a/lib/tbot/identity/identity.go b/lib/tbot/identity/identity.go
@@ -243,7 +243,7 @@ func (i *Identity) SSHClientConfig() (*ssh.ClientConfig, error) {
 		User:            i.SSHCert.ValidPrincipals[0],
 		Auth:            []ssh.AuthMethod{ssh.PublicKeys(i.KeySigner)},
 		HostKeyCallback: callback,
-		Timeout:         apidefaults.DefaultDialTimeout,
+		Timeout:         apidefaults.DefaultIOTimeout,
 	}, nil
 }