Skip to content

Restrict the number of ENFs created and take advantage in GoodFaith introspection #3539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bbakerman merged 7 commits into from
Mar 22, 2024
Merged

Restrict the number of ENFs created and take advantage in GoodFaith introspection #3539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b94f152
add the ability to restrict the number of ENFs created
andimarek Mar 21, 2024
9d37ffc
handle aliases correctly for good faith check
andimarek Mar 21, 2024
83a21e9
Added good faith code to create ENO that is a fixed size
bbakerman Mar 21, 2024
2c29b53
Uppped limit as discussed
bbakerman Mar 21, 2024
bd6ec10
Fixed tests and also added unit test for depth in good faith
bbakerman Mar 22, 2024
55f2e5d
Added benchmark code
bbakerman Mar 22, 2024
70895d7
Check max depth seen more often
bbakerman Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/main/java/graphql/execution/ExecutionContext.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ public class ExecutionContext {
this.errors.set(builder.errors);
this.localContext = builder.localContext;
this.executionInput = builder.executionInput;
queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables));
this.queryTree = FpKit.interThreadMemoize(() -> ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation(graphQLSchema, operationDefinition, fragmentsByName, coercedVariables));
Copy link
Member

@bbakerman bbakerman Mar 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

minor consistency issue - uses this.

}


Expand Down
35 changes: 34 additions & 1 deletion src/main/java/graphql/introspection/GoodFaithIntrospection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;

import static graphql.normalized.ExecutableNormalizedOperationFactory.Options;
import static graphql.normalized.ExecutableNormalizedOperationFactory.createExecutableNormalizedOperation;
import static graphql.schema.FieldCoordinates.coordinates;

/**
Expand All @@ -44,6 +46,14 @@ public class GoodFaithIntrospection {
public static final String GOOD_FAITH_INTROSPECTION_DISABLED = "GOOD_FAITH_INTROSPECTION_DISABLED";

private static final AtomicBoolean ENABLED_STATE = new AtomicBoolean(true);
/**
* This is the maximum number of executable fields that can be in a good faith introspection query
*/
public static final int GOOD_FAITH_MAX_FIELDS_COUNT = 500;
/**
* This is the maximum depth a good faith introspection query can be
*/
public static final int GOOD_FAITH_MAX_DEPTH_COUNT = 20;

/**
* @return true if good faith introspection is enabled
Expand Down Expand Up @@ -75,7 +85,7 @@ public static boolean enabledJvmWide(boolean flag) {

public static Optional<ExecutionResult> checkIntrospection(ExecutionContext executionContext) {
if (isIntrospectionEnabled(executionContext.getGraphQLContext())) {
ExecutableNormalizedOperation operation = executionContext.getNormalizedQueryTree().get();
ExecutableNormalizedOperation operation = mkOperation(executionContext);
ImmutableListMultimap<FieldCoordinates, ExecutableNormalizedField> coordinatesToENFs = operation.getCoordinatesToNormalizedFields();
for (Map.Entry<FieldCoordinates, Integer> entry : ALLOWED_FIELD_INSTANCES.entrySet()) {
FieldCoordinates coordinates = entry.getKey();
Expand All @@ -90,6 +100,29 @@ public static Optional<ExecutionResult> checkIntrospection(ExecutionContext exec
return Optional.empty();
}

/**
* This makes an executable operation limited in size then which suits a good faith introspection query. This helps guard
* against malicious queries.
*
* @param executionContext the execution context
*
* @return an executable operation
*/
private static ExecutableNormalizedOperation mkOperation(ExecutionContext executionContext) {
Options options = Options.defaultOptions()
.maxFieldsCount(GOOD_FAITH_MAX_FIELDS_COUNT)
.maxChildrenDepth(GOOD_FAITH_MAX_DEPTH_COUNT)
.locale(executionContext.getLocale())
.graphQLContext(executionContext.getGraphQLContext());

return createExecutableNormalizedOperation(executionContext.getGraphQLSchema(),
executionContext.getOperationDefinition(),
executionContext.getFragmentsByName(),
executionContext.getCoercedVariables(),
options);

}

private static boolean isIntrospectionEnabled(GraphQLContext graphQlContext) {
if (!isEnabledJvmWide()) {
return false;
Expand Down
24 changes: 12 additions & 12 deletions src/main/java/graphql/introspection/Introspection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,20 +115,20 @@ public static boolean isEnabledJvmWide() {
*/
public static Optional<ExecutionResult> isIntrospectionSensible(MergedSelectionSet mergedSelectionSet, ExecutionContext executionContext) {
GraphQLContext graphQLContext = executionContext.getGraphQLContext();
MergedField schemaField = mergedSelectionSet.getSubField(SchemaMetaFieldDef.getName());
if (schemaField != null) {
if (!isIntrospectionEnabled(graphQLContext)) {
return mkDisabledError(schemaField);
}
}
MergedField typeField = mergedSelectionSet.getSubField(TypeMetaFieldDef.getName());
if (typeField != null) {
if (!isIntrospectionEnabled(graphQLContext)) {
return mkDisabledError(typeField);

boolean isIntrospection = false;
for (String key : mergedSelectionSet.getKeys()) {
String fieldName = mergedSelectionSet.getSubField(key).getName();
if (fieldName.equals(SchemaMetaFieldDef.getName())
|| fieldName.equals(TypeMetaFieldDef.getName())) {
if (!isIntrospectionEnabled(graphQLContext)) {
return mkDisabledError(mergedSelectionSet.getSubField(key));
}
isIntrospection = true;
break;
}
}
if (schemaField != null || typeField != null)
{
if (isIntrospection) {
return GoodFaithIntrospection.checkIntrospection(executionContext);
}
return Optional.empty();
Expand Down
23 changes: 21 additions & 2 deletions src/main/java/graphql/normalized/ExecutableNormalizedOperation.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public class ExecutableNormalizedOperation {
private final Map<ExecutableNormalizedField, MergedField> normalizedFieldToMergedField;
private final Map<ExecutableNormalizedField, QueryDirectives> normalizedFieldToQueryDirectives;
private final ImmutableListMultimap<FieldCoordinates, ExecutableNormalizedField> coordinatesToNormalizedFields;
private final int operationFieldCount;
private final int operationDepth;

public ExecutableNormalizedOperation(
OperationDefinition.Operation operation,
Expand All @@ -39,15 +41,18 @@ public ExecutableNormalizedOperation(
ImmutableListMultimap<Field, ExecutableNormalizedField> fieldToNormalizedField,
Map<ExecutableNormalizedField, MergedField> normalizedFieldToMergedField,
Map<ExecutableNormalizedField, QueryDirectives> normalizedFieldToQueryDirectives,
ImmutableListMultimap<FieldCoordinates, ExecutableNormalizedField> coordinatesToNormalizedFields
) {
ImmutableListMultimap<FieldCoordinates, ExecutableNormalizedField> coordinatesToNormalizedFields,
int operationFieldCount,
int operationDepth) {
this.operation = operation;
this.operationName = operationName;
this.topLevelFields = topLevelFields;
this.fieldToNormalizedField = fieldToNormalizedField;
this.normalizedFieldToMergedField = normalizedFieldToMergedField;
this.normalizedFieldToQueryDirectives = normalizedFieldToQueryDirectives;
this.coordinatesToNormalizedFields = coordinatesToNormalizedFields;
this.operationFieldCount = operationFieldCount;
this.operationDepth = operationDepth;
}

/**
Expand All @@ -64,6 +69,20 @@ public String getOperationName() {
return operationName;
}

/**
* @return This returns how many {@link ExecutableNormalizedField}s are in the operation.
*/
public int getOperationFieldCount() {
return operationFieldCount;
}

/**
* @return This returns the depth of the operation
*/
public int getOperationDepth() {
return operationDepth;
}

/**
* This multimap shows how a given {@link ExecutableNormalizedField} maps to a one or more field coordinate in the schema
*
Expand Down
92 changes: 77 additions & 15 deletions src/main/java/graphql/normalized/ExecutableNormalizedOperationFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@
import static graphql.util.FpKit.filterSet;
import static graphql.util.FpKit.groupingBy;
import static graphql.util.FpKit.intersection;
import static java.util.Collections.max;
Copy link
Member

@dondonz dondonz Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this import is not used. Math.max is used

import static java.util.Collections.singleton;
import static java.util.Collections.singletonList;
import static java.util.stream.Collectors.toCollection;
Expand All @@ -80,24 +81,28 @@ public static class Options {
private final GraphQLContext graphQLContext;
private final Locale locale;
private final int maxChildrenDepth;
private final int maxFieldsCount;

private final boolean deferSupport;

private Options(GraphQLContext graphQLContext,
Locale locale,
int maxChildrenDepth,
int maxFieldsCount,
boolean deferSupport) {
this.graphQLContext = graphQLContext;
this.locale = locale;
this.maxChildrenDepth = maxChildrenDepth;
this.deferSupport = deferSupport;
this.maxFieldsCount = maxFieldsCount;
}

public static Options defaultOptions() {
return new Options(
GraphQLContext.getDefault(),
Locale.getDefault(),
Integer.MAX_VALUE,
Integer.MAX_VALUE,
false);
}

Expand All @@ -111,7 +116,7 @@ public static Options defaultOptions() {
* @return new options object to use
*/
public Options locale(Locale locale) {
return new Options(this.graphQLContext, locale, this.maxChildrenDepth, this.deferSupport);
return new Options(this.graphQLContext, locale, this.maxChildrenDepth, this.maxFieldsCount, this.deferSupport);
}

/**
Expand All @@ -124,7 +129,7 @@ public Options locale(Locale locale) {
* @return new options object to use
*/
public Options graphQLContext(GraphQLContext graphQLContext) {
return new Options(graphQLContext, this.locale, this.maxChildrenDepth, this.deferSupport);
return new Options(graphQLContext, this.locale, this.maxChildrenDepth, this.maxFieldsCount, this.deferSupport);
}

/**
Expand All @@ -136,7 +141,19 @@ public Options graphQLContext(GraphQLContext graphQLContext) {
* @return new options object to use
*/
public Options maxChildrenDepth(int maxChildrenDepth) {
return new Options(this.graphQLContext, this.locale, maxChildrenDepth, this.deferSupport);
return new Options(this.graphQLContext, this.locale, maxChildrenDepth, this.maxFieldsCount, this.deferSupport);
}

/**
* Controls the maximum number of ENFs created. Can be used to prevent
* against malicious operations.
*
* @param maxFieldsCount the max number of ENFs created
*
* @return new options object to use
*/
public Options maxFieldsCount(int maxFieldsCount) {
return new Options(this.graphQLContext, this.locale, maxChildrenDepth, maxFieldsCount, this.deferSupport);
Copy link
Member

@dondonz dondonz Mar 22, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this have been this.maxChildrenDepth?

}

/**
Expand All @@ -148,7 +165,7 @@ public Options maxChildrenDepth(int maxChildrenDepth) {
*/
@ExperimentalApi
public Options deferSupport(boolean deferSupport) {
return new Options(this.graphQLContext, this.locale, this.maxChildrenDepth, deferSupport);
return new Options(this.graphQLContext, this.locale, this.maxChildrenDepth, this.maxFieldsCount, deferSupport);
}

/**
Expand Down Expand Up @@ -178,6 +195,10 @@ public int getMaxChildrenDepth() {
return maxChildrenDepth;
}

public int getMaxFieldsCount() {
return maxFieldsCount;
}

/**
* @return whether support for defer is enabled
*
Expand Down Expand Up @@ -266,13 +287,36 @@ public static ExecutableNormalizedOperation createExecutableNormalizedOperation(
OperationDefinition operationDefinition,
Map<String, FragmentDefinition> fragments,
CoercedVariables coercedVariableValues) {
return createExecutableNormalizedOperation(graphQLSchema,
operationDefinition,
fragments,
coercedVariableValues,
Options.defaultOptions());
}

/**
* This will create a runtime representation of the graphql operation that would be executed
* in a runtime sense.
*
* @param graphQLSchema the schema to be used
* @param operationDefinition the operation to be executed
* @param fragments a set of fragments associated with the operation
* @param coercedVariableValues the coerced variables to use
*
* @return a runtime representation of the graphql operation.
*/
public static ExecutableNormalizedOperation createExecutableNormalizedOperation(GraphQLSchema graphQLSchema,
OperationDefinition operationDefinition,
Map<String, FragmentDefinition> fragments,
CoercedVariables coercedVariableValues,
Options options) {
return new ExecutableNormalizedOperationFactoryImpl(
graphQLSchema,
operationDefinition,
fragments,
coercedVariableValues,
null,
Options.defaultOptions()
options
).createNormalizedQueryImpl();
}

Expand Down Expand Up @@ -386,6 +430,8 @@ private static class ExecutableNormalizedOperationFactoryImpl {
private final ImmutableMap.Builder<ExecutableNormalizedField, MergedField> normalizedFieldToMergedField = ImmutableMap.builder();
private final ImmutableMap.Builder<ExecutableNormalizedField, QueryDirectives> normalizedFieldToQueryDirectives = ImmutableMap.builder();
private final ImmutableListMultimap.Builder<FieldCoordinates, ExecutableNormalizedField> coordinatesToNormalizedFields = ImmutableListMultimap.builder();
private int fieldCount = 0;
private int maxDepthSeen = 0;

private ExecutableNormalizedOperationFactoryImpl(
GraphQLSchema graphQLSchema,
Expand Down Expand Up @@ -420,10 +466,11 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl() {
updateFieldToNFMap(topLevel, fieldAndAstParents);
updateCoordinatedToNFMap(topLevel);

buildFieldWithChildren(
int depthSeen = buildFieldWithChildren(
topLevel,
fieldAndAstParents,
1);
maxDepthSeen = Math.max(maxDepthSeen,depthSeen);
}
// getPossibleMergerList
for (PossibleMerger possibleMerger : possibleMergerList) {
Expand All @@ -437,7 +484,9 @@ private ExecutableNormalizedOperation createNormalizedQueryImpl() {
fieldToNormalizedField.build(),
normalizedFieldToMergedField.build(),
normalizedFieldToQueryDirectives.build(),
coordinatesToNormalizedFields.build()
coordinatesToNormalizedFields.build(),
fieldCount,
maxDepthSeen
);
}

Expand All @@ -448,15 +497,14 @@ private void captureMergedField(ExecutableNormalizedField enf, MergedField merge
normalizedFieldToMergedField.put(enf, mergedFld);
}

private void buildFieldWithChildren(ExecutableNormalizedField executableNormalizedField,
private int buildFieldWithChildren(ExecutableNormalizedField executableNormalizedField,
ImmutableList<FieldAndAstParent> fieldAndAstParents,
int curLevel) {
if (curLevel > this.options.getMaxChildrenDepth()) {
throw new AbortExecutionException("Maximum query depth exceeded " + curLevel + " > " + this.options.getMaxChildrenDepth());
}
checkMaxDepthExceeded(curLevel);

CollectNFResult nextLevel = collectFromMergedField(executableNormalizedField, fieldAndAstParents, curLevel + 1);

int maxDepthSeen = curLevel;
for (ExecutableNormalizedField childENF : nextLevel.children) {
executableNormalizedField.addChild(childENF);
ImmutableList<FieldAndAstParent> childFieldAndAstParents = nextLevel.normalizedFieldToAstFields.get(childENF);
Expand All @@ -467,9 +515,19 @@ private void buildFieldWithChildren(ExecutableNormalizedField executableNormaliz
updateFieldToNFMap(childENF, childFieldAndAstParents);
updateCoordinatedToNFMap(childENF);

buildFieldWithChildren(childENF,
int depthSeen = buildFieldWithChildren(childENF,
childFieldAndAstParents,
curLevel + 1);
maxDepthSeen = Math.max(maxDepthSeen,depthSeen);

checkMaxDepthExceeded(maxDepthSeen);
}
return maxDepthSeen;
}

private void checkMaxDepthExceeded(int depthSeen) {
if (depthSeen > this.options.getMaxChildrenDepth()) {
throw new AbortExecutionException("Maximum query depth exceeded. " + depthSeen + " > " + this.options.getMaxChildrenDepth());
}
}

Expand Down Expand Up @@ -578,6 +636,11 @@ private void createNFs(ImmutableList.Builder<ExecutableNormalizedField> nfListBu
private ExecutableNormalizedField createNF(CollectedFieldGroup collectedFieldGroup,
int level,
ExecutableNormalizedField parent) {

this.fieldCount++;
if (this.fieldCount > this.options.getMaxFieldsCount()) {
throw new AbortExecutionException("Maximum field count exceeded. " + this.fieldCount + " > " + this.options.getMaxFieldsCount());
}
Field field;
Set<GraphQLObjectType> objectTypes = collectedFieldGroup.objectTypes;
field = collectedFieldGroup.fields.iterator().next().field;
Expand All @@ -590,7 +653,6 @@ private ExecutableNormalizedField createNF(CollectedFieldGroup collectedFieldGro
normalizedArgumentValues = ValuesResolver.getNormalizedArgumentValues(fieldDefinition.getArguments(), field.getArguments(), this.normalizedVariableValues);
}
ImmutableList<String> objectTypeNames = map(objectTypes, GraphQLObjectType::getName);

return ExecutableNormalizedField.newNormalizedField()
.alias(field.getAlias())
.resolvedArguments(argumentValues)
Expand Down Expand Up @@ -763,8 +825,8 @@ private void collectInlineFragment(List<CollectedField> result,

private NormalizedDeferredExecution buildDeferredExecution(
List<Directive> directives,
Set<GraphQLObjectType> newPossibleObjects) {
if(!options.deferSupport) {
Set<GraphQLObjectType> newPossibleObjects) {
if (!options.deferSupport) {
return null;
}

Expand Down
Loading