Merge pull request #121120 from enj/enj/i/h2_dos

Prevent rapid reset http2 DOS on API server

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/apimachinery/pkg/util/runtime/runtime_test.go

@@ -24,7 +24,9 @@ import (
 	"os"
 	"regexp"
 	"strings"
+	"sync"
 	"testing"
+	"time"
 )
 
 func TestHandleCrash(t *testing.T) {
@@ -156,3 +158,27 @@ func captureStderr(f func()) (string, error) {
 
 	return <-resultCh, nil
 }
+
+func Test_rudimentaryErrorBackoff_OnError_ParallelSleep(t *testing.T) {
+	r := &rudimentaryErrorBackoff{
+		minPeriod: time.Second,
+	}
+
+	start := make(chan struct{})
+	var wg sync.WaitGroup
+	for i := 0; i < 30; i++ {
+		wg.Add(1)
+		go func() {
+			<-start
+			r.OnError(nil) // input error is ignored
+			wg.Done()
+		}()
+	}
+	st := time.Now()
+	close(start)
+	wg.Wait()
+
+	if since := time.Since(st); since > 5*time.Second {
+		t.Errorf("OnError slept for too long: %s", since)
+	}
+}

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

@@ -29,8 +29,11 @@ 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"
+	genericfeatures "k8s.io/apiserver/pkg/features"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/klog/v2"
 )
 
@@ -101,6 +104,18 @@ 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 the
+		// performance of http1 with keep-alive disabled).
+		if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.UnauthenticatedHTTP2DOSMitigation) && req.ProtoMajor == 2 && isAnonymousUser(resp.User) {
+			// limit this connection to just this request,
+			// and then send a GOAWAY and tear down the TCP connection
+			// https://github.com/golang/net/commit/97aa3a539ec716117a9d15a4659a911f50d13c3c
+			w.Header().Set("Connection", "close")
+		}
+
 		req = req.WithContext(genericapirequest.WithUser(req.Context(), resp.User))
 		handler.ServeHTTP(w, req)
 	})
@@ -108,6 +123,17 @@ func withAuthentication(handler http.Handler, auth authenticator.Request, failed
 
 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 the
+		// performance of http1 with keep-alive disabled).
+		if utilfeature.DefaultFeatureGate.Enabled(genericfeatures.UnauthenticatedHTTP2DOSMitigation) && req.ProtoMajor == 2 {
+			// limit this connection to just this request,
+			// and then send a GOAWAY and tear down the TCP connection
+			// https://github.com/golang/net/commit/97aa3a539ec716117a9d15a4659a911f50d13c3c
+			w.Header().Set("Connection", "close")
+		}
 		ctx := req.Context()
 		requestInfo, found := genericapirequest.RequestInfoFrom(ctx)
 		if !found {
@@ -127,3 +153,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/endpoints/filters/authentication_test.go

@@ -18,20 +18,31 @@ package filters
 
 import (
 	"context"
+	"crypto/tls"
+	"crypto/x509"
 	"errors"
+	"io"
+	"net"
 	"net/http"
 	"net/http/httptest"
+	"sync/atomic"
 	"testing"
 	"time"
 
 	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
+	"golang.org/x/net/http2"
 
 	"k8s.io/apiserver/pkg/authentication/authenticator"
 	"k8s.io/apiserver/pkg/authentication/authenticatorfactory"
+	"k8s.io/apiserver/pkg/authentication/request/anonymous"
 	"k8s.io/apiserver/pkg/authentication/request/headerrequest"
 	"k8s.io/apiserver/pkg/authentication/user"
 	genericapirequest "k8s.io/apiserver/pkg/endpoints/request"
+	"k8s.io/apiserver/pkg/features"
+	utilfeature "k8s.io/apiserver/pkg/util/feature"
+	"k8s.io/client-go/kubernetes/scheme"
+	featuregatetesting "k8s.io/component-base/featuregate/testing"
 )
 
 func TestAuthenticateRequestWithAud(t *testing.T) {
@@ -465,3 +476,191 @@ func TestAuthenticateRequestClearHeaders(t *testing.T) {
 		})
 	}
 }
+
+func TestUnauthenticatedHTTP2ClientConnectionClose(t *testing.T) {
+	s := httptest.NewUnstartedServer(WithAuthentication(
+		http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { _, _ = w.Write([]byte("ok")) }),
+		authenticator.RequestFunc(func(r *http.Request) (*authenticator.Response, bool, error) {
+			switch r.Header.Get("Authorization") {
+			case "known":
+				return &authenticator.Response{User: &user.DefaultInfo{Name: "panda"}}, true, nil
+			case "error":
+				return nil, false, errors.New("authn err")
+			case "anonymous":
+				return anonymous.NewAuthenticator().AuthenticateRequest(r)
+			case "anonymous_group":
+				return &authenticator.Response{User: &user.DefaultInfo{Groups: []string{user.AllUnauthenticated}}}, true, nil
+			default:
+				return nil, false, nil
+			}
+		}),
+		http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			r = r.WithContext(genericapirequest.WithRequestInfo(r.Context(), &genericapirequest.RequestInfo{}))
+			Unauthorized(scheme.Codecs).ServeHTTP(w, r)
+		}),
+		nil,
+		nil,
+	))
+
+	http2Options := &http2.Server{}
+
+	if err := http2.ConfigureServer(s.Config, http2Options); err != nil {
+		t.Fatal(err)
+	}
+
+	s.TLS = s.Config.TLSConfig
+
+	s.StartTLS()
+	t.Cleanup(s.Close)
+
+	const reqs = 4
+
+	cases := []struct {
+		name                   string
+		authorizationHeader    string
+		skipHTTP2DOSMitigation bool
+		expectConnections      uint64
+	}{
+		{
+			name:                   "known",
+			authorizationHeader:    "known",
+			skipHTTP2DOSMitigation: false,
+			expectConnections:      1,
+		},
+		{
+			name:                   "error",
+			authorizationHeader:    "error",
+			skipHTTP2DOSMitigation: false,
+			expectConnections:      reqs,
+		},
+		{
+			name:                   "anonymous",
+			authorizationHeader:    "anonymous",
+			skipHTTP2DOSMitigation: false,
+			expectConnections:      reqs,
+		},
+		{
+			name:                   "anonymous_group",
+			authorizationHeader:    "anonymous_group",
+			skipHTTP2DOSMitigation: false,
+			expectConnections:      reqs,
+		},
+		{
+			name:                   "other",
+			authorizationHeader:    "other",
+			skipHTTP2DOSMitigation: false,
+			expectConnections:      reqs,
+		},
+
+		{
+			name:                   "known skip=true",
+			authorizationHeader:    "known",
+			skipHTTP2DOSMitigation: true,
+			expectConnections:      1,
+		},
+		{
+			name:                   "error skip=true",
+			authorizationHeader:    "error",
+			skipHTTP2DOSMitigation: true,
+			expectConnections:      1,
+		},
+		{
+			name:                   "anonymous skip=true",
+			authorizationHeader:    "anonymous",
+			skipHTTP2DOSMitigation: true,
+			expectConnections:      1,
+		},
+		{
+			name:                   "anonymous_group skip=true",
+			authorizationHeader:    "anonymous_group",
+			skipHTTP2DOSMitigation: true,
+			expectConnections:      1,
+		},
+		{
+			name:                   "other skip=true",
+			authorizationHeader:    "other",
+			skipHTTP2DOSMitigation: true,
+			expectConnections:      1,
+		},
+	}
+
+	rootCAs := x509.NewCertPool()
+	rootCAs.AddCert(s.Certificate())
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			f := func(t *testing.T, nextProto string, expectConnections uint64) {
+				defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.UnauthenticatedHTTP2DOSMitigation, !tc.skipHTTP2DOSMitigation)()
+
+				var localAddrs atomic.Uint64 // indicates how many TCP connection set up
+
+				tlsConfig := &tls.Config{
+					RootCAs:    rootCAs,
+					NextProtos: []string{nextProto},
+				}
+
+				dailer := tls.Dialer{
+					Config: tlsConfig,
+				}
+
+				tr := &http.Transport{
+					TLSHandshakeTimeout: 10 * time.Second,
+					TLSClientConfig:     tlsConfig,
+					DialTLSContext: func(ctx context.Context, network, addr string) (net.Conn, error) {
+						conn, err := dailer.DialContext(ctx, network, addr)
+						if err != nil {
+							return nil, err
+						}
+
+						localAddrs.Add(1)
+
+						return conn, nil
+					},
+				}
+
+				tr.MaxIdleConnsPerHost = 1 // allow http1 to have keep alive connections open
+				if nextProto == http2.NextProtoTLS {
+					// Disable connection pooling to avoid additional connections
+					// that cause the test to flake
+					tr.MaxIdleConnsPerHost = -1
+					if err := http2.ConfigureTransport(tr); err != nil {
+						t.Fatal(err)
+					}
+				}
+
+				client := &http.Client{
+					Transport: tr,
+				}
+
+				for i := 0; i < reqs; i++ {
+					req, err := http.NewRequest(http.MethodGet, s.URL, nil)
+					if err != nil {
+						t.Fatal(err)
+					}
+					if len(tc.authorizationHeader) > 0 {
+						req.Header.Set("Authorization", tc.authorizationHeader)
+					}
+
+					resp, err := client.Do(req)
+					if err != nil {
+						t.Fatal(err)
+					}
+					_, _ = io.Copy(io.Discard, resp.Body)
+					_ = resp.Body.Close()
+				}
+
+				if expectConnections != localAddrs.Load() {
+					t.Fatalf("expect TCP connection: %d, actual: %d", expectConnections, localAddrs.Load())
+				}
+			}
+
+			t.Run(http2.NextProtoTLS, func(t *testing.T) {
+				f(t, http2.NextProtoTLS, tc.expectConnections)
+			})
+
+			t.Run("http/1.1", func(t *testing.T) {
+				f(t, "http/1.1", 1)
+			})
+		})
+	}
+}

staging/src/k8s.io/apiserver/pkg/features/kube_features.go

@@ -184,6 +184,24 @@ const (
 	// Enables server-side field validation.
 	ServerSideFieldValidation featuregate.Feature = "ServerSideFieldValidation"
 
+	// owner: @enj
+	// beta: v1.29
+	//
+	// Enables http2 DOS mitigations for unauthenticated clients.
+	//
+	// Some known reasons to disable these mitigations:
+	//
+	// An API server that is fronted by an L7 load balancer that is set up
+	// to mitigate http2 attacks may opt to disable this protection to prevent
+	// unauthenticated clients from disabling connection reuse between the load
+	// balancer and the API server (many incoming connections could share the
+	// same backend connection).
+	//
+	// An API server that is on a private network may opt to disable this
+	// protection to prevent performance regressions for unauthenticated
+	// clients.
+	UnauthenticatedHTTP2DOSMitigation featuregate.Feature = "UnauthenticatedHTTP2DOSMitigation"
+
 	// owner: @caesarxuchao @roycaihw
 	// alpha: v1.20
 	//
@@ -287,6 +305,8 @@ var defaultKubernetesFeatureGates = map[featuregate.Feature]featuregate.FeatureS
 
 	StructuredAuthenticationConfiguration: {Default: false, PreRelease: featuregate.Alpha},
 
+	UnauthenticatedHTTP2DOSMitigation: {Default: true, PreRelease: featuregate.Beta},
+
 	WatchBookmark: {Default: true, PreRelease: featuregate.GA, LockToDefault: true},
 
 	InPlacePodVerticalScaling: {Default: false, PreRelease: featuregate.Alpha},

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