This is an automated email from the ASF dual-hosted git repository. kezhenxu94 pushed a commit to branch kezhenxu94/issue8694 in repository https://gitbox.apache.org/repos/asf/skywalking.git
commit 27b89f8f2763d413f20ef99ab632594b090061b0 Author: kezhenxu94 <[email protected]> AuthorDate: Fri Mar 18 13:19:41 2022 +0800 Add complexity limitation for GraphQL query to avoid malicious query --- CHANGES.md | 1 + docs/en/setup/backend/configuration-vocabulary.md | 1 + .../oap/query/graphql/GraphQLQueryConfig.java | 1 + .../oap/query/graphql/GraphQLQueryHandler.java | 26 ++++++++++++++++++---- .../oap/query/graphql/GraphQLQueryProvider.java | 3 ++- .../src/main/resources/application.yml | 3 +++ 6 files changed, 30 insertions(+), 5 deletions(-) diff --git a/CHANGES.md b/CHANGES.md index c7bbe28..52ba41e 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -119,6 +119,7 @@ Release Notes. requirement of using HashMap to communicate between data object and database native structure. * [Breaking Change] Break all existing 3rd-party storage extensions. * Remove hard requirement of BASE64 encoding for binary field. +* Add complexity limitation for GraphQL query to avoid malicious query. #### UI diff --git a/docs/en/setup/backend/configuration-vocabulary.md b/docs/en/setup/backend/configuration-vocabulary.md index 76679e3..a3e6161 100644 --- a/docs/en/setup/backend/configuration-vocabulary.md +++ b/docs/en/setup/backend/configuration-vocabulary.md @@ -230,6 +230,7 @@ core|default|role|Option values: `Mixed/Receiver/Aggregator`. **Receiver** mode | - | - | sampleRate | Sampling rate for receiving trace. Precise to 1/10000. 10000 means sampling rate of 100% by default. | SW_RECEIVER_BROWSER_SAMPLE_RATE | 10000 | | query | graphql | - | GraphQL query implementation. | - | | - | - | enableLogTestTool | Enable the log testing API to test the LAL. **NOTE**: This API evaluates untrusted code on the OAP server. A malicious script can do significant damage (steal keys and secrets, remove files and directories, install malware, etc). As such, please enable this API only when you completely trust your users. | SW_QUERY_GRAPHQL_ENABLE_LOG_TEST_TOOL | false | +| - | - | maxQueryComplexity | Maximum complexity allowed for the GraphQL query that can be used to abort a query if the total number of data fields queried exceeds the defined threshold. | SW_QUERY_MAX_QUERY_COMPLEXITY | 30 | | alarm | default | - | Read [alarm doc](backend-alarm.md) for more details. | - | | telemetry | - | - | Read [telemetry doc](backend-telemetry.md) for more details. | - | | - | none| - | No op implementation. | - | diff --git a/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryConfig.java b/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryConfig.java index 4e5d129..8d520b1 100644 --- a/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryConfig.java +++ b/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryConfig.java @@ -29,4 +29,5 @@ import org.apache.skywalking.oap.server.library.module.ModuleConfig; @Setter public class GraphQLQueryConfig extends ModuleConfig { private boolean enableLogTestTool; + private int maxQueryComplexity; } diff --git a/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryHandler.java b/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryHandler.java index 449e815..0eb46f4 100644 --- a/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryHandler.java +++ b/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryHandler.java @@ -24,19 +24,37 @@ import com.linecorp.armeria.server.ServiceRequestContext; import com.linecorp.armeria.server.annotation.Blocking; import com.linecorp.armeria.server.annotation.Post; import com.linecorp.armeria.server.graphql.GraphqlService; +import graphql.analysis.MaxQueryComplexityInstrumentation; import graphql.schema.GraphQLSchema; +import lombok.extern.slf4j.Slf4j; +@Slf4j public class GraphQLQueryHandler { private final GraphqlService graphqlService; - public GraphQLQueryHandler(final GraphQLSchema schema) { - graphqlService = GraphqlService.builder().schema(schema).build(); + public GraphQLQueryHandler( + final GraphQLQueryConfig config, + final GraphQLSchema schema) { + final int allowedComplexity = config.getMaxQueryComplexity(); + graphqlService = + GraphqlService + .builder() + .schema(schema) + .instrumentation(new MaxQueryComplexityInstrumentation(allowedComplexity, info -> { + log.warn( + "Aborting query because it's too complex, maximum allowed is [{}] but was [{}]", + allowedComplexity, + info.getComplexity()); + return true; + })) + .build(); } @Blocking @Post("/graphql") - public HttpResponse graphql(ServiceRequestContext ctx, - HttpRequest req) throws Exception { + public HttpResponse graphql( + final ServiceRequestContext ctx, + final HttpRequest req) throws Exception { return graphqlService.serve(ctx, req); } } diff --git a/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryProvider.java b/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryProvider.java index 2f48e00..8b06511 100644 --- a/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryProvider.java +++ b/oap-server/server-query-plugin/query-graphql-plugin/src/main/java/org/apache/skywalking/oap/query/graphql/GraphQLQueryProvider.java @@ -122,7 +122,8 @@ public class GraphQLQueryProvider extends ModuleProvider { HTTPHandlerRegister service = getManager().find(CoreModule.NAME) .provider() .getService(HTTPHandlerRegister.class); - service.addHandler(new GraphQLQueryHandler(schemaBuilder.build().makeExecutableSchema())); + service.addHandler( + new GraphQLQueryHandler(config, schemaBuilder.build().makeExecutableSchema())); } @Override diff --git a/oap-server/server-starter/src/main/resources/application.yml b/oap-server/server-starter/src/main/resources/application.yml index ce4d5b1..d1dac86 100755 --- a/oap-server/server-starter/src/main/resources/application.yml +++ b/oap-server/server-starter/src/main/resources/application.yml @@ -413,6 +413,9 @@ query: # A malicious script can do significant damage (steal keys and secrets, remove files and directories, install malware, etc). # As such, please enable this API only when you completely trust your users. enableLogTestTool: ${SW_QUERY_GRAPHQL_ENABLE_LOG_TEST_TOOL:false} + # Maximum complexity allowed for the GraphQL query that can be used to + # abort a query if the total number of data fields queried exceeds the defined threshold. + maxQueryComplexity: ${SW_QUERY_MAX_QUERY_COMPLEXITY:30} alarm: selector: ${SW_ALARM:default}
