Skip to content

Commit

Permalink
Reworked Default Security Policies (#7524)
Browse files Browse the repository at this point in the history
  • Loading branch information
michaelstaib authored Sep 27, 2024
1 parent 7f298a8 commit 8cc3f57
Show file tree
Hide file tree
Showing 35 changed files with 954 additions and 95 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ public static class HotChocolateAspNetCoreHostingBuilderExtensions
/// <param name="maxAllowedRequestSize">
/// The max allowed GraphQL request size.
/// </param>
/// <param name="disableCostAnalyzer">
/// Defines if the cost analyzer should be disabled.
/// <param name="disableDefaultSecurity">
/// Defines if the default security policy should be disabled.
/// </param>
/// <returns></returns>
public static IRequestExecutorBuilder AddGraphQL(
this IHostApplicationBuilder builder,
string? schemaName = default,
int maxAllowedRequestSize = MaxAllowedRequestSize,
bool disableCostAnalyzer = false)
=> builder.Services.AddGraphQLServer(schemaName, maxAllowedRequestSize, disableCostAnalyzer);
bool disableDefaultSecurity = false)
=> builder.Services.AddGraphQLServer(schemaName, maxAllowedRequestSize, disableDefaultSecurity);
}
Original file line number Diff line number Diff line change
Expand Up @@ -94,8 +94,8 @@ public static IServiceCollection AddGraphQLServerCore(
/// <param name="maxAllowedRequestSize">
/// The max allowed GraphQL request size.
/// </param>
/// <param name="disableCostAnalyzer">
/// Defines if the cost analyzer should be disabled.
/// <param name="disableDefaultSecurity">
/// Defines if the default security policy should be disabled.
/// </param>
/// <returns>
/// Returns the <see cref="IRequestExecutorBuilder"/> so that configuration can be chained.
Expand All @@ -104,22 +104,28 @@ public static IRequestExecutorBuilder AddGraphQLServer(
this IServiceCollection services,
string? schemaName = default,
int maxAllowedRequestSize = MaxAllowedRequestSize,
bool disableCostAnalyzer = false)
bool disableDefaultSecurity = false)
{
var builder = services
.AddGraphQLServerCore(maxAllowedRequestSize)
.AddGraphQL(schemaName)
.AddDefaultHttpRequestInterceptor()
.AddSubscriptionServices();

if (!disableCostAnalyzer)
if (!disableDefaultSecurity)
{
builder.AddCostAnalyzer();
builder.AddIntrospectionAllowedRule(
(sp, _) =>
{
var environment = sp.GetService<IHostEnvironment>();
return (environment?.IsDevelopment() ?? true) == false;
return environment?.IsDevelopment() == false;
});
builder.AddMaxAllowedFieldCycleDepthRule(
isEnabled: (sp, _) =>
{
var environment = sp.GetService<IHostEnvironment>();
return environment?.IsDevelopment() == false;
});
}

Expand All @@ -145,7 +151,7 @@ public static IRequestExecutorBuilder AddGraphQLServer(
this IRequestExecutorBuilder builder,
string? schemaName = default,
bool disableCostAnalyzer = false)
=> builder.Services.AddGraphQLServer(schemaName, disableCostAnalyzer: disableCostAnalyzer);
=> builder.Services.AddGraphQLServer(schemaName, disableDefaultSecurity: disableCostAnalyzer);

/// <summary>
/// Registers the GraphQL Upload Scalar.
Expand Down
2 changes: 1 addition & 1 deletion src/HotChocolate/Caching/test/Caching.Tests/SchemaTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public async Task Allow_CacheControl_On_FieldDefinition()
{
var schema =
await new ServiceCollection()
.AddGraphQLServer(disableCostAnalyzer: true)
.AddGraphQLServer(disableDefaultSecurity: true)
.AddTypeExtension(typeof(Query))
.ConfigureSchema(
b => b.TryAddRootType(
Expand Down
10 changes: 10 additions & 0 deletions src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,16 @@ public static class Validation
/// The introspection is not allowed for the current request
/// </summary>
public const string IntrospectionNotAllowed = "HC0046";

/// <summary>
/// The maximum allowed introspection depth was exceeded.
/// </summary>
public const string MaxIntrospectionDepthOverflow = "HC0086";

/// <summary>
/// The maximum allowed coordinate cycle depth was exceeded.
/// </summary>
public const string MaxCoordinateCycleDepthOverflow = "HC0087";
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ internal sealed class AuthorizeValidationRule(AuthorizationCache cache) : IDocum
private readonly AuthorizeValidationVisitor _visitor = new();
private readonly AuthorizationCache _cache = cache ?? throw new ArgumentNullException(nameof(cache));

public ushort Priority => ushort.MaxValue;

public bool IsCacheable => false;

public void Validate(IDocumentValidatorContext context, DocumentNode document)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using HotChocolate;
using HotChocolate.Execution.Configuration;
using HotChocolate.Validation;
using HotChocolate.Validation.Options;
Expand Down Expand Up @@ -187,14 +188,18 @@ public static IRequestExecutorBuilder AddValidationResultAggregator<T>(
/// <param name="allowRequestOverrides">
/// Defines if request depth overrides are allowed on a per request basis.
/// </param>
/// <param name="isEnabled">
/// Defines if the validation rule is enabled.
/// </param>
/// <returns>
/// Returns the <see cref="IRequestExecutorBuilder"/> for configuration chaining.
/// </returns>
public static IRequestExecutorBuilder AddMaxExecutionDepthRule(
this IRequestExecutorBuilder builder,
int maxAllowedExecutionDepth,
bool skipIntrospectionFields = false,
bool allowRequestOverrides = false)
bool allowRequestOverrides = false,
Func<IServiceProvider, ValidationOptions, bool>? isEnabled = null)
{
if (builder is null)
{
Expand All @@ -206,7 +211,8 @@ public static IRequestExecutorBuilder AddMaxExecutionDepthRule(
b => b.AddMaxExecutionDepthRule(
maxAllowedExecutionDepth,
skipIntrospectionFields,
allowRequestOverrides));
allowRequestOverrides,
isEnabled));
return builder;
}

Expand Down Expand Up @@ -280,6 +286,45 @@ public static IRequestExecutorBuilder SetMaxAllowedValidationErrors(
return builder;
}

/// <summary>
/// Adds a validation rule that restricts the coordinate cycle depth in a GraphQL operation.
/// </summary>
public static IRequestExecutorBuilder AddMaxAllowedFieldCycleDepthRule(
this IRequestExecutorBuilder builder,
ushort? defaultCycleLimit = 3,
(SchemaCoordinate Coordinate, ushort MaxAllowed)[]? coordinateCycleLimits = null,
Func<IServiceProvider, ValidationOptions, bool>? isEnabled = null)
{
if (builder is null)
{
throw new ArgumentNullException(nameof(builder));
}

ConfigureValidation(
builder,
b => b.AddMaxAllowedFieldCycleDepthRule(
defaultCycleLimit,
coordinateCycleLimits,
isEnabled));

return builder;
}

/// <summary>
/// Removes the validation rule that restricts the coordinate cycle depth in a GraphQL operation.
/// </summary>
public static IRequestExecutorBuilder RemoveMaxAllowedFieldCycleDepthRule(
this IRequestExecutorBuilder builder)
{
if (builder is null)
{
throw new ArgumentNullException(nameof(builder));
}

ConfigureValidation(builder, b => b.RemoveMaxAllowedFieldCycleDepthRule());
return builder;
}

private static IRequestExecutorBuilder ConfigureValidation(
IRequestExecutorBuilder builder,
Action<IValidationBuilder> configure)
Expand Down
27 changes: 27 additions & 0 deletions src/HotChocolate/Core/src/Validation/CoordinateLimit.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
namespace HotChocolate.Validation;

internal sealed class CoordinateLimit
{
public ushort MaxAllowed { get; private set; }

public ushort Count { get; private set; }

public bool Add()
{
if (Count < MaxAllowed)
{
Count++;
return true;
}

return false;
}

public void Remove() => Count--;

public void Reset(ushort maxAllowed)
{
MaxAllowed = maxAllowed;
Count = 0;
}
}
8 changes: 8 additions & 0 deletions src/HotChocolate/Core/src/Validation/DocumentValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,9 @@ public DocumentValidator(
_nonCacheableRules = _allRules.Where(t => !t.IsCacheable).ToArray();
_aggregators = resultAggregators.ToArray();
_maxAllowedErrors = errorOptions.MaxAllowedErrors;

Array.Sort(_allRules, (a, b) => a.Priority.CompareTo(b.Priority));
Array.Sort(_nonCacheableRules, (a, b) => a.Priority.CompareTo(b.Priority));
}

/// <inheritdoc />
Expand Down Expand Up @@ -102,6 +105,11 @@ public ValueTask<DocumentValidatorResult> ValidateAsync(
for (var i = 0; i < length; i++)
{
Unsafe.Add(ref start, i).Validate(context, document);

if (context.FatalErrorDetected)
{
break;
}
}

if (_aggregators.Length == 0)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ public IOutputType NonNullString

public bool UnexpectedErrorsDetected { get; set; }

public bool FatalErrorDetected { get; set; }

public int Count { get; set; }

public int Max { get; set; }
Expand All @@ -110,6 +112,8 @@ public IOutputType NonNullString

public HashSet<FieldInfoPair> ProcessedFieldPairs { get; } = [];

public FieldDepthCycleTracker FieldDepth { get; } = new();

public IList<FieldInfo> RentFieldInfoList()
{
var buffer = _buffers.Peek();
Expand Down Expand Up @@ -166,7 +170,9 @@ public void Clear()
CurrentFieldPairs.Clear();
NextFieldPairs.Clear();
ProcessedFieldPairs.Clear();
FieldDepth.Reset();
UnexpectedErrorsDetected = false;
FatalErrorDetected = false;
Count = 0;
Max = 0;
Allowed = 0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,18 @@ public class DocumentValidatorRule<TVisitor>
{
private readonly TVisitor _visitor;

public DocumentValidatorRule(TVisitor visitor, bool isCacheable = true)
public DocumentValidatorRule(
TVisitor visitor,
bool isCacheable = true,
ushort property = ushort.MaxValue)
{
_visitor = visitor;
IsCacheable = isCacheable;
Priority = property;
}

public ushort Priority { get; }

public bool IsCacheable { get; }

public void Validate(IDocumentValidatorContext context, DocumentNode document)
Expand Down
29 changes: 29 additions & 0 deletions src/HotChocolate/Core/src/Validation/ErrorHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -707,4 +707,33 @@ public static IError StreamOnNonListField(
.SpecifiedBy("sec-Stream-Directives-Are-Used-On-List-Fields")
.SetPath(context.CreateErrorPath())
.Build();

public static void ReportMaxIntrospectionDepthOverflow(
this IDocumentValidatorContext context,
ISyntaxNode selection)
{
context.FatalErrorDetected = true;
context.ReportError(
ErrorBuilder.New()
.SetMessage("Maximum allowed introspection depth exceeded.")
.SetCode(ErrorCodes.Validation.MaxIntrospectionDepthOverflow)
.SetLocations([selection])
.SetPath(context.CreateErrorPath())
.Build());
}

public static void ReportMaxCoordinateCycleDepthOverflow(
this IDocumentValidatorContext context,
ISyntaxNode selection)
{
context.FatalErrorDetected = true;

context.ReportError(
ErrorBuilder.New()
.SetMessage("Maximum allowed coordinate cycle depth was exceeded.")
.SetCode(ErrorCodes.Validation.MaxIntrospectionDepthOverflow)
.SetLocations([selection])
.SetPath(context.CreateErrorPath())
.Build());
}
}
Loading

0 comments on commit 8cc3f57

Please sign in to comment.