Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OCPBUGS-20380: [release-4.14] UPSTREAM: 121120: Prevent rapid reset http2 DOS on API server #1752

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 9 additions & 6 deletions staging/src/k8s.io/apimachinery/pkg/util/runtime/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand All @@ -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
}
2 changes: 1 addition & 1 deletion staging/src/k8s.io/apiserver/pkg/server/filters/goaway.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
Expand Down
32 changes: 29 additions & 3 deletions staging/src/k8s.io/apiserver/pkg/server/filters/timeout.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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()

Expand All @@ -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)
Expand All @@ -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()
Expand Down
5 changes: 4 additions & 1 deletion staging/src/k8s.io/apiserver/pkg/server/secure_serving.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down