From 997ec12ec837160e28984eb22ab8eea438b32703 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 22 Sep 2022 12:13:28 +0200 Subject: [PATCH] set ReadHeaderTimeout to address G112: Potential Slowloris Attack (gosec) After discussing in the maintainers meeting, we concluded that Slowloris attacks are not a real risk other than potentially having some additional goroutines lingering around, so setting a long timeout to satisfy the linter, and to at least have "some" timeout. libnetwork/diagnostic/server.go:96:10: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec) srv := &http.Server{ Addr: net.JoinHostPort(ip, strconv.Itoa(port)), Handler: s, } api/server/server.go:60:10: G112: Potential Slowloris Attack because ReadHeaderTimeout is not configured in the http.Server (gosec) srv: &http.Server{ Addr: addr, }, daemon/metrics_unix.go:34:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec) if err := http.Serve(l, mux); err != nil && !strings.Contains(err.Error(), "use of closed network connection") { ^ cmd/dockerd/metrics.go:27:13: G114: Use of net/http serve function that has no support for setting timeouts (gosec) if err := http.Serve(l, mux); err != nil && !strings.Contains(err.Error(), "use of closed network connection") { ^ Signed-off-by: Sebastiaan van Stijn (cherry picked from commit 55fd77f7246a8113b81eb0de5644ce651db1ace0) Signed-off-by: Sebastiaan van Stijn --- api/server/server.go | 4 +++- cmd/dockerd/metrics.go | 7 ++++++- daemon/metrics_unix.go | 7 ++++++- libnetwork/diagnostic/server.go | 6 ++++-- 4 files changed, 19 insertions(+), 5 deletions(-) diff --git a/api/server/server.go b/api/server/server.go index 5f01a8b1dd..e8714eb060 100644 --- a/api/server/server.go +++ b/api/server/server.go @@ -6,6 +6,7 @@ import ( "net" "net/http" "strings" + "time" "github.com/docker/docker/api/server/httpstatus" "github.com/docker/docker/api/server/httputils" @@ -58,7 +59,8 @@ func (s *Server) Accept(addr string, listeners ...net.Listener) { for _, listener := range listeners { httpServer := &HTTPServer{ srv: &http.Server{ - Addr: addr, + Addr: addr, + ReadHeaderTimeout: 5 * time.Minute, // "G112: Potential Slowloris Attack (gosec)"; not a real concern for our use, so setting a long timeout. }, l: listener, } diff --git a/cmd/dockerd/metrics.go b/cmd/dockerd/metrics.go index 4ea8321b5d..a13a5d2670 100644 --- a/cmd/dockerd/metrics.go +++ b/cmd/dockerd/metrics.go @@ -4,6 +4,7 @@ import ( "net" "net/http" "strings" + "time" metrics "github.com/docker/go-metrics" "github.com/sirupsen/logrus" @@ -24,7 +25,11 @@ func startMetricsServer(addr string) error { mux.Handle("/metrics", metrics.Handler()) go func() { logrus.Infof("metrics API listening on %s", l.Addr()) - if err := http.Serve(l, mux); err != nil && !strings.Contains(err.Error(), "use of closed network connection") { + srv := &http.Server{ + Handler: mux, + ReadHeaderTimeout: 5 * time.Minute, // "G112: Potential Slowloris Attack (gosec)"; not a real concern for our use, so setting a long timeout. + } + if err := srv.Serve(l); err != nil && !strings.Contains(err.Error(), "use of closed network connection") { logrus.WithError(err).Error("error serving metrics API") } }() diff --git a/daemon/metrics_unix.go b/daemon/metrics_unix.go index 7869712541..6acc469c9c 100644 --- a/daemon/metrics_unix.go +++ b/daemon/metrics_unix.go @@ -8,6 +8,7 @@ import ( "net/http" "path/filepath" "strings" + "time" "github.com/docker/docker/pkg/plugingetter" "github.com/docker/docker/pkg/plugins" @@ -31,7 +32,11 @@ func (daemon *Daemon) listenMetricsSock() (string, error) { mux.Handle("/metrics", metrics.Handler()) go func() { logrus.Debugf("metrics API listening on %s", l.Addr()) - if err := http.Serve(l, mux); err != nil && !strings.Contains(err.Error(), "use of closed network connection") { + srv := &http.Server{ + Handler: mux, + ReadHeaderTimeout: 5 * time.Minute, // "G112: Potential Slowloris Attack (gosec)"; not a real concern for our use, so setting a long timeout. + } + if err := srv.Serve(l); err != nil && !strings.Contains(err.Error(), "use of closed network connection") { logrus.WithError(err).Error("error serving metrics API") } }() diff --git a/libnetwork/diagnostic/server.go b/libnetwork/diagnostic/server.go index a919631493..4af9bcc679 100644 --- a/libnetwork/diagnostic/server.go +++ b/libnetwork/diagnostic/server.go @@ -9,6 +9,7 @@ import ( "strconv" "sync" "sync/atomic" + "time" "github.com/docker/docker/libnetwork/internal/caller" "github.com/docker/docker/pkg/stack" @@ -94,8 +95,9 @@ func (s *Server) EnableDiagnostic(ip string, port int) { logrus.Infof("Starting the diagnostic server listening on %d for commands", port) srv := &http.Server{ - Addr: net.JoinHostPort(ip, strconv.Itoa(port)), - Handler: s, + Addr: net.JoinHostPort(ip, strconv.Itoa(port)), + Handler: s, + ReadHeaderTimeout: 5 * time.Minute, // "G112: Potential Slowloris Attack (gosec)"; not a real concern for our use, so setting a long timeout. } s.srv = srv s.enable = 1