diff --git a/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go b/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go index d738725caf0..3674914f701 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go +++ b/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. diff --git a/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime_test.go b/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime_test.go index 2368a513b91..c886b6826ff 100644 --- a/staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime_test.go +++ b/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) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authentication.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authentication.go index 277bdcdfe5f..64b3569d0d9 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authentication.go +++ b/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 +} diff --git a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authentication_test.go b/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authentication_test.go index 2bdde2741eb..910eecb8997 100644 --- a/staging/src/k8s.io/apiserver/pkg/endpoints/filters/authentication_test.go +++ b/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) + }) + }) + } +} diff --git a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go b/staging/src/k8s.io/apiserver/pkg/features/kube_features.go index 68b13d720ec..5468c253997 100644 --- a/staging/src/k8s.io/apiserver/pkg/features/kube_features.go +++ b/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}, diff --git a/staging/src/k8s.io/apiserver/pkg/server/secure_serving.go b/staging/src/k8s.io/apiserver/pkg/server/secure_serving.go index 64bcc87ebf1..0a4fdc6932e 100644 --- a/staging/src/k8s.io/apiserver/pkg/server/secure_serving.go +++ b/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