Skip to content

Commit 184d024

Browse files
author
Oleksandr Gorkun
committed
MC-19927: Implement hash-whitelisting, dynamic CSP
1 parent 3ff82c4 commit 184d024

File tree

7 files changed

+66
-27
lines changed

7 files changed

+66
-27
lines changed

‎app/code/Magento/Csp/Helper/InlineUtil.php

Lines changed: 50 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -75,9 +75,9 @@ private function generateHashValue(string $content): array
7575
*/
7676
private function extractHost(string $url): ?string
7777
{
78+
// phpcs:ignore Magento2.Functions.DiscouragedFunction
7879
$urlData = parse_url($url);
79-
if (
80-
!$urlData
80+
if (!$urlData
8181
|| empty($urlData['scheme'])
8282
|| ($urlData['scheme'] !== 'http' && $urlData['scheme'] !== 'https')
8383
) {
@@ -106,31 +106,69 @@ private function extractRemoteFonts(string $styleContent): array
106106
}
107107

108108
/**
109-
* @inheritDoc
109+
* Extract remote hosts utilized.
110+
*
111+
* @param string $tag
112+
* @param string|null $content
113+
* @return string[]
110114
*/
111-
public function renderTag(string $tagName, array $attributes, ?string $content = null): string
115+
private function extractRemoteHosts(string $tag, ?string $content): array
112116
{
113-
if (!array_key_exists($tagName, self::$tagMeta)) {
114-
throw new \InvalidArgumentException('Unknown source type - ' .$tagName);
115-
}
116117
/** @var string[] $remotes */
117118
$remotes = [];
118-
foreach (self::$tagMeta[$tagName]['remote'] as $remoteAttr) {
119+
foreach (self::$tagMeta[$tag]['remote'] as $remoteAttr) {
119120
if (!empty($attributes[$remoteAttr]) && $host = $this->extractHost($attributes[$remoteAttr])) {
120121
$remotes[] = $host;
121122
break;
122123
}
123124
}
124-
/** @var string $policyId */
125-
$policyId = self::$tagMeta[$tagName]['id'];
126-
if ($tagName === 'style' && $content) {
125+
if ($tag === 'style' && $content) {
127126
$remotes += $this->extractRemoteFonts($content);
128127
}
129128

129+
return $remotes;
130+
}
131+
132+
/**
133+
* Render tag.
134+
*
135+
* @param string $tag
136+
* @param string[] $attributes
137+
* @param string|null $content
138+
* @return string
139+
*/
140+
private function render(string $tag, array $attributes, ?string $content): string
141+
{
142+
$html = '<' .$tag;
143+
foreach ($attributes as $attribute => $value) {
144+
$html .= ' ' .$attribute .'="' .$value .'"';
145+
}
146+
if ($content) {
147+
$html .= '>' .$content .'</' .$tag .'>';
148+
} else {
149+
$html .= ' />';
150+
}
151+
152+
return $html;
153+
}
154+
155+
/**
156+
* @inheritDoc
157+
*/
158+
public function renderTag(string $tagName, array $attributes, ?string $content = null): string
159+
{
160+
//Processing tag data
161+
if (!array_key_exists($tagName, self::$tagMeta)) {
162+
throw new \InvalidArgumentException('Unknown source type - ' .$tagName);
163+
}
164+
/** @var string $policyId */
165+
$policyId = self::$tagMeta[$tagName]['id'];
166+
$remotes = $this->extractRemoteHosts($tagName, $content);
130167
if (empty($remotes) && !$content) {
131168
throw new \InvalidArgumentException('Either remote URL or hashable content is required to whitelist');
132169
}
133170

171+
//Adding required policies.
134172
if ($remotes) {
135173
$this->dynamicCollector->add(
136174
new FetchPolicy($policyId, false, $remotes)
@@ -142,17 +180,8 @@ public function renderTag(string $tagName, array $attributes, ?string $content =
142180
);
143181
}
144182

145-
$html = '<' .$tagName;
146-
foreach ($attributes as $attribute => $value) {
147-
$html .= ' ' .$attribute .'="' .$value .'"';
148-
}
149-
if ($content) {
150-
$html .= '>' .$content .'</' .$tagName .'>';
151-
} else {
152-
$html .= ' />';
153-
}
154183

155-
return $html;
184+
return $this->render($tagName, $attributes, $content);
156185
}
157186

158187
/**

‎app/code/Magento/Csp/Plugin/CspAwareControllerPlugin.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ public function __construct(ControllerCollector $collector)
3636
* @param RouterInterface $router
3737
* @param ActionInterface|null $matched
3838
* @return ActionInterface|null
39+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
3940
*/
4041
public function afterMatch(RouterInterface $router, ?ActionInterface $matched): ?ActionInterface
4142
{

‎app/code/Magento/Csp/Plugin/PhtmlRenderingPlugin.php renamed to ‎app/code/Magento/Csp/Plugin/TemplateRenderingPlugin.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,9 @@
1212
use Magento\Framework\View\TemplateEngine\Php;
1313

1414
/**
15-
* Plugin that adds CSP utility to .phtml templates context.
15+
* Plugin that adds CSP utility to templates context.
1616
*/
17-
class PhtmlRenderingPlugin
17+
class TemplateRenderingPlugin
1818
{
1919
/**
2020
* @var InlineUtilInterface
@@ -37,6 +37,7 @@ public function __construct(InlineUtilInterface $util)
3737
* @param string $fileName
3838
* @param array $dictionary
3939
* @return array
40+
* @SuppressWarnings(PHPMD.UnusedFormalParameter)
4041
*/
4142
public function beforeRender(Php $renderer, BlockInterface $block, $fileName, array $dictionary): array
4243
{

‎app/code/Magento/Csp/etc/di.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@
5050
</arguments>
5151
</type>
5252
<type name="Magento\Framework\View\TemplateEngine\Php">
53-
<plugin name="csp_helper_plugin" type="Magento\Csp\Plugin\PhtmlRenderingPlugin" />
53+
<plugin name="csp_helper_plugin" type="Magento\Csp\Plugin\TemplateRenderingPlugin" />
5454
</type>
5555
<type name="Magento\Framework\App\RouterInterface">
5656
<plugin name="csp_aware_plugin" type="Magento\Csp\Plugin\CspAwareControllerPlugin" />

‎dev/tests/integration/_files/Magento/TestModuleCspUtil/Controller/Csp/Aware.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ public function modifyCsp(array $appliedPolicies): array
3232
{
3333
$policies = [];
3434
foreach ($appliedPolicies as $policy) {
35-
if ($policy instanceof FetchPolicy && in_array('http://controller.magento.com', $policy->getHostSources(), true)) {
35+
if ($policy instanceof FetchPolicy
36+
&& in_array('http://controller.magento.com', $policy->getHostSources(), true)
37+
) {
3638
$policies[] = new FetchPolicy(
3739
'script-src',
3840
false,

‎dev/tests/integration/_files/Magento/TestModuleCspUtil/view/frontend/templates/helper.phtml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,12 @@
11
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
27
/** @var \Magento\Csp\Api\InlineUtilInterface $csp */
38
?>
49
<h1>Hello there!</h1>
5-
<?= $csp->renderTag('script', ['src' => 'http://my.magento.com/static/script.js']); ?>
6-
<?= $csp->renderTag('script', [], "\n let myVar = 1;\n") ?>
10+
<?= /* @noEscape */ $csp->renderTag('script', ['src' => 'http://my.magento.com/static/script.js']); ?>
11+
<?= /* @noEscape */ $csp->renderTag('script', [], "\n let myVar = 1;\n") ?>
712

‎dev/tests/integration/testsuite/Magento/Csp/Helper/InlineUtilTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ public function testRenderTag(
7070
* Test data for tag rendering test.
7171
*
7272
* @return array
73+
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
7374
*/
7475
public function getTags(): array
7576
{

0 commit comments

Comments
 (0)