From 7f9ad511a489a60fa45bdf70e76839301129e1c3 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Sat, 17 Oct 2026 15:43:46 +0000 Subject: [PATCH] api/v2: bound every request body huma gives RawBody no limit, and validates a whole body before the handler runs. The token form is parsed before the secret check. --- hscontrol/api/v2/acl.go | 5 +++++ hscontrol/api/v2/api.go | 5 +++++ hscontrol/api/v2/devices.go | 5 +++++ hscontrol/api/v2/guard_test.go | 21 +++++++++++++++++++++ hscontrol/api/v2/keys.go | 15 ++++++++------- hscontrol/api/v2/oauth.go | 6 ++++++ hscontrol/apiv2_acl_test.go | 12 ++++++++++++ hscontrol/apiv2_oauth_test.go | 18 ++++++++++++++++++ 8 files changed, 80 insertions(+), 7 deletions(-) diff --git a/hscontrol/api/v2/acl.go b/hscontrol/api/v2/acl.go index 3c7dd28fb..0bd07a9aa 100644 --- a/hscontrol/api/v2/acl.go +++ b/hscontrol/api/v2/acl.go @@ -8,6 +8,7 @@ import ( "net/http" "os" "strings" + "time" "github.com/danielgtaylor/huma/v2" "github.com/juanfont/headscale/hscontrol/scope" @@ -86,6 +87,10 @@ func registerACL(api huma.API, b Backend) { // The body is an opaque HuJSON document captured raw; skip huma's // schema validation (which would expect a base64 string). SkipValidateBody: true, + // huma's own Body defaults, which it never applies to RawBody; also + // what v1 setPolicy accepts. + MaxBodyBytes: 1 << 20, + BodyReadTimeout: 5 * time.Second, Errors: []int{ http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound, http.StatusPreconditionFailed, http.StatusInternalServerError, diff --git a/hscontrol/api/v2/api.go b/hscontrol/api/v2/api.go index e42c51b83..122bcf7bc 100644 --- a/hscontrol/api/v2/api.go +++ b/hscontrol/api/v2/api.go @@ -45,6 +45,11 @@ type Backend struct { // or as a Bearer token. var security = []map[string][]string{{"basicAuth": {}}, {"bearerAuth": {}}} +// maxBodyBytes replaces huma's 1 MiB default on operations whose body huma +// validates: validation records an error per bad element before the handler +// runs, and a real body is a few names, tags or routes. +const maxBodyBytes = 64 << 10 + // registrations is populated by each resource file's init(), so adding a // resource means adding a file rather than editing a shared list. var registrations []func(huma.API, Backend) diff --git a/hscontrol/api/v2/devices.go b/hscontrol/api/v2/devices.go index ce803401a..febff5b72 100644 --- a/hscontrol/api/v2/devices.go +++ b/hscontrol/api/v2/devices.go @@ -195,6 +195,7 @@ func registerDevices(api huma.API, b Backend) { Tags: deviceTags, Security: security, DefaultStatus: http.StatusOK, + MaxBodyBytes: maxBodyBytes, Errors: []int{http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound}, }, scope.DevicesCore), func(ctx context.Context, in *setAuthorizedInput) (*emptyOutput, error) { _, err := lookupNode(b, in.DeviceID) @@ -222,6 +223,7 @@ func registerDevices(api huma.API, b Backend) { Tags: deviceTags, Security: security, DefaultStatus: http.StatusOK, + MaxBodyBytes: maxBodyBytes, Errors: []int{http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound}, }, scope.DevicesCore), func(ctx context.Context, in *setNameInput) (*emptyOutput, error) { node, err := lookupNode(b, in.DeviceID) @@ -247,6 +249,7 @@ func registerDevices(api huma.API, b Backend) { Tags: deviceTags, Security: security, DefaultStatus: http.StatusOK, + MaxBodyBytes: maxBodyBytes, Errors: []int{http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound}, }, scope.DevicesCore), func(ctx context.Context, in *setTagsInput) (*emptyOutput, error) { node, err := lookupNode(b, in.DeviceID) @@ -293,6 +296,7 @@ func registerDevices(api huma.API, b Backend) { Tags: deviceTags, Security: security, DefaultStatus: http.StatusOK, + MaxBodyBytes: maxBodyBytes, Errors: []int{http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound}, }, scope.DevicesCore), func(ctx context.Context, in *setKeyInput) (*emptyOutput, error) { node, err := lookupNode(b, in.DeviceID) @@ -326,6 +330,7 @@ func registerDevices(api huma.API, b Backend) { Tags: deviceTags, Security: security, DefaultStatus: http.StatusOK, + MaxBodyBytes: maxBodyBytes, Errors: []int{http.StatusBadRequest, http.StatusUnauthorized, http.StatusForbidden, http.StatusNotFound}, }, scope.DevicesRoutes), func(ctx context.Context, in *setSubnetRoutesInput) (*deviceRoutesOutput, error) { node, err := lookupNode(b, in.DeviceID) diff --git a/hscontrol/api/v2/guard_test.go b/hscontrol/api/v2/guard_test.go index 7d52e0c79..cfefbe606 100644 --- a/hscontrol/api/v2/guard_test.go +++ b/hscontrol/api/v2/guard_test.go @@ -47,6 +47,27 @@ func TestEveryAuthenticatedOperationDeclaresScope(t *testing.T) { } } +// TestEveryRequestBodyIsBounded fails an operation whose body has no size or +// read-time limit, or a validated body above maxBodyBytes. huma sets its +// defaults only for a Body input, never for RawBody. +func TestEveryRequestBodyIsBounded(t *testing.T) { + api := NewAPI(chi.NewMux(), Backend{}) + + for path, item := range api.OpenAPI().Paths { + for method, op := range humaOperations(item) { + if op == nil || op.RequestBody == nil { + continue + } + + if op.MaxBodyBytes <= 0 || op.BodyReadTimeout <= 0 || + (!op.SkipValidateBody && op.MaxBodyBytes > maxBodyBytes) { + t.Errorf("%s %s: MaxBodyBytes=%d BodyReadTimeout=%s", + method, path, op.MaxBodyBytes, op.BodyReadTimeout) + } + } + } +} + func humaOperations(item *huma.PathItem) map[string]*huma.Operation { return map[string]*huma.Operation{ "GET": item.Get, diff --git a/hscontrol/api/v2/keys.go b/hscontrol/api/v2/keys.go index 64b53ab18..84a8ce972 100644 --- a/hscontrol/api/v2/keys.go +++ b/hscontrol/api/v2/keys.go @@ -126,13 +126,14 @@ func registerKeys(api huma.API, b Backend) { keysTags := []string{"Keys", "Tailscale compat"} huma.Register(api, huma.Operation{ - OperationID: "createKey", - Method: http.MethodPost, - Path: "/api/v2/tailnet/{tailnet}/keys", - Summary: "Create an auth key or OAuth client", - Description: "Requires the `auth_keys` scope for an auth key, or `oauth_keys` for an OAuth client (an admin API key is all-access).", - Tags: keysTags, - Security: security, + OperationID: "createKey", + Method: http.MethodPost, + Path: "/api/v2/tailnet/{tailnet}/keys", + Summary: "Create an auth key or OAuth client", + Description: "Requires the `auth_keys` scope for an auth key, or `oauth_keys` for an OAuth client (an admin API key is all-access).", + Tags: keysTags, + Security: security, + MaxBodyBytes: maxBodyBytes, Errors: []int{ http.StatusBadRequest, http.StatusUnauthorized, diff --git a/hscontrol/api/v2/oauth.go b/hscontrol/api/v2/oauth.go index 897e53dad..452fed482 100644 --- a/hscontrol/api/v2/oauth.go +++ b/hscontrol/api/v2/oauth.go @@ -16,6 +16,10 @@ import ( // Tailscale's non-configurable one hour. const accessTokenTTL = time.Hour +// maxTokenRequestBytes caps the token request form, which is read before the +// secret is checked and whose scope and tag lists are stored on the token. +const maxTokenRequestBytes = 16 << 10 + // registerOAuthToken mounts POST /api/v2/oauth/token on the router. It is a plain // handler, not a Huma operation: it consumes application/x-www-form-urlencoded // and emits RFC 6749 OAuth2 error bodies ({"error","error_description"}), neither @@ -38,6 +42,8 @@ type tokenResponse struct { // short-lived bearer token. func oauthTokenHandler(b Backend) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { + r.Body = http.MaxBytesReader(w, r.Body, maxTokenRequestBytes) + err := r.ParseForm() if err != nil { writeOAuthError(w, http.StatusBadRequest, "invalid_request", "could not parse request body") diff --git a/hscontrol/apiv2_acl_test.go b/hscontrol/apiv2_acl_test.go index 7ba2797cf..9e707a47d 100644 --- a/hscontrol/apiv2_acl_test.go +++ b/hscontrol/apiv2_acl_test.go @@ -10,6 +10,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "strings" "testing" "github.com/danielgtaylor/huma/v2/humatest" @@ -128,6 +129,17 @@ func TestAPIv2ACLSetHuJSONWithComments(t *testing.T) { assert.Equal(t, policyETagOf(policy), raw.Header().Get("Etag")) } +// TestAPIv2ACLSetBodyLimited: huma gives a RawBody input no size limit of its +// own, so setACL must declare one. +func TestAPIv2ACLSetBodyLimited(t *testing.T) { + app := createTestApp(t) + api := registerAPIV2(t, app) + + set := setACL(t, api, allowAllPolicy+strings.Repeat(" ", 1<<20), "application/json") + assert.Equal(t, http.StatusRequestEntityTooLarge, set.Code) + assert.Empty(t, storedPolicy(t, app)) +} + func TestAPIv2ACLETagChangesOnChangeStableOnNoop(t *testing.T) { app := createTestApp(t) api := registerAPIV2(t, app) diff --git a/hscontrol/apiv2_oauth_test.go b/hscontrol/apiv2_oauth_test.go index 82063c8fe..f585f13f2 100644 --- a/hscontrol/apiv2_oauth_test.go +++ b/hscontrol/apiv2_oauth_test.go @@ -13,6 +13,7 @@ import ( "testing" apiv2 "github.com/juanfont/headscale/hscontrol/api/v2" + "github.com/juanfont/headscale/hscontrol/scope" "github.com/juanfont/headscale/hscontrol/types/testcapture" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -222,6 +223,23 @@ func TestAPIv2OAuth_TokenEndpoint(t *testing.T) { assert.Equal(t, "no-store", resp.Header.Get("Cache-Control")) } +// TestAPIv2OAuth_RequestBodiesBounded: the token form is parsed before the +// secret is checked, and huma validates a whole body before the handler runs. +func TestAPIv2OAuth_RequestBodiesBounded(t *testing.T) { + _, baseURL, admin := newOAuthTestServer(t) + _, secret := createClient(t, baseURL, admin, []string{"users:read"}, nil) + + scopes := strings.TrimSpace(strings.Repeat("users:read ", 2000)) + status, m := mintToken(t, baseURL, "", secret, scopes, false) + assert.Equalf(t, http.StatusBadRequest, status, "%v", m) + assert.Equal(t, "invalid_request", m["error"]) + + tok := mintScopedToken(t, baseURL, admin, scope.AuthKeys) + status, _ = apiReq(t, http.MethodPost, baseURL+"/api/v2/tailnet/-/keys", tok, + map[string]any{"tags": make([]int, 40000)}) + assert.Equal(t, http.StatusRequestEntityTooLarge, status) +} + func TestAPIv2OAuth_ScopeEnforcement(t *testing.T) { _, baseURL, admin := newOAuthTestServer(t)