diff --git a/lib/services/access_request.go b/lib/services/access_request.go index 4de491ecdcc05..9a62cc22c11f7 100644 --- a/lib/services/access_request.go +++ b/lib/services/access_request.go @@ -46,6 +46,7 @@ import ( const ( maxAccessRequestReasonSize = 4096 maxResourcesPerRequest = 300 + maxResourcesLength = 2048 // A day is sometimes 23 hours, sometimes 25 hours, usually 24 hours. day = 24 * time.Hour @@ -1157,6 +1158,7 @@ func (m *RequestValidator) Validate(ctx context.Context, req types.AccessRequest // deduplicate requested resource IDs var deduplicated []types.ResourceID seen := make(map[string]struct{}) + resourcesLen := 0 for _, resource := range req.GetRequestedResourceIDs() { id := types.ResourceIDToString(resource) if _, isDuplicate := seen[id]; isDuplicate { @@ -1164,9 +1166,17 @@ func (m *RequestValidator) Validate(ctx context.Context, req types.AccessRequest } seen[id] = struct{}{} deduplicated = append(deduplicated, resource) + resourcesLen += len(id) } req.SetRequestedResourceIDs(deduplicated) + // In addition to capping the maximum number of resources in a single request, + // we also need to ensure that the sum of the resource IDs in the request doesn't + // get too big. + if resourcesLen > maxResourcesLength { + return trace.BadParameter("access request exceeds maximum length: try reducing the number of resources in the request") + } + // determine the roles which should be requested for a resource access // request, and write them to the request if err := m.setRolesForResourceRequest(ctx, req); err != nil { diff --git a/lib/services/access_request_test.go b/lib/services/access_request_test.go index 1d1712bb6f72a..d761c95f6a6cc 100644 --- a/lib/services/access_request_test.go +++ b/lib/services/access_request_test.go @@ -2150,7 +2150,7 @@ func (mcg mockClusterGetter) GetRemoteCluster(ctx context.Context, clusterName s return nil, trace.NotFound("remote cluster %q was not found", clusterName) } -func TestValidateDuplicateRequestedResources(t *testing.T) { +func TestValidateResourceRequestSizeLimits(t *testing.T) { g := &mockGetter{ roles: make(map[string]types.Role), userStates: make(map[string]*userloginstate.UserLoginState), @@ -2212,6 +2212,17 @@ func TestValidateDuplicateRequestedResources(t *testing.T) { require.Len(t, req.GetRequestedResourceIDs(), 2) require.Equal(t, "/someCluster/node/resource1", types.ResourceIDToString(req.GetRequestedResourceIDs()[0])) require.Equal(t, "/someCluster/node/resource2", types.ResourceIDToString(req.GetRequestedResourceIDs()[1])) + + var requestedResourceIDs []types.ResourceID + for i := 0; i < 200; i++ { + requestedResourceIDs = append(requestedResourceIDs, types.ResourceID{ + ClusterName: "someCluster", + Kind: "node", + Name: "resource" + strconv.Itoa(i), + }) + } + req.SetRequestedResourceIDs(requestedResourceIDs) + require.ErrorContains(t, ValidateAccessRequestForUser(context.Background(), clock, g, req, identity, ExpandVars(true)), "access request exceeds maximum length") } func TestValidateAccessRequestClusterNames(t *testing.T) {