From 7d0fe3f04974860b177ce42a90e129ead65ee526 Mon Sep 17 00:00:00 2001 From: Vincent Date: Wed, 15 Nov 2023 21:18:23 +0100 Subject: [PATCH] sec: Make GraphQL cost limits configurable (#58346) * add ratelimit configuration to site-admin (cherry picked from commit 8ac56cacf3ecf6c4ca5f706265edae8d85a7145a) --- CHANGELOG.md | 1 + cmd/frontend/graphqlbackend/graphqlbackend.go | 2 +- cmd/frontend/graphqlbackend/rate_limit.go | 4 ---- cmd/frontend/graphqlbackend/search_test.go | 2 +- cmd/frontend/graphqlbackend/testing.go | 3 ++- cmd/frontend/internal/httpapi/graphql.go | 6 +++-- internal/conf/computed.go | 23 +++++++++++++++++++ schema/schema.go | 12 +++++++++- schema/site.schema.json | 21 +++++++++++++++++ 9 files changed, 64 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6084f9c10d6d..22893b352c14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,7 @@ All notable changes to Sourcegraph are documented in this file. - Added a new authorization configuration options to GitLab code host connections: "markInternalReposAsPublic". Setting "markInternalReposAsPublic" to true is useful for organizations that have a large amount of internal repositories that everyone on the instance should be able to access, removing the need to have permissions to access these repositories. Additionally, when configuring a GitLab auth provider, you can specify "syncInternalRepoPermissions": false, which will remove the need to sync permissions for these internal repositories. [#57858](https://github.com/sourcegraph/sourcegraph/pull/57858) - Experimental support for OpenAI powered autocomplete has been added. [#57872](https://github.com/sourcegraph/sourcegraph/pull/57872) +- Added configurable GraphQL query cost limitations to prevent unintended resource exhaustion. Default values are now provided and enforced, replacing the previously unlimited behaviour. For more information, please refer to: [GraphQL Cost Limits Documentation](https://docs.sourcegraph.com/api/graphql#cost-limits). See details at [#58346](https://github.com/sourcegraph/sourcegraph/pull/58346). ### Changed diff --git a/cmd/frontend/graphqlbackend/graphqlbackend.go b/cmd/frontend/graphqlbackend/graphqlbackend.go index 6d6813f5abc1..207760ab429c 100644 --- a/cmd/frontend/graphqlbackend/graphqlbackend.go +++ b/cmd/frontend/graphqlbackend/graphqlbackend.go @@ -634,7 +634,7 @@ func NewSchema( opts := []graphql.SchemaOpt{ graphql.Tracer(newRequestTracer(logger, db)), graphql.UseStringDescriptions(), - graphql.MaxDepth(maxDepth), + graphql.MaxDepth(conf.RateLimits().GraphQLMaxDepth), } opts = append(opts, graphqlOpts...) return graphql.ParseSchema( diff --git a/cmd/frontend/graphqlbackend/rate_limit.go b/cmd/frontend/graphqlbackend/rate_limit.go index 70f7c7c1e2fd..529e71d7db10 100644 --- a/cmd/frontend/graphqlbackend/rate_limit.go +++ b/cmd/frontend/graphqlbackend/rate_limit.go @@ -22,10 +22,6 @@ import ( // the algorithm const costEstimateVersion = 2 -const MaxAliasCount = 500 // SECURITY: prevent too many aliased queries -const MaxFieldCount = 500 * 1000 // SECURITY: prevent deeply nested or overly broad queries -const maxDepth = 30 // SECURITY: prevent deep queries that consume too many resources - type QueryCost struct { FieldCount int MaxDepth int diff --git a/cmd/frontend/graphqlbackend/search_test.go b/cmd/frontend/graphqlbackend/search_test.go index 5580c2d83269..278a29cc93cb 100644 --- a/cmd/frontend/graphqlbackend/search_test.go +++ b/cmd/frontend/graphqlbackend/search_test.go @@ -122,7 +122,7 @@ func TestSearch(t *testing.T) { sr := newSchemaResolver(db, gsClient) gqlSchema, err := graphql.ParseSchema(mainSchema, sr, graphql.Tracer(newRequestTracer(logtest.Scoped(t), db)), - graphql.MaxDepth(maxDepth)) + graphql.MaxDepth(conf.RateLimits().GraphQLMaxDepth)) if err != nil { t.Fatal(err) } diff --git a/cmd/frontend/graphqlbackend/testing.go b/cmd/frontend/graphqlbackend/testing.go index 42f8da3e7fa6..66d9c1ce50f7 100644 --- a/cmd/frontend/graphqlbackend/testing.go +++ b/cmd/frontend/graphqlbackend/testing.go @@ -15,6 +15,7 @@ import ( gqlerrors "github.com/graph-gophers/graphql-go/errors" "github.com/stretchr/testify/require" + "github.com/sourcegraph/sourcegraph/internal/conf" "github.com/sourcegraph/sourcegraph/internal/database" "github.com/sourcegraph/sourcegraph/internal/gitserver" ) @@ -31,7 +32,7 @@ func mustParseGraphQLSchemaWithClient(t *testing.T, db database.DB, gitserverCli gitserverClient, []OptionalResolver{}, graphql.PanicHandler(printStackTrace{&gqlerrors.DefaultPanicHandler{}}), - graphql.MaxDepth(maxDepth), + graphql.MaxDepth(conf.RateLimits().GraphQLMaxDepth), ) if parseSchemaErr != nil { t.Fatal(parseSchemaErr) diff --git a/cmd/frontend/internal/httpapi/graphql.go b/cmd/frontend/internal/httpapi/graphql.go index 162c356c132b..4bc1f83223ad 100644 --- a/cmd/frontend/internal/httpapi/graphql.go +++ b/cmd/frontend/internal/httpapi/graphql.go @@ -102,11 +102,13 @@ func serveGraphQL(logger log.Logger, schema *graphql.Schema, rlw graphqlbackend. } else if cost != nil { traceData.cost = cost - if !isInternal && (cost.AliasCount > graphqlbackend.MaxAliasCount) { + rl := conf.RateLimits() + + if !isInternal && (cost.AliasCount > rl.GraphQLMaxAliases) { return writeViolationError(w, "query exceeds maximum query cost") } - if !isInternal && (cost.FieldCount > graphqlbackend.MaxFieldCount) { + if !isInternal && (cost.FieldCount > rl.GraphQLMaxFieldCount) { return writeViolationError(w, "query exceeds maximum query cost") } diff --git a/internal/conf/computed.go b/internal/conf/computed.go index 20d74297b32a..6e3b7ca1f694 100644 --- a/internal/conf/computed.go +++ b/internal/conf/computed.go @@ -450,6 +450,29 @@ func PasswordPolicyEnabled() bool { return pc.Enabled } +func RateLimits() schema.RateLimits { + rl := schema.RateLimits{ + GraphQLMaxAliases: 500, + GraphQLMaxFieldCount: 500_000, + GraphQLMaxDepth: 30, + } + + configured := Get().RateLimits + + if configured != nil { + if configured.GraphQLMaxAliases <= 0 { + rl.GraphQLMaxAliases = configured.GraphQLMaxAliases + } + if configured.GraphQLMaxFieldCount <= 0 { + rl.GraphQLMaxFieldCount = configured.GraphQLMaxFieldCount + } + if configured.GraphQLMaxDepth <= 0 { + rl.GraphQLMaxDepth = configured.GraphQLMaxDepth + } + } + return rl +} + // By default, password reset links are valid for 4 hours. const defaultPasswordLinkExpiry = 14400 diff --git a/schema/schema.go b/schema/schema.go index 4d356cce6355..b452c8adb410 100644 --- a/schema/schema.go +++ b/schema/schema.go @@ -1993,6 +1993,14 @@ type Ranking struct { // RepoScores description: a map of URI directories to numeric scores for specifying search result importance, like {"github.com": 500, "github.com/sourcegraph": 300, "github.com/sourcegraph/sourcegraph": 100}. Would rank "github.com/sourcegraph/sourcegraph" as 500+300+100=900, and "github.com/other/foo" as 500. RepoScores map[string]float64 `json:"repoScores,omitempty"` } +type RateLimits struct { + // GraphQLMaxAliases description: Maximum number of aliases allowed in a GraphQL query + GraphQLMaxAliases int `json:"graphQLMaxAliases,omitempty"` + // GraphQLMaxDepth description: Maximum depth of nested objects allowed for GraphQL queries. Changes to this setting require a restart. + GraphQLMaxDepth int `json:"graphQLMaxDepth,omitempty"` + // GraphQLMaxFieldCount description: Maximum number of estimated fields allowed in a GraphQL response + GraphQLMaxFieldCount int `json:"graphQLMaxFieldCount,omitempty"` +} // RepoPurgeWorker description: Configuration for repository purge worker. type RepoPurgeWorker struct { @@ -2743,7 +2751,8 @@ type SiteConfiguration struct { // PermissionsUserMapping description: Settings for Sourcegraph explicit permissions, which allow the site admin to explicitly manage repository permissions via the GraphQL API. This will mark repositories as restricted by default. PermissionsUserMapping *PermissionsUserMapping `json:"permissions.userMapping,omitempty"` // ProductResearchPageEnabled description: Enables users access to the product research page in their settings. - ProductResearchPageEnabled *bool `json:"productResearchPage.enabled,omitempty"` + ProductResearchPageEnabled *bool `json:"productResearchPage.enabled,omitempty"` + RateLimits *RateLimits `json:"rateLimits,omitempty"` // RedactOutboundRequestHeaders description: Enables redacting sensitive information from outbound requests. Important: We only respect this setting in development environments. In production, we always redact outbound requests. RedactOutboundRequestHeaders *bool `json:"redactOutboundRequestHeaders,omitempty"` // RepoConcurrentExternalServiceSyncers description: The number of concurrent external service syncers that can run. @@ -2919,6 +2928,7 @@ func (v *SiteConfiguration) UnmarshalJSON(data []byte) error { delete(m, "permissions.syncUsersMaxConcurrency") delete(m, "permissions.userMapping") delete(m, "productResearchPage.enabled") + delete(m, "rateLimits") delete(m, "redactOutboundRequestHeaders") delete(m, "repoConcurrentExternalServiceSyncers") delete(m, "repoListUpdateInterval") diff --git a/schema/site.schema.json b/schema/site.schema.json index bf5ee15aabe4..f6ec5af1240a 100644 --- a/schema/site.schema.json +++ b/schema/site.schema.json @@ -49,6 +49,27 @@ "type": "boolean", "default": false }, + "rateLimits": { + "type": "object", + "additionalProperties": false, + "properties": { + "graphQLMaxDepth": { + "description": "Maximum depth of nested objects allowed for GraphQL queries. Changes to this setting require a restart.", + "type": "integer", + "default": 30 + }, + "graphQLMaxAliases": { + "description": "Maximum number of aliases allowed in a GraphQL query", + "type": "integer", + "default": 500 + }, + "graphQLMaxFieldCount": { + "description": "Maximum number of estimated fields allowed in a GraphQL response", + "type": "integer", + "default": 500000 + } + } + }, "exportUsageTelemetry": { "type": "object", "additionalProperties": false,