Limit admission webhook request body size

cmd/cluster-agent/admission/server.go

@@ -44,6 +44,10 @@ import (
 
 const jsonContentType = "application/json"
 
+// maxRequestBodyBytes bounds the AdmissionReview body the webhook will buffer.
+// Matches controller-runtime's own maxRequestSize.
+const maxRequestBodyBytes = int64(7 << 20)
+
 // Request contains the information of an admission request
 type Request struct {
 	// UID is the unique identifier of the AdmissionRequest
@@ -200,17 +204,23 @@ func (s *Server) handle(w http.ResponseWriter, r *http.Request, webhookName stri
 		return
 	}
 
-	body, err := io.ReadAll(r.Body)
-	if err != nil {
+	if contentType := r.Header.Get("Content-Type"); contentType != jsonContentType {
 		w.WriteHeader(http.StatusBadRequest)
-		log.Warnf("Could not read request body: %v", err)
+		log.Warnf("Unsupported content type %s, only %s is supported", contentType, jsonContentType)
 		return
 	}
-	defer r.Body.Close()
 
-	if contentType := r.Header.Get("Content-Type"); contentType != jsonContentType {
+	defer r.Body.Close()
+	body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, maxRequestBodyBytes))
+	if err != nil {
+		var maxBytesErr *http.MaxBytesError
+		if errors.As(err, &maxBytesErr) {
+			w.WriteHeader(http.StatusRequestEntityTooLarge)
+			log.Warnf("Request body exceeds %d bytes", maxRequestBodyBytes)
+			return
+		}
 		w.WriteHeader(http.StatusBadRequest)
-		log.Warnf("Unsupported content type %s, only %s is supported", contentType, jsonContentType)
+		log.Warnf("Could not read request body: %v", err)
 		return
 	}
 

cmd/cluster-agent/admission/server_test.go

@@ -8,7 +8,11 @@
 package admission
 
 import (
+	"bytes"
 	"encoding/json"
+	"io"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -84,3 +88,138 @@ func TestProbeResponse_ProbeObject(t *testing.T) {
 	require.True(t, ok)
 	assert.Equal(t, "true", annotations[admicommon.ProbeReceivedAnnotationKey])
 }
+
+// newTestServer skips NewServer to avoid leaking a health.RegisterReadiness
+// goroutine that these tests never drain or deregister.
+func newTestServer(t *testing.T) *Server {
+	t.Helper()
+	s := &Server{mux: http.NewServeMux()}
+	s.initDecoder()
+	return s
+}
+
+// countingFiller hands out up to `remaining` bytes without allocating them
+// up front, and records how many were actually read.
+type countingFiller struct {
+	remaining int64
+	read      int64
+}
+
+func (f *countingFiller) Read(p []byte) (int, error) {
+	if f.remaining <= 0 {
+		return 0, io.EOF
+	}
+	n := len(p)
+	if int64(n) > f.remaining {
+		n = int(f.remaining)
+	}
+	for i := range p[:n] {
+		p[i] = 'a'
+	}
+	f.remaining -= int64(n)
+	f.read += int64(n)
+	return n, nil
+}
+
+func TestHandleBodyLimits(t *testing.T) {
+	// Well over maxRequestBodyBytes (7 MiB), but small enough to not stress the runner.
+	const oversizedBodySize = 32 << 20
+
+	validReview := []byte(`{"apiVersion":"admission.k8s.io/v1","kind":"AdmissionReview","request":{"uid":"test-uid"}}`)
+
+	tests := []struct {
+		name           string
+		method         string
+		contentType    string
+		body           []byte
+		oversized      bool
+		wantStatus     int
+		wantWebhookHit bool
+	}{
+		{
+			name:        "oversized body is rejected before being fully buffered",
+			method:      http.MethodPost,
+			contentType: jsonContentType,
+			oversized:   true,
+			wantStatus:  http.StatusRequestEntityTooLarge,
+		},
+		{
+			name:        "oversized body with wrong content type is rejected without reading",
+			method:      http.MethodPost,
+			contentType: "text/plain",
+			oversized:   true,
+			wantStatus:  http.StatusBadRequest,
+		},
+		{
+			name:        "oversized body with wrong method is rejected without reading",
+			method:      http.MethodGet,
+			contentType: jsonContentType,
+			oversized:   true,
+			wantStatus:  http.StatusMethodNotAllowed,
+		},
+		{
+			name:           "valid v1 review is accepted",
+			method:         http.MethodPost,
+			contentType:    jsonContentType,
+			body:           validReview,
+			wantStatus:     http.StatusOK,
+			wantWebhookHit: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			s := newTestServer(t)
+			hit := false
+			s.Register("/injectconfig", "test", admicommon.MutatingWebhook, func(req *Request) *admiv1.AdmissionResponse {
+				hit = true
+				return &admiv1.AdmissionResponse{Allowed: true}
+			}, nil, nil)
+
+			var body io.Reader
+			var filler *countingFiller
+			if tt.oversized {
+				filler = &countingFiller{remaining: oversizedBodySize}
+				body = filler
+			} else {
+				body = bytes.NewReader(tt.body)
+			}
+
+			req := httptest.NewRequest(tt.method, "/injectconfig", body)
+			req.Header.Set("Content-Type", tt.contentType)
+			w := httptest.NewRecorder()
+
+			s.mux.ServeHTTP(w, req)
+
+			assert.Equal(t, tt.wantStatus, w.Code)
+			assert.Equal(t, tt.wantWebhookHit, hit)
+			if filler != nil {
+				// Handler must not buffer meaningfully more than the cap.
+				assert.LessOrEqual(t, filler.read, maxRequestBodyBytes+int64(64<<10),
+					"handler buffered more than the configured cap")
+			}
+		})
+	}
+}
+
+func TestHandleBodyLimitAppliesToEveryRoute(t *testing.T) {
+	s := newTestServer(t)
+	for _, uri := range []string{"/injectconfig", "/autoscaling"} {
+		s.Register(uri, "test", admicommon.MutatingWebhook, func(*Request) *admiv1.AdmissionResponse {
+			t.Fatalf("webhook func should not be called for an oversized request to %s", uri)
+			return nil
+		}, nil, nil)
+	}
+
+	for _, uri := range []string{"/injectconfig", "/autoscaling"} {
+		filler := &countingFiller{remaining: 32 << 20}
+		req := httptest.NewRequest(http.MethodPost, uri, filler)
+		req.Header.Set("Content-Type", jsonContentType)
+		w := httptest.NewRecorder()
+
+		s.mux.ServeHTTP(w, req)
+
+		assert.Equal(t, http.StatusRequestEntityTooLarge, w.Code, "route %s", uri)
+		assert.LessOrEqual(t, filler.read, maxRequestBodyBytes+int64(64<<10), "route %s", uri)
+	}
+}

releasenotes-dca/notes/admission-webhook-body-size-limit-4030c456f1261543.yaml

@@ -0,0 +1,4 @@
+---
+security:
+  - |
+    The Cluster Agent's admission controller webhook now enforces a size limit on incoming request bodies and validates the request content type before reading the body.