This is an automated email from the ASF dual-hosted git repository.

kezhenxu94 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/skywalking.git


The following commit(s) were added to refs/heads/master by this push:
     new 6486b13  Add complexity limitation for GraphQL query to avoid 
malicious query (#8703)
6486b13 is described below

commit 6486b13a78f5edcac7932fb80d28d84e4f910c30
Author: kezhenxu94 <[email protected]>
AuthorDate: Fri Mar 18 15:38:23 2022 +0800

    Add complexity limitation for GraphQL query to avoid malicious query (#8703)
---
 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..1fcc250 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 | 100 |
 | 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..35f1f7a 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 = 100;
 }
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..a401e11 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:100}
 
 alarm:
   selector: ${SW_ALARM:default}

Reply via email to