Specify behavior in case of malformed policies

[andypaicu]

This PR specifies the behavior when a malformed policy is encountered.
Fixes: #6

I believe there is little risk in breaking a significant number of existing sites that have malformed policy because judging by the metrics in the chrome dev channel, about 0.00015% percent of page views have a malformed policy.

In contrast, I have recently helped investigate an issue where a malformed policy was left undetected and it was only evident because of divergent browser behavior (webcompat/web-bugs#18902).

ccing @shekyan @metromoxie @michaelficarra as people that participated in the discussion in the issue, for any thoughts or feedback.

---

Preview | Diff

[mikewest]

I think the approach here is fine. That said, we should solicit some feedback from other vendors before shipping this in Chrome... Any objections from folks like @ckerschb, @dbates-wk, and @ericlaw1979?

[mikewest]

Spec change LGTM, thanks!

[shekyan]

Wouldn't it make sense to replace malformed policy with most restrictive policy then? E.g. explicitly list all other non-fetch directives?
default-src 'none'; base-uri 'none'; form-action 'none'; navigate-to 'none'; frame-ancestors 'none'; plugin-types; sandbox;

Also, there are non-fetch directives defined in other specs, so making them define most restrictive directive-value and having a mechanism in parent spec to consume it is something to consider doing.

[mikewest]

At the limit, we could just fail to render the page with an onscreen error message that says "OMG! You had a typo in your CSP!" I'm not actually sure that that's helpful (see also draconian XML error messages).

It seems like default-src 'none' is already more than enough to make it clear that something's gone awry.

[michaelficarra]

A malformed CSP should assume the most restrictive variants of each directive. If an adversary is able to cause a server's CSP to become malformed, that shouldn't grant more privilege than the author intended.

On the other hand, a malformed CSP should not restrict anything other than what CSP already has the authority to govern. So completely blocking the loading of the page because of a malformed CSP would be inappropriate.

[mikewest]

A malformed CSP should assume the most restrictive variants of each directive. If an adversary is able to cause a server's CSP to become malformed, that shouldn't grant more privilege than the author intended.

An attacker that can manipulate headers is one that seems outside CSP's threat model. I don't see the goal here as addressing that kind of attack. I see it as helping developers find their typos. Nothing more, but nothing less.

On the other hand, a malformed CSP should not restrict anything other than what CSP already has the authority to govern. So completely blocking the loading of the page because of a malformed CSP would be inappropriate.

I think we agree that a full-page interstitial is inappropriate, even if we disagree about why. :)

[michaelficarra]

@mikewest There's a range of capabilities that can lead to an intentionally malformed CSP. You're right that an adversary who has full control of the response headers is a very powerful one, but an adversary need only flip a random bit (something that could even be done against the ciphertext using a mode like ECB) to cause the CSP to become malformed. I think that that's a threat that should be considered, especially since there's little downside in going into full CSP lock down mode vs just default-src 'none'.

[andypaicu]

I wanted to avoid frame-ancestors since that might make sending console messages to the developer slightly more awkward since the frames does not load in the first place. If the frame does not load it makes it difficult to (for example) filter by frame in the console to identify what sort of error messages cause the issue, a feature which I believe most browsers have. If there is an actual attack we're trying to prevent here and this is not just a convenience for developers than frame-ancestors should be set indeed but I don't really see it that way.

Having a sort of error message like Mike suggests seems to me that it's displaying to the user an error message that is intended for the developer (and for which we should just use the dev console as the rest of CSP does).

plugin-types seems to be covered by object-src 'none' (since it covers embed and object elements), I don't see anything wrong with adding it it just seemed redundant to me. Maybe I was just trying to be too clever.

sandbox makes sense thinking about it by trying to restrict everything as much as possible, I'll add it.

@mikewest There's a range of capabilities that can lead to an intentionally malformed CSP. You're right that an adversary who has full control of the response headers is a very powerful one, but an adversary need only flip a random bit (something that could even be done against the ciphertext using a mode like ECB) to cause the CSP to become malformed. I think that that's a threat that should be considered, especially since there's little downside in going into full CSP lock down mode vs just default-src 'none'.

Are there cryptosystems that are currently used in TLS where an attacker can flip bits while keeping the message valid? I was under the impression that there are validation mechanisms in place to prevent any sort of tampering like this. But of course my knowledge of this is very limited.

Also my goal here is not to only use default-src 'none', I've tried to include all directives that don't interfere with the developers ability to easily identify what the root issue for the broken page is (i.e. frame-ancestors). It seems we're coming at this from slightly different angles, I'm coming at it as something to help developers when they have malformed policies (as the example provided where they used &nbsp instead of an ascii space).

[mikewest]

but an adversary need only flip a random bit

If the attacker can flip a bit in the directive name, they'll "break" the policy's enforcement, but we won't stop them from doing so because forward compatibility. It's not clear that this level of enforcement is a defense against bit-flipping attacks, nor is it clear to me that CSP includes (or should include) that kind of attack in its threat model.

Also my goal here is not to only use default-src 'none'

It seems like Content-Security-Policy: sandbox would be enough, really, as it pushes the content into an opaque origin where it can do little harm, and has less of a visible effect on users (as per your "displaying to the user an error message that is intended for the developer" note above, with which I agree).

[andypaicu]

I've added plugin-types and sandbox

[shekyan]

If we are trying to help developers identify the problem, enforcing most strict directive value for every directive seems to be more helpful.

Accidentally found an example of broken policy[here] to illustrate the benefits of restricting everything, and not some.

[andypaicu]

If we are trying to help developers identify the problem, enforcing most strict directive value for every directive seems to be more helpful.

Accidentally found an example of broken policy[here] to illustrate the benefits of restricting everything, and not some.

I've now restricted every directive except frame-ancestors for which I've explained my reasoning. Does this seem reasonable?

[mikewest]

So, by shipping this, we'd totally break Chase's login page? Which isn't even trying to use CSP for XSS prevention but for framing? And which is actually protected against framing because it's also delivering X-FRAME-OPTIONS: DENY which isn't ignored because the frame-ancestors: typo doesn't trigger the ignoring behavior?

Why is totally breaking the page better than sandboxing it, throwing a console message, and maybe flipping its address bar indicator to "Not Secure"? Why is it good to use users as a cudgel against the sites' developers?

It's entirely reasonable to believe we made a mistake by not handling errors in a draconian fashion ~7 years ago when we shipped CSP1. I'm not sure that it's reasonable to say that we can inject draconian error handling today when we know that we'll break real sites that real people rely on.

@andypaicu: We might need to look at that 0.00015% number more carefully.

[andypaicu]

Ok maybe we should let the metrics reach beta to get more real-life data. The metrics are from the dev channel currently.

But is it not dangerous for Chase that they believe they have a policy that prevents any framing? Sure they got what the want because they also have the X-Frame-Options header but I'm pretty sure they will not be happy when they find out their CSP header is broken and does nothing.

Wouldn't sandboxing it stop the page from working anyway, effectively breaking it? It can't run scripts or have forms, can it still function properly as a login page with an opaque origin? A console message is clearly not enough, they already have a console message telling them the policy in invalid, but they don't care. If we make caveats to allow pages to not break and function normally in the presence of a malformed policy we might as well not do anything at all... this is precisely the current state.

[mikewest]

Wouldn't sandboxing it stop the page from working anyway, effectively breaking it?

That's fair. I forgot that sandboxing disallowed form submission. Let's pretend I said sandbox allow-forms allow-script etc.. :)

If we make caveats to allow pages to not break and function normally in the presence of a malformed policy we might as well not do anything at all... this is precisely the current state.

As you said earlier in the thread, we'd be "displaying to the user an error message that is intended for the developer". It's not clear to me that that's a reasonable goal. If we can reduce the impact of the main risk of a malformed CSP by pushing pages with malformed policies into opaque origins (and thereby denying an XSS them access to the origin's contents and the ability to request data as the origin), we address the central concern, while reducing the likelyhood that we break the page entirely.

Given that we didn't ship this error handling style to begin with, and that there are certainly pages in the wild that would break (and therefore annoy users), what's the justification for jumping to the draconian handling?

[shekyan]

This PR doesn't cover some of the issues raised in #6.
While it is necessary to replace with placeholder every policy that cannot be parsed, we do not cover the actions for malformed directive-value.
Consider replacing the value for specific directive type with most restrictive directive placeholder if

• value doesn't match directive-value grammar
• value doesn't match directive-specific grammar

[mbrodesser-Igalia]

Firefox's and Chrome's behavior differ for a malformed "trusted-types" CSP directive [1].

Check e.g. [2] with Firefox Nightly and the pref "dom.security.trusted_types.enabled" set to true. Be aware that Firefox's implementation is work-in-progress and currently only partially supports Trusted Types.
Firefox will ignore the whole "trusted-types" directive, when it contains an invalid policy name. Chrome ignores only the invalid policy name.
AFAIU the spec, the desired behavior is currently not spec'ed.

CC @otherdaniel @lukewarlow

[1] https://www.w3.org/TR/trusted-types/#trusted-types-csp-directive
[2] https://jsfiddle.net/kar0dgfv/

[lukewarlow]

@mbrodesser-Igalia fwiw WebKit matches Chrome's behaviour (you can test Safari Tech Preview with the Trusted Types flag enabled), and that wasn't a deliberate choice more than just accidental happenstance.

[mozfreddyb]

I think we should discard the specific trusted type policy, not throw away the whole directive.