Skip to content
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.

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

feat(CSP): add 'trusted-types' CSP directive support #58

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,11 @@ public class CspBuilder
/// Setups up rules for controlling when subresource integrity must be used
/// </summary>
public CspRequireSriBuilder RequireSri { get; } = new CspRequireSriBuilder();

/// <summary>
/// Setups up rules for controlling when to restrict the creation of Trusted Types policies
Copy link
Owner

Choose a reason for hiding this comment

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

Typo: "Setups up" -> "Sets up". I do notice this typo is in existing code as well :\

It could be good to link to the MDN documentation here.

/// </summary>
public CspTrustedTypesBuilder RequireTrustedTypes { get; } = new CspTrustedTypesBuilder();

public Func<CspSendingHeaderContext, Task> OnSendingHeader { get; set; } = context => Task.CompletedTask;

Expand Down Expand Up @@ -150,6 +155,7 @@ public CspOptions BuildCspOptions()
_options.Prefetch = AllowPrefetch.BuildOptions();
_options.BaseUri = AllowBaseUri.BuildOptions();
_options.RequireSri = RequireSri.BuildOptions();
_options.TrustedTypes = RequireTrustedTypes.BuildOptions();
_options.OnSendingHeader = OnSendingHeader;
return _options;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
using System;
using Joonasw.AspNetCore.SecurityHeaders.Csp.Options;

namespace Joonasw.AspNetCore.SecurityHeaders.Csp.Builder
{
/// <summary>
/// Builder for Content Security Policy
/// rules related to Trusted Types.
/// </summary>
public class CspTrustedTypesBuilder
{
private readonly CspTrustedTypesOptions _options = new CspTrustedTypesOptions();

/// <summary>
/// Disallows creating any Trusted Type policy (same as not specifying any <c>policyName</c>).
/// </summary>
public void DisallowAll()
{
_options.AllowNone = true;
}

/// <summary>
/// Allow any unique policy name ('allow-duplicates' may relax that further)
/// </summary>
/// <returns>The builder for call chaining</returns>
public CspTrustedTypesBuilder WithAnyUniquePolicy()
{
_options.AllowAny = true;
return this;
}

/// <summary>
/// Allow CSS from the given
/// <paramref name="policyName"/>.
/// </summary>
/// <param name="policyName">A valid policy name consists only of alphanumeric characters, or one of "-#=_/@.%". </param>
Copy link
Owner

Choose a reason for hiding this comment

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

If there is a valid set of characters, should we validate it?

/// <returns>The builder for call chaining</returns>
public CspTrustedTypesBuilder WithPolicyName(string policyName)
{
if (policyName == null) throw new ArgumentNullException(nameof(policyName));
if (policyName.Length == 0) throw new ArgumentException("Policy Name can't be empty", nameof(policyName));

_options.TrustedPolicies.Add(policyName);
return this;
}

public CspTrustedTypesBuilder AllowDuplicates()
{
_options.AllowDuplicates = true;
return this;
}

public CspTrustedTypesBuilder RequireTrustedTypesForScript()
Copy link
Owner

Choose a reason for hiding this comment

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

I think this function needs a comment explaining what effect it has.

{
_options.RequireTrustedTypesForScript = true;
return this;
}

public CspTrustedTypesOptions BuildOptions()
{
return _options;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
using System.Collections.Generic;

namespace Joonasw.AspNetCore.SecurityHeaders.Csp.Options
{
/// <summary>
/// This directive declares an allow-list of trusted type policy names created with
/// <c>TrustedTypes.createPolicy</c> from Trusted Types API
/// </summary>
public class CspTrustedTypesOptions
{
/// <summary>
/// Collection of sources from where these resources can be loaded.
/// </summary>
/// <remark>
/// A valid policy name consists only of alphanumeric characters, or one of "-#=_/@.%".
/// </remark>
public ICollection<string> TrustedPolicies { get; set; }

/// <summary>
/// If <c>true</c> allows for creating policies with a name that was already used.
/// </summary>
public bool AllowDuplicates { get; set; }

/// <summary>
/// Disallows creating any Trusted Type policy (same as not specifying any <c>policyName</c>).
/// </summary>
public bool AllowNone { get; set; }

/// <summary>
/// A star (*) as a policy name instructs the user agent to allow any unique policy name
/// (<c>'allow-duplicates'</c> may relax that further).
/// </summary>
public bool AllowAny { get; set; }

/// <summary>
/// Instructs user agents to control the data passed to DOM XSS sink functions, like Element.innerHTML setter.
/// Those functions only accept non-spoofable, typed values created by Trusted Type policies, and reject strings.
Copy link
Owner

Choose a reason for hiding this comment

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

This comment could use an MDN link as well.

/// </summary>
public bool RequireTrustedTypesForScript { get; set; }

public CspTrustedTypesOptions()
{
TrustedPolicies = new List<string>();
}

private ICollection<string> GetParts()
{
ICollection<string> parts = new List<string>();

if (AllowNone)
{
parts.Add("'none'");
}
else
{
if (AllowAny)
{
parts.Add("*");
}

foreach (string allowedSource in TrustedPolicies)
{
parts.Add(allowedSource);
}

if (AllowDuplicates)
{
parts.Add("'allow-duplicates'");
}
}
return parts;
}

/// <inheritdoc />
public override string ToString()
{
ICollection<string> parts = GetParts();

if (parts.Count == 0)
{
return string.Empty;
}

var result = "trusted-types " + string.Join(" ", parts);

if (RequireTrustedTypesForScript)
{
result += "; require-trusted-types-for 'script'";
}

return result;
}
}
}
9 changes: 8 additions & 1 deletion src/Joonasw.AspNetCore.SecurityHeaders/CspOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ public class CspOptions
/// Options for controlling what subresource integrity attributes should be required on
/// </summary>
public CspRequireSriOptions RequireSri { get; set; }

/// <summary>
/// Options for controlling how user agents restricts the creation of Trusted Types policies
/// </summary>
public CspTrustedTypesOptions TrustedTypes { get; set; }

/// <summary>
/// The URL where violation reports should be sent.
Expand Down Expand Up @@ -144,6 +149,7 @@ public class CspOptions

public CspOptions()
{
TrustedTypes = new CspTrustedTypesOptions();
Script = new CspScriptSrcOptions();
Style = new CspStyleSrcOptions();
Default = new CspDefaultSrcOptions();
Expand Down Expand Up @@ -200,7 +206,8 @@ public CspOptions()
Worker.ToString(nonceService),
Prefetch.ToString(nonceService),
BaseUri.ToString(nonceService),
RequireSri.ToString()
RequireSri.ToString(),
TrustedTypes.ToString()
};
if (BlockAllMixedContent)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
<!-- /Source Link support-->
</PropertyGroup>
<ItemGroup>
<None Include="..\..\LICENSE.txt" Pack="true" PackagePath="LICENSE.txt"/>
<None Include="..\..\LICENSE.txt" Pack="true" PackagePath="LICENSE.txt" />
</ItemGroup>
<ItemGroup>
<PackageReference Include="System.ValueTuple" Version="4.4.0" />
Expand Down
13 changes: 13 additions & 0 deletions test/Joonasw.AspNetCore.SecurityHeaders.Tests/CspBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,19 @@ public void RequireSriFor_ReturnsCorrectHeader()

Assert.Equal("require-sri-for script", headerValue);
}


[Fact]
public void RequireTrustedTypes_ReturnsCorrectHeader()
Copy link
Owner

Choose a reason for hiding this comment

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

A couple more unit tests would be nice. At least one with a specific policy name and one that sets up the require-trusted-types-for directive.

{
var builder = new CspBuilder();

builder.RequireTrustedTypes.DisallowAll();

var headerValue = builder.BuildCspOptions().ToString(null).headerValue;

Assert.Equal("trusted-types 'none'", headerValue);
}

[Fact]
public async Task OnSendingHeader_ShouldNotSendTest()
Expand Down