From b361146acf3d11996fc3905d2d66199faf3700fd Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Sat, 16 Mar 2024 12:26:23 +1100 Subject: [PATCH 1/5] This provides a GoodFaithIntrospectionInstrumentation option --- ...GoodFaithIntrospectionInstrumentation.java | 87 +++++++++++++++++++ ...ithIntrospectionInstrumentationTest.groovy | 67 ++++++++++++++ 2 files changed, 154 insertions(+) create mode 100644 src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java create mode 100644 src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy diff --git a/src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java b/src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java new file mode 100644 index 0000000000..39841f2159 --- /dev/null +++ b/src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java @@ -0,0 +1,87 @@ +package graphql.introspection; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableListMultimap; +import graphql.PublicApi; +import graphql.execution.AbortExecutionException; +import graphql.execution.ExecutionContext; +import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext; +import graphql.execution.instrumentation.InstrumentationState; +import graphql.execution.instrumentation.SimplePerformantInstrumentation; +import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters; +import graphql.normalized.ExecutableNormalizedField; +import graphql.normalized.ExecutableNormalizedOperation; +import graphql.schema.FieldCoordinates; +import org.jetbrains.annotations.Nullable; + +import java.util.List; +import java.util.Map; + +import static graphql.schema.FieldCoordinates.coordinates; + +/** + * This {@link graphql.execution.instrumentation.Instrumentation} ensure that a submitted introspection query is done in + * good faith. + *

+ * There are attack vectors where a crafted introspection query can cause the engine to spend too much time + * producing introspection data. This is especially true on large schemas with lots of types and fields. + *

+ * Schemas form a cyclic graph and hence it's possible to send in introspection queries that can reference those cycles + * and in large schemas this can be expensive and perhaps a "denial of service". + *

+ * This instrumentation only allows one __schema field or one __type field to be present, and it does not allow the `__Type` fields + * to form a cycle, i.e., that can only be present once. This allows the standard and common introspection queries to work + * so tooling such as graphiql can work. + */ +@PublicApi +public class GoodFaithIntrospectionInstrumentation extends SimplePerformantInstrumentation { + + private static final Map ALLOWED_FIELD_INSTANCES = Map.of( + coordinates("Query", "__schema"), 1 + , coordinates("Query", "__type"), 1 + + , coordinates("__Type", "fields"), 1 + , coordinates("__Type", "inputFields"), 1 + , coordinates("__Type", "interfaces"), 1 + , coordinates("__Type", "possibleTypes"), 1 + ); + + @Override + public @Nullable ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { + + // We check first to see if there are any introspection fields present on the top level field set + // if there isn't, then there is nothing to check. This ensures we are performant. + List topLevelFieldName = parameters.getExecutionStrategyParameters().getFields().getKeys(); + boolean weHaveIntrospectionFields = false; + for (String fieldName : topLevelFieldName) { + if (Introspection.SchemaMetaFieldDef.getName().equals(fieldName) || Introspection.TypeMetaFieldDef.getName().equals(fieldName)) { + weHaveIntrospectionFields = true; + break; + } + } + if (weHaveIntrospectionFields) { + ensureTheyAreInGoodFaith(parameters.getExecutionContext()); + } + return ExecutionStrategyInstrumentationContext.NOOP; + } + + private void ensureTheyAreInGoodFaith(ExecutionContext executionContext) { + ExecutableNormalizedOperation operation = executionContext.getNormalizedQueryTree().get(); + ImmutableListMultimap coordinatesToENFs = operation.getCoordinatesToNormalizedFields(); + for (Map.Entry entry : ALLOWED_FIELD_INSTANCES.entrySet()) { + FieldCoordinates coordinates = entry.getKey(); + Integer allowSize = entry.getValue(); + ImmutableList normalizedFields = coordinatesToENFs.get(coordinates); + if (normalizedFields.size() > allowSize) { + throw new BadFaithIntrospectionAbortExecutionException(coordinates.toString()); + } + } + } + + @PublicApi + public static class BadFaithIntrospectionAbortExecutionException extends AbortExecutionException { + public BadFaithIntrospectionAbortExecutionException(String qualifiedField) { + super(String.format("This request is not asking for introspection in good faith - %s is present too often!", qualifiedField)); + } + } +} diff --git a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy new file mode 100644 index 0000000000..ad0d057a5f --- /dev/null +++ b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy @@ -0,0 +1,67 @@ +package graphql.introspection + +import graphql.ExecutionResult +import graphql.TestUtil +import spock.lang.Specification + +class GoodFaithIntrospectionInstrumentationTest extends Specification { + + def graphql = TestUtil.graphQL("type Query { f : String }").instrumentation(new GoodFaithIntrospectionInstrumentation()).build() + + def "test asking for introspection in good faith"() { + + when: + ExecutionResult er = graphql.execute(IntrospectionQuery.INTROSPECTION_QUERY) + then: + er.errors.isEmpty() + } + + def "test asking for introspection in bad faith"() { + + when: + ExecutionResult er = graphql.execute(query) + then: + !er.errors.isEmpty() + er.errors[0] instanceof GoodFaithIntrospectionInstrumentation.BadFaithIntrospectionAbortExecutionException + + where: + query | _ + // a case for __Type interfaces + """ query badActor { + __schema { types { interfaces { fields { type { interfaces { name } } } } } } + } + """ | _ + // a case for __Type inputFields + """ query badActor { + __schema { types { inputFields { type { inputFields { name }}}}} + } + """ | _ + // a case for __Type possibleTypes + """ query badActor { + __schema { types { inputFields { type { inputFields { name }}}}} + } + """ | _ + // a case leading from __InputValue + """ query badActor { + __schema { types { fields { args { type { name fields { name }}}}}} + } + """ | _ + // a case leading from __Field + """ query badActor { + __schema { types { fields { type { name fields { name }}}}} + } + """ | _ + // a case for __type + """ query badActor { + __type(name : "t") { name } + alias1 : __type(name : "t1") { name } + } + """ | _ + // a case for schema repeated - dont ask twice + """ query badActor { + __schema { types { name} } + alias1 : __schema { types { name} } + } + """ | _ + } +} From d719d4fd5786adec3e4f07313d2bdcf5e853110d Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Sat, 16 Mar 2024 13:12:41 +1100 Subject: [PATCH 2/5] This provides a GoodFaithIntrospectionInstrumentation option - longer attack test --- ...ithIntrospectionInstrumentationTest.groovy | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy index ad0d057a5f..318af0649c 100644 --- a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy +++ b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy @@ -25,43 +25,47 @@ class GoodFaithIntrospectionInstrumentationTest extends Specification { er.errors[0] instanceof GoodFaithIntrospectionInstrumentation.BadFaithIntrospectionAbortExecutionException where: - query | _ + query | _ + // long attack + """ + query badActor{__schema{types{fields{type{fields{type{fields{type{fields{type{name}}}}}}}}}}} + """ | _ // a case for __Type interfaces """ query badActor { __schema { types { interfaces { fields { type { interfaces { name } } } } } } } - """ | _ + """ | _ // a case for __Type inputFields """ query badActor { __schema { types { inputFields { type { inputFields { name }}}}} } - """ | _ + """ | _ // a case for __Type possibleTypes """ query badActor { __schema { types { inputFields { type { inputFields { name }}}}} } - """ | _ + """ | _ // a case leading from __InputValue """ query badActor { __schema { types { fields { args { type { name fields { name }}}}}} } - """ | _ + """ | _ // a case leading from __Field """ query badActor { __schema { types { fields { type { name fields { name }}}}} } - """ | _ + """ | _ // a case for __type """ query badActor { __type(name : "t") { name } alias1 : __type(name : "t1") { name } } - """ | _ + """ | _ // a case for schema repeated - dont ask twice """ query badActor { __schema { types { name} } alias1 : __schema { types { name} } } - """ | _ + """ | _ } } From d304767e32d234b37d2958098c12c94536ad723e Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Sat, 16 Mar 2024 15:17:33 +1100 Subject: [PATCH 3/5] This provides a GoodFaithIntrospectionInstrumentation option - now with builder to allow you to specify extras if you want --- ...GoodFaithIntrospectionInstrumentation.java | 63 ++++++++++++++- ...ithIntrospectionInstrumentationTest.groovy | 81 +++++++++++++++++++ 2 files changed, 143 insertions(+), 1 deletion(-) diff --git a/src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java b/src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java index 39841f2159..d95567cd9c 100644 --- a/src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java +++ b/src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java @@ -14,6 +14,7 @@ import graphql.schema.FieldCoordinates; import org.jetbrains.annotations.Nullable; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -45,6 +46,15 @@ public class GoodFaithIntrospectionInstrumentation extends SimplePerformantInstr , coordinates("__Type", "interfaces"), 1 , coordinates("__Type", "possibleTypes"), 1 ); + private final Map allowFieldInstances; + + public GoodFaithIntrospectionInstrumentation() { + this(ALLOWED_FIELD_INSTANCES); + } + + private GoodFaithIntrospectionInstrumentation(Map allowFieldInstances) { + this.allowFieldInstances = allowFieldInstances; + } @Override public @Nullable ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { @@ -68,7 +78,7 @@ public class GoodFaithIntrospectionInstrumentation extends SimplePerformantInstr private void ensureTheyAreInGoodFaith(ExecutionContext executionContext) { ExecutableNormalizedOperation operation = executionContext.getNormalizedQueryTree().get(); ImmutableListMultimap coordinatesToENFs = operation.getCoordinatesToNormalizedFields(); - for (Map.Entry entry : ALLOWED_FIELD_INSTANCES.entrySet()) { + for (Map.Entry entry : allowFieldInstances.entrySet()) { FieldCoordinates coordinates = entry.getKey(); Integer allowSize = entry.getValue(); ImmutableList normalizedFields = coordinatesToENFs.get(coordinates); @@ -84,4 +94,55 @@ public BadFaithIntrospectionAbortExecutionException(String qualifiedField) { super(String.format("This request is not asking for introspection in good faith - %s is present too often!", qualifiedField)); } } + + public static Builder newGoodFaithIntrospection() { + return new Builder(); + } + + public static class Builder { + + + private final Map allowFieldInstances = new LinkedHashMap<>(ALLOWED_FIELD_INSTANCES); + + /** + * This allows you to set how many __type(name : "x) field instances are allowed in an introspection query + * + * @param maxInstances the number allowed + * + * @return this builder + */ + public Builder maximumUnderscoreTypeInstances(int maxInstances) { + allowFieldInstances.put(coordinates("Query", "__type"), maxInstances); + return this; + } + + /** + * This allows you to set how many __schema field instances are allowed in an introspection query + * + * @param maxInstances the number allowed + * + * @return this builder + */ + public Builder maximumUnderscoreSchemaInstances(int maxInstances) { + allowFieldInstances.put(coordinates("Query", "__schema"), maxInstances); + return this; + } + + /** + * This allows you to set how many qualified field instances are allowed in an introspection query + * + * @param coordinates - the qualified field name + * @param maxInstances the number allowed + * + * @return this builder + */ + public Builder maximumFieldInstances(FieldCoordinates coordinates, int maxInstances) { + allowFieldInstances.put(coordinates, maxInstances); + return this; + } + + public GoodFaithIntrospectionInstrumentation build() { + return new GoodFaithIntrospectionInstrumentation(allowFieldInstances); + } + } } diff --git a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy index 318af0649c..1450aa456c 100644 --- a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy +++ b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy @@ -2,6 +2,7 @@ package graphql.introspection import graphql.ExecutionResult import graphql.TestUtil +import graphql.schema.FieldCoordinates import spock.lang.Specification class GoodFaithIntrospectionInstrumentationTest extends Specification { @@ -68,4 +69,84 @@ class GoodFaithIntrospectionInstrumentationTest extends Specification { } """ | _ } + + def "test builder"() { + GoodFaithIntrospectionInstrumentation goodFaithIntrospectionInstrumentation = GoodFaithIntrospectionInstrumentation.newGoodFaithIntrospection() + .maximumUnderscoreSchemaInstances(2) + .maximumUnderscoreTypeInstances(3) + .maximumFieldInstances(FieldCoordinates.coordinates("__Type", "fields"), 2) + .build() + + def graphql = TestUtil.graphQL("type Query { f : String }").instrumentation(goodFaithIntrospectionInstrumentation).build() + + when: + def er = graphql.execute(IntrospectionQuery.INTROSPECTION_QUERY) + then: + er.errors.isEmpty() + + when: + er = graphql.execute(""" + query ok { + __schema { types { name } } + alias1: __schema { types { name } } + } + """) + then: + er.errors.isEmpty() + + when: + er = graphql.execute(""" + query ok { + __schema { types { name } } + alias1: __schema { types { name } } + alias2: __schema { types { name } } + } + """) + then: + !er.errors.isEmpty() + er.errors[0] instanceof GoodFaithIntrospectionInstrumentation.BadFaithIntrospectionAbortExecutionException + + when: + er = graphql.execute(""" + query ok { + __type(name : "Query") { name } + alias1 : __type(name : "Query") { name } + alias2 : __type(name : "Query") { name } + } + """) + then: + er.errors.isEmpty() + + when: + er = graphql.execute(""" + query ok { + __type(name : "Query") { name } + alias1 : __type(name : "Query") { name } + alias2 : __type(name : "Query") { name } + alias3 : __type(name : "Query") { name } + } + """) + then: + !er.errors.isEmpty() + er.errors[0] instanceof GoodFaithIntrospectionInstrumentation.BadFaithIntrospectionAbortExecutionException + + when: + er = graphql.execute(""" + query ok { + __schema { types { fields { type { fields { name }}}}} + } + """) + then: + er.errors.isEmpty() + + when: + er = graphql.execute(""" + query ok { + __schema { types { fields { type { fields { type { fields { name }}}}}}} + } + """) + then: + !er.errors.isEmpty() + er.errors[0] instanceof GoodFaithIntrospectionInstrumentation.BadFaithIntrospectionAbortExecutionException + } } From 7e5b45b5068a57de3e76df7954b6c1f481063ac9 Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Mon, 18 Mar 2024 13:28:16 +1100 Subject: [PATCH 4/5] GoodFaithIntrospection is now on by defaut and always a available - no longer an instrumentation --- .../execution/AsyncExecutionStrategy.java | 2 +- .../AsyncSerialExecutionStrategy.java | 2 +- .../introspection/GoodFaithIntrospection.java | 122 +++++++++++++++ ...GoodFaithIntrospectionInstrumentation.java | 148 ------------------ .../graphql/introspection/Introspection.java | 10 +- ...ithIntrospectionInstrumentationTest.groovy | 99 +++++------- 6 files changed, 172 insertions(+), 211 deletions(-) create mode 100644 src/main/java/graphql/introspection/GoodFaithIntrospection.java delete mode 100644 src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java diff --git a/src/main/java/graphql/execution/AsyncExecutionStrategy.java b/src/main/java/graphql/execution/AsyncExecutionStrategy.java index 30c0cd45dd..1e7bc7c79f 100644 --- a/src/main/java/graphql/execution/AsyncExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncExecutionStrategy.java @@ -48,7 +48,7 @@ public CompletableFuture execute(ExecutionContext executionCont MergedSelectionSet fields = parameters.getFields(); List fieldNames = fields.getKeys(); - Optional isNotSensible = Introspection.isIntrospectionSensible(executionContext.getGraphQLContext(),fields); + Optional isNotSensible = Introspection.isIntrospectionSensible(fields, executionContext); if (isNotSensible.isPresent()) { return CompletableFuture.completedFuture(isNotSensible.get()); } diff --git a/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java b/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java index 3938c54d75..6f64b8cd8c 100644 --- a/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java +++ b/src/main/java/graphql/execution/AsyncSerialExecutionStrategy.java @@ -44,7 +44,7 @@ public CompletableFuture execute(ExecutionContext executionCont // this is highly unlikely since Mutations cant do introspection BUT in theory someone could make the query strategy this code // so belts and braces - Optional isNotSensible = Introspection.isIntrospectionSensible(executionContext.getGraphQLContext(), fields); + Optional isNotSensible = Introspection.isIntrospectionSensible(fields, executionContext); if (isNotSensible.isPresent()) { return CompletableFuture.completedFuture(isNotSensible.get()); } diff --git a/src/main/java/graphql/introspection/GoodFaithIntrospection.java b/src/main/java/graphql/introspection/GoodFaithIntrospection.java new file mode 100644 index 0000000000..7e853016ac --- /dev/null +++ b/src/main/java/graphql/introspection/GoodFaithIntrospection.java @@ -0,0 +1,122 @@ +package graphql.introspection; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableListMultimap; +import graphql.ErrorClassification; +import graphql.ExecutionResult; +import graphql.GraphQLContext; +import graphql.GraphQLError; +import graphql.PublicApi; +import graphql.execution.ExecutionContext; +import graphql.language.SourceLocation; +import graphql.normalized.ExecutableNormalizedField; +import graphql.normalized.ExecutableNormalizedOperation; +import graphql.schema.FieldCoordinates; + +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.concurrent.atomic.AtomicBoolean; + +import static graphql.schema.FieldCoordinates.coordinates; + +/** + * This {@link graphql.execution.instrumentation.Instrumentation} ensure that a submitted introspection query is done in + * good faith. + *

+ * There are attack vectors where a crafted introspection query can cause the engine to spend too much time + * producing introspection data. This is especially true on large schemas with lots of types and fields. + *

+ * Schemas form a cyclic graph and hence it's possible to send in introspection queries that can reference those cycles + * and in large schemas this can be expensive and perhaps a "denial of service". + *

+ * This instrumentation only allows one __schema field or one __type field to be present, and it does not allow the `__Type` fields + * to form a cycle, i.e., that can only be present once. This allows the standard and common introspection queries to work + * so tooling such as graphiql can work. + */ +@PublicApi +public class GoodFaithIntrospection { + + /** + * Placing a boolean value under this key in the per request {@link GraphQLContext} will enable + * or disable Good Faith Introspection on that request. + */ + public static final String GOOD_FAITH_INTROSPECTION_DISABLED = "GOOD_FAITH_INTROSPECTION_DISABLED"; + + private static final AtomicBoolean ENABLED_STATE = new AtomicBoolean(true); + + /** + * @return true if good faith introspection is enabled + */ + public static boolean isEnabledJvmWide() { + return ENABLED_STATE.get(); + } + + /** + * This allows you to disable good faith introspection, which is on by default. + * + * @param flag the desired state + * + * @return the previous state + */ + public static boolean enabledJvmWide(boolean flag) { + return ENABLED_STATE.getAndSet(flag); + } + + private static final Map ALLOWED_FIELD_INSTANCES = Map.of( + coordinates("Query", "__schema"), 1 + , coordinates("Query", "__type"), 1 + + , coordinates("__Type", "fields"), 1 + , coordinates("__Type", "inputFields"), 1 + , coordinates("__Type", "interfaces"), 1 + , coordinates("__Type", "possibleTypes"), 1 + ); + + public static Optional checkIntrospection(ExecutionContext executionContext) { + if (isIntrospectionEnabled(executionContext.getGraphQLContext())) { + ExecutableNormalizedOperation operation = executionContext.getNormalizedQueryTree().get(); + ImmutableListMultimap coordinatesToENFs = operation.getCoordinatesToNormalizedFields(); + for (Map.Entry entry : ALLOWED_FIELD_INSTANCES.entrySet()) { + FieldCoordinates coordinates = entry.getKey(); + Integer allowSize = entry.getValue(); + ImmutableList normalizedFields = coordinatesToENFs.get(coordinates); + if (normalizedFields.size() > allowSize) { + BadFaithIntrospectionError error = new BadFaithIntrospectionError(coordinates.toString()); + return Optional.of(ExecutionResult.newExecutionResult().addError(error).build()); + } + } + } + return Optional.empty(); + } + + private static boolean isIntrospectionEnabled(GraphQLContext graphQlContext) { + if (!isEnabledJvmWide()) { + return false; + } + return !graphQlContext.getOrDefault(GOOD_FAITH_INTROSPECTION_DISABLED, false); + } + + public static class BadFaithIntrospectionError implements GraphQLError { + private final String message; + + public BadFaithIntrospectionError(String qualifiedField) { + this.message = String.format("This request is not asking for introspection in good faith - %s is present too often!", qualifiedField); + } + + @Override + public String getMessage() { + return message; + } + + @Override + public ErrorClassification getErrorType() { + return ErrorClassification.errorClassification("BadFaithIntrospection"); + } + + @Override + public List getLocations() { + return null; + } + } +} diff --git a/src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java b/src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java deleted file mode 100644 index d95567cd9c..0000000000 --- a/src/main/java/graphql/introspection/GoodFaithIntrospectionInstrumentation.java +++ /dev/null @@ -1,148 +0,0 @@ -package graphql.introspection; - -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableListMultimap; -import graphql.PublicApi; -import graphql.execution.AbortExecutionException; -import graphql.execution.ExecutionContext; -import graphql.execution.instrumentation.ExecutionStrategyInstrumentationContext; -import graphql.execution.instrumentation.InstrumentationState; -import graphql.execution.instrumentation.SimplePerformantInstrumentation; -import graphql.execution.instrumentation.parameters.InstrumentationExecutionStrategyParameters; -import graphql.normalized.ExecutableNormalizedField; -import graphql.normalized.ExecutableNormalizedOperation; -import graphql.schema.FieldCoordinates; -import org.jetbrains.annotations.Nullable; - -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; - -import static graphql.schema.FieldCoordinates.coordinates; - -/** - * This {@link graphql.execution.instrumentation.Instrumentation} ensure that a submitted introspection query is done in - * good faith. - *

- * There are attack vectors where a crafted introspection query can cause the engine to spend too much time - * producing introspection data. This is especially true on large schemas with lots of types and fields. - *

- * Schemas form a cyclic graph and hence it's possible to send in introspection queries that can reference those cycles - * and in large schemas this can be expensive and perhaps a "denial of service". - *

- * This instrumentation only allows one __schema field or one __type field to be present, and it does not allow the `__Type` fields - * to form a cycle, i.e., that can only be present once. This allows the standard and common introspection queries to work - * so tooling such as graphiql can work. - */ -@PublicApi -public class GoodFaithIntrospectionInstrumentation extends SimplePerformantInstrumentation { - - private static final Map ALLOWED_FIELD_INSTANCES = Map.of( - coordinates("Query", "__schema"), 1 - , coordinates("Query", "__type"), 1 - - , coordinates("__Type", "fields"), 1 - , coordinates("__Type", "inputFields"), 1 - , coordinates("__Type", "interfaces"), 1 - , coordinates("__Type", "possibleTypes"), 1 - ); - private final Map allowFieldInstances; - - public GoodFaithIntrospectionInstrumentation() { - this(ALLOWED_FIELD_INSTANCES); - } - - private GoodFaithIntrospectionInstrumentation(Map allowFieldInstances) { - this.allowFieldInstances = allowFieldInstances; - } - - @Override - public @Nullable ExecutionStrategyInstrumentationContext beginExecutionStrategy(InstrumentationExecutionStrategyParameters parameters, InstrumentationState state) { - - // We check first to see if there are any introspection fields present on the top level field set - // if there isn't, then there is nothing to check. This ensures we are performant. - List topLevelFieldName = parameters.getExecutionStrategyParameters().getFields().getKeys(); - boolean weHaveIntrospectionFields = false; - for (String fieldName : topLevelFieldName) { - if (Introspection.SchemaMetaFieldDef.getName().equals(fieldName) || Introspection.TypeMetaFieldDef.getName().equals(fieldName)) { - weHaveIntrospectionFields = true; - break; - } - } - if (weHaveIntrospectionFields) { - ensureTheyAreInGoodFaith(parameters.getExecutionContext()); - } - return ExecutionStrategyInstrumentationContext.NOOP; - } - - private void ensureTheyAreInGoodFaith(ExecutionContext executionContext) { - ExecutableNormalizedOperation operation = executionContext.getNormalizedQueryTree().get(); - ImmutableListMultimap coordinatesToENFs = operation.getCoordinatesToNormalizedFields(); - for (Map.Entry entry : allowFieldInstances.entrySet()) { - FieldCoordinates coordinates = entry.getKey(); - Integer allowSize = entry.getValue(); - ImmutableList normalizedFields = coordinatesToENFs.get(coordinates); - if (normalizedFields.size() > allowSize) { - throw new BadFaithIntrospectionAbortExecutionException(coordinates.toString()); - } - } - } - - @PublicApi - public static class BadFaithIntrospectionAbortExecutionException extends AbortExecutionException { - public BadFaithIntrospectionAbortExecutionException(String qualifiedField) { - super(String.format("This request is not asking for introspection in good faith - %s is present too often!", qualifiedField)); - } - } - - public static Builder newGoodFaithIntrospection() { - return new Builder(); - } - - public static class Builder { - - - private final Map allowFieldInstances = new LinkedHashMap<>(ALLOWED_FIELD_INSTANCES); - - /** - * This allows you to set how many __type(name : "x) field instances are allowed in an introspection query - * - * @param maxInstances the number allowed - * - * @return this builder - */ - public Builder maximumUnderscoreTypeInstances(int maxInstances) { - allowFieldInstances.put(coordinates("Query", "__type"), maxInstances); - return this; - } - - /** - * This allows you to set how many __schema field instances are allowed in an introspection query - * - * @param maxInstances the number allowed - * - * @return this builder - */ - public Builder maximumUnderscoreSchemaInstances(int maxInstances) { - allowFieldInstances.put(coordinates("Query", "__schema"), maxInstances); - return this; - } - - /** - * This allows you to set how many qualified field instances are allowed in an introspection query - * - * @param coordinates - the qualified field name - * @param maxInstances the number allowed - * - * @return this builder - */ - public Builder maximumFieldInstances(FieldCoordinates coordinates, int maxInstances) { - allowFieldInstances.put(coordinates, maxInstances); - return this; - } - - public GoodFaithIntrospectionInstrumentation build() { - return new GoodFaithIntrospectionInstrumentation(allowFieldInstances); - } - } -} diff --git a/src/main/java/graphql/introspection/Introspection.java b/src/main/java/graphql/introspection/Introspection.java index 4015ff393b..c228f1c39f 100644 --- a/src/main/java/graphql/introspection/Introspection.java +++ b/src/main/java/graphql/introspection/Introspection.java @@ -7,6 +7,7 @@ import graphql.GraphQLContext; import graphql.Internal; import graphql.PublicApi; +import graphql.execution.ExecutionContext; import graphql.execution.MergedField; import graphql.execution.MergedSelectionSet; import graphql.execution.ValuesResolver; @@ -108,10 +109,12 @@ public static boolean isEnabledJvmWide() { * that can be returned to the user. * * @param mergedSelectionSet the fields to be executed + * @param executionContext the execution context in play * * @return an optional error result */ - public static Optional isIntrospectionSensible(GraphQLContext graphQLContext, MergedSelectionSet mergedSelectionSet) { + public static Optional isIntrospectionSensible(MergedSelectionSet mergedSelectionSet, ExecutionContext executionContext) { + GraphQLContext graphQLContext = executionContext.getGraphQLContext(); MergedField schemaField = mergedSelectionSet.getSubField(SchemaMetaFieldDef.getName()); if (schemaField != null) { if (!isIntrospectionEnabled(graphQLContext)) { @@ -124,7 +127,10 @@ public static Optional isIntrospectionSensible(GraphQLContext g return mkDisabledError(typeField); } } - // later we can put a good faith check code here to check the fields make sense + if (schemaField != null || typeField != null) + { + return GoodFaithIntrospection.checkIntrospection(executionContext); + } return Optional.empty(); } diff --git a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy index 1450aa456c..969e0abd60 100644 --- a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy +++ b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy @@ -1,13 +1,17 @@ package graphql.introspection +import graphql.ExecutionInput import graphql.ExecutionResult import graphql.TestUtil -import graphql.schema.FieldCoordinates import spock.lang.Specification class GoodFaithIntrospectionInstrumentationTest extends Specification { - def graphql = TestUtil.graphQL("type Query { f : String }").instrumentation(new GoodFaithIntrospectionInstrumentation()).build() + def graphql = TestUtil.graphQL("type Query { normalField : String }").build() + + def cleanup() { + GoodFaithIntrospection.enabledJvmWide(true) + } def "test asking for introspection in good faith"() { @@ -23,7 +27,7 @@ class GoodFaithIntrospectionInstrumentationTest extends Specification { ExecutionResult er = graphql.execute(query) then: !er.errors.isEmpty() - er.errors[0] instanceof GoodFaithIntrospectionInstrumentation.BadFaithIntrospectionAbortExecutionException + er.errors[0] instanceof GoodFaithIntrospection.BadFaithIntrospectionError where: query | _ @@ -70,83 +74,60 @@ class GoodFaithIntrospectionInstrumentationTest extends Specification { """ | _ } - def "test builder"() { - GoodFaithIntrospectionInstrumentation goodFaithIntrospectionInstrumentation = GoodFaithIntrospectionInstrumentation.newGoodFaithIntrospection() - .maximumUnderscoreSchemaInstances(2) - .maximumUnderscoreTypeInstances(3) - .maximumFieldInstances(FieldCoordinates.coordinates("__Type", "fields"), 2) - .build() - - def graphql = TestUtil.graphQL("type Query { f : String }").instrumentation(goodFaithIntrospectionInstrumentation).build() + def "mixed general queries and introspections will be stopped anyway"() { + def query = """ + query goodAndBad { + normalField + __schema{types{fields{type{fields{type{fields{type{fields{type{name}}}}}}}}}} + } + """ when: - def er = graphql.execute(IntrospectionQuery.INTROSPECTION_QUERY) + ExecutionResult er = graphql.execute(query) then: - er.errors.isEmpty() + !er.errors.isEmpty() + er.errors[0] instanceof GoodFaithIntrospection.BadFaithIntrospectionError + er.data == null // it stopped hard - it did not continue to normal business + } + def "can be disabled"() { when: - er = graphql.execute(""" - query ok { - __schema { types { name } } - alias1: __schema { types { name } } - } - """) + def currentState = GoodFaithIntrospection.isEnabledJvmWide() + then: - er.errors.isEmpty() + currentState when: - er = graphql.execute(""" - query ok { - __schema { types { name } } - alias1: __schema { types { name } } - alias2: __schema { types { name } } - } - """) + def prevState = GoodFaithIntrospection.enabledJvmWide(false) + then: - !er.errors.isEmpty() - er.errors[0] instanceof GoodFaithIntrospectionInstrumentation.BadFaithIntrospectionAbortExecutionException + prevState when: - er = graphql.execute(""" - query ok { - __type(name : "Query") { name } - alias1 : __type(name : "Query") { name } - alias2 : __type(name : "Query") { name } - } - """) + ExecutionResult er = graphql.execute("query badActor{__schema{types{fields{type{fields{type{fields{type{fields{type{name}}}}}}}}}}}") + then: er.errors.isEmpty() + } + def "can be disabled per request"() { when: - er = graphql.execute(""" - query ok { - __type(name : "Query") { name } - alias1 : __type(name : "Query") { name } - alias2 : __type(name : "Query") { name } - alias3 : __type(name : "Query") { name } - } - """) - then: - !er.errors.isEmpty() - er.errors[0] instanceof GoodFaithIntrospectionInstrumentation.BadFaithIntrospectionAbortExecutionException + def context = [(GoodFaithIntrospection.GOOD_FAITH_INTROSPECTION_DISABLED): true] + ExecutionInput executionInput = ExecutionInput.newExecutionInput("query badActor{__schema{types{fields{type{fields{type{fields{type{fields{type{name}}}}}}}}}}}") + .graphQLContext(context).build() + ExecutionResult er = graphql.execute(executionInput) - when: - er = graphql.execute(""" - query ok { - __schema { types { fields { type { fields { name }}}}} - } - """) then: er.errors.isEmpty() when: - er = graphql.execute(""" - query ok { - __schema { types { fields { type { fields { type { fields { name }}}}}}} - } - """) + context = [(GoodFaithIntrospection.GOOD_FAITH_INTROSPECTION_DISABLED): false] + executionInput = ExecutionInput.newExecutionInput("query badActor{__schema{types{fields{type{fields{type{fields{type{fields{type{name}}}}}}}}}}}") + .graphQLContext(context).build() + er = graphql.execute(executionInput) + then: !er.errors.isEmpty() - er.errors[0] instanceof GoodFaithIntrospectionInstrumentation.BadFaithIntrospectionAbortExecutionException + er.errors[0] instanceof GoodFaithIntrospection.BadFaithIntrospectionError } } From e51361bfc22e61dbcb262c04000039822a87852d Mon Sep 17 00:00:00 2001 From: Brad Baker Date: Mon, 18 Mar 2024 17:20:55 +1100 Subject: [PATCH 5/5] fixing tests - for one at a time --- ...ithIntrospectionInstrumentationTest.groovy | 3 ++ ...ormalizedOperationToAstCompilerTest.groovy | 48 ++++++++++++------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy index 969e0abd60..f1ffc2c570 100644 --- a/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy +++ b/src/test/groovy/graphql/introspection/GoodFaithIntrospectionInstrumentationTest.groovy @@ -9,6 +9,9 @@ class GoodFaithIntrospectionInstrumentationTest extends Specification { def graphql = TestUtil.graphQL("type Query { normalField : String }").build() + def setup() { + GoodFaithIntrospection.enabledJvmWide(true) + } def cleanup() { GoodFaithIntrospection.enabledJvmWide(true) } diff --git a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy index 27c4c89a6d..a135ac3f9b 100644 --- a/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy +++ b/src/test/groovy/graphql/normalized/ExecutableNormalizedOperationToAstCompilerTest.groovy @@ -1387,17 +1387,10 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat ''' } - def "introspection query can be printed"() { + def "introspection query can be printed __schema"() { def sdl = ''' type Query { - foo1: Foo - } - interface Foo { - test: String - } - type AFoo implements Foo { - test: String - aFoo: String + f: String } ''' def query = ''' @@ -1409,14 +1402,7 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat } } } - - __type(name: "World") { - name - fields { - name - } - } - } + } ''' GraphQLSchema schema = mkSchema(sdl) @@ -1433,6 +1419,34 @@ abstract class ExecutableNormalizedOperationToAstCompilerTest extends Specificat } } } +} +''' + } + + def "introspection query can be printed __type"() { + def sdl = ''' + type Query { + f: String + } + ''' + def query = ''' + query introspection_query { + __type(name: "World") { + name + fields { + name + } + } + } + ''' + + GraphQLSchema schema = mkSchema(sdl) + def fields = createNormalizedFields(schema, query) + when: + def result = localCompileToDocument(schema, QUERY, null, fields, noVariables) + def documentPrinted = AstPrinter.printAst(new AstSorter().sort(result.document)) + then: + documentPrinted == '''{ __type(name: "World") { fields { name