Skip to content

Commit

Permalink
[BUGFIX] Extend CSP directives and sources
Browse files Browse the repository at this point in the history
The CSP directives 'report-to', 'require-trusted-types-for' and
'trusted-types' have been added. Albeit there aren't any typed value
counterparts yet, they can be wrapped in a RawValue object, e.g.

new Mutation(
    MutationMode::Set,
    Directive::RequireTrustedTypesFor,
    new RawValue("'script'")
),

The stand-alone directives 'sandbox', 'trusted-types' and
'upgrade-insecure-request' now can be used without any values.
The cases for 'unsafe-hashes' and 'strict-dynamic' were accidentally
added as directives instead of source keywords and have been removed.

The source schemes 'filesystem:' and 'mediastream' have been added.

Besides that, the frontend CSP configuration now limits using the
`<base>` element to same-origin URIs. The backend CSP configuration
is now even stricter since using `<base>`, `<embed>` and `<object>`
elements is blocked.

Resolves: #101477
Releases: main, 12.4
Change-Id: Ie1ce2e30dc0d79faa5b7a923fa39a88dbee17292
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/80208
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
  • Loading branch information
ohader committed Jul 28, 2023
1 parent 74105af commit 07124e2
Show file tree
Hide file tree
Showing 6 changed files with 57 additions and 16 deletions.
Expand Up @@ -34,5 +34,9 @@
new Mutation(MutationMode::Set, Directive::WorkerSrc, SourceKeyword::self, SourceScheme::blob),
// `frame-src blob:` required for es-module-shims blob: URLs
new Mutation(MutationMode::Extend, Directive::FrameSrc, SourceScheme::blob),
// deny `<base>` element which might be used for cross-origin targets
new Mutation(MutationMode::Set, Directive::BaseUri, SourceKeyword::none),
// deny `<object>` and `<embed>` elements
new Mutation(MutationMode::Set, Directive::ObjectSrc, SourceKeyword::none),
),
]);
Expand Up @@ -23,6 +23,12 @@
*/
enum Directive: string
{
private const STAND_ALONE = [
self::Sandbox,
self::TrustedTypes,
self::UpgradeInsecureRequests,
];

case DefaultSrc = 'default-src';
case BaseUri = 'base-uri';
case ChildSrc = 'child-src';
Expand All @@ -37,17 +43,19 @@ enum Directive: string
case ObjectSrc = 'object-src';
// @deprecated (used for Safari, see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/plugin-types)
case PluginTypes = 'plugin-types';
case ReportTo = 'report-to';
// @deprecated (see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/report-uri)
// but `report-uri` is still used for being compatible other older browsers
case ReportUri = 'report-uri';
case RequireTrustedTypesFor = 'require-trusted-types-for';
case Sandbox = 'sandbox';
case ScriptSrc = 'script-src';
case ScriptSrcAttr = 'script-src-attr';
case ScriptSrcElem = 'script-src-elem';
case StrictDynamic = 'strict-dynamic';
case StyleSrc = 'style-src';
case StyleSrcAttr = 'style-src-attr';
case StyleSrcElem = 'style-src-elem';
case UnsafeHashes = 'unsafe-hashes';
case TrustedTypes = 'trusted-types';
case UpgradeInsecureRequests = 'upgrade-insecure-requests';
case WorkerSrc = 'worker-src';

Expand All @@ -56,11 +64,7 @@ enum Directive: string
*/
public function getAncestors(): array
{
$ancestors = self::ancestorMap()[$this] ?? [];
if ($this !== self::DefaultSrc) {
$ancestors[] = self::DefaultSrc;
}
return $ancestors;
return self::ancestorMap()[$this] ?? [];
}

/**
Expand All @@ -72,19 +76,37 @@ public function isMutationReasonable(): bool
return in_array($this, self::reasonableMutationItems(), true);
}

/**
* Determines whether the current directive can be used without any values,
* like for instance `sandbox`, `trusted-types` or `upgrade-insecure-requests`.
*/
public function isStandAlone(): bool
{
return in_array($this, self::STAND_ALONE, true);
}

/**
* @return \WeakMap<self, list<self>>
*/
private static function ancestorMap(): \WeakMap
{
/** @var \WeakMap<self, list<self>> $map temporary, internal \WeakMap */
$map = new \WeakMap();
$map[self::ScriptSrcAttr] = [self::ScriptSrc];
$map[self::ScriptSrcElem] = [self::ScriptSrc];
$map[self::StyleSrcAttr] = [self::StyleSrc];
$map[self::StyleSrcElem] = [self::StyleSrc];
$map[self::FrameSrc] = [self::ChildSrc];
$map[self::WorkerSrc] = [self::ChildSrc, self::ScriptSrc];
$map[self::ChildSrc] = [self::DefaultSrc];
$map[self::ConnectSrc] = [self::DefaultSrc];
$map[self::FontSrc] = [self::DefaultSrc];
$map[self::FrameSrc] = [self::ChildSrc, self::DefaultSrc];
$map[self::ImgSrc] = [self::DefaultSrc];
$map[self::ManifestSrc] = [self::DefaultSrc];
$map[self::MediaSrc] = [self::DefaultSrc];
$map[self::ObjectSrc] = [self::DefaultSrc];
$map[self::ScriptSrc] = [self::DefaultSrc];
$map[self::ScriptSrcAttr] = [self::ScriptSrc, self::DefaultSrc];
$map[self::ScriptSrcElem] = [self::ScriptSrc, self::DefaultSrc];
$map[self::StyleSrc] = [self::DefaultSrc];
$map[self::StyleSrcAttr] = [self::StyleSrc, self::DefaultSrc];
$map[self::StyleSrcElem] = [self::StyleSrc, self::DefaultSrc];
$map[self::WorkerSrc] = [self::ChildSrc, self::ScriptSrc, self::DefaultSrc];
return $map;
}

Expand Down
Expand Up @@ -91,7 +91,7 @@ public function extend(Directive $directive, SourceCollection|SourceInterface ..
{
$collection = $this->asMergedSourceCollection(...$sources);
$collection = $this->purgeNonApplicableSources($directive, $collection);
if ($collection->isEmpty()) {
if ($collection->isEmpty() && !$directive->isStandAlone()) {
return $this;
}
foreach ($directive->getAncestors() as $ancestorDirective) {
Expand Down Expand Up @@ -209,7 +209,7 @@ public function compile(Nonce $nonce, ?FrontendInterface $cache = null): string
$service = GeneralUtility::makeInstance(ModelService::class, $cache);
foreach ($this->prepare()->directives as $directive => $collection) {
$directiveParts = $service->compileSources($nonce, $collection);
if ($directiveParts !== []) {
if ($directiveParts !== [] || $directive->isStandAlone()) {
array_unshift($directiveParts, $directive->value);
$policyParts[] = implode(' ', $directiveParts);
}
Expand Down Expand Up @@ -269,7 +269,7 @@ protected function compareSources(SourceInterface $a, SourceInterface $b): int

protected function changeDirectiveSources(Directive $directive, SourceCollection $sources): self
{
if ($sources->isEmpty()) {
if ($sources->isEmpty() && !$directive->isStandAlone()) {
return $this;
}
$target = clone $this;
Expand Down
Expand Up @@ -25,8 +25,10 @@ enum SourceScheme: string implements SourceInterface
{
case blob = 'blob';
case data = 'data';
case filesystem = 'filesystem';
case http = 'http';
case https = 'https';
case mediastream = 'mediastream';
case ws = 'ws';
case wss = 'wss';
}
Expand Up @@ -105,6 +105,17 @@ public function newDirectiveExtendsDefault(): void
self::assertSame("default-src 'self'; script-src 'self' 'unsafe-inline'", $policy->compile($this->nonce));
}

/**
* @test
*/
public function nonAncestorDirectiveDoesNotExtendDefault(): void
{
$policy = (new Policy(SourceKeyword::self))
->extend(Directive::Sandbox)
->extend(Directive::TrustedTypes);
self::assertSame("default-src 'self'; sandbox; trusted-types", $policy->compile($this->nonce));
}

/**
* @test
*/
Expand Down
Expand Up @@ -26,5 +26,7 @@
new Mutation(MutationMode::Set, Directive::StyleSrcAttr, SourceKeyword::unsafeInline),
// allow `data:` images
new Mutation(MutationMode::Extend, Directive::ImgSrc, SourceScheme::data),
// limits `<base>` element to be use just for same-origin URIs
new Mutation(MutationMode::Set, Directive::BaseUri, SourceKeyword::self),
),
]);

0 comments on commit 07124e2

Please sign in to comment.