diff --git a/router-tests/security_test.go b/router-tests/security_test.go index 742301c21b..dde331b55f 100644 --- a/router-tests/security_test.go +++ b/router-tests/security_test.go @@ -1,6 +1,8 @@ package integration import ( + "fmt" + "github.com/wundergraph/cosmo/router/core" "net/http" "testing" @@ -109,3 +111,255 @@ func TestParserHardLimits(t *testing.T) { }) }) } + +func TestQueryNamingLimits(t *testing.T) { + t.Parallel() + + t.Run("verify operation query naming limits", func(t *testing.T) { + t.Parallel() + + t.Run("with large query name and no operation name", func(t *testing.T) { + t.Parallel() + maxLength := 2 + queryName := "longstring" + + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: func(securityConfiguration *config.SecurityConfiguration) { + securityConfiguration.OperationNameLengthLimit = maxLength + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + expectedErrorMessage := fmt.Sprintf(`{"errors":[{"message":"operation name of length %d exceeds max length of %d"}]}`, len(queryName), maxLength) + + resPost, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: "query " + queryName + " { employees { id } }", + }) + require.NoError(t, err) + require.JSONEq(t, expectedErrorMessage, resPost.Body) + require.Equal(t, http.StatusBadRequest, resPost.Response.StatusCode) + + resGet, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + Query: "query " + queryName + " { employees { id } }", + }) + require.NoError(t, err) + require.JSONEq(t, expectedErrorMessage, resGet.Body) + require.Equal(t, http.StatusBadRequest, resGet.Response.StatusCode) + }) + }) + + t.Run("with large query name and small operation name", func(t *testing.T) { + t.Parallel() + maxLength := 6 + queryName := "longstring" + operationNameGet := `short` + operationNamePost := `"short"` + + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: func(securityConfiguration *config.SecurityConfiguration) { + securityConfiguration.OperationNameLengthLimit = maxLength + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + expectedErrorMessage := fmt.Sprintf(`{"errors":[{"message":"operation name of length %d exceeds max length of %d"}]}`, len(queryName), maxLength) + + resPost, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: "query " + queryName + " { employees { id } }", + OperationName: []byte(operationNamePost), + }) + require.NoError(t, err) + require.JSONEq(t, expectedErrorMessage, resPost.Body) + require.Equal(t, http.StatusBadRequest, resPost.Response.StatusCode) + + resGet, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + Query: "query " + queryName + " { employees { id } }", + OperationName: []byte(operationNameGet), + }) + require.NoError(t, err) + require.JSONEq(t, expectedErrorMessage, resGet.Body) + require.Equal(t, http.StatusBadRequest, resGet.Response.StatusCode) + }) + }) + + t.Run("with small query name and large operation name", func(t *testing.T) { + t.Parallel() + + maxLength := 6 + queryName := "short" + operationNameGet := `longname` + operationNamePost := `"longname"` + + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: func(securityConfiguration *config.SecurityConfiguration) { + securityConfiguration.OperationNameLengthLimit = maxLength + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + expectedErrorMessage := fmt.Sprintf(`{"errors":[{"message":"operation name of length %d exceeds max length of %d"}]}`, len(operationNameGet), maxLength) + + resPost, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: "query " + queryName + " { employees { id } }", + OperationName: []byte(operationNamePost), + }) + require.NoError(t, err) + require.JSONEq(t, expectedErrorMessage, resPost.Body) + require.Equal(t, http.StatusBadRequest, resPost.Response.StatusCode) + + resGet, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + Query: "query " + queryName + " { employees { id } }", + OperationName: []byte(operationNameGet), + }) + require.NoError(t, err) + require.JSONEq(t, expectedErrorMessage, resGet.Body) + require.Equal(t, http.StatusBadRequest, resGet.Response.StatusCode) + }) + }) + + t.Run("with small query name and small operation name", func(t *testing.T) { + t.Parallel() + + liitSize := 7 + queryName := "short" + operationNameGet := `short` + operationNamePost := `"short"` + + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: func(securityConfiguration *config.SecurityConfiguration) { + securityConfiguration.OperationNameLengthLimit = liitSize + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + resPost, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: "query " + queryName + " { employees { id } }", + OperationName: []byte(operationNamePost), + }) + require.NoError(t, err) + require.JSONEq(t, `{"data":{"employees":[{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":7},{"id":8},{"id":10},{"id":11},{"id":12}]}}`, resPost.Body) + require.Equal(t, http.StatusOK, resPost.Response.StatusCode) + + resGet, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + Query: "query " + queryName + " { employees { id } }", + OperationName: []byte(operationNameGet), + }) + require.NoError(t, err) + require.JSONEq(t, `{"data":{"employees":[{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":7},{"id":8},{"id":10},{"id":11},{"id":12}]}}`, resGet.Body) + require.Equal(t, http.StatusOK, resGet.Response.StatusCode) + }) + }) + + t.Run("with multiple queries of which one is large", func(t *testing.T) { + t.Parallel() + + maxLength := 6 + query1Name := "short" + query2Name := "longstring" + + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: func(securityConfiguration *config.SecurityConfiguration) { + securityConfiguration.OperationNameLengthLimit = maxLength + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + expectedErrorMessage := fmt.Sprintf(`{"errors":[{"message":"operation name of length %d exceeds max length of %d"}]}`, len(query2Name), maxLength) + + resPost, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: "query " + query1Name + " { employees { id } } query " + query2Name + " { employees { id } }", + }) + require.NoError(t, err) + require.JSONEq(t, expectedErrorMessage, resPost.Body) + require.Equal(t, http.StatusBadRequest, resPost.Response.StatusCode) + + resGet, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + Query: "query " + query1Name + " { employees { id } } query " + query2Name + " { employees { id } }", + }) + require.NoError(t, err) + require.JSONEq(t, expectedErrorMessage, resGet.Body) + require.Equal(t, http.StatusBadRequest, resGet.Response.StatusCode) + }) + }) + + t.Run("with multiple queries of which both are small", func(t *testing.T) { + t.Parallel() + + maxLength := 6 + query1Name := "short1" + query2Name := "short2" + + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: func(securityConfiguration *config.SecurityConfiguration) { + securityConfiguration.OperationNameLengthLimit = maxLength + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + resPost, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: "query " + query1Name + " { employees { id } } query " + query2Name + " { employees { id } }", + }) + require.NoError(t, err) + require.JSONEq(t, `{"data":{"employees":[{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":7},{"id":8},{"id":10},{"id":11},{"id":12}]}}`, resPost.Body) + require.Equal(t, http.StatusOK, resPost.Response.StatusCode) + + resGet, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + Query: "query " + query1Name + " { employees { id } } query " + query2Name + " { employees { id } }", + }) + require.NoError(t, err) + require.JSONEq(t, `{"data":{"employees":[{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":7},{"id":8},{"id":10},{"id":11},{"id":12}]}}`, resGet.Body) + require.Equal(t, http.StatusOK, resGet.Response.StatusCode) + }) + }) + + t.Run("with large queries with max length of 0 where the validation is not enabled", func(t *testing.T) { + t.Parallel() + + maxLength := 0 + query1Name := "longlonglonglonglonglonglonglonglonglong1" + query2Name := "short2" + + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: func(securityConfiguration *config.SecurityConfiguration) { + securityConfiguration.OperationNameLengthLimit = maxLength + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + resPost, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: "query " + query1Name + " { employees { id } } query " + query2Name + " { employees { id } }", + }) + require.NoError(t, err) + require.JSONEq(t, `{"data":{"employees":[{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":7},{"id":8},{"id":10},{"id":11},{"id":12}]}}`, resPost.Body) + require.Equal(t, http.StatusOK, resPost.Response.StatusCode) + + resGet, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + Query: "query " + query1Name + " { employees { id } } query " + query2Name + " { employees { id } }", + }) + require.NoError(t, err) + require.JSONEq(t, `{"data":{"employees":[{"id":1},{"id":2},{"id":3},{"id":4},{"id":5},{"id":7},{"id":8},{"id":10},{"id":11},{"id":12}]}}`, resGet.Body) + require.Equal(t, http.StatusOK, resGet.Response.StatusCode) + }) + }) + + // In case of introspection checks, we could potentially early return + t.Run("with multiple queries with introspection disabled", func(t *testing.T) { + t.Parallel() + + maxLength := 6 + query1Name := "longquery" + query2Name := "short2" + + testenv.Run(t, &testenv.Config{ + ModifySecurityConfiguration: func(securityConfiguration *config.SecurityConfiguration) { + securityConfiguration.OperationNameLengthLimit = maxLength + }, + RouterOptions: []core.Option{ + core.WithIntrospection(false), + }, + }, func(t *testing.T, xEnv *testenv.Environment) { + expectedErrorMessage := fmt.Sprintf(`{"errors":[{"message":"operation name of length %d exceeds max length of %d"}]}`, len(query1Name), maxLength) + + resPost, err := xEnv.MakeGraphQLRequest(testenv.GraphQLRequest{ + Query: "query " + query1Name + " { __schema { __typename } } query " + query2Name + " { employees { id } }", + }) + require.NoError(t, err) + require.JSONEq(t, expectedErrorMessage, resPost.Body) + require.Equal(t, http.StatusBadRequest, resPost.Response.StatusCode) + + resGet, err := xEnv.MakeGraphQLRequestOverGET(testenv.GraphQLRequest{ + Query: "query " + query1Name + " { __schema { __typename } } query " + query2Name + " { employees { id } }", + }) + require.NoError(t, err) + require.JSONEq(t, expectedErrorMessage, resGet.Body) + require.Equal(t, http.StatusBadRequest, resGet.Response.StatusCode) + }) + }) + }) +} diff --git a/router/core/graph_server.go b/router/core/graph_server.go index 870d274f3d..87b02afd2d 100644 --- a/router/core/graph_server.go +++ b/router/core/graph_server.go @@ -1207,6 +1207,7 @@ func (s *graphServer) buildGraphMux( MaxDepth: s.Config.securityConfiguration.ParserLimits.ApproximateDepthLimit, MaxFields: s.Config.securityConfiguration.ParserLimits.TotalFieldsLimit, }, + OperationNameLengthLimit: s.securityConfiguration.OperationNameLengthLimit, ApolloCompatibilityFlags: s.apolloCompatibilityFlags, ApolloRouterCompatibilityFlags: s.apolloRouterCompatibilityFlags, DisableExposingVariablesContentOnValidationError: s.engineExecutionConfiguration.DisableExposingVariablesContentOnValidationError, diff --git a/router/core/graphql_prehandler.go b/router/core/graphql_prehandler.go index 1592df8cf0..bd8c38d719 100644 --- a/router/core/graphql_prehandler.go +++ b/router/core/graphql_prehandler.go @@ -529,6 +529,15 @@ func (h *PreHandler) handleOperation(req *http.Request, variablesParser *astjson } } + if operationKit.isOperationNameLengthLimitExceeded(operationKit.parsedOperation.Request.OperationName) { + return &httpGraphqlError{ + message: fmt.Sprintf("operation name of length %d exceeds max length of %d", + len(operationKit.parsedOperation.Request.OperationName), + operationKit.operationProcessor.operationNameLengthLimit), + statusCode: http.StatusBadRequest, + } + } + // Compute the operation sha256 hash as soon as possible for observability reasons if h.shouldComputeOperationSha256(operationKit) { if err := operationKit.ComputeOperationSha256(); err != nil { diff --git a/router/core/operation_processor.go b/router/core/operation_processor.go index 06c0a8264f..0033b9de72 100644 --- a/router/core/operation_processor.go +++ b/router/core/operation_processor.go @@ -116,6 +116,7 @@ type OperationProcessorOptions struct { DisableExposingVariablesContentOnValidationError bool ComplexityLimits *config.ComplexityLimits ParserTokenizerLimits astparser.TokenizerLimits + OperationNameLengthLimit int } // OperationProcessor provides shared resources to the parseKit and OperationKit. @@ -131,6 +132,7 @@ type OperationProcessor struct { parseKitOptions *parseKitOptions complexityLimits *config.ComplexityLimits parserTokenizerLimits astparser.TokenizerLimits + operationNameLengthLimit int } // parseKit is a helper struct to parse, normalize and validate operations @@ -487,6 +489,14 @@ func (o *OperationKit) isIntrospectionQuery() (result bool, err error) { ref := possibleOperationDefinitionRefs[i] name := o.kit.doc.OperationDefinitionNameString(ref) + if o.isOperationNameLengthLimitExceeded(name) { + return false, &httpGraphqlError{ + message: fmt.Sprintf("operation name of length %d exceeds max length of %d", + len(name), o.operationProcessor.operationNameLengthLimit), + statusCode: http.StatusBadRequest, + } + } + if o.parsedOperation.Request.OperationName == name { operationDefinitionRef = ref break @@ -527,6 +537,13 @@ func (o *OperationKit) isIntrospectionQuery() (result bool, err error) { return false, nil } +func (o *OperationKit) isOperationNameLengthLimitExceeded(operationName string) bool { + if o.operationProcessor.operationNameLengthLimit == 0 { + return false + } + return len(operationName) > o.operationProcessor.operationNameLengthLimit +} + // Parse parses the operation, populates the document and set the operation type. // UnmarshalOperationFromBody must be called before calling this method. func (o *OperationKit) Parse() error { @@ -560,6 +577,11 @@ func (o *OperationKit) Parse() error { isIntrospection, err := o.isIntrospectionQuery() if err != nil { + var httpGqlError *httpGraphqlError + if errors.As(err, &httpGqlError) { + return httpGqlError + } + return &httpGraphqlError{ message: "could not determine if operation was an introspection query", statusCode: http.StatusOK, @@ -582,6 +604,7 @@ func (o *OperationKit) Parse() error { o.kit.numOperations++ ref := o.kit.doc.RootNodes[i].Ref name := string(o.kit.doc.OperationDefinitionNameBytes(ref)) + if len(name) == 0 { anonymousOperationCount++ if anonymousOperationDefinitionRef == -1 { @@ -589,6 +612,15 @@ func (o *OperationKit) Parse() error { } continue } + + if o.isOperationNameLengthLimitExceeded(name) { + return &httpGraphqlError{ + message: fmt.Sprintf("operation name of length %d exceeds max length of %d", + len(name), o.operationProcessor.operationNameLengthLimit), + statusCode: http.StatusBadRequest, + } + } + if o.parsedOperation.Request.OperationName == "" { o.operationDefinitionRef = ref o.originalOperationNameRef = o.kit.doc.OperationDefinitions[ref].Name @@ -1256,6 +1288,7 @@ func NewOperationProcessor(opts OperationProcessorOptions) *OperationProcessor { parseKitSemaphore: make(chan int, opts.ParseKitPoolSize), introspectionEnabled: opts.IntrospectionEnabled, parserTokenizerLimits: opts.ParserTokenizerLimits, + operationNameLengthLimit: opts.OperationNameLengthLimit, complexityLimits: opts.ComplexityLimits, parseKitOptions: &parseKitOptions{ apolloCompatibilityFlags: opts.ApolloCompatibilityFlags, diff --git a/router/pkg/config/config.go b/router/pkg/config/config.go index c51e2810f5..afb6eb1a1a 100644 --- a/router/pkg/config/config.go +++ b/router/pkg/config/config.go @@ -408,6 +408,7 @@ type SecurityConfiguration struct { ComplexityLimits *ComplexityLimits `yaml:"complexity_limits"` DepthLimit *QueryDepthConfiguration `yaml:"depth_limit"` ParserLimits ParserLimitsConfiguration `yaml:"parser_limits"` + OperationNameLengthLimit int `yaml:"operation_name_length_limit" envDefault:"512" env:"SECURITY_OPERATION_NAME_LENGTH_LIMIT"` // 0 is disabled } type ParserLimitsConfiguration struct { diff --git a/router/pkg/config/config.schema.json b/router/pkg/config/config.schema.json index 54b2635627..048376349a 100644 --- a/router/pkg/config/config.schema.json +++ b/router/pkg/config/config.schema.json @@ -2457,6 +2457,12 @@ } } }, + "operation_name_length_limit": { + "type": "integer", + "description": "The maximum allowed length of the operation name, 0 allows any length.", + "default": "512", + "minimum": 0 + }, "depth_limit": { "type": "object", "description": "DEPRECATED (move to complexity_limits.depth): The configuration for adding a max depth limit for query (how many nested levels you can have in a query).", diff --git a/router/pkg/config/fixtures/full.yaml b/router/pkg/config/fixtures/full.yaml index e7b9c882c6..e8f7cd6ec1 100644 --- a/router/pkg/config/fixtures/full.yaml +++ b/router/pkg/config/fixtures/full.yaml @@ -427,6 +427,7 @@ security: enabled: true limit: 4 ignore_persisted_operations: true + operation_name_length_limit: 2000 persisted_operations: safelist: enabled: true diff --git a/router/pkg/config/testdata/config_defaults.json b/router/pkg/config/testdata/config_defaults.json index 506275c0b7..b045f5b538 100644 --- a/router/pkg/config/testdata/config_defaults.json +++ b/router/pkg/config/testdata/config_defaults.json @@ -315,7 +315,8 @@ "ParserLimits": { "ApproximateDepthLimit": 100, "TotalFieldsLimit": 500 - } + }, + "OperationNameLengthLimit": 512 }, "EngineExecutionConfiguration": { "Debug": { diff --git a/router/pkg/config/testdata/config_full.json b/router/pkg/config/testdata/config_full.json index 4426798c21..3bb087c26f 100644 --- a/router/pkg/config/testdata/config_full.json +++ b/router/pkg/config/testdata/config_full.json @@ -674,7 +674,8 @@ "ParserLimits": { "ApproximateDepthLimit": 100, "TotalFieldsLimit": 500 - } + }, + "OperationNameLengthLimit": 2000 }, "EngineExecutionConfiguration": { "Debug": {