UPSTREAM: 121120: Prevent rapid reset http2 DOS on API server

This change fully addresses CVE-2023-44487 and CVE-2023-39325 for
the API server when combined with
golang/net@b225e7c

The changes to util/runtime are required because otherwise a large
number of requests can get blocked on the time.Sleep calls.

For unauthenticated clients (either via 401 or the anonymous user),
we simply no longer allow such clients to hold open http2
connections.  They can use http2, but with the performance of http1
(or possibly slightly worse).

For all other clients, we detect if the request ended via a timeout
before the context's deadline.  This likely means that the client
reset the http2 stream early.  We close the connection in this case
as well.  To mitigate issues related to clients creating more
streams than the configured max, we rely on the golang.org/x/net fix
noted above.  The Kube API server now uses a max stream of 100
instead of 250 (this matches the Go http2 client default).  This
lowers the abuse limit from 1000 to 400.

Signed-off-by: Monis Khan <[email protected]>
Signed-off-by: Andy Goldstein <[email protected]>

Author: enj

staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go

@@ -126,14 +126,17 @@ type rudimentaryErrorBackoff struct {
 // OnError will block if it is called more often than the embedded period time.
 // This will prevent overly tight hot error loops.
 func (r *rudimentaryErrorBackoff) OnError(error) {
+	now := time.Now() // start the timer before acquiring the lock
 	r.lastErrorTimeLock.Lock()
-	defer r.lastErrorTimeLock.Unlock()
-	d := time.Since(r.lastErrorTime)
-	if d < r.minPeriod {
-		// If the time moves backwards for any reason, do nothing
-		time.Sleep(r.minPeriod - d)
-	}
+	d := now.Sub(r.lastErrorTime)
 	r.lastErrorTime = time.Now()
+	r.lastErrorTimeLock.Unlock()
+
+	// Do not sleep with the lock held because that causes all callers of HandleError to block.
+	// We only want the current goroutine to block.
+	// A negative or zero duration causes time.Sleep to return immediately.
+	// If the time moves backwards for any reason, do nothing.
+	time.Sleep(r.minPeriod - d)
 }
 
 // GetCaller returns the caller of the function that calls it.

staging/src/k8s.io/apiserver/pkg/endpoints/filters/authentication.go

@@ -29,6 +29,7 @@ import (
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/authenticatorfactory"
 	"k8s.io/apiserver/pkg/authentication/request/headerrequest"
+	"k8s.io/apiserver/pkg/authentication/user"
 	"k8s.io/apiserver/pkg/endpoints/handlers/responsewriters"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
 	"k8s.io/klog/v2"
@@ -101,13 +102,28 @@ func withAuthentication(handler http.Handler, auth authenticator.Request, failed
 			)
 		}
 
+		// http2 is an expensive protocol that is prone to abuse,
+		// see CVE-2023-44487 and CVE-2023-39325 for an example.
+		// Do not allow unauthenticated clients to keep these
+		// connections open (i.e. basically degrade them to http1).
+		if req.ProtoMajor == 2 && isAnonymousUser(resp.User) {
+			w.Header().Set("Connection", "close")
+		}
+
 		req = req.WithContext(genericapirequest.WithUser(req.Context(), resp.User))
 		handler.ServeHTTP(w, req)
 	})
 }
 
 func Unauthorized(s runtime.NegotiatedSerializer) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) {
+		// http2 is an expensive protocol that is prone to abuse,
+		// see CVE-2023-44487 and CVE-2023-39325 for an example.
+		// Do not allow unauthenticated clients to keep these
+		// connections open (i.e. basically degrade them to http1).
+		if req.ProtoMajor == 2 {
+			w.Header().Set("Connection", "close")
+		}
 		ctx := req.Context()
 		requestInfo, found := genericapirequest.RequestInfoFrom(ctx)
 		if !found {
@@ -127,3 +143,15 @@ func audiencesAreAcceptable(apiAuds, responseAudiences authenticator.Audiences)
 
 	return len(apiAuds.Intersect(responseAudiences)) > 0
 }
+
+func isAnonymousUser(u user.Info) bool {
+	if u.GetName() == user.Anonymous {
+		return true
+	}
+	for _, group := range u.GetGroups() {
+		if group == user.AllUnauthenticated {
+			return true
+		}
+	}
+	return false
+}

staging/src/k8s.io/apiserver/pkg/server/filters/goaway.go

@@ -64,7 +64,7 @@ type goaway struct {
 
 // ServeHTTP implement HTTP handler
 func (h *goaway) ServeHTTP(w http.ResponseWriter, r *http.Request) {
-	if r.Proto == "HTTP/2.0" && h.decider.Goaway(r) {
+	if r.ProtoMajor == 2 && h.decider.Goaway(r) {
 		// Send a GOAWAY and tear down the TCP connection when idle.
 		w.Header().Set("Connection", "close")
 	}

staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go

@@ -146,13 +146,13 @@ func (t *timeoutHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 			return false
 		})
 		defer postTimeoutFn()
-		tw.timeout(err)
+		tw.timeout(r, err)
 	}
 }
 
 type timeoutWriter interface {
 	http.ResponseWriter
-	timeout(*apierrors.StatusError)
+	timeout(*http.Request, *apierrors.StatusError)
 }
 
 func newTimeoutWriter(w http.ResponseWriter) (timeoutWriter, http.ResponseWriter) {
@@ -245,7 +245,7 @@ func copyHeaders(dst, src http.Header) {
 	}
 }
 
-func (tw *baseTimeoutWriter) timeout(err *apierrors.StatusError) {
+func (tw *baseTimeoutWriter) timeout(r *http.Request, err *apierrors.StatusError) {
 	tw.mu.Lock()
 	defer tw.mu.Unlock()
 
@@ -255,6 +255,14 @@ func (tw *baseTimeoutWriter) timeout(err *apierrors.StatusError) {
 	// We can safely timeout the HTTP request by sending by a timeout
 	// handler
 	if !tw.wroteHeader && !tw.hijacked {
+		// http2 is an expensive protocol that is prone to abuse,
+		// see CVE-2023-44487 and CVE-2023-39325 for an example.
+		// Do not allow clients to reset these connections
+		// prematurely as that can trivially OOM the api server
+		// (i.e. basically degrade them to http1).
+		if isLikelyEarlyHTTP2Reset(r) {
+			tw.w.Header().Set("Connection", "close")
+		}
 		tw.w.WriteHeader(http.StatusGatewayTimeout)
 		enc := json.NewEncoder(tw.w)
 		enc.Encode(&err.ErrStatus)
@@ -277,6 +285,24 @@ func (tw *baseTimeoutWriter) timeout(err *apierrors.StatusError) {
 	}
 }
 
+// isLikelyEarlyHTTP2Reset returns true if an http2 stream was reset before the request deadline.
+// Note that this does not prevent a client from trying to create more streams than the configured
+// max, but https://github.com/golang/net/commit/b225e7ca6dde1ef5a5ae5ce922861bda011cfabd protects
+// us from abuse via that vector.
+func isLikelyEarlyHTTP2Reset(r *http.Request) bool {
+	if r.ProtoMajor != 2 {
+		return false
+	}
+
+	deadline, ok := r.Context().Deadline()
+	if !ok {
+		return true // this context had no deadline but was canceled meaning the client likely reset it early
+	}
+
+	// this context was canceled before its deadline meaning the client likely reset it early
+	return time.Now().Before(deadline)
+}
+
 func (tw *baseTimeoutWriter) CloseNotify() <-chan bool {
 	tw.mu.Lock()
 	defer tw.mu.Unlock()

staging/src/k8s.io/apiserver/pkg/server/secure_serving.go

@@ -189,7 +189,10 @@ func (s *SecureServingInfo) Serve(handler http.Handler, shutdownTimeout time.Dur
 	if s.HTTP2MaxStreamsPerConnection > 0 {
 		http2Options.MaxConcurrentStreams = uint32(s.HTTP2MaxStreamsPerConnection)
 	} else {
-		http2Options.MaxConcurrentStreams = 250
+		// match http2.initialMaxConcurrentStreams used by clients
+		// this makes it so that a malicious client can only open 400 streams before we forcibly close the connection
+		// https://github.com/golang/net/commit/b225e7ca6dde1ef5a5ae5ce922861bda011cfabd
+		http2Options.MaxConcurrentStreams = 100
 	}
 
 	// increase the connection buffer size from the 1MB default to handle the specified number of concurrent streams