Skip to content

Commit

Permalink
[Backport 5.2] sec: Make GraphQL cost limits configurable (#58352)
Browse files Browse the repository at this point in the history
sec: Make GraphQL cost limits configurable (#58346)

* add ratelimit configuration to site-admin

(cherry picked from commit 8ac56ca)
  • Loading branch information
evict committed Nov 15, 2023
1 parent ae6d930 commit e10060c
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 10 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion cmd/frontend/graphqlbackend/graphqlbackend.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 0 additions & 4 deletions cmd/frontend/graphqlbackend/rate_limit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion cmd/frontend/graphqlbackend/search_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
3 changes: 2 additions & 1 deletion cmd/frontend/graphqlbackend/testing.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)
Expand All @@ -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)
Expand Down
6 changes: 4 additions & 2 deletions cmd/frontend/internal/httpapi/graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand Down
23 changes: 23 additions & 0 deletions internal/conf/computed.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
12 changes: 11 additions & 1 deletion schema/schema.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

21 changes: 21 additions & 0 deletions schema/site.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit e10060c

Please sign in to comment.