Require-sri-for speciální CSP pravidlo

[peldax]

• bug fix / new feature - tak trochu obojí
• BC break? no
• doc PR: None ATM

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Security-Policy/require-sri-for

Speciální záznam pro CSP, který ukazuje pro který content má prohlížeč vyžadovat subresource integrity.

csp
    require-sri-for: [ script, style ]

Při použití tohoto pravidla v konfigu nette vloží uvozovky kolem script a style - protože se dávají kolem none, self a dalších klíčovách slov. Bohužel očekávané je nekonzistetní bez uvozovek, chrome uvozovky nezkousne.

Přidal jsem kontrolu zdali se jedná o typ require-sri-for a pokud ano nedají se uvozovky.

[peldax]

Další speciální pravidlo block-all-mixed-content funguje vpořádku při použití konfigurace:

block-all-mixed-content: []

[dg]

Probably sandbox is the same case.

Can you add a test?

[peldax]

I can add a test.

Wouldnt be better to have some kind of whitelist with all the keywords?

[dg]

Maybe yes.

[peldax]

I'll add whitelist of all NON qouted keywords. Nonces and hashes are qouted.

I dont really understand the reasoning behind the specification...

[harmim]

@peldax As you mentioned block-all-mixed-content directive. There are other directives without source list: disown-opener, upgrade-insecure-requests and maybe some others. These directives are quite useful by the way.
The thing is you are able to define these directives in HTTP extension somehow like this:

csp:
    upgrade-insecure-requests:
    block-all-mixed-content: []

and it doesn't make sense to me. It seems like these directives are disabled. More meaningful would be something like this:

csp:
    upgrade-insecure-requests: true
    block-all-mixed-content: true

What do you think about it David? @dg

[peldax]

This would require special section in config or built in list of those "empty" directives.
It would be nice, but:

• it would be possible BC break - some apps already implement current state
• it IS working in current state, this PR intented to fix require-sri-for, which wasnt working at all

[peldax]

@harmim We could create PR with your suggestion or even add it to my PR #142 - which currently by far exceeded its original purpose.

What would you expect to be outcome of:

csp:
    upgrade-insecure-requests: false
    block-all-mixed-content: false

No directive at all?

[harmim]

Regarding require-sri-for.
There are many others directive values which must not be quoted. Of course it could be URL or just a scheme then it could be asterisk symbol *, then data:, mediastream:, blob: and filesystem:. All of these should work right now. But in CSP you can specify referrer directive which accepts these unquoted values no-referrer, no-referrer-when-downgrade, same-origin, origin, strict-origin, origin-when-cross-origin, strict-origin-when-cross-origin and unsafe-url. Then there could be reflected-xss with values allow, block and filter. report-to accepting token. require-sri-for with values script and filter. sandbox with values sandbox, allow-forms, allow-same-origin, allow-scripts, allow-top-navigation, allow-modals, allow-orientation-lock, allow-popups, allow-presentation, allow-popups-to-escape-sandbox, allow-top-navigation and allow-pointer-lock. report-uri is just unquoted URI. plugin-types is unquoted MIME type and so on...
You are not able to whitelist it.

The behaviour should be right opposite. Something like this:

private const CSP_QUOTED_SOURCE_VALUES = ['none', 'self', 'unsafe-inline', 'unsafe-eval', 'nonce', 'sha256', 'strict-dynamic', 'unsafe-hashed-attributes'];

...

$isQuoted = false;
foreach (self::CSP_QUOTED_SOURCE_VALUES as $value) {
    if (\strpos($item, $value) !== false) {
        $isQuoted = true;
        break;
    }
}
$value .= $isQuoted ? " '$item'" : " $item";

[peldax]

@harmim Please see #142

[peldax]

You have to decide to whitelist one list or the other, whatever you decide to whitelist, there is always going to emerge some new keyword which has to be added to the list.

For example hash algorithm doesnt need to be sha256.

[harmim]

What would you expect to be outcome of:

csp:
   upgrade-insecure-requests: false
   block-all-mixed-content: false

I don't know. Maybe no directive at all, as you said.

[peldax]

@harmim I agree, its just one type-sensitive check. I suggest we PR it later after this is resolved. Syntactic sugar can wait :).

[harmim]

Isn't this solution for true/false directives without source list (arguments)?

if (\is_bool($policy)) {
	if ($policy) {
		$value .= $type . '; ';
	}
} else {
	$value .= $type;
	foreach ((array) $policy as $item) {
		$value .= \preg_match('#^[a-z-]+\z#', $item) ? " '$item'" : " $item";
	}
	$value .= '; ';
}

[peldax]

It is, I would just write it differently - without else - with continue;