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}