Skip to content

Commit 9419d94

Browse files
author
Oleksandr Gorkun
committed
MC-19927: Implement hash-whitelisting, dynamic CSP
1 parent e88d507 commit 9419d94

File tree

5 files changed

+451
-57
lines changed

5 files changed

+451
-57
lines changed

‎app/code/Magento/Csp/Api/InlineUtilInterface.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ public function renderTag(string $tagName, array $attributes, ?string $content =
3030
/**
3131
* Render event listener as an HTML attribute and whitelist it as trusted source.
3232
*
33-
* Do not use user-provided as any of the parameters.
33+
* Do not use user-provided values as any of the parameters.
3434
*
35-
* @param string $eventName
35+
* @param string $eventName Full attribute name like "onclick".
3636
* @param string $javascript
3737
* @return string
3838
*/

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

Lines changed: 101 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
use Magento\Csp\Api\InlineUtilInterface;
1111
use Magento\Csp\Model\Collector\DynamicCollector;
1212
use Magento\Csp\Model\Policy\FetchPolicy;
13+
use ParagonIE\Sodium\Core\Curve25519\Fe;
1314

1415
/**
1516
* Helper for classes responsible for rendering and templates.
@@ -26,78 +27,119 @@ class InlineUtil implements InlineUtilInterface
2627
/**
2728
* @var bool
2829
*/
29-
private $eventHandlersEnabled = false;
30+
private $useUnsafeHashes;
31+
32+
private static $tagMeta = [
33+
'script' => ['id' => 'script-src', 'remote' => ['src'], 'hash' => true],
34+
'style' => ['id' => 'style-src', 'remote' => [], 'hash' => true],
35+
'img' => ['id' => 'img-src', 'remote' => ['src']],
36+
'audio' => ['id' => 'media-src', 'remote' => ['src']],
37+
'video' => ['id' => 'media-src', 'remote' => ['src']],
38+
'track' => ['id' => 'media-src', 'remote' => ['src']],
39+
'source' => ['id' => 'media-src', 'remote' => ['src']],
40+
'object' => ['id' => 'object-src', 'remote' => ['data', 'archive']],
41+
'embed' => ['id' => 'object-src', 'remote' => ['src']],
42+
'applet' => ['id' => 'object-src', 'remote' => ['code', 'archive']],
43+
'link' => ['id' => 'style-src', 'remote' => ['href']],
44+
'form' => ['id' => 'form-action', 'remote' => ['action']],
45+
'iframe' => ['id' => 'frame-src', 'remote' => ['src']],
46+
'frame' => ['id' => 'frame-src', 'remote' => ['src']]
47+
];
3048

3149
/**
3250
* @param DynamicCollector $dynamicCollector
51+
* @param bool $useUnsafeHashes Use 'unsafe-hashes' policy (not supported by CSP v2).
3352
*/
34-
public function __construct(DynamicCollector $dynamicCollector)
53+
public function __construct(DynamicCollector $dynamicCollector, bool $useUnsafeHashes = false)
3554
{
3655
$this->dynamicCollector = $dynamicCollector;
56+
$this->useUnsafeHashes = $useUnsafeHashes;
3757
}
3858

3959
/**
4060
* Generate fetch policy hash for some content.
4161
*
4262
* @param string $content
43-
* @return string
63+
* @return array Hash data to insert into a FetchPolicy.
64+
*/
65+
private function generateHashValue(string $content): array
66+
{
67+
return [base64_encode(hash('sha256', $content, true)) => 'sha256'];
68+
}
69+
70+
/**
71+
* Extract host for a fetch policy from a URL.
72+
*
73+
* @param string $url
74+
* @return string|null Null is returned when URL does not point to a remote host.
75+
*/
76+
private function extractHost(string $url): ?string
77+
{
78+
$urlData = parse_url($url);
79+
if (
80+
!$urlData
81+
|| empty($urlData['scheme'])
82+
|| ($urlData['scheme'] !== 'http' && $urlData['scheme'] !== 'https')
83+
) {
84+
return null;
85+
}
86+
87+
return $urlData['scheme'] .'://' .$urlData['host'];
88+
}
89+
90+
/**
91+
* Extract remote hosts used to get fonts.
92+
*
93+
* @param string $styleContent
94+
* @return string[]
4495
*/
45-
private function generateHash(string $content): string
96+
private function extractRemoteFonts(string $styleContent): array
4697
{
47-
return 'sha256-' .base64_encode(hash('sha256', $content, true));
98+
$urlsFound = [[]];
99+
preg_match_all('/\@font\-face\s*?\{([^\}]*)[^\}]*?\}/im', $styleContent, $fontFaces);
100+
foreach ($fontFaces[1] as $fontFaceContent) {
101+
preg_match_all('/url\((http(s)?\:[^\)]+)\)/i', $fontFaceContent, $urls);
102+
$urlsFound[] = $urls[1];
103+
}
104+
105+
return array_map([$this, 'extractHost'], array_merge(...$urlsFound));
48106
}
49107

50108
/**
51109
* @inheritDoc
52110
*/
53111
public function renderTag(string $tagName, array $attributes, ?string $content = null): string
54112
{
55-
$remote = !empty($attributes['src'])
56-
? $attributes['src'] : (!empty($attributes['href']) ? $attributes['href'] : null);
57-
if (!$remote && !$content) {
58-
throw new \InvalidArgumentException('Either remote URL or hashable content is required to whitelist');
113+
if (!array_key_exists($tagName, self::$tagMeta)) {
114+
throw new \InvalidArgumentException('Unknown source type - ' .$tagName);
59115
}
60-
switch ($tagName) {
61-
case 'script':
62-
$policyId = 'script-src';
63-
break;
64-
case 'style':
65-
$policyId = 'style-src';
66-
break;
67-
case 'img':
68-
$policyId = 'img-src';
116+
/** @var string[] $remotes */
117+
$remotes = [];
118+
foreach (self::$tagMeta[$tagName]['remote'] as $remoteAttr) {
119+
if (!empty($attributes[$remoteAttr]) && $host = $this->extractHost($attributes[$remoteAttr])) {
120+
$remotes[] = $host;
69121
break;
70-
case 'audio':
71-
case 'video':
72-
case 'track':
73-
$policyId = 'media-src';
74-
break;
75-
case 'object':
76-
case 'embed':
77-
case 'applet':
78-
$policyId = 'object-src';
79-
break;
80-
case 'link':
81-
if (empty($attributes['rel']) || $attributes['rel'] !== 'stylesheet') {
82-
throw new \InvalidArgumentException('Only remote styles can be whitelisted via "link" tag');
83-
}
84-
$policyId = 'style-src';
85-
break;
86-
default:
87-
throw new \InvalidArgumentException('Unknown source type - ' .$tagName);
122+
}
123+
}
124+
/** @var string $policyId */
125+
$policyId = self::$tagMeta[$tagName]['id'];
126+
if ($tagName === 'style' && $content) {
127+
$remotes += $this->extractRemoteFonts($content);
88128
}
89129

90-
if ($remote) {
91-
$urlData = parse_url($remote);
130+
if (empty($remotes) && !$content) {
131+
throw new \InvalidArgumentException('Either remote URL or hashable content is required to whitelist');
132+
}
133+
134+
if ($remotes) {
92135
$this->dynamicCollector->add(
93-
new FetchPolicy($policyId, false, [$urlData['scheme'] .'://' .$urlData['host']])
136+
new FetchPolicy($policyId, false, $remotes)
94137
);
95-
} elseif ($policyId === 'style-src' || $policyId === 'script-src') {
138+
}
139+
if ($content && !empty(self::$tagMeta[$tagName]['hash'])) {
96140
$this->dynamicCollector->add(
97-
new FetchPolicy($policyId, false, [], [], false, false, false, [], [$this->generateHash($content)])
141+
new FetchPolicy($policyId, false, [], [], false, false, false, [], $this->generateHashValue($content))
98142
);
99-
} else {
100-
throw new \InvalidArgumentException('Only inline scripts and styles can be whitelisted');
101143
}
102144

103145
$html = '<' .$tagName;
@@ -118,16 +160,24 @@ public function renderTag(string $tagName, array $attributes, ?string $content =
118160
*/
119161
public function renderEventListener(string $eventName, string $javascript): string
120162
{
121-
if (!$this->eventHandlersEnabled) {
122-
$this->dynamicCollector->add(
123-
new FetchPolicy('default-src', false, [], [], false, false, false, [], [], false, true)
163+
if ($this->useUnsafeHashes) {
164+
$policy = new FetchPolicy(
165+
'script-src',
166+
false,
167+
[],
168+
[],
169+
false,
170+
false,
171+
false,
172+
[],
173+
$this->generateHashValue($javascript),
174+
false,
175+
true
124176
);
125-
$this->eventHandlersEnabled = true;
177+
} else {
178+
$policy = new FetchPolicy('script-src', false, [], [], false, true);
126179
}
127-
128-
$this->dynamicCollector->add(
129-
new FetchPolicy('script-src', false, [], [], false, false, false, [], [$this->generateHash($javascript)])
130-
);
180+
$this->dynamicCollector->add($policy);
131181

132182
return $eventName .'="' .$javascript .'"';
133183
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,10 @@
1818
<type name="Magento\Csp\Model\CompositePolicyCollector">
1919
<arguments>
2020
<argument name="collectors" xsi:type="array">
21-
<item name="config" xsi:type="object">Magento\Csp\Model\Collector\ConfigCollector</item>
22-
<item name="csp_whitelist" xsi:type="object">Magento\Csp\Model\Collector\CspWhitelistXmlCollector</item>
23-
<item name="dynamic" xsi:type="object">Magento\Csp\Model\Collector\DynamicCollector</item>
24-
<item name="controller" xsi:type="object">Magento\Csp\Model\Collector\ControllerCollector</item>
21+
<item name="1" xsi:type="object">Magento\Csp\Model\Collector\ConfigCollector</item>
22+
<item name="2" xsi:type="object">Magento\Csp\Model\Collector\CspWhitelistXmlCollector</item>
23+
<item name="3" xsi:type="object">Magento\Csp\Model\Collector\DynamicCollector</item>
24+
<item name="100" xsi:type="object">Magento\Csp\Model\Collector\ControllerCollector</item>
2525
</argument>
2626
<argument name="mergers" xsi:type="array">
2727
<item name="fetch" xsi:type="object">Magento\Csp\Model\Collector\FetchPolicyMerger</item>

0 commit comments

Comments
 (0)