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

⚠️ Use limited reader in webhooks #2598

Merged
merged 5 commits into from Jan 8, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
29 changes: 28 additions & 1 deletion pkg/webhook/admission/http.go
Expand Up @@ -34,6 +34,26 @@ import (
var admissionScheme = runtime.NewScheme()
var admissionCodecs = serializer.NewCodecFactory(admissionScheme)

// adapted from https://github.com/kubernetes/kubernetes/blob/c28c2009181fcc44c5f6b47e10e62dacf53e4da0/staging/src/k8s.io/pod-security-admission/cmd/webhook/server/server.go
//
// From https://github.com/kubernetes/apiserver/blob/d6876a0600de06fef75968c4641c64d7da499f25/pkg/server/config.go#L433-L442C5:
//
// 1.5MB is the recommended client request size in byte
// the etcd server should accept. See
sbueringer marked this conversation as resolved.
Show resolved Hide resolved
// https://github.com/etcd-io/etcd/blob/release-3.4/embed/config.go#L56.
// A request body might be encoded in json, and is converted to
// proto when persisted in etcd, so we allow 2x as the largest request
// body size to be accepted and decoded in a write request.
//
// For the admission request, we can infer that it contains at most two objects
// (the old and new versions of the object being admitted), each of which can
// be at most 3MB in size. For the rest of the request, we can assume that
// it will be less than 1MB in size. Therefore, we can set the max request
// size to 7MB.
// If your use case requires larger max request sizes, please
// open an issue (https://github.com/kubernetes-sigs/controller-runtime/issues/new).
var maxRequestSize = int64(7 * 1024 * 1024)
inteon marked this conversation as resolved.
Show resolved Hide resolved

func init() {
utilruntime.Must(v1.AddToScheme(admissionScheme))
utilruntime.Must(v1beta1.AddToScheme(admissionScheme))
Expand All @@ -55,12 +75,19 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}

defer r.Body.Close()
body, err := io.ReadAll(r.Body)
limitedReader := &io.LimitedReader{R: r.Body, N: maxRequestSize}
body, err := io.ReadAll(limitedReader)
if err != nil {
wh.getLogger(nil).Error(err, "unable to read the body from the incoming request")
wh.writeResponse(w, Errored(http.StatusBadRequest, err))
return
}
if limitedReader.N <= 0 {
err := fmt.Errorf("request entity is too large; limit is %d bytes", maxRequestSize)
wh.getLogger(nil).Error(err, "unable to read the body from the incoming request; limit reached")
wh.writeResponse(w, Errored(http.StatusRequestEntityTooLarge, err))
return
}

// verify the content type is accurate
if contentType := r.Header.Get("Content-Type"); contentType != "application/json" {
Expand Down
14 changes: 14 additions & 0 deletions pkg/webhook/admission/http_test.go
Expand Up @@ -19,6 +19,7 @@ package admission
import (
"bytes"
"context"
"crypto/rand"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -97,6 +98,19 @@ var _ = Describe("Admission Webhooks", func() {
Expect(respRecorder.Body.String()).To(Equal(expected))
})

It("should error when given an infinite body", func() {
req := &http.Request{
Header: http.Header{"Content-Type": []string{"application/json"}},
Method: http.MethodPost,
Body: nopCloser{Reader: rand.Reader},
}

expected := `{"response":{"uid":"","allowed":false,"status":{"metadata":{},"message":"request entity is too large; limit is 7340032 bytes","code":413}}}
`
webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})

It("should return the response given by the handler with version defaulted to v1", func() {
req := &http.Request{
Header: http.Header{"Content-Type": []string{"application/json"}},
Expand Down
16 changes: 15 additions & 1 deletion pkg/webhook/authentication/http.go
Expand Up @@ -34,6 +34,13 @@ import (
var authenticationScheme = runtime.NewScheme()
var authenticationCodecs = serializer.NewCodecFactory(authenticationScheme)

// The TokenReview resource mostly contains a bearer token which
// at most should have a few KB's of size, so we picked 1 MB to
// have plenty of buffer.
// If your use case requires larger max request sizes, please
// open an issue (https://github.com/kubernetes-sigs/controller-runtime/issues/new).
var maxRequestSize = int64(1 * 1024 * 1024)
inteon marked this conversation as resolved.
Show resolved Hide resolved

func init() {
utilruntime.Must(authenticationv1.AddToScheme(authenticationScheme))
utilruntime.Must(authenticationv1beta1.AddToScheme(authenticationScheme))
Expand All @@ -55,12 +62,19 @@ func (wh *Webhook) ServeHTTP(w http.ResponseWriter, r *http.Request) {
}

defer r.Body.Close()
body, err := io.ReadAll(r.Body)
limitedReader := &io.LimitedReader{R: r.Body, N: maxRequestSize}
body, err := io.ReadAll(limitedReader)
if err != nil {
wh.getLogger(nil).Error(err, "unable to read the body from the incoming request")
wh.writeResponse(w, Errored(err))
return
}
if limitedReader.N <= 0 {
err := fmt.Errorf("request entity is too large; limit is %d bytes", maxRequestSize)
wh.getLogger(nil).Error(err, "unable to read the body from the incoming request; limit reached")
wh.writeResponse(w, Errored(err))
return
}

// verify the content type is accurate
if contentType := r.Header.Get("Content-Type"); contentType != "application/json" {
Expand Down
14 changes: 14 additions & 0 deletions pkg/webhook/authentication/http_test.go
Expand Up @@ -19,6 +19,7 @@ package authentication
import (
"bytes"
"context"
"crypto/rand"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -107,6 +108,19 @@ var _ = Describe("Authentication Webhooks", func() {
Expect(respRecorder.Body.String()).To(Equal(expected))
})

It("should error when given an infinite body", func() {
req := &http.Request{
Header: http.Header{"Content-Type": []string{"application/json"}},
Method: http.MethodPost,
Body: nopCloser{Reader: rand.Reader},
}

expected := `{"metadata":{"creationTimestamp":null},"spec":{},"status":{"user":{},"error":"request entity is too large; limit is 1048576 bytes"}}
`
webhook.ServeHTTP(respRecorder, req)
Expect(respRecorder.Body.String()).To(Equal(expected))
})

It("should return the response given by the handler with version defaulted to v1", func() {
req := &http.Request{
Header: http.Header{"Content-Type": []string{"application/json"}},
Expand Down